1. Patchew
  2. linux
  3. [PATCH 0/2] PCI/pwrctrl: A couple of fixes
  4. View patch

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

Manivannan Sadhasivam via B4 Relay posted 2 patches 1 month, 2 weeks ago
There is a newer version of this series
[PATCH 2/2] PCI/pwrctrl: Only create pwrctrl device if the device node is of type "pci"
Posted by Manivannan Sadhasivam via B4 Relay 1 month, 2 weeks ago 
From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

The PCI host bridge node can have non-PCI child nodes as well, like OPP
tables, USB controller node etc... 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@oss.qualcomm.com>
Closes: https://lore.kernel.org/all/20260212-rb3gen2-upd-gl3590-v1-1-18fb04bb32b0@oss.qualcomm.com
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---
 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;
+
 	if (!of_property_present(np, "compatible"))
 		return false;
 

-- 
2.51.0
Re: [PATCH 2/2] PCI/pwrctrl: Only create pwrctrl device if the device node is of type "pci"
Posted by Krishna Chaitanya Chundru 1 month, 2 weeks ago 

On 2/17/2026 3:48 PM, Manivannan Sadhasivam via B4 Relay wrote:
> From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
>
> The PCI host bridge node can have non-PCI child nodes as well, like OPP
> tables, USB controller node etc... 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@oss.qualcomm.com>
> Closes: https://lore.kernel.org/all/20260212-rb3gen2-upd-gl3590-v1-1-18fb04bb32b0@oss.qualcomm.com
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
Reviewed-by: Krishna Chaitanya Chundru<krishna.chundru@oss.qualcomm.com>

- Krishna Chaitanya.
> ---
>   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;
> +
>   	if (!of_property_present(np, "compatible"))
>   		return false;
>   
>
Re: [PATCH 2/2] PCI/pwrctrl: Only create pwrctrl device if the device node is of type "pci"
Posted by Bartosz Golaszewski 1 month, 2 weeks ago 
On Tue, 17 Feb 2026 11:18:47 +0100, Manivannan Sadhasivam via B4 Relay
<devnull+manivannan.sadhasivam.oss.qualcomm.com@kernel.org> said:
> From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
>
> The PCI host bridge node can have non-PCI child nodes as well, like OPP
> tables, USB controller node etc... 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@oss.qualcomm.com>
> Closes: https://lore.kernel.org/all/20260212-rb3gen2-upd-gl3590-v1-1-18fb04bb32b0@oss.qualcomm.com
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> ---

Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Re: [PATCH 2/2] PCI/pwrctrl: Only create pwrctrl device if the device node is of type "pci"
Posted by Bjorn Andersson 1 month, 2 weeks ago 
On Tue, Feb 17, 2026 at 03:48:47PM +0530, Manivannan Sadhasivam via B4 Relay wrote:
> From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> 
> 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.

> 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@oss.qualcomm.com>
> Closes: https://lore.kernel.org/all/20260212-rb3gen2-upd-gl3590-v1-1-18fb04bb32b0@oss.qualcomm.com
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> ---
>  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).

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@oss.qualcomm.com/

Regards,
Bjorn

> +
>  	if (!of_property_present(np, "compatible"))
>  		return false;
>  
> 
> -- 
> 2.51.0
> 
>
Re: [PATCH 2/2] PCI/pwrctrl: Only create pwrctrl device if the device node is of type "pci"
Posted by Manivannan Sadhasivam 1 month, 1 week ago 
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@oss.qualcomm.com>
> > 
> > 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@oss.qualcomm.com>
> > Closes: https://lore.kernel.org/all/20260212-rb3gen2-upd-gl3590-v1-1-18fb04bb32b0@oss.qualcomm.com
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> > ---
> >  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@oss.qualcomm.com/
> 

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

-- 
மணிவண்ணன் சதாசிவம்
Re: [PATCH 2/2] PCI/pwrctrl: Only create pwrctrl device if the device node is of type "pci"
Posted by Bjorn Andersson 1 month, 1 week ago 
On Wed, Feb 18, 2026 at 05:54:58PM +0530, Manivannan Sadhasivam wrote:
> 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@oss.qualcomm.com>
[..]
> > > @@ -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 :(
> 

Thanks for confirming my understanding (about the property).

> > 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@oss.qualcomm.com/
> > 
> 
> 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.
> 

It does look a bit crude, but it does resolve my issue. And I don't have
a much better suggestion at this time.

Regards,
Bjorn

> ---
> 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
> 
> -- 
> மணிவண்ணன் சதாசிவம்
Re: [PATCH 2/2] PCI/pwrctrl: Only create pwrctrl device if the device node is of type "pci"
Posted by Manivannan Sadhasivam 1 month, 1 week ago 
On Mon, Feb 23, 2026 at 08:25:43AM -0600, Bjorn Andersson wrote:
> On Wed, Feb 18, 2026 at 05:54:58PM +0530, Manivannan Sadhasivam wrote:
> > 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@oss.qualcomm.com>
> [..]
> > > > @@ -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 :(
> > 
> 
> Thanks for confirming my understanding (about the property).
> 
> > > 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@oss.qualcomm.com/
> > > 
> > 
> > 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.
> > 
> 
> It does look a bit crude, but it does resolve my issue. And I don't have
> a much better suggestion at this time.
> 

Thanks for confirming. I'll respin the patches with the changes.

- Mani

> Regards,
> Bjorn
> 
> > ---
> > 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
> > 
> > -- 
> > மணிவண்ணன் சதாசிவம்

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

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.