Re: [PATCH v3 5/5] iio: magnetometer: qmc5883p: add PM support

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

On Sun, Apr 19, 2026 at 10:33:06PM +0000, Hardik Phalet wrote:
> Add runtime PM with a 2 s autosuspend delay. Per datasheet §6.2.1
> the chip continuously samples in MODE_NORMAL; putting it into
> MODE_SUSPEND when idle drops current from up to 1180 uA to ~22 uA
> (datasheet Table 2).
> 
> Wrap qmc5883p_get_measure() and qmc5883p_write_raw() with
> pm_runtime_resume_and_get() / pm_runtime_put_autosuspend(), converting
> early returns to a goto so the put is always paired.
> 
> System sleep is delegated to the runtime callbacks via
> SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, pm_runtime_force_resume).
> 
> A devm action is registered before devm_pm_runtime_enable() so that
> LIFO teardown on unbind runs pm_runtime_disable() first (freezing PM
> state) and then suspends the hardware via MODE_SUSPEND.

...

> +	ret = pm_runtime_resume_and_get(data->dev);
> +	if (ret < 0)
> +		return ret;

Don't you want to use PM_RUNTIME_ACQUIRE_AUTOSUSPEND()?

...

>  	guard(mutex)(&data->mutex);
>  
> -	ret = regmap_field_write(data->rf.mode, QMC5883P_MODE_SUSPEND);
> +	ret = pm_runtime_resume_and_get(data->dev);
>  	if (ret)
>  		return ret;
>  
> +	ret = regmap_field_write(data->rf.mode, QMC5883P_MODE_SUSPEND);
> +	if (ret)
> +		goto out;

No way, please, read what is written in the top of cleanup.h, also use
the advice I gave above.

> +out:
> +	pm_runtime_put_autosuspend(data->dev);
>  	return ret;
>  }

...

> +static const struct dev_pm_ops qmc5883p_dev_pm_ops = {
> +	SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, pm_runtime_force_resume)
> +		RUNTIME_PM_OPS(qmc5883p_runtime_suspend,
> +			       qmc5883p_runtime_resume, NULL)

Besides broken indentation, use proper macro from DEFINE_PM_*() family:
DEFINE_RUNTIME_DEV_PM_OPS().

> +};

-- 
With Best Regards,
Andy Shevchenko