Re: [PATCH] xhci: Change pm_runtime_forbid to pm_runtime_disable

[Mathias Nyman <mathias.nyman@linux.intel.com>]

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