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

[Kunihiko Hayashi]

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