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

[Manivannan Sadhasivam]

On Sat, Jul 04, 2026 at 10:25:03PM +0200, Marek Vasut wrote:
> 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.
> This is a preparatory patch for R-Car Gen5 support.
> 
> Signed-off-by: Marek Vasut <marek.vasut+renesas@xxxxxxxxxxx>
> ---
> Cc: "Krzysztof Wilczyński" <kwilczynski@xxxxxxxxxx>
> Cc: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
> Cc: Conor Dooley <conor+dt@xxxxxxxxxx>
> Cc: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
> Cc: Krzysztof Kozlowski <krzk+dt@xxxxxxxxxx>
> Cc: Lorenzo Pieralisi <lpieralisi@xxxxxxxxxx>
> Cc: Manivannan Sadhasivam <mani@xxxxxxxxxx>
> Cc: Rob Herring <robh@xxxxxxxxxx>
> Cc: Yoshihiro Shimoda <yoshihiro.shimoda.uh@xxxxxxxxxxx>
> Cc: devicetree@xxxxxxxxxxxxxxx
> Cc: linux-kernel@xxxxxxxxxxxxxxx
> Cc: linux-pci@xxxxxxxxxxxxxxx
> Cc: linux-renesas-soc@xxxxxxxxxxxxxxx
> ---
>  drivers/pci/controller/dwc/pcie-rcar-gen4.c | 42 +++++++++++++++++++--
>  1 file changed, 39 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> index 05c22cc648135..186eedb33c27d 100644
> --- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> @@ -98,6 +98,7 @@ struct rcar_gen4_pcie {
>  	void __iomem *base;
>  	void __iomem *phy_base;
>  	struct platform_device *pdev;
> +	struct reset_control *perst;
>  	const struct rcar_gen4_pcie_drvdata *drvdata;
>  };
>  #define to_rcar_gen4_pcie(_dw)	container_of(_dw, struct rcar_gen4_pcie, dw)
> @@ -299,10 +300,27 @@ 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 reset_control *perst;
> +
>  	rcar->phy_base = devm_platform_ioremap_resource_byname(rcar->pdev, "phy");
>  	if (IS_ERR(rcar->phy_base))
>  		return PTR_ERR(rcar->phy_base);
>  
> +	rcar->perst = NULL;
> +	for_each_available_child_of_node_scoped(dev->of_node, of_port) {
> +		perst = of_reset_control_get(of_port, "perst");
> +		if (IS_ERR(perst)) {
> +			if (PTR_ERR(perst) != -EPROBE_DEFER)
> +				dev_err(dev, "Failed to get PERST#\n");
> +			return PTR_ERR(perst);

return dev_err_probe(dev, PTR_ERR(perst), "Failed to get PERST#\n")?

> +		}
> +
> +		/* There is only one root port. */
> +		rcar->perst = perst;
> +		break;

It feels weird to see for_each_available_child_of_node_scoped() and then
breaking with first node. Maybe you can just use of_get_next_available_child().

> +	}
> +
>  	/* Renesas-specific registers */
>  	rcar->base = devm_platform_ioremap_resource_byname(rcar->pdev, "app");
>  
> @@ -425,6 +443,22 @@ static int rcar_gen4_pcie_host_msi_init(struct dw_pcie_rp *pp)
>  	return ret;
>  }
>  
> +static void rcar_gen4_pcie_host_perst(struct dw_pcie_rp *pp, int enable)
> +{

rcar_gen4_pcie_host_perst_assert(struct dw_pcie_rp *pp, bool assert)

> +	struct dw_pcie *dw = to_dw_pcie_from_pp(pp);
> +	struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
> +
> +	gpiod_set_value_cansleep(dw->pe_rst, enable);
> +
> +	if (!rcar->perst)
> +		return;
> +
> +	if (enable)
> +		reset_control_assert(rcar->perst);
> +	else
> +		reset_control_deassert(rcar->perst);
> +}
> +
>  /* Host mode */
>  static int rcar_gen4_pcie_host_init(struct dw_pcie_rp *pp)
>  {
> @@ -432,7 +466,7 @@ static int rcar_gen4_pcie_host_init(struct dw_pcie_rp *pp)
>  	struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
>  	int ret;
>  
> -	gpiod_set_value_cansleep(dw->pe_rst, 1);
> +	rcar_gen4_pcie_host_perst(pp, 1);
>  
>  	ret = rcar_gen4_pcie_common_init(rcar);
>  	if (ret)
> @@ -453,7 +487,7 @@ static int rcar_gen4_pcie_host_init(struct dw_pcie_rp *pp)
>  
>  	msleep(PCIE_T_PVPERL_MS);	/* pe_rst requires 100msec delay */
>  
> -	gpiod_set_value_cansleep(dw->pe_rst, 0);
> +	rcar_gen4_pcie_host_perst(pp, 0);

rcar_gen4_pcie_host_perst_assert(pp, false)

>  
>  	return 0;
>  
> @@ -467,7 +501,7 @@ static void rcar_gen4_pcie_host_deinit(struct dw_pcie_rp *pp)
>  	struct dw_pcie *dw = to_dw_pcie_from_pp(pp);
>  	struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
>  
> -	gpiod_set_value_cansleep(dw->pe_rst, 1);
> +	rcar_gen4_pcie_host_perst(pp, 1);

rcar_gen4_pcie_host_perst_assert(pp, true)

>  	rcar_gen4_pcie_common_deinit(rcar);
>  }
>  
> @@ -671,6 +705,8 @@ static void rcar_gen4_pcie_remove(struct platform_device *pdev)
>  
>  	rcar_gen4_remove_dw_pcie(rcar);
>  	rcar_gen4_pcie_unprepare(rcar);
> +	if (rcar->perst)
> +		reset_control_put(rcar->perst);

I don't see reset_control_put() in any error paths of probe().

- Mani

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