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

[PATCH v2] PCI: brcmstb: Assign pcie->gen from of_pci_get_max_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().

Whether the 'max-link-speed' property is not specified, or it exceeds
Gen3, resort to the HW defaults.

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>
Change-Id: Ic5431469dd7a6c8b391f84f27119dad876256880
---
Changes in v2:

- utilize of_pci_get_max_link_speed() directly
- also validate against exceeding Gen3

 drivers/pci/controller/pcie-brcmstb.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
index 714bcab97b60..08a0e7091ced 100644
--- a/drivers/pci/controller/pcie-brcmstb.c
+++ b/drivers/pci/controller/pcie-brcmstb.c
@@ -2072,8 +2072,10 @@ 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)
+	if (ret < 0 || ret > 3)
 		pcie->gen = 0;
+	else
+		pcie->gen = ret;
 
 	pcie->ssc = of_property_read_bool(np, "brcm,enable-ssc");
 
-- 
2.34.1

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

[Manivannan Sadhasivam]

On Wed, May 06, 2026 at 09:45:37AM -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().
> 
> Whether the 'max-link-speed' property is not specified, or it exceeds
> Gen3, resort to the HW defaults.
> 
> 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>

Since this is a 7.1 material:

Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>

> Change-Id: Ic5431469dd7a6c8b391f84f27119dad876256880

This Change-Id is irrelevant to upstream.

- Mani

> ---
> Changes in v2:
> 
> - utilize of_pci_get_max_link_speed() directly
> - also validate against exceeding Gen3
> 
>  drivers/pci/controller/pcie-brcmstb.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
> index 714bcab97b60..08a0e7091ced 100644
> --- a/drivers/pci/controller/pcie-brcmstb.c
> +++ b/drivers/pci/controller/pcie-brcmstb.c
> @@ -2072,8 +2072,10 @@ 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)
> +	if (ret < 0 || ret > 3)
>  		pcie->gen = 0;
> +	else
> +		pcie->gen = ret;
>  
>  	pcie->ssc = of_property_read_bool(np, "brcm,enable-ssc");
>  
> -- 
> 2.34.1
> 

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

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

[Florian Fainelli]

On 5/6/26 10:08, Manivannan Sadhasivam wrote:
> On Wed, May 06, 2026 at 09:45:37AM -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().
>>
>> Whether the 'max-link-speed' property is not specified, or it exceeds
>> Gen3, resort to the HW defaults.
>>
>> 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>
> 
> Since this is a 7.1 material:
> 
> Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>

Thanks!

> 
>> Change-Id: Ic5431469dd7a6c8b391f84f27119dad876256880
> 
> This Change-Id is irrelevant to upstream.

Should I resubmit or can it be stripped while applying?
-- 
Florian

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The error handling path in `brcm_pcie_probe()` causes a NULL pointer dereference if `pci_host_probe()` fails.
- [High] The driver fails to handle the `msi-map` device tree binding variant when determining if the internal MSI controller should be enabled.
--

commit 8f216627ede49601fd793dae393dc6232ac6f438
Author: Florian Fainelli <florian.fainelli@broadcom.com>

PCI: brcmstb: Assign pcie->gen from of_pci_get_max_link_speed()

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().

Whether the 'max-link-speed' property is not specified, or it exceeds
Gen3, resort to the HW defaults.

Link: https://github.com/raspberrypi/linux/issues/7343

> diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
> index 714bcab97b603..08a0e7091cedc 100644
> --- a/drivers/pci/controller/pcie-brcmstb.c
> +++ b/drivers/pci/controller/pcie-brcmstb.c
> @@ -2072,8 +2072,10 @@ 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)
> +	if (ret < 0 || ret > 3)
>  		pcie->gen = 0;
> +	else
> +		pcie->gen = ret;
>  
>  	pcie->ssc = of_property_read_bool(np, "brcm,enable-ssc");

This isn't a regression from this commit, but while reviewing this function
I noticed a pre-existing regression. The driver currently checks for
msi-parent but does not check for the msi-map device tree binding variant:

drivers/pci/controller/pcie-brcmstb.c:brcm_pcie_probe() {
    ...
	if (pci_msi_enabled()) {
		struct device_node *msi_np = of_parse_phandle(pcie->np, "msi-parent", 0);

		if (msi_np == pcie->np)
			ret = brcm_pcie_enable_msi(pcie);
    ...
}

Will this bypass brcm_pcie_enable_msi() and prevent the creation of the
local MSI domain if a platform's DT uses msi-map instead of msi-parent
to reference the built-in MSI controller?

I also noticed another pre-existing regression in the probe error handling path:

drivers/pci/controller/pcie-brcmstb.c:brcm_pcie_probe() {
    ...
	ret = pci_host_probe(bridge);
	if (!ret && !brcm_pcie_link_up(pcie))
		ret = -ENODEV;

	if (ret) {
		brcm_pcie_remove(pdev);
		return ret;
	}
    ...
}

If pci_host_probe(bridge) fails early before allocating bridge->bus,
bridge->bus will remain NULL.

Inside brcm_pcie_remove(pdev), bridge->bus is passed directly to
pci_stop_root_bus(bridge->bus).

The function pci_stop_root_bus() evaluates pci_is_root_bus(bus), which
dereferences the pointer via !(bus->parent).

Does this cause a NULL pointer dereference and a kernel panic if
pci_host_probe() fails?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260506164537.103196-1-florian.fainelli@broadcom.com?part=1

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

[Hans Zhang]

On 5/7/26 00:45, 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().
> 
> Whether the 'max-link-speed' property is not specified, or it exceeds
> Gen3, resort to the HW defaults.
> 
> 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>
> Change-Id: Ic5431469dd7a6c8b391f84f27119dad876256880

Reviewed-by: Hans Zhang <18255117159@163.com>

> ---
> Changes in v2:
> 
> - utilize of_pci_get_max_link_speed() directly
> - also validate against exceeding Gen3
> 
>   drivers/pci/controller/pcie-brcmstb.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
> index 714bcab97b60..08a0e7091ced 100644
> --- a/drivers/pci/controller/pcie-brcmstb.c
> +++ b/drivers/pci/controller/pcie-brcmstb.c
> @@ -2072,8 +2072,10 @@ 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)
> +	if (ret < 0 || ret > 3)
>   		pcie->gen = 0;
> +	else
> +		pcie->gen = ret;
>   
>   	pcie->ssc = of_property_read_bool(np, "brcm,enable-ssc");
>   

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

[Manivannan Sadhasivam]

On Wed, May 06, 2026 at 11:15:50AM -0700, Florian Fainelli wrote:
> On 5/6/26 10:08, Manivannan Sadhasivam wrote:
> > On Wed, May 06, 2026 at 09:45:37AM -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().
> > > 
> > > Whether the 'max-link-speed' property is not specified, or it exceeds
> > > Gen3, resort to the HW defaults.
> > > 
> > > 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>
> > 
> > Since this is a 7.1 material:
> > 
> > Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
> 
> Thanks!
> 
> > 
> > > Change-Id: Ic5431469dd7a6c8b391f84f27119dad876256880
> > 
> > This Change-Id is irrelevant to upstream.
> 
> Should I resubmit or can it be stripped while applying?

I hope Bjorn can drop it while applying.

- Mani

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