linux-kernel - Re: [PATCH 3/6] PCI/pwrctrl: Add support for toggling PERST#

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAMRc=Me2P9r9w-UPtjMAEvuQ_oNtibzPBg6tE7s1wdKkLmQgcQ@mail.gmail.com>
Date: Wed, 27 Aug 2025 18:32:30 +0200
From: Bartosz Golaszewski <brgl@...ev.pl>
To: manivannan.sadhasivam@....qualcomm.com
Cc: Manivannan Sadhasivam <mani@...nel.org>, Lorenzo Pieralisi <lpieralisi@...nel.org>, 
	Krzysztof Wilczyński <kwilczynski@...nel.org>, 
	Rob Herring <robh@...nel.org>, Bjorn Helgaas <bhelgaas@...gle.com>, 
	Saravana Kannan <saravanak@...gle.com>, linux-pci@...r.kernel.org, 
	linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org, 
	devicetree@...r.kernel.org, 
	Krishna Chaitanya Chundru <krishna.chundru@....qualcomm.com>, Brian Norris <briannorris@...omium.org>
Subject: Re: [PATCH 3/6] PCI/pwrctrl: Add support for toggling PERST#

On Tue, Aug 19, 2025 at 9:15 AM Manivannan Sadhasivam via B4 Relay
<devnull+manivannan.sadhasivam.oss.qualcomm.com@...nel.org> wrote:
>
> From: Manivannan Sadhasivam <manivannan.sadhasivam@....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@....qualcomm.com>
> ---
>  drivers/pci/pwrctrl/core.c | 27 +++++++++++++++++++++++++++
>  include/linux/pci.h        |  1 +
>  2 files changed, 28 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..9eeee84d550bb9f15a90b5db9da03fccef8097ee 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -605,6 +605,7 @@ 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);
> +       void (*toggle_perst)(struct pci_host_bridge *bridge, struct device_node *np, bool assert);

Shouldn't this be wrapped in an #if IS_ENABLED(PCI_PWRCTL)?

Bart

>         void            *release_data;
>         unsigned int    ignore_reset_delay:1;   /* For entire hierarchy */
>         unsigned int    no_ext_tags:1;          /* No Extended Tags */
>
> --
> 2.45.2
>
>