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

[Manivannan Sadhasivam]

On Sat, Sep 05, 2026 at 11:26:26PM +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
> ---
> V2: - Use dev_err_probe() for perst in rcar_gen4_pcie_get_resources()
>     - Use of_get_next_available_child() to obtain root port OF node
>     - Switch to of_reset_control_get_optional_exclusive() to deal with
>       R-Car Gen4, where the PERST is handled as GPIO instead of reset
>     - Rename rcar_gen4_pcie_host_perst() to rcar_gen4_pcie_host_perst_assert()
>       and use bool type for assert and deassert selection
>     - Add missing reset_control_put() into rcar_gen4_pcie_probe() fail path
> V3: - Handle controllers without root port DT node
>     - Swap fail path order
> V4: Rebase on next-20260904
> ---
>  drivers/pci/controller/dwc/pcie-rcar-gen4.c | 44 +++++++++++++++++++--
>  1 file changed, 40 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> index c768a9c7b3b76..89472a4becc46 100644
> --- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> @@ -99,6 +99,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)
> @@ -317,12 +318,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 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);
> +	if (root_port) {
> +		rcar->perst = of_reset_control_get_optional_exclusive(root_port, "perst");
> +		of_node_put(root_port);
> +		if (IS_ERR(rcar->perst))
> +			return dev_err_probe(dev, PTR_ERR(rcar->perst), "Failed to get PERST#\n");
> +	} else {
> +		rcar->perst = NULL;

'rcar->perst' is NULL by default.

> +	}
> +
>  	/* 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);
>  }
> @@ -494,6 +510,22 @@ static int rcar_gen4_pcie_enable_device(struct pci_host_bridge *bridge,
>  	return 0;
>  }
>  
> +static void 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, assert);
> +
> +	if (!rcar->perst)
> +		return;
> +
> +	if (assert)
> +		reset_control_assert(rcar->perst);
> +	else
> +		reset_control_deassert(rcar->perst);

So the controller will only have one form of PERST# implemented at a time. Even
though this code is technically correct, it also gives an impression that both
form could co-exist.

So I'd recommend using:

        if (dw->pe_rst) {
                gpiod_set_value_cansleep(dw->pe_rst, assert);
        } else {
                if (assert)
                        reset_control_assert(rcar->perst);
                else
                        reset_control_deassert(rcar->perst);
        }

to makes it clear that only one form of PERST# is supported.

I'll implement both of these changes while applying.

- Mani

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