Hi, Jiaxun, On Tue, Aug 8, 2023 at 3:38 PM Jiaxun Yang <jiaxun.yang@xxxxxxxxxxx> wrote: > > > > 在 2023/8/6 22:30, Huacai Chen 写道: > > Hi, Jiaxun, > > > > On Sun, Aug 6, 2023 at 10:20 AM Jiaxun Yang <jiaxun.yang@xxxxxxxxxxx> wrote: > >> > >> > >> 在 2023/7/25 14:10, Jiaxun Yang 写道: > >>> This is a partial revert of commit 8b3517f88ff2 ("PCI: > >>> loongson: Prevent LS7A MRRS increases") for MIPS based Loongson. > >>> > >>> There are many MIPS based Loongson systems in wild that > >>> shipped with firmware which does not set maximum MRRS properly. > >>> > >>> Limiting MRRS to 256 for all as MIPS Loongson comes with higher > >>> MRRS support is considered rare. > >>> > >>> Cc: stable@xxxxxxxxxxxxxxx > >>> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217680 > >>> Fixes: 8b3517f88ff2 ("PCI: loongson: Prevent LS7A MRRS increases") > >>> Signed-off-by: Jiaxun Yang <jiaxun.yang@xxxxxxxxxxx> > >> Ping? > >> I expect this patch to go through PCI fixes tree. > > Can we do it like this by modifying the existing loongson_mrrs_quirk()? > > Hmm, I'm not sure this will work, since loongson_mrrs_quirk only run on > bridges But it is worth to try, and you can walk the children to set mrrs when the quirk runs on bridges, I think. > but the old quirk should run on every single device. Your current patch has a drawback that both quirks will run for MIPS, and their order is random (though it may cause nothing, but not elegant). Huacai > > Thanks > Jiaxun > > > > > static void loongson_mrrs_quirk(struct pci_dev *pdev) > > { > > /* > > * Some Loongson PCIe ports have h/w limitations of maximum read > > * request size. They can't handle anything larger than this. So > > * force this limit on any devices attached under these ports. > > */ > > struct pci_host_bridge *bridge = pci_find_host_bridge(pdev->bus); > > > > #ifdef CONFIG_MIPS > > set_pcie_ports_to_mrrs_256_to_emulate_the_firmware_behavior(); > > #endif > > > > bridge->no_inc_mrrs = 1; > > } > > > >> Thanks > >> - Jiaxun > >> > >>> --- > >>> v2: Rename quirk name to: loongson_old_mrrs_quirk > >>> --- > >>> drivers/pci/controller/pci-loongson.c | 38 +++++++++++++++++++++++++++ > >>> 1 file changed, 38 insertions(+) > >>> > >>> diff --git a/drivers/pci/controller/pci-loongson.c b/drivers/pci/controller/pci-loongson.c > >>> index fe0f732f6e43..d0f68b102d10 100644 > >>> --- a/drivers/pci/controller/pci-loongson.c > >>> +++ b/drivers/pci/controller/pci-loongson.c > >>> @@ -108,6 +108,44 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON, > >>> DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON, > >>> DEV_LS7A_PCIE_PORT6, loongson_mrrs_quirk); > >>> > >>> +#ifdef CONFIG_MIPS > >>> +static void loongson_old_mrrs_quirk(struct pci_dev *pdev) > >>> +{ > >>> + struct pci_bus *bus = pdev->bus; > >>> + struct pci_dev *bridge; > >>> + static const struct pci_device_id bridge_devids[] = { > >>> + { PCI_VDEVICE(LOONGSON, DEV_LS2K_PCIE_PORT0) }, > >>> + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT0) }, > >>> + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT1) }, > >>> + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT2) }, > >>> + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT3) }, > >>> + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT4) }, > >>> + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT5) }, > >>> + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT6) }, > >>> + { 0, }, > >>> + }; > >>> + > >>> + /* look for the matching bridge */ > >>> + while (!pci_is_root_bus(bus)) { > >>> + bridge = bus->self; > >>> + bus = bus->parent; > >>> + /* > >>> + * There are still some wild MIPS Loongson firmware won't > >>> + * set MRRS properly. Limiting MRRS to 256 as MIPS Loongson > >>> + * comes with higher MRRS support is considered rare. > >>> + */ > >>> + if (pci_match_id(bridge_devids, bridge)) { > >>> + if (pcie_get_readrq(pdev) > 256) { > >>> + pci_info(pdev, "limiting MRRS to 256\n"); > >>> + pcie_set_readrq(pdev, 256); > >>> + } > >>> + break; > >>> + } > >>> + } > >>> +} > >>> +DECLARE_PCI_FIXUP_ENABLE(PCI_ANY_ID, PCI_ANY_ID, loongson_old_mrrs_quirk); > >>> +#endif > >>> + > >>> static void loongson_pci_pin_quirk(struct pci_dev *pdev) > >>> { > >>> pdev->pin = 1 + (PCI_FUNC(pdev->devfn) & 3); >