Re: [PATCH V3] Revert "PCI: tegra194: Enable support for 256 Byte payload"

[Vidya Sagar]

On 7/18/2023 4:39 PM, Bjorn Helgaas wrote:
> External email: Use caution opening links or attachments
>
>
> On Tue, Jul 18, 2023 at 08:03:47AM +0530, Vidya Sagar wrote:
> > On 7/14/2023 3:09 AM, Bjorn Helgaas wrote:
> > > On Mon, Jun 19, 2023 at 03:56:04PM +0530, Vidya Sagar wrote:
> > > > This reverts commit 4fb8e46c1bc4 ("PCI: tegra194: Enable
> > > > support for 256 Byte payload").
> > > >
> > > > Consider a PCIe hierarchy with a PCIe switch and a device connected
> > > > downstream of the switch that has support for MPS which is the minimum in
> > > > the hierarchy, and root port programmed with an MPS in its DevCtl register
> > > > that is greater than the minimum. In this scenario, the default bus
> > > > configuration of the kernel i.e. "PCIE_BUS_DEFAULT" doesn't configure the
> > > > MPS settings in the hierarchy correctly resulting in the device with
> > > > support for minimum MPS in the hierarchy receiving the TLPs of size more
> > > > than that. Although this can be addressed by appending "pci=pcie_bus_safe"
> > > > to the kernel command line, it doesn't seem to be a good idea to always
> > > > have this commandline argument even for the basic functionality to work.
> > >
> > > I think this has some irrelevant detail (IIUC the problem should
> > > happen even without a switch) and could be more specific (I think the
> > > problem case is RP MPS=256, EP only supports MPS=128).
> >
> > The issue is present only if there is a switch.
>
> So if there's no switch, and an EP that only supports MPS=128, the PCI
> core changes the RP MPS setting to 128?  Just based on reading the
Yes. The code after the if condition here takes care of that.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pci/probe.c?h=v6.5-rc2#n2049

> code, I thought we would leave RP MPS=256 and EP MPS=128, which would
> be a problem.  But maybe the PCI core changes the RP down to MPS=128?
>
> Bjorn