[PATCH 1/2] PCI: dwc: rcar-gen4: Replace pe_rst GPIO PERST with GPIO reset aux driver

[PATCH 1/2] PCI: dwc: rcar-gen4: Replace pe_rst GPIO PERST with GPIO reset aux driver

[Marek Vasut]

There is currently one user of core DWC PCIe struct gpio_desc *pe_rst
field, which is R-Car Gen4 driver. This driver already does convoluted
handling of both GPIO controlled PERST# and reset controller controlled
PERST#. Simplify the situation using reset GPIO aux driver.

First, eliminate the pe_rst GPIO from DWC core, so it is not in the
way, this is the easy top half of this patch.

Second, deal with the resets using reset GPIO aux driver in the R-Car
Gen4 PCIe driver. The R-Car Gen4 PCIe driver currently handles two
variants of DT description which can be passed to it:

1) Combined mandatory PCIe core resets and optional GPIO PERST#:
   pciec@nnn {
       compatible = "renesas,r8a779g0-pcie", "renesas,rcar-gen4-pcie";
       ...
       resets = <&reset 624>, <&reset 1121>;
       reset-names = "pwr", "core";
       reset-gpios = <&gpio2 2 GPIO_ACTIVE_LOW>; // This is GPIO PERST#
       ...
   };
   The PCIe core resets are requested already in pcie-designware.c
   dw_pcie_get_resets(), but the reset-gpios GPIO PERST# has to be
   handled later, in R-Car Gen4 PCIe driver host .init callback,
   to avoid interference with the core dw_pcie_get_resets(). The
   GPIO PERST# has to be requested by index since there is no
   reset-names entry for it, using the first unused reset index.

2) Separate mandatory PCIe core resets and optional reset controller PERST#:
   pciec@nnn {
       compatible = "renesas,r8a78000-pcie4", "renesas,rcar-gen5-pcie4";
       ...
       resets = <&reset 345>;
       reset-names = "pwr";
       ...
       pciec40_rp: pci@0,0 {
           compatible = "pciclass,0604";
           ...
           resets = <&reset 815>; // This is reset controller PERST#
           reset-names = "perst";
       };
   };
   The PCIe core resets are requested already in pcie-designware.c
   dw_pcie_get_resets(), and root port reset controller PERST# used
   to be handled in rcar_gen4_pcie_get_resources(), but now the root
   port reset controller PERST# has to be handled later, in R-Car Gen4
   PCIe driver host .init callback, to avoid interference with the core
   dw_pcie_get_resets().

Ultimately, this simplifies both the core and the R-Car Gen4 PCIe driver.

Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
---
Cc: "Krzysztof Wilczyński" <kwilczynski@kernel.org>
Cc: Alex Elder <elder@riscstar.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Bartosz Golaszewski <brgl@kernel.org>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Frank Li <Frank.Li@nxp.com>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Jingoo Han <jingoohan1@gmail.com>
Cc: Koichiro Den <den@valinux.co.jp>
Cc: Lorenzo Pieralisi <lpieralisi@kernel.org>
Cc: Magnus Damm <magnus.damm@gmail.com>
Cc: Manivannan Sadhasivam <mani@kernel.org>
Cc: Neil Armstrong <neil.armstrong@linaro.org>
Cc: Randolph Lin <randolph@andestech.com>
Cc: Rob Herring <robh@kernel.org>
Cc: Sherry Sun <sherry.sun@nxp.com>
Cc: Siddharth Vadapalli <s-vadapalli@ti.com>
Cc: Vidya Sagar <vidyas@nvidia.com>
Cc: Vincent Guittot <vincent.guittot@linaro.org>
Cc: Wilfred Mallawa <wilfred.mallawa@wdc.com>
Cc: Xincheng Zhang <zhangxincheng@ultrarisc.com>
Cc: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Cc: linux-kernel@vger.kernel.org
Cc: linux-pci@vger.kernel.org
Cc: linux-renesas-soc@vger.kernel.org
---
NOTE: Thanks Bartosz for helping me figure the GPIO reset aux driver out
---
 drivers/pci/controller/dwc/Kconfig           |   1 +
 drivers/pci/controller/dwc/pcie-designware.c |   4 -
 drivers/pci/controller/dwc/pcie-designware.h |   1 -
 drivers/pci/controller/dwc/pcie-rcar-gen4.c  | 107 ++++++++++++++-----
 4 files changed, 83 insertions(+), 30 deletions(-)

diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
index 0a0d56fcf0ac9..d65cf1e9d842b 100644
--- a/drivers/pci/controller/dwc/Kconfig
+++ b/drivers/pci/controller/dwc/Kconfig
@@ -350,6 +350,7 @@ config PCIE_RCAR_GEN4_HOST
 	depends on PCI_MSI
 	select PCIE_DW_HOST
 	select PCIE_RCAR_GEN4
+	select RESET_GPIO
 	help
 	  Say Y here if you want PCIe controller (host mode) on R-Car Gen4 SoCs.
 	  To compile this driver as a module, choose M here: the module will be
diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index 12f102f48d5d5..b09c01853d807 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -104,10 +104,6 @@ static int dw_pcie_get_resets(struct dw_pcie *pci)
 	if (ret)
 		return ret;
 
-	pci->pe_rst = devm_gpiod_get_optional(pci->dev, "reset", GPIOD_OUT_HIGH);
-	if (IS_ERR(pci->pe_rst))
-		return PTR_ERR(pci->pe_rst);
-
 	return 0;
 }
 
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 6d788aa000117..9dc86b66e415d 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -575,7 +575,6 @@ struct dw_pcie {
 	struct clk_bulk_data	core_clks[DW_PCIE_NUM_CORE_CLKS];
 	struct reset_control_bulk_data	app_rsts[DW_PCIE_NUM_APP_RSTS];
 	struct reset_control_bulk_data	core_rsts[DW_PCIE_NUM_CORE_RSTS];
-	struct gpio_desc		*pe_rst;
 	bool			suspended;
 	struct debugfs_info	*debugfs;
 	enum			dw_pcie_device_mode mode;
diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
index fceecbd16355c..d3bf01485e11d 100644
--- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
+++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
@@ -401,7 +401,6 @@ 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)) {
@@ -411,18 +410,8 @@ static int rcar_gen4_pcie_get_resources(struct rcar_gen4_pcie *rcar)
 			return PTR_ERR(rcar->phy);
 	}
 
-	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");
-	}
-
 	/* 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);
 }
@@ -594,19 +583,84 @@ static int rcar_gen4_pcie_enable_device(struct pci_host_bridge *bridge,
 	return 0;
 }
 
+static int rcar_gen4_pcie_host_perst_get(struct dw_pcie *dw)
+{
+	struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
+	struct device_node *root_port;
+	struct device *dev = dw->dev;
+	struct reset_control *perst;
+	int index;
+
+	/*
+	 * Handle two forms of DT which can be passed to this driver:
+	 *
+	 * 1) Combined mandatory PCIe core resets and optional GPIO PERST#:
+	 *    pciec@nnn {
+	 *        compatible = "renesas,r8a779g0-pcie", "renesas,rcar-gen4-pcie";
+	 *        ...
+	 *        resets = <&reset 624>, <&reset 1121>;
+	 *        reset-names = "pwr", "core";
+	 *        reset-gpios = <&gpio2 2 GPIO_ACTIVE_LOW>; // This is GPIO PERST#
+	 *        ...
+	 *    };
+	 *    The PCIe core resets are requested already in pcie-designware.c
+	 *    dw_pcie_get_resets(), but the reset-gpios GPIO PERST# has to be
+	 *    handled at this point here. The GPIO PERST# has to be requested
+	 *    by index since there is no reset-names entry for it, using the
+	 *    first unused reset index.
+	 *
+	 * 2) Separate mandatory PCIe core resets and optional reset controller PERST#:
+	 *    pciec@nnn {
+	 *        compatible = "renesas,r8a78000-pcie4", "renesas,rcar-gen5-pcie4";
+	 *        ...
+	 *        resets = <&reset 345>;
+	 *        reset-names = "pwr";
+	 *        ...
+	 *        pciec40_rp: pci@0,0 {
+	 *            compatible = "pciclass,0604";
+	 *            ...
+	 *            resets = <&reset 815>; // This is reset controller PERST#
+	 *            reset-names = "perst";
+	 *        };
+	 *    };
+	 *    The PCIe core resets are requested already in pcie-designware.c
+	 *    dw_pcie_get_resets(), but the root port reset controller PERST#
+	 *    has to be handled at this point here using subnode request.
+	 */
+	index = reset_control_get_count(dev);
+	if (index < 0)
+		return index;
+
+	/* Attempt to request GPIO PERST# in PCIe controller node. */
+	perst = of_reset_control_get_exclusive_by_index(dev->of_node, index);
+	if (!IS_ERR(perst))
+		goto exit;
+
+	/* No GPIO reset, test for root port subnode. */
+	root_port = of_get_next_available_child(dev->of_node, NULL);
+	if (!root_port)
+		return dev_err_probe(dev, -EINVAL, "Failed to get PERST# in root port\n");
+
+	/* Attempt to request reset controller PERST# in root port subnode. */
+	perst = of_reset_control_get_optional_exclusive(root_port, "perst");
+	of_node_put(root_port);
+	if (IS_ERR(perst))
+		return dev_err_probe(dev, PTR_ERR(perst), "Failed to get PERST#\n");
+
+exit:
+	rcar->perst = perst;
+	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);
 
