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

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

[Joshua Crofts via B4 Relay]

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>
---
 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,
-- 
Joshua Crofts <joshua.crofts1@gmail.com>

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

[Andy Shevchenko]

On Wed, May 13, 2026 at 04:35:52PM +0200, Joshua Crofts via B4 Relay wrote:
> 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.

I think Suggested-by: Jonathan is appropriate here.

...

> +static int ak8975_buffer_preenable(struct iio_dev *indio_dev)
> +{
> +	struct ak8975_data *data = iio_priv(indio_dev);
> +	struct device *dev = &data->client->dev;

Not sure if actually may use indio_dev->dev.parent here instead of this, but at
least this one is robust against changing parent-child relations).

> +	return pm_runtime_resume_and_get(dev);
> +}

-- 
With Best Regards,
Andy Shevchenko

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

[Joshua Crofts]

On Wed, 13 May 2026 at 22:40, Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
>
> On Wed, May 13, 2026 at 04:35:52PM +0200, Joshua Crofts via B4 Relay wrote:
> > 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.
>
> I think Suggested-by: Jonathan is appropriate here.
>
> ...
>
> > +static int ak8975_buffer_preenable(struct iio_dev *indio_dev)
> > +{
> > +     struct ak8975_data *data = iio_priv(indio_dev);
> > +     struct device *dev = &data->client->dev;
>
> Not sure if actually may use indio_dev->dev.parent here instead of this, but at
> least this one is robust against changing parent-child relations).

It being robust was my intention, considering this will be backported at some
point.

-- 
Kind regards

CJD

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

[Joshua Crofts]

On Wed, 13 May 2026 at 16:35, 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>
> ---

FYI, Sashiko reported an additional issue concerning the leaking
of uninitialized kernel stack memory.

https://sashiko.dev/#/patchset/20260513-ak8975-fix-v1-1-104ea605dd54%40gmail.com

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

[Jonathan Cameron]

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,

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

[Joshua Crofts]

On Fri, 15 May 2026 at 15:17, Jonathan Cameron <jic23@kernel.org> wrote:
>
> 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.

Ah, I forgot to send the v2 with your Suggested-by tag. Up to you to amend
it I guess.

Thanks

-- 
Kind regards

CJD

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

[Jonathan Cameron]

On Fri, 15 May 2026 15:22:42 +0200
Joshua Crofts <joshua.crofts1@gmail.com> wrote:

> On Fri, 15 May 2026 at 15:17, Jonathan Cameron <jic23@kernel.org> wrote:
> >
> > 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.  
> 
> Ah, I forgot to send the v2 with your Suggested-by tag. Up to you to amend
> it I guess.
I'm lazy and don't care + don't resend to add tags if there is nothing
else. Just rely to the thread with it then b4 will pick it up.
I'll have to tweak the order though as it'll put it at the end of
the tag block.
> 
> Thanks
>