From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

As per PCIe spec r6.0, sec 6.6.1, PERST# is an auxiliary signal provided by
the system to a component as a Fundamental Reset. This signal if available,
should conform to the rules defined by the electromechanical form factor
specifications like PCIe CEM spec r4.0, sec 2.2.

Since pwrctrl driver is meant to control the power supplies, it should also
control the PERST# signal if available. But traditionally, the host bridge
(controller) drivers are the ones parsing and controlling the PERST#
signal. They also sometimes need to assert PERST# during their own hardware
initialization. So it is not possible to move the PERST# control away from
the controller drivers and it must be shared logically.

Hence, add a new callback 'pci_host_bridge::toggle_perst', that allows the
pwrctrl core to toggle PERST# with the help of the controller drivers. But
care must be taken care by the controller drivers to not deassert the
PERST# signal if this callback is populated.

This callback if available, will be called by the pwrctrl core during the
device power up and power down scenarios. Controller drivers should
identify the device using the 'struct device_node' passed during the
callback and toggle PERST# accordingly.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---
 drivers/pci/pwrctrl/core.c | 27 +++++++++++++++++++++++++++
 include/linux/pci.h        |  3 +++
 2 files changed, 30 insertions(+)

diff --git a/drivers/pci/pwrctrl/core.c b/drivers/pci/pwrctrl/core.c
index 6bdbfed584d6d79ce28ba9e384a596b065ca69a4..8a26f432436d064acb7ebbbc9ce8fc339909fbe9 100644
--- a/drivers/pci/pwrctrl/core.c
+++ b/drivers/pci/pwrctrl/core.c
@@ -6,6 +6,7 @@
 #include <linux/device.h>
 #include <linux/export.h>
 #include <linux/kernel.h>
+#include <linux/of.h>
 #include <linux/pci.h>
 #include <linux/pci-pwrctrl.h>
 #include <linux/property.h>
@@ -61,6 +62,28 @@ void pci_pwrctrl_init(struct pci_pwrctrl *pwrctrl, struct device *dev)
 }
 EXPORT_SYMBOL_GPL(pci_pwrctrl_init);
 
+static void pci_pwrctrl_perst_deassert(struct pci_pwrctrl *pwrctrl)
+{
+	struct pci_host_bridge *host_bridge = to_pci_host_bridge(pwrctrl->dev->parent);
+	struct device_node *np = dev_of_node(pwrctrl->dev);
+
+	if (!host_bridge->toggle_perst)
+		return;
+
+	host_bridge->toggle_perst(host_bridge, np, false);
+}
+
+static void pci_pwrctrl_perst_assert(struct pci_pwrctrl *pwrctrl)
+{
+	struct pci_host_bridge *host_bridge = to_pci_host_bridge(pwrctrl->dev->parent);
+	struct device_node *np = dev_of_node(pwrctrl->dev);
+
+	if (!host_bridge->toggle_perst)
+		return;
+
+	host_bridge->toggle_perst(host_bridge, np, true);
+}
+
 /**
  * pci_pwrctrl_device_set_ready() - Notify the pwrctrl subsystem that the PCI
  * device is powered-up and ready to be detected.
@@ -82,6 +105,8 @@ int pci_pwrctrl_device_set_ready(struct pci_pwrctrl *pwrctrl)
 	if (!pwrctrl->dev)
 		return -ENODEV;
 
+	pci_pwrctrl_perst_deassert(pwrctrl);
+
 	pwrctrl->nb.notifier_call = pci_pwrctrl_notify;
 	ret = bus_register_notifier(&pci_bus_type, &pwrctrl->nb);
 	if (ret)
@@ -103,6 +128,8 @@ void pci_pwrctrl_device_unset_ready(struct pci_pwrctrl *pwrctrl)
 {
 	cancel_work_sync(&pwrctrl->work);
 
+	pci_pwrctrl_perst_assert(pwrctrl);
+
 	/*
 	 * We don't have to delete the link here. Typically, this function
 	 * is only called when the power control device is being detached. If
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 59876de13860dbe50ee6c207cd57e54f51a11079..3da62e37dba5993b52413f16ec401ba3fb970a55 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -605,6 +605,9 @@ struct pci_host_bridge {
 	void (*release_fn)(struct pci_host_bridge *);
 	int (*enable_device)(struct pci_host_bridge *bridge, struct pci_dev *dev);
 	void (*disable_device)(struct pci_host_bridge *bridge, struct pci_dev *dev);
+#if IS_ENABLED(CONFIG_PCI_PWRCTRL)
+	void (*toggle_perst)(struct pci_host_bridge *bridge, struct device_node *np, bool assert);
+#endif
 	void		*release_data;
 	unsigned int	ignore_reset_delay:1;	/* For entire hierarchy */
 	unsigned int	no_ext_tags:1;		/* No Extended Tags */

-- 
2.45.2

On Wed, Sep 03, 2025 at 12:43:25PM +0530, Manivannan Sadhasivam via B4 Relay wrote:
> As per PCIe spec r6.0, sec 6.6.1, PERST# is an auxiliary signal provided by
> the system to a component as a Fundamental Reset. This signal if available,
> should conform to the rules defined by the electromechanical form factor
> specifications like PCIe CEM spec r4.0, sec 2.2.
> 
> Since pwrctrl driver is meant to control the power supplies, it should also
> control the PERST# signal if available.

Why?  Probably obvious to hardware folks, but a sentence about the
necessary connection between power supply and reset would help me.

> But traditionally, the host bridge
> (controller) drivers are the ones parsing and controlling the PERST#
> signal. They also sometimes need to assert PERST# during their own hardware
> initialization. So it is not possible to move the PERST# control away from
> the controller drivers and it must be shared logically.
> 
> Hence, add a new callback 'pci_host_bridge::toggle_perst', that allows the
> pwrctrl core to toggle PERST# with the help of the controller drivers. But
> care must be taken care by the controller drivers to not deassert the
> PERST# signal if this callback is populated.
> 
> This callback if available, will be called by the pwrctrl core during the
> device power up and power down scenarios. Controller drivers should
> identify the device using the 'struct device_node' passed during the
> callback and toggle PERST# accordingly.

"Toggle" isn't my favorite description because it implies that you
don't need to supply the new state; you're just switching from the
current state to the other state, and you wouldn't need to pass a
state.  Maybe something like "set_perst" or "set_perst_state" like we
do for set_cpu_online(), *_set_power_state(), etc?

> +static void pci_pwrctrl_perst_deassert(struct pci_pwrctrl *pwrctrl)
> +{
> +	struct pci_host_bridge *host_bridge = to_pci_host_bridge(pwrctrl->dev->parent);
> +	struct device_node *np = dev_of_node(pwrctrl->dev);
> +
> +	if (!host_bridge->toggle_perst)
> +		return;
> +
> +	host_bridge->toggle_perst(host_bridge, np, false);
> +}
> +
> +static void pci_pwrctrl_perst_assert(struct pci_pwrctrl *pwrctrl)
> +{
> +	struct pci_host_bridge *host_bridge = to_pci_host_bridge(pwrctrl->dev->parent);
> +	struct device_node *np = dev_of_node(pwrctrl->dev);
> +
> +	if (!host_bridge->toggle_perst)
> +		return;
> +
> +	host_bridge->toggle_perst(host_bridge, np, true);
> +}

On Mon, Sep 08, 2025 at 02:35:29PM GMT, Bjorn Helgaas wrote:
> On Wed, Sep 03, 2025 at 12:43:25PM +0530, Manivannan Sadhasivam via B4 Relay wrote:
> > As per PCIe spec r6.0, sec 6.6.1, PERST# is an auxiliary signal provided by
> > the system to a component as a Fundamental Reset. This signal if available,
> > should conform to the rules defined by the electromechanical form factor
> > specifications like PCIe CEM spec r4.0, sec 2.2.
> > 
> > Since pwrctrl driver is meant to control the power supplies, it should also
> > control the PERST# signal if available.
> 
> Why?  Probably obvious to hardware folks, but a sentence about the
> necessary connection between power supply and reset would help me.
> 
> > But traditionally, the host bridge
> > (controller) drivers are the ones parsing and controlling the PERST#
> > signal. They also sometimes need to assert PERST# during their own hardware
> > initialization. So it is not possible to move the PERST# control away from
> > the controller drivers and it must be shared logically.
> > 
> > Hence, add a new callback 'pci_host_bridge::toggle_perst', that allows the
> > pwrctrl core to toggle PERST# with the help of the controller drivers. But
> > care must be taken care by the controller drivers to not deassert the
> > PERST# signal if this callback is populated.
> > 
> > This callback if available, will be called by the pwrctrl core during the
> > device power up and power down scenarios. Controller drivers should
> > identify the device using the 'struct device_node' passed during the
> > callback and toggle PERST# accordingly.
> 
> "Toggle" isn't my favorite description because it implies that you
> don't need to supply the new state; you're just switching from the
> current state to the other state, and you wouldn't need to pass a
> state.  Maybe something like "set_perst" or "set_perst_state" like we
> do for set_cpu_online(), *_set_power_state(), etc?
> 

Since the spec mentions the state change as 'assertion' and 'deassertion', how
about:

	 pci_host_bridge::perst_assert(struct pci_pwrctrl *pwrctrl, bool assert)

- Mani

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