[PATCH v3 0/7] PCI: rzg3s-host: Add PCIe hotplug support

[PATCH v3 0/7] 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 v2:
- 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 (6):
  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

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

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

-- 
2.43.0

[PATCH v3 1/7] 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 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 3f7393c49a08..830cd7c76699 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 v3 2/7] 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 v3:
- none, this patch is new

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

diff --git a/drivers/pci/controller/pcie-rzg3s-host.c b/drivers/pci/controller/pcie-rzg3s-host.c
index 830cd7c76699..bbc5bcec03f1 100644
--- a/drivers/pci/controller/pcie-rzg3s-host.c
+++ b/drivers/pci/controller/pcie-rzg3s-host.c
@@ -1961,19 +1961,32 @@ 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 runtime PM usage count was incremented by
+	 *   pm_runtime_get_noresume() in the system suspend/resume code before
+	 *   executing the subsystem-level .prepare() callback
+	 *   (in device_prepare()),
+	 * - runtime PM has been disabled by the system suspend/resume code
+	 *   before executing the subsystem-level .suspend_late() callback
+	 *   (in device_suspend_late()), and
+	 * - the PCIe driver's runtime PM state remains RPM_ACTIVE (it was
+	 *   runtime resumed in probe()),
+	 *
+	 * any runtime PM operation becomes a no-op and may lead to unexpected
+	 * failures.
+	 *
+	 * Let the power domain's genpd_suspend_noirq() callback disable the
+	 * clocks, as it already does.
+	 */
 
 	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 +2024,28 @@ 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 runtime PM usage count was incremented by
+	 *   pm_runtime_get_noresume() in the system suspend/resume code before
+	 *   executing the subsystem-level .prepare() callback
+	 *   (in device_prepare()),
+	 * - runtime PM has been disabled by the system suspend/resume code
+	 *   before executing the subsystem-level .suspend_late() callback
+	 *   (in device_suspend_late()),
+	 * - on resume pm_runtime_enable() is called after executing the
+	 *   subsystem-level .resume_early() (in device_resume_early())
+	 *
+	 * any runtime PM operation becomes a no-op and may lead to unexpected
+	 * failures.
+	 *
+	 * Let the power domain's genpd_resume_noirq() callback enable the
+	 * clocks, as it already does.
+	 */
 
 	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 +2061,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 v3 2/7] PCI: rzg3s-host: Fix runtime PM handling in the NOIRQ suspend/resume phase

[Manivannan Sadhasivam]

On Fri, Aug 14, 2026 at 05:13:07PM +0300, Claudiu Beznea 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 v3:
> - none, this patch is new
> 
>  drivers/pci/controller/pcie-rzg3s-host.c | 50 ++++++++++++++++++------
>  1 file changed, 38 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/pci/controller/pcie-rzg3s-host.c b/drivers/pci/controller/pcie-rzg3s-host.c
> index 830cd7c76699..bbc5bcec03f1 100644
> --- a/drivers/pci/controller/pcie-rzg3s-host.c
> +++ b/drivers/pci/controller/pcie-rzg3s-host.c
> @@ -1961,19 +1961,32 @@ 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 runtime PM usage count was incremented by
> +	 *   pm_runtime_get_noresume() in the system suspend/resume code before
> +	 *   executing the subsystem-level .prepare() callback
> +	 *   (in device_prepare()),
> +	 * - runtime PM has been disabled by the system suspend/resume code
> +	 *   before executing the subsystem-level .suspend_late() callback
> +	 *   (in device_suspend_late()), and
> +	 * - the PCIe driver's runtime PM state remains RPM_ACTIVE (it was
> +	 *   runtime resumed in probe()),
> +	 *
> +	 * any runtime PM operation becomes a no-op and may lead to unexpected
> +	 * failures.
> +	 *
> +	 * Let the power domain's genpd_suspend_noirq() callback disable the
> +	 * clocks, as it already does.
> +	 */

I believe you can shorten the comment. Like:

	/*
	 * 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 +2024,28 @@ 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 runtime PM usage count was incremented by
> +	 *   pm_runtime_get_noresume() in the system suspend/resume code before
> +	 *   executing the subsystem-level .prepare() callback
> +	 *   (in device_prepare()),
> +	 * - runtime PM has been disabled by the system suspend/resume code
> +	 *   before executing the subsystem-level .suspend_late() callback
> +	 *   (in device_suspend_late()),
> +	 * - on resume pm_runtime_enable() is called after executing the
> +	 *   subsystem-level .resume_early() (in device_resume_early())
> +	 *
> +	 * any runtime PM operation becomes a no-op and may lead to unexpected
> +	 * failures.
> +	 *
> +	 * Let the power domain's genpd_resume_noirq() callback enable the
> +	 * clocks, as it already does.
> +	 */

Same as above.

- Mani

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

[PATCH v3 3/7] 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 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 bbc5bcec03f1..3a28f5a15719 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 v3 4/7] 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 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 3a28f5a15719..579386d1aaa3 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 v3 5/7] 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 v3:
- none, this patch is new

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

