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: [email protected]
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217680
Fixes: 8b3517f88ff2 ("PCI: loongson: Prevent LS7A MRRS increases")
Signed-off-by: Jiaxun Yang <[email protected]>
---
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);
--
2.39.2
在 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: [email protected]
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217680
> Fixes: 8b3517f88ff2 ("PCI: loongson: Prevent LS7A MRRS increases")
> Signed-off-by: Jiaxun Yang <[email protected]>
Ping?
I expect this patch to go through PCI fixes tree.
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);
Hi, Jiaxun,
On Sun, Aug 6, 2023 at 10:20 AM Jiaxun Yang <[email protected]> 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: [email protected]
> > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217680
> > Fixes: 8b3517f88ff2 ("PCI: loongson: Prevent LS7A MRRS increases")
> > Signed-off-by: Jiaxun Yang <[email protected]>
>
> Ping?
> I expect this patch to go through PCI fixes tree.
Can we do it like this by modifying the existing loongson_mrrs_quirk()?
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);
>
在 2023/8/6 22:30, Huacai Chen 写道:
> Hi, Jiaxun,
>
> On Sun, Aug 6, 2023 at 10:20 AM Jiaxun Yang <[email protected]> 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: [email protected]
>>> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217680
>>> Fixes: 8b3517f88ff2 ("PCI: loongson: Prevent LS7A MRRS increases")
>>> Signed-off-by: Jiaxun Yang <[email protected]>
>> 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 the old quirk should run on every single device.
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);
Hi, Thorsten here, the Linux kernel's regression tracker. Top-posting
for once, to make this easily accessible to everyone.
What's the status here? From my point it looks like this regression that
was ported six weeks ago is still not fixed -- and it seems nothing has
happened since three weeks now. But maybe I'm just missing something,
that's why I'm asking.
Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
If I did something stupid, please tell me, as explained on that page.
#regzbot poke
On 08.08.23 09:38, Jiaxun Yang wrote:
>
>
> 在 2023/8/6 22:30, Huacai Chen 写道:
>> Hi, Jiaxun,
>>
>> On Sun, Aug 6, 2023 at 10:20 AM Jiaxun Yang <[email protected]>
>> 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: [email protected]
>>>> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217680
>>>> Fixes: 8b3517f88ff2 ("PCI: loongson: Prevent LS7A MRRS increases")
>>>> Signed-off-by: Jiaxun Yang <[email protected]>
>>> 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 the old quirk should run on every single device.
>
> 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);
>
On Thu, Aug 31, 2023 at 4:58 PM Jiaxun Yang <[email protected]> wrote:
>
>
>
> 在 2023/8/31 14:22, Huacai Chen 写道:
>
> [...]
> > But it is worth to try, and you can walk the children to set mrrs when
> > the quirk runs on bridges, I think.
> No, this will break hotplug, I had managed to get hotplug work on 2K1000.
> Also we have no guarantee on order of discovering devices.
> >> 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).
> Actually loongson_mrrs_quirk is declared by DECLARE_PCI_FIXUP_EARLY but
> loongson_old_mrrs_quirk is declared by DECLARE_PCI_FIXUP_ENABLE,
> which means loongson_old_mrrs_quirk always runs after loongson_mrrs_quirk as
> expected.
Then I think it is better to put this quirk in arch/mips/loongson64/,
just as pci_fixup_radeon().
Huacai
>
> Thanks
> - Jiaxun
>
> [...]
在 2023/8/31 14:22, Huacai Chen 写道:
[...]
> But it is worth to try, and you can walk the children to set mrrs when
> the quirk runs on bridges, I think.
No, this will break hotplug, I had managed to get hotplug work on 2K1000.
Also we have no guarantee on order of discovering devices.
>> 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).
Actually loongson_mrrs_quirk is declared by DECLARE_PCI_FIXUP_EARLY but
loongson_old_mrrs_quirk is declared by DECLARE_PCI_FIXUP_ENABLE,
which means loongson_old_mrrs_quirk always runs after loongson_mrrs_quirk as
expected.
Thanks
- Jiaxun
[...]
在 2023/8/31 17:42, Huacai Chen 写道:
> On Thu, Aug 31, 2023 at 4:58 PM Jiaxun Yang <[email protected]> wrote:
>>
>>
>> 在 2023/8/31 14:22, Huacai Chen 写道:
>>
>> [...]
>>> But it is worth to try, and you can walk the children to set mrrs when
>>> the quirk runs on bridges, I think.
>> No, this will break hotplug, I had managed to get hotplug work on 2K1000.
>> Also we have no guarantee on order of discovering devices.
>>>> 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).
>> Actually loongson_mrrs_quirk is declared by DECLARE_PCI_FIXUP_EARLY but
>> loongson_old_mrrs_quirk is declared by DECLARE_PCI_FIXUP_ENABLE,
>> which means loongson_old_mrrs_quirk always runs after loongson_mrrs_quirk as
>> expected.
> Then I think it is better to put this quirk in arch/mips/loongson64/,
> just as pci_fixup_radeon().
Hmm, spiting two quirks with similar functionality into two places
doesn't sound
like a good idea.
PCI folks, what do you think?
Thanks.
- Jiaxun
[...]
[CCing the regression list, as it should be in the loop for regressions:
https://docs.kernel.org/admin-guide/reporting-regressions.html]
On 07.09.23 07:08, Manivannan Sadhasivam wrote:
> On Thu, Sep 07, 2023 at 11:13:00AM +0800, Jiaxun Yang wrote:
>> 在 2023/9/7 9:18, Manivannan Sadhasivam 写道:
>> [...]
>>> Why do you need to walk through every single device instead of just bridges?
>>> I'm not the maintainer, but my suggestion is to go for Huacai Chen's solution.
>>
>> Thanks for your reply, unfortunately Huacai's solution is impractical in
>> this case.
>>
>> The problem we have, is firmware (or BIOS) setting improper MRRS for devices
>> attached under those bridges. So we have to fix up MRRS for every single
>> device.
>> We can't iterate child device in bridge quirk because there is no guarantee
>> that
>> bridge will be probed before it's child device, partly due to hotplug.
>
> Okay, this clarifies and also warrants improvement in commit message.
>
> You could also use pci_walk_bus() after pci_host_probe() to iterate over the
> child devices under root bridge and set MRRS. IMO that would look neat.
Hi, Thorsten here, the Linux kernel's regression tracker. What's the
status here? The regression that was supposed to be fixed by the patched
that started this thread was reported 9 weeks ago[1] and the culprit
made it to many stable kernels as well. Would be really good to finally
fix this, as a regression like this should ideally be fixed within 2 to
3 weeks (in both mainline and stable). With a revert if necessary -- is
this maybe still a option, or would that cause more trouble then it
solved (I guess that's the case).
[1] https://bugzilla.kernel.org/show_bug.cgi?id=217680
Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
If I did something stupid, please tell me, as explained on that page.
>> This quirk has been in tree for a while, until Huacai refactored it and
>> broke some
>> systems in 8b3517f88ff2 ("PCI: loongson: Prevent LS7A MRRS increases").
>>
>> Also to note that ks_pcie_quirk in drivers/pci/controller/dwc/pci-keystone.c
>> uses similar approach.
>>> This avoids iterating over bridges/devices two times.
>>>
>>> Also, please rename firmware to BIOS, as firmware commonly represents the
>>> software running on PCIe endpoint devices.
>> Ack, will fix in next reversion.
>>
>> Thanks
>> - Jiaxun
>>>
>>> - Mani
>> [...]
>
Hi, Thorsten here, the Linux kernel's regression tracker. Top-posting
for once, to make this easily accessible to everyone.
Just wondering what's up here. The patch this thread is about afaics was
supposed to fix a regression reported in July
(https://bugzilla.kernel.org/show_bug.cgi?id=217680 ), but has made not
steps closer to get mainlined during the past few weeks. Is there a
reason, or did it maybe fell through the cracks?
Jiaxun Yang, from it quick look it seems like you wanted to post a v3,
but never did so; but I might be mistaken there.
Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
If I did something stupid, please tell me, as explained on that page.
#regzbot poke
On 20.09.23 14:33, Linux regression tracking (Thorsten Leemhuis) wrote:
> [CCing the regression list, as it should be in the loop for regressions:
> https://docs.kernel.org/admin-guide/reporting-regressions.html]
>
> On 07.09.23 07:08, Manivannan Sadhasivam wrote:
>> On Thu, Sep 07, 2023 at 11:13:00AM +0800, Jiaxun Yang wrote:
>>> 在 2023/9/7 9:18, Manivannan Sadhasivam 写道:
>>> [...]
>>>> Why do you need to walk through every single device instead of just bridges?
>>>> I'm not the maintainer, but my suggestion is to go for Huacai Chen's solution.
>>>
>>> Thanks for your reply, unfortunately Huacai's solution is impractical in
>>> this case.
>>>
>>> The problem we have, is firmware (or BIOS) setting improper MRRS for devices
>>> attached under those bridges. So we have to fix up MRRS for every single
>>> device.
>>> We can't iterate child device in bridge quirk because there is no guarantee
>>> that
>>> bridge will be probed before it's child device, partly due to hotplug.
>>
>> Okay, this clarifies and also warrants improvement in commit message.
>>
>> You could also use pci_walk_bus() after pci_host_probe() to iterate over the
>> child devices under root bridge and set MRRS. IMO that would look neat.
>
> Hi, Thorsten here, the Linux kernel's regression tracker. What's the
> status here? The regression that was supposed to be fixed by the patched
> that started this thread was reported 9 weeks ago[1] and the culprit
> made it to many stable kernels as well. Would be really good to finally
> fix this, as a regression like this should ideally be fixed within 2 to
> 3 weeks (in both mainline and stable). With a revert if necessary -- is
> this maybe still a option, or would that cause more trouble then it
> solved (I guess that's the case).
>
> [1] https://bugzilla.kernel.org/show_bug.cgi?id=217680
>
> Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
> --
> Everything you wanna know about Linux kernel regression tracking:
> https://linux-regtracking.leemhuis.info/about/#tldr
> If I did something stupid, please tell me, as explained on that page.
>
>>> This quirk has been in tree for a while, until Huacai refactored it and
>>> broke some
>>> systems in 8b3517f88ff2 ("PCI: loongson: Prevent LS7A MRRS increases").
>>>
>>> Also to note that ks_pcie_quirk in drivers/pci/controller/dwc/pci-keystone.c
>>> uses similar approach.
>>>> This avoids iterating over bridges/devices two times.
>>>>
>>>> Also, please rename firmware to BIOS, as firmware commonly represents the
>>>> software running on PCIe endpoint devices.
>>> Ack, will fix in next reversion.
>>>
>>> Thanks
>>> - Jiaxun
>>>>
>>>> - Mani
>>> [...]
>>
在2023年10月20日十月 下午12:04，Thorsten Leemhuis写道：
> Hi, Thorsten here, the Linux kernel's regression tracker. Top-posting
> for once, to make this easily accessible to everyone.
>
> Just wondering what's up here. The patch this thread is about afaics was
> supposed to fix a regression reported in July
> (https://bugzilla.kernel.org/show_bug.cgi?id=217680 ), but has made not
> steps closer to get mainlined during the past few weeks. Is there a
> reason, or did it maybe fell through the cracks?
>
> Jiaxun Yang, from it quick look it seems like you wanted to post a v3,
> but never did so; but I might be mistaken there.
I just figure out the problem of this implementation.
Will resend and explain.
Thanks
- Jiaxun
>
> Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
> --
> Everything you wanna know about Linux kernel regression tracking:
> https://linux-regtracking.leemhuis.info/about/#tldr
> If I did something stupid, please tell me, as explained on that page.
>
> #regzbot poke
>
> On 20.09.23 14:33, Linux regression tracking (Thorsten Leemhuis) wrote:
>> [CCing the regression list, as it should be in the loop for regressions:
>> https://docs.kernel.org/admin-guide/reporting-regressions.html]
>>
>> On 07.09.23 07:08, Manivannan Sadhasivam wrote:
>>> On Thu, Sep 07, 2023 at 11:13:00AM +0800, Jiaxun Yang wrote:
>>>> 在 2023/9/7 9:18, Manivannan Sadhasivam 写道:
>>>> [...]
>>>>> Why do you need to walk through every single device instead of just bridges?
>>>>> I'm not the maintainer, but my suggestion is to go for Huacai Chen's solution.
>>>>
>>>> Thanks for your reply, unfortunately Huacai's solution is impractical in
>>>> this case.
>>>>
>>>> The problem we have, is firmware (or BIOS) setting improper MRRS for devices
>>>> attached under those bridges. So we have to fix up MRRS for every single
>>>> device.
>>>> We can't iterate child device in bridge quirk because there is no guarantee
>>>> that
>>>> bridge will be probed before it's child device, partly due to hotplug.
>>>
>>> Okay, this clarifies and also warrants improvement in commit message.
>>>
>>> You could also use pci_walk_bus() after pci_host_probe() to iterate over the
>>> child devices under root bridge and set MRRS. IMO that would look neat.
>>
>> Hi, Thorsten here, the Linux kernel's regression tracker. What's the
>> status here? The regression that was supposed to be fixed by the patched
>> that started this thread was reported 9 weeks ago[1] and the culprit
>> made it to many stable kernels as well. Would be really good to finally
>> fix this, as a regression like this should ideally be fixed within 2 to
>> 3 weeks (in both mainline and stable). With a revert if necessary -- is
>> this maybe still a option, or would that cause more trouble then it
>> solved (I guess that's the case).
>>
>> [1] https://bugzilla.kernel.org/show_bug.cgi?id=217680
>>
>> Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
>> --
>> Everything you wanna know about Linux kernel regression tracking:
>> https://linux-regtracking.leemhuis.info/about/#tldr
>> If I did something stupid, please tell me, as explained on that page.
>>
>>>> This quirk has been in tree for a while, until Huacai refactored it and
>>>> broke some
>>>> systems in 8b3517f88ff2 ("PCI: loongson: Prevent LS7A MRRS increases").
>>>>
>>>> Also to note that ks_pcie_quirk in drivers/pci/controller/dwc/pci-keystone.c
>>>> uses similar approach.
>>>>> This avoids iterating over bridges/devices two times.
>>>>>
>>>>> Also, please rename firmware to BIOS, as firmware commonly represents the
>>>>> software running on PCIe endpoint devices.
>>>> Ack, will fix in next reversion.
>>>>
>>>> Thanks
>>>> - Jiaxun
>>>>>
>>>>> - Mani
>>>> [...]
>>>
--
- Jiaxun
在2023年9月7日九月 上午6:08，Manivannan Sadhasivam写道：
> On Thu, Sep 07, 2023 at 11:13:00AM +0800, Jiaxun Yang wrote:
>>
>>
>> 在 2023/9/7 9:18, Manivannan Sadhasivam 写道:
>> [...]
>> > Why do you need to walk through every single device instead of just bridges?
>> > I'm not the maintainer, but my suggestion is to go for Huacai Chen's solution.
>> Hi Mani,
>>
>> Thanks for your reply, unfortunately Huacai's solution is impractical in
>> this case.
>>
>> The problem we have, is firmware (or BIOS) setting improper MRRS for devices
>> attached under those bridges. So we have to fix up MRRS for every single
>> device.
>> We can't iterate child device in bridge quirk because there is no guarantee
>> that
>> bridge will be probed before it's child device, partly due to hotplug.
>>
>
> Okay, this clarifies and also warrants improvement in commit message.
>
> You could also use pci_walk_bus() after pci_host_probe() to iterate over the
> child devices under root bridge and set MRRS. IMO that would look neat.
This won't work as disabling device will reset MRRS and you have to set
it again, while pci_host_probe will only run at init.
So quirk at device enablement is still the best solution.
Thanks
- Jiaxun
>
> - Mani
>
>> This quirk has been in tree for a while, until Huacai refactored it and
>> broke some
>> systems in 8b3517f88ff2 ("PCI: loongson: Prevent LS7A MRRS increases").
>>
>> Also to note that ks_pcie_quirk in drivers/pci/controller/dwc/pci-keystone.c
>> uses similar approach.
>> > This avoids iterating over bridges/devices two times.
>> >
>> > Also, please rename firmware to BIOS, as firmware commonly represents the
>> > software running on PCIe endpoint devices.
>> Ack, will fix in next reversion.
>>
>> Thanks
>> - Jiaxun
>> >
>> > - Mani
>> [...]
>
> --
> மணிவண்ணன் சதாசிவம்
--
- Jiaxun