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

Hans Zhang
2026-05-04T17:28:01Z

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