[PATCH v4 0/8] PCI: rzg3s-host: Add PCIe hotplug support

[PATCH v4 0/8] PCI: rzg3s-host: Add PCIe hotplug support

[Claudiu Beznea]

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Hi,

Series adds hotplug support for the rzg3s-host driver. This is a
continuation of the series John posted initially at [1]. Along with it,
prerequisites fixes and cleanup patches were added.

Series applies on top of [2].

Thank you,
Claudiu

Changes in v4:
- simplified comments in patch 2/8
- kept only DL_UpDown changes in patch 7/8
- added patch 8/8 for the .reset_root_port() support

Changes in v3:
- added patches 1-6
- re-worked patch 7 to use struct pci_host_bridge::reset_root_port() API

[1] https://lore.kernel.org/all/20260630141720.3938514-1-john.madieu.xa@bp.renesas.com/
[2] https://lore.kernel.org/all/20260811200013.185956-1-prabhakar.mahadev-lad.rj@bp.renesas.com/

Claudiu Beznea (7):
  PCI: rzg3s-host: Follow hardware manual clock/reset initialization
    order
  PCI: rzg3s-host: Fix runtime PM handling in the NOIRQ suspend/resume
    phase
  PCI: rzg3s-host: Drop nop instructions
  PCI: rzg3s-host: Move host configuration code together
  PCI: rzg3s-host: Move suspend/resume code into dedicated functions
  PCI: rzg3s-host: Move IRQ domain setup code
  PCI: rzg3s-host: Add bridge::reset_root_port()

John Madieu (1):
  PCI: rzg3s-host: Re-enumerate the bus on PCIe link-state changes

 drivers/pci/controller/pcie-rzg3s-host.c | 815 ++++++++++++++---------
 1 file changed, 508 insertions(+), 307 deletions(-)

-- 
2.43.0

[PATCH v4 1/8] PCI: rzg3s-host: Follow hardware manual clock/reset initialization order

[Claudiu Beznea]

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

The RZ/G3S PCIe hardware manual specifies that the clocks must be enabled
before the reset signals are deasserted during initialization.

Follow this sequence in the probe(), suspend(), and resume() paths to
match the hardware requirements and avoid potential issues.

Fixes: 7ef502fb35b2 ("PCI: Add Renesas RZ/G3S host controller driver")
Cc: stable@vger.kernel.org
Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v4:
- none

Changes in v3:
- none, this patch is new

 drivers/pci/controller/pcie-rzg3s-host.c | 40 +++++++++++++-----------
 1 file changed, 21 insertions(+), 19 deletions(-)

diff --git a/drivers/pci/controller/pcie-rzg3s-host.c b/drivers/pci/controller/pcie-rzg3s-host.c
index 077cfb0834b3..e1105b1f2652 100644
--- a/drivers/pci/controller/pcie-rzg3s-host.c
+++ b/drivers/pci/controller/pcie-rzg3s-host.c
@@ -1888,10 +1888,6 @@ static int rzg3s_pcie_probe(struct platform_device *pdev)
 	if (ret)
 		goto sysc_signal_restore;
 
-	ret = rzg3s_pcie_power_resets_deassert(host);
-	if (ret)
-		goto sysc_signal_restore;
-
 	pm_runtime_enable(dev);
 
 	/*
@@ -1902,12 +1898,16 @@ static int rzg3s_pcie_probe(struct platform_device *pdev)
 	if (ret)
 		goto rpm_disable;
 
+	ret = rzg3s_pcie_power_resets_deassert(host);
+	if (ret)
+		goto rpm_put;
+
 	raw_spin_lock_init(&host->hw_lock);
 
 	ret = rzg3s_pcie_host_setup(host, rzg3s_pcie_init_irqdomain,
 				    rzg3s_pcie_teardown_irqdomain);
 	if (ret)
-		goto rpm_put;
+		goto power_resets_assert;
 
 	bridge->sysdata = host;
 	bridge->ops = &rzg3s_pcie_root_ops;
@@ -1922,12 +1922,13 @@ static int rzg3s_pcie_probe(struct platform_device *pdev)
 	clk_disable_unprepare(host->port.refclk);
 	rzg3s_pcie_teardown_irqdomain(host);
 	host->data->config_deinit(host);
+power_resets_assert:
+	reset_control_bulk_assert(host->data->num_power_resets,
+				  host->power_resets);
 rpm_put:
 	pm_runtime_put_sync(dev);
 rpm_disable:
 	pm_runtime_disable(dev);
-	reset_control_bulk_assert(host->data->num_power_resets,
-				  host->power_resets);
 sysc_signal_restore:
 	/*
 	 * SYSC RST_RSM_B signal need to be asserted before turning off the
@@ -1948,10 +1949,6 @@ static int rzg3s_pcie_suspend_noirq(struct device *dev)
 	struct rzg3s_sysc *sysc = host->sysc;
 	int ret;
 
-	ret = pm_runtime_put_sync(dev);
-	if (ret)
-		return ret;
-
 	clk_disable_unprepare(port->refclk);
 
 	/* SoC-specific de-initialization */
@@ -1964,13 +1961,19 @@ static int rzg3s_pcie_suspend_noirq(struct device *dev)
 	if (ret)
 		goto config_reinit;
 
-	ret = rzg3s_sysc_config_func(sysc, RZG3S_SYSC_FUNC_ID_RST_RSM_B, 0);
+	ret = pm_runtime_put_sync(dev);
 	if (ret)
 		goto power_resets_restore;
 
+	ret = rzg3s_sysc_config_func(sysc, RZG3S_SYSC_FUNC_ID_RST_RSM_B, 0);
+	if (ret)
+		goto rpm_resume;
+
 	return 0;
 
 	/* Restore the previous state if any error happens */
+rpm_resume:
+	pm_runtime_resume_and_get(dev);
 power_resets_restore:
 	reset_control_bulk_deassert(data->num_power_resets,
 				    host->power_resets);
@@ -1980,7 +1983,6 @@ static int rzg3s_pcie_suspend_noirq(struct device *dev)
 	data->config_post_init(host);
 refclk_restore:
 	clk_prepare_enable(port->refclk);
-	pm_runtime_resume_and_get(dev);
 	return ret;
 }
 
@@ -2009,18 +2011,18 @@ static int rzg3s_pcie_resume_noirq(struct device *dev)
 			goto assert_rst_rsm_b;
 	}
 
-	ret = rzg3s_pcie_power_resets_deassert(host);
+	ret = pm_runtime_resume_and_get(dev);
 	if (ret)
 		goto assert_rst_rsm_b;
 
-	ret = pm_runtime_resume_and_get(dev);
+	ret = rzg3s_pcie_power_resets_deassert(host);
 	if (ret)
-		goto assert_power_resets;
+		goto rpm_put;
 
 	ret = rzg3s_pcie_host_setup(host, rzg3s_pcie_msi_hw_setup,
 				    rzg3s_pcie_msi_hw_teardown);
 	if (ret)
-		goto rpm_put;
+		goto assert_power_resets;
 
 	return 0;
 
@@ -2028,11 +2030,11 @@ static int rzg3s_pcie_resume_noirq(struct device *dev)
 	 * If any error happens there is no way to recover the IP. Put it in the
 	 * lowest possible power state.
 	 */
-rpm_put:
-	pm_runtime_put_sync(dev);
 assert_power_resets:
 	reset_control_bulk_assert(data->num_power_resets,
 				  host->power_resets);
+rpm_put:
+	pm_runtime_put_sync(dev);
 assert_rst_rsm_b:
 	rzg3s_sysc_config_func(sysc, RZG3S_SYSC_FUNC_ID_RST_RSM_B, 0);
 	return ret;
-- 
2.43.0

[PATCH v4 2/8] PCI: rzg3s-host: Fix runtime PM handling in the NOIRQ suspend/resume phase

[Claudiu Beznea]

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

The runtime PM documentation states the following:
- During system suspend, pm_runtime_get_noresume() is called for every
  device right before executing the subsystem-level .prepare() callback
  (in device_prepare()). In addition, the PM core disables runtime PM for
  every device right before executing the subsystem-level .suspend_late()
  callback (in device_suspend_late()).
- During system resume, pm_runtime_enable() is called for every device
  right after executing the subsystem-level .resume_early() callback (in
  device_resume_early()), and pm_runtime_put() is called right after
  executing the subsystem-level .complete() callback (in
  device_complete()).

The driver's .suspend_noirq() callback is invoked after .suspend_late(),
while .resume_noirq() is invoked before .resume_early().

If:
- the device is not part of the wake-up path, and
- its runtime PM status is not RPM_SUSPENDED,
the generic power domain .suspend_noirq()/.resume_noirq() callbacks
(genpd_suspend_noirq()/genpd_resume_noirq()) invoke the driver's
.suspend_noirq()/.resume_noirq() callbacks and call
genpd_stop_dev()/genpd_start_dev() before and after them, respectively.

Calling genpd_stop_dev()/genpd_start_dev() allows devices whose power is
controlled by generic power domains to be powered off and on during
system suspend and resume, even though their runtime PM usage count does
not reach zero.

Since the runtime PM usage count is incremented in device_prepare() and
decremented in device_complete(), runtime PM operations performed from
the driver's .suspend_noirq()/.resume_noirq() callbacks are no-ops. The
actual power transitions are handled by the generic power domain
.suspend_noirq()/.resume_noirq() callbacks.

Moreover, attempting to runtime resume a device while runtime PM is
disabled may return -EACCES. This may cause system resume to fail when
resuming after a failed Root Port reset, as described in a subsequent
patch adding hot-plug support.

Remove the runtime PM calls from the driver's
.suspend_noirq()/.resume_noirq() callbacks and rely on the generic power
domain callbacks to power the device off and on.

Fixes: 7ef502fb35b2 ("PCI: Add Renesas RZ/G3S host controller driver")
Cc: stable@vger.kernel.org
Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v4:
- simplified the comments in rzg3s_pcie_suspend_noirq()/rzg3s_pcie_resume_noirq()

Changes in v3:
- none, this patch is new

 drivers/pci/controller/pcie-rzg3s-host.c | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/pci/controller/pcie-rzg3s-host.c b/drivers/pci/controller/pcie-rzg3s-host.c
index e1105b1f2652..0ef49bb5ab1a 100644
--- a/drivers/pci/controller/pcie-rzg3s-host.c
+++ b/drivers/pci/controller/pcie-rzg3s-host.c
@@ -1961,19 +1961,18 @@ static int rzg3s_pcie_suspend_noirq(struct device *dev)
 	if (ret)
 		goto config_reinit;
 
-	ret = pm_runtime_put_sync(dev);
-	if (ret)
-		goto power_resets_restore;
+	/*
+	 * Since the power domain's genpd_suspend_noirq() will disable clocks,
+	 * there is no need to manually invoke runtime PM API here.
+	 */
 
 	ret = rzg3s_sysc_config_func(sysc, RZG3S_SYSC_FUNC_ID_RST_RSM_B, 0);
 	if (ret)
-		goto rpm_resume;
+		goto power_resets_restore;
 
 	return 0;
 
 	/* Restore the previous state if any error happens */
-rpm_resume:
-	pm_runtime_resume_and_get(dev);
 power_resets_restore:
 	reset_control_bulk_deassert(data->num_power_resets,
 				    host->power_resets);
@@ -2011,13 +2010,14 @@ static int rzg3s_pcie_resume_noirq(struct device *dev)
 			goto assert_rst_rsm_b;
 	}
 
-	ret = pm_runtime_resume_and_get(dev);
-	if (ret)
-		goto assert_rst_rsm_b;
+	/*
+	 * Since the power domain's genpd_resume_noirq() will enable clocks,
+	 * there is no need to manually invoke runtime PM API here.
+	 */
 
 	ret = rzg3s_pcie_power_resets_deassert(host);
 	if (ret)
-		goto rpm_put;
+		goto assert_rst_rsm_b;
 
 	ret = rzg3s_pcie_host_setup(host, rzg3s_pcie_msi_hw_setup,
 				    rzg3s_pcie_msi_hw_teardown);
@@ -2033,8 +2033,6 @@ static int rzg3s_pcie_resume_noirq(struct device *dev)
 assert_power_resets:
 	reset_control_bulk_assert(data->num_power_resets,
 				  host->power_resets);
-rpm_put:
-	pm_runtime_put_sync(dev);
 assert_rst_rsm_b:
 	rzg3s_sysc_config_func(sysc, RZG3S_SYSC_FUNC_ID_RST_RSM_B, 0);
 	return ret;
-- 
2.43.0

[PATCH v4 3/8] PCI: rzg3s-host: Drop nop instructions

[Claudiu Beznea]

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

On the RZ/G3S PCIe IP variant (which is similar to those used on the
RZ/G3E, RZ/V2H, and RZ/V2N SoCs), access to the PCIe Type 1 registers
requires setting the PCI_PERM.CFG_HWINIT_EN bit. Since this bit is not
set on the code paths where rzg3s_pcie_set_max_link_speed() is called,
the writes to PCI_EXP_LNKCTL2.TLS are nops. Drop them.

Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v4:
- none

Changes in v3:
- none, this patch is new

 drivers/pci/controller/pcie-rzg3s-host.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/pci/controller/pcie-rzg3s-host.c b/drivers/pci/controller/pcie-rzg3s-host.c
index 0ef49bb5ab1a..362aa923978f 100644
--- a/drivers/pci/controller/pcie-rzg3s-host.c
+++ b/drivers/pci/controller/pcie-rzg3s-host.c
@@ -1144,11 +1144,6 @@ static int rzg3s_pcie_set_max_link_speed(struct rzg3s_pcie_host *host)
 	    remote_supported_link_speeds != max_supported_link_speeds)
 		return 0;
 
-	/* Set target Link speed */
-	rzg3s_pcie_update_bits(host->pcie, pcie_cap + PCI_EXP_LNKCTL2,
-			       PCI_EXP_LNKCTL2_TLS,
-			       FIELD_PREP(PCI_EXP_LNKCTL2_TLS, link_speed));
-
 	/* Request link speed change */
 	rzg3s_pcie_update_bits(host->axi, RZG3S_PCI_PCCTRL2,
 			       RZG3S_PCI_PCCTRL2_LS_CHG_REQ |
-- 
2.43.0

[PATCH v4 4/8] PCI: rzg3s-host: Move host configuration code together

[Claudiu Beznea]

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Move host configuration code together to have it grouped. This prepares for
the addition of hotplug support.

Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v4:
- none

Changes in v3:
- none, this patch is new

 drivers/pci/controller/pcie-rzg3s-host.c | 230 +++++++++++------------
 1 file changed, 115 insertions(+), 115 deletions(-)

diff --git a/drivers/pci/controller/pcie-rzg3s-host.c b/drivers/pci/controller/pcie-rzg3s-host.c
index 362aa923978f..3ecada238402 100644
--- a/drivers/pci/controller/pcie-rzg3s-host.c
+++ b/drivers/pci/controller/pcie-rzg3s-host.c
@@ -1356,121 +1356,6 @@ static int rzg3s_pcie_resets_prepare_and_get(struct rzg3s_pcie_host *host)
 							      host->cfg_resets);
 }
 
-static int rzg3s_pcie_host_parse_port(struct rzg3s_pcie_host *host)
-{
-	struct device_node *of_port __free(device_node) =
-		of_get_next_child(host->dev->of_node, NULL);
-	struct rzg3s_pcie_port *port = &host->port;
-	int ret;
-
-	ret = of_property_read_u32(of_port, "vendor-id", &port->vendor_id);
-	if (ret)
-		return ret;
-
-	ret = of_property_read_u32(of_port, "device-id", &port->device_id);
-	if (ret)
-		return ret;
-
-	port->refclk = of_clk_get_by_name(of_port, "ref");
-	if (IS_ERR(port->refclk))
-		return PTR_ERR(port->refclk);
-
-	return 0;
-}
-
-static int rzg3s_pcie_host_init_port(struct rzg3s_pcie_host *host)
-{
-	struct rzg3s_pcie_port *port = &host->port;
-	struct device *dev = host->dev;
-	int ret;
-
-	/* Enable access control to the CFGU */
-	writel_relaxed(RZG3S_PCI_PERM_CFG_HWINIT_EN,
-		       host->axi + RZG3S_PCI_PERM);
-
-	/* Update vendor ID and device ID */
-	writew_relaxed(port->vendor_id, host->pcie + PCI_VENDOR_ID);
-	writew_relaxed(port->device_id, host->pcie + PCI_DEVICE_ID);
-
-	/* Disable access control to the CFGU */
-	writel_relaxed(0, host->axi + RZG3S_PCI_PERM);
-
-	ret = clk_prepare_enable(port->refclk);
-	if (ret)
-		return dev_err_probe(dev, ret, "Failed to enable refclk!\n");
-
-	/* Set the PHY, if any */
-	if (host->data->init_phy) {
-		ret = host->data->init_phy(host);
-		if (ret) {
-			dev_err_probe(dev, ret, "Failed to set the PHY!\n");
-			goto refclk_disable;
-		}
-	}
-
-	return 0;
-
-refclk_disable:
-	clk_disable_unprepare(port->refclk);
-	return ret;
-}
-
-static int rzg3s_pcie_host_init(struct rzg3s_pcie_host *host)
-{
-	u32 val;
-	int ret;
-
-	/* SoC-specific pre-configuration */
-	if (host->data->config_pre_init)
-		host->data->config_pre_init(host);
-
-	/* Initialize the PCIe related registers */
-	ret = rzg3s_pcie_config_init(host);
-	if (ret)
-		goto config_deinit;
-
-	ret = rzg3s_pcie_host_init_port(host);
-	if (ret)
-		goto config_deinit;
-
-	/* Enable ASPM L1 transition for SoCs that use it */
-	ret = rzg3s_sysc_config_func(host->sysc,
-				     RZG3S_SYSC_FUNC_ID_L1_ALLOW, 1);
-	if (ret)
-		goto config_deinit_and_refclk;
-
-	/* Initialize the interrupts */
-	rzg3s_pcie_irq_init(host);
-
-	/* SoC-specific post-configuration */
-	ret = host->data->config_post_init(host);
-	if (ret)
-		goto config_deinit_and_refclk;
-
-	/* Wait for link up */
-	ret = readl_poll_timeout(host->axi + RZG3S_PCI_PCSTAT1, val,
-				 !(val & RZG3S_PCI_PCSTAT1_DL_DOWN_STS),
-				 PCIE_LINK_WAIT_SLEEP_MS * MILLI,
-				 PCIE_LINK_WAIT_SLEEP_MS * MILLI *
-				 PCIE_LINK_WAIT_MAX_RETRIES);
-	if (ret)
-		goto config_deinit_post;
-
-	val = readl_relaxed(host->axi + RZG3S_PCI_PCSTAT2);
-	dev_info(host->dev, "PCIe link status [0x%x]\n", val);
-
-	return 0;
-
-config_deinit_post:
-	host->data->config_deinit(host);
-config_deinit_and_refclk:
-	clk_disable_unprepare(host->port.refclk);
-config_deinit:
-	if (host->data->config_pre_init)
-		host->data->config_deinit(host);
-	return ret;
-}
-
 static void rzg3s_pcie_set_inbound_window(struct rzg3s_pcie_host *host,
 					  u64 cpu_addr, u64 pci_addr, u64 size,
 					  int id)
@@ -1698,6 +1583,121 @@ static int rzg3s_soc_pcie_init_phy(struct rzg3s_pcie_host *host)
 	return 0;
 }
 
+static int rzg3s_pcie_host_parse_port(struct rzg3s_pcie_host *host)
+{
+	struct device_node *of_port __free(device_node) =
+		of_get_next_child(host->dev->of_node, NULL);
+	struct rzg3s_pcie_port *port = &host->port;
+	int ret;
+
+	ret = of_property_read_u32(of_port, "vendor-id", &port->vendor_id);
+	if (ret)
+		return ret;
+
+	ret = of_property_read_u32(of_port, "device-id", &port->device_id);
+	if (ret)
+		return ret;
+
+	port->refclk = of_clk_get_by_name(of_port, "ref");
+	if (IS_ERR(port->refclk))
+		return PTR_ERR(port->refclk);
+
+	return 0;
+}
+
+static int rzg3s_pcie_host_init_port(struct rzg3s_pcie_host *host)
+{
+	struct rzg3s_pcie_port *port = &host->port;
+	struct device *dev = host->dev;
+	int ret;
+
+	/* Enable access control to the CFGU */
+	writel_relaxed(RZG3S_PCI_PERM_CFG_HWINIT_EN,
+		       host->axi + RZG3S_PCI_PERM);
+
+	/* Update vendor ID and device ID */
+	writew_relaxed(port->vendor_id, host->pcie + PCI_VENDOR_ID);
+	writew_relaxed(port->device_id, host->pcie + PCI_DEVICE_ID);
+
+	/* Disable access control to the CFGU */
+	writel_relaxed(0, host->axi + RZG3S_PCI_PERM);
+
+	ret = clk_prepare_enable(port->refclk);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to enable refclk!\n");
+
+	/* Set the PHY, if any */
+	if (host->data->init_phy) {
+		ret = host->data->init_phy(host);
+		if (ret) {
+			dev_err_probe(dev, ret, "Failed to set the PHY!\n");
+			goto refclk_disable;
+		}
+	}
+
+	return 0;
+
+refclk_disable:
+	clk_disable_unprepare(port->refclk);
+	return ret;
+}
+
+static int rzg3s_pcie_host_init(struct rzg3s_pcie_host *host)
+{
+	u32 val;
+	int ret;
+
+	/* SoC-specific pre-configuration */
+	if (host->data->config_pre_init)
+		host->data->config_pre_init(host);
+
+	/* Initialize the PCIe related registers */
+	ret = rzg3s_pcie_config_init(host);
+	if (ret)
+		goto config_deinit;
+
+	ret = rzg3s_pcie_host_init_port(host);
+	if (ret)
+		goto config_deinit;
+
+	/* Enable ASPM L1 transition for SoCs that use it */
+	ret = rzg3s_sysc_config_func(host->sysc,
+				     RZG3S_SYSC_FUNC_ID_L1_ALLOW, 1);
+	if (ret)
+		goto config_deinit_and_refclk;
+
+	/* Initialize the interrupts */
+	rzg3s_pcie_irq_init(host);
+
+	/* SoC-specific post-configuration */
+	ret = host->data->config_post_init(host);
+	if (ret)
+		goto config_deinit_and_refclk;
+
+	/* Wait for link up */
+	ret = readl_poll_timeout(host->axi + RZG3S_PCI_PCSTAT1, val,
+				 !(val & RZG3S_PCI_PCSTAT1_DL_DOWN_STS),
+				 PCIE_LINK_WAIT_SLEEP_MS * MILLI,
+				 PCIE_LINK_WAIT_SLEEP_MS * MILLI *
+				 PCIE_LINK_WAIT_MAX_RETRIES);
+	if (ret)
+		goto config_deinit_post;
+
+	val = readl_relaxed(host->axi + RZG3S_PCI_PCSTAT2);
+	dev_info(host->dev, "PCIe link status [0x%x]\n", val);
+
+	return 0;
+
+config_deinit_post:
+	host->data->config_deinit(host);
+config_deinit_and_refclk:
+	clk_disable_unprepare(host->port.refclk);
+config_deinit:
+	if (host->data->config_pre_init)
+		host->data->config_deinit(host);
+	return ret;
+}
+
 static int
 rzg3s_pcie_host_setup(struct rzg3s_pcie_host *host,
 		      int (*init_irqdomain)(struct rzg3s_pcie_host *host),
-- 
2.43.0

[PATCH v4 5/8] PCI: rzg3s-host: Move suspend/resume code into dedicated functions

[Claudiu Beznea]

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

In preparation for implementing hotplug using
pci_host_bridge::reset_root_port(), move the suspend/resume code into
rzg3s_pcie_host_stop() and rzg3s_pcie_host_start(). These functions
will later be reused by the hotplug implementation through
pci_host_bridge::reset_root_port().

Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v4:
- none

Changes in v3:
- none, this patch is new

 drivers/pci/controller/pcie-rzg3s-host.c | 181 ++++++++++++-----------
 1 file changed, 96 insertions(+), 85 deletions(-)

diff --git a/drivers/pci/controller/pcie-rzg3s-host.c b/drivers/pci/controller/pcie-rzg3s-host.c
index 3ecada238402..40d5ef3e347e 100644
--- a/drivers/pci/controller/pcie-rzg3s-host.c
+++ b/drivers/pci/controller/pcie-rzg3s-host.c
@@ -1742,6 +1742,100 @@ rzg3s_pcie_host_setup(struct rzg3s_pcie_host *host,
 	return ret;
 }
 
+static int rzg3s_pcie_host_stop(struct rzg3s_pcie_host *host)
+{
+	const struct rzg3s_pcie_soc_data *data = host->data;
+	struct rzg3s_pcie_port *port = &host->port;
+	struct rzg3s_sysc *sysc = host->sysc;
+	int ret;
+
+	clk_disable_unprepare(port->refclk);
+
+	/* SoC-specific de-initialization */
+	ret = data->config_deinit(host);
+	if (ret)
+		goto refclk_restore;
+
+	ret = reset_control_bulk_assert(data->num_power_resets,
+					host->power_resets);
+	if (ret)
+		goto config_reinit;
+
+	/*
+	 * Since the power domain's genpd_suspend_noirq() will disable clocks,
+	 * there is no need to manually invoke runtime PM API here.
+	 */
+
+	ret = rzg3s_sysc_config_func(sysc, RZG3S_SYSC_FUNC_ID_RST_RSM_B, 0);
+	if (ret)
+		goto power_resets_restore;
+
+	return 0;
+
+	/* Restore the previous state if any error happens */
+power_resets_restore:
+	reset_control_bulk_deassert(data->num_power_resets,
+				    host->power_resets);
+config_reinit:
+	if (data->config_pre_init)
+		data->config_pre_init(host);
+	data->config_post_init(host);
+refclk_restore:
+	clk_prepare_enable(port->refclk);
+	return ret;
+}
+
+static int rzg3s_pcie_host_start(struct rzg3s_pcie_host *host)
+{
+	const struct rzg3s_pcie_soc_data *data = host->data;
+	struct rzg3s_sysc *sysc = host->sysc;
+	int ret;
+
+	ret = rzg3s_sysc_config_func(sysc, RZG3S_SYSC_FUNC_ID_MODE, 1);
+	if (ret)
+		return ret;
+
+	ret = rzg3s_sysc_config_func(sysc, RZG3S_SYSC_FUNC_ID_RST_RSM_B, 1);
+	if (ret)
+		return ret;
+
+	if (host->num_lanes) {
+		ret = rzg3s_sysc_config_func(host->sysc,
+					     RZG3S_SYSC_FUNC_ID_LINK_MASTER,
+					     host->num_lanes == 2  ?
+					     RZG3S_SYSC_LINK_MODE_DUAL_X2 :
+					     RZG3S_SYSC_LINK_MODE_SINGLE_X4);
+		if (ret)
+			goto assert_rst_rsm_b;
+	}
+
+	/*
+	 * Since the power domain's genpd_resume_noirq() will enable clocks,
+	 * there is no need to manually invoke runtime PM API here.
+	 */
+
+	ret = rzg3s_pcie_power_resets_deassert(host);
+	if (ret)
+		goto assert_rst_rsm_b;
+
+	ret = rzg3s_pcie_host_setup(host, rzg3s_pcie_msi_hw_setup,
+				    rzg3s_pcie_msi_hw_teardown);
+	if (ret)
+		goto assert_power_resets;
+
+	return 0;
+
+	/*
+	 * If any error happens there is no way to recover the IP. Put it in the
+	 * lowest possible power state.
+	 */
+assert_power_resets:
+	reset_control_bulk_assert(data->num_power_resets, host->power_resets);
+assert_rst_rsm_b:
+	rzg3s_sysc_config_func(sysc, RZG3S_SYSC_FUNC_ID_RST_RSM_B, 0);
+	return ret;
+}
+
 static int rzg3s_pcie_get_controller_id(struct rzg3s_pcie_host *host)
 {
 	struct device_node *np = host->dev->of_node;
@@ -1939,98 +2033,15 @@ static int rzg3s_pcie_probe(struct platform_device *pdev)
 static int rzg3s_pcie_suspend_noirq(struct device *dev)
 {
 	struct rzg3s_pcie_host *host = dev_get_drvdata(dev);
-	const struct rzg3s_pcie_soc_data *data = host->data;
-	struct rzg3s_pcie_port *port = &host->port;
-	struct rzg3s_sysc *sysc = host->sysc;
-	int ret;
-
-	clk_disable_unprepare(port->refclk);
-
-	/* SoC-specific de-initialization */
-	ret = data->config_deinit(host);
-	if (ret)
-		goto refclk_restore;
-
-	ret = reset_control_bulk_assert(data->num_power_resets,
-					host->power_resets);
-	if (ret)
-		goto config_reinit;
-
-	/*
-	 * Since the power domain's genpd_suspend_noirq() will disable clocks,
-	 * there is no need to manually invoke runtime PM API here.
-	 */
-
-	ret = rzg3s_sysc_config_func(sysc, RZG3S_SYSC_FUNC_ID_RST_RSM_B, 0);
-	if (ret)
-		goto power_resets_restore;
 
-	return 0;
-
-	/* Restore the previous state if any error happens */
-power_resets_restore:
-	reset_control_bulk_deassert(data->num_power_resets,
-				    host->power_resets);
-config_reinit:
-	if (data->config_pre_init)
-		data->config_pre_init(host);
-	data->config_post_init(host);
-refclk_restore:
-	clk_prepare_enable(port->refclk);
-	return ret;
+	return rzg3s_pcie_host_stop(host);
 }
 
 static int rzg3s_pcie_resume_noirq(struct device *dev)
 {
 	struct rzg3s_pcie_host *host = dev_get_drvdata(dev);
-	const struct rzg3s_pcie_soc_data *data = host->data;
-	struct rzg3s_sysc *sysc = host->sysc;
-	int ret;
 
-	ret = rzg3s_sysc_config_func(sysc, RZG3S_SYSC_FUNC_ID_MODE, 1);
-	if (ret)
-		return ret;
-
-	ret = rzg3s_sysc_config_func(sysc, RZG3S_SYSC_FUNC_ID_RST_RSM_B, 1);
-	if (ret)
-		return ret;
-
-	if (host->num_lanes) {
-		ret = rzg3s_sysc_config_func(host->sysc,
-					     RZG3S_SYSC_FUNC_ID_LINK_MASTER,
-					     host->num_lanes == 2  ?
-					     RZG3S_SYSC_LINK_MODE_DUAL_X2 :
-					     RZG3S_SYSC_LINK_MODE_SINGLE_X4);
-		if (ret)
-			goto assert_rst_rsm_b;
-	}
-
-	/*
-	 * Since the power domain's genpd_resume_noirq() will enable clocks,
-	 * there is no need to manually invoke runtime PM API here.
-	 */
-
-	ret = rzg3s_pcie_power_resets_deassert(host);
-	if (ret)
-		goto assert_rst_rsm_b;
-
-	ret = rzg3s_pcie_host_setup(host, rzg3s_pcie_msi_hw_setup,
-				    rzg3s_pcie_msi_hw_teardown);
-	if (ret)
-		goto assert_power_resets;
-
-	return 0;
-
-	/*
-	 * If any error happens there is no way to recover the IP. Put it in the
-	 * lowest possible power state.
-	 */
-assert_power_resets:
-	reset_control_bulk_assert(data->num_power_resets,
-				  host->power_resets);
-assert_rst_rsm_b:
-	rzg3s_sysc_config_func(sysc, RZG3S_SYSC_FUNC_ID_RST_RSM_B, 0);
-	return ret;
+	return rzg3s_pcie_host_start(host);
 }
 
 static const struct dev_pm_ops rzg3s_pcie_pm_ops = {
-- 
2.43.0

[PATCH v4 6/8] PCI: rzg3s-host: Move IRQ domain setup code

[Claudiu Beznea]

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Subsequent patches add support for the event IRQ to handle link
up/down events. The event IRQ handler will use
rzg3s_pcie_set_max_link_speed(). In preparation for adding event IRQ
support, move the IRQ domain initialization code after
rzg3s_pcie_set_max_link_speed().

Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v4:
- none

Changes in v3:
- none, this patch is new

 drivers/pci/controller/pcie-rzg3s-host.c | 147 +++++++++++------------
 1 file changed, 73 insertions(+), 74 deletions(-)

diff --git a/drivers/pci/controller/pcie-rzg3s-host.c b/drivers/pci/controller/pcie-rzg3s-host.c
index 40d5ef3e347e..f4b27a77a89d 100644
--- a/drivers/pci/controller/pcie-rzg3s-host.c
+++ b/drivers/pci/controller/pcie-rzg3s-host.c
@@ -1006,80 +1006,6 @@ static const struct irq_domain_ops rzg3s_pcie_intx_domain_ops = {
 	.xlate = irq_domain_xlate_onetwocell,
 };
 
-static void rzg3s_pcie_teardown_intx(struct rzg3s_pcie_host *host,
-				     int count)
-{
-	while (--count >= 0) {
-		irq_set_chained_handler_and_data(host->intx_irqs[count], NULL,
-						 NULL);
-	}
-
-	if (host->intx_domain)
-		irq_domain_remove(host->intx_domain);
-}
-
-static int rzg3s_pcie_init_irqdomain(struct rzg3s_pcie_host *host)
-{
-	struct device *dev = host->dev;
-	struct platform_device *pdev = to_platform_device(dev);
-	int i, ret;
-
-	for (i = 0; i < PCI_NUM_INTX; i++) {
-		char irq_name[5] = {0};
-		int irq;
-
-		scnprintf(irq_name, ARRAY_SIZE(irq_name), "int%c", 'a' + i);
-
-		irq = platform_get_irq_byname(pdev, irq_name);
-		if (irq < 0) {
-			ret = irq;
-			dev_err_probe(dev, ret,
-				      "Failed to parse and map INT%c IRQ\n",
-				      'A' + i);
-			goto teardown_intx;
-		}
-
-		host->intx_irqs[i] = irq;
-		irq_set_chained_handler_and_data(irq,
-						 rzg3s_pcie_intx_irq_handler,
-						 host);
-	}
-
-	host->intx_domain = irq_domain_create_linear(dev_fwnode(dev),
-						     PCI_NUM_INTX,
-						     &rzg3s_pcie_intx_domain_ops,
-						     host);
-	if (!host->intx_domain) {
-		ret = -EINVAL;
-		dev_err_probe(dev, ret,
-			      "Failed to add irq domain for INTx IRQs\n");
-		goto teardown_intx;
-	}
-	irq_domain_update_bus_token(host->intx_domain, DOMAIN_BUS_WIRED);
-
-	if (IS_ENABLED(CONFIG_PCI_MSI)) {
-		ret = rzg3s_pcie_init_msi(host);
-
-		if (ret)
-			goto teardown_intx;
-	}
-
-	return 0;
-
-teardown_intx:
-	rzg3s_pcie_teardown_intx(host, i);
-
-	return ret;
-}
-
-static void rzg3s_pcie_teardown_irqdomain(struct rzg3s_pcie_host *host)
-{
-	if (IS_ENABLED(CONFIG_PCI_MSI))
-		rzg3s_pcie_teardown_msi(host);
-
-	rzg3s_pcie_teardown_intx(host, PCI_NUM_INTX);
-}
-
 static int rzg3s_pcie_set_max_link_speed(struct rzg3s_pcie_host *host)
 {
 	u32 remote_supported_link_speeds, max_supported_link_speeds;
@@ -1169,6 +1095,79 @@ static int rzg3s_pcie_set_max_link_speed(struct rzg3s_pcie_host *host)
 	return ret;
 }
 
+static void rzg3s_pcie_teardown_intx(struct rzg3s_pcie_host *host, int count)
+{
+	if (host->intx_domain)
+		irq_domain_remove(host->intx_domain);
+
+	while (--count >= 0) {
+		irq_set_chained_handler_and_data(host->intx_irqs[count], NULL,
+						 NULL);
+	}
+}
+
+static int rzg3s_pcie_init_irqdomain(struct rzg3s_pcie_host *host)
+{
+	struct device *dev = host->dev;
+	struct platform_device *pdev = to_platform_device(dev);
+	int i, ret;
+
+	for (i = 0; i < PCI_NUM_INTX; i++) {
+		char irq_name[5] = {0};
+		int irq;
+
+		scnprintf(irq_name, ARRAY_SIZE(irq_name), "int%c", 'a' + i);
+
+		irq = platform_get_irq_byname(pdev, irq_name);
+		if (irq < 0) {
+			ret = irq;
+			dev_err_probe(dev, ret,
+				      "Failed to parse and map INT%c IRQ\n",
+				      'A' + i);
+			goto teardown_intx;
+		}
+
+		host->intx_irqs[i] = irq;
+		irq_set_chained_handler_and_data(irq,
+						 rzg3s_pcie_intx_irq_handler,
+						 host);
+	}
+
+	host->intx_domain = irq_domain_create_linear(dev_fwnode(dev),
+						     PCI_NUM_INTX,
+						     &rzg3s_pcie_intx_domain_ops,
+						     host);
+	if (!host->intx_domain) {
+		ret = -EINVAL;
+		dev_err_probe(dev, ret,
+			      "Failed to add irq domain for INTx IRQs\n");
+		goto teardown_intx;
+	}
+	irq_domain_update_bus_token(host->intx_domain, DOMAIN_BUS_WIRED);
+
+	if (IS_ENABLED(CONFIG_PCI_MSI)) {
+		ret = rzg3s_pcie_init_msi(host);
+
+		if (ret)
+			goto teardown_intx;
+	}
+
+	return 0;
+
+teardown_intx:
+	rzg3s_pcie_teardown_intx(host, i);
+
+	return ret;
+}
+
+static void rzg3s_pcie_teardown_irqdomain(struct rzg3s_pcie_host *host)
+{
+	if (IS_ENABLED(CONFIG_PCI_MSI))
+		rzg3s_pcie_teardown_msi(host);
+
+	rzg3s_pcie_teardown_intx(host, PCI_NUM_INTX);
+}
+
 static int rzg3s_pcie_config_init(struct rzg3s_pcie_host *host)
 {
 	struct pci_host_bridge *bridge = pci_host_bridge_from_priv(host);
-- 
2.43.0

[PATCH v4 7/8] PCI: rzg3s-host: Re-enumerate the bus on PCIe link-state changes

[Claudiu Beznea]

From: John Madieu <john.madieu.xa@bp.renesas.com>

The RZ/G3{E, S}, RZ/V2{H(P), N} PCIe controllers does not expose the
standard PCIe Slot Capability registers, so the generic pciehp driver
cannot be used. The only link-state signal the hardware provides is the
DL_UpDown bit in the PEIS0 event status register, which is raised on every
Data Link layer up/down transition.

Enable DL_UpDown in PEIE0 and hook up an interrupt handler so the driver
can react to link-state changes: a device that trains after boot gets
enumerated, and a device that disappears on link loss is removed. This
provides hotplug-like behavior without the PCI hotplug core, which is
unavailable for the reason above.

On a DL_UpDown event the handler acks the W1C status bit and schedules a
worker that inspects PCSTAT1.DL_DOWN_STS:

  - link up: re-run max link speed negotiation, wait for the link to
    settle and pci_rescan_bus() the root bus;
  - link down: walk the bus in reverse and pci_stop_and_remove_bus_device()
    each child.

Both paths take pci_lock_rescan_remove() to serialize against the PCI
core.

Link events are processed only after the controller has been fully
initialized.

While at it, make probe tolerant of an absent device. Previously, if the
link failed to come up during rzg3s_pcie_host_init(), probe tore the
controller back down and failed. Distinguish this case with -ENODEV,
leave the controller and refclk running, and let the link-up path
enumerate the device once it appears.

Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
Co-developed-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v4:
- dropped .reset_root_port() changes

Changes in v3:
- added RZG3S_PCI_PEIE0_DL_UPDOWN
- re-worked the support by implemeting
  struct pci_host_bridge::reset_root_port()
- introduced the struct rzg3s_pcie_host::state to:
-- avoid touching the controller while a reset root port is in progress
-- and avoid touching the controller in case a reset root port failed
-- and to be able to re-use the already existing code in the reset
   root port function
-- and added CLASS() constructs helpers for it to keep the state handling
   code simpler
- updated the patch description to reflect the updates

 drivers/pci/controller/pcie-rzg3s-host.c | 155 ++++++++++++++++++++---
 1 file changed, 139 insertions(+), 16 deletions(-)

diff --git a/drivers/pci/controller/pcie-rzg3s-host.c b/drivers/pci/controller/pcie-rzg3s-host.c
index f4b27a77a89d..1c0353598b59 100644
--- a/drivers/pci/controller/pcie-rzg3s-host.c
+++ b/drivers/pci/controller/pcie-rzg3s-host.c
@@ -86,6 +86,7 @@
 #define RZG3S_PCI_MSGRCVIS_MRI			BIT(24)
 
 #define RZG3S_PCI_PEIE0				0x200
+#define RZG3S_PCI_PEIE0_DL_UPDOWN		BIT(9)
 
 #define RZG3S_PCI_PEIS0				0x204
 #define RZG3S_PCI_PEIS0_RX_DLLP_PM_ENTER	BIT(12)
@@ -323,6 +324,7 @@ struct rzg3s_pcie_port {
  * @msi: MSI data structure
  * @port: PCIe Root Port
  * @hw_lock: lock for access to the HW resources
+ * @event_irq: PCIe event interrupt for DL_UpDown detection
  * @intx_irqs: INTx interrupts
  * @max_link_speed: maximum supported link speed
  * @controller_id: PCIe controller identifier, used for System Controller access
@@ -340,6 +342,7 @@ struct rzg3s_pcie_host {
 	struct rzg3s_pcie_msi msi;
 	struct rzg3s_pcie_port port;
 	raw_spinlock_t hw_lock;
+	int event_irq;
 	int intx_irqs[PCI_NUM_INTX];
 	int max_link_speed;
 	enum rzg3s_pcie_controller_id controller_id;
@@ -1095,6 +1098,89 @@ static int rzg3s_pcie_set_max_link_speed(struct rzg3s_pcie_host *host)
 	return ret;
 }
 
+static void rzg3s_pcie_link_event(struct rzg3s_pcie_host *host)
+{
+	struct pci_host_bridge *bridge = pci_host_bridge_from_priv(host);
+	struct pci_bus *bus = bridge->bus;
+	u32 val;
+
+	val = readl_relaxed(host->axi + RZG3S_PCI_PCSTAT1);
+	if (val & RZG3S_PCI_PCSTAT1_DL_DOWN_STS) {
+		struct pci_dev *dev, *tmp;
+
+		dev_info(host->dev, "PCIe link down, removing devices\n");
+
+		pci_lock_rescan_remove();
+		list_for_each_entry_safe_reverse(dev, tmp, &bus->devices,
+						 bus_list)
+			pci_stop_and_remove_bus_device(dev);
+		pci_unlock_rescan_remove();
+	} else {
+		int ret;
+
+		dev_info(host->dev, "PCIe link up, rescanning bus\n");
+
+		/*
+		 * Attempt link speed negotiation now that the link is up.
+		 * Failure is non-fatal: the device works at the negotiated
+		 * speed.
+		 */
+		ret = rzg3s_pcie_set_max_link_speed(host);
+		if (ret)
+			dev_info(host->dev, "Failed to set max link speed\n");
+
+		pci_host_common_link_train_delay(host->max_link_speed);
+
+		pci_lock_rescan_remove();
+		pci_rescan_bus(bus);
+		pci_unlock_rescan_remove();
+	}
+}
+
+static irqreturn_t rzg3s_pcie_event_irq_thread(int irq, void *data)
+{
+	struct rzg3s_pcie_host *host = data;
+	u32 status;
+
+	status = readl_relaxed(host->axi + RZG3S_PCI_PEIS0);
+
+	if (!(status & RZG3S_PCI_PEIS0_DL_UPDOWN))
+		return IRQ_NONE;
+
+	/* Clear the DL_UpDown status (W1C) */
+	writel_relaxed(RZG3S_PCI_PEIS0_DL_UPDOWN, host->axi + RZG3S_PCI_PEIS0);
+
+	rzg3s_pcie_link_event(host);
+
+	return IRQ_HANDLED;
+}
+
+static int rzg3s_pcie_request_event_irq(struct rzg3s_pcie_host *host)
+{
+	struct device *dev = host->dev;
+	struct platform_device *pdev = to_platform_device(dev);
+	const char *evt_name;
+	int ret, irq;
+
+	evt_name = devm_kasprintf(dev, GFP_KERNEL, "%s-evt", dev_name(dev));
+	if (!evt_name)
+		return -ENOMEM;
+
+	irq = platform_get_irq_byname(pdev, "pcie_evt");
+	if (irq < 0)
+		return irq;
+
+	ret = request_threaded_irq(irq, NULL, rzg3s_pcie_event_irq_thread,
+				   IRQF_ONESHOT, evt_name, host);
+	if (ret) {
+		return dev_err_probe(dev, ret,
+				     "Failed to request pcie_evt IRQ\n");
+	}
+	host->event_irq = irq;
+
+	return 0;
+}
+
 static void rzg3s_pcie_teardown_intx(struct rzg3s_pcie_host *host, int count)
 {
 	if (host->intx_domain)
@@ -1106,6 +1192,17 @@ static void rzg3s_pcie_teardown_intx(struct rzg3s_pcie_host *host, int count)
 	}
 }
 
+static void rzg3s_pcie_teardown_irqdomain(struct rzg3s_pcie_host *host)
+{
+	if (host->event_irq > 0)
+		free_irq(host->event_irq, host);
+
+	if (IS_ENABLED(CONFIG_PCI_MSI))
+		rzg3s_pcie_teardown_msi(host);
+
+	rzg3s_pcie_teardown_intx(host, PCI_NUM_INTX);
+}
+
 static int rzg3s_pcie_init_irqdomain(struct rzg3s_pcie_host *host)
 {
 	struct device *dev = host->dev;
@@ -1152,22 +1249,21 @@ static int rzg3s_pcie_init_irqdomain(struct rzg3s_pcie_host *host)
 			goto teardown_intx;
 	}
 
+	ret = rzg3s_pcie_request_event_irq(host);
+	if (ret)
+		goto teardown_msi;
+
 	return 0;
 
+teardown_msi:
+	if (IS_ENABLED(CONFIG_PCI_MSI))
+		rzg3s_pcie_teardown_msi(host);
 teardown_intx:
 	rzg3s_pcie_teardown_intx(host, i);
 
 	return ret;
 }
 
-static void rzg3s_pcie_teardown_irqdomain(struct rzg3s_pcie_host *host)
-{
-	if (IS_ENABLED(CONFIG_PCI_MSI))
-		rzg3s_pcie_teardown_msi(host);
-
-	rzg3s_pcie_teardown_intx(host, PCI_NUM_INTX);
-}
-
 static int rzg3s_pcie_config_init(struct rzg3s_pcie_host *host)
 {
 	struct pci_host_bridge *bridge = pci_host_bridge_from_priv(host);
@@ -1679,16 +1775,21 @@ static int rzg3s_pcie_host_init(struct rzg3s_pcie_host *host)
 				 PCIE_LINK_WAIT_SLEEP_MS * MILLI,
 				 PCIE_LINK_WAIT_SLEEP_MS * MILLI *
 				 PCIE_LINK_WAIT_MAX_RETRIES);
-	if (ret)
-		goto config_deinit_post;
+	if (ret) {
+		/*
+		 * Link is down. Leave the controller running so the
+		 * DL_UpDown handler can enumerate a device that appears
+		 * later.
+		 */
+		dev_info(host->dev, "PCIe link down, waiting for DL_UpDown\n");
+		ret = -ENODEV;
+	}
 
 	val = readl_relaxed(host->axi + RZG3S_PCI_PCSTAT2);
 	dev_info(host->dev, "PCIe link status [0x%x]\n", val);
 
-	return 0;
+	return ret;
 
-config_deinit_post:
-	host->data->config_deinit(host);
 config_deinit_and_refclk:
 	clk_disable_unprepare(host->port.refclk);
 config_deinit:
@@ -1723,8 +1824,14 @@ rzg3s_pcie_host_setup(struct rzg3s_pcie_host *host,
 
 	ret = rzg3s_pcie_host_init(host);
 	if (ret) {
-		dev_err_probe(dev, ret, "Failed to initialize the HW!\n");
-		goto teardown_irqdomain;
+		if (ret != -ENODEV) {
+			dev_err_probe(dev, ret,
+				      "Failed to initialize the HW!\n");
+			goto teardown_irqdomain;
+		}
+
+		/* Link is down: hotplug via DL_UpDown will recover. */
+		return 0;
 	}
 
 	ret = rzg3s_pcie_set_max_link_speed(host);
@@ -2004,6 +2111,14 @@ static int rzg3s_pcie_probe(struct platform_device *pdev)
 	if (ret)
 		goto host_probe_teardown;
 
+	/*
+	 * Unmask the PCIe event IRQ at the end of probe to avoid
+	 * spurious link-state events during controller setup and bus
+	 * enumeration. From here on, DL_UpDown events trigger the link
+	 * IRQ thread to (re)scan the bus.
+	 */
+	writel_relaxed(RZG3S_PCI_PEIE0_DL_UPDOWN, host->axi + RZG3S_PCI_PEIE0);
+
 	return 0;
 
 host_probe_teardown:
@@ -2039,8 +2154,16 @@ static int rzg3s_pcie_suspend_noirq(struct device *dev)
 static int rzg3s_pcie_resume_noirq(struct device *dev)
 {
 	struct rzg3s_pcie_host *host = dev_get_drvdata(dev);
+	int ret;
 
-	return rzg3s_pcie_host_start(host);
+	ret = rzg3s_pcie_host_start(host);
+	if (ret)
+		return ret;
+
+	/* Unmask link up/down IRQ. */
+	writel_relaxed(RZG3S_PCI_PEIE0_DL_UPDOWN, host->axi + RZG3S_PCI_PEIE0);
+
+	return 0;
 }
 
 static const struct dev_pm_ops rzg3s_pcie_pm_ops = {
-- 
2.43.0

[PATCH v4 8/8] PCI: rzg3s-host: Add bridge::reset_root_port()

[Claudiu Beznea]

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Add bridge::reset_root_port(). This allows, on some devices, performing
reads and writes to an NVMe endpoint after a link down/link up cycle
without failures.

The implementation of struct pci_host_bridge::reset_root_port() masks all
enabled interrupts and synchronizes them before resetting the controller
to prevent asynchronous events from interfering with the reset operation.

struct rzg3s_pcie_host::started boolean was added to keep track of the
controller state, since rzg3s_pcie_host_start() and rzg3s_pcie_host_stop()
are called from multiple paths: .reset_root_port(), .suspend_noirq() and
.resume_noirq(). The flag prevents double start/stop and breaking
reference counters for clocks and resets.

Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v4:
- this patch is new and is a split from patch 7 of v3
- use a boolean to keep track of the controller start state; with this
  the state management code from patch 7 of v3 was dropped 

 drivers/pci/controller/pcie-rzg3s-host.c | 77 +++++++++++++++++++++++-
 1 file changed, 75 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/pcie-rzg3s-host.c b/drivers/pci/controller/pcie-rzg3s-host.c
index 1c0353598b59..9cba898a9536 100644
--- a/drivers/pci/controller/pcie-rzg3s-host.c
+++ b/drivers/pci/controller/pcie-rzg3s-host.c
@@ -328,6 +328,7 @@ struct rzg3s_pcie_port {
  * @intx_irqs: INTx interrupts
  * @max_link_speed: maximum supported link speed
  * @controller_id: PCIe controller identifier, used for System Controller access
+ * @started: The PCIe controller state (started or not)
  * @num_lanes: The number of lanes
  */
 struct rzg3s_pcie_host {
@@ -346,6 +347,7 @@ struct rzg3s_pcie_host {
 	int intx_irqs[PCI_NUM_INTX];
 	int max_link_speed;
 	enum rzg3s_pcie_controller_id controller_id;
+	bool started;
 	u8 num_lanes;
 };
 
@@ -1110,6 +1112,11 @@ static void rzg3s_pcie_link_event(struct rzg3s_pcie_host *host)
 
 		dev_info(host->dev, "PCIe link down, removing devices\n");
 
+		for_each_pci_bridge(dev, bridge->bus) {
+			if (pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT)
+				pci_host_handle_link_down(dev);
+		}
+
 		pci_lock_rescan_remove();
 		list_for_each_entry_safe_reverse(dev, tmp, &bus->devices,
 						 bus_list)
@@ -1855,6 +1862,9 @@ static int rzg3s_pcie_host_stop(struct rzg3s_pcie_host *host)
 	struct rzg3s_sysc *sysc = host->sysc;
 	int ret;
 
+	if (!host->started)
+		return 0;
+
 	clk_disable_unprepare(port->refclk);
 
 	/* SoC-specific de-initialization */
@@ -1876,6 +1886,8 @@ static int rzg3s_pcie_host_stop(struct rzg3s_pcie_host *host)
 	if (ret)
 		goto power_resets_restore;
 
+	host->started = false;
+
 	return 0;
 
 	/* Restore the previous state if any error happens */
@@ -1891,12 +1903,15 @@ static int rzg3s_pcie_host_stop(struct rzg3s_pcie_host *host)
 	return ret;
 }
 
-static int rzg3s_pcie_host_start(struct rzg3s_pcie_host *host)
+static int rzg3s_pcie_host_start(struct rzg3s_pcie_host *host, bool set_started)
 {
 	const struct rzg3s_pcie_soc_data *data = host->data;
 	struct rzg3s_sysc *sysc = host->sysc;
 	int ret;
 
+	if (host->started)
+		return 0;
+
 	ret = rzg3s_sysc_config_func(sysc, RZG3S_SYSC_FUNC_ID_MODE, 1);
 	if (ret)
 		return ret;
@@ -1929,6 +1944,9 @@ static int rzg3s_pcie_host_start(struct rzg3s_pcie_host *host)
 	if (ret)
 		goto assert_power_resets;
 
+	if (set_started)
+		host->started = true;
+
 	return 0;
 
 	/*
@@ -1942,6 +1960,58 @@ static int rzg3s_pcie_host_start(struct rzg3s_pcie_host *host)
 	return ret;
 }
 
+static int rzg3s_pcie_host_reset_root_port(struct pci_host_bridge *bridge,
+					   struct pci_dev *pdev)
+{
+	struct rzg3s_pcie_host *host = pci_host_bridge_priv(bridge);
+	u32 irqs;
+	int ret;
+
+	/* Mask link up/down interrupts. */
+	writel(0, host->axi + RZG3S_PCI_PEIE0);
+
+	/* Mask INTx and MSI interrupts. */
+	irqs = readl_relaxed(host->axi + RZG3S_PCI_PINTRCVIE);
+	writel(0, host->axi + RZG3S_PCI_PINTRCVIE);
+
+	/*
+	 * Make sure the next operations are not disturbed by any pending
+	 * IRQs.
+	 */
+	synchronize_irq(host->msi.irq);
+	for (unsigned int i = 0; i < PCI_NUM_INTX; i++)
+		synchronize_irq(host->intx_irqs[i]);
+
+	ret = rzg3s_pcie_host_stop(host);
+	if (ret) {
+		dev_err(host->dev, "Failed to stop the host!\n");
+		goto unmask_irqs;
+	}
+
+	ret = rzg3s_pcie_host_start(host, false);
+	if (ret) {
+		dev_err(host->dev, "Failed to start the host!\n");
+
+		/*
+		 * Don't unmask IRQs. We are in a bad state here and we
+		 * can recover only through a suspend/resume cycle. Just
+		 * return and preserve the stop state.
+		 */
+		return ret;
+	}
+
+unmask_irqs:
+	/* Unmask INTx and MSI interrupts. */
+	writel_relaxed(irqs, host->axi + RZG3S_PCI_PINTRCVIE);
+
+	/* Unmask link up/down interrupts. */
+	writel(RZG3S_PCI_PEIE0_DL_UPDOWN, host->axi + RZG3S_PCI_PEIE0);
+
+	host->started = true;
+
+	return ret;
+}
+
 static int rzg3s_pcie_get_controller_id(struct rzg3s_pcie_host *host)
 {
 	struct device_node *np = host->dev->of_node;
@@ -2104,9 +2174,12 @@ static int rzg3s_pcie_probe(struct platform_device *pdev)
 	if (ret)
 		goto power_resets_assert;
 
+	host->started = true;
+
 	bridge->sysdata = host;
 	bridge->ops = &rzg3s_pcie_root_ops;
 	bridge->child_ops = &rzg3s_pcie_child_ops;
+	bridge->reset_root_port = rzg3s_pcie_host_reset_root_port;
 	ret = pci_host_probe(bridge);
 	if (ret)
 		goto host_probe_teardown;
@@ -2156,7 +2229,7 @@ static int rzg3s_pcie_resume_noirq(struct device *dev)
 	struct rzg3s_pcie_host *host = dev_get_drvdata(dev);
 	int ret;
 
-	ret = rzg3s_pcie_host_start(host);
+	ret = rzg3s_pcie_host_start(host, true);
 	if (ret)
 		return ret;
 
-- 
2.43.0

Re: [PATCH v4 5/8] PCI: rzg3s-host: Move suspend/resume code into dedicated functions

[sashiko-bot]

> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> 
> In preparation for implementing hotplug using
> pci_host_bridge::reset_root_port(), move the suspend/resume code into
> rzg3s_pcie_host_stop() and rzg3s_pcie_host_start(). These functions
> will later be reused by the hotplug implementation through
> pci_host_bridge::reset_root_port().
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260922144629.586997-1-claudiu.beznea@kernel.org?part=5

Re: [PATCH v4 2/8] PCI: rzg3s-host: Fix runtime PM handling in the NOIRQ suspend/resume phase

[sashiko-bot]

> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> 
> The runtime PM documentation states the following:
> - During system suspend, pm_runtime_get_noresume() is called for every
>   device right before executing the subsystem-level .prepare() callback
>   (in device_prepare()). In addition, the PM core disables runtime PM for
>   every device right before executing the subsystem-level .suspend_late()
>   callback (in device_suspend_late()).
> - During system resume, pm_runtime_enable() is called for every device
>   right after executing the subsystem-level .resume_early() callback (in
>   device_resume_early()), and pm_runtime_put() is called right after
>   executing the subsystem-level .complete() callback (in
>   device_complete()).
> 
> The driver's .suspend_noirq() callback is invoked after .suspend_late(),
> [ ... ]
> domain callbacks to power the device off and on.
> 
> Fixes: 7ef502fb35b2 ("PCI: Add Renesas RZ/G3S host controller driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260922144629.586997-1-claudiu.beznea@kernel.org?part=2

Re: [PATCH v4 3/8] PCI: rzg3s-host: Drop nop instructions

[sashiko-bot]

> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> 
> On the RZ/G3S PCIe IP variant (which is similar to those used on the
> RZ/G3E, RZ/V2H, and RZ/V2N SoCs), access to the PCIe Type 1 registers
> requires setting the PCI_PERM.CFG_HWINIT_EN bit. Since this bit is not
> set on the code paths where rzg3s_pcie_set_max_link_speed() is called,
> the writes to PCI_EXP_LNKCTL2.TLS are nops. Drop them.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260922144629.586997-1-claudiu.beznea@kernel.org?part=3

Re: [PATCH v4 4/8] PCI: rzg3s-host: Move host configuration code together

[sashiko-bot]

> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> 
> Move host configuration code together to have it grouped. This prepares for
> the addition of hotplug support.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260922144629.586997-1-claudiu.beznea@kernel.org?part=4

Re: [PATCH v4 1/8] PCI: rzg3s-host: Follow hardware manual clock/reset initialization order

[sashiko-bot]

> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> 
> The RZ/G3S PCIe hardware manual specifies that the clocks must be enabled
> before the reset signals are deasserted during initialization.
> 
> Follow this sequence in the probe(), suspend(), and resume() paths to
> match the hardware requirements and avoid potential issues.
> 
> Fixes: 7ef502fb35b2 ("PCI: Add Renesas RZ/G3S host controller driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260922144629.586997-1-claudiu.beznea@kernel.org?part=1

Re: [PATCH v4 6/8] PCI: rzg3s-host: Move IRQ domain setup code

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] pci: UAF in rzg3s_pcie_teardown_intx() due to inverted teardown order
- [Medium] pci: silent removal of PCI_EXP_LNKCTL2_TLS configuration
--

commit 3b9e569b1b1720a0862f47115eba6a10d96f3784
Author: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

PCI: rzg3s-host: Move IRQ domain setup code

Subsequent patches add support for the event IRQ to handle link
up/down events. The event IRQ handler will use
rzg3s_pcie_set_max_link_speed(). In preparation for adding event IRQ
support, move the IRQ domain initialization code after
rzg3s_pcie_set_max_link_speed().

> PCI: rzg3s-host: Move IRQ domain setup code

[Severity: Medium]
The commit message indicates this is a refactoring commit to move IRQ domain
setup code. Is the hardware target link speed configuration
(PCI_EXP_LNKCTL2_TLS) silently omitted from rzg3s_pcie_set_max_link_speed()
in this patch?

Omitting the hardware target link speed configuration (PCI_EXP_LNKCTL2)
could lead to link training issues or incorrect PCIe state during local
initialization or hotplug paths.

> diff --git a/drivers/pci/controller/pcie-rzg3s-host.c b/drivers/pci/controller/pcie-rzg3s-host.c
> index 40d5ef3e347e4..f4b27a77a89df 100644
> --- a/drivers/pci/controller/pcie-rzg3s-host.c
> +++ b/drivers/pci/controller/pcie-rzg3s-host.c
[ ... ]
> @@ -1169,6 +1095,79 @@ static int rzg3s_pcie_set_max_link_speed(struct rzg3s_pcie_host *host)
>  	return ret;
>  }
>  
> +static void rzg3s_pcie_teardown_intx(struct rzg3s_pcie_host *host, int count)
> +{
> +	if (host->intx_domain)
> +		irq_domain_remove(host->intx_domain);

[Severity: High]
Does this inverted teardown order introduce a use-after-free risk?

By calling irq_domain_remove() before unchaining the interrupt handlers,
host->intx_domain is freed early. If an INTx interrupt fires in this window,
rzg3s_pcie_intx_irq_handler() will be executed and dereference the freed
host->intx_domain.

Can we safely destroy the IRQ domain after unchaining the interrupt handlers?

> +
> +	while (--count >= 0) {
> +		irq_set_chained_handler_and_data(host->intx_irqs[count], NULL,
> +						 NULL);
> +	}
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260922144629.586997-1-claudiu.beznea@kernel.org?part=6

Re: [PATCH v4 8/8] PCI: rzg3s-host: Add bridge::reset_root_port()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:
- [High] pci: lockless RMW of RZG3S_PCI_PINTRCVIE clobbers IRQ masks
- [High] pci: lockless traversal of bus->devices causes list corruption
- [High] pci: missing synchronization of event_irq triggers abort
- [High] pci: Root Port removal breaks AER recovery logic
- [High] pci: latched GIC interrupts crash controller during reset
- [High] pci: data race on host->started imbalances clocks
- [Medium] pci: synchronize_irq(0) stall when CONFIG_PCI_MSI is disabled
--

commit 58da40640a7be86e3db083244a44a0cf4c9eb5f2
Author: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

    PCI: rzg3s-host: Add bridge::reset_root_port()
    
    Add bridge::reset_root_port(). This allows, on some devices, performing
    reads and writes to an NVMe endpoint after a link down/link up cycle
    without failures.

> diff --git a/drivers/pci/controller/pcie-rzg3s-host.c b/drivers/pci/controller/pcie-rzg3s-host.c
> index 1c0353598b599..9cba898a9536d 100644
> --- a/drivers/pci/controller/pcie-rzg3s-host.c
> +++ b/drivers/pci/controller/pcie-rzg3s-host.c
[ ... ]
> @@ -1110,6 +1112,11 @@ static void rzg3s_pcie_link_event(struct rzg3s_pcie_host *host)
>  
>  		dev_info(host->dev, "PCIe link down, removing devices\n");
>  
> +		for_each_pci_bridge(dev, bridge->bus) {
> +			if (pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT)
> +				pci_host_handle_link_down(dev);
> +		}
> +
>  		pci_lock_rescan_remove();

[Severity: High]
Does traversing bridge->bus->devices here without holding pci_bus_sem or
pci_lock_rescan_remove() risk a use-after-free?

If concurrent hotplug, AER recovery, or sysfs unbind operations modify the
list, could the traversal follow corrupted pointers?

>  		list_for_each_entry_safe_reverse(dev, tmp, &bus->devices,
>  						 bus_list)
>  			pci_stop_and_remove_bus_device(dev);

[Severity: High]
Does this code remove the Root Port device right after initiating its recovery?

When a link down event occurs, the loop above iterates over devices on the root
bus (which are the Root Ports themselves) and invokes
pci_host_handle_link_down(dev) to schedule DPC/AER recovery.

Immediately afterward, this list_for_each_entry_safe_reverse loop executes
pci_stop_and_remove_bus_device(dev) on the exact same Root Port device. Could
this destroy the Root Port and unbind its drivers just as recovery is taking
place, breaking the recovery logic and future enumeration?

[ ... ]
> @@ -1855,6 +1862,9 @@ static int rzg3s_pcie_host_stop(struct rzg3s_pcie_host *host)
>  	struct rzg3s_sysc *sysc = host->sysc;
>  	int ret;
>  
> +	if (!host->started)
> +		return 0;
> +
>  	clk_disable_unprepare(port->refclk);

[Severity: High]
Is host->started adequately protected against concurrent access?

Because host->started is checked and modified in rzg3s_pcie_host_stop() and
rzg3s_pcie_host_reset_root_port() without any locking, concurrent invocations
of sysfs reset_root_port or PM transitions could both see host->started == true.

Could both threads subsequently disable the reference clocks and assert resets,
leading to imbalanced clock use counts and broken hardware state?

[ ... ]
> +static int rzg3s_pcie_host_reset_root_port(struct pci_host_bridge *bridge,
> +					   struct pci_dev *pdev)
> +{
> +	struct rzg3s_pcie_host *host = pci_host_bridge_priv(bridge);
> +	u32 irqs;
> +	int ret;
> +
> +	/* Mask link up/down interrupts. */
> +	writel(0, host->axi + RZG3S_PCI_PEIE0);
> +
> +	/* Mask INTx and MSI interrupts. */
> +	irqs = readl_relaxed(host->axi + RZG3S_PCI_PINTRCVIE);
> +	writel(0, host->axi + RZG3S_PCI_PINTRCVIE);

[Severity: High]
Should this mask register be read and written while holding host->hw_lock?

Concurrent calls to rzg3s_pcie_intx_irq_mask() running on another CPU will
modify this register while holding hw_lock. Could reading and clearing it here
locklessly, and restoring it later, clobber dynamic mask/unmask operations
that occurred during the reset window?

> +
> +	/*
> +	 * Make sure the next operations are not disturbed by any pending
> +	 * IRQs.
> +	 */
> +	synchronize_irq(host->msi.irq);

[Severity: Medium]
Will this stall if CONFIG_PCI_MSI is disabled?

host->msi.irq is populated only if CONFIG_PCI_MSI is enabled; otherwise, it
remains 0. Synchronizing against IRQ 0 targets a core architectural interrupt
or an invalid descriptor, which causes invalid synchronization behavior and
potential stalls.

> +	for (unsigned int i = 0; i < PCI_NUM_INTX; i++)
> +		synchronize_irq(host->intx_irqs[i]);
> +
> +	ret = rzg3s_pcie_host_stop(host);

[Severity: High]
Are there missing synchronization checks for the event_irq here?

Because host->event_irq is not synchronized, if the link state changes, the
rzg3s_pcie_event_irq_thread() can execute concurrently and attempt to read PCIe
configuration or AXI registers. Accessing the controller while clocks are
disabled by rzg3s_pcie_host_stop() immediately triggers a synchronous external
abort.

[Severity: High]
Is synchronize_irq() sufficient to prevent latched GIC interrupts from
crashing the controller during reset?

Because MSIs are edge-triggered at the GIC, an interrupt that fires immediately
before the IP-level mask can remain latched in the GIC. synchronize_irq() only
waits for currently executing handlers, so the pending GIC interrupt remains
queued.

When the CPU enables local interrupts, the GIC dispatches the handler, which
attempts to read host->axi + RZG3S_PCI_PINTRCVIS while the controller clocks
are disabled by the reset, triggering a synchronous external abort. Should
disable_irq() be used instead?

[ ... ]
> +unmask_irqs:
> +	/* Unmask INTx and MSI interrupts. */
> +	writel_relaxed(irqs, host->axi + RZG3S_PCI_PINTRCVIE);

[Severity: High]
Could this lockless restore overwrite concurrent updates made by the IRQ
subsystem?

As noted above, restoring the previously read mask state locklessly clobbers
any dynamic mask/unmask operations that occurred during the reset window, which
can cause interrupt storms or lost interrupts.

> +
> +	/* Unmask link up/down interrupts. */
> +	writel(RZG3S_PCI_PEIE0_DL_UPDOWN, host->axi + RZG3S_PCI_PEIE0);
> +
> +	host->started = true;
> +
> +	return ret;
> +}

[Severity: High]
As mentioned earlier, does writing host->started without locking allow
concurrent updates to overwrite and desync the expected controller state?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260922144629.586997-1-claudiu.beznea@kernel.org?part=8

Re: [PATCH v4 7/8] PCI: rzg3s-host: Re-enumerate the bus on PCIe link-state changes

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] pci: rzg3s-host: misses PCIe link state changes during system suspend
- [Medium] pci: rzg3s-host: TOCTOU race in link event handler causes device confusion
--

commit d39725457acf3fbf2e1dca55279894b4bb68e696
Author: John Madieu <john.madieu.xa@bp.renesas.com>
Subject: PCI: rzg3s-host: Re-enumerate the bus on PCIe link-state changes

This commit enables the DL_UpDown interrupt to provide hotplug-like behavior
by re-enumerating the bus on link-state changes. It processes link up and
down events to scan or remove devices accordingly, and updates the probe path
to tolerate an absent device.

> diff --git a/drivers/pci/controller/pcie-rzg3s-host.c b/drivers/pci/controller/pcie-rzg3s-host.c
> index f4b27a77a89df..1c0353598b599 100644
> --- a/drivers/pci/controller/pcie-rzg3s-host.c
> +++ b/drivers/pci/controller/pcie-rzg3s-host.c
[ ... ]
> @@ -1095,6 +1098,89 @@ static int rzg3s_pcie_set_max_link_speed(struct rzg3s_pcie_host *host)
>  	return ret;
>  }
>  
> +static void rzg3s_pcie_link_event(struct rzg3s_pcie_host *host)
> +{
> +	struct pci_host_bridge *bridge = pci_host_bridge_from_priv(host);
> +	struct pci_bus *bus = bridge->bus;
> +	u32 val;
> +
> +	val = readl_relaxed(host->axi + RZG3S_PCI_PCSTAT1);
> +	if (val & RZG3S_PCI_PCSTAT1_DL_DOWN_STS) {

[Severity: Medium]
Is there a TOCTOU race here if the link flaps rapidly?

The handler reads the live link state to decide whether to remove or rescan
devices. If the link goes down and comes back up before this threaded handler
runs, it will see DL_DOWN_STS == 0 and take the else path.

> +		struct pci_dev *dev, *tmp;
> +
> +		dev_info(host->dev, "PCIe link down, removing devices\n");
> +
> +		pci_lock_rescan_remove();
> +		list_for_each_entry_safe_reverse(dev, tmp, &bus->devices,
> +						 bus_list)
> +			pci_stop_and_remove_bus_device(dev);
> +		pci_unlock_rescan_remove();
> +	} else {
> +		int ret;
> +
> +		dev_info(host->dev, "PCIe link up, rescanning bus\n");
> +
> +		/*
> +		 * Attempt link speed negotiation now that the link is up.
> +		 * Failure is non-fatal: the device works at the negotiated
> +		 * speed.
> +		 */
> +		ret = rzg3s_pcie_set_max_link_speed(host);
> +		if (ret)
> +			dev_info(host->dev, "Failed to set max link speed\n");
> +
> +		pci_host_common_link_train_delay(host->max_link_speed);
> +
> +		pci_lock_rescan_remove();
> +		pci_rescan_bus(bus);

Because the DOWN event was missed, old devices are never removed from
bus->devices. Calling pci_rescan_bus() here will skip enumerating them,
leaving the kernel with a stale pci_dev bound to a potentially reset or
swapped device.

Could we tear down existing devices before rescanning, or otherwise account
for missed DOWN events?

> +		pci_unlock_rescan_remove();
> +	}
> +}
[ ... ]
> @@ -1723,8 +1824,14 @@ rzg3s_pcie_host_setup(struct rzg3s_pcie_host *host,
>  
>  	ret = rzg3s_pcie_host_init(host);
>  	if (ret) {
> -		dev_err_probe(dev, ret, "Failed to initialize the HW!\n");
> -		goto teardown_irqdomain;
> +		if (ret != -ENODEV) {
> +			dev_err_probe(dev, ret,
> +				      "Failed to initialize the HW!\n");
> +			goto teardown_irqdomain;
> +		}
> +
> +		/* Link is down: hotplug via DL_UpDown will recover. */
> +		return 0;

[Severity: High]
Does this miss PCIe link state changes that happen during system suspend?

During system suspend, power is lost and the link state is reset. On
resume, rzg3s_pcie_resume_noirq() restarts the host, which eventually
calls rzg3s_pcie_host_setup(). 

If the link fails to come up, -ENODEV is swallowed here, assuming the
DL_UPDOWN interrupt will recover it. However, the DL_UPDOWN bit won't
trigger an edge interrupt on resume because the controller was just
initialized and isn't transitioning. 

If a device was unplugged or plugged in during sleep, the PCI core is
never notified, potentially leading to device timeouts or state
corruption. Should we manually check the link state and synchronize the
PCI core after unmasking the interrupt in rzg3s_pcie_resume_noirq()?

>  	}
>  
>  	ret = rzg3s_pcie_set_max_link_speed(host);

[ ... ]

[Severity: High]
This is a pre-existing issue, but does the read-modify-write on W1C
interrupt registers drop concurrent interrupts?

In rzg3s_pcie_msi_irq() and rzg3s_pcie_intx_irq_ack(), the driver uses
rzg3s_pcie_update_bits() to acknowledge specific interrupts:

	rzg3s_pcie_update_bits(host->axi, RZG3S_PCI_PINTRCVIS,
			       RZG3S_PCI_PINTRCVIS_MSI,
			       RZG3S_PCI_PINTRCVIS_MSI);

Registers like RZG3S_PCI_PINTRCVIS are W1C (Write 1 to Clear). Reading
yields a 1 for every currently pending interrupt, and writing that value
back writes a 1 to all those bits. Any concurrent interrupts that were
pending at the time of the read will be silently cleared and lost,
resulting in device timeouts.

[Severity: Medium]
This is a pre-existing issue, but is the MSI hardware configured
unconditionally even if CONFIG_PCI_MSI is disabled?

During device resume, rzg3s_pcie_host_start() passes
rzg3s_pcie_msi_hw_setup() as the initialization callback:

	ret = rzg3s_pcie_host_setup(host, rzg3s_pcie_msi_hw_setup,
				    rzg3s_pcie_msi_hw_teardown);

If CONFIG_PCI_MSI is disabled, MSI software structures (like
msi->window_base) are never initialized and remain 0. This pushes
uninitialized zero values to hardware registers and routes MSI interrupts
to physical address 0. Any DMA write to address 0 will trigger a
spurious hardware event.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260922144629.586997-1-claudiu.beznea@kernel.org?part=7

Re: [PATCH v4 1/8] PCI: rzg3s-host: Follow hardware manual clock/reset initialization order

[Lad, Prabhakar]

On Tue, Sep 22, 2026 at 3:53 PM Claudiu Beznea
<claudiu.beznea@kernel.org> wrote:
>
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>
> The RZ/G3S PCIe hardware manual specifies that the clocks must be enabled
> before the reset signals are deasserted during initialization.
>
> Follow this sequence in the probe(), suspend(), and resume() paths to
> match the hardware requirements and avoid potential issues.
>
> Fixes: 7ef502fb35b2 ("PCI: Add Renesas RZ/G3S host controller driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---
>
> Changes in v4:
> - none
>
> Changes in v3:
> - none, this patch is new
>
>  drivers/pci/controller/pcie-rzg3s-host.c | 40 +++++++++++++-----------
>  1 file changed, 21 insertions(+), 19 deletions(-)
>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Tested-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Cheers,
Prabhakar

> diff --git a/drivers/pci/controller/pcie-rzg3s-host.c b/drivers/pci/controller/pcie-rzg3s-host.c
> index 077cfb0834b3..e1105b1f2652 100644
> --- a/drivers/pci/controller/pcie-rzg3s-host.c
> +++ b/drivers/pci/controller/pcie-rzg3s-host.c
> @@ -1888,10 +1888,6 @@ static int rzg3s_pcie_probe(struct platform_device *pdev)
>         if (ret)
>                 goto sysc_signal_restore;
>
> -       ret = rzg3s_pcie_power_resets_deassert(host);
> -       if (ret)
> -               goto sysc_signal_restore;
> -
>         pm_runtime_enable(dev);
>
>         /*
> @@ -1902,12 +1898,16 @@ static int rzg3s_pcie_probe(struct platform_device *pdev)
>         if (ret)
>                 goto rpm_disable;
>
> +       ret = rzg3s_pcie_power_resets_deassert(host);
> +       if (ret)
> +               goto rpm_put;
> +
>         raw_spin_lock_init(&host->hw_lock);
>
>         ret = rzg3s_pcie_host_setup(host, rzg3s_pcie_init_irqdomain,
>                                     rzg3s_pcie_teardown_irqdomain);
>         if (ret)
> -               goto rpm_put;
> +               goto power_resets_assert;
>
>         bridge->sysdata = host;
>         bridge->ops = &rzg3s_pcie_root_ops;
> @@ -1922,12 +1922,13 @@ static int rzg3s_pcie_probe(struct platform_device *pdev)
>         clk_disable_unprepare(host->port.refclk);
>         rzg3s_pcie_teardown_irqdomain(host);
>         host->data->config_deinit(host);
> +power_resets_assert:
> +       reset_control_bulk_assert(host->data->num_power_resets,
> +                                 host->power_resets);
>  rpm_put:
>         pm_runtime_put_sync(dev);
>  rpm_disable:
>         pm_runtime_disable(dev);
> -       reset_control_bulk_assert(host->data->num_power_resets,
> -                                 host->power_resets);
>  sysc_signal_restore:
>         /*
>          * SYSC RST_RSM_B signal need to be asserted before turning off the
> @@ -1948,10 +1949,6 @@ static int rzg3s_pcie_suspend_noirq(struct device *dev)
>         struct rzg3s_sysc *sysc = host->sysc;
>         int ret;
>
> -       ret = pm_runtime_put_sync(dev);
> -       if (ret)
> -               return ret;
> -
>         clk_disable_unprepare(port->refclk);
>
>         /* SoC-specific de-initialization */
> @@ -1964,13 +1961,19 @@ static int rzg3s_pcie_suspend_noirq(struct device *dev)
>         if (ret)
>                 goto config_reinit;
>
> -       ret = rzg3s_sysc_config_func(sysc, RZG3S_SYSC_FUNC_ID_RST_RSM_B, 0);
> +       ret = pm_runtime_put_sync(dev);
>         if (ret)
>                 goto power_resets_restore;
>
> +       ret = rzg3s_sysc_config_func(sysc, RZG3S_SYSC_FUNC_ID_RST_RSM_B, 0);
> +       if (ret)
> +               goto rpm_resume;
> +
>         return 0;
>
>         /* Restore the previous state if any error happens */
> +rpm_resume:
> +       pm_runtime_resume_and_get(dev);
>  power_resets_restore:
>         reset_control_bulk_deassert(data->num_power_resets,
>                                     host->power_resets);
> @@ -1980,7 +1983,6 @@ static int rzg3s_pcie_suspend_noirq(struct device *dev)
>         data->config_post_init(host);
>  refclk_restore:
>         clk_prepare_enable(port->refclk);
> -       pm_runtime_resume_and_get(dev);
>         return ret;
>  }
>
> @@ -2009,18 +2011,18 @@ static int rzg3s_pcie_resume_noirq(struct device *dev)
>                         goto assert_rst_rsm_b;
>         }
>
> -       ret = rzg3s_pcie_power_resets_deassert(host);
> +       ret = pm_runtime_resume_and_get(dev);
>         if (ret)
>                 goto assert_rst_rsm_b;
>
> -       ret = pm_runtime_resume_and_get(dev);
> +       ret = rzg3s_pcie_power_resets_deassert(host);
>         if (ret)
> -               goto assert_power_resets;
> +               goto rpm_put;
>
>         ret = rzg3s_pcie_host_setup(host, rzg3s_pcie_msi_hw_setup,
>                                     rzg3s_pcie_msi_hw_teardown);
>         if (ret)
> -               goto rpm_put;
> +               goto assert_power_resets;
>
>         return 0;
>
> @@ -2028,11 +2030,11 @@ static int rzg3s_pcie_resume_noirq(struct device *dev)
>          * If any error happens there is no way to recover the IP. Put it in the
>          * lowest possible power state.
>          */
> -rpm_put:
> -       pm_runtime_put_sync(dev);
>  assert_power_resets:
>         reset_control_bulk_assert(data->num_power_resets,
>                                   host->power_resets);
> +rpm_put:
> +       pm_runtime_put_sync(dev);
>  assert_rst_rsm_b:
>         rzg3s_sysc_config_func(sysc, RZG3S_SYSC_FUNC_ID_RST_RSM_B, 0);
>         return ret;
> --
> 2.43.0
>
>

Re: [PATCH v4 2/8] PCI: rzg3s-host: Fix runtime PM handling in the NOIRQ suspend/resume phase

[Lad, Prabhakar]

On Tue, Sep 22, 2026 at 3:57 PM Claudiu Beznea
<claudiu.beznea@kernel.org> wrote:
>
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>
> The runtime PM documentation states the following:
> - During system suspend, pm_runtime_get_noresume() is called for every
>   device right before executing the subsystem-level .prepare() callback
>   (in device_prepare()). In addition, the PM core disables runtime PM for
>   every device right before executing the subsystem-level .suspend_late()
>   callback (in device_suspend_late()).
> - During system resume, pm_runtime_enable() is called for every device
>   right after executing the subsystem-level .resume_early() callback (in
>   device_resume_early()), and pm_runtime_put() is called right after
>   executing the subsystem-level .complete() callback (in
>   device_complete()).
>
> The driver's .suspend_noirq() callback is invoked after .suspend_late(),
> while .resume_noirq() is invoked before .resume_early().
>
> If:
> - the device is not part of the wake-up path, and
> - its runtime PM status is not RPM_SUSPENDED,
> the generic power domain .suspend_noirq()/.resume_noirq() callbacks
> (genpd_suspend_noirq()/genpd_resume_noirq()) invoke the driver's
> .suspend_noirq()/.resume_noirq() callbacks and call
> genpd_stop_dev()/genpd_start_dev() before and after them, respectively.
>
> Calling genpd_stop_dev()/genpd_start_dev() allows devices whose power is
> controlled by generic power domains to be powered off and on during
> system suspend and resume, even though their runtime PM usage count does
> not reach zero.
>
> Since the runtime PM usage count is incremented in device_prepare() and
> decremented in device_complete(), runtime PM operations performed from
> the driver's .suspend_noirq()/.resume_noirq() callbacks are no-ops. The
> actual power transitions are handled by the generic power domain
> .suspend_noirq()/.resume_noirq() callbacks.
>
> Moreover, attempting to runtime resume a device while runtime PM is
> disabled may return -EACCES. This may cause system resume to fail when
> resuming after a failed Root Port reset, as described in a subsequent
> patch adding hot-plug support.
>
> Remove the runtime PM calls from the driver's
> .suspend_noirq()/.resume_noirq() callbacks and rely on the generic power
> domain callbacks to power the device off and on.
>
> Fixes: 7ef502fb35b2 ("PCI: Add Renesas RZ/G3S host controller driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---
>
> Changes in v4:
> - simplified the comments in rzg3s_pcie_suspend_noirq()/rzg3s_pcie_resume_noirq()
>
> Changes in v3:
> - none, this patch is new
>
>  drivers/pci/controller/pcie-rzg3s-host.c | 22 ++++++++++------------
>  1 file changed, 10 insertions(+), 12 deletions(-)
>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Tested-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Cheers,
Prabhakar

> diff --git a/drivers/pci/controller/pcie-rzg3s-host.c b/drivers/pci/controller/pcie-rzg3s-host.c
> index e1105b1f2652..0ef49bb5ab1a 100644
> --- a/drivers/pci/controller/pcie-rzg3s-host.c
> +++ b/drivers/pci/controller/pcie-rzg3s-host.c
> @@ -1961,19 +1961,18 @@ static int rzg3s_pcie_suspend_noirq(struct device *dev)
>         if (ret)
>                 goto config_reinit;
>
> -       ret = pm_runtime_put_sync(dev);
> -       if (ret)
> -               goto power_resets_restore;
> +       /*
> +        * Since the power domain's genpd_suspend_noirq() will disable clocks,
> +        * there is no need to manually invoke runtime PM API here.
> +        */
>
>         ret = rzg3s_sysc_config_func(sysc, RZG3S_SYSC_FUNC_ID_RST_RSM_B, 0);
>         if (ret)
> -               goto rpm_resume;
> +               goto power_resets_restore;
>
>         return 0;
>
>         /* Restore the previous state if any error happens */
> -rpm_resume:
> -       pm_runtime_resume_and_get(dev);
>  power_resets_restore:
>         reset_control_bulk_deassert(data->num_power_resets,
>                                     host->power_resets);
> @@ -2011,13 +2010,14 @@ static int rzg3s_pcie_resume_noirq(struct device *dev)
>                         goto assert_rst_rsm_b;
>         }
>
> -       ret = pm_runtime_resume_and_get(dev);
> -       if (ret)
> -               goto assert_rst_rsm_b;
> +       /*
> +        * Since the power domain's genpd_resume_noirq() will enable clocks,
> +        * there is no need to manually invoke runtime PM API here.
> +        */
>
>         ret = rzg3s_pcie_power_resets_deassert(host);
>         if (ret)
> -               goto rpm_put;
> +               goto assert_rst_rsm_b;
>
>         ret = rzg3s_pcie_host_setup(host, rzg3s_pcie_msi_hw_setup,
>                                     rzg3s_pcie_msi_hw_teardown);
> @@ -2033,8 +2033,6 @@ static int rzg3s_pcie_resume_noirq(struct device *dev)
>  assert_power_resets:
>         reset_control_bulk_assert(data->num_power_resets,
>                                   host->power_resets);
> -rpm_put:
> -       pm_runtime_put_sync(dev);
>  assert_rst_rsm_b:
>         rzg3s_sysc_config_func(sysc, RZG3S_SYSC_FUNC_ID_RST_RSM_B, 0);
>         return ret;
> --
> 2.43.0
>
>

Re: [PATCH v4 3/8] PCI: rzg3s-host: Drop nop instructions

[Lad, Prabhakar]

On Tue, Sep 22, 2026 at 3:54 PM Claudiu Beznea
<claudiu.beznea@kernel.org> wrote:
>
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>
> On the RZ/G3S PCIe IP variant (which is similar to those used on the
> RZ/G3E, RZ/V2H, and RZ/V2N SoCs), access to the PCIe Type 1 registers
> requires setting the PCI_PERM.CFG_HWINIT_EN bit. Since this bit is not
> set on the code paths where rzg3s_pcie_set_max_link_speed() is called,
> the writes to PCI_EXP_LNKCTL2.TLS are nops. Drop them.
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---
>
> Changes in v4:
> - none
>
> Changes in v3:
> - none, this patch is new
>
>  drivers/pci/controller/pcie-rzg3s-host.c | 5 -----
>  1 file changed, 5 deletions(-)
>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Tested-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Cheers,
Prabhakar

> diff --git a/drivers/pci/controller/pcie-rzg3s-host.c b/drivers/pci/controller/pcie-rzg3s-host.c
> index 0ef49bb5ab1a..362aa923978f 100644
> --- a/drivers/pci/controller/pcie-rzg3s-host.c
> +++ b/drivers/pci/controller/pcie-rzg3s-host.c
> @@ -1144,11 +1144,6 @@ static int rzg3s_pcie_set_max_link_speed(struct rzg3s_pcie_host *host)
>             remote_supported_link_speeds != max_supported_link_speeds)
>                 return 0;
>
> -       /* Set target Link speed */
> -       rzg3s_pcie_update_bits(host->pcie, pcie_cap + PCI_EXP_LNKCTL2,
> -                              PCI_EXP_LNKCTL2_TLS,
> -                              FIELD_PREP(PCI_EXP_LNKCTL2_TLS, link_speed));
> -
>         /* Request link speed change */
>         rzg3s_pcie_update_bits(host->axi, RZG3S_PCI_PCCTRL2,
>                                RZG3S_PCI_PCCTRL2_LS_CHG_REQ |
> --
> 2.43.0
>
>

Re: [PATCH v4 4/8] PCI: rzg3s-host: Move host configuration code together

[Lad, Prabhakar]

On Tue, Sep 22, 2026 at 3:54 PM Claudiu Beznea
<claudiu.beznea@kernel.org> wrote:
>
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>
> Move host configuration code together to have it grouped. This prepares for
> the addition of hotplug support.
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---
>
> Changes in v4:
> - none
>
> Changes in v3:
> - none, this patch is new
>
>  drivers/pci/controller/pcie-rzg3s-host.c | 230 +++++++++++------------
>  1 file changed, 115 insertions(+), 115 deletions(-)
>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Tested-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Cheers,
Prabhakar

> diff --git a/drivers/pci/controller/pcie-rzg3s-host.c b/drivers/pci/controller/pcie-rzg3s-host.c
> index 362aa923978f..3ecada238402 100644
> --- a/drivers/pci/controller/pcie-rzg3s-host.c
> +++ b/drivers/pci/controller/pcie-rzg3s-host.c
> @@ -1356,121 +1356,6 @@ static int rzg3s_pcie_resets_prepare_and_get(struct rzg3s_pcie_host *host)
>                                                               host->cfg_resets);
>  }
>
> -static int rzg3s_pcie_host_parse_port(struct rzg3s_pcie_host *host)
> -{
> -       struct device_node *of_port __free(device_node) =
> -               of_get_next_child(host->dev->of_node, NULL);
> -       struct rzg3s_pcie_port *port = &host->port;
> -       int ret;
> -
> -       ret = of_property_read_u32(of_port, "vendor-id", &port->vendor_id);
> -       if (ret)
> -               return ret;
> -
> -       ret = of_property_read_u32(of_port, "device-id", &port->device_id);
> -       if (ret)
> -               return ret;
> -
> -       port->refclk = of_clk_get_by_name(of_port, "ref");
> -       if (IS_ERR(port->refclk))
> -               return PTR_ERR(port->refclk);
> -
> -       return 0;
> -}
> -
> -static int rzg3s_pcie_host_init_port(struct rzg3s_pcie_host *host)
> -{
> -       struct rzg3s_pcie_port *port = &host->port;
> -       struct device *dev = host->dev;
> -       int ret;
> -
> -       /* Enable access control to the CFGU */
> -       writel_relaxed(RZG3S_PCI_PERM_CFG_HWINIT_EN,
> -                      host->axi + RZG3S_PCI_PERM);
> -
> -       /* Update vendor ID and device ID */
> -       writew_relaxed(port->vendor_id, host->pcie + PCI_VENDOR_ID);
> -       writew_relaxed(port->device_id, host->pcie + PCI_DEVICE_ID);
> -
> -       /* Disable access control to the CFGU */
> -       writel_relaxed(0, host->axi + RZG3S_PCI_PERM);
> -
> -       ret = clk_prepare_enable(port->refclk);
> -       if (ret)
> -               return dev_err_probe(dev, ret, "Failed to enable refclk!\n");
> -
> -       /* Set the PHY, if any */
> -       if (host->data->init_phy) {
> -               ret = host->data->init_phy(host);
> -               if (ret) {
> -                       dev_err_probe(dev, ret, "Failed to set the PHY!\n");
> -                       goto refclk_disable;
> -               }
> -       }
> -
> -       return 0;
> -
> -refclk_disable:
> -       clk_disable_unprepare(port->refclk);
> -       return ret;
> -}
> -
> -static int rzg3s_pcie_host_init(struct rzg3s_pcie_host *host)
> -{
> -       u32 val;
> -       int ret;
> -
> -       /* SoC-specific pre-configuration */
> -       if (host->data->config_pre_init)
> -               host->data->config_pre_init(host);
> -
> -       /* Initialize the PCIe related registers */
> -       ret = rzg3s_pcie_config_init(host);
> -       if (ret)
> -               goto config_deinit;
> -
> -       ret = rzg3s_pcie_host_init_port(host);
> -       if (ret)
> -               goto config_deinit;
> -
> -       /* Enable ASPM L1 transition for SoCs that use it */
> -       ret = rzg3s_sysc_config_func(host->sysc,
> -                                    RZG3S_SYSC_FUNC_ID_L1_ALLOW, 1);
> -       if (ret)
> -               goto config_deinit_and_refclk;
> -
> -       /* Initialize the interrupts */
> -       rzg3s_pcie_irq_init(host);
> -
> -       /* SoC-specific post-configuration */
> -       ret = host->data->config_post_init(host);
> -       if (ret)
> -               goto config_deinit_and_refclk;
> -
> -       /* Wait for link up */
> -       ret = readl_poll_timeout(host->axi + RZG3S_PCI_PCSTAT1, val,
> -                                !(val & RZG3S_PCI_PCSTAT1_DL_DOWN_STS),
> -                                PCIE_LINK_WAIT_SLEEP_MS * MILLI,
> -                                PCIE_LINK_WAIT_SLEEP_MS * MILLI *
> -                                PCIE_LINK_WAIT_MAX_RETRIES);
> -       if (ret)
> -               goto config_deinit_post;
> -
> -       val = readl_relaxed(host->axi + RZG3S_PCI_PCSTAT2);
> -       dev_info(host->dev, "PCIe link status [0x%x]\n", val);
> -
> -       return 0;
> -
> -config_deinit_post:
> -       host->data->config_deinit(host);
> -config_deinit_and_refclk:
> -       clk_disable_unprepare(host->port.refclk);
> -config_deinit:
> -       if (host->data->config_pre_init)
> -               host->data->config_deinit(host);
> -       return ret;
> -}
> -
>  static void rzg3s_pcie_set_inbound_window(struct rzg3s_pcie_host *host,
>                                           u64 cpu_addr, u64 pci_addr, u64 size,
>                                           int id)
> @@ -1698,6 +1583,121 @@ static int rzg3s_soc_pcie_init_phy(struct rzg3s_pcie_host *host)
>         return 0;
>  }
>
> +static int rzg3s_pcie_host_parse_port(struct rzg3s_pcie_host *host)
> +{
> +       struct device_node *of_port __free(device_node) =
> +               of_get_next_child(host->dev->of_node, NULL);
> +       struct rzg3s_pcie_port *port = &host->port;
> +       int ret;
> +
> +       ret = of_property_read_u32(of_port, "vendor-id", &port->vendor_id);
> +       if (ret)
> +               return ret;
> +
> +       ret = of_property_read_u32(of_port, "device-id", &port->device_id);
> +       if (ret)
> +               return ret;
> +
> +       port->refclk = of_clk_get_by_name(of_port, "ref");
> +       if (IS_ERR(port->refclk))
> +               return PTR_ERR(port->refclk);
> +
> +       return 0;
> +}
> +
> +static int rzg3s_pcie_host_init_port(struct rzg3s_pcie_host *host)
> +{
> +       struct rzg3s_pcie_port *port = &host->port;
> +       struct device *dev = host->dev;
> +       int ret;
> +
> +       /* Enable access control to the CFGU */
> +       writel_relaxed(RZG3S_PCI_PERM_CFG_HWINIT_EN,
> +                      host->axi + RZG3S_PCI_PERM);
> +
> +       /* Update vendor ID and device ID */
> +       writew_relaxed(port->vendor_id, host->pcie + PCI_VENDOR_ID);
> +       writew_relaxed(port->device_id, host->pcie + PCI_DEVICE_ID);
> +
> +       /* Disable access control to the CFGU */
> +       writel_relaxed(0, host->axi + RZG3S_PCI_PERM);
> +
> +       ret = clk_prepare_enable(port->refclk);
> +       if (ret)
> +               return dev_err_probe(dev, ret, "Failed to enable refclk!\n");
> +
> +       /* Set the PHY, if any */
> +       if (host->data->init_phy) {
> +               ret = host->data->init_phy(host);
> +               if (ret) {
> +                       dev_err_probe(dev, ret, "Failed to set the PHY!\n");
> +                       goto refclk_disable;
> +               }
> +       }
> +
> +       return 0;
> +
> +refclk_disable:
> +       clk_disable_unprepare(port->refclk);
> +       return ret;
> +}
> +
> +static int rzg3s_pcie_host_init(struct rzg3s_pcie_host *host)
> +{
> +       u32 val;
> +       int ret;
> +
> +       /* SoC-specific pre-configuration */
> +       if (host->data->config_pre_init)
> +               host->data->config_pre_init(host);
> +
> +       /* Initialize the PCIe related registers */
> +       ret = rzg3s_pcie_config_init(host);
> +       if (ret)
> +               goto config_deinit;
> +
> +       ret = rzg3s_pcie_host_init_port(host);
> +       if (ret)
> +               goto config_deinit;
> +
> +       /* Enable ASPM L1 transition for SoCs that use it */
> +       ret = rzg3s_sysc_config_func(host->sysc,
> +                                    RZG3S_SYSC_FUNC_ID_L1_ALLOW, 1);
> +       if (ret)
> +               goto config_deinit_and_refclk;
> +
> +       /* Initialize the interrupts */
> +       rzg3s_pcie_irq_init(host);
> +
> +       /* SoC-specific post-configuration */
> +       ret = host->data->config_post_init(host);
> +       if (ret)
> +               goto config_deinit_and_refclk;
> +
> +       /* Wait for link up */
> +       ret = readl_poll_timeout(host->axi + RZG3S_PCI_PCSTAT1, val,
> +                                !(val & RZG3S_PCI_PCSTAT1_DL_DOWN_STS),
> +                                PCIE_LINK_WAIT_SLEEP_MS * MILLI,
> +                                PCIE_LINK_WAIT_SLEEP_MS * MILLI *
> +                                PCIE_LINK_WAIT_MAX_RETRIES);
> +       if (ret)
> +               goto config_deinit_post;
> +
> +       val = readl_relaxed(host->axi + RZG3S_PCI_PCSTAT2);
> +       dev_info(host->dev, "PCIe link status [0x%x]\n", val);
> +
> +       return 0;
> +
> +config_deinit_post:
> +       host->data->config_deinit(host);
> +config_deinit_and_refclk:
> +       clk_disable_unprepare(host->port.refclk);
> +config_deinit:
> +       if (host->data->config_pre_init)
> +               host->data->config_deinit(host);
> +       return ret;
> +}
> +
>  static int
>  rzg3s_pcie_host_setup(struct rzg3s_pcie_host *host,
>                       int (*init_irqdomain)(struct rzg3s_pcie_host *host),
> --
> 2.43.0
>
>

Re: [PATCH v4 5/8] PCI: rzg3s-host: Move suspend/resume code into dedicated functions

[Lad, Prabhakar]

On Tue, Sep 22, 2026 at 3:56 PM Claudiu Beznea
<claudiu.beznea@kernel.org> wrote:
>
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>
> In preparation for implementing hotplug using
> pci_host_bridge::reset_root_port(), move the suspend/resume code into
> rzg3s_pcie_host_stop() and rzg3s_pcie_host_start(). These functions
> will later be reused by the hotplug implementation through
> pci_host_bridge::reset_root_port().
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---
>
> Changes in v4:
> - none
>
> Changes in v3:
> - none, this patch is new
>
>  drivers/pci/controller/pcie-rzg3s-host.c | 181 ++++++++++++-----------
>  1 file changed, 96 insertions(+), 85 deletions(-)
>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Tested-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Cheers,
Prabhakar

> diff --git a/drivers/pci/controller/pcie-rzg3s-host.c b/drivers/pci/controller/pcie-rzg3s-host.c
> index 3ecada238402..40d5ef3e347e 100644
> --- a/drivers/pci/controller/pcie-rzg3s-host.c
> +++ b/drivers/pci/controller/pcie-rzg3s-host.c
> @@ -1742,6 +1742,100 @@ rzg3s_pcie_host_setup(struct rzg3s_pcie_host *host,
>         return ret;
>  }
>
> +static int rzg3s_pcie_host_stop(struct rzg3s_pcie_host *host)
> +{
> +       const struct rzg3s_pcie_soc_data *data = host->data;
> +       struct rzg3s_pcie_port *port = &host->port;
> +       struct rzg3s_sysc *sysc = host->sysc;
> +       int ret;
> +
> +       clk_disable_unprepare(port->refclk);
> +
> +       /* SoC-specific de-initialization */
> +       ret = data->config_deinit(host);
> +       if (ret)
> +               goto refclk_restore;
> +
> +       ret = reset_control_bulk_assert(data->num_power_resets,
> +                                       host->power_resets);
> +       if (ret)
> +               goto config_reinit;
> +
> +       /*
> +        * Since the power domain's genpd_suspend_noirq() will disable clocks,
> +        * there is no need to manually invoke runtime PM API here.
> +        */
> +
> +       ret = rzg3s_sysc_config_func(sysc, RZG3S_SYSC_FUNC_ID_RST_RSM_B, 0);
> +       if (ret)
> +               goto power_resets_restore;
> +
> +       return 0;
> +
> +       /* Restore the previous state if any error happens */
> +power_resets_restore:
> +       reset_control_bulk_deassert(data->num_power_resets,
> +                                   host->power_resets);
> +config_reinit:
> +       if (data->config_pre_init)
> +               data->config_pre_init(host);
> +       data->config_post_init(host);
> +refclk_restore:
> +       clk_prepare_enable(port->refclk);
> +       return ret;
> +}
> +
> +static int rzg3s_pcie_host_start(struct rzg3s_pcie_host *host)
> +{
> +       const struct rzg3s_pcie_soc_data *data = host->data;
> +       struct rzg3s_sysc *sysc = host->sysc;
> +       int ret;
> +
> +       ret = rzg3s_sysc_config_func(sysc, RZG3S_SYSC_FUNC_ID_MODE, 1);
> +       if (ret)
> +               return ret;
> +
> +       ret = rzg3s_sysc_config_func(sysc, RZG3S_SYSC_FUNC_ID_RST_RSM_B, 1);
> +       if (ret)
> +               return ret;
> +
> +       if (host->num_lanes) {
> +               ret = rzg3s_sysc_config_func(host->sysc,
> +                                            RZG3S_SYSC_FUNC_ID_LINK_MASTER,
> +                                            host->num_lanes == 2  ?
> +                                            RZG3S_SYSC_LINK_MODE_DUAL_X2 :
> +                                            RZG3S_SYSC_LINK_MODE_SINGLE_X4);
> +               if (ret)
> +                       goto assert_rst_rsm_b;
> +       }
> +
> +       /*
> +        * Since the power domain's genpd_resume_noirq() will enable clocks,
> +        * there is no need to manually invoke runtime PM API here.
> +        */
> +
> +       ret = rzg3s_pcie_power_resets_deassert(host);
> +       if (ret)
> +               goto assert_rst_rsm_b;
> +
> +       ret = rzg3s_pcie_host_setup(host, rzg3s_pcie_msi_hw_setup,
> +                                   rzg3s_pcie_msi_hw_teardown);
> +       if (ret)
> +               goto assert_power_resets;
> +
> +       return 0;
> +
> +       /*
> +        * If any error happens there is no way to recover the IP. Put it in the
> +        * lowest possible power state.
> +        */
> +assert_power_resets:
> +       reset_control_bulk_assert(data->num_power_resets, host->power_resets);
> +assert_rst_rsm_b:
> +       rzg3s_sysc_config_func(sysc, RZG3S_SYSC_FUNC_ID_RST_RSM_B, 0);
> +       return ret;
> +}
> +
>  static int rzg3s_pcie_get_controller_id(struct rzg3s_pcie_host *host)
>  {
>         struct device_node *np = host->dev->of_node;
> @@ -1939,98 +2033,15 @@ static int rzg3s_pcie_probe(struct platform_device *pdev)
>  static int rzg3s_pcie_suspend_noirq(struct device *dev)
>  {
>         struct rzg3s_pcie_host *host = dev_get_drvdata(dev);
> -       const struct rzg3s_pcie_soc_data *data = host->data;
> -       struct rzg3s_pcie_port *port = &host->port;
> -       struct rzg3s_sysc *sysc = host->sysc;
> -       int ret;
> -
> -       clk_disable_unprepare(port->refclk);
> -
> -       /* SoC-specific de-initialization */
> -       ret = data->config_deinit(host);
> -       if (ret)
> -               goto refclk_restore;
> -
> -       ret = reset_control_bulk_assert(data->num_power_resets,
> -                                       host->power_resets);
> -       if (ret)
> -               goto config_reinit;
> -
> -       /*
> -        * Since the power domain's genpd_suspend_noirq() will disable clocks,
> -        * there is no need to manually invoke runtime PM API here.
> -        */
> -
> -       ret = rzg3s_sysc_config_func(sysc, RZG3S_SYSC_FUNC_ID_RST_RSM_B, 0);
> -       if (ret)
> -               goto power_resets_restore;
>
> -       return 0;
> -
> -       /* Restore the previous state if any error happens */
> -power_resets_restore:
> -       reset_control_bulk_deassert(data->num_power_resets,
> -                                   host->power_resets);
> -config_reinit:
> -       if (data->config_pre_init)
> -               data->config_pre_init(host);
> -       data->config_post_init(host);
> -refclk_restore:
> -       clk_prepare_enable(port->refclk);
> -       return ret;
> +       return rzg3s_pcie_host_stop(host);
>  }
>
>  static int rzg3s_pcie_resume_noirq(struct device *dev)
>  {
>         struct rzg3s_pcie_host *host = dev_get_drvdata(dev);
> -       const struct rzg3s_pcie_soc_data *data = host->data;
> -       struct rzg3s_sysc *sysc = host->sysc;
> -       int ret;
>
> -       ret = rzg3s_sysc_config_func(sysc, RZG3S_SYSC_FUNC_ID_MODE, 1);
> -       if (ret)
> -               return ret;
> -
> -       ret = rzg3s_sysc_config_func(sysc, RZG3S_SYSC_FUNC_ID_RST_RSM_B, 1);
> -       if (ret)
> -               return ret;
> -
> -       if (host->num_lanes) {
> -               ret = rzg3s_sysc_config_func(host->sysc,
> -                                            RZG3S_SYSC_FUNC_ID_LINK_MASTER,
> -                                            host->num_lanes == 2  ?
> -                                            RZG3S_SYSC_LINK_MODE_DUAL_X2 :
> -                                            RZG3S_SYSC_LINK_MODE_SINGLE_X4);
> -               if (ret)
> -                       goto assert_rst_rsm_b;
> -       }
> -
> -       /*
> -        * Since the power domain's genpd_resume_noirq() will enable clocks,
> -        * there is no need to manually invoke runtime PM API here.
> -        */
> -
> -       ret = rzg3s_pcie_power_resets_deassert(host);
> -       if (ret)
> -               goto assert_rst_rsm_b;
> -
> -       ret = rzg3s_pcie_host_setup(host, rzg3s_pcie_msi_hw_setup,
> -                                   rzg3s_pcie_msi_hw_teardown);
> -       if (ret)
> -               goto assert_power_resets;
> -
> -       return 0;
> -
> -       /*
> -        * If any error happens there is no way to recover the IP. Put it in the
> -        * lowest possible power state.
> -        */
> -assert_power_resets:
> -       reset_control_bulk_assert(data->num_power_resets,
> -                                 host->power_resets);
> -assert_rst_rsm_b:
> -       rzg3s_sysc_config_func(sysc, RZG3S_SYSC_FUNC_ID_RST_RSM_B, 0);
> -       return ret;
> +       return rzg3s_pcie_host_start(host);
>  }
>
>  static const struct dev_pm_ops rzg3s_pcie_pm_ops = {
> --
> 2.43.0
>
>

Re: [PATCH v4 6/8] PCI: rzg3s-host: Move IRQ domain setup code

[Lad, Prabhakar]

On Tue, Sep 22, 2026 at 3:54 PM Claudiu Beznea
<claudiu.beznea@kernel.org> wrote:
>
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>
> Subsequent patches add support for the event IRQ to handle link
> up/down events. The event IRQ handler will use
> rzg3s_pcie_set_max_link_speed(). In preparation for adding event IRQ
> support, move the IRQ domain initialization code after
> rzg3s_pcie_set_max_link_speed().
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---
>
> Changes in v4:
> - none
>
> Changes in v3:
> - none, this patch is new
>
>  drivers/pci/controller/pcie-rzg3s-host.c | 147 +++++++++++------------
>  1 file changed, 73 insertions(+), 74 deletions(-)
>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Tested-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Cheers,
Prabhakar

> diff --git a/drivers/pci/controller/pcie-rzg3s-host.c b/drivers/pci/controller/pcie-rzg3s-host.c
> index 40d5ef3e347e..f4b27a77a89d 100644
> --- a/drivers/pci/controller/pcie-rzg3s-host.c
> +++ b/drivers/pci/controller/pcie-rzg3s-host.c
> @@ -1006,80 +1006,6 @@ static const struct irq_domain_ops rzg3s_pcie_intx_domain_ops = {
>         .xlate = irq_domain_xlate_onetwocell,
>  };
>
> -static void rzg3s_pcie_teardown_intx(struct rzg3s_pcie_host *host,
> -                                    int count)
> -{
> -       while (--count >= 0) {
> -               irq_set_chained_handler_and_data(host->intx_irqs[count], NULL,
> -                                                NULL);
> -       }
> -
> -       if (host->intx_domain)
> -               irq_domain_remove(host->intx_domain);
> -}
> -
> -static int rzg3s_pcie_init_irqdomain(struct rzg3s_pcie_host *host)
> -{
> -       struct device *dev = host->dev;
> -       struct platform_device *pdev = to_platform_device(dev);
> -       int i, ret;
> -
> -       for (i = 0; i < PCI_NUM_INTX; i++) {
> -               char irq_name[5] = {0};
> -               int irq;
> -
> -               scnprintf(irq_name, ARRAY_SIZE(irq_name), "int%c", 'a' + i);
> -
> -               irq = platform_get_irq_byname(pdev, irq_name);
> -               if (irq < 0) {
> -                       ret = irq;
> -                       dev_err_probe(dev, ret,
> -                                     "Failed to parse and map INT%c IRQ\n",
> -                                     'A' + i);
> -                       goto teardown_intx;
> -               }
> -
> -               host->intx_irqs[i] = irq;
> -               irq_set_chained_handler_and_data(irq,
> -                                                rzg3s_pcie_intx_irq_handler,
> -                                                host);
> -       }
> -
> -       host->intx_domain = irq_domain_create_linear(dev_fwnode(dev),
> -                                                    PCI_NUM_INTX,
> -                                                    &rzg3s_pcie_intx_domain_ops,
> -                                                    host);
> -       if (!host->intx_domain) {
> -               ret = -EINVAL;
> -               dev_err_probe(dev, ret,
> -                             "Failed to add irq domain for INTx IRQs\n");
> -               goto teardown_intx;
> -       }
> -       irq_domain_update_bus_token(host->intx_domain, DOMAIN_BUS_WIRED);
> -
> -       if (IS_ENABLED(CONFIG_PCI_MSI)) {
> -               ret = rzg3s_pcie_init_msi(host);
> -
> -               if (ret)
> -                       goto teardown_intx;
> -       }
> -
> -       return 0;
> -
> -teardown_intx:
> -       rzg3s_pcie_teardown_intx(host, i);
> -
> -       return ret;
> -}
> -
> -static void rzg3s_pcie_teardown_irqdomain(struct rzg3s_pcie_host *host)
> -{
> -       if (IS_ENABLED(CONFIG_PCI_MSI))
> -               rzg3s_pcie_teardown_msi(host);
> -
> -       rzg3s_pcie_teardown_intx(host, PCI_NUM_INTX);
> -}
> -
>  static int rzg3s_pcie_set_max_link_speed(struct rzg3s_pcie_host *host)
>  {
>         u32 remote_supported_link_speeds, max_supported_link_speeds;
> @@ -1169,6 +1095,79 @@ static int rzg3s_pcie_set_max_link_speed(struct rzg3s_pcie_host *host)
>         return ret;
>  }
>
> +static void rzg3s_pcie_teardown_intx(struct rzg3s_pcie_host *host, int count)
> +{
> +       if (host->intx_domain)
> +               irq_domain_remove(host->intx_domain);
> +
> +       while (--count >= 0) {
> +               irq_set_chained_handler_and_data(host->intx_irqs[count], NULL,
> +                                                NULL);
> +       }
> +}
> +
> +static int rzg3s_pcie_init_irqdomain(struct rzg3s_pcie_host *host)
> +{
> +       struct device *dev = host->dev;
> +       struct platform_device *pdev = to_platform_device(dev);
> +       int i, ret;
> +
> +       for (i = 0; i < PCI_NUM_INTX; i++) {
> +               char irq_name[5] = {0};
> +               int irq;
> +
> +               scnprintf(irq_name, ARRAY_SIZE(irq_name), "int%c", 'a' + i);
> +
> +               irq = platform_get_irq_byname(pdev, irq_name);
> +               if (irq < 0) {
> +                       ret = irq;
> +                       dev_err_probe(dev, ret,
> +                                     "Failed to parse and map INT%c IRQ\n",
> +                                     'A' + i);
> +                       goto teardown_intx;
> +               }
> +
> +               host->intx_irqs[i] = irq;
> +               irq_set_chained_handler_and_data(irq,
> +                                                rzg3s_pcie_intx_irq_handler,
> +                                                host);
> +       }
> +
> +       host->intx_domain = irq_domain_create_linear(dev_fwnode(dev),
> +                                                    PCI_NUM_INTX,
> +                                                    &rzg3s_pcie_intx_domain_ops,
> +                                                    host);
> +       if (!host->intx_domain) {
> +               ret = -EINVAL;
> +               dev_err_probe(dev, ret,
> +                             "Failed to add irq domain for INTx IRQs\n");
> +               goto teardown_intx;
> +       }
> +       irq_domain_update_bus_token(host->intx_domain, DOMAIN_BUS_WIRED);
> +
> +       if (IS_ENABLED(CONFIG_PCI_MSI)) {
> +               ret = rzg3s_pcie_init_msi(host);
> +
> +               if (ret)
> +                       goto teardown_intx;
> +       }
> +
> +       return 0;
> +
> +teardown_intx:
> +       rzg3s_pcie_teardown_intx(host, i);
> +
> +       return ret;
> +}
> +
> +static void rzg3s_pcie_teardown_irqdomain(struct rzg3s_pcie_host *host)
> +{
> +       if (IS_ENABLED(CONFIG_PCI_MSI))
> +               rzg3s_pcie_teardown_msi(host);
> +
> +       rzg3s_pcie_teardown_intx(host, PCI_NUM_INTX);
> +}
> +
>  static int rzg3s_pcie_config_init(struct rzg3s_pcie_host *host)
>  {
>         struct pci_host_bridge *bridge = pci_host_bridge_from_priv(host);
> --
> 2.43.0
>
>

Re: [PATCH v4 7/8] PCI: rzg3s-host: Re-enumerate the bus on PCIe link-state changes

[Lad, Prabhakar]

On Tue, Sep 22, 2026 at 3:55 PM Claudiu Beznea
<claudiu.beznea@kernel.org> wrote:
>
> From: John Madieu <john.madieu.xa@bp.renesas.com>
>
> The RZ/G3{E, S}, RZ/V2{H(P), N} PCIe controllers does not expose the
> standard PCIe Slot Capability registers, so the generic pciehp driver
> cannot be used. The only link-state signal the hardware provides is the
> DL_UpDown bit in the PEIS0 event status register, which is raised on every
> Data Link layer up/down transition.
>
> Enable DL_UpDown in PEIE0 and hook up an interrupt handler so the driver
> can react to link-state changes: a device that trains after boot gets
> enumerated, and a device that disappears on link loss is removed. This
> provides hotplug-like behavior without the PCI hotplug core, which is
> unavailable for the reason above.
>
> On a DL_UpDown event the handler acks the W1C status bit and schedules a
> worker that inspects PCSTAT1.DL_DOWN_STS:
>
>   - link up: re-run max link speed negotiation, wait for the link to
>     settle and pci_rescan_bus() the root bus;
>   - link down: walk the bus in reverse and pci_stop_and_remove_bus_device()
>     each child.
>
> Both paths take pci_lock_rescan_remove() to serialize against the PCI
> core.
>
> Link events are processed only after the controller has been fully
> initialized.
>
> While at it, make probe tolerant of an absent device. Previously, if the
> link failed to come up during rzg3s_pcie_host_init(), probe tore the
> controller back down and failed. Distinguish this case with -ENODEV,
> leave the controller and refclk running, and let the link-up path
> enumerate the device once it appears.
>
> Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
> Co-developed-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---
>
> Changes in v4:
> - dropped .reset_root_port() changes
>
> Changes in v3:
> - added RZG3S_PCI_PEIE0_DL_UPDOWN
> - re-worked the support by implemeting
>   struct pci_host_bridge::reset_root_port()
> - introduced the struct rzg3s_pcie_host::state to:
> -- avoid touching the controller while a reset root port is in progress
> -- and avoid touching the controller in case a reset root port failed
> -- and to be able to re-use the already existing code in the reset
>    root port function
> -- and added CLASS() constructs helpers for it to keep the state handling
>    code simpler
> - updated the patch description to reflect the updates
>
>  drivers/pci/controller/pcie-rzg3s-host.c | 155 ++++++++++++++++++++---
>  1 file changed, 139 insertions(+), 16 deletions(-)
>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Tested-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Cheers,
Prabhakar

> diff --git a/drivers/pci/controller/pcie-rzg3s-host.c b/drivers/pci/controller/pcie-rzg3s-host.c
> index f4b27a77a89d..1c0353598b59 100644
> --- a/drivers/pci/controller/pcie-rzg3s-host.c
> +++ b/drivers/pci/controller/pcie-rzg3s-host.c
> @@ -86,6 +86,7 @@
>  #define RZG3S_PCI_MSGRCVIS_MRI                 BIT(24)
>
>  #define RZG3S_PCI_PEIE0                                0x200
> +#define RZG3S_PCI_PEIE0_DL_UPDOWN              BIT(9)
>
>  #define RZG3S_PCI_PEIS0                                0x204
>  #define RZG3S_PCI_PEIS0_RX_DLLP_PM_ENTER       BIT(12)
> @@ -323,6 +324,7 @@ struct rzg3s_pcie_port {
>   * @msi: MSI data structure
>   * @port: PCIe Root Port
>   * @hw_lock: lock for access to the HW resources
> + * @event_irq: PCIe event interrupt for DL_UpDown detection
>   * @intx_irqs: INTx interrupts
>   * @max_link_speed: maximum supported link speed
>   * @controller_id: PCIe controller identifier, used for System Controller access
> @@ -340,6 +342,7 @@ struct rzg3s_pcie_host {
>         struct rzg3s_pcie_msi msi;
>         struct rzg3s_pcie_port port;
>         raw_spinlock_t hw_lock;
> +       int event_irq;
>         int intx_irqs[PCI_NUM_INTX];
>         int max_link_speed;
>         enum rzg3s_pcie_controller_id controller_id;
> @@ -1095,6 +1098,89 @@ static int rzg3s_pcie_set_max_link_speed(struct rzg3s_pcie_host *host)
>         return ret;
>  }
>
> +static void rzg3s_pcie_link_event(struct rzg3s_pcie_host *host)
> +{
> +       struct pci_host_bridge *bridge = pci_host_bridge_from_priv(host);
> +       struct pci_bus *bus = bridge->bus;
> +       u32 val;
> +
> +       val = readl_relaxed(host->axi + RZG3S_PCI_PCSTAT1);
> +       if (val & RZG3S_PCI_PCSTAT1_DL_DOWN_STS) {
> +               struct pci_dev *dev, *tmp;
> +
> +               dev_info(host->dev, "PCIe link down, removing devices\n");
> +
> +               pci_lock_rescan_remove();
> +               list_for_each_entry_safe_reverse(dev, tmp, &bus->devices,
> +                                                bus_list)
> +                       pci_stop_and_remove_bus_device(dev);
> +               pci_unlock_rescan_remove();
> +       } else {
> +               int ret;
> +
> +               dev_info(host->dev, "PCIe link up, rescanning bus\n");
> +
> +               /*
> +                * Attempt link speed negotiation now that the link is up.
> +                * Failure is non-fatal: the device works at the negotiated
> +                * speed.
> +                */
> +               ret = rzg3s_pcie_set_max_link_speed(host);
> +               if (ret)
> +                       dev_info(host->dev, "Failed to set max link speed\n");
> +
> +               pci_host_common_link_train_delay(host->max_link_speed);
> +
> +               pci_lock_rescan_remove();
> +               pci_rescan_bus(bus);
> +               pci_unlock_rescan_remove();
> +       }
> +}
> +
> +static irqreturn_t rzg3s_pcie_event_irq_thread(int irq, void *data)
> +{
> +       struct rzg3s_pcie_host *host = data;
> +       u32 status;
> +
> +       status = readl_relaxed(host->axi + RZG3S_PCI_PEIS0);
> +
> +       if (!(status & RZG3S_PCI_PEIS0_DL_UPDOWN))
> +               return IRQ_NONE;
> +
> +       /* Clear the DL_UpDown status (W1C) */
> +       writel_relaxed(RZG3S_PCI_PEIS0_DL_UPDOWN, host->axi + RZG3S_PCI_PEIS0);
> +
> +       rzg3s_pcie_link_event(host);
> +
> +       return IRQ_HANDLED;
> +}
> +
> +static int rzg3s_pcie_request_event_irq(struct rzg3s_pcie_host *host)
> +{
> +       struct device *dev = host->dev;
> +       struct platform_device *pdev = to_platform_device(dev);
> +       const char *evt_name;
> +       int ret, irq;
> +
> +       evt_name = devm_kasprintf(dev, GFP_KERNEL, "%s-evt", dev_name(dev));
> +       if (!evt_name)
> +               return -ENOMEM;
> +
> +       irq = platform_get_irq_byname(pdev, "pcie_evt");
> +       if (irq < 0)
> +               return irq;
> +
> +       ret = request_threaded_irq(irq, NULL, rzg3s_pcie_event_irq_thread,
> +                                  IRQF_ONESHOT, evt_name, host);
> +       if (ret) {
> +               return dev_err_probe(dev, ret,
> +                                    "Failed to request pcie_evt IRQ\n");
> +       }
> +       host->event_irq = irq;
> +
> +       return 0;
> +}
> +
>  static void rzg3s_pcie_teardown_intx(struct rzg3s_pcie_host *host, int count)
>  {
>         if (host->intx_domain)
> @@ -1106,6 +1192,17 @@ static void rzg3s_pcie_teardown_intx(struct rzg3s_pcie_host *host, int count)
>         }
>  }
>
> +static void rzg3s_pcie_teardown_irqdomain(struct rzg3s_pcie_host *host)
> +{
> +       if (host->event_irq > 0)
> +               free_irq(host->event_irq, host);
> +
> +       if (IS_ENABLED(CONFIG_PCI_MSI))
> +               rzg3s_pcie_teardown_msi(host);
> +
> +       rzg3s_pcie_teardown_intx(host, PCI_NUM_INTX);
> +}
> +
>  static int rzg3s_pcie_init_irqdomain(struct rzg3s_pcie_host *host)
>  {
>         struct device *dev = host->dev;
> @@ -1152,22 +1249,21 @@ static int rzg3s_pcie_init_irqdomain(struct rzg3s_pcie_host *host)
>                         goto teardown_intx;
>         }
>
> +       ret = rzg3s_pcie_request_event_irq(host);
> +       if (ret)
> +               goto teardown_msi;
> +
>         return 0;
>
> +teardown_msi:
> +       if (IS_ENABLED(CONFIG_PCI_MSI))
> +               rzg3s_pcie_teardown_msi(host);
>  teardown_intx:
>         rzg3s_pcie_teardown_intx(host, i);
>
>         return ret;
>  }
>
> -static void rzg3s_pcie_teardown_irqdomain(struct rzg3s_pcie_host *host)
> -{
> -       if (IS_ENABLED(CONFIG_PCI_MSI))
> -               rzg3s_pcie_teardown_msi(host);
> -
> -       rzg3s_pcie_teardown_intx(host, PCI_NUM_INTX);
> -}
> -
>  static int rzg3s_pcie_config_init(struct rzg3s_pcie_host *host)
>  {
>         struct pci_host_bridge *bridge = pci_host_bridge_from_priv(host);
> @@ -1679,16 +1775,21 @@ static int rzg3s_pcie_host_init(struct rzg3s_pcie_host *host)
>                                  PCIE_LINK_WAIT_SLEEP_MS * MILLI,
>                                  PCIE_LINK_WAIT_SLEEP_MS * MILLI *
>                                  PCIE_LINK_WAIT_MAX_RETRIES);
> -       if (ret)
> -               goto config_deinit_post;
> +       if (ret) {
> +               /*
> +                * Link is down. Leave the controller running so the
> +                * DL_UpDown handler can enumerate a device that appears
> +                * later.
> +                */
> +               dev_info(host->dev, "PCIe link down, waiting for DL_UpDown\n");
> +               ret = -ENODEV;
> +       }
>
>         val = readl_relaxed(host->axi + RZG3S_PCI_PCSTAT2);
>         dev_info(host->dev, "PCIe link status [0x%x]\n", val);
>
> -       return 0;
> +       return ret;
>
> -config_deinit_post:
> -       host->data->config_deinit(host);
>  config_deinit_and_refclk:
>         clk_disable_unprepare(host->port.refclk);
>  config_deinit:
> @@ -1723,8 +1824,14 @@ rzg3s_pcie_host_setup(struct rzg3s_pcie_host *host,
>
>         ret = rzg3s_pcie_host_init(host);
>         if (ret) {
> -               dev_err_probe(dev, ret, "Failed to initialize the HW!\n");
> -               goto teardown_irqdomain;
> +               if (ret != -ENODEV) {
> +                       dev_err_probe(dev, ret,
> +                                     "Failed to initialize the HW!\n");
> +                       goto teardown_irqdomain;
> +               }
> +
> +               /* Link is down: hotplug via DL_UpDown will recover. */
> +               return 0;
>         }
>
>         ret = rzg3s_pcie_set_max_link_speed(host);
> @@ -2004,6 +2111,14 @@ static int rzg3s_pcie_probe(struct platform_device *pdev)
>         if (ret)
>                 goto host_probe_teardown;
>
> +       /*
> +        * Unmask the PCIe event IRQ at the end of probe to avoid
> +        * spurious link-state events during controller setup and bus
> +        * enumeration. From here on, DL_UpDown events trigger the link
> +        * IRQ thread to (re)scan the bus.
> +        */
> +       writel_relaxed(RZG3S_PCI_PEIE0_DL_UPDOWN, host->axi + RZG3S_PCI_PEIE0);
> +
>         return 0;
>
>  host_probe_teardown:
> @@ -2039,8 +2154,16 @@ static int rzg3s_pcie_suspend_noirq(struct device *dev)
>  static int rzg3s_pcie_resume_noirq(struct device *dev)
>  {
>         struct rzg3s_pcie_host *host = dev_get_drvdata(dev);
> +       int ret;
>
> -       return rzg3s_pcie_host_start(host);
> +       ret = rzg3s_pcie_host_start(host);
> +       if (ret)
> +               return ret;
> +
> +       /* Unmask link up/down IRQ. */
> +       writel_relaxed(RZG3S_PCI_PEIE0_DL_UPDOWN, host->axi + RZG3S_PCI_PEIE0);
> +
> +       return 0;
>  }
>
>  static const struct dev_pm_ops rzg3s_pcie_pm_ops = {
> --
> 2.43.0
>
>

Re: [PATCH v4 8/8] PCI: rzg3s-host: Add bridge::reset_root_port()

[Lad, Prabhakar]

On Tue, Sep 22, 2026 at 3:55 PM Claudiu Beznea
<claudiu.beznea@kernel.org> wrote:
>
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>
> Add bridge::reset_root_port(). This allows, on some devices, performing
> reads and writes to an NVMe endpoint after a link down/link up cycle
> without failures.
>
> The implementation of struct pci_host_bridge::reset_root_port() masks all
> enabled interrupts and synchronizes them before resetting the controller
> to prevent asynchronous events from interfering with the reset operation.
>
> struct rzg3s_pcie_host::started boolean was added to keep track of the
> controller state, since rzg3s_pcie_host_start() and rzg3s_pcie_host_stop()
> are called from multiple paths: .reset_root_port(), .suspend_noirq() and
> .resume_noirq(). The flag prevents double start/stop and breaking
> reference counters for clocks and resets.
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---
>
> Changes in v4:
> - this patch is new and is a split from patch 7 of v3
> - use a boolean to keep track of the controller start state; with this
>   the state management code from patch 7 of v3 was dropped
>
>  drivers/pci/controller/pcie-rzg3s-host.c | 77 +++++++++++++++++++++++-
>  1 file changed, 75 insertions(+), 2 deletions(-)
>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Tested-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Cheers,
Prabhakar

> diff --git a/drivers/pci/controller/pcie-rzg3s-host.c b/drivers/pci/controller/pcie-rzg3s-host.c
> index 1c0353598b59..9cba898a9536 100644
> --- a/drivers/pci/controller/pcie-rzg3s-host.c
> +++ b/drivers/pci/controller/pcie-rzg3s-host.c
> @@ -328,6 +328,7 @@ struct rzg3s_pcie_port {
>   * @intx_irqs: INTx interrupts
>   * @max_link_speed: maximum supported link speed
>   * @controller_id: PCIe controller identifier, used for System Controller access
> + * @started: The PCIe controller state (started or not)
>   * @num_lanes: The number of lanes
>   */
>  struct rzg3s_pcie_host {
> @@ -346,6 +347,7 @@ struct rzg3s_pcie_host {
>         int intx_irqs[PCI_NUM_INTX];
>         int max_link_speed;
>         enum rzg3s_pcie_controller_id controller_id;
> +       bool started;
>         u8 num_lanes;
>  };
>
> @@ -1110,6 +1112,11 @@ static void rzg3s_pcie_link_event(struct rzg3s_pcie_host *host)
>
>                 dev_info(host->dev, "PCIe link down, removing devices\n");
>
> +               for_each_pci_bridge(dev, bridge->bus) {
> +                       if (pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT)
> +                               pci_host_handle_link_down(dev);
> +               }
> +
>                 pci_lock_rescan_remove();
>                 list_for_each_entry_safe_reverse(dev, tmp, &bus->devices,
>                                                  bus_list)
> @@ -1855,6 +1862,9 @@ static int rzg3s_pcie_host_stop(struct rzg3s_pcie_host *host)
>         struct rzg3s_sysc *sysc = host->sysc;
>         int ret;
>
> +       if (!host->started)
> +               return 0;
> +
>         clk_disable_unprepare(port->refclk);
>
>         /* SoC-specific de-initialization */
> @@ -1876,6 +1886,8 @@ static int rzg3s_pcie_host_stop(struct rzg3s_pcie_host *host)
>         if (ret)
>                 goto power_resets_restore;
>
> +       host->started = false;
> +
>         return 0;
>
>         /* Restore the previous state if any error happens */
> @@ -1891,12 +1903,15 @@ static int rzg3s_pcie_host_stop(struct rzg3s_pcie_host *host)
>         return ret;
>  }
>
> -static int rzg3s_pcie_host_start(struct rzg3s_pcie_host *host)
> +static int rzg3s_pcie_host_start(struct rzg3s_pcie_host *host, bool set_started)
>  {
>         const struct rzg3s_pcie_soc_data *data = host->data;
>         struct rzg3s_sysc *sysc = host->sysc;
>         int ret;
>
> +       if (host->started)
> +               return 0;
> +
>         ret = rzg3s_sysc_config_func(sysc, RZG3S_SYSC_FUNC_ID_MODE, 1);
>         if (ret)
>                 return ret;
> @@ -1929,6 +1944,9 @@ static int rzg3s_pcie_host_start(struct rzg3s_pcie_host *host)
>         if (ret)
>                 goto assert_power_resets;
>
> +       if (set_started)
> +               host->started = true;
> +
>         return 0;
>
>         /*
> @@ -1942,6 +1960,58 @@ static int rzg3s_pcie_host_start(struct rzg3s_pcie_host *host)
>         return ret;
>  }
>
> +static int rzg3s_pcie_host_reset_root_port(struct pci_host_bridge *bridge,
> +                                          struct pci_dev *pdev)
> +{
> +       struct rzg3s_pcie_host *host = pci_host_bridge_priv(bridge);
> +       u32 irqs;
> +       int ret;
> +
> +       /* Mask link up/down interrupts. */
> +       writel(0, host->axi + RZG3S_PCI_PEIE0);
> +
> +       /* Mask INTx and MSI interrupts. */
> +       irqs = readl_relaxed(host->axi + RZG3S_PCI_PINTRCVIE);
> +       writel(0, host->axi + RZG3S_PCI_PINTRCVIE);
> +
> +       /*
> +        * Make sure the next operations are not disturbed by any pending
> +        * IRQs.
> +        */
> +       synchronize_irq(host->msi.irq);
> +       for (unsigned int i = 0; i < PCI_NUM_INTX; i++)
> +               synchronize_irq(host->intx_irqs[i]);
> +
> +       ret = rzg3s_pcie_host_stop(host);
> +       if (ret) {
> +               dev_err(host->dev, "Failed to stop the host!\n");
> +               goto unmask_irqs;
> +       }
> +
> +       ret = rzg3s_pcie_host_start(host, false);
> +       if (ret) {
> +               dev_err(host->dev, "Failed to start the host!\n");
> +
> +               /*
> +                * Don't unmask IRQs. We are in a bad state here and we
> +                * can recover only through a suspend/resume cycle. Just
> +                * return and preserve the stop state.
> +                */
> +               return ret;
> +       }
> +
> +unmask_irqs:
> +       /* Unmask INTx and MSI interrupts. */
> +       writel_relaxed(irqs, host->axi + RZG3S_PCI_PINTRCVIE);
> +
> +       /* Unmask link up/down interrupts. */
> +       writel(RZG3S_PCI_PEIE0_DL_UPDOWN, host->axi + RZG3S_PCI_PEIE0);
> +
> +       host->started = true;
> +
> +       return ret;
> +}
> +
>  static int rzg3s_pcie_get_controller_id(struct rzg3s_pcie_host *host)
>  {
>         struct device_node *np = host->dev->of_node;
> @@ -2104,9 +2174,12 @@ static int rzg3s_pcie_probe(struct platform_device *pdev)
>         if (ret)
>                 goto power_resets_assert;
>
> +       host->started = true;
> +
>         bridge->sysdata = host;
>         bridge->ops = &rzg3s_pcie_root_ops;
>         bridge->child_ops = &rzg3s_pcie_child_ops;
> +       bridge->reset_root_port = rzg3s_pcie_host_reset_root_port;
>         ret = pci_host_probe(bridge);
>         if (ret)
>                 goto host_probe_teardown;
> @@ -2156,7 +2229,7 @@ static int rzg3s_pcie_resume_noirq(struct device *dev)
>         struct rzg3s_pcie_host *host = dev_get_drvdata(dev);
>         int ret;
>
> -       ret = rzg3s_pcie_host_start(host);
> +       ret = rzg3s_pcie_host_start(host, true);
>         if (ret)
>                 return ret;
>
> --
> 2.43.0
>
>