Re: [PATCH v2] i2c: xiic: restore runtime PM teardown in remove to fix clk WARN flood

Andi Shyti <[email protected]>
Newsgroups org.kernel.vger.linux-i2c,org.infradead.lists.linux-arm-kernel,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
Hi Abdurraham,

> On Sun, Aug 16, 2026 at 06:14:08PM +0200, Andi Shyti wrote:
> > isn't devm_pm_runtime_set_active_enabled() calling
> > pm_runtime_disable() + pm_runtime_set_suspended() +
> > pm_runtime_dont_use_autosuspend() at teardown?
> 
> It does call all three, but the ordering defeats it, in two places:
> 
> 1. Within the helper's own release, pm_runtime_disable_action() calls
>    pm_runtime_dont_use_autosuspend() *before* pm_runtime_disable() --
>    i.e. while runtime PM is still enabled.
> 
> 2. devres is LIFO, and devm_clk_get_enabled() is registered before
>    devm_pm_runtime_set_active_enabled() in probe(), so the clock's
>    clk_disable_unprepare() release runs *after* the runtime-PM releases.
> 
> pm_runtime_put_sync() in remove() leaves the device RPM_ACTIVE with the
> autosuspend timer armed: rpm_idle() falls through to
> rpm_suspend(RPM_AUTO), the delay hasn't expired, so it just re-arms the
> timer -- runtime_status stays ACTIVE and the clock stays enabled.
> 
> Then the devres teardown runs. pm_runtime_dont_use_autosuspend() clears
> use_autosuspend; update_autosuspend() takes the else branch and calls
> rpm_idle(RPM_AUTO), but now pm_runtime_autosuspend_expiration() returns
> 0, so rpm_suspend() no longer defers and suspends immediately --
> xiic_i2c_runtime_suspend() clk_disable()s the clock, all while runtime
> PM is still enabled. A moment later, further down the LIFO chain,
> devm_clk_get_enabled()'s release does clk_disable_unprepare() on the
> already-disabled clock and clk_core_disable() WARNs, once per device.
> 
> Calling pm_runtime_disable() in remove(), before any devres release
> runs, closes that window: with runtime PM disabled the helper's
> dont_use_autosuspend() can no longer trigger a suspend, so the clock
> stays enabled and the clk release balances it cleanly.
> 
> You're right that set_suspended() and dont_use_autosuspend() are then
> redundant with the helper -- pm_runtime_disable() is the only call that
> has to happen early. So for v3 I'll drop those two and keep just
> pm_runtime_disable() (with a short comment, and the full reasoning in
> the commit log). Does that work for you?

I understand the logic, but this still leaves runtime PM
unbalanced. The explicit pm_runtime_disable() is followed by the
managed cleanup calling pm_runtime_disable() again.

A solution would be to drop the managed runtime-PM helper and
restore the non managed runtime PM setup and teardown. Otherwise,
this ordering issue needs to be addressed in the runtime-PM/clock
framework.

Does that work?

Andi
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.