Re: [PATCH 01/10] i2c: i801: Don't call pm_runtime_allow

[Jarkko Nikula <jarkko.nikula@linux.intel.com>]

Hi

On 8/2/21 7:31 PM, Heiner Kallweit wrote:
> On 02.08.2021 14:53, Jean Delvare wrote:
>> Hi Heiner,
>>
>> On Sun, 01 Aug 2021 16:16:56 +0200, Heiner Kallweit wrote:
>>> Drivers should not call pm_runtime_allow(), see
>>> Documentation/power/pci.rst. Therefore remove the call and leave this
>>> to user space. Also remove the not needed call to pm_runtime_forbid().
>>>
>>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>>> ---
>>>   drivers/i2c/busses/i2c-i801.c | 2 --
>>>   1 file changed, 2 deletions(-)
>>>
>>> diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
>>> index 92ec291c0..362e74761 100644
>>> --- a/drivers/i2c/busses/i2c-i801.c
>>> +++ b/drivers/i2c/busses/i2c-i801.c
>>> @@ -1891,7 +1891,6 @@ static int i801_probe(struct pci_dev *dev, const struct pci_device_id *id)
>>>   	pm_runtime_set_autosuspend_delay(&dev->dev, 1000);
>>>   	pm_runtime_use_autosuspend(&dev->dev);
>>>   	pm_runtime_put_autosuspend(&dev->dev);
>>> -	pm_runtime_allow(&dev->dev);
>>>   
>>>   	return 0;
>>>   }
>>> @@ -1900,7 +1899,6 @@ static void i801_remove(struct pci_dev *dev)
>>>   {
>>>   	struct i801_priv *priv = pci_get_drvdata(dev);
>>>   
>>> -	pm_runtime_forbid(&dev->dev);
>>>   	pm_runtime_get_noresume(&dev->dev);
>>>   
>>>   	i801_disable_host_notify(priv);
>>
>> These calls were added by Jarkko (Cc'd) and I'm not familiar with power
>> management so I'll need an explicit ack from him before I can accept
>> this patch.
>>
> The calls were part of the initial submission for rpm support and supposedly
> just copied from another driver. But fine with me to wait for his feedback.
> 
Yes, I'm quite sure I've copied it from another driver :-)

This patch will cause the device here won't go automatically to D3 
before some user space script allows it. E.g

echo auto > /sys/bus/pci/devices/0000\:00\:1f.3/power/control

I think this is kind of PM regression with this patch. It's not clear to 
me from the Documentation/power/pci.rst why driver should not call the 
pm_runtime_allow() and what would be allowed kernel alternative for it.

Rafael: what would be the correct way here to allow runtime PM from the 
driver or does it really require some user space script for it?

Jarkko