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

[Abdurrahman Hussain]

On Tue Aug 18, 2026 at 11:48 PM PDT, Andy Shevchenko wrote:
> On Wed, Aug 19, 2026 at 09:45:41AM +0300, Andy Shevchenko wrote:
>> On Tue, Aug 18, 2026 at 08:35:10AM -0700, Abdurrahman Hussain wrote:
>
> ...
>
>> >  	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)?
>
> (To be clear: I'm talking about next call(s) that may fail, like xiic_reinit()
>  later in the probe.)
>
>> The rule of thumb is that, no devm_*() call should be followed by a goto.

Makes sense. I haven't hit it on our hardware yet, but I understand the
concern: once the probe unwinds runtime PM by hand via goto, the
devm-registered handler stays live across that teardown (and across any
future failing step added after it). I'll switch to
request_threaded_irq()/free_irq() and free it explicitly in the probe
error path and in remove() for v4.

Thanks for careful review!

Best regards,
Abdurrahman