Re: [PATCH v2] serial: 8250_uniphier: Use devm_clk_get_enabled()

[Kunihiko Hayashi <hayashi.kunihiko@socionext.com>]

On 2026/09/24 23:09, Andy Shevchenko wrote:
> On Thu, Sep 24, 2026 at 06:07:37PM +0900, Kunihiko Hayashi wrote:
>> On 2026/09/17 13:05, Malathi A wrote:
>>> uniphier_uart_probe() enables priv->clk by hand and is then
> responsible
>>> for disabling it again on every error path. It misses one: when
>>> uart_read_port_properties() fails the function returns directly,
> leaving
>>> the clock prepared and enabled. The neighbouring path, taken when
>>> serial8250_register_8250_port() fails, does call
> clk_disable_unprepare(),
>>> which shows the leak is an oversight rather than intent.
>>>
>>> Rather than adding another manual unwind, switch to
>>> devm_clk_get_enabled() so the clock is released by devres. That
> removes
>>> the error path entirely, along with the explicit disables in the
>>> register failure path and in uniphier_uart_remove().
>>>
>>> The clock is still disabled and re-enabled by hand across system
> sleep,
>>> which stays balanced: only SET_SYSTEM_SLEEP_PM_OPS is used, so the
>>> device cannot be unbound while suspended and devres always sees an
>>> enabled clock at detach.
> 
> ...
> 
>> I'm a bit concerned about the error path in uniphier_uart_resume().
>> The clock may have been disabled in uniphier_uart_suspend(), and
>> clk_prepare_enable() can fail when trying to enable it again.
>>
>> In that case, wouldn't devres try to disable an already disabled
>> clock on detach?
> 
> Isn't there is a guarantee that the .remove() is called with PM runtime on
> and
> get, meaning it's called with the enabled clock? But if not the case, we
> might
> use PM runtime force operations (resume and suspend in the respective
> cases.
> 

My concern was just the case where clk_prepare_enable() in the system
resume callback fails, leaving the clock disabled before devres cleanup.

This driver currently only uses SET_SYSTEM_SLEEP_PM_OPS(), so I wasn't
considering runtime PM here.

I don't have a strong opinion on whether runtime PM should be involved,
but if it guarantees that the clock is enabled before .remove(),
that would address my concern.

Thank you,

---
Best Regards
Kunihiko Hayashi