| Message ID | 20240925161520.2736895-1-Basavaraj.Natikar@amd.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | xhci: Change pm_runtime_forbid to pm_runtime_disable | expand |
On 25.9.2024 19.15, Basavaraj Natikar wrote: > Change pm_runtime_forbid to pm_runtime_disable to disable RPM on > platforms that don't support runtime D3, as re-enabling it through sysfs > auto power control may cause the controller to malfunction. This can lead > to issues such as hotplug devices not being detected due to failed > interrupt generation. > > Fixes: a5d6264b638e ("xhci: Enable RPM on controllers that support low-power states") > Suggested-by: Mario Limonciello <mario.limonciello@amd.com> > Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com> > --- Added to queue Thanks -Mathias
On Wed, Sep 25, 2024 at 09:45:20PM +0530, Basavaraj Natikar wrote: > Change pm_runtime_forbid to pm_runtime_disable to disable RPM on > platforms that don't support runtime D3, as re-enabling it through sysfs > auto power control may cause the controller to malfunction. This can lead > to issues such as hotplug devices not being detected due to failed > interrupt generation. > > Fixes: a5d6264b638e ("xhci: Enable RPM on controllers that support low-power states") > Suggested-by: Mario Limonciello <mario.limonciello@amd.com> > Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com> > --- > drivers/usb/host/xhci-pci.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c > index dc1e345ab67e..39ab32fed3f4 100644 > --- a/drivers/usb/host/xhci-pci.c > +++ b/drivers/usb/host/xhci-pci.c > @@ -635,7 +635,7 @@ static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id) > pm_runtime_put_noidle(&dev->dev); > > if (pci_choose_state(dev, PMSG_SUSPEND) == PCI_D0) > - pm_runtime_forbid(&dev->dev); > + pm_runtime_disable(&dev->dev); This looks broken. Runtime PM enable/disable should always be balanced, and unlike forbid(), disable() doesn't make sure that the device is actually active. If you want to prevent the device from runtime suspending you can just increment the usage counter here in probe() and drop it in remove(). > else if (xhci->quirks & XHCI_DEFAULT_PM_RUNTIME_ALLOW) > pm_runtime_allow(&dev->dev); NAK. Johan
On 30.9.2024 15.57, Johan Hovold wrote: > On Wed, Sep 25, 2024 at 09:45:20PM +0530, Basavaraj Natikar wrote: >> Change pm_runtime_forbid to pm_runtime_disable to disable RPM on >> platforms that don't support runtime D3, as re-enabling it through sysfs >> auto power control may cause the controller to malfunction. This can lead >> to issues such as hotplug devices not being detected due to failed >> interrupt generation. >> >> Fixes: a5d6264b638e ("xhci: Enable RPM on controllers that support low-power states") >> Suggested-by: Mario Limonciello <mario.limonciello@amd.com> >> Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com> >> --- >> drivers/usb/host/xhci-pci.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c >> index dc1e345ab67e..39ab32fed3f4 100644 >> --- a/drivers/usb/host/xhci-pci.c >> +++ b/drivers/usb/host/xhci-pci.c >> @@ -635,7 +635,7 @@ static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id) >> pm_runtime_put_noidle(&dev->dev); >> >> if (pci_choose_state(dev, PMSG_SUSPEND) == PCI_D0) >> - pm_runtime_forbid(&dev->dev); >> + pm_runtime_disable(&dev->dev); > > This looks broken. Runtime PM enable/disable should always be balanced, > and unlike forbid(), disable() doesn't make sure that the device is > actually active. > > If you want to prevent the device from runtime suspending you can just > increment the usage counter here in probe() and drop it in remove(). Good point. Adding a pm_runtime_get() here, and pm_runtime_put() in remove sounds much better The PCI code should have set it active and enabled it already. Dropping this Thanks Mathias
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c index dc1e345ab67e..39ab32fed3f4 100644 --- a/drivers/usb/host/xhci-pci.c +++ b/drivers/usb/host/xhci-pci.c @@ -635,7 +635,7 @@ static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id) pm_runtime_put_noidle(&dev->dev); if (pci_choose_state(dev, PMSG_SUSPEND) == PCI_D0) - pm_runtime_forbid(&dev->dev); + pm_runtime_disable(&dev->dev); else if (xhci->quirks & XHCI_DEFAULT_PM_RUNTIME_ALLOW) pm_runtime_allow(&dev->dev);
Change pm_runtime_forbid to pm_runtime_disable to disable RPM on platforms that don't support runtime D3, as re-enabling it through sysfs auto power control may cause the controller to malfunction. This can lead to issues such as hotplug devices not being detected due to failed interrupt generation. Fixes: a5d6264b638e ("xhci: Enable RPM on controllers that support low-power states") Suggested-by: Mario Limonciello <mario.limonciello@amd.com> Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com> --- drivers/usb/host/xhci-pci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)