[PATCH] PCI: brcmstb: Assign pcie->gen from pcie_get_link_speed()

[PATCH] PCI: brcmstb: Assign pcie->gen from pcie_get_link_speed()

[Florian Fainelli]

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

Re: [PATCH] PCI: brcmstb: Assign pcie->gen from pcie_get_link_speed()

[Bjorn Helgaas]

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
> 

Re: [PATCH] PCI: brcmstb: Assign pcie->gen from pcie_get_link_speed()

[Florian Fainelli]

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

Re: [PATCH] PCI: brcmstb: Assign pcie->gen from pcie_get_link_speed()

[Bjorn Helgaas]

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
> 

Re: [PATCH] PCI: brcmstb: Assign pcie->gen from pcie_get_link_speed()

[Florian Fainelli]

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

Re: [PATCH] PCI: brcmstb: Assign pcie->gen from pcie_get_link_speed()

[Hans Zhang]

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

Re: [PATCH] PCI: brcmstb: Assign pcie->gen from pcie_get_link_speed()

[Florian Fainelli]

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