Re: [PATCH togreg v2 2/2] iio: imu: inv_icm42607: restore runtime PM on system resume errors

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

On Tue, Aug 11, 2026 at 10:03:45AM +0800, Linmao Li wrote:
> pm_runtime_force_suspend() leaves runtime PM disabled after it succeeds and
> expects pm_runtime_force_resume() to restore runtime PM management during
> system resume.
> 
> The resume callback returns early if enabling the vddio regulator or
> synchronizing the register cache fails, skipping the matching
> pm_runtime_force_resume() call. Runtime PM consequently remains disabled
> after the system has resumed, so runtime autosuspend can no longer turn off
> sensors enabled afterward.
> 
> Call pm_runtime_force_resume() on both error paths. Keep the first error as
> the return value and report a runtime PM restore failure separately.

...

>  	/* Sync the regcache again after regulator shutdown. */
>  	regcache_mark_dirty(st->map);
> -	ret = regcache_sync(st->map);
> -	if (ret)

> +

I wouldn't add this blank line as these two are quite coupled. OTOH it's a
better style, so I leave it to Jonathan and others to decide.

> +	return regcache_sync(st->map);
> +}

...

> +static int inv_icm42607_resume(struct device *dev)
> +{
> +	struct inv_icm42607_state *st = dev_get_drvdata(dev);
> +	int resume_ret;
> +	int ret;
> +
> +	ret = inv_icm42607_resume_core(st);
> +
> +	resume_ret = pm_runtime_force_resume(dev);
> +	if (ret) {

I still don't get the logic here. Shouldn't we rather call the force_suspend()
last in the .suspend() and force_resume() first here?

> +		if (resume_ret)
> +			dev_warn(dev, "Failed to restore runtime PM state: %d\n",
> +				 resume_ret);
> +
>  		return ret;
> +	}
>  
> -	return pm_runtime_force_resume(dev);
> +	return resume_ret;
>  }

Okay, after reading other drivers I think the above can be written in a bit better form.

	struct inv_icm42607_state *st = dev_get_drvdata(dev);
	int ret;

	ret = inv_icm42607_resume_core(st);
	if (ret) {
		int rc;

		rc = pm_runtime_force_resume(dev);
		if (rc)
			dev_warn(dev, "Failed to restore runtime PM state: %d\n", rc);

// and yes, I would go with longer line here (that's why I renamed variable)

		return ret;
	}

	return pm_runtime_force_resume(dev);

It seems that if the above analysis is true (I haven't deeply checked that),
there are many drivers in the kernel suffer from the very same issue (when they
need to do some work before runtime PM resume may be called from inside the
system .resume() callback.

-- 
With Best Regards,
Andy Shevchenko