[+cc Edward, Martin (SFC maintainers), Ben, Keith (just FYI)]
On Thu, Jun 24, 2021 at 07:14:17PM +0200, Marek Behún wrote:
> The pci_device_add() function calls header fixups only after
> pci_configure_device(), which configures MPS.
This makes good sense; the call graph looks like:
pci_device_add
pci_configure_device
pci_configure_mps
pcie_get_mps(dev)
pcie_get_mps(bridge)
+ pcie_set_mps(dev) # added by 27d868b5e6cfa
pci_fixup_device(pci_fixup_header)
> So in order to have MPS fixups working, they need to be called early.
>
> Signed-off-by: Marek Behún <kabel@xxxxxxxxxx>
> Fixes: 27d868b5e6cfa ("PCI: Set MPS to match upstream bridge")
Before 27d868b5e6cfa, pci_configure_device() really didn't *do*
anything [1]. It read the MPS settings from the device and upstream
bridge and possibly printed a warning, but didn't change anything.
After 27d868b5e6cfa, pci_configure_device() did actually call
pcie_set_mps(), which updates the Device Control register (possibly
restricted by dev->pcie_mpss, which is set by this quirk).
The fixup_mpss_256() quirk was added in 2011 by a94d072b2023 ("PCI:
Add quirk for known incorrect MPSS"). Interesting that 27d868b5e6cfa
was merged in 2015 but apparently nobody noticed until now. I guess
those Solarflare devices aren't widely used?
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pci/probe.c?id=27d868b5e6cfa^#n1278
> Cc: stable@xxxxxxxxxxxxxxx
> ---
> drivers/pci/quirks.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 22b2bb1109c9..4d9b9d8fbc43 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -3233,12 +3233,12 @@ static void fixup_mpss_256(struct pci_dev *dev)
> {
> dev->pcie_mpss = 1; /* 256 bytes */
> }
> -DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SOLARFLARE,
> - PCI_DEVICE_ID_SOLARFLARE_SFC4000A_0, fixup_mpss_256);
> -DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SOLARFLARE,
> - PCI_DEVICE_ID_SOLARFLARE_SFC4000A_1, fixup_mpss_256);
> -DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SOLARFLARE,
> - PCI_DEVICE_ID_SOLARFLARE_SFC4000B, fixup_mpss_256);
> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_SOLARFLARE,
> + PCI_DEVICE_ID_SOLARFLARE_SFC4000A_0, fixup_mpss_256);
> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_SOLARFLARE,
> + PCI_DEVICE_ID_SOLARFLARE_SFC4000A_1, fixup_mpss_256);
> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_SOLARFLARE,
> + PCI_DEVICE_ID_SOLARFLARE_SFC4000B, fixup_mpss_256);
>
> /*
> * Intel 5000 and 5100 Memory controllers have an erratum with read completion
> --
> 2.31.1
>