-	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);
-	}
+	if (assert)
+		reset_control_assert(rcar->perst);
+	else
+		reset_control_deassert(rcar->perst);
 }
 
 /* Host mode */
@@ -616,6 +670,10 @@ static int rcar_gen4_pcie_host_init(struct dw_pcie_rp *pp)
 	struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
 	int ret;
 
+	ret = rcar_gen4_pcie_host_perst_get(dw);
+	if (ret)
+		return ret;
+
 	if (pp->bridge)
 		pp->bridge->enable_device = rcar_gen4_pcie_enable_device;
 
@@ -623,7 +681,7 @@ static int rcar_gen4_pcie_host_init(struct dw_pcie_rp *pp)
 
 	ret = rcar->drvdata->init(rcar);
 	if (ret)
-		return ret;
+		goto err_init;
 
 	/*
 	 * According to the section 3.5.7.2 "RC Mode" in DWC PCIe Dual Mode
@@ -646,6 +704,8 @@ static int rcar_gen4_pcie_host_init(struct dw_pcie_rp *pp)
 
 err:
 	rcar->drvdata->deinit(rcar);
+err_init:
+	reset_control_put(rcar->perst);
 	return ret;
 }
 
@@ -656,6 +716,7 @@ static void rcar_gen4_pcie_host_deinit(struct dw_pcie_rp *pp)
 
 	rcar_gen4_pcie_host_perst_assert(pp, true);
 	rcar->drvdata->deinit(rcar);
+	reset_control_put(rcar->perst);
 }
 
 static const struct dw_pcie_host_ops rcar_gen4_pcie_host_ops = {
@@ -830,7 +891,7 @@ static int rcar_gen4_pcie_probe(struct platform_device *pdev)
 
 	err = rcar_gen4_pcie_prepare(rcar);
 	if (err)
-		goto err_prepare;
+		return err;
 
 	err = rcar_gen4_add_dw_pcie(rcar);
 	if (err)
@@ -841,9 +902,6 @@ static int rcar_gen4_pcie_probe(struct platform_device *pdev)
 err_unprepare:
 	rcar_gen4_pcie_unprepare(rcar);
 
-err_prepare:
-	reset_control_put(rcar->perst);
-
 	return err;
 }
 
@@ -867,7 +925,6 @@ static void rcar_gen4_pcie_remove(struct platform_device *pdev)
 
 	rcar_gen4_remove_dw_pcie(rcar);
 	rcar_gen4_pcie_unprepare(rcar);
-	reset_control_put(rcar->perst);
 }
 
 static int r8a779f0_pcie_ltssm_control(struct rcar_gen4_pcie *rcar, bool enable)
-- 
2.53.0

[PATCH 2/2] PCI: rcar-gen4: Inline rcar_gen4_pcie_host_perst_assert() again

[Marek Vasut]

The rcar_gen4_pcie_host_perst_assert() no longer has any purpose,
inline reset_control_assert()/reset_control_deassert() back into
the rest of the code and drop rcar_gen4_pcie_host_perst_assert().

Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
---
Cc: "Krzysztof Wilczyński" <kwilczynski@kernel.org>
Cc: Alex Elder <elder@riscstar.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Bartosz Golaszewski <brgl@kernel.org>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Frank Li <Frank.Li@nxp.com>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Jingoo Han <jingoohan1@gmail.com>
Cc: Koichiro Den <den@valinux.co.jp>
Cc: Lorenzo Pieralisi <lpieralisi@kernel.org>
Cc: Magnus Damm <magnus.damm@gmail.com>
Cc: Manivannan Sadhasivam <mani@kernel.org>
Cc: Neil Armstrong <neil.armstrong@linaro.org>
Cc: Randolph Lin <randolph@andestech.com>
Cc: Rob Herring <robh@kernel.org>
Cc: Sherry Sun <sherry.sun@nxp.com>
Cc: Siddharth Vadapalli <s-vadapalli@ti.com>
Cc: Vidya Sagar <vidyas@nvidia.com>
Cc: Vincent Guittot <vincent.guittot@linaro.org>
Cc: Wilfred Mallawa <wilfred.mallawa@wdc.com>
Cc: Xincheng Zhang <zhangxincheng@ultrarisc.com>
Cc: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Cc: linux-kernel@vger.kernel.org
Cc: linux-pci@vger.kernel.org
Cc: linux-renesas-soc@vger.kernel.org
---
 drivers/pci/controller/dwc/pcie-rcar-gen4.c | 17 +++--------------
 1 file changed, 3 insertions(+), 14 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
index d3bf01485e11d..9a64a9f4ab0d1 100644
--- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
+++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
@@ -652,17 +652,6 @@ static int rcar_gen4_pcie_host_perst_get(struct dw_pcie *dw)
 	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);
-
-	if (assert)
-		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)
 {
@@ -677,7 +666,7 @@ static int rcar_gen4_pcie_host_init(struct dw_pcie_rp *pp)
 	if (pp->bridge)
 		pp->bridge->enable_device = rcar_gen4_pcie_enable_device;
 
-	rcar_gen4_pcie_host_perst_assert(pp, true);
+	reset_control_assert(rcar->perst);
 
 	ret = rcar->drvdata->init(rcar);
 	if (ret)
@@ -698,7 +687,7 @@ static int rcar_gen4_pcie_host_init(struct dw_pcie_rp *pp)
 
 	msleep(PCIE_T_PVPERL_MS);	/* pe_rst requires 100msec delay */
 
