Thanks for clearing up some confusion. I was misled on some aspects. But
I think there's still a problem in here:
On Thu, Jul 24, 2025 at 07:43:38PM +0530, Manivannan Sadhasivam wrote:
> 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:
> > > 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?
...
> 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.
That implies it's not a 1:1 correlation between PERST# GPIO and pwrctrl
device. Multiple endpoints might need powered up, but they may share a
PERST#. I don't think this patch solves this properly, as it allows the
first one to deassert PERST# before the other(s) are powered.
Or maybe I'm missing something yet again :)
Brian
