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

[Hans Zhang <18255117159@163.com>]

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!