-	rcar_gen4_pcie_host_perst_assert(pp, false);
+	reset_control_deassert(rcar->perst);
 
 	return 0;
 
@@ -714,7 +703,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);
 
-	rcar_gen4_pcie_host_perst_assert(pp, true);
+	reset_control_assert(rcar->perst);
 	rcar->drvdata->deinit(rcar);
 	reset_control_put(rcar->perst);
 }
-- 
2.53.0

Re: [PATCH 1/2] PCI: dwc: rcar-gen4: Replace pe_rst GPIO PERST with GPIO reset aux driver

[Marek Vasut]

On 10/28/26 6:21 PM, Marek Vasut wrote:

[...]

> 1) Combined mandatory PCIe core resets and optional GPIO PERST#:
>     pciec@nnn {
>         compatible = "renesas,r8a779g0-pcie", "renesas,rcar-gen4-pcie";
>         ...
>         resets = <&reset 624>, <&reset 1121>;
>         reset-names = "pwr", "core";
>         reset-gpios = <&gpio2 2 GPIO_ACTIVE_LOW>; // This is GPIO PERST#
>         ...
>     };

[...]

> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index 12f102f48d5d5..b09c01853d807 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -104,10 +104,6 @@ static int dw_pcie_get_resets(struct dw_pcie *pci)
>   	if (ret)
>   		return ret;
>   
> -	pci->pe_rst = devm_gpiod_get_optional(pci->dev, "reset", GPIOD_OUT_HIGH);
> -	if (IS_ERR(pci->pe_rst))
> -		return PTR_ERR(pci->pe_rst);
Bartosz, alternatively, I wonder if there is any way to request the 
reset-gpios as reset controller reset here already. That might be even 
cleaner, but I don't think there is anything like 
devm_reset_control_bulk_get_optional_exclusive_and_also_request_the_gpio_reset() 
or devm_reset_control_get_optional_exclusive_gpio_reset() . Maybe 
something like that could be added to the reset subsystem instead ?

Thank you for your help !

Re: [PATCH 2/2] PCI: rcar-gen4: Inline rcar_gen4_pcie_host_perst_assert() again

[sashiko-bot]

> The rcar_gen4_pcie_host_perst_assert() no longer has any purpose,
> inline reset_control_assert()/reset_control_deassert() back into
> the rest of the code and drop rcar_gen4_pcie_host_perst_assert().
> 
> Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>

