linux-kernel - Re: [PATCH RFC 2/3] PCI/pwrctrl: Allow pwrctrl core to control PERST# GPIO if available

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: <uh7r37l7a2btd3p5dighewfmat2caewrlyf2lwjtslolbr5bov@jgstvnfhxur6>
Date: Thu, 24 Jul 2025 19:43:38 +0530
From: Manivannan Sadhasivam <mani@...nel.org>
To: Brian Norris <briannorris@...omium.org>
Cc: Bartosz Golaszewski <brgl@...ev.pl>, 
	Bjorn Helgaas <bhelgaas@...gle.com>, Jingoo Han <jingoohan1@...il.com>, 
	Lorenzo Pieralisi <lpieralisi@...nel.org>, Krzysztof Wilczyński <kwilczynski@...nel.org>, 
	Rob Herring <robh@...nel.org>, linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-arm-msm@...r.kernel.org, Krishna Chaitanya Chundru <krishna.chundru@....qualcomm.com>
Subject: Re: [PATCH RFC 2/3] PCI/pwrctrl: Allow pwrctrl core to control
 PERST# GPIO if available

On Sat, Jul 12, 2025 at 01:59:34PM GMT, Manivannan Sadhasivam wrote:
> On Fri, Jul 11, 2025 at 05:38:16PM GMT, Brian Norris wrote:
> > Sorry for so many individual reviews, but I've passed over this a few
> > times and had new questions/comments several times:
> > 
> 
> That's fine. I'm happy to answer as someone other than me is interested in
> pwrctrl :)
> 
> > On Mon, Jul 07, 2025 at 11:48:39PM +0530, Manivannan Sadhasivam wrote:
> > > PERST# is an (optional) auxiliary signal provided by the PCIe host to
> > > components for signalling 'Fundamental Reset' as per the PCIe spec r6.0,
> > > sec 6.6.1.
> > 
> > >  void pci_pwrctrl_init(struct pci_pwrctrl *pwrctrl, struct device *dev)
> > >  {
> > > +	struct pci_host_bridge *host_bridge = to_pci_host_bridge(dev->parent);
> > > +	int devfn;
> > > +
> > >  	pwrctrl->dev = dev;
> > >  	INIT_WORK(&pwrctrl->work, rescan_work_func);
> > > +
> > > +	if (!host_bridge->perst)
> > > +		return;
> > > +
> > > +	devfn = of_pci_get_devfn(dev_of_node(dev));
> > > +	if (devfn >= 0 && host_bridge->perst[PCI_SLOT(devfn)])
> > 
> > This seems to imply a 1:1 correlation between slots and pwrctrl devices,
> > almost as if you expect everyone is using drivers/pci/pwrctrl/slot.c.
> > But there is also endpoint-specific pwrctrl support, and there's quite
> > a bit of flexibility around what these hierarchies can look like.
> > 
> > How do you account for that?
> > 
> > For example, couldn't you have both a "port" and an "endpoint" pwrctrl? Would
> > they both grab the same PERST# GPIO here? And might that incur excessive
> > resets, possibly even clobbering each other?
> > 
> 
> If both port and endpoint nodes are present, then only one will contain
> 'reset-gpios'. Right now, the DT binding only supports PERST#, WAKE#, CLKREQ#
> properties in RP node, but that won't work if we have multiple lines per slot/
> controller. Ideally, we would want the properties to be present in endpoint node
> if available. But if we have only standard expansion slots, then it makes sense
> to define them in the port node. But doing so, we can only expect the slot to
> have only one instance of these properties as we cannot reliably map which
> property corresponds to the endpoint.
> 
> I've opened a dtschema issue for this:
> https://github.com/devicetree-org/dt-schema/issues/168
> 

I realized that there is no need to define these properties (PERST#, WAKE#,
CLKREQ#) in the endpoint node (the DT binding also doesn't allow now anyway).
These properties should just exist in the Root Port node as there can be only
one set per hierarchy i.e., Root Complex would only use one set of these GPIOs
per Root Port and the endpoint need to share them.

So I closed the dtschema issue.

- Mani

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