Re: [PATCH] xhci: Change pm_runtime_forbid to pm_runtime_disable

[Johan Hovold <johan@kernel.org>]

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