Sashiko has reviewed this patch and found no issues.

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

Re: [PATCH 1/2] PCI: dwc: rcar-gen4: Replace pe_rst GPIO PERST with GPIO reset aux driver

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] PCI: dwc: rcar-gen4: broken error handling in rcar_gen4_pcie_host_perst_get()
- [High] PCI: dwc: rcar-gen4: rcar->perst UAF across suspend/resume in host_deinit()
- [Medium] PCI: dwc: Kconfig: PCIE_RCAR_GEN4_HOST selects RESET_GPIO without dependencies
--

commit 5d439d30a4d77a2c89c803394ba82b6874d3a0c5
Author: Marek Vasut <marek.vasut+renesas@mailbox.org>

PCI: dwc: rcar-gen4: Replace pe_rst GPIO PERST with GPIO reset aux driver

This commit removes the pe_rst GPIO descriptor from the DesignWare PCIe core
and switches the R-Car Gen4 PCIe host driver to acquire PERST# as a reset
control via the reset-gpio auxiliary driver or from a root port subnode in
rcar_gen4_pcie_host_init().

> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> index 0a0d56fcf0ac9..d65cf1e9d842b 100644
> --- a/drivers/pci/controller/dwc/Kconfig
> +++ b/drivers/pci/controller/dwc/Kconfig
> @@ -350,6 +350,7 @@ config PCIE_RCAR_GEN4_HOST
>  	depends on PCI_MSI
>  	select PCIE_DW_HOST
>  	select PCIE_RCAR_GEN4
> +	select RESET_GPIO

[Severity: Medium]
In drivers/reset/Kconfig, RESET_GPIO is guarded by if RESET_CONTROLLER and
has depends on GPIOLIB:

drivers/reset/Kconfig:
if RESET_CONTROLLER
...
config RESET_GPIO
	tristate "GPIO reset controller"
	depends on GPIOLIB
	select AUXILIARY_BUS

Since PCIE_RCAR_GEN4_HOST only depends on ARM64 && (ARCH_RENESAS ||
COMPILE_TEST) and PCI_MSI, can selecting RESET_GPIO here trigger Kconfig
unmet direct dependency warnings and build failures when RESET_CONTROLLER=n
or GPIOLIB=n (for example with COMPILE_TEST=y)?

> diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> index fceecbd16355c..d3bf01485e11d 100644
> --- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
[ ... ]
> @@ -594,19 +583,84 @@ static int rcar_gen4_pcie_enable_device(struct pci_host_bridge *bridge,
>  	return 0;
>  }
>  
> +static int rcar_gen4_pcie_host_perst_get(struct dw_pcie *dw)
> +{
> +	struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
> +	struct device_node *root_port;
> +	struct device *dev = dw->dev;
> +	struct reset_control *perst;
> +	int index;
[ ... ]
> +	index = reset_control_get_count(dev);
> +	if (index < 0)
> +		return index;
> +
> +	/* Attempt to request GPIO PERST# in PCIe controller node. */
> +	perst = of_reset_control_get_exclusive_by_index(dev->of_node, index);
> +	if (!IS_ERR(perst))
> +		goto exit;
> +
> +	/* No GPIO reset, test for root port subnode. */
> +	root_port = of_get_next_available_child(dev->of_node, NULL);
> +	if (!root_port)
> +		return dev_err_probe(dev, -EINVAL, "Failed to get PERST# in root port\n");
> +
> +	/* Attempt to request reset controller PERST# in root port subnode. */
> +	perst = of_reset_control_get_optional_exclusive(root_port, "perst");
> +	of_node_put(root_port);
> +	if (IS_ERR(perst))
> +		return dev_err_probe(dev, PTR_ERR(perst), "Failed to get PERST#\n");

[Severity: High]
What happens in rcar_gen4_pcie_host_perst_get() when reset-gpios is present
in dev->of_node, but of_reset_control_get_exclusive_by_index() returns
ERR_PTR(-EPROBE_DEFER) because the GPIO provider or the reset-gpio auxiliary
driver has not bound yet?

In drivers/reset/core.c:__fwnode_reset_control_get(), if reset-gpios is
present in the node, __reset_add_reset_gpio_device() or __reset_find_rcdev()
returns -EPROBE_DEFER until the GPIO device and the reset.gpio auxiliary
device are ready.

Because rcar_gen4_pcie_host_perst_get() does not check whether PTR_ERR(perst)
is something other than -ENOENT before falling through to the root_port
lookup:

1. On R-Car V4H boards (such as r8a779g0.dtsi with White Hawk or Sparrow
Hawk), the controller node has a pci@0,0 child node without a "perst" reset
property while reset-gpios is in the controller node. Falling through on
-EPROBE_DEFER causes of_reset_control_get_optional_exclusive(root_port,
"perst") to return NULL, so rcar->perst is set to NULL and probe succeeds
without acquiring or deasserting the GPIO PERST#.

2. On R-Car S4 and V4M boards (such as r8a779f0.dtsi Spider and r8a779h0.dtsi
Gray Hawk), there is no root port child node, so falling through on
-EPROBE_DEFER hits if (!root_port) and fails probe permanently with -EINVAL
instead of deferring.

3. Since reset-gpios is optional according to the binding and commit
description, if a controller node has neither reset-gpios nor a root port
child node, of_reset_control_get_exclusive_by_index() returns ERR_PTR(-ENOENT)
and if (!root_port) now fails probe with -EINVAL instead of treating PERST# as
optional.

Should this check for PTR_ERR(perst) != -ENOENT after
of_reset_control_get_exclusive_by_index(), and allow a missing root_port when
PTR_ERR(perst) == -ENOENT?

[ ... ]
> @@ -616,6 +670,10 @@ static int rcar_gen4_pcie_host_init(struct dw_pcie_rp *pp)
>  	struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
>  	int ret;
>  
> +	ret = rcar_gen4_pcie_host_perst_get(dw);
> +	if (ret)
> +		return ret;
> +
>  	if (pp->bridge)
>  		pp->bridge->enable_device = rcar_gen4_pcie_enable_device;
>  
> @@ -623,7 +681,7 @@ static int rcar_gen4_pcie_host_init(struct dw_pcie_rp *pp)
>  
>  	ret = rcar->drvdata->init(rcar);
>  	if (ret)
> -		return ret;
> +		goto err_init;
>  
>  	/*
>  	 * According to the section 3.5.7.2 "RC Mode" in DWC PCIe Dual Mode
> @@ -646,6 +704,8 @@ static int rcar_gen4_pcie_host_init(struct dw_pcie_rp *pp)
>  
>  err:
>  	rcar->drvdata->deinit(rcar);
> +err_init:
> +	reset_control_put(rcar->perst);
>  	return ret;
>  }
>  
> @@ -656,6 +716,7 @@ static void rcar_gen4_pcie_host_deinit(struct dw_pcie_rp *pp)
>  
>  	rcar_gen4_pcie_host_perst_assert(pp, true);
>  	rcar->drvdata->deinit(rcar);
> +	reset_control_put(rcar->perst);
>  }

[Severity: High]
Can moving rcar_gen4_pcie_host_perst_get() and reset_control_put(rcar->perst)
into the dw_pcie_host_ops .init and .deinit callbacks cause a use-after-free
and double-free of rcar->perst across system suspend/resume followed by
driver removal?

In drivers/pci/controller/dwc/pcie-designware-host.c, the .deinit and .init
callbacks are also invoked during noirq suspend and resume:

dw_pcie_suspend_noirq()
  pci->pp.ops->deinit(&pci->pp)
    rcar_gen4_pcie_host_deinit()
      reset_control_put(rcar->perst);

dw_pcie_resume_noirq()
  pci->pp.ops->init(&pci->pp)
    rcar_gen4_pcie_host_init()
  ...
err_deinit:
  pci->pp.ops->deinit(&pci->pp)
    rcar_gen4_pcie_host_deinit()
      reset_control_put(rcar->perst);

When dw_pcie_suspend_noirq() runs, rcar_gen4_pcie_host_deinit() frees
rcar->perst via reset_control_put() while leaving the dangling pointer in
rcar->perst.

If the device is later unbound while suspended, or if dw_pcie_resume_noirq()
fails (either inside rcar_gen4_pcie_host_init() or at err_deinit: when link
training times out in dw_pcie_wait_for_link()), a subsequent
rcar_gen4_pcie_remove() -> dw_pcie_host_deinit() ->
rcar_gen4_pcie_host_deinit() will call reset_control_assert(rcar->perst) and
reset_control_put(rcar->perst) on the already-freed rcar->perst pointer.

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