[PATCH v2] pci: loongson: Workaround MIPS firmware MRRS settings

[PATCH v2] pci: loongson: Workaround MIPS firmware MRRS settings

[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@vger.kernel.org
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@flygoat.com>
---
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

Re: [PATCH v2] pci: loongson: Workaround MIPS firmware MRRS settings

[Jiaxun Yang]

在 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@vger.kernel.org
> 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@flygoat.com>

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);

Re: [PATCH v2] pci: loongson: Workaround MIPS firmware MRRS settings

[Huacai Chen]

Hi, Jiaxun,

On Sun, Aug 6, 2023 at 10:20 AM Jiaxun Yang <jiaxun.yang@flygoat.com> 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@vger.kernel.org
> > 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@flygoat.com>
>
> 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);
>

Re: [PATCH v2] pci: loongson: Workaround MIPS firmware MRRS settings

[Jiaxun Yang]

在 2023/8/6 22:30, Huacai Chen 写道:
> Hi, Jiaxun,
>
> On Sun, Aug 6, 2023 at 10:20 AM Jiaxun Yang <jiaxun.yang@flygoat.com> 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@vger.kernel.org
>>> 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@flygoat.com>
>> 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);

Re: [PATCH v2] pci: loongson: Workaround MIPS firmware MRRS settings

[Linux regression tracking (Thorsten Leemhuis)]

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 <jiaxun.yang@flygoat.com>
>> 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@vger.kernel.org
>>>> 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@flygoat.com>
>>> 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);
> 

Re: [PATCH v2] pci: loongson: Workaround MIPS firmware MRRS settings

[Jiaxun Yang]

在 2023/8/29 20:26, Linux regression tracking (Thorsten Leemhuis) 写道:
> 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.

Hi Thorsten,

Thanks for reaching back, unfortunately there is no progress so far.

+ Huacai, what's your opinion here? I do think this fix is in best shape for
now.

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 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 <jiaxun.yang@flygoat.com>
>>> 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@vger.kernel.org
>>>>> 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@flygoat.com>
>>>> 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);

Re: [PATCH v2] pci: loongson: Workaround MIPS firmware MRRS settings

[Huacai Chen]

Hi, Jiaxun,

On Tue, Aug 8, 2023 at 3:38 PM Jiaxun Yang <jiaxun.yang@flygoat.com> wrote:
>
>
>
> 在 2023/8/6 22:30, Huacai Chen 写道:
> > Hi, Jiaxun,
> >
> > On Sun, Aug 6, 2023 at 10:20 AM Jiaxun Yang <jiaxun.yang@flygoat.com> 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@vger.kernel.org
> >>> 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@flygoat.com>
> >> 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);
>

Re: [PATCH v2] pci: loongson: Workaround MIPS firmware MRRS settings

[Jiaxun Yang]

在 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

[...]

Re: [PATCH v2] pci: loongson: Workaround MIPS firmware MRRS settings

[Huacai Chen]

On Thu, Aug 31, 2023 at 4:58 PM Jiaxun Yang <jiaxun.yang@flygoat.com> 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
>
> [...]

Re: [PATCH v2] pci: loongson: Workaround MIPS firmware MRRS settings

[Jiaxun Yang]

在 2023/8/31 17:42, Huacai Chen 写道:
> On Thu, Aug 31, 2023 at 4:58 PM Jiaxun Yang <jiaxun.yang@flygoat.com> 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

[...]

Re: [PATCH v2] pci: loongson: Workaround MIPS firmware MRRS settings

[Jiaxun Yang]

在 2023/8/31 18:04, Jiaxun Yang 写道:
>
>
> 在 2023/8/31 17:42, Huacai Chen 写道:
>> On Thu, Aug 31, 2023 at 4:58 PM Jiaxun Yang <jiaxun.yang@flygoat.com> 
>> 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, if you are happy with changes in this patch, do you mind to give 
out a
conditional R-b tag?

Since this regression is in tree for a while, I really don't want to see 
it lapse into
next release.

>
> Hmm, spiting two quirks with similar functionality into two places 
> doesn't sound
> like a good idea.
>
> PCI folks, what do you think?
Bjorn, Lorenzo and Krzysztof, do you mind to give some input on this fix?

Thanks
- Jiaxun

>
> Thanks.
> - Jiaxun
>
> [...]

Re: [PATCH v2] pci: loongson: Workaround MIPS firmware MRRS settings

[Manivannan Sadhasivam]

On Tue, Aug 08, 2023 at 03:38:19PM +0800, Jiaxun Yang wrote:
> 
> 
> 在 2023/8/6 22:30, Huacai Chen 写道:
> > Hi, Jiaxun,
> > 
> > On Sun, Aug 6, 2023 at 10:20 AM Jiaxun Yang <jiaxun.yang@flygoat.com> 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@vger.kernel.org
> > > > 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@flygoat.com>
> > > 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.
> 

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.

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.

- Mani

> 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);
> 

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v2] pci: loongson: Workaround MIPS firmware MRRS settings

[Jiaxun Yang]

在 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.

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
[...]

Re: [PATCH v2] pci: loongson: Workaround MIPS firmware MRRS settings

[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.

- 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
> [...]

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v2] pci: loongson: Workaround MIPS firmware MRRS settings

[Linux regression tracking (Thorsten Leemhuis)]

[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
>> [...]
> 

Re: [PATCH v2] pci: loongson: Workaround MIPS firmware MRRS settings

[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.

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
>>> [...]
>>

Re: [PATCH v2] pci: loongson: Workaround MIPS firmware MRRS settings

[Jiaxun Yang]

在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

Re: [PATCH v2] pci: loongson: Workaround MIPS firmware MRRS settings

[Jiaxun Yang]

在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