diff --git a/drivers/pci/controller/pcie-rzg3s-host.c b/drivers/pci/controller/pcie-rzg3s-host.c
index 579386d1aaa3..834b03d56713 100644
--- a/drivers/pci/controller/pcie-rzg3s-host.c
+++ b/drivers/pci/controller/pcie-rzg3s-host.c
@@ -1742,6 +1742,128 @@ 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 runtime PM usage count was incremented by
+	 *   pm_runtime_get_noresume() in the system suspend/resume code before
+	 *   executing the subsystem-level .prepare() callback
+	 *   (in device_prepare()),
+	 * - runtime PM has been disabled by the system suspend/resume code
+	 *   before executing the subsystem-level .suspend_late() callback
+	 *   (in device_suspend_late()), and
+	 * - the PCIe driver's runtime PM state remains RPM_ACTIVE (it was
+	 *   runtime resumed in probe()),
+	 *
+	 * any runtime PM operation becomes a no-op and may lead to unexpected
+	 * failures.
+	 *
+	 * Let the power domain's genpd_suspend_noirq() callback disable the
+	 * clocks, as it already does.
+	 */
+
+	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 runtime PM usage count was incremented by
+	 *   pm_runtime_get_noresume() in the system suspend/resume code before
+	 *   executing the subsystem-level .prepare() callback
+	 *   (in device_prepare()),
+	 * - runtime PM has been disabled by the system suspend/resume code
+	 *   before executing the subsystem-level .suspend_late() callback
+	 *   (in device_suspend_late()),
+	 * - on resume pm_runtime_enable() is called after executing the
+	 *   subsystem-level .resume_early() (in device_resume_early())
+	 *
+	 * any runtime PM operation becomes a no-op and may lead to unexpected
+	 * failures.
+	 *
+	 * Let the power domain's genpd_resume_noirq() callback enable the
+	 * clocks, as it already does.
+	 */
+
+	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,126 +2061,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 runtime PM usage count was incremented by
-	 *   pm_runtime_get_noresume() in the system suspend/resume code before
-	 *   executing the subsystem-level .prepare() callback
-	 *   (in device_prepare()),
-	 * - runtime PM has been disabled by the system suspend/resume code
-	 *   before executing the subsystem-level .suspend_late() callback
-	 *   (in device_suspend_late()), and
-	 * - the PCIe driver's runtime PM state remains RPM_ACTIVE (it was
-	 *   runtime resumed in probe()),
-	 *
-	 * any runtime PM operation becomes a no-op and may lead to unexpected
-	 * failures.
-	 *
-	 * Let the power domain's genpd_suspend_noirq() callback disable the
-	 * clocks, as it already does.
-	 */
-
-	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 runtime PM usage count was incremented by
-	 *   pm_runtime_get_noresume() in the system suspend/resume code before
-	 *   executing the subsystem-level .prepare() callback
-	 *   (in device_prepare()),
-	 * - runtime PM has been disabled by the system suspend/resume code
-	 *   before executing the subsystem-level .suspend_late() callback
-	 *   (in device_suspend_late()),
-	 * - on resume pm_runtime_enable() is called after executing the
-	 *   subsystem-level .resume_early() (in device_resume_early())
-	 *
-	 * any runtime PM operation becomes a no-op and may lead to unexpected
-	 * failures.
-	 *
-	 * Let the power domain's genpd_resume_noirq() callback enable the
-	 * clocks, as it already does.
-	 */
-
-	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 v3 6/7] 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 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 834b03d56713..4765ac1befba 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 v3 7/7] 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: reset the root port, 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.

While enumeration succeeds without resetting the root port, performing
reads and writes to an NVMe endpoint after a link down/link up cycle
results in failures on some devices. Address this by implementing
pci_host_bridge::reset_root_port() for the RZ/G3S PCIe driver.

The implementation of 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.

After the controller is reset, all previously masked interrupts are
restored.

Since rzg3s_pcie_host_stop() or rzg3s_pcie_host_start() can fail
during a root port reset, introduce struct rzg3s_pcie_host::state to
track the host controller state. The interrupt handlers and register
access paths consult this state to avoid accessing the controller after a
failed reset. This was implemented to be able to re-use the
rzg3s_pcie_host_stop()/rzg3s_pcie_host_start() as is (since they call
functions which can sleep).

The introduced states are START, STOP, PROCESS, and PORT_RESET. The initial
state is STOP. After the controller is initialized, the state is switched
to START. Any API exposed through struct pci_ops switches the controller
to the PROCESS state, as do the interrupt handlers.

rzg3s_pcie_host_reset_root_port() switches the state to PORT_RESET to
prevent any controller access while the Root Port reset is in progress.

A controller left in a broken state (STOP) after a failed root port reset
can recover after a system suspend/resume cycle, since
rzg3s_pcie_host_start() is invoked again during resume.

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 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 | 375 +++++++++++++++++++++--
 1 file changed, 356 insertions(+), 19 deletions(-)

diff --git a/drivers/pci/controller/pcie-rzg3s-host.c b/drivers/pci/controller/pcie-rzg3s-host.c
index 4765ac1befba..b3531468ef38 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)
@@ -310,6 +311,24 @@ struct rzg3s_pcie_port {
 	u32 device_id;
 };
 
+/**
+ * enum rzg3s_pcie_host_state - RZ/G3S PCIe Host state
+ * @RZG3S_PCIE_HOST_STATE_STOP: Host is stopped (initial state, reached from
+ *                              PORT_RESET, START)
+ * @RZG3S_PCIE_HOST_STATE_START: Host is started (reached from STOP, PROCESS,
+ *                               PORT_RESET)
+ * @RZG3S_PCIE_HOST_STATE_PROCESS: Host is started and is processing requests
+ *                                 (reached from START)
+ * @RZG3S_PCIE_HOST_STATE_PORT_RESET: Host root port is resetting (reached from
+ *                                    START)
+ */
+enum rzg3s_pcie_host_state {
+	RZG3S_PCIE_HOST_STATE_STOP,
+	RZG3S_PCIE_HOST_STATE_START,
+	RZG3S_PCIE_HOST_STATE_PORT_RESET,
+	RZG3S_PCIE_HOST_STATE_PROCESS,
+};
+
 /**
  * struct rzg3s_pcie_host - RZ/G3S PCIe data structure
  * @axi: base address for AXI registers
@@ -323,6 +342,8 @@ struct rzg3s_pcie_port {
  * @msi: MSI data structure
  * @port: PCIe Root Port
  * @hw_lock: lock for access to the HW resources
+ * @state: PCIe controller state
+ * @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 +361,8 @@ struct rzg3s_pcie_host {
 	struct rzg3s_pcie_msi msi;
 	struct rzg3s_pcie_port port;
 	raw_spinlock_t hw_lock;
+	atomic_t state;
+	int event_irq;
 	int intx_irqs[PCI_NUM_INTX];
 	int max_link_speed;
 	enum rzg3s_pcie_controller_id controller_id;
@@ -348,6 +371,61 @@ struct rzg3s_pcie_host {
 
 #define rzg3s_msi_to_host(_msi)	container_of(_msi, struct rzg3s_pcie_host, msi)
 
+/**
+ * struct rzg3s_pcie_host_atomic_state - RZ/G3S PCIe state data structure
+ * @state: Atomic state variable to operate on. Should point to
+ *         struct rzg3s_pcie_host::state.
+ * @expect: Expected host state. The host state is not changed if the
+ *          current host state differs from the expected state.
+ * @saved: Saved host state. When the host state is changed, the previous
+ *         state is saved in this variable. This is necessary to restore
+ *         the previous state after the protected section is executed.
+ *
+ * This structure is necessary for state setting and restoration using
+ * CLASS() constructs.
+ */
+struct rzg3s_pcie_host_atomic_state {
+	atomic_t *state;
+	enum rzg3s_pcie_host_state expect;
+	enum rzg3s_pcie_host_state saved;
+};
+
+static struct rzg3s_pcie_host_atomic_state
+rzg3s_pcie_host_atomic_state_save(atomic_t *state, int expect, int newval)
+{
+	return (struct rzg3s_pcie_host_atomic_state){
+		.state = state,
+		.expect = expect,
+		.saved = atomic_cmpxchg(state, expect, newval),
+	};
+}
+
+static void
+rzg3s_pcie_host_atomic_state_restore(struct rzg3s_pcie_host_atomic_state state)
+{
+	if (state.saved == state.expect)
+		atomic_xchg(state.state, state.expect);
+}
+
+DEFINE_CLASS(rzg3s_pcie_host_state_lock,
+	     struct rzg3s_pcie_host_atomic_state,
+	     rzg3s_pcie_host_atomic_state_restore(_T),
+	     rzg3s_pcie_host_atomic_state_save(state, expect, newval),
+	     atomic_t *state, int expect, int newval)
+
+/* Use it to change the state w/o the need to restore it on function exit. */
+#define RZG3S_PCIE_HOST_STATE_CHANGE(_state, _from, _to) \
+	CLASS(rzg3s_pcie_host_state_lock, _lock) \
+	      (_state, RZG3S_PCIE_HOST_STATE_##_from, \
+		       RZG3S_PCIE_HOST_STATE_##_to) \
+
+/*
+ * Use it to check if the state change failed. _from is the initial state.
+ * Use it in conjunction with RZG3S_PCIE_HOST_STATE_CHANGE().
+ */
+#define RZG3S_PCIE_HOST_STATE_CHANGE_FAILED(_from) \
+	(_lock.saved != RZG3S_PCIE_HOST_STATE_##_from)
+
 /*
  * RZ/V2H(P) supports a total of 4 lanes shared across two controllers.
  * rzv2h_lane_lock serialises both the counter update and the SYSC
@@ -459,6 +537,10 @@ static int rzg3s_pcie_child_read(struct pci_bus *bus, unsigned int devfn,
 	struct rzg3s_pcie_host *host = bus->sysdata;
 	int ret;
 
+	RZG3S_PCIE_HOST_STATE_CHANGE(&host->state, START, PROCESS);
+	if (RZG3S_PCIE_HOST_STATE_CHANGE_FAILED(START))
+		return PCIBIOS_SET_FAILED;
+
 	ret = rzg3s_pcie_child_read_conf(host, bus, devfn, where, val);
 	if (ret != PCIBIOS_SUCCESSFUL)
 		return ret;
@@ -504,6 +586,10 @@ static int rzg3s_pcie_child_write(struct pci_bus *bus, unsigned int devfn,
 	u32 data, shift;
 	int ret;
 
+	RZG3S_PCIE_HOST_STATE_CHANGE(&host->state, START, PROCESS);
+	if (RZG3S_PCIE_HOST_STATE_CHANGE_FAILED(START))
+		return PCIBIOS_SET_FAILED;
+
 	if (size == 4)
 		return rzg3s_pcie_child_write_conf(host, bus, devfn, where, val);
 
@@ -559,9 +645,35 @@ static void __iomem *rzg3s_pcie_root_map_bus(struct pci_bus *bus,
 	return host->pcie + where;
 }
 
+/* Serialization is provided by 'pci_lock' in drivers/pci/access.c */
+static int rzg3s_pcie_root_write(struct pci_bus *bus, unsigned int devfn,
+				 int where, int size, u32 val)
+{
+	struct rzg3s_pcie_host *host = bus->sysdata;
+
+	RZG3S_PCIE_HOST_STATE_CHANGE(&host->state, START, PROCESS);
+	if (RZG3S_PCIE_HOST_STATE_CHANGE_FAILED(START))
+		return PCIBIOS_SET_FAILED;
+
+	return pci_generic_config_write(bus, devfn, where, size, val);
+}
+
+/* Serialization is provided by 'pci_lock' in drivers/pci/access.c */
+static int rzg3s_pcie_root_read(struct pci_bus *bus, unsigned int devfn,
+				int where, int size, u32 *val)
+{
+	struct rzg3s_pcie_host *host = bus->sysdata;
+
+	RZG3S_PCIE_HOST_STATE_CHANGE(&host->state, START, PROCESS);
+	if (RZG3S_PCIE_HOST_STATE_CHANGE_FAILED(START))
+		return PCIBIOS_SET_FAILED;
+
+	return pci_generic_config_read(bus, devfn, where, size, val);
+}
+
 static struct pci_ops rzg3s_pcie_root_ops = {
-	.read		= pci_generic_config_read,
-	.write		= pci_generic_config_write,
+	.read		= rzg3s_pcie_root_read,
+	.write		= rzg3s_pcie_root_write,
 	.map_bus	= rzg3s_pcie_root_map_bus,
 };
 
@@ -627,6 +739,10 @@ static void rzg3s_pcie_msi_irq_ack(struct irq_data *d)
 	u8 reg_bit = d->hwirq % RZG3S_PCI_MSI_INT_PER_REG;
 	u8 reg_id = d->hwirq / RZG3S_PCI_MSI_INT_PER_REG;
 
+	RZG3S_PCIE_HOST_STATE_CHANGE(&host->state, START, PROCESS);
+	if (RZG3S_PCIE_HOST_STATE_CHANGE_FAILED(START))
+		return;
+
 	writel_relaxed(BIT(reg_bit), host->axi + RZG3S_PCI_MSIRS(reg_id));
 }
 
@@ -639,6 +755,10 @@ static void rzg3s_pcie_msi_irq_mask(struct irq_data *d)
 
 	guard(raw_spinlock_irqsave)(&host->hw_lock);
 
+	RZG3S_PCIE_HOST_STATE_CHANGE(&host->state, START, PROCESS);
+	if (RZG3S_PCIE_HOST_STATE_CHANGE_FAILED(START))
+		return;
+
 	rzg3s_pcie_update_bits(host->axi, RZG3S_PCI_MSIRM(reg_id), BIT(reg_bit),
 			       BIT(reg_bit));
 }
@@ -652,6 +772,10 @@ static void rzg3s_pcie_msi_irq_unmask(struct irq_data *d)
 
 	guard(raw_spinlock_irqsave)(&host->hw_lock);
 
+	RZG3S_PCIE_HOST_STATE_CHANGE(&host->state, START, PROCESS);
+	if (RZG3S_PCIE_HOST_STATE_CHANGE_FAILED(START))
+		return;
+
 	rzg3s_pcie_update_bits(host->axi, RZG3S_PCI_MSIRM(reg_id), BIT(reg_bit),
 			       0);
 }
@@ -663,6 +787,10 @@ static void rzg3s_pcie_irq_compose_msi_msg(struct irq_data *data,
 	struct rzg3s_pcie_host *host = rzg3s_msi_to_host(msi);
 	u32 lo, hi;
 
+	RZG3S_PCIE_HOST_STATE_CHANGE(&host->state, START, PROCESS);
+	if (RZG3S_PCIE_HOST_STATE_CHANGE_FAILED(START))
+		return;
+
 	/*
 	 * Enable and msg data enable bits are part of the address lo. Drop
 	 * them along with the unused bit.
@@ -958,6 +1086,10 @@ static void rzg3s_pcie_intx_irq_ack(struct irq_data *d)
 {
 	struct rzg3s_pcie_host *host = irq_data_get_irq_chip_data(d);
 
+	RZG3S_PCIE_HOST_STATE_CHANGE(&host->state, START, PROCESS);
+	if (RZG3S_PCIE_HOST_STATE_CHANGE_FAILED(START))
+		return;
+
 	rzg3s_pcie_update_bits(host->axi, RZG3S_PCI_PINTRCVIS,
 			       RZG3S_PCI_PINTRCVIS_INTX(d->hwirq),
 			       RZG3S_PCI_PINTRCVIS_INTX(d->hwirq));
@@ -969,6 +1101,10 @@ static void rzg3s_pcie_intx_irq_mask(struct irq_data *d)
 
 	guard(raw_spinlock_irqsave)(&host->hw_lock);
 
+	RZG3S_PCIE_HOST_STATE_CHANGE(&host->state, START, PROCESS);
+	if (RZG3S_PCIE_HOST_STATE_CHANGE_FAILED(START))
+		return;
+
 	rzg3s_pcie_update_bits(host->axi, RZG3S_PCI_PINTRCVIE,
 			       RZG3S_PCI_PINTRCVIE_INTX(d->hwirq), 0);
 }
@@ -979,6 +1115,10 @@ static void rzg3s_pcie_intx_irq_unmask(struct irq_data *d)
 
 	guard(raw_spinlock_irqsave)(&host->hw_lock);
 
+	RZG3S_PCIE_HOST_STATE_CHANGE(&host->state, START, PROCESS);
+	if (RZG3S_PCIE_HOST_STATE_CHANGE_FAILED(START))
+		return;
+
 	rzg3s_pcie_update_bits(host->axi, RZG3S_PCI_PINTRCVIE,
 			       RZG3S_PCI_PINTRCVIE_INTX(d->hwirq),
 			       RZG3S_PCI_PINTRCVIE_INTX(d->hwirq));
@@ -1095,6 +1235,94 @@ 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");
+
+		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)
+			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 +1334,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 +1391,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 +1917,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 +1966,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);
@@ -1748,6 +1997,9 @@ static int rzg3s_pcie_host_stop(struct rzg3s_pcie_host *host)
 	struct rzg3s_sysc *sysc = host->sysc;
 	int ret;
 
+	if (atomic_read(&host->state) == RZG3S_PCIE_HOST_STATE_STOP)
+		return 0;
+
 	clk_disable_unprepare(port->refclk);
 
 	/* SoC-specific de-initialization */
@@ -1783,6 +2035,8 @@ static int rzg3s_pcie_host_stop(struct rzg3s_pcie_host *host)
 	if (ret)
 		goto power_resets_restore;
 
+	atomic_set(&host->state, RZG3S_PCIE_HOST_STATE_STOP);
+
 	return 0;
 
 	/* Restore the previous state if any error happens */
@@ -1798,12 +2052,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_state)
 {
 	const struct rzg3s_pcie_soc_data *data = host->data;
 	struct rzg3s_sysc *sysc = host->sysc;
 	int ret;
 
+	if (atomic_read(&host->state) == RZG3S_PCIE_HOST_STATE_START)
+		return 0;
+
 	ret = rzg3s_sysc_config_func(sysc, RZG3S_SYSC_FUNC_ID_MODE, 1);
 	if (ret)
 		return ret;
@@ -1850,6 +2107,9 @@ static int rzg3s_pcie_host_start(struct rzg3s_pcie_host *host)
 	if (ret)
 		goto assert_power_resets;
 
+	if (set_state)
+		atomic_set(&host->state, RZG3S_PCIE_HOST_STATE_START);
+
 	return 0;
 
 	/*
@@ -1863,6 +2123,64 @@ 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);
+	enum rzg3s_pcie_host_state state;
+	u32 irqs;
+	int ret;
+
+	state = atomic_cmpxchg(&host->state, RZG3S_PCIE_HOST_STATE_START,
+			       RZG3S_PCIE_HOST_STATE_PORT_RESET);
+	if (state != RZG3S_PCIE_HOST_STATE_START)
+		return -EBUSY;
+
+	/* 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);
+
+	atomic_xchg(&host->state, state);
+
+	return ret;
+}
+
 static int rzg3s_pcie_get_controller_id(struct rzg3s_pcie_host *host)
 {
 	struct device_node *np = host->dev->of_node;
@@ -2025,13 +2343,24 @@ static int rzg3s_pcie_probe(struct platform_device *pdev)
 	if (ret)
 		goto power_resets_assert;
 
+	atomic_set(&host->state, RZG3S_PCIE_HOST_STATE_START);
+
 	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;
 
+	/*
+	 * 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:
@@ -2067,8 +2396,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, true);
+	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 v3 7/7] PCI: rzg3s-host: Re-enumerate the bus on PCIe link-state changes

[Manivannan Sadhasivam]

On Fri, Aug 14, 2026 at 05:13:12PM +0300, Claudiu Beznea 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: reset the root port, 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.
> 
> While enumeration succeeds without resetting the root port, performing
> reads and writes to an NVMe endpoint after a link down/link up cycle
> results in failures on some devices. Address this by implementing
> pci_host_bridge::reset_root_port() for the RZ/G3S PCIe driver.
> 
> The implementation of 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.
> 
> After the controller is reset, all previously masked interrupts are
> restored.
> 
> Since rzg3s_pcie_host_stop() or rzg3s_pcie_host_start() can fail
> during a root port reset, introduce struct rzg3s_pcie_host::state to
> track the host controller state. The interrupt handlers and register
> access paths consult this state to avoid accessing the controller after a
> failed reset. This was implemented to be able to re-use the
> rzg3s_pcie_host_stop()/rzg3s_pcie_host_start() as is (since they call
> functions which can sleep).
> 
> The introduced states are START, STOP, PROCESS, and PORT_RESET. The initial
> state is STOP. After the controller is initialized, the state is switched
> to START. Any API exposed through struct pci_ops switches the controller
> to the PROCESS state, as do the interrupt handlers.
> 
> rzg3s_pcie_host_reset_root_port() switches the state to PORT_RESET to
> prevent any controller access while the Root Port reset is in progress.
> 
> A controller left in a broken state (STOP) after a failed root port reset
> can recover after a system suspend/resume cycle, since
> rzg3s_pcie_host_start() is invoked again during resume.
> 
> 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.
> 

Please split the DL_UpDown addition and reset_root_port() into separate patches.

> 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 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 | 375 +++++++++++++++++++++--
>  1 file changed, 356 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/pci/controller/pcie-rzg3s-host.c b/drivers/pci/controller/pcie-rzg3s-host.c
> index 4765ac1befba..b3531468ef38 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)
> @@ -310,6 +311,24 @@ struct rzg3s_pcie_port {
>  	u32 device_id;
>  };
>  
> +/**
> + * enum rzg3s_pcie_host_state - RZ/G3S PCIe Host state
> + * @RZG3S_PCIE_HOST_STATE_STOP: Host is stopped (initial state, reached from
> + *                              PORT_RESET, START)
> + * @RZG3S_PCIE_HOST_STATE_START: Host is started (reached from STOP, PROCESS,
> + *                               PORT_RESET)
> + * @RZG3S_PCIE_HOST_STATE_PROCESS: Host is started and is processing requests
> + *                                 (reached from START)
> + * @RZG3S_PCIE_HOST_STATE_PORT_RESET: Host root port is resetting (reached from
> + *                                    START)
> + */
> +enum rzg3s_pcie_host_state {
> +	RZG3S_PCIE_HOST_STATE_STOP,
> +	RZG3S_PCIE_HOST_STATE_START,
> +	RZG3S_PCIE_HOST_STATE_PORT_RESET,
> +	RZG3S_PCIE_HOST_STATE_PROCESS,
> +};
> +
>  /**
>   * struct rzg3s_pcie_host - RZ/G3S PCIe data structure
>   * @axi: base address for AXI registers
> @@ -323,6 +342,8 @@ struct rzg3s_pcie_port {
>   * @msi: MSI data structure
>   * @port: PCIe Root Port
>   * @hw_lock: lock for access to the HW resources
> + * @state: PCIe controller state
> + * @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 +361,8 @@ struct rzg3s_pcie_host {
>  	struct rzg3s_pcie_msi msi;
>  	struct rzg3s_pcie_port port;
>  	raw_spinlock_t hw_lock;
> +	atomic_t state;
> +	int event_irq;
>  	int intx_irqs[PCI_NUM_INTX];
>  	int max_link_speed;
>  	enum rzg3s_pcie_controller_id controller_id;
> @@ -348,6 +371,61 @@ struct rzg3s_pcie_host {
>  
>  #define rzg3s_msi_to_host(_msi)	container_of(_msi, struct rzg3s_pcie_host, msi)
>  
> +/**
> + * struct rzg3s_pcie_host_atomic_state - RZ/G3S PCIe state data structure
> + * @state: Atomic state variable to operate on. Should point to
> + *         struct rzg3s_pcie_host::state.
> + * @expect: Expected host state. The host state is not changed if the
> + *          current host state differs from the expected state.
> + * @saved: Saved host state. When the host state is changed, the previous
> + *         state is saved in this variable. This is necessary to restore
> + *         the previous state after the protected section is executed.
> + *
> + * This structure is necessary for state setting and restoration using
> + * CLASS() constructs.
> + */
> +struct rzg3s_pcie_host_atomic_state {
> +	atomic_t *state;
> +	enum rzg3s_pcie_host_state expect;
> +	enum rzg3s_pcie_host_state saved;
> +};
> +
> +static struct rzg3s_pcie_host_atomic_state
> +rzg3s_pcie_host_atomic_state_save(atomic_t *state, int expect, int newval)
> +{
> +	return (struct rzg3s_pcie_host_atomic_state){
> +		.state = state,
> +		.expect = expect,
> +		.saved = atomic_cmpxchg(state, expect, newval),
> +	};
> +}
> +
> +static void
> +rzg3s_pcie_host_atomic_state_restore(struct rzg3s_pcie_host_atomic_state state)
> +{
> +	if (state.saved == state.expect)
> +		atomic_xchg(state.state, state.expect);
> +}
> +
> +DEFINE_CLASS(rzg3s_pcie_host_state_lock,
> +	     struct rzg3s_pcie_host_atomic_state,
> +	     rzg3s_pcie_host_atomic_state_restore(_T),
> +	     rzg3s_pcie_host_atomic_state_save(state, expect, newval),
> +	     atomic_t *state, int expect, int newval)
> +
> +/* Use it to change the state w/o the need to restore it on function exit. */
> +#define RZG3S_PCIE_HOST_STATE_CHANGE(_state, _from, _to) \
> +	CLASS(rzg3s_pcie_host_state_lock, _lock) \
> +	      (_state, RZG3S_PCIE_HOST_STATE_##_from, \
> +		       RZG3S_PCIE_HOST_STATE_##_to) \
> +
> +/*
> + * Use it to check if the state change failed. _from is the initial state.
> + * Use it in conjunction with RZG3S_PCIE_HOST_STATE_CHANGE().
> + */
> +#define RZG3S_PCIE_HOST_STATE_CHANGE_FAILED(_from) \
> +	(_lock.saved != RZG3S_PCIE_HOST_STATE_##_from)
> +

I don't really see a need for all these state management. None of the other
controller drivers are doing and the serialization provided by the PCI core
using pci_lock should be enough. If you find any issue or race, please share it
here. But I really believe that we can fix it without all these complicated
state management.

- Mani

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

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

[claudiu beznea]

Hi, Mani,

On 9/10/26 17:38, Manivannan Sadhasivam wrote:
> On Fri, Aug 14, 2026 at 05:13:12PM +0300, Claudiu Beznea 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: reset the root port, 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.
>>
>> While enumeration succeeds without resetting the root port, performing
>> reads and writes to an NVMe endpoint after a link down/link up cycle
>> results in failures on some devices. Address this by implementing
>> pci_host_bridge::reset_root_port() for the RZ/G3S PCIe driver.
>>
>> The implementation of 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.
>>
>> After the controller is reset, all previously masked interrupts are
>> restored.
>>
>> Since rzg3s_pcie_host_stop() or rzg3s_pcie_host_start() can fail
>> during a root port reset, introduce struct rzg3s_pcie_host::state to
>> track the host controller state. The interrupt handlers and register
>> access paths consult this state to avoid accessing the controller after a
>> failed reset. This was implemented to be able to re-use the
>> rzg3s_pcie_host_stop()/rzg3s_pcie_host_start() as is (since they call
>> functions which can sleep).
>>
>> The introduced states are START, STOP, PROCESS, and PORT_RESET. The initial
>> state is STOP. After the controller is initialized, the state is switched
>> to START. Any API exposed through struct pci_ops switches the controller
>> to the PROCESS state, as do the interrupt handlers.
>>
>> rzg3s_pcie_host_reset_root_port() switches the state to PORT_RESET to
>> prevent any controller access while the Root Port reset is in progress.
>>
>> A controller left in a broken state (STOP) after a failed root port reset
>> can recover after a system suspend/resume cycle, since
>> rzg3s_pcie_host_start() is invoked again during resume.
>>
>> 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.
>>
> 
> Please split the DL_UpDown addition and reset_root_port() into separate patches.

If splitting, it would mean the DL_UpDown patch will be broken (at least for 
RZ/G3S SoC). Would that be OK with you?

> 
>> 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 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 | 375 +++++++++++++++++++++--
>>   1 file changed, 356 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/pci/controller/pcie-rzg3s-host.c b/drivers/pci/controller/pcie-rzg3s-host.c
>> index 4765ac1befba..b3531468ef38 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)
>> @@ -310,6 +311,24 @@ struct rzg3s_pcie_port {
>>   	u32 device_id;
>>   };
>>   
>> +/**
>> + * enum rzg3s_pcie_host_state - RZ/G3S PCIe Host state
>> + * @RZG3S_PCIE_HOST_STATE_STOP: Host is stopped (initial state, reached from
>> + *                              PORT_RESET, START)
>> + * @RZG3S_PCIE_HOST_STATE_START: Host is started (reached from STOP, PROCESS,
>> + *                               PORT_RESET)
>> + * @RZG3S_PCIE_HOST_STATE_PROCESS: Host is started and is processing requests
>> + *                                 (reached from START)
>> + * @RZG3S_PCIE_HOST_STATE_PORT_RESET: Host root port is resetting (reached from
>> + *                                    START)
>> + */
>> +enum rzg3s_pcie_host_state {
>> +	RZG3S_PCIE_HOST_STATE_STOP,
>> +	RZG3S_PCIE_HOST_STATE_START,
>> +	RZG3S_PCIE_HOST_STATE_PORT_RESET,
>> +	RZG3S_PCIE_HOST_STATE_PROCESS,
>> +};
>> +
>>   /**
>>    * struct rzg3s_pcie_host - RZ/G3S PCIe data structure
>>    * @axi: base address for AXI registers
>> @@ -323,6 +342,8 @@ struct rzg3s_pcie_port {
>>    * @msi: MSI data structure
>>    * @port: PCIe Root Port
>>    * @hw_lock: lock for access to the HW resources
>> + * @state: PCIe controller state
>> + * @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 +361,8 @@ struct rzg3s_pcie_host {
>>   	struct rzg3s_pcie_msi msi;
>>   	struct rzg3s_pcie_port port;
>>   	raw_spinlock_t hw_lock;
>> +	atomic_t state;
>> +	int event_irq;
>>   	int intx_irqs[PCI_NUM_INTX];
>>   	int max_link_speed;
>>   	enum rzg3s_pcie_controller_id controller_id;
>> @@ -348,6 +371,61 @@ struct rzg3s_pcie_host {
>>   
>>   #define rzg3s_msi_to_host(_msi)	container_of(_msi, struct rzg3s_pcie_host, msi)
>>   
>> +/**
>> + * struct rzg3s_pcie_host_atomic_state - RZ/G3S PCIe state data structure
>> + * @state: Atomic state variable to operate on. Should point to
>> + *         struct rzg3s_pcie_host::state.
>> + * @expect: Expected host state. The host state is not changed if the
>> + *          current host state differs from the expected state.
>> + * @saved: Saved host state. When the host state is changed, the previous
>> + *         state is saved in this variable. This is necessary to restore
>> + *         the previous state after the protected section is executed.
>> + *
>> + * This structure is necessary for state setting and restoration using
>> + * CLASS() constructs.
>> + */
>> +struct rzg3s_pcie_host_atomic_state {
>> +	atomic_t *state;
>> +	enum rzg3s_pcie_host_state expect;
>> +	enum rzg3s_pcie_host_state saved;
>> +};
>> +
>> +static struct rzg3s_pcie_host_atomic_state
>> +rzg3s_pcie_host_atomic_state_save(atomic_t *state, int expect, int newval)
>> +{
>> +	return (struct rzg3s_pcie_host_atomic_state){
>> +		.state = state,
>> +		.expect = expect,
>> +		.saved = atomic_cmpxchg(state, expect, newval),
>> +	};
>> +}
>> +
>> +static void
>> +rzg3s_pcie_host_atomic_state_restore(struct rzg3s_pcie_host_atomic_state state)
>> +{
>> +	if (state.saved == state.expect)
>> +		atomic_xchg(state.state, state.expect);
>> +}
>> +
>> +DEFINE_CLASS(rzg3s_pcie_host_state_lock,
>> +	     struct rzg3s_pcie_host_atomic_state,
>> +	     rzg3s_pcie_host_atomic_state_restore(_T),
>> +	     rzg3s_pcie_host_atomic_state_save(state, expect, newval),
>> +	     atomic_t *state, int expect, int newval)
>> +
>> +/* Use it to change the state w/o the need to restore it on function exit. */
>> +#define RZG3S_PCIE_HOST_STATE_CHANGE(_state, _from, _to) \
>> +	CLASS(rzg3s_pcie_host_state_lock, _lock) \
>> +	      (_state, RZG3S_PCIE_HOST_STATE_##_from, \
>> +		       RZG3S_PCIE_HOST_STATE_##_to) \
>> +
>> +/*
>> + * Use it to check if the state change failed. _from is the initial state.
>> + * Use it in conjunction with RZG3S_PCIE_HOST_STATE_CHANGE().
>> + */
>> +#define RZG3S_PCIE_HOST_STATE_CHANGE_FAILED(_from) \
>> +	(_lock.saved != RZG3S_PCIE_HOST_STATE_##_from)
>> +
> 
> I don't really see a need for all these state management. None of the other
> controller drivers are doing and the serialization provided by the PCI core
> using pci_lock should be enough. If you find any issue or race, please share it
> here.

The problems I see are the following:

1/ In theory, the rzg3s_pcie_host_start() in .reset_root_port() can fail. If
    that happens the controller is left with the clocks and resets disabled.
    Accessing  it later in this state may lead to sync aborts, and the system
    being fully blocked. This is how the HW behaves. Re-enabling clocks and
    resets in this situation may lead to unbalanced  reference counters for them.

2/ In theory, struct pci_ops::{read, write} ops could be called while the reset
    is in progress, if I'm not wrong. That can be avoided by using a common spin
    lock but we can't use it in .reset_root_port() since this calls APIs
    that could sleep (e.g. clk_prepare_enable()/clk_disable_unprepare())

3/ Since at the moment the .suspend() is called the IRQs are enabled, the link
    event and the .suspend() can race. This could also be avoided by using
    a spin lock but, again, there are APIs on .reset_root_port() that can sleep.

4/ To keep sane the reference counted resources, handled in
    rzg3s_pcie_host_stop()/rzg3s_pcie_host_start(), the state track would be
    needed, for .reset_root_port() failure case and racing, since these functions
    are also called in suspend/resume

I think I should drop the state check from the IRQ ack APIs.

Please let me know if you have any suggestions.

Thank you,
Claudiu

> But I really believe that we can fix it without all these complicated
> state management.
> 
> - Mani
>