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

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

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.

-- 
With Best Regards,
Andy Shevchenko