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

Vidya Sagar
2023-07-19T11:01:38Z

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