On 31.08.2021 22:43, Heiner Kallweit wrote:
> On 31.08.2021 13:26, Jean Delvare wrote:
>> On Tue, 31 Aug 2021 08:05:41 +0200, Heiner Kallweit wrote:
>>> On 26.08.2021 16:00, Jean Delvare wrote:
>>>> If pm_runtime_set_autosuspend_delay() is not suitable for the task then
>>>> maybe we need a better API. I will admit I'm at a loss when it comes to
>>>> the many pm_runtime_* calls, I'm not going to claim I fully understand
>>>> what each of them is doing exactly. But don't we want to simply call
>>>> pm_runtime_dont_use_autosuspend() here?
>>>>
>>>> If not and there's no suitable API for the task at the moment, then
>>>> better do not apply this patch, and instead ask the PM subsystem
>>>> maintainers if they would be willing to implement what we need.
>>>
>>> To follow-up on this: This patch has been applied already. Therefore,
>>> if decision is to not go with it, it would need to be reverted.
>>
>> Technically it's not in Linus' tree yet ;-)
>>
>> I'm still interested to know if pm_runtime_dont_use_autosuspend() is
>> the right call to use in this situation.
>>
> I don't think so. It disable auto-suspending, but leaves "normal"
> runtime-suspending active. Calling pm_runtime_disable() may be an
> alternative.
> Or we use the following to re-establish the old behavior with a little
> less overhead. Getting the mutex isn't needed here because the PCI
> core increments the rpm usage_count before calling the remove() hook.
>
Just figured out that what I proposed wasn't fully correct. We should
only access priv->acpi_reserved once we're sure the ACPI io handler
can't run in parallel.
Small disclaimer: I'm not fully sure how acpi_remove_address_space_handler()
behaves if it's called whilst the handler is running.
IOW: Whether we can be sure that after the call to acpi_remove_address_space_handler()
the handler isn't running.
On a sidenote:
At least the call to pm_runtime_forbid() isn't needed because runtime pm
is disabled anyway and the calls to pm_runtime_forbid/allow don't
have to be balanced.
diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index 1f929e6c3..6394c8340 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -1623,7 +1623,7 @@ i801_acpi_io_handler(u32 function, acpi_physical_address address, u32 bits,
* BIOS is accessing the host controller so prevent it from
* suspending automatically from now on.
*/
- pm_runtime_set_autosuspend_delay(&pdev->dev, -1);
+ pm_runtime_get_sync(&pdev->dev);
}
if ((function & ACPI_IO_MASK) == ACPI_READ)
@@ -1890,9 +1890,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);
i801_del_mux(priv);
i2c_del_adapter(&priv->adapter);
@@ -1901,6 +1898,10 @@ static void i801_remove(struct pci_dev *dev)
platform_device_unregister(priv->tco_pdev);
+ pm_runtime_forbid(&dev->dev);
+ /* if acpi_reserved is set then usage_count is incremented already */
+ if (!priv->acpi_reserved)
+ pm_runtime_get_noresume(&dev->dev);
/*
* do not call pci_disable_device(dev) since it can cause hard hangs on
* some systems during power-off (eg. Fujitsu-Siemens Lifebook E8010)
--
2.33.0
