Linux IIO development
 help / color / mirror / Atom feed
* [PATCH] iio: magnetometer: ak8975: ensure device is awake for buffered capture
@ 2026-05-13 14:35 Joshua Crofts via B4 Relay
  2026-05-13 20:40 ` Andy Shevchenko
  0 siblings, 1 reply; 3+ messages in thread
From: Joshua Crofts via B4 Relay @ 2026-05-13 14:35 UTC (permalink / raw)
  To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
	Gregor Boirie
  Cc: linux-iio, linux-kernel, Sashiko, Joshua Crofts

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>



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] iio: magnetometer: ak8975: ensure device is awake for buffered capture
  2026-05-13 14:35 [PATCH] iio: magnetometer: ak8975: ensure device is awake for buffered capture Joshua Crofts via B4 Relay
@ 2026-05-13 20:40 ` Andy Shevchenko
  2026-05-14  7:32   ` Joshua Crofts
  0 siblings, 1 reply; 3+ messages in thread
From: Andy Shevchenko @ 2026-05-13 20:40 UTC (permalink / raw)
  To: joshua.crofts1
  Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
	Gregor Boirie, linux-iio, linux-kernel, Sashiko

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



^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] iio: magnetometer: ak8975: ensure device is awake for buffered capture
  2026-05-13 20:40 ` Andy Shevchenko
@ 2026-05-14  7:32   ` Joshua Crofts
  0 siblings, 0 replies; 3+ messages in thread
From: Joshua Crofts @ 2026-05-14  7:32 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
	Gregor Boirie, linux-iio, linux-kernel, Sashiko

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-05-14  7:32 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-13 14:35 [PATCH] iio: magnetometer: ak8975: ensure device is awake for buffered capture Joshua Crofts via B4 Relay
2026-05-13 20:40 ` Andy Shevchenko
2026-05-14  7:32   ` Joshua Crofts

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox