Re: [PATCH] iio: magnetometer: ak8975: ensure device is awake for buffered capture

[Jonathan Cameron <jic23@kernel.org>]

On Wed, 13 May 2026 16:35:52 +0200
Joshua Crofts via B4 Relay <devnull+joshua.crofts1.gmail.com@kernel.org> wrote:

> From: Joshua Crofts <joshua.crofts1@gmail.com>
> 
> Currently, the ak8975_start_read_axis() can be called while the device
> is autosuspended, causing two issues:
> 
> 1. I2C transfers in the aforementioned function will fail or timeout
> because ak8975_runtime_suspend() disables the device regulators.
> 2. Since ak8975_fill_buffer() does not hold runtime references,
> ak8975_runtime_suspend() can run concurrently, and since PM callbacks
> do not use a locking mechanism, it may cause a race accessing the
> control register via the I2C bus.
> 
> Fix this issue by adding struct iio_buffer_setup_ops that contains
> preenable and postdisable functions to ensure correct that device is
> powered on when running a buffered capture.
> 
> Fixes: bc11ca4a0b84 ("iio:magnetometer:ak8975: triggered buffer support")
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://sashiko.dev/#/patchset/20260511-magnetometer-fixes-post-pickup-v7-0-9d910faa28b6%40gmail.com
> Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
I'm going to take this as a clear improvement but the point sashiko makes
about turning runtime PM on only after the interfaces are exposed is possibly valid
as a general comment.  It applies to all the other paths though, not just
the buffered capture.

I believe we are actually fine here because the device is powered up
at that point (the set_active making that clear).

Open to other people's views on this though.

This is another place where we really should work out a best practice
guide / set of standard patterns to use.

Anyhow, applied this one and this is one I've marked for stable.
However given mass of stuff around this driver it'll still go the slow
way and we may need to manually do backports.  So applied to the
testing branch of iio.git.

Thanks,

Jonathan


> ---
>  drivers/iio/magnetometer/ak8975.c | 24 +++++++++++++++++++++++-
>  1 file changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
> index dbab4d0bba348be70e5c8b71678de935879bff42..0fb2fd03d11ce195a67949056c000c473a8d3e99 100644
> --- a/drivers/iio/magnetometer/ak8975.c
> +++ b/drivers/iio/magnetometer/ak8975.c
> @@ -899,6 +899,28 @@ static irqreturn_t ak8975_handle_trigger(int irq, void *p)
>  	return IRQ_HANDLED;
>  }
>  
> +static int ak8975_buffer_preenable(struct iio_dev *indio_dev)
> +{
> +	struct ak8975_data *data = iio_priv(indio_dev);
> +	struct device *dev = &data->client->dev;
> +
> +	return pm_runtime_resume_and_get(dev);
> +}
> +
> +static int ak8975_buffer_postdisable(struct iio_dev *indio_dev)
> +{
> +	struct ak8975_data *data = iio_priv(indio_dev);
> +	struct device *dev = &data->client->dev;
> +
> +	pm_runtime_put_autosuspend(dev);
> +
> +	return 0;
> +}
> +
> +static const struct iio_buffer_setup_ops ak8975_buffer_setup_ops = {
> +	.preenable = ak8975_buffer_preenable,
> +	.postdisable = ak8975_buffer_postdisable,
> +};
>  static int ak8975_probe(struct i2c_client *client)
>  {
>  	const struct i2c_device_id *id = i2c_client_get_device_id(client);
> @@ -992,7 +1014,7 @@ static int ak8975_probe(struct i2c_client *client)
>  	indio_dev->name = name;
>  
>  	ret = iio_triggered_buffer_setup(indio_dev, NULL, ak8975_handle_trigger,
> -					 NULL);
> +					 &ak8975_buffer_setup_ops);
>  	if (ret) {
>  		dev_err(&client->dev, "triggered buffer setup failed\n");
>  		goto power_off;
> 
> ---
> base-commit: 1548c54e9adc32a719499216f63fba14b2fc07c3
> change-id: 20260513-ak8975-fix-53b1b02857dd
> 
> Best regards,