Re: [PATCH v2 4/5] PCI: dwc: rcar-gen4: Handle PERST via reset subsystem

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Incorrectly ordered error labels in `rcar_gen4_pcie_probe()` result in a leaked reset control and unbalanced runtime PM operations.
- [High] Probe fails unconditionally with -EINVAL on device trees without a root port child node (e.g., existing R-Car Gen4 DTs and PCIe endpoints).
--

--- Patch [4]: [PATCH v2 4/5] PCI: dwc: rcar-gen4: Handle PERST via reset subsystem ---
commit ee4c8b0343b92a78672b94439fca7074d908ca06
Author: Marek Vasut <marek.vasut+renesas@mailbox.org>

PCI: dwc: rcar-gen4: Handle PERST via reset subsystem

Handle PERST via both GPIO and reset subsystem. On R-Car Gen4, the PERST
signal is operated as a GPIO, on R-Car Gen5 it might only be accessible via
SCMI reset via reset subsystem. Support both options.

> diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> index 73b01e74b42cb..f88168f2df195 100644
> --- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
[ ... ]
> @@ -316,12 +317,23 @@ static void rcar_gen4_pcie_unprepare(struct rcar_gen4_pcie *rcar)
>  
>  static int rcar_gen4_pcie_get_resources(struct rcar_gen4_pcie *rcar)
>  {
> +	struct device *dev = rcar->dw.dev;
> +	struct device_node *root_port;
> +
>  	rcar->phy_base = devm_platform_ioremap_resource_byname(rcar->pdev, "phy");
>  	if (IS_ERR(rcar->phy_base))
>  		return PTR_ERR(rcar->phy_base);
>  
> +	root_port = of_get_next_available_child(dev->of_node, NULL);
> +	rcar->perst = of_reset_control_get_optional_exclusive(root_port, "perst");

[Severity: High]
Does this cause a probe failure on device trees without a root port child
node?

If of_get_next_available_child() returns NULL (for example, on existing R-Car
Gen4 setups or endpoints), passing a NULL root_port to
of_reset_control_get_optional_exclusive() will return ERR_PTR(-EINVAL). This
error is then returned, causing the probe to fail unconditionally.

> +	of_node_put(root_port);
> +	if (IS_ERR(rcar->perst))
> +		return dev_err_probe(dev, PTR_ERR(rcar->perst), "Failed to get PERST#\n");
> +
>  	/* Renesas-specific registers */
>  	rcar->base = devm_platform_ioremap_resource_byname(rcar->pdev, "app");
> +	if (IS_ERR(rcar->base))
> +		reset_control_put(rcar->perst);
>  
>  	return PTR_ERR_OR_ZERO(rcar->base);
>  }
[ ... ]
> @@ -654,7 +682,7 @@ static int rcar_gen4_pcie_probe(struct platform_device *pdev)
>  
>  	err = rcar_gen4_pcie_prepare(rcar);
>  	if (err)
> -		return err;
> +		goto err_prepare;
>  
>  	err = rcar_gen4_add_dw_pcie(rcar);
>  	if (err)
> @@ -662,6 +690,8 @@ static int rcar_gen4_pcie_probe(struct platform_device *pdev)
>  
>  	return 0;
>  
> +err_prepare:
> +	reset_control_put(rcar->perst);
>  err_unprepare:
>  	rcar_gen4_pcie_unprepare(rcar);

[Severity: High]
Are the error labels out of order here?

If rcar_gen4_pcie_prepare() fails, the code jumps to err_prepare which then
falls through to err_unprepare. This calls rcar_gen4_pcie_unprepare() and
performs an unbalanced pm_runtime_put().

Conversely, if rcar_gen4_add_dw_pcie() fails, the code jumps to err_unprepare,
skipping err_prepare entirely. This results in a leaked rcar->perst reset
control.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720020505.315335-1-marek.vasut+renesas@mailbox.org?part=4