On 10/25/23 08:41, Tony Lindgren wrote:
> * Kevin Hilman <khilman@xxxxxxxxxx> [231024 18:36]:
>> Tony Lindgren <tony@xxxxxxxxxxx> writes:
>>
>>> * Kevin Hilman <khilman@xxxxxxxxxx> [231023 21:31]:
>>>> Instead, what should be happening is that when `no_console_suspend` is
>>>> set, there should be an extra pm_runtime_get() which increases the
>>>> device usecount such that the device never runtime suspends, and thus
>>>> the domain will not get powered off.
>>>
>>> We already have the runtime PM usage count kept in the driver (unless
>>> there's a bug somewhere). The issue is that on suspend the power domain
>>> still gets shut down.
>>>
>>> I suspect that some of the SoC power domains get
>>> force shut down on suspend somewhere?
>>
>> If setting GENPD_FLAG_ALWAYS_ON works as this patch proposes, then a
>> force shutdown would override that genpd flag also, so I suspect the
>> runtime PM usage count is not correct.
>
> OK good point.
>
>> I quick skim of 8250_omap.c, and I don't see any pm_runtime_get() calls
>> that are conditional on no_console_suspend, which is what I would
>> suspect for the domain to stay on.
>
> If a serial console is attached, we now have runtime PM usage count
> always kept. Users can detach the console via sysfs as needed. See these
> two earlier commits from Andy:
>
> a3cb39d258ef ("serial: core: Allow detach and attach serial device for console")
> bedb404e91bb ("serial: 8250_port: Don't use power management for kernel console")
>
> Sounds like there's a bug somewhere. It's worth verifying if the runtime
> PM usage count is kept for 8250_omap on suspend.
>
> Thomas, care to check your test case with the attached debug hack
> and by adding a call for pm_runtime_get_usage_count() on the suspend
> path?
Hi Tony,
Please find below the logs of the test you asked me.
I added the call of pm_runtime_get_usage_count at the end of the suspend
function.
The console is attached on 2800000.serial, it has usage_count=4.
Other serial has usage_count=3.
[ 4.859058] port 2830000.serial:0.0: PM: calling
pm_runtime_force_suspend+0x0/0x134 @ 114, parent: 2830000.serial:0
[ 4.869478] port 2830000.serial:0.0: PM:
pm_runtime_force_suspend+0x0/0x134 returned 0 after 0 usecs
[ 4.878602] omap8250 2830000.serial: PM: calling
omap8250_suspend+0x0/0x144 @ 114, parent: bus@100000
[ 4.887813] omap8250 2830000.serial: omap8250_suspend: 1634:
usage_count = 3
[ 4.894851] omap8250 2830000.serial: PM: omap8250_suspend+0x0/0x144
returned 0 after 7042 usecs
[ 4.903538] port 2810000.serial:0.0: PM: calling
pm_runtime_force_suspend+0x0/0x134 @ 114, parent: 2810000.serial:0
[ 4.913957] port 2810000.serial:0.0: PM:
pm_runtime_force_suspend+0x0/0x134 returned 0 after 0 usecs
[ 4.923080] omap8250 2810000.serial: PM: calling
omap8250_suspend+0x0/0x144 @ 114, parent: bus@100000
[ 4.932288] omap8250 2810000.serial: omap8250_suspend: 1634:
usage_count = 3
[ 4.939323] omap8250 2810000.serial: PM: omap8250_suspend+0x0/0x144
returned 0 after 7038 usecs
[ 4.948010] port 2800000.serial:0.0: PM: calling
pm_runtime_force_suspend+0x0/0x134 @ 114, parent: 2800000.serial:0
[ 4.958433] port 2800000.serial:0.0: PM:
pm_runtime_force_suspend+0x0/0x134 returned 0 after 1 usecs
[ 4.967557] omap8250 2800000.serial: PM: calling
omap8250_suspend+0x0/0x144 @ 114, parent: bus@100000
[ 4.976764] omap8250 2800000.serial: omap8250_suspend: 1634:
usage_count = 4
[ 4.983799] omap8250 2800000.serial: PM: omap8250_suspend+0x0/0x144
returned 0 after 7036 usecs
[ 4.992488] port 40a00000.serial:0.0: PM: calling
pm_runtime_force_suspend+0x0/0x134 @ 114, parent: 40a00000.serial:0
[ 5.003081] port 40a00000.serial:0.0: PM:
pm_runtime_force_suspend+0x0/0x134 returned 0 after 0 usecs
[ 5.012291] omap8250 40a00000.serial: PM: calling
omap8250_suspend+0x0/0x144 @ 114, parent: bus@100000:bus@28380000
[ 5.022714] omap8250 40a00000.serial: omap8250_suspend: 1634:
usage_count = 3
[ 5.029836] omap8250 40a00000.serial: PM: omap8250_suspend+0x0/0x144
returned 0 after 7124 usecs
Regards,
Thomas
8< -------------------------------
diff --git a/drivers/tty/serial/8250/8250_omap.c
b/drivers/tty/serial/8250/8250_omap.c
index ca972fd37725..b978f12fd542 100644
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -1631,6 +1631,9 @@ static int omap8250_suspend(struct device *dev)
err = pm_runtime_force_suspend(dev);
flush_work(&priv->qos_work);
+ dev_info(dev, "%s: %d: usage_count = %d\n",
+ __func__, __LINE__, pm_runtime_get_usage_count(dev));
+
return err;
}
>
> Regards,
>
> Tony
>
> 8< -------------------------------
> diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h
> --- a/include/linux/pm_runtime.h
> +++ b/include/linux/pm_runtime.h
> @@ -129,6 +129,11 @@ static inline void pm_runtime_get_noresume(struct device *dev)
> atomic_inc(&dev->power.usage_count);
> }
>
> +static inline int pm_runtime_get_usage_count(struct device *dev)
> +{
> + return atomic_read(&dev->power.usage_count);
> +}
> +
> /**
> * pm_runtime_put_noidle - Drop runtime PM usage counter of a device.
> * @dev: Target device.
