Re: [PATCH v3] i2c: xiic: restore non-managed runtime PM to fix clk WARN flood

On Tue, Aug 18, 2026 at 08:35:10AM -0700, Abdurrahman Hussain wrote:
> The devres conversion replaced manual pm_runtime_enable()/disable() with
> devm_pm_runtime_set_active_enabled() and dropped the remove-time runtime
> PM teardown. The managed release tears runtime PM down in the wrong
> order: it calls pm_runtime_dont_use_autosuspend() before
> pm_runtime_disable(), i.e. while runtime PM is still enabled, and devres
> is LIFO so the devm_clk_get_enabled() release runs afterwards.
> 
> At remove(), pm_runtime_put_sync() leaves the device active with the
> autosuspend timer armed. Clearing use_autosuspend then makes rpm_idle()
> suspend immediately, and xiic_i2c_runtime_suspend() clk_disable()s the
> clock. The later devm_clk_get_enabled() release clk_disable_unprepare()s
> the already-disabled clock, so clk_core_disable() WARNs ("clkN already
> disabled") on every teardown.
> 
> Drop the managed helper and restore the non-managed runtime PM setup and
> teardown, so runtime PM is enabled once in probe and disabled once in
> remove and the clock enable count stays balanced.

...

>  	pm_runtime_set_autosuspend_delay(dev, XIIC_PM_TIMEOUT);
>  	pm_runtime_use_autosuspend(dev);
> -	ret = devm_pm_runtime_set_active_enabled(dev);
> -	if (ret)
> -		return ret;
> +	/*
> +	 * Enable runtime PM by hand: devm_pm_runtime_set_active_enabled()
> +	 * tears down in an order that races the devm-enabled clock release and
> +	 * makes clk_core_disable() WARN (see xiic_i2c_remove()).
> +	 */
> +	pm_runtime_set_active(dev);
> +	pm_runtime_enable(dev);
>  
>  	/* SCL frequency configuration */
>  	i2c->input_clk = clk_get_rate(i2c->clk);

>  	ret = devm_request_threaded_irq(dev, irq, NULL, xiic_process,
>  					IRQF_ONESHOT, pdev->name, i2c);
>  	if (ret)
> -		return ret;
> +		goto err_pm_disable;

This might be problematic now. You need to unwind the IRQ request in non-devm
manner as well. Scenario is that IRQ comes exactly after PM is disabled
in the error path. Is it a problem today? What about tomorrow (assuming some
new chips / code is added)?

The rule of thumb is that, no devm_*() call should be followed by a goto.

-- 
With Best Regards,
Andy Shevchenko