* [PATCH] PCI: brcmstb: Assign pcie->gen from pcie_get_link_speed()
@ 2026-05-01 20:24 Florian Fainelli
2026-05-01 22:03 ` Bjorn Helgaas
2026-05-02 11:40 ` Bjorn Helgaas
0 siblings, 2 replies; 9+ messages in thread
From: Florian Fainelli @ 2026-05-01 20:24 UTC (permalink / raw)
To: linux-pci
Cc: Florian Fainelli, Dom Cobley, Phil Elwell, Jim Quinlan,
Broadcom internal kernel review list, Lorenzo Pieralisi,
Krzysztof Wilczyński, Manivannan Sadhasivam, Rob Herring,
Bjorn Helgaas, Hans Zhang,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
open list
After commit 03f920936977 ("PCI: controller: Validate max-link-speed"),
pcie->gen stopped being assigned and as a result the established PCIe
link would stop supporting Gen3 speeds on 2712 since pcie->gen is used
to populate LnkCntl2 and LnkCap in brcm_pcie_set_gen().
Link: https://github.com/raspberrypi/linux/issues/7343
Reported-by: Dom Cobley <popcornmix@gmail.com>
Reported-by: Phil Elwell <phil@raspberrypi.com>
Fixes: 03f920936977 ("PCI: controller: Validate max-link-speed")
Signed-off-by: Florian Fainelli <florian.fainelli@broadcom.com>
---
drivers/pci/controller/pcie-brcmstb.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
index 714bcab97b60..6138fc4bc064 100644
--- a/drivers/pci/controller/pcie-brcmstb.c
+++ b/drivers/pci/controller/pcie-brcmstb.c
@@ -2072,8 +2072,7 @@ static int brcm_pcie_probe(struct platform_device *pdev)
return PTR_ERR(pcie->clk);
ret = of_pci_get_max_link_speed(np);
- if (pcie_get_link_speed(ret) == PCI_SPEED_UNKNOWN)
- pcie->gen = 0;
+ pcie->gen = pcie_get_link_speed(ret);
pcie->ssc = of_property_read_bool(np, "brcm,enable-ssc");
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] PCI: brcmstb: Assign pcie->gen from pcie_get_link_speed()
2026-05-01 20:24 [PATCH] PCI: brcmstb: Assign pcie->gen from pcie_get_link_speed() Florian Fainelli
@ 2026-05-01 22:03 ` Bjorn Helgaas
2026-05-01 22:05 ` Florian Fainelli
2026-05-02 11:40 ` Bjorn Helgaas
1 sibling, 1 reply; 9+ messages in thread
From: Bjorn Helgaas @ 2026-05-01 22:03 UTC (permalink / raw)
To: Florian Fainelli
Cc: linux-pci, Dom Cobley, Phil Elwell, Jim Quinlan,
Broadcom internal kernel review list, Lorenzo Pieralisi,
Krzysztof Wilczyński, Manivannan Sadhasivam, Rob Herring,
Bjorn Helgaas, Hans Zhang,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
open list
On Fri, May 01, 2026 at 01:24:38PM -0700, Florian Fainelli wrote:
> After commit 03f920936977 ("PCI: controller: Validate max-link-speed"),
> pcie->gen stopped being assigned and as a result the established PCIe
> link would stop supporting Gen3 speeds on 2712 since pcie->gen is used
> to populate LnkCntl2 and LnkCap in brcm_pcie_set_gen().
>
> Link: https://github.com/raspberrypi/linux/issues/7343
> Reported-by: Dom Cobley <popcornmix@gmail.com>
> Reported-by: Phil Elwell <phil@raspberrypi.com>
> Fixes: 03f920936977 ("PCI: controller: Validate max-link-speed")
> Signed-off-by: Florian Fainelli <florian.fainelli@broadcom.com>
Applied to pci/for-linus for v7.1, thanks, and sorry for the breakage.
> ---
> drivers/pci/controller/pcie-brcmstb.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
> index 714bcab97b60..6138fc4bc064 100644
> --- a/drivers/pci/controller/pcie-brcmstb.c
> +++ b/drivers/pci/controller/pcie-brcmstb.c
> @@ -2072,8 +2072,7 @@ static int brcm_pcie_probe(struct platform_device *pdev)
> return PTR_ERR(pcie->clk);
>
> ret = of_pci_get_max_link_speed(np);
> - if (pcie_get_link_speed(ret) == PCI_SPEED_UNKNOWN)
> - pcie->gen = 0;
> + pcie->gen = pcie_get_link_speed(ret);
>
> pcie->ssc = of_property_read_bool(np, "brcm,enable-ssc");
>
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] PCI: brcmstb: Assign pcie->gen from pcie_get_link_speed()
2026-05-01 22:03 ` Bjorn Helgaas
@ 2026-05-01 22:05 ` Florian Fainelli
0 siblings, 0 replies; 9+ messages in thread
From: Florian Fainelli @ 2026-05-01 22:05 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linux-pci, Dom Cobley, Phil Elwell, Jim Quinlan,
Broadcom internal kernel review list, Lorenzo Pieralisi,
Krzysztof Wilczyński, Manivannan Sadhasivam, Rob Herring,
Bjorn Helgaas, Hans Zhang,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
open list
On 5/1/26 15:03, Bjorn Helgaas wrote:
> On Fri, May 01, 2026 at 01:24:38PM -0700, Florian Fainelli wrote:
>> After commit 03f920936977 ("PCI: controller: Validate max-link-speed"),
>> pcie->gen stopped being assigned and as a result the established PCIe
>> link would stop supporting Gen3 speeds on 2712 since pcie->gen is used
>> to populate LnkCntl2 and LnkCap in brcm_pcie_set_gen().
>>
>> Link: https://github.com/raspberrypi/linux/issues/7343
>> Reported-by: Dom Cobley <popcornmix@gmail.com>
>> Reported-by: Phil Elwell <phil@raspberrypi.com>
>> Fixes: 03f920936977 ("PCI: controller: Validate max-link-speed")
>> Signed-off-by: Florian Fainelli <florian.fainelli@broadcom.com>
>
> Applied to pci/for-linus for v7.1, thanks, and sorry for the breakage.
I reviewed the original change and completely missed that, so no
worries, the fault is mine to have missed it in the first place. Thanks
for fast tracking this!
--
Florian
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] PCI: brcmstb: Assign pcie->gen from pcie_get_link_speed()
2026-05-01 20:24 [PATCH] PCI: brcmstb: Assign pcie->gen from pcie_get_link_speed() Florian Fainelli
2026-05-01 22:03 ` Bjorn Helgaas
@ 2026-05-02 11:40 ` Bjorn Helgaas
2026-05-04 16:58 ` Florian Fainelli
1 sibling, 1 reply; 9+ messages in thread
From: Bjorn Helgaas @ 2026-05-02 11:40 UTC (permalink / raw)
To: Florian Fainelli
Cc: linux-pci, Dom Cobley, Phil Elwell, Jim Quinlan,
Broadcom internal kernel review list, Lorenzo Pieralisi,
Krzysztof Wilczyński, Manivannan Sadhasivam, Rob Herring,
Bjorn Helgaas, Hans Zhang,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
open list
On Fri, May 01, 2026 at 01:24:38PM -0700, Florian Fainelli wrote:
> After commit 03f920936977 ("PCI: controller: Validate max-link-speed"),
> pcie->gen stopped being assigned and as a result the established PCIe
> link would stop supporting Gen3 speeds on 2712 since pcie->gen is used
> to populate LnkCntl2 and LnkCap in brcm_pcie_set_gen().
>
> Link: https://github.com/raspberrypi/linux/issues/7343
> Reported-by: Dom Cobley <popcornmix@gmail.com>
> Reported-by: Phil Elwell <phil@raspberrypi.com>
> Fixes: 03f920936977 ("PCI: controller: Validate max-link-speed")
> Signed-off-by: Florian Fainelli <florian.fainelli@broadcom.com>
> ---
> drivers/pci/controller/pcie-brcmstb.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
> index 714bcab97b60..6138fc4bc064 100644
> --- a/drivers/pci/controller/pcie-brcmstb.c
> +++ b/drivers/pci/controller/pcie-brcmstb.c
> @@ -2072,8 +2072,7 @@ static int brcm_pcie_probe(struct platform_device *pdev)
> return PTR_ERR(pcie->clk);
>
> ret = of_pci_get_max_link_speed(np);
> - if (pcie_get_link_speed(ret) == PCI_SPEED_UNKNOWN)
> - pcie->gen = 0;
> + pcie->gen = pcie_get_link_speed(ret);
Take a look at https://sashiko.dev/#/patchset/20260501202438.376033-1-florian.fainelli%40broadcom.com
The notes at https://github.com/raspberrypi/linux/issues/7343 assumed
PCI_SPEED_UNKNOWN was 0, but in fact it is 0xff, which means you might
want the more defensive patch instead.
I'll be happy to replace what's on pci/for-linus if so.
> pcie->ssc = of_property_read_bool(np, "brcm,enable-ssc");
>
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] PCI: brcmstb: Assign pcie->gen from pcie_get_link_speed()
2026-05-02 11:40 ` Bjorn Helgaas
@ 2026-05-04 16:58 ` Florian Fainelli
2026-05-04 17:26 ` Hans Zhang
0 siblings, 1 reply; 9+ messages in thread
From: Florian Fainelli @ 2026-05-04 16:58 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linux-pci, Dom Cobley, Phil Elwell, Jim Quinlan,
Broadcom internal kernel review list, Lorenzo Pieralisi,
Krzysztof Wilczyński, Manivannan Sadhasivam, Rob Herring,
Bjorn Helgaas, Hans Zhang,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
open list
On 5/2/26 04:40, Bjorn Helgaas wrote:
> On Fri, May 01, 2026 at 01:24:38PM -0700, Florian Fainelli wrote:
>> After commit 03f920936977 ("PCI: controller: Validate max-link-speed"),
>> pcie->gen stopped being assigned and as a result the established PCIe
>> link would stop supporting Gen3 speeds on 2712 since pcie->gen is used
>> to populate LnkCntl2 and LnkCap in brcm_pcie_set_gen().
>>
>> Link: https://github.com/raspberrypi/linux/issues/7343
>> Reported-by: Dom Cobley <popcornmix@gmail.com>
>> Reported-by: Phil Elwell <phil@raspberrypi.com>
>> Fixes: 03f920936977 ("PCI: controller: Validate max-link-speed")
>> Signed-off-by: Florian Fainelli <florian.fainelli@broadcom.com>
>> ---
>> drivers/pci/controller/pcie-brcmstb.c | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
>> index 714bcab97b60..6138fc4bc064 100644
>> --- a/drivers/pci/controller/pcie-brcmstb.c
>> +++ b/drivers/pci/controller/pcie-brcmstb.c
>> @@ -2072,8 +2072,7 @@ static int brcm_pcie_probe(struct platform_device *pdev)
>> return PTR_ERR(pcie->clk);
>>
>> ret = of_pci_get_max_link_speed(np);
>> - if (pcie_get_link_speed(ret) == PCI_SPEED_UNKNOWN)
>> - pcie->gen = 0;
>> + pcie->gen = pcie_get_link_speed(ret);
>
> Take a look at https://sashiko.dev/#/patchset/20260501202438.376033-1-florian.fainelli%40broadcom.com
>
> The notes at https://github.com/raspberrypi/linux/issues/7343 assumed
> PCI_SPEED_UNKNOWN was 0, but in fact it is 0xff, which means you might
> want the more defensive patch instead.
>
> I'll be happy to replace what's on pci/for-linus if so.
I am starting to think a revert is the simplest path forward, it's not
clear what pcie_get_link_speed() brings to the table honestly.
--
Florian
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] PCI: brcmstb: Assign pcie->gen from pcie_get_link_speed()
2026-05-04 16:58 ` Florian Fainelli
@ 2026-05-04 17:26 ` Hans Zhang
2026-05-04 23:46 ` Florian Fainelli
0 siblings, 1 reply; 9+ messages in thread
From: Hans Zhang @ 2026-05-04 17:26 UTC (permalink / raw)
To: Florian Fainelli, Bjorn Helgaas
Cc: linux-pci, Dom Cobley, Phil Elwell, Jim Quinlan,
Broadcom internal kernel review list, Lorenzo Pieralisi,
Krzysztof Wilczyński, Manivannan Sadhasivam, Rob Herring,
Bjorn Helgaas,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
open list
On 5/5/26 00:58, Florian Fainelli wrote:
> On 5/2/26 04:40, Bjorn Helgaas wrote:
>> On Fri, May 01, 2026 at 01:24:38PM -0700, Florian Fainelli wrote:
>>> After commit 03f920936977 ("PCI: controller: Validate max-link-speed"),
>>> pcie->gen stopped being assigned and as a result the established PCIe
>>> link would stop supporting Gen3 speeds on 2712 since pcie->gen is used
>>> to populate LnkCntl2 and LnkCap in brcm_pcie_set_gen().
>>>
>>> Link: https://github.com/raspberrypi/linux/issues/7343
>>> Reported-by: Dom Cobley <popcornmix@gmail.com>
>>> Reported-by: Phil Elwell <phil@raspberrypi.com>
>>> Fixes: 03f920936977 ("PCI: controller: Validate max-link-speed")
>>> Signed-off-by: Florian Fainelli <florian.fainelli@broadcom.com>
>>> ---
>>> drivers/pci/controller/pcie-brcmstb.c | 3 +--
>>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/
>>> controller/pcie-brcmstb.c
>>> index 714bcab97b60..6138fc4bc064 100644
>>> --- a/drivers/pci/controller/pcie-brcmstb.c
>>> +++ b/drivers/pci/controller/pcie-brcmstb.c
>>> @@ -2072,8 +2072,7 @@ static int brcm_pcie_probe(struct
>>> platform_device *pdev)
>>> return PTR_ERR(pcie->clk);
>>> ret = of_pci_get_max_link_speed(np);
>>> - if (pcie_get_link_speed(ret) == PCI_SPEED_UNKNOWN)
>>> - pcie->gen = 0;
>>> + pcie->gen = pcie_get_link_speed(ret);
>>
>> Take a look at https://sashiko.dev/#/patchset/20260501202438.376033-1-
>> florian.fainelli%40broadcom.com
>>
>> The notes at https://github.com/raspberrypi/linux/issues/7343 assumed
>> PCI_SPEED_UNKNOWN was 0, but in fact it is 0xff, which means you might
>> want the more defensive patch instead.
>>
>> I'll be happy to replace what's on pci/for-linus if so.
>
> I am starting to think a revert is the simplest path forward, it's not
> clear what pcie_get_link_speed() brings to the table honestly.
Hi Florian,
The pcie_get_link_speed function is designed to prevent other Root Port
drivers from accessing the array pcie_link_speed out of bounds.
commit 03f920936977 ("PCI: controller: Validate max-link-speed")
diff --git a/drivers/pci/controller/pcie-brcmstb.c
b/drivers/pci/controller/pcie-brcmstb.c
index 062f55690012..714bcab97b60 100644
--- a/drivers/pci/controller/pcie-brcmstb.c
+++ b/drivers/pci/controller/pcie-brcmstb.c
@@ -1442,7 +1442,7 @@ static int brcm_pcie_start_link(struct brcm_pcie
*pcie)
cls = FIELD_GET(PCI_EXP_LNKSTA_CLS, lnksta);
nlw = FIELD_GET(PCI_EXP_LNKSTA_NLW, lnksta);
dev_info(dev, "link up, %s x%u %s\n",
- pci_speed_string(pcie_link_speed[cls]), nlw,
+ pci_speed_string(pcie_get_link_speed(cls)), nlw,
ssc_good ? "(SSC)" : "(!SSC)");
return 0;
@@ -2072,7 +2072,8 @@ static int brcm_pcie_probe(struct platform_device
*pdev)
return PTR_ERR(pcie->clk);
ret = of_pci_get_max_link_speed(np);
- pcie->gen = (ret < 0) ? 0 : ret;
+ if (pcie_get_link_speed(ret) == PCI_SPEED_UNKNOWN)
+ pcie->gen = 0;
Sorry, it's my fault. Should it be changed to the following?
Florian, what do you think?
diff --git a/drivers/pci/controller/pcie-brcmstb.c
b/drivers/pci/controller/pcie-brcmstb.c
index 714bcab97b60..9a09a48acfb5 100644
--- a/drivers/pci/controller/pcie-brcmstb.c
+++ b/drivers/pci/controller/pcie-brcmstb.c
@@ -2074,6 +2074,8 @@ static int brcm_pcie_probe(struct platform_device
*pdev)
ret = of_pci_get_max_link_speed(np);
if (pcie_get_link_speed(ret) == PCI_SPEED_UNKNOWN)
pcie->gen = 0;
+ else
+ pcie->gen = ret;
pcie->ssc = of_property_read_bool(np, "brcm,enable-ssc");
or
diff --git a/drivers/pci/controller/pcie-brcmstb.c
b/drivers/pci/controller/pcie-brcmstb.c
index 714bcab97b60..1187e19fbe89 100644
--- a/drivers/pci/controller/pcie-brcmstb.c
+++ b/drivers/pci/controller/pcie-brcmstb.c
@@ -2072,8 +2072,7 @@ static int brcm_pcie_probe(struct platform_device
*pdev)
return PTR_ERR(pcie->clk);
ret = of_pci_get_max_link_speed(np);
- if (pcie_get_link_speed(ret) == PCI_SPEED_UNKNOWN)
- pcie->gen = 0;
+ pcie->gen = (ret < 0) ? 0 : ret;
Best regards,
Hans
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] PCI: brcmstb: Assign pcie->gen from pcie_get_link_speed()
2026-05-04 17:26 ` Hans Zhang
@ 2026-05-04 23:46 ` Florian Fainelli
2026-05-05 12:54 ` Hans Zhang
2026-05-05 16:01 ` Bjorn Helgaas
0 siblings, 2 replies; 9+ messages in thread
From: Florian Fainelli @ 2026-05-04 23:46 UTC (permalink / raw)
To: Hans Zhang, Bjorn Helgaas
Cc: linux-pci, Dom Cobley, Phil Elwell, Jim Quinlan,
Broadcom internal kernel review list, Lorenzo Pieralisi,
Krzysztof Wilczyński, Manivannan Sadhasivam, Rob Herring,
Bjorn Helgaas,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
open list
On 5/4/26 10:26, Hans Zhang wrote:
>
>
> On 5/5/26 00:58, Florian Fainelli wrote:
>> On 5/2/26 04:40, Bjorn Helgaas wrote:
>>> On Fri, May 01, 2026 at 01:24:38PM -0700, Florian Fainelli wrote:
>>>> After commit 03f920936977 ("PCI: controller: Validate max-link-speed"),
>>>> pcie->gen stopped being assigned and as a result the established PCIe
>>>> link would stop supporting Gen3 speeds on 2712 since pcie->gen is used
>>>> to populate LnkCntl2 and LnkCap in brcm_pcie_set_gen().
>>>>
>>>> Link: https://github.com/raspberrypi/linux/issues/7343
>>>> Reported-by: Dom Cobley <popcornmix@gmail.com>
>>>> Reported-by: Phil Elwell <phil@raspberrypi.com>
>>>> Fixes: 03f920936977 ("PCI: controller: Validate max-link-speed")
>>>> Signed-off-by: Florian Fainelli <florian.fainelli@broadcom.com>
>>>> ---
>>>> drivers/pci/controller/pcie-brcmstb.c | 3 +--
>>>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/
>>>> controller/pcie-brcmstb.c
>>>> index 714bcab97b60..6138fc4bc064 100644
>>>> --- a/drivers/pci/controller/pcie-brcmstb.c
>>>> +++ b/drivers/pci/controller/pcie-brcmstb.c
>>>> @@ -2072,8 +2072,7 @@ static int brcm_pcie_probe(struct
>>>> platform_device *pdev)
>>>> return PTR_ERR(pcie->clk);
>>>> ret = of_pci_get_max_link_speed(np);
>>>> - if (pcie_get_link_speed(ret) == PCI_SPEED_UNKNOWN)
>>>> - pcie->gen = 0;
>>>> + pcie->gen = pcie_get_link_speed(ret);
>>>
>>> Take a look at https://sashiko.dev/#/
>>> patchset/20260501202438.376033-1- florian.fainelli%40broadcom.com
>>>
>>> The notes at https://github.com/raspberrypi/linux/issues/7343 assumed
>>> PCI_SPEED_UNKNOWN was 0, but in fact it is 0xff, which means you might
>>> want the more defensive patch instead.
>>>
>>> I'll be happy to replace what's on pci/for-linus if so.
>>
>> I am starting to think a revert is the simplest path forward, it's not
>> clear what pcie_get_link_speed() brings to the table honestly.
>
> Hi Florian,
>
> The pcie_get_link_speed function is designed to prevent other Root Port
> drivers from accessing the array pcie_link_speed out of bounds.
Yes, so that's useful in the first hunk of your commit where we were
printing the link speed without checking that the 'max-link-speed' would
be bounds check, however it is not really useful for assigning to
pcie->gen in our case, so I would prefer the second option:
ret = of_pci_get_max_link_speed(np);
- if (pcie_get_link_speed(ret) == PCI_SPEED_UNKNOWN)
- pcie->gen = 0;
+ pcie->gen = (ret < 0) ? 0 : ret;
Thank you for stepping in!
--
Florian
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] PCI: brcmstb: Assign pcie->gen from pcie_get_link_speed()
2026-05-04 23:46 ` Florian Fainelli
@ 2026-05-05 12:54 ` Hans Zhang
2026-05-05 16:01 ` Bjorn Helgaas
1 sibling, 0 replies; 9+ messages in thread
From: Hans Zhang @ 2026-05-05 12:54 UTC (permalink / raw)
To: Florian Fainelli, Bjorn Helgaas
Cc: linux-pci, Dom Cobley, Phil Elwell, Jim Quinlan,
Broadcom internal kernel review list, Lorenzo Pieralisi,
Krzysztof Wilczyński, Manivannan Sadhasivam, Rob Herring,
Bjorn Helgaas,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
open list
On 5/5/26 07:46, Florian Fainelli wrote:
> On 5/4/26 10:26, Hans Zhang wrote:
>>
>>
>> On 5/5/26 00:58, Florian Fainelli wrote:
>>> On 5/2/26 04:40, Bjorn Helgaas wrote:
>>>> On Fri, May 01, 2026 at 01:24:38PM -0700, Florian Fainelli wrote:
>>>>> After commit 03f920936977 ("PCI: controller: Validate max-link-
>>>>> speed"),
>>>>> pcie->gen stopped being assigned and as a result the established PCIe
>>>>> link would stop supporting Gen3 speeds on 2712 since pcie->gen is used
>>>>> to populate LnkCntl2 and LnkCap in brcm_pcie_set_gen().
>>>>>
>>>>> Link: https://github.com/raspberrypi/linux/issues/7343
>>>>> Reported-by: Dom Cobley <popcornmix@gmail.com>
>>>>> Reported-by: Phil Elwell <phil@raspberrypi.com>
>>>>> Fixes: 03f920936977 ("PCI: controller: Validate max-link-speed")
>>>>> Signed-off-by: Florian Fainelli <florian.fainelli@broadcom.com>
>>>>> ---
>>>>> drivers/pci/controller/pcie-brcmstb.c | 3 +--
>>>>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/
>>>>> controller/pcie-brcmstb.c
>>>>> index 714bcab97b60..6138fc4bc064 100644
>>>>> --- a/drivers/pci/controller/pcie-brcmstb.c
>>>>> +++ b/drivers/pci/controller/pcie-brcmstb.c
>>>>> @@ -2072,8 +2072,7 @@ static int brcm_pcie_probe(struct
>>>>> platform_device *pdev)
>>>>> return PTR_ERR(pcie->clk);
>>>>> ret = of_pci_get_max_link_speed(np);
>>>>> - if (pcie_get_link_speed(ret) == PCI_SPEED_UNKNOWN)
>>>>> - pcie->gen = 0;
>>>>> + pcie->gen = pcie_get_link_speed(ret);
>>>>
>>>> Take a look at https://sashiko.dev/#/
>>>> patchset/20260501202438.376033-1- florian.fainelli%40broadcom.com
>>>>
>>>> The notes at https://github.com/raspberrypi/linux/issues/7343 assumed
>>>> PCI_SPEED_UNKNOWN was 0, but in fact it is 0xff, which means you might
>>>> want the more defensive patch instead.
>>>>
>>>> I'll be happy to replace what's on pci/for-linus if so.
>>>
>>> I am starting to think a revert is the simplest path forward, it's
>>> not clear what pcie_get_link_speed() brings to the table honestly.
>>
>> Hi Florian,
>>
>> The pcie_get_link_speed function is designed to prevent other Root
>> Port drivers from accessing the array pcie_link_speed out of bounds.
>
> Yes, so that's useful in the first hunk of your commit where we were
> printing the link speed without checking that the 'max-link-speed' would
> be bounds check, however it is not really useful for assigning to pcie-
> >gen in our case, so I would prefer the second option:
>
>
> ret = of_pci_get_max_link_speed(np);
> - if (pcie_get_link_speed(ret) == PCI_SPEED_UNKNOWN)
> - pcie->gen = 0;
> + pcie->gen = (ret < 0) ? 0 : ret;
>
Hi Bjorn,
Please replace the pci/for-linus branch with the above modifications. Or
do you need me to create a new patch again?
Best regards,
Hans
> Thank you for stepping in!
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] PCI: brcmstb: Assign pcie->gen from pcie_get_link_speed()
2026-05-04 23:46 ` Florian Fainelli
2026-05-05 12:54 ` Hans Zhang
@ 2026-05-05 16:01 ` Bjorn Helgaas
1 sibling, 0 replies; 9+ messages in thread
From: Bjorn Helgaas @ 2026-05-05 16:01 UTC (permalink / raw)
To: Florian Fainelli
Cc: Hans Zhang, linux-pci, Dom Cobley, Phil Elwell, Jim Quinlan,
Broadcom internal kernel review list, Lorenzo Pieralisi,
Krzysztof Wilczyński, Manivannan Sadhasivam, Rob Herring,
Bjorn Helgaas,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
open list
On Mon, May 04, 2026 at 04:46:03PM -0700, Florian Fainelli wrote:
> On 5/4/26 10:26, Hans Zhang wrote:
> > On 5/5/26 00:58, Florian Fainelli wrote:
> > > On 5/2/26 04:40, Bjorn Helgaas wrote:
> > > > On Fri, May 01, 2026 at 01:24:38PM -0700, Florian Fainelli wrote:
> > > > > After commit 03f920936977 ("PCI: controller: Validate max-link-speed"),
> > > > > pcie->gen stopped being assigned and as a result the established PCIe
> > > > > link would stop supporting Gen3 speeds on 2712 since pcie->gen is used
> > > > > to populate LnkCntl2 and LnkCap in brcm_pcie_set_gen().
> > > > >
> > > > > Link: https://github.com/raspberrypi/linux/issues/7343
> > > > > Reported-by: Dom Cobley <popcornmix@gmail.com>
> > > > > Reported-by: Phil Elwell <phil@raspberrypi.com>
> > > > > Fixes: 03f920936977 ("PCI: controller: Validate max-link-speed")
> > > > > Signed-off-by: Florian Fainelli <florian.fainelli@broadcom.com>
> > > > > ---
> > > > > drivers/pci/controller/pcie-brcmstb.c | 3 +--
> > > > > 1 file changed, 1 insertion(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/drivers/pci/controller/pcie-brcmstb.c
> > > > > b/drivers/pci/ controller/pcie-brcmstb.c
> > > > > index 714bcab97b60..6138fc4bc064 100644
> > > > > --- a/drivers/pci/controller/pcie-brcmstb.c
> > > > > +++ b/drivers/pci/controller/pcie-brcmstb.c
> > > > > @@ -2072,8 +2072,7 @@ static int brcm_pcie_probe(struct
> > > > > platform_device *pdev)
> > > > > return PTR_ERR(pcie->clk);
> > > > > ret = of_pci_get_max_link_speed(np);
> > > > > - if (pcie_get_link_speed(ret) == PCI_SPEED_UNKNOWN)
> > > > > - pcie->gen = 0;
> > > > > + pcie->gen = pcie_get_link_speed(ret);
> > > >
> > > > Take a look at https://sashiko.dev/#/
> > > > patchset/20260501202438.376033-1-
> > > > florian.fainelli%40broadcom.com
> > > >
> > > > The notes at https://github.com/raspberrypi/linux/issues/7343 assumed
> > > > PCI_SPEED_UNKNOWN was 0, but in fact it is 0xff, which means you might
> > > > want the more defensive patch instead.
> > > >
> > > > I'll be happy to replace what's on pci/for-linus if so.
> > >
> > > I am starting to think a revert is the simplest path forward, it's
> > > not clear what pcie_get_link_speed() brings to the table honestly.
> >
> > The pcie_get_link_speed function is designed to prevent other Root Port
> > drivers from accessing the array pcie_link_speed out of bounds.
>
> Yes, so that's useful in the first hunk of your commit where we were
> printing the link speed without checking that the 'max-link-speed' would be
> bounds check, however it is not really useful for assigning to pcie->gen in
> our case, so I would prefer the second option:
>
> ret = of_pci_get_max_link_speed(np);
> - if (pcie_get_link_speed(ret) == PCI_SPEED_UNKNOWN)
> - pcie->gen = 0;
> + pcie->gen = (ret < 0) ? 0 : ret;
The point of this validation is to make sure we don't program the
brcmstb hardware with something it doesn't support.
of_pci_get_max_link_speed() only returns an error (ret < 0) if DT
didn't contain a 'max-link-speed' property. That doesn't tell us
anything about what brcmstb devices support.
I think this test should be something like what advk_pcie_probe() or
rockchip_pcie_parse_dt() do. If of_pci_get_max_link_speed() fails or
returns something not supported by the hardware, they default to the
fastest speed known to be supported.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-05-05 16:01 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-01 20:24 [PATCH] PCI: brcmstb: Assign pcie->gen from pcie_get_link_speed() Florian Fainelli
2026-05-01 22:03 ` Bjorn Helgaas
2026-05-01 22:05 ` Florian Fainelli
2026-05-02 11:40 ` Bjorn Helgaas
2026-05-04 16:58 ` Florian Fainelli
2026-05-04 17:26 ` Hans Zhang
2026-05-04 23:46 ` Florian Fainelli
2026-05-05 12:54 ` Hans Zhang
2026-05-05 16:01 ` Bjorn Helgaas
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox