linux-kernel - Re: [PATCH v2 3/5] 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: <20250908193529.GA1439341@bhelgaas>
Date: Mon, 8 Sep 2025 14:35:29 -0500
From: Bjorn Helgaas <helgaas@...nel.org>
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>,
	Bartosz Golaszewski <brgl@...ev.pl>,
	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 v2 3/5] PCI/pwrctrl: Add support for toggling PERST#

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);
> +}