diff mbox series

[v2,4/8] PCI: Change the parent to correctly represent pcie hierarchy

Message ID 20240803-qps615-v2-4-9560b7c71369@quicinc.com (mailing list archive)
State New
Delegated to: Manivannan Sadhasivam
Headers show
Series PCI: Enable Power and configure the QPS615 PCIe switch | expand

Commit Message

Krishna chaitanya chundru Aug. 3, 2024, 3:22 a.m. UTC
Currently the pwrctl driver is child of pci-pci bridge driver,
this will cause issue when suspend resume is introduced in the pwr
control driver. If the supply is removed to the endpoint in the
power control driver then the config space access by the
pci-pci bridge driver can cause issues like Timeouts.

For this reason change the parent to controller from pci-pci bridge.

Fixes: 4565d2652a37 ("PCI/pwrctl: Add PCI power control core code")
Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
---
 drivers/pci/bus.c         | 3 ++-
 drivers/pci/pwrctl/core.c | 9 ++++++++-
 2 files changed, 10 insertions(+), 2 deletions(-)

Comments

Bjorn Helgaas Aug. 6, 2024, 7:07 p.m. UTC | #1
On Sat, Aug 03, 2024 at 08:52:50AM +0530, Krishna chaitanya chundru wrote:
> Currently the pwrctl driver is child of pci-pci bridge driver,
> this will cause issue when suspend resume is introduced in the pwr
> control driver. If the supply is removed to the endpoint in the
> power control driver then the config space access by the
> pci-pci bridge driver can cause issues like Timeouts.

If "pci-pci bridge driver" refers to portdrv, please use "portdrv" to
avoid confusion.

Can you be a little more specific about config accesses by the bridge
driver?  Generally portdrv wouldn't touch devices below the bridge.
It sounds like you've tripped over something here, so you probably
have an example of a timeout.

s/pcie/PCIe/ in subject, although it'd be nice if the whole subject
could be a little more specific.  I don't think pwrctl is directly
part of the PCIe hierarchy, so I don't quite understand what you're
saying there.

> For this reason change the parent to controller from pci-pci bridge.
> 
> Fixes: 4565d2652a37 ("PCI/pwrctl: Add PCI power control core code")

Will need an ack from Bartosz, of course, since he added this.  Moved
from cc: to to: list to make sure he sees this.

> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
> ---
>  drivers/pci/bus.c         | 3 ++-
>  drivers/pci/pwrctl/core.c | 9 ++++++++-
>  2 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
> index 55c853686051..15b42f0f588f 100644
> --- a/drivers/pci/bus.c
> +++ b/drivers/pci/bus.c
> @@ -328,6 +328,7 @@ void __weak pcibios_bus_add_device(struct pci_dev *pdev) { }
>   */
>  void pci_bus_add_device(struct pci_dev *dev)
>  {
> +	struct pci_host_bridge *host = pci_find_host_bridge(dev->bus);
>  	struct device_node *dn = dev->dev.of_node;
>  	int retval;
>  
> @@ -352,7 +353,7 @@ void pci_bus_add_device(struct pci_dev *dev)
>  
>  	if (dev_of_node(&dev->dev) && pci_is_bridge(dev)) {
>  		retval = of_platform_populate(dev_of_node(&dev->dev), NULL, NULL,
> -					      &dev->dev);
> +					      host->dev.parent);

I'm not sure host->dev.parent is always valid.  There are
pci_create_root_bus() callers that supply a NULL parent pointer.

>  		if (retval)
>  			pci_err(dev, "failed to populate child OF nodes (%d)\n",
>  				retval);
> diff --git a/drivers/pci/pwrctl/core.c b/drivers/pci/pwrctl/core.c
> index feca26ad2f6a..4f2ffa0b0a5f 100644
> --- a/drivers/pci/pwrctl/core.c
> +++ b/drivers/pci/pwrctl/core.c
> @@ -11,6 +11,8 @@
>  #include <linux/property.h>
>  #include <linux/slab.h>
>  
> +#include "../pci.h"
> +
>  static int pci_pwrctl_notify(struct notifier_block *nb, unsigned long action,
>  			     void *data)
>  {
> @@ -64,18 +66,23 @@ static int pci_pwrctl_notify(struct notifier_block *nb, unsigned long action,
>   */
>  int pci_pwrctl_device_set_ready(struct pci_pwrctl *pwrctl)
>  {
> +	struct pci_bus *bus;
>  	int ret;
>  
>  	if (!pwrctl->dev)
>  		return -ENODEV;
>  
> +	bus = pci_find_bus(of_get_pci_domain_nr(pwrctl->dev->parent->of_node), 0);
> +	if (!bus)
> +		return -ENODEV;
> +
>  	pwrctl->nb.notifier_call = pci_pwrctl_notify;
>  	ret = bus_register_notifier(&pci_bus_type, &pwrctl->nb);
>  	if (ret)
>  		return ret;
>  
>  	pci_lock_rescan_remove();
> -	pci_rescan_bus(to_pci_dev(pwrctl->dev->parent)->bus);
> +	pci_rescan_bus(bus);
>  	pci_unlock_rescan_remove();
>  
>  	return 0;
> 
> -- 
> 2.34.1
>
Bartosz Golaszewski Aug. 6, 2024, 8:06 p.m. UTC | #2
On Tue, Aug 6, 2024 at 9:07 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> On Sat, Aug 03, 2024 at 08:52:50AM +0530, Krishna chaitanya chundru wrote:
> > Currently the pwrctl driver is child of pci-pci bridge driver,
> > this will cause issue when suspend resume is introduced in the pwr
> > control driver. If the supply is removed to the endpoint in the
> > power control driver then the config space access by the
> > pci-pci bridge driver can cause issues like Timeouts.
>
> If "pci-pci bridge driver" refers to portdrv, please use "portdrv" to
> avoid confusion.
>
> Can you be a little more specific about config accesses by the bridge
> driver?  Generally portdrv wouldn't touch devices below the bridge.
> It sounds like you've tripped over something here, so you probably
> have an example of a timeout.
>
> s/pcie/PCIe/ in subject, although it'd be nice if the whole subject
> could be a little more specific.  I don't think pwrctl is directly
> part of the PCIe hierarchy, so I don't quite understand what you're
> saying there.
>
> > For this reason change the parent to controller from pci-pci bridge.
> >
> > Fixes: 4565d2652a37 ("PCI/pwrctl: Add PCI power control core code")
>
> Will need an ack from Bartosz, of course, since he added this.  Moved
> from cc: to to: list to make sure he sees this.
>

I would drop the Fixes tag altogether. This is a change in
implementation but it doesn't really fix a bug or regression.

Other than that: please feel free to add

Acked-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

I will also review the pwrctl part of the series shortly.

Bart

> > Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
> > ---
> >  drivers/pci/bus.c         | 3 ++-
> >  drivers/pci/pwrctl/core.c | 9 ++++++++-
> >  2 files changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
> > index 55c853686051..15b42f0f588f 100644
> > --- a/drivers/pci/bus.c
> > +++ b/drivers/pci/bus.c
> > @@ -328,6 +328,7 @@ void __weak pcibios_bus_add_device(struct pci_dev *pdev) { }
> >   */
> >  void pci_bus_add_device(struct pci_dev *dev)
> >  {
> > +     struct pci_host_bridge *host = pci_find_host_bridge(dev->bus);
> >       struct device_node *dn = dev->dev.of_node;
> >       int retval;
> >
> > @@ -352,7 +353,7 @@ void pci_bus_add_device(struct pci_dev *dev)
> >
> >       if (dev_of_node(&dev->dev) && pci_is_bridge(dev)) {
> >               retval = of_platform_populate(dev_of_node(&dev->dev), NULL, NULL,
> > -                                           &dev->dev);
> > +                                           host->dev.parent);
>
> I'm not sure host->dev.parent is always valid.  There are
> pci_create_root_bus() callers that supply a NULL parent pointer.
>
> >               if (retval)
> >                       pci_err(dev, "failed to populate child OF nodes (%d)\n",
> >                               retval);
> > diff --git a/drivers/pci/pwrctl/core.c b/drivers/pci/pwrctl/core.c
> > index feca26ad2f6a..4f2ffa0b0a5f 100644
> > --- a/drivers/pci/pwrctl/core.c
> > +++ b/drivers/pci/pwrctl/core.c
> > @@ -11,6 +11,8 @@
> >  #include <linux/property.h>
> >  #include <linux/slab.h>
> >
> > +#include "../pci.h"
> > +
> >  static int pci_pwrctl_notify(struct notifier_block *nb, unsigned long action,
> >                            void *data)
> >  {
> > @@ -64,18 +66,23 @@ static int pci_pwrctl_notify(struct notifier_block *nb, unsigned long action,
> >   */
> >  int pci_pwrctl_device_set_ready(struct pci_pwrctl *pwrctl)
> >  {
> > +     struct pci_bus *bus;
> >       int ret;
> >
> >       if (!pwrctl->dev)
> >               return -ENODEV;
> >
> > +     bus = pci_find_bus(of_get_pci_domain_nr(pwrctl->dev->parent->of_node), 0);
> > +     if (!bus)
> > +             return -ENODEV;
> > +
> >       pwrctl->nb.notifier_call = pci_pwrctl_notify;
> >       ret = bus_register_notifier(&pci_bus_type, &pwrctl->nb);
> >       if (ret)
> >               return ret;
> >
> >       pci_lock_rescan_remove();
> > -     pci_rescan_bus(to_pci_dev(pwrctl->dev->parent)->bus);
> > +     pci_rescan_bus(bus);
> >       pci_unlock_rescan_remove();
> >
> >       return 0;
> >
> > --
> > 2.34.1
> >
Bartosz Golaszewski Aug. 13, 2024, 7:15 p.m. UTC | #3
On Sat, Aug 3, 2024 at 5:23 AM Krishna chaitanya chundru
<quic_krichai@quicinc.com> wrote:
>
> Currently the pwrctl driver is child of pci-pci bridge driver,
> this will cause issue when suspend resume is introduced in the pwr
> control driver. If the supply is removed to the endpoint in the
> power control driver then the config space access by the
> pci-pci bridge driver can cause issues like Timeouts.
>
> For this reason change the parent to controller from pci-pci bridge.
>
> Fixes: 4565d2652a37 ("PCI/pwrctl: Add PCI power control core code")
> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
> ---

Tested-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Bjorn,

I think this should go into v6.11 as it does indeed better represent
the underlying logic.

Bart
Bjorn Helgaas Aug. 22, 2024, 7:28 p.m. UTC | #4
On Tue, Aug 13, 2024 at 09:15:06PM +0200, Bartosz Golaszewski wrote:
> On Sat, Aug 3, 2024 at 5:23 AM Krishna chaitanya chundru
> <quic_krichai@quicinc.com> wrote:
> >
> > Currently the pwrctl driver is child of pci-pci bridge driver,
> > this will cause issue when suspend resume is introduced in the pwr
> > control driver. If the supply is removed to the endpoint in the
> > power control driver then the config space access by the
> > pci-pci bridge driver can cause issues like Timeouts.
> >
> > For this reason change the parent to controller from pci-pci bridge.
> >
> > Fixes: 4565d2652a37 ("PCI/pwrctl: Add PCI power control core code")
> > Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
> > ---
> 
> Tested-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> 
> Bjorn,
> 
> I think this should go into v6.11 as it does indeed better represent
> the underlying logic.

Is this patch independent of the rest?  I don't think the whole series
looks like v6.11 material, but if this patch can be applied
independently, *and* we can make a case in the commit log for why it
is v6.11 material, we can do that.

Right now the commit log doesn't tell me enough to justify a
post-merge window change.

Bjorn
Bartosz Golaszewski Aug. 22, 2024, 8:01 p.m. UTC | #5
On Thu, Aug 22, 2024 at 9:28 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> On Tue, Aug 13, 2024 at 09:15:06PM +0200, Bartosz Golaszewski wrote:
> > On Sat, Aug 3, 2024 at 5:23 AM Krishna chaitanya chundru
> > <quic_krichai@quicinc.com> wrote:
> > >
> > > Currently the pwrctl driver is child of pci-pci bridge driver,
> > > this will cause issue when suspend resume is introduced in the pwr
> > > control driver. If the supply is removed to the endpoint in the
> > > power control driver then the config space access by the
> > > pci-pci bridge driver can cause issues like Timeouts.
> > >
> > > For this reason change the parent to controller from pci-pci bridge.
> > >
> > > Fixes: 4565d2652a37 ("PCI/pwrctl: Add PCI power control core code")
> > > Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
> > > ---
> >
> > Tested-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >
> > Bjorn,
> >
> > I think this should go into v6.11 as it does indeed better represent
> > the underlying logic.
>
> Is this patch independent of the rest?  I don't think the whole series
> looks like v6.11 material, but if this patch can be applied
> independently, *and* we can make a case in the commit log for why it
> is v6.11 material, we can do that.
>
> Right now the commit log doesn't tell me enough to justify a
> post-merge window change.
>
> Bjorn

Please, apply this patch independently. FYI I have a WiP branch[1]
with a v3 of the fixes series rebased on top of this one. Manivannan
and I are working on fixing one last remaining issue and I'll resend
it. This should go into v6.11.

Bart

[1] https://git.codelinaro.org/bartosz_golaszewski/linux/-/tree/topic/pci-pwrctl-fixes
Bjorn Helgaas Aug. 22, 2024, 9:13 p.m. UTC | #6
On Thu, Aug 22, 2024 at 10:01:04PM +0200, Bartosz Golaszewski wrote:
> On Thu, Aug 22, 2024 at 9:28 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
> >
> > On Tue, Aug 13, 2024 at 09:15:06PM +0200, Bartosz Golaszewski wrote:
> > > On Sat, Aug 3, 2024 at 5:23 AM Krishna chaitanya chundru
> > > <quic_krichai@quicinc.com> wrote:
> > > >
> > > > Currently the pwrctl driver is child of pci-pci bridge driver,
> > > > this will cause issue when suspend resume is introduced in the pwr
> > > > control driver. If the supply is removed to the endpoint in the
> > > > power control driver then the config space access by the
> > > > pci-pci bridge driver can cause issues like Timeouts.
> > > >
> > > > For this reason change the parent to controller from pci-pci bridge.
> > > >
> > > > Fixes: 4565d2652a37 ("PCI/pwrctl: Add PCI power control core code")
> > > > Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
> > > > ---
> > >
> > > Tested-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> > >
> > > Bjorn,
> > >
> > > I think this should go into v6.11 as it does indeed better represent
> > > the underlying logic.
> >
> > Is this patch independent of the rest?  I don't think the whole series
> > looks like v6.11 material, but if this patch can be applied
> > independently, *and* we can make a case in the commit log for why it
> > is v6.11 material, we can do that.
> >
> > Right now the commit log doesn't tell me enough to justify a
> > post-merge window change.
> 
> Please, apply this patch independently. FYI I have a WiP branch[1]
> with a v3 of the fixes series rebased on top of this one. Manivannan
> and I are working on fixing one last remaining issue and I'll resend
> it. This should go into v6.11.

OK.  I just need to be able to justify *why* we need it in v6.11, so I
can apply it as soon as somebody supplies that kind of text for the
commit log.  I.e., what is broken without this change?  What bad
things happen if we defer it to v6.12?

> [1] https://git.codelinaro.org/bartosz_golaszewski/linux/-/tree/topic/pci-pwrctl-fixes
Manivannan Sadhasivam Aug. 23, 2024, 7:23 a.m. UTC | #7
On Sat, Aug 03, 2024 at 08:52:50AM +0530, Krishna chaitanya chundru wrote:
> Currently the pwrctl driver is child of pci-pci bridge driver,
> this will cause issue when suspend resume is introduced in the pwr
> control driver. If the supply is removed to the endpoint in the
> power control driver then the config space access by the
> pci-pci bridge driver can cause issues like Timeouts.
> 
> For this reason change the parent to controller from pci-pci bridge.
> 

Also, what if the PCIe controller driver tries to access the device? Like for
sending PME_Turn_Off etc... during suspend? I think you should also make sure
that the suspend callback of the pwrctl driver has to happen _after_ the
controller driver.

Still the parent-child hierarchy is not going to change, but only the devlink
part.

- Mani

> Fixes: 4565d2652a37 ("PCI/pwrctl: Add PCI power control core code")
> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
> ---
>  drivers/pci/bus.c         | 3 ++-
>  drivers/pci/pwrctl/core.c | 9 ++++++++-
>  2 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
> index 55c853686051..15b42f0f588f 100644
> --- a/drivers/pci/bus.c
> +++ b/drivers/pci/bus.c
> @@ -328,6 +328,7 @@ void __weak pcibios_bus_add_device(struct pci_dev *pdev) { }
>   */
>  void pci_bus_add_device(struct pci_dev *dev)
>  {
> +	struct pci_host_bridge *host = pci_find_host_bridge(dev->bus);
>  	struct device_node *dn = dev->dev.of_node;
>  	int retval;
>  
> @@ -352,7 +353,7 @@ void pci_bus_add_device(struct pci_dev *dev)
>  
>  	if (dev_of_node(&dev->dev) && pci_is_bridge(dev)) {
>  		retval = of_platform_populate(dev_of_node(&dev->dev), NULL, NULL,
> -					      &dev->dev);
> +					      host->dev.parent);
>  		if (retval)
>  			pci_err(dev, "failed to populate child OF nodes (%d)\n",
>  				retval);
> diff --git a/drivers/pci/pwrctl/core.c b/drivers/pci/pwrctl/core.c
> index feca26ad2f6a..4f2ffa0b0a5f 100644
> --- a/drivers/pci/pwrctl/core.c
> +++ b/drivers/pci/pwrctl/core.c
> @@ -11,6 +11,8 @@
>  #include <linux/property.h>
>  #include <linux/slab.h>
>  
> +#include "../pci.h"
> +
>  static int pci_pwrctl_notify(struct notifier_block *nb, unsigned long action,
>  			     void *data)
>  {
> @@ -64,18 +66,23 @@ static int pci_pwrctl_notify(struct notifier_block *nb, unsigned long action,
>   */
>  int pci_pwrctl_device_set_ready(struct pci_pwrctl *pwrctl)
>  {
> +	struct pci_bus *bus;
>  	int ret;
>  
>  	if (!pwrctl->dev)
>  		return -ENODEV;
>  
> +	bus = pci_find_bus(of_get_pci_domain_nr(pwrctl->dev->parent->of_node), 0);
> +	if (!bus)
> +		return -ENODEV;
> +
>  	pwrctl->nb.notifier_call = pci_pwrctl_notify;
>  	ret = bus_register_notifier(&pci_bus_type, &pwrctl->nb);
>  	if (ret)
>  		return ret;
>  
>  	pci_lock_rescan_remove();
> -	pci_rescan_bus(to_pci_dev(pwrctl->dev->parent)->bus);
> +	pci_rescan_bus(bus);
>  	pci_unlock_rescan_remove();
>  
>  	return 0;
> 
> -- 
> 2.34.1
>
Manivannan Sadhasivam Aug. 23, 2024, 8:30 a.m. UTC | #8
On Thu, Aug 22, 2024 at 04:13:36PM -0500, Bjorn Helgaas wrote:
> On Thu, Aug 22, 2024 at 10:01:04PM +0200, Bartosz Golaszewski wrote:
> > On Thu, Aug 22, 2024 at 9:28 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
> > >
> > > On Tue, Aug 13, 2024 at 09:15:06PM +0200, Bartosz Golaszewski wrote:
> > > > On Sat, Aug 3, 2024 at 5:23 AM Krishna chaitanya chundru
> > > > <quic_krichai@quicinc.com> wrote:
> > > > >
> > > > > Currently the pwrctl driver is child of pci-pci bridge driver,
> > > > > this will cause issue when suspend resume is introduced in the pwr
> > > > > control driver. If the supply is removed to the endpoint in the
> > > > > power control driver then the config space access by the
> > > > > pci-pci bridge driver can cause issues like Timeouts.
> > > > >
> > > > > For this reason change the parent to controller from pci-pci bridge.
> > > > >
> > > > > Fixes: 4565d2652a37 ("PCI/pwrctl: Add PCI power control core code")
> > > > > Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
> > > > > ---
> > > >
> > > > Tested-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> > > >
> > > > Bjorn,
> > > >
> > > > I think this should go into v6.11 as it does indeed better represent
> > > > the underlying logic.
> > >
> > > Is this patch independent of the rest?  I don't think the whole series
> > > looks like v6.11 material, but if this patch can be applied
> > > independently, *and* we can make a case in the commit log for why it
> > > is v6.11 material, we can do that.
> > >
> > > Right now the commit log doesn't tell me enough to justify a
> > > post-merge window change.
> > 
> > Please, apply this patch independently. FYI I have a WiP branch[1]
> > with a v3 of the fixes series rebased on top of this one. Manivannan
> > and I are working on fixing one last remaining issue and I'll resend
> > it. This should go into v6.11.
> 
> OK.  I just need to be able to justify *why* we need it in v6.11, so I
> can apply it as soon as somebody supplies that kind of text for the
> commit log.  I.e., what is broken without this change?  What bad
> things happen if we defer it to v6.12?
> 

I'm not sure if this is a 6.11 material as this patch is not fixing any crash or
potential breakage in 6.11. This patch changes the hierarchy in such a way that
the suspend/resume could work fine once added in the pwrctl drivers.

At the same time, I'd like to get it merged separately for 6.12 instead of
bundling it in this same series.

- Mani
Bartosz Golaszewski Aug. 23, 2024, 8:31 a.m. UTC | #9
On Fri, 23 Aug 2024 at 10:30, Manivannan Sadhasivam
<manivannan.sadhasivam@linaro.org> wrote:
>
> > > >
> > > > Right now the commit log doesn't tell me enough to justify a
> > > > post-merge window change.
> > >
> > > Please, apply this patch independently. FYI I have a WiP branch[1]
> > > with a v3 of the fixes series rebased on top of this one. Manivannan
> > > and I are working on fixing one last remaining issue and I'll resend
> > > it. This should go into v6.11.
> >
> > OK.  I just need to be able to justify *why* we need it in v6.11, so I
> > can apply it as soon as somebody supplies that kind of text for the
> > commit log.  I.e., what is broken without this change?  What bad
> > things happen if we defer it to v6.12?
> >
>
> I'm not sure if this is a 6.11 material as this patch is not fixing any crash or
> potential breakage in 6.11. This patch changes the hierarchy in such a way that
> the suspend/resume could work fine once added in the pwrctl drivers.
>
> At the same time, I'd like to get it merged separately for 6.12 instead of
> bundling it in this same series.
>

Ok, I'll find a way to rework my patches so that this one's not needed.

Bart
diff mbox series

Patch

diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
index 55c853686051..15b42f0f588f 100644
--- a/drivers/pci/bus.c
+++ b/drivers/pci/bus.c
@@ -328,6 +328,7 @@  void __weak pcibios_bus_add_device(struct pci_dev *pdev) { }
  */
 void pci_bus_add_device(struct pci_dev *dev)
 {
+	struct pci_host_bridge *host = pci_find_host_bridge(dev->bus);
 	struct device_node *dn = dev->dev.of_node;
 	int retval;
 
@@ -352,7 +353,7 @@  void pci_bus_add_device(struct pci_dev *dev)
 
 	if (dev_of_node(&dev->dev) && pci_is_bridge(dev)) {
 		retval = of_platform_populate(dev_of_node(&dev->dev), NULL, NULL,
-					      &dev->dev);
+					      host->dev.parent);
 		if (retval)
 			pci_err(dev, "failed to populate child OF nodes (%d)\n",
 				retval);
diff --git a/drivers/pci/pwrctl/core.c b/drivers/pci/pwrctl/core.c
index feca26ad2f6a..4f2ffa0b0a5f 100644
--- a/drivers/pci/pwrctl/core.c
+++ b/drivers/pci/pwrctl/core.c
@@ -11,6 +11,8 @@ 
 #include <linux/property.h>
 #include <linux/slab.h>
 
+#include "../pci.h"
+
 static int pci_pwrctl_notify(struct notifier_block *nb, unsigned long action,
 			     void *data)
 {
@@ -64,18 +66,23 @@  static int pci_pwrctl_notify(struct notifier_block *nb, unsigned long action,
  */
 int pci_pwrctl_device_set_ready(struct pci_pwrctl *pwrctl)
 {
+	struct pci_bus *bus;
 	int ret;
 
 	if (!pwrctl->dev)
 		return -ENODEV;
 
+	bus = pci_find_bus(of_get_pci_domain_nr(pwrctl->dev->parent->of_node), 0);
+	if (!bus)
+		return -ENODEV;
+
 	pwrctl->nb.notifier_call = pci_pwrctl_notify;
 	ret = bus_register_notifier(&pci_bus_type, &pwrctl->nb);
 	if (ret)
 		return ret;
 
 	pci_lock_rescan_remove();
-	pci_rescan_bus(to_pci_dev(pwrctl->dev->parent)->bus);
+	pci_rescan_bus(bus);
 	pci_unlock_rescan_remove();
 
 	return 0;