Re: [PATCH 2/2] PCI/pwrctrl: Only create pwrctrl device if the device node is of type "pci"

[Manivannan Sadhasivam]

On Tue, Feb 17, 2026 at 12:09:56PM -0600, Bjorn Andersson wrote:
> On Tue, Feb 17, 2026 at 03:48:47PM +0530, Manivannan Sadhasivam via B4 Relay wrote:
> > From: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxxxxxxxx>
> > 
> > The PCI host bridge node can have non-PCI child nodes as well, like OPP
> > tables, USB controller node etc...
> 
> Yes, but the usb-controller in my case is a PCI child and should be
> pwrctrl'ed. The entity that I don't want pcictrl to handle is the HUB
> child on the USB bus.
> 

I was wrong here. More below...

> > So the pwrctrl core must check for the
> > presence of 'device_type' property with value of "pci" to ensure that the
> > pwrctrl device is only created for PCI device nodes.
> > 
> > Fixes: 4c4132489201 ("PCI/pwrctrl: Add APIs to create, destroy pwrctrl devices")
> > Reported-by: Bjorn Andersson <bjorn.andersson@xxxxxxxxxxxxxxxx>
> > Closes: https://lore.kernel.org/all/20260212-rb3gen2-upd-gl3590-v1-1-18fb04bb32b0@xxxxxxxxxxxxxxxx
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxxxxxxxx>
> > ---
> >  drivers/pci/pwrctrl/core.c | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/pci/pwrctrl/core.c b/drivers/pci/pwrctrl/core.c
> > index 8325858cc379..7404d48427ce 100644
> > --- a/drivers/pci/pwrctrl/core.c
> > +++ b/drivers/pci/pwrctrl/core.c
> > @@ -272,8 +272,9 @@ EXPORT_SYMBOL_GPL(pci_pwrctrl_power_on_devices);
> >   * Check whether the pwrctrl device really needs to be created or not. The
> >   * pwrctrl device will only be created if the node satisfies below requirements:
> >   *
> > - * 1. Presence of compatible property to match against the pwrctrl driver (AND)
> > - * 2. At least one of the power supplies defined in the devicetree node of the
> > + * 1. Presence of 'device_type = "pci"' property to identify PCI node (AND)
> > + * 2. Presence of compatible property to match against the pwrctrl driver (AND)
> > + * 3. At least one of the power supplies defined in the devicetree node of the
> >   *    device (OR) in the remote endpoint parent node to indicate pwrctrl
> >   *    requirement.
> >   */
> > @@ -281,6 +282,9 @@ static bool pci_pwrctrl_is_required(struct device_node *np)
> >  {
> >  	struct device_node *endpoint;
> >  
> > +	if (!of_node_is_type(np, "pci"))
> > +		return false;
> 
> This "solves" my problem by no longer attempting to power on the
> onboard_hub. But it also ensures that Neil's added μPD720201 power
> controller doesn't power on.
> 
> This is based on the expectation that I shouldn't mark the PCI device
> (the μPD720201) as device_type = "pci", right? (Doing so makes dtc barf,
> but the μPD720201 is powered up, and functional).
> 

Ah, this property is applicable only to bridges, not endpoints. So dt checker
was right and I was wrong :(

> If this is incorrect, can you please help me understand how the
> usb-controller@0,0 node should look like in
> https://lore.kernel.org/all/20260212-rb3gen2-upd-gl3590-v1-1-18fb04bb32b0@xxxxxxxxxxxxxxxx/
> 

Will the below diff on top of this series help? It checks for the presence of
the 'pci' prefix in the device compatible, which should always exist for all
kind of PCI devices including the USB controller and will be absent for the
internal USB hub, which will have 'usb' prefix.

---
diff --git a/drivers/pci/pwrctrl/core.c b/drivers/pci/pwrctrl/core.c
index 7404d48427ce..7754baed67f2 100644
--- a/drivers/pci/pwrctrl/core.c
+++ b/drivers/pci/pwrctrl/core.c
@@ -272,20 +272,23 @@ EXPORT_SYMBOL_GPL(pci_pwrctrl_power_on_devices);
  * Check whether the pwrctrl device really needs to be created or not. The
  * pwrctrl device will only be created if the node satisfies below requirements:
  *
- * 1. Presence of 'device_type = "pci"' property to identify PCI node (AND)
- * 2. Presence of compatible property to match against the pwrctrl driver (AND)
- * 3. At least one of the power supplies defined in the devicetree node of the
+ * 1. Presence of compatible property with "pci" prefix to match against the
+ *    pwrctrl driver (AND)
+ * 2. At least one of the power supplies defined in the devicetree node of the
  *    device (OR) in the remote endpoint parent node to indicate pwrctrl
  *    requirement.
  */
 static bool pci_pwrctrl_is_required(struct device_node *np)
 {
        struct device_node *endpoint;
+       const char *compat;
+       int ret;
 
-       if (!of_node_is_type(np, "pci"))
+       ret = of_property_read_string(np, "compatible", &compat);
+       if (ret < 0)
                return false;
 
-       if (!of_property_present(np, "compatible"))
+       if (!strstarts(compat, "pci"))
                return false;
 
        if (of_pci_supply_present(np))

- Mani

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