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

Andy Shevchenko
2026-04-20T09:52:32Z

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