[PATCH v5 00/18] iio: magnetometer: ak8975: modernize and cleanup driver

[PATCH v5 00/18] iio: magnetometer: ak8975: modernize and cleanup driver

[Joshua Crofts via B4 Relay]

This series modernizes and cleans up the ak8975 driver, migrating to
more modern kernel APIs and cleaning up legacy code. Patches 7 through
17 were provided by Andy Shevchenko.

The series was compile-tested and ran in QEMU with i2c-stub and
no issues were found.

Changes include:
 - replacing usleep_range() with fsleep() for optimization
 - migrating to iopoll macros for better device polling
 - removing unused headers and adding new ones
 - checkpatch.pl fixes
 - switching driver to use devm_* functions

Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
---
Changes in v2:
- PATCH 1: reverted alphabetical header ordering per logical change
  separation
- PATCH 2: added newline between fsleep() and return
- PATCH 3: fixed open parenthesis formatting
- PATCH 4: fixed whitespace issues, added <linux/time.h> for
  USEC_PER_MSEC macro

Changes in v3:
- PATCH 1 & 2: split alphabetical ordering and header removal into two
  separate patches
- PATCH 4: commit message fix
- PATCH 5 & 6: split error handling and polling loop replacement into
  two separate patches

Changes in v4:
- PATCH 7-17: am Andy's changes into this series
- PATCH 2, 5, 6: fixup Andy's changes to existing patches

Changes in v5:
- PATCH 6 & 7: reorder patch series, edit commit messages
- PATCH 8: add check for negative val from readx_*() call
- Various cleanups

---

---
Andy Shevchenko (10):
      iio: magnetometer: ak8975: avoid using temporary variable
      iio: magnetometer: ak8975: drop duplicate NULL check
      iio: magnetometer: ak8975: remove duplicate error message
      iio: magnetometer: ak8975: reduce usage of magic lengths of the buffer
      iio: magnetometer: ak8975: unify return code variable name
      iio: magnetometer: ak8975: switch to using managed resources
      iio: magnetometer: ak8975: consistently use 'data' parameter
      iio: magnetometer: ak8975: unify messages with help of dev_err_probe()
      iio: magnetometer: ak8975: use temporary variable for struct device
      iio: magnetometer: ak8975: make use of the macros from bits.h

Joshua Crofts (8):
      iio: magnetometer: ak8975: sort headers alphabetically
      iio: magnetometer: ak8975: update headers per IWYU principle
      iio: magnetometer: ak8975: replace usleep_range() with fsleep()
      iio: magnetometer: ak8975: change 'u8*' to 'u8 *' in cast
      iio: magnetometer: ak8975: fix wrong errno on return
      iio: magnetometer: ak8975: pass conversion timeouts as arguments
      iio: magnetometer: ak8975: modernize polling loops with iopoll() macros
      iio: magnetometer: ak8975: check if gpiod read was successful

 drivers/iio/magnetometer/ak8975.c | 369 +++++++++++++++++++-------------------
 1 file changed, 180 insertions(+), 189 deletions(-)
---
base-commit: 440c14da1af1cc6cc9bb18841603711d845ed900
change-id: 20260505-magnetometer-fixes-d3a96180ab4c

Best regards,
-- 
Joshua Crofts <joshua.crofts1@gmail.com>

[PATCH v5 01/18] iio: magnetometer: ak8975: sort headers alphabetically

[Joshua Crofts via B4 Relay]

From: Joshua Crofts <joshua.crofts1@gmail.com>

Sort include headers alphabetically to improve coding style and
readability.

No functional change.

Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
---
 drivers/iio/magnetometer/ak8975.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
index 44782c26698bcac5ebfc62ce3740f361078fd0ce..569cd6fa839aec6ee8758a261d0e659fc20131c6 100644
--- a/drivers/iio/magnetometer/ak8975.c
+++ b/drivers/iio/magnetometer/ak8975.c
@@ -7,23 +7,23 @@
  * Copyright (c) 2010, NVIDIA Corporation.
  */
 
-#include <linux/module.h>
-#include <linux/mod_devicetable.h>
-#include <linux/kernel.h>
-#include <linux/slab.h>
+#include <linux/bitops.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/gpio/consumer.h>
 #include <linux/i2c.h>
 #include <linux/interrupt.h>
-#include <linux/err.h>
+#include <linux/kernel.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
 #include <linux/mutex.h>
-#include <linux/delay.h>
-#include <linux/bitops.h>
-#include <linux/gpio/consumer.h>
-#include <linux/regulator/consumer.h>
 #include <linux/pm_runtime.h>
+#include <linux/regulator/consumer.h>
+#include <linux/slab.h>
 
+#include <linux/iio/buffer.h>
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
-#include <linux/iio/buffer.h>
 #include <linux/iio/trigger.h>
 #include <linux/iio/trigger_consumer.h>
 #include <linux/iio/triggered_buffer.h>

-- 
2.47.3

Re: [PATCH v5 01/18] iio: magnetometer: ak8975: sort headers alphabetically

[Maxwell Doose]

On Tue, May 5, 2026 at 6:46 AM Joshua Crofts via B4 Relay
<devnull+joshua.crofts1.gmail.com@kernel.org> wrote:
>
> From: Joshua Crofts <joshua.crofts1@gmail.com>
>
> Sort include headers alphabetically to improve coding style and
> readability.
>
> No functional change.
>
> Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
> ---
>  drivers/iio/magnetometer/ak8975.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
>
[snip]
>

LGTM.

Reviewed-by: Maxwell Doose <m32285159@gmail.com>

best regards,
max

Re: [PATCH v5 01/18] iio: magnetometer: ak8975: sort headers alphabetically

[Jonathan Cameron]

On Tue, 5 May 2026 18:46:15 -0500
Maxwell Doose <m32285159@gmail.com> wrote:

> On Tue, May 5, 2026 at 6:46 AM Joshua Crofts via B4 Relay
> <devnull+joshua.crofts1.gmail.com@kernel.org> wrote:
> >
> > From: Joshua Crofts <joshua.crofts1@gmail.com>
> >
> > Sort include headers alphabetically to improve coding style and
> > readability.
> >
> > No functional change.
> >
> > Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
> > ---
> >  drivers/iio/magnetometer/ak8975.c | 20 ++++++++++----------
> >  1 file changed, 10 insertions(+), 10 deletions(-)
> >  
> [snip]
> >  
> 
> LGTM.
> 
> Reviewed-by: Maxwell Doose <m32285159@gmail.com>
> 
> best regards,
> max

Too many patches floating around so I'm going to pick up
the start of this series at least.  Applied this one to the
testing branch of iio.git.

[PATCH v5 02/18] iio: magnetometer: ak8975: update headers per IWYU principle

[Joshua Crofts via B4 Relay]

From: Joshua Crofts <joshua.crofts1@gmail.com>

Remove kernel.h proxy header and unused headers (slab.h, iio/sysfs.h,
iio/trigger.h). Add missing headers to ensure atomicity (array_size.h,
dev_printk.h, asm/byteorder.h, irqreturn.h, minmax.h, property.h,
types.h, wait.h).

Audited using the include-what-you-use tool.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
---
 drivers/iio/magnetometer/ak8975.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
index 569cd6fa839aec6ee8758a261d0e659fc20131c6..dac0a90c33eebd43f8aeed846c9cbf5fd1a6ab91 100644
--- a/drivers/iio/magnetometer/ak8975.c
+++ b/drivers/iio/magnetometer/ak8975.c
@@ -7,24 +7,29 @@
  * Copyright (c) 2010, NVIDIA Corporation.
  */
 
+#include <linux/array_size.h>
 #include <linux/bitops.h>
 #include <linux/delay.h>
+#include <linux/dev_printk.h>
 #include <linux/err.h>
 #include <linux/gpio/consumer.h>
 #include <linux/i2c.h>
 #include <linux/interrupt.h>
-#include <linux/kernel.h>
+#include <linux/iopoll.h>
+#include <linux/minmax.h>
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/pm_runtime.h>
+#include <linux/property.h>
 #include <linux/regulator/consumer.h>
-#include <linux/slab.h>
+#include <linux/types.h>
+#include <linux/wait.h>
+
+#include <asm/byteorder.h>
 
 #include <linux/iio/buffer.h>
 #include <linux/iio/iio.h>
-#include <linux/iio/sysfs.h>
-#include <linux/iio/trigger.h>
 #include <linux/iio/trigger_consumer.h>
 #include <linux/iio/triggered_buffer.h>
 

-- 
2.47.3

Re: [PATCH v5 02/18] iio: magnetometer: ak8975: update headers per IWYU principle

[Maxwell Doose]

On Tue, May 5, 2026 at 6:58 AM Joshua Crofts via B4 Relay
<devnull+joshua.crofts1.gmail.com@kernel.org> wrote:
>
> From: Joshua Crofts <joshua.crofts1@gmail.com>
>
> Remove kernel.h proxy header and unused headers (slab.h, iio/sysfs.h,
> iio/trigger.h). Add missing headers to ensure atomicity (array_size.h,
> dev_printk.h, asm/byteorder.h, irqreturn.h, minmax.h, property.h,
> types.h, wait.h).
>
> Audited using the include-what-you-use tool.
>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
> ---
>  drivers/iio/magnetometer/ak8975.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
>
[snip]
>

LGTM.

Reviewed-by: Maxwell Doose <m32285159@gmail.com>

best regards,
max

Re: [PATCH v5 02/18] iio: magnetometer: ak8975: update headers per IWYU principle

[Jonathan Cameron]

On Tue, 5 May 2026 19:38:55 -0500
Maxwell Doose <m32285159@gmail.com> wrote:

> On Tue, May 5, 2026 at 6:58 AM Joshua Crofts via B4 Relay
> <devnull+joshua.crofts1.gmail.com@kernel.org> wrote:
> >
> > From: Joshua Crofts <joshua.crofts1@gmail.com>
> >
> > Remove kernel.h proxy header and unused headers (slab.h, iio/sysfs.h,
> > iio/trigger.h). Add missing headers to ensure atomicity (array_size.h,
> > dev_printk.h, asm/byteorder.h, irqreturn.h, minmax.h, property.h,
> > types.h, wait.h).
> >
> > Audited using the include-what-you-use tool.
> >
> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
> > ---
> >  drivers/iio/magnetometer/ak8975.c | 13 +++++++++----
> >  1 file changed, 9 insertions(+), 4 deletions(-)
> >  
> [snip]
> >  
> 
> LGTM.
> 
> Reviewed-by: Maxwell Doose <m32285159@gmail.com>
Applied to the testing branch of iio.git. 
If you do a v6 base either base on that (and state it) or
just drop the ones I've applied and don't state a base.

Thanks,

Jonathan

> 
> best regards,
> max

[PATCH v5 03/18] iio: magnetometer: ak8975: replace usleep_range() with fsleep()

[Joshua Crofts via B4 Relay]

From: Joshua Crofts <joshua.crofts1@gmail.com>

Replace usleep_range() calls with fsleep(), passing the minimum value
required by the sensor for hardware delays.

fsleep() automatically selects the optimal sleep mechanism, simplifying
driver code and time management.

Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
---
 drivers/iio/magnetometer/ak8975.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
index dac0a90c33eebd43f8aeed846c9cbf5fd1a6ab91..a84a05a1b64280e26c428e9752a39380e386c234 100644
--- a/drivers/iio/magnetometer/ak8975.c
+++ b/drivers/iio/magnetometer/ak8975.c
@@ -461,7 +461,8 @@ static int ak8975_power_on(const struct ak8975_data *data)
 	 * and the minimum wait time before mode setting is 100us, in
 	 * total 300us. Add some margin and say minimum 500us here.
 	 */
-	usleep_range(500, 1000);
+	fsleep(500);
+
 	return 0;
 }
 
@@ -551,7 +552,7 @@ static int ak8975_set_mode(struct ak8975_data *data, enum ak_ctrl_mode mode)
 
 	data->cntl_cache = regval;
 	/* After mode change wait at least 100us */
-	usleep_range(100, 500);
+	fsleep(100);
 
 	return 0;
 }

-- 
2.47.3

Re: [PATCH v5 03/18] iio: magnetometer: ak8975: replace usleep_range() with fsleep()

[Maxwell Doose]

Hi Joshua,

On Tue, May 5, 2026 at 6:53 AM Joshua Crofts via B4 Relay
<devnull+joshua.crofts1.gmail.com@kernel.org> wrote:
>
> From: Joshua Crofts <joshua.crofts1@gmail.com>
>
> Replace usleep_range() calls with fsleep(), passing the minimum value
> required by the sensor for hardware delays.
>
> fsleep() automatically selects the optimal sleep mechanism, simplifying
> driver code and time management.
>
> Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
> ---
>  drivers/iio/magnetometer/ak8975.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
[snip]

Looks fine overall but has this been tested on actual hardware? Just
want to double check since this touches the set_mode and power_on
logic. Also I will say it may be better to keep usleep_range() since
it's more explicit, which will likely be better for the future to make
it obvious. Also, I was taking a look at the docs for fsleep() and it
provides "maximum 25% slack", which means we're stripping away almost
375us on one of these calls. And besides, it would eventually just
call usleep_range() anyways.

best regards,
max

Re: [PATCH v5 03/18] iio: magnetometer: ak8975: replace usleep_range() with fsleep()

[Joshua Crofts]

On Tue, 5 May 2026 at 22:30, Maxwell Doose <m32285159@gmail.com> wrote:
>
> Hi Joshua,
>
> On Tue, May 5, 2026 at 6:53 AM Joshua Crofts via B4 Relay
> <devnull+joshua.crofts1.gmail.com@kernel.org> wrote:
> >
> > From: Joshua Crofts <joshua.crofts1@gmail.com>
> >
> > Replace usleep_range() calls with fsleep(), passing the minimum value
> > required by the sensor for hardware delays.
> >
> > fsleep() automatically selects the optimal sleep mechanism, simplifying
> > driver code and time management.
> >
> > Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
> > ---
> >  drivers/iio/magnetometer/ak8975.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> [snip]

Hi Max,

> Looks fine overall but has this been tested on actual hardware? Just
> want to double check since this touches the set_mode and power_on
> logic.

Fair enough, this series mostly concerned code cleanup and moving
away from older driver programming practices, so it shouldn't mess
with the functioning logic as much - however it would be more than
welcome if someone had the hardware to test it.

> Also I will say it may be better to keep usleep_range() since
> it's more explicit, which will likely be better for the future to make
> it obvious. Also, I was taking a look at the docs for fsleep() and it
> provides "maximum 25% slack", which means we're stripping away almost
> 375us on one of these calls. And besides, it would eventually just
> call usleep_range() anyways.

I disagree here. If you look at the documentation for msleep(), you can see
that "slack" is actually an addition to the delay time, not a
subtraction. fsleep()
will always guarantee a delay of at least what's supplied as the parameter.

About fsleep() translating to usleep_range() - yes, you're correct about that,
however not only it's more modern practice, it also doesn't require the
programmer to add an additional arbitrary max value, fsleep() just
adds it itself.
As for explicitness, I think fsleep(500) is explicit enough, as described above.

-- 
Kind regards

CJD

Re: [PATCH v5 03/18] iio: magnetometer: ak8975: replace usleep_range() with fsleep()

[Maxwell Doose]

On Tue, May 5, 2026 at 4:26 PM Joshua Crofts <joshua.crofts1@gmail.com> wrote:
>
> Hi Max,
>
> > Looks fine overall but has this been tested on actual hardware? Just
> > want to double check since this touches the set_mode and power_on
> > logic.
>
> Fair enough, this series mostly concerned code cleanup and moving
> away from older driver programming practices, so it shouldn't mess
> with the functioning logic as much - however it would be more than
> welcome if someone had the hardware to test it.
>
> > Also I will say it may be better to keep usleep_range() since
> > it's more explicit, which will likely be better for the future to make
> > it obvious. Also, I was taking a look at the docs for fsleep() and it
> > provides "maximum 25% slack", which means we're stripping away almost
> > 375us on one of these calls. And besides, it would eventually just
> > call usleep_range() anyways.
>
> I disagree here. If you look at the documentation for msleep(), you can see
> that "slack" is actually an addition to the delay time, not a
> subtraction. fsleep()
> will always guarantee a delay of at least what's supplied as the parameter.
>

Sorry if there was any confusion, I just double checked the docs
myself, I was saying if we do fsleep(100); wouldn't that mean the
allowable range is 100-125us? Might put some extra strain vs. the
400us we had before. If I'm incorrect on that you can call me out on
that.

>
> About fsleep() translating to usleep_range() - yes, you're correct about that,
> however not only it's more modern practice, it also doesn't require the
> programmer to add an additional arbitrary max value, fsleep() just
> adds it itself.
> As for explicitness, I think fsleep(500) is explicit enough, as described above.
>

Probably true, but like above my only concern is timing.

best regards,
max



>
> --
> Kind regards
>
> CJD

Re: [PATCH v5 03/18] iio: magnetometer: ak8975: replace usleep_range() with fsleep()

[Joshua Crofts]

On Tue, 5 May 2026 at 23:42, Maxwell Doose <m32285159@gmail.com> wrote:
>
> On Tue, May 5, 2026 at 4:26 PM Joshua Crofts <joshua.crofts1@gmail.com> wrote:
> >
> > Hi Max,
> >
> > > Looks fine overall but has this been tested on actual hardware? Just
> > > want to double check since this touches the set_mode and power_on
> > > logic.
> >
> > Fair enough, this series mostly concerned code cleanup and moving
> > away from older driver programming practices, so it shouldn't mess
> > with the functioning logic as much - however it would be more than
> > welcome if someone had the hardware to test it.
> >
> > > Also I will say it may be better to keep usleep_range() since
> > > it's more explicit, which will likely be better for the future to make
> > > it obvious. Also, I was taking a look at the docs for fsleep() and it
> > > provides "maximum 25% slack", which means we're stripping away almost
> > > 375us on one of these calls. And besides, it would eventually just
> > > call usleep_range() anyways.
> >
> > I disagree here. If you look at the documentation for msleep(), you can see
> > that "slack" is actually an addition to the delay time, not a
> > subtraction. fsleep()
> > will always guarantee a delay of at least what's supplied as the parameter.
> >
>
> Sorry if there was any confusion, I just double checked the docs
> myself, I was saying if we do fsleep(100); wouldn't that mean the
> allowable range is 100-125us? Might put some extra strain vs. the
> 400us we had before. If I'm incorrect on that you can call me out on
> that.

Okay, now I understand. From a scheduling perspective you're right,
it would be less strain. At the same time I don't think that was the original
intention - the author probably just slipped a random value relatively in
range to satisfy the max parameter.

Either way, we should perhaps take this up with the reviewers, as Jonathan
and Andy (and Sashiko) didn't have any issue with this.

-- 
Kind regards

CJD

Re: [PATCH v5 03/18] iio: magnetometer: ak8975: replace usleep_range() with fsleep()

[Maxwell Doose]

On Tue, May 5, 2026 at 4:59 PM Joshua Crofts <joshua.crofts1@gmail.com> wrote:
>
> Okay, now I understand. From a scheduling perspective you're right,
> it would be less strain. At the same time I don't think that was the original
> intention - the author probably just slipped a random value relatively in
> range to satisfy the max parameter.
>
> Either way, we should perhaps take this up with the reviewers, as Jonathan
> and Andy (and Sashiko) didn't have any issue with this.
>

Sounds like a good idea, they'll likely have a better idea of this
driver than I do.

best regards,
max



> --
> Kind regards
>
> CJD

Re: [PATCH v5 03/18] iio: magnetometer: ak8975: replace usleep_range() with fsleep()

[Andy Shevchenko]

On Tue, May 05, 2026 at 11:26:08PM +0200, Joshua Crofts wrote:
> On Tue, 5 May 2026 at 22:30, Maxwell Doose <m32285159@gmail.com> wrote:
> > On Tue, May 5, 2026 at 6:53 AM Joshua Crofts via B4 Relay
> > <devnull+joshua.crofts1.gmail.com@kernel.org> wrote:
> > >
> > > Replace usleep_range() calls with fsleep(), passing the minimum value
> > > required by the sensor for hardware delays.
> > >
> > > fsleep() automatically selects the optimal sleep mechanism, simplifying
> > > driver code and time management.

[snip]

> > Looks fine overall but has this been tested on actual hardware? Just
> > want to double check since this touches the set_mode and power_on
> > logic.
> 
> Fair enough, this series mostly concerned code cleanup and moving
> away from older driver programming practices, so it shouldn't mess
> with the functioning logic as much - however it would be more than
> welcome if someone had the hardware to test it.

This simple patch doesn't change the minimum time to sleep, assuming the driver
was tested previously (and it had to be), the given change is very unlikely to
introduce regressions. Also note that the similar changes have been done all
around the kernel in the past few month and I haven't heard it brought a
regression to the users (but if you, Maxwell, have some pointers to otherwise,
please share, I would really like to see such a report!).

> > Also I will say it may be better to keep usleep_range() since
> > it's more explicit, which will likely be better for the future to make
> > it obvious. Also, I was taking a look at the docs for fsleep() and it
> > provides "maximum 25% slack", which means we're stripping away almost
> > 375us on one of these calls. And besides, it would eventually just
> > call usleep_range() anyways.
> 
> I disagree here. If you look at the documentation for msleep(), you can see
> that "slack" is actually an addition to the delay time, not a
> subtraction. fsleep()
> will always guarantee a delay of at least what's supplied as the parameter.
> 
> About fsleep() translating to usleep_range() - yes, you're correct about that,
> however not only it's more modern practice, it also doesn't require the
> programmer to add an additional arbitrary max value, fsleep() just
> adds it itself.
> As for explicitness, I think fsleep(500) is explicit enough, as described above.

Yes, fsleep() is recommended way of non-atomic sleeps with a microsecond
granularity.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 03/18] iio: magnetometer: ak8975: replace usleep_range() with fsleep()

[Maxwell Doose]

On Wed, May 6, 2026 at 1:19 AM Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
>
[snip]
>
> This simple patch doesn't change the minimum time to sleep, assuming the driver
> was tested previously (and it had to be), the given change is very unlikely to
> introduce regressions. Also note that the similar changes have been done all
> around the kernel in the past few month and I haven't heard it brought a
> regression to the users (but if you, Maxwell, have some pointers to otherwise,
> please share, I would really like to see such a report!).
>

Alright, fair enough. I'm just not as familiar as I'd like to be with
the fsleep() stuff and wanted to make sure the timings were good
before guaranteeing anything.

best regards,
max



>
> Yes, fsleep() is recommended way of non-atomic sleeps with a microsecond
> granularity.
>
> --
> With Best Regards,
> Andy Shevchenko
>
>

Re: [PATCH v5 03/18] iio: magnetometer: ak8975: replace usleep_range() with fsleep()

[Jonathan Cameron]

On Wed, 6 May 2026 01:35:32 -0500
Maxwell Doose <m32285159@gmail.com> wrote:

> On Wed, May 6, 2026 at 1:19 AM Andy Shevchenko
> <andriy.shevchenko@intel.com> wrote:
> >  
> [snip]
> >
> > This simple patch doesn't change the minimum time to sleep, assuming the driver
> > was tested previously (and it had to be), the given change is very unlikely to
> > introduce regressions. Also note that the similar changes have been done all
> > around the kernel in the past few month and I haven't heard it brought a
> > regression to the users (but if you, Maxwell, have some pointers to otherwise,
> > please share, I would really like to see such a report!).
> >  
> 
> Alright, fair enough. I'm just not as familiar as I'd like to be with
> the fsleep() stuff and wanted to make sure the timings were good
> before guaranteeing anything.
> 
> best regards,
> max
Applied this one to the testing branch of iio.git

Thanks,

Jonathan

> 
> 
> 
> >
> > Yes, fsleep() is recommended way of non-atomic sleeps with a microsecond
> > granularity.
> >
> > --
> > With Best Regards,
> > Andy Shevchenko
> >
> >  

[PATCH v5 04/18] iio: magnetometer: ak8975: change 'u8*' to 'u8 *' in cast

[Joshua Crofts via B4 Relay]

From: Joshua Crofts <joshua.crofts1@gmail.com>

Change 'u8*' cast to 'u8 *' as the former triggers a checkpatch error.
Also fix the indentation of parameters in
i2c_smbus_read_i2c_block_data_or_emulated() function.

No functional change.

Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
---
 drivers/iio/magnetometer/ak8975.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
index a84a05a1b64280e26c428e9752a39380e386c234..6ca8c24b9fba452b6fed8155957a8e5fcbc6511c 100644
--- a/drivers/iio/magnetometer/ak8975.c
+++ b/drivers/iio/magnetometer/ak8975.c
@@ -759,9 +759,10 @@ static int ak8975_read_axis(struct iio_dev *indio_dev, int index, int *val)
 	if (ret)
 		goto exit;
 
-	ret = i2c_smbus_read_i2c_block_data_or_emulated(
-			client, def->data_regs[index],
-			sizeof(rval), (u8*)&rval);
+	ret = i2c_smbus_read_i2c_block_data_or_emulated(client,
+							def->data_regs[index],
+							sizeof(rval),
+							(u8 *)&rval);
 	if (ret < 0)
 		goto exit;
 

-- 
2.47.3

Re: [PATCH v5 04/18] iio: magnetometer: ak8975: change 'u8*' to 'u8 *' in cast

[Maxwell Doose]

On Tue, May 5, 2026 at 6:46 AM Joshua Crofts via B4 Relay
<devnull+joshua.crofts1.gmail.com@kernel.org> wrote:
>
> From: Joshua Crofts <joshua.crofts1@gmail.com>
>
> Change 'u8*' cast to 'u8 *' as the former triggers a checkpatch error.
> Also fix the indentation of parameters in
> i2c_smbus_read_i2c_block_data_or_emulated() function.
>
> No functional change.
>
> Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
> ---
>  drivers/iio/magnetometer/ak8975.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
[snip]
>

LGTM.

Reviewed-by: Maxwell Doose <m32285159@gmail.com>

best regards,
max

Re: [PATCH v5 04/18] iio: magnetometer: ak8975: change 'u8*' to 'u8 *' in cast

[Andy Shevchenko]

On Tue, May 05, 2026 at 01:46:00PM +0200, Joshua Crofts via B4 Relay wrote:

> Change 'u8*' cast to 'u8 *' as the former triggers a checkpatch error.
> Also fix the indentation of parameters in
> i2c_smbus_read_i2c_block_data_or_emulated() function.
> 
> No functional change.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 04/18] iio: magnetometer: ak8975: change 'u8*' to 'u8 *' in cast

[Jonathan Cameron]

On Wed, 6 May 2026 09:30:54 +0300
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Tue, May 05, 2026 at 01:46:00PM +0200, Joshua Crofts via B4 Relay wrote:
> 
> > Change 'u8*' cast to 'u8 *' as the former triggers a checkpatch error.
> > Also fix the indentation of parameters in
> > i2c_smbus_read_i2c_block_data_or_emulated() function.
> > 
> > No functional change.  
> 
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> 
Applied.

[PATCH v5 05/18] iio: magnetometer: ak8975: fix wrong errno on return

[Joshua Crofts via B4 Relay]

From: Joshua Crofts <joshua.crofts1@gmail.com>

The driver currently returns -EINVAL on polling timeout instead of
-ETIMEDOUT.

Replace return code for -ETIMEDOUT and remove unnecessary error
message as -ETIMEDOUT is a standard POSIX error. Also replace
instances of -EINVAL in comments.

Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
---
 drivers/iio/magnetometer/ak8975.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
index 6ca8c24b9fba452b6fed8155957a8e5fcbc6511c..57f50c09cca539c3733f516a1617375e9134c349 100644
--- a/drivers/iio/magnetometer/ak8975.c
+++ b/drivers/iio/magnetometer/ak8975.c
@@ -662,10 +662,8 @@ static int wait_conversion_complete_gpio(struct ak8975_data *data)
 			break;
 		timeout_ms -= AK8975_CONVERSION_DONE_POLL_TIME;
 	}
-	if (!timeout_ms) {
-		dev_err(&client->dev, "Conversion timeout happened\n");
-		return -EINVAL;
-	}
+	if (!timeout_ms)
+		return -ETIMEDOUT;
 
 	ret = i2c_smbus_read_byte_data(client, data->def->ctrl_regs[ST1]);
 	if (ret < 0)
@@ -695,15 +693,13 @@ static int wait_conversion_complete_polled(struct ak8975_data *data)
 			break;
 		timeout_ms -= AK8975_CONVERSION_DONE_POLL_TIME;
 	}
-	if (!timeout_ms) {
-		dev_err(&client->dev, "Conversion timeout happened\n");
-		return -EINVAL;
-	}
+	if (!timeout_ms)
+		return -ETIMEDOUT;
 
 	return read_status;
 }
 
-/* Returns 0 if the end of conversion interrupt occurred or -ETIME otherwise */
+/* Returns 0 if the end of conversion interrupt occurred or -ETIMEDOUT otherwise */
 static int wait_conversion_complete_interrupt(struct ak8975_data *data)
 {
 	int ret;
@@ -713,7 +709,7 @@ static int wait_conversion_complete_interrupt(struct ak8975_data *data)
 				 AK8975_DATA_READY_TIMEOUT);
 	clear_bit(0, &data->flags);
 
-	return ret > 0 ? 0 : -ETIME;
+	return ret > 0 ? 0 : -ETIMEDOUT;
 }
 
 static int ak8975_start_read_axis(struct ak8975_data *data,

-- 
2.47.3

Re: [PATCH v5 05/18] iio: magnetometer: ak8975: fix wrong errno on return

[Jonathan Cameron]

On Tue, 05 May 2026 13:46:01 +0200
Joshua Crofts via B4 Relay <devnull+joshua.crofts1.gmail.com@kernel.org> wrote:

> From: Joshua Crofts <joshua.crofts1@gmail.com>
> 
> The driver currently returns -EINVAL on polling timeout instead of
> -ETIMEDOUT.
> 
> Replace return code for -ETIMEDOUT and remove unnecessary error
> message as -ETIMEDOUT is a standard POSIX error. Also replace
> instances of -EINVAL in comments.
> 
> Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
Applied.

Tah

[PATCH v5 06/18] iio: magnetometer: ak8975: pass conversion timeouts as arguments

[Joshua Crofts via B4 Relay]

From: Joshua Crofts <joshua.crofts1@gmail.com>

Refactor wait_conversion_complete*_() helper function to accept poll
and timeout values directly as parameters. This improves the
readability of the code and does not rely on hardcoded macros.

Besides that, fix the home grown and obviously wrong in some cases the
jiffy-based timeout.

Co-developed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
---
 drivers/iio/magnetometer/ak8975.c | 42 +++++++++++++++++++--------------------
 1 file changed, 20 insertions(+), 22 deletions(-)

diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
index 57f50c09cca539c3733f516a1617375e9134c349..83878ea28b91d8316d2cc31e57fde6da957af689 100644
--- a/drivers/iio/magnetometer/ak8975.c
+++ b/drivers/iio/magnetometer/ak8975.c
@@ -16,6 +16,7 @@
 #include <linux/i2c.h>
 #include <linux/interrupt.h>
 #include <linux/iopoll.h>
+#include <linux/jiffies.h>
 #include <linux/minmax.h>
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
@@ -132,13 +133,6 @@
 
 #define AK09912_MAX_REGS		AK09912_REG_ASAZ
 
-/*
- * Miscellaneous values.
- */
-#define AK8975_MAX_CONVERSION_TIMEOUT	500
-#define AK8975_CONVERSION_DONE_POLL_TIME 10
-#define AK8975_DATA_READY_TIMEOUT	((100*HZ)/1000)
-
 /*
  * Precalculate scale factor (in Gauss units) for each axis and
  * store in the device data.
@@ -649,18 +643,19 @@ static int ak8975_setup(struct i2c_client *client)
 	return 0;
 }
 
-static int wait_conversion_complete_gpio(struct ak8975_data *data)
+static int wait_conversion_complete_gpio(struct ak8975_data *data,
+					 unsigned int poll_ms,
+					 unsigned int timeout_ms)
 {
 	struct i2c_client *client = data->client;
-	u32 timeout_ms = AK8975_MAX_CONVERSION_TIMEOUT;
 	int ret;
 
 	/* Wait for the conversion to complete. */
 	while (timeout_ms) {
-		msleep(AK8975_CONVERSION_DONE_POLL_TIME);
+		msleep(poll_ms);
 		if (gpiod_get_value(data->eoc_gpiod))
 			break;
-		timeout_ms -= AK8975_CONVERSION_DONE_POLL_TIME;
+		timeout_ms -= poll_ms;
 	}
 	if (!timeout_ms)
 		return -ETIMEDOUT;
@@ -672,16 +667,17 @@ static int wait_conversion_complete_gpio(struct ak8975_data *data)
 	return ret;
 }
 
-static int wait_conversion_complete_polled(struct ak8975_data *data)
+static int wait_conversion_complete_polled(struct ak8975_data *data,
+					   unsigned int poll_ms,
+					   unsigned int timeout_ms)
 {
 	struct i2c_client *client = data->client;
 	u8 read_status;
-	u32 timeout_ms = AK8975_MAX_CONVERSION_TIMEOUT;
 	int ret;
 
 	/* Wait for the conversion to complete. */
 	while (timeout_ms) {
-		msleep(AK8975_CONVERSION_DONE_POLL_TIME);
+		msleep(poll_ms);
 		ret = i2c_smbus_read_byte_data(client,
 					       data->def->ctrl_regs[ST1]);
 		if (ret < 0) {
@@ -691,7 +687,7 @@ static int wait_conversion_complete_polled(struct ak8975_data *data)
 		read_status = ret;
 		if (read_status)
 			break;
-		timeout_ms -= AK8975_CONVERSION_DONE_POLL_TIME;
+		timeout_ms -= poll_ms;
 	}
 	if (!timeout_ms)
 		return -ETIMEDOUT;
@@ -700,13 +696,14 @@ static int wait_conversion_complete_polled(struct ak8975_data *data)
 }
 
 /* Returns 0 if the end of conversion interrupt occurred or -ETIMEDOUT otherwise */
-static int wait_conversion_complete_interrupt(struct ak8975_data *data)
+static int wait_conversion_complete_interrupt(struct ak8975_data *data,
+					      unsigned int timeout_ms)
 {
 	int ret;
 
 	ret = wait_event_timeout(data->data_ready_queue,
 				 test_bit(0, &data->flags),
-				 AK8975_DATA_READY_TIMEOUT);
+				 msecs_to_jiffies(timeout_ms));
 	clear_bit(0, &data->flags);
 
 	return ret > 0 ? 0 : -ETIMEDOUT;
@@ -715,9 +712,10 @@ static int wait_conversion_complete_interrupt(struct ak8975_data *data)
 static int ak8975_start_read_axis(struct ak8975_data *data,
 				  const struct i2c_client *client)
 {
-	/* Set up the device for taking a sample. */
-	int ret = ak8975_set_mode(data, MODE_ONCE);
+	int ret;
 
+	/* Set up the device for taking a sample. */
+	ret = ak8975_set_mode(data, MODE_ONCE);
 	if (ret < 0) {
 		dev_err(&client->dev, "Error in setting operating mode\n");
 		return ret;
@@ -725,11 +723,11 @@ static int ak8975_start_read_axis(struct ak8975_data *data,
 
 	/* Wait for the conversion to complete. */
 	if (data->eoc_irq)
-		ret = wait_conversion_complete_interrupt(data);
+		ret = wait_conversion_complete_interrupt(data, 100);
 	else if (data->eoc_gpiod)
-		ret = wait_conversion_complete_gpio(data);
+		ret = wait_conversion_complete_gpio(data, 10, 500);
 	else
-		ret = wait_conversion_complete_polled(data);
+		ret = wait_conversion_complete_polled(data, 10, 500);
 	if (ret < 0)
 		return ret;
 

-- 
2.47.3

Re: [PATCH v5 06/18] iio: magnetometer: ak8975: pass conversion timeouts as arguments

[Andy Shevchenko]

On Tue, May 05, 2026 at 01:46:02PM +0200, Joshua Crofts via B4 Relay wrote:

> Refactor wait_conversion_complete*_() helper function to accept poll

functions

> and timeout values directly as parameters. This improves the
> readability of the code and does not rely on hardcoded macros.
> 
> Besides that, fix the home grown and obviously wrong in some cases the
> jiffy-based timeout.

...

>  static int ak8975_start_read_axis(struct ak8975_data *data,
>  				  const struct i2c_client *client)
>  {
> -	/* Set up the device for taking a sample. */
> -	int ret = ak8975_set_mode(data, MODE_ONCE);
> +	int ret;
>  
> +	/* Set up the device for taking a sample. */
> +	ret = ak8975_set_mode(data, MODE_ONCE);

But this piece does not belong to the main scope of the patch.

>  	if (ret < 0) {
>  		dev_err(&client->dev, "Error in setting operating mode\n");
>  		return ret;

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 06/18] iio: magnetometer: ak8975: pass conversion timeouts as arguments

[Joshua Crofts]

On Wed, 6 May 2026 at 08:34, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Tue, May 05, 2026 at 01:46:02PM +0200, Joshua Crofts via B4 Relay wrote:
>
> > Refactor wait_conversion_complete*_() helper function to accept poll
>
> functions
>
> > and timeout values directly as parameters. This improves the
> > readability of the code and does not rely on hardcoded macros.
> >
> > Besides that, fix the home grown and obviously wrong in some cases the
> > jiffy-based timeout.
>
> ...
>
> >  static int ak8975_start_read_axis(struct ak8975_data *data,
> >                                 const struct i2c_client *client)
> >  {
> > -     /* Set up the device for taking a sample. */
> > -     int ret = ak8975_set_mode(data, MODE_ONCE);
> > +     int ret;
> >
> > +     /* Set up the device for taking a sample. */
> > +     ret = ak8975_set_mode(data, MODE_ONCE);
>
> But this piece does not belong to the main scope of the patch.

Fair, probably a badly resolved merge conflict.

-- 
Kind regards

CJD

Re: [PATCH v5 06/18] iio: magnetometer: ak8975: pass conversion timeouts as arguments

[Andy Shevchenko]

On Wed, May 06, 2026 at 09:02:15AM +0200, Joshua Crofts wrote:
> On Wed, 6 May 2026 at 08:34, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Tue, May 05, 2026 at 01:46:02PM +0200, Joshua Crofts via B4 Relay wrote:

...

> > >  static int ak8975_start_read_axis(struct ak8975_data *data,
> > >                                 const struct i2c_client *client)
> > >  {
> > > -     /* Set up the device for taking a sample. */
> > > -     int ret = ak8975_set_mode(data, MODE_ONCE);
> > > +     int ret;
> > >
> > > +     /* Set up the device for taking a sample. */
> > > +     ret = ak8975_set_mode(data, MODE_ONCE);
> >
> > But this piece does not belong to the main scope of the patch.
> 
> Fair, probably a badly resolved merge conflict.

Hmm... Usually what we use is interactive rebase. Very powerful tool!

	`git rebase --rebase-merges --interactive $SHA`

where $SHA is the same one that you used as --base for the series.

In conjunction with

	`git reset HEAD~1; git commit -s --interactive -c ORIG_HEAD`

technique, you can easily move parts between patches.

(There are even more advanced techniques, but I'm an old-school guy.)

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 06/18] iio: magnetometer: ak8975: pass conversion timeouts as arguments

[Joshua Crofts]

On Wed, 6 May 2026 at 09:20, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Wed, May 06, 2026 at 09:02:15AM +0200, Joshua Crofts wrote:
> > On Wed, 6 May 2026 at 08:34, Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> > > On Tue, May 05, 2026 at 01:46:02PM +0200, Joshua Crofts via B4 Relay wrote:
>
> ...
>
> > > >  static int ak8975_start_read_axis(struct ak8975_data *data,
> > > >                                 const struct i2c_client *client)
> > > >  {
> > > > -     /* Set up the device for taking a sample. */
> > > > -     int ret = ak8975_set_mode(data, MODE_ONCE);
> > > > +     int ret;
> > > >
> > > > +     /* Set up the device for taking a sample. */
> > > > +     ret = ak8975_set_mode(data, MODE_ONCE);
> > >
> > > But this piece does not belong to the main scope of the patch.
> >
> > Fair, probably a badly resolved merge conflict.
>
> Hmm... Usually what we use is interactive rebase. Very powerful tool!
>
>         `git rebase --rebase-merges --interactive $SHA`

I see, well I use `git rebase -i HEAD~no_of_commits_in_series` and
sometimes I move a different thing by accident. It does get a bit
crowded with 18 patches (yours were fine but I also had to do some
resolution after moving the order)

> where $SHA is the same one that you used as --base for the series.
>
> In conjunction with
>
>         `git reset HEAD~1; git commit -s --interactive -c ORIG_HEAD`
>
> technique, you can easily move parts between patches.
>
> (There are even more advanced techniques, but I'm an old-school guy.)

Good to know, thanks!

-- 
Kind regards

CJD

Re: [PATCH v5 06/18] iio: magnetometer: ak8975: pass conversion timeouts as arguments

[Andy Shevchenko]

On Wed, May 06, 2026 at 09:28:06AM +0200, Joshua Crofts wrote:
> On Wed, 6 May 2026 at 09:20, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Wed, May 06, 2026 at 09:02:15AM +0200, Joshua Crofts wrote:
> > > On Wed, 6 May 2026 at 08:34, Andy Shevchenko
> > > <andriy.shevchenko@linux.intel.com> wrote:
> > > > On Tue, May 05, 2026 at 01:46:02PM +0200, Joshua Crofts via B4 Relay wrote:

...

> > > Fair, probably a badly resolved merge conflict.
> >
> > Hmm... Usually what we use is interactive rebase. Very powerful tool!
> >
> >         `git rebase --rebase-merges --interactive $SHA`
> 
> I see, well I use `git rebase -i HEAD~no_of_commits_in_series`

Good that you are already onboard!

> and sometimes I move a different thing by accident.

Yeah, this is usually unrelated to the tools, human factor, you know :-)

> It does get a bit
> crowded with 18 patches (yours were fine but I also had to do some
> resolution after moving the order)
> 
> > where $SHA is the same one that you used as --base for the series.
> >
> > In conjunction with
> >
> >         `git reset HEAD~1; git commit -s --interactive -c ORIG_HEAD`
> >
> > technique, you can easily move parts between patches.
> >
> > (There are even more advanced techniques, but I'm an old-school guy.)
> 
> Good to know, thanks!

You're welcome!

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 06/18] iio: magnetometer: ak8975: pass conversion timeouts as arguments

[Jonathan Cameron]

On Wed, 6 May 2026 10:32:49 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> On Wed, May 06, 2026 at 09:28:06AM +0200, Joshua Crofts wrote:
> > On Wed, 6 May 2026 at 09:20, Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:  
> > > On Wed, May 06, 2026 at 09:02:15AM +0200, Joshua Crofts wrote:  
> > > > On Wed, 6 May 2026 at 08:34, Andy Shevchenko
> > > > <andriy.shevchenko@linux.intel.com> wrote:  
> > > > > On Tue, May 05, 2026 at 01:46:02PM +0200, Joshua Crofts via B4 Relay wrote:  
> 
> ...
> 
> > > > Fair, probably a badly resolved merge conflict.  
> > >
> > > Hmm... Usually what we use is interactive rebase. Very powerful tool!
> > >
> > >         `git rebase --rebase-merges --interactive $SHA`  
> > 
> > I see, well I use `git rebase -i HEAD~no_of_commits_in_series`  
> 
> Good that you are already onboard!
> 
> > and sometimes I move a different thing by accident.  
> 
> Yeah, this is usually unrelated to the tools, human factor, you know :-)
> 
> > It does get a bit
> > crowded with 18 patches (yours were fine but I also had to do some
> > resolution after moving the order)
> >   
> > > where $SHA is the same one that you used as --base for the series.
> > >
> > > In conjunction with
> > >
> > >         `git reset HEAD~1; git commit -s --interactive -c ORIG_HEAD`
> > >
> > > technique, you can easily move parts between patches.
> > >
> > > (There are even more advanced techniques, but I'm an old-school guy.)  
> > 
> > Good to know, thanks!  
> 
> You're welcome!
> 

Given the issue was minor I applied and dropped that change that should
have been elsewhere. 

Thanks,

Jonathan

[PATCH v5 07/18] iio: magnetometer: ak8975: modernize polling loops with iopoll() macros

[Joshua Crofts via B4 Relay]

From: Joshua Crofts <joshua.crofts1@gmail.com>

The driver currently uses while loops and msleep() for polling during
conversion waits.

Replace the custom polling loops with readx_poll_timeout() and
read_poll_timeout() macros from <linux/iopoll.h>. This reduces
boilerplate, standardizes timeout handling and improves overall code
readability, keeping the original timing and error behaviour. Add
<linux/time.h> for USEC_PER_MSEC macro instead of using magic numbers.

Assisted-by: Gemini:3.1-Pro
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
---
 drivers/iio/magnetometer/ak8975.c | 43 +++++++++++++++++----------------------
 1 file changed, 19 insertions(+), 24 deletions(-)

diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
index 83878ea28b91d8316d2cc31e57fde6da957af689..129c98c97071b4cf8efa3abf2364a3abbde129ba 100644
--- a/drivers/iio/magnetometer/ak8975.c
+++ b/drivers/iio/magnetometer/ak8975.c
@@ -24,6 +24,7 @@
 #include <linux/pm_runtime.h>
 #include <linux/property.h>
 #include <linux/regulator/consumer.h>
+#include <linux/time.h>
 #include <linux/types.h>
 #include <linux/wait.h>
 
@@ -649,16 +650,14 @@ static int wait_conversion_complete_gpio(struct ak8975_data *data,
 {
 	struct i2c_client *client = data->client;
 	int ret;
+	int val;
 
 	/* Wait for the conversion to complete. */
-	while (timeout_ms) {
-		msleep(poll_ms);
-		if (gpiod_get_value(data->eoc_gpiod))
-			break;
-		timeout_ms -= poll_ms;
-	}
-	if (!timeout_ms)
-		return -ETIMEDOUT;
+	ret = readx_poll_timeout(gpiod_get_value, data->eoc_gpiod, val, val != 0,
+				 poll_ms * USEC_PER_MSEC,
+				 timeout_ms * USEC_PER_MSEC);
+	if (ret)
+		return ret;
 
 	ret = i2c_smbus_read_byte_data(client, data->def->ctrl_regs[ST1]);
 	if (ret < 0)
@@ -672,27 +671,23 @@ static int wait_conversion_complete_polled(struct ak8975_data *data,
 					   unsigned int timeout_ms)
 {
 	struct i2c_client *client = data->client;
-	u8 read_status;
 	int ret;
+	int val;
 
 	/* Wait for the conversion to complete. */
-	while (timeout_ms) {
-		msleep(poll_ms);
-		ret = i2c_smbus_read_byte_data(client,
-					       data->def->ctrl_regs[ST1]);
-		if (ret < 0) {
-			dev_err(&client->dev, "Error in reading ST1\n");
-			return ret;
-		}
-		read_status = ret;
-		if (read_status)
-			break;
-		timeout_ms -= poll_ms;
+	ret = read_poll_timeout(i2c_smbus_read_byte_data, val, val != 0,
+				poll_ms * USEC_PER_MSEC,
+				timeout_ms * USEC_PER_MSEC,
+				true,
+				client, data->def->ctrl_regs[ST1]);
+	if (ret)
+		return ret;
+	if (val < 0) {
+		dev_err(&client->dev, "Error in reading ST1\n");
+		return val;
 	}
-	if (!timeout_ms)
-		return -ETIMEDOUT;
 
-	return read_status;
+	return val;
 }
 
 /* Returns 0 if the end of conversion interrupt occurred or -ETIMEDOUT otherwise */

-- 
2.47.3

Re: [PATCH v5 07/18] iio: magnetometer: ak8975: modernize polling loops with iopoll() macros

[Andy Shevchenko]

On Tue, May 05, 2026 at 01:46:03PM +0200, Joshua Crofts via B4 Relay wrote:

> The driver currently uses while loops and msleep() for polling during
> conversion waits.
> 
> Replace the custom polling loops with readx_poll_timeout() and
> read_poll_timeout() macros from <linux/iopoll.h>. This reduces
> boilerplate, standardizes timeout handling and improves overall code
> readability, keeping the original timing and error behaviour. Add
> <linux/time.h> for USEC_PER_MSEC macro instead of using magic numbers.

...

> +	ret = read_poll_timeout(i2c_smbus_read_byte_data, val, val != 0,
> +				poll_ms * USEC_PER_MSEC,
> +				timeout_ms * USEC_PER_MSEC,
> +				true,
> +				client, data->def->ctrl_regs[ST1]);
> +	if (ret)
> +		return ret;
> +	if (val < 0) {
> +		dev_err(&client->dev, "Error in reading ST1\n");
> +		return val;
>  	}
> -	if (!timeout_ms)
> -		return -ETIMEDOUT;
>  
> -	return read_status;
> +	return val;

Besides the unneeded return val duplication, I think this is the wrong location
for this check and it changes behaviour (really subtle change!).

Before if the last iteration gives an error from the device, we return
-ETIMEDOUT instead of the whatever the i2c_smbus_read_byte_data() returns.


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 07/18] iio: magnetometer: ak8975: modernize polling loops with iopoll() macros

[Joshua Crofts]

On Wed, 6 May 2026 at 08:53, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Tue, May 05, 2026 at 01:46:03PM +0200, Joshua Crofts via B4 Relay wrote:
>
> > The driver currently uses while loops and msleep() for polling during
> > conversion waits.
> >
> > Replace the custom polling loops with readx_poll_timeout() and
> > read_poll_timeout() macros from <linux/iopoll.h>. This reduces
> > boilerplate, standardizes timeout handling and improves overall code
> > readability, keeping the original timing and error behaviour. Add
> > <linux/time.h> for USEC_PER_MSEC macro instead of using magic numbers.
>
> ...
>
> > +     ret = read_poll_timeout(i2c_smbus_read_byte_data, val, val != 0,
> > +                             poll_ms * USEC_PER_MSEC,
> > +                             timeout_ms * USEC_PER_MSEC,
> > +                             true,
> > +                             client, data->def->ctrl_regs[ST1]);
> > +     if (ret)
> > +             return ret;
> > +     if (val < 0) {
> > +             dev_err(&client->dev, "Error in reading ST1\n");
> > +             return val;
> >       }
> > -     if (!timeout_ms)
> > -             return -ETIMEDOUT;
> >
> > -     return read_status;
> > +     return val;
>
> Besides the unneeded return val duplication, I think this is the wrong location
> for this check and it changes behaviour (really subtle change!).
>
> Before if the last iteration gives an error from the device, we return
> -ETIMEDOUT instead of the whatever the i2c_smbus_read_byte_data() returns.

Yeah, that is pretty subtle, but I agree.

-- 
Kind regards

CJD

Re: [PATCH v5 07/18] iio: magnetometer: ak8975: modernize polling loops with iopoll() macros

[Joshua Crofts]

On Wed, 6 May 2026 at 09:01, Joshua Crofts <joshua.crofts1@gmail.com> wrote:
>
> On Wed, 6 May 2026 at 08:53, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > On Tue, May 05, 2026 at 01:46:03PM +0200, Joshua Crofts via B4 Relay wrote:
> >
> > > The driver currently uses while loops and msleep() for polling during
> > > conversion waits.
> > >
> > > Replace the custom polling loops with readx_poll_timeout() and
> > > read_poll_timeout() macros from <linux/iopoll.h>. This reduces
> > > boilerplate, standardizes timeout handling and improves overall code
> > > readability, keeping the original timing and error behaviour. Add
> > > <linux/time.h> for USEC_PER_MSEC macro instead of using magic numbers.
> >
> > ...
> >
> > > +     ret = read_poll_timeout(i2c_smbus_read_byte_data, val, val != 0,
> > > +                             poll_ms * USEC_PER_MSEC,
> > > +                             timeout_ms * USEC_PER_MSEC,
> > > +                             true,
> > > +                             client, data->def->ctrl_regs[ST1]);
> > > +     if (ret)
> > > +             return ret;
> > > +     if (val < 0) {
> > > +             dev_err(&client->dev, "Error in reading ST1\n");
> > > +             return val;
> > >       }
> > > -     if (!timeout_ms)
> > > -             return -ETIMEDOUT;
> > >
> > > -     return read_status;
> > > +     return val;
> >
> > Besides the unneeded return val duplication, I think this is the wrong location
> > for this check and it changes behaviour (really subtle change!).

Hmm, rechecking this, I agree with the unnecessary return, but I don't think it
changes the behaviour of the function.

> > Before if the last iteration gives an error from the device, we return
> > -ETIMEDOUT instead of the whatever the i2c_smbus_read_byte_data() returns.

Looking at the original function, it always prioritized returning the
i2c_smbus_read_byte_data() error code before returning -ETIMEDOUT (even
on the final iteration). In the new version, if the i2c read is bad,
the read_poll_timeout()
code will still be zero, therefore allowing the code to jump to the
i2c value check
and then return if bad (this is still the same behaviour IMO).

Maybe I'm looking at it from the wrong angle though. AFAIC Sashiko nitpicked
everything except this (but yes, it's still an AI so we have to tread lightly).

Same goes for the other patch where you raised this issue.

-- 
Kind regards

CJD

Re: [PATCH v5 07/18] iio: magnetometer: ak8975: modernize polling loops with iopoll() macros

[Andy Shevchenko]

On Wed, May 06, 2026 at 12:50:09PM +0200, Joshua Crofts wrote:
> On Wed, 6 May 2026 at 09:01, Joshua Crofts <joshua.crofts1@gmail.com> wrote:
> > On Wed, 6 May 2026 at 08:53, Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> > > On Tue, May 05, 2026 at 01:46:03PM +0200, Joshua Crofts via B4 Relay wrote:

...

> > > > +     ret = read_poll_timeout(i2c_smbus_read_byte_data, val, val != 0,
> > > > +                             poll_ms * USEC_PER_MSEC,
> > > > +                             timeout_ms * USEC_PER_MSEC,
> > > > +                             true,
> > > > +                             client, data->def->ctrl_regs[ST1]);
> > > > +     if (ret)
> > > > +             return ret;
> > > > +     if (val < 0) {
> > > > +             dev_err(&client->dev, "Error in reading ST1\n");
> > > > +             return val;
> > > >       }
> > > > -     if (!timeout_ms)
> > > > -             return -ETIMEDOUT;
> > > >
> > > > -     return read_status;
> > > > +     return val;
> > >
> > > Besides the unneeded return val duplication, I think this is the wrong location
> > > for this check and it changes behaviour (really subtle change!).
> 
> Hmm, rechecking this, I agree with the unnecessary return, but I don't think it
> changes the behaviour of the function.
> 
> > > Before if the last iteration gives an error from the device, we return
> > > -ETIMEDOUT instead of the whatever the i2c_smbus_read_byte_data() returns.
> 
> Looking at the original function, it always prioritized returning the
> i2c_smbus_read_byte_data() error code before returning -ETIMEDOUT (even
> on the final iteration). In the new version, if the i2c read is bad,
> the read_poll_timeout()
> code will still be zero, therefore allowing the code to jump to the
> i2c value check
> and then return if bad (this is still the same behaviour IMO).

It looks like I stand corrected! Indeed, we have two conditions to follow, one
is provided in a macro parameter, and the other is for timeout. Here the Q is
which one logically should be checked first? More thinking on it tends to your
direction as it follows usual pattern as we check the return values (errors)
from the callee first *then* validate the results.

> Maybe I'm looking at it from the wrong angle though. AFAIC Sashiko nitpicked
> everything except this (but yes, it's still an AI so we have to tread lightly).

> Same goes for the other patch where you raised this issue.

Yep.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 07/18] iio: magnetometer: ak8975: modernize polling loops with iopoll() macros

[Jonathan Cameron]

On Wed, 6 May 2026 16:08:21 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> On Wed, May 06, 2026 at 12:50:09PM +0200, Joshua Crofts wrote:
> > On Wed, 6 May 2026 at 09:01, Joshua Crofts <joshua.crofts1@gmail.com> wrote:  
> > > On Wed, 6 May 2026 at 08:53, Andy Shevchenko
> > > <andriy.shevchenko@linux.intel.com> wrote:  
> > > > On Tue, May 05, 2026 at 01:46:03PM +0200, Joshua Crofts via B4 Relay wrote:  
> 
> ...
> 
> > > > > +     ret = read_poll_timeout(i2c_smbus_read_byte_data, val, val != 0,
> > > > > +                             poll_ms * USEC_PER_MSEC,
> > > > > +                             timeout_ms * USEC_PER_MSEC,
> > > > > +                             true,
> > > > > +                             client, data->def->ctrl_regs[ST1]);
> > > > > +     if (ret)
> > > > > +             return ret;
> > > > > +     if (val < 0) {
> > > > > +             dev_err(&client->dev, "Error in reading ST1\n");
> > > > > +             return val;
> > > > >       }
> > > > > -     if (!timeout_ms)
> > > > > -             return -ETIMEDOUT;
> > > > >
> > > > > -     return read_status;
> > > > > +     return val;  
> > > >
> > > > Besides the unneeded return val duplication, I think this is the wrong location
> > > > for this check and it changes behaviour (really subtle change!).  
> > 
> > Hmm, rechecking this, I agree with the unnecessary return, but I don't think it
> > changes the behaviour of the function.
> >   
> > > > Before if the last iteration gives an error from the device, we return
> > > > -ETIMEDOUT instead of the whatever the i2c_smbus_read_byte_data() returns.  
> > 
> > Looking at the original function, it always prioritized returning the
> > i2c_smbus_read_byte_data() error code before returning -ETIMEDOUT (even
> > on the final iteration). In the new version, if the i2c read is bad,
> > the read_poll_timeout()
> > code will still be zero, therefore allowing the code to jump to the
> > i2c value check
> > and then return if bad (this is still the same behaviour IMO).  
> 
> It looks like I stand corrected! Indeed, we have two conditions to follow, one
> is provided in a macro parameter, and the other is for timeout. Here the Q is
> which one logically should be checked first? More thinking on it tends to your
> direction as it follows usual pattern as we check the return values (errors)
> from the callee first *then* validate the results.
> 
> > Maybe I'm looking at it from the wrong angle though. AFAIC Sashiko nitpicked
> > everything except this (but yes, it's still an AI so we have to tread lightly).  
> 
> > Same goes for the other patch where you raised this issue.  
> 
> Yep.
> 

I'll leave this one for now as the return val duplication can be tidied up.
I think the rest of the patch looks good tome.

Thanks,

Jonathan

[PATCH v5 08/18] iio: magnetometer: ak8975: check if gpiod read was successful

[Joshua Crofts via B4 Relay]

From: Joshua Crofts <joshua.crofts1@gmail.com>

Add a check that ensures that valid data has been read from GPIOD. If
not, log an error and return the negative read value.

Suggested-by: Jonathan Cameron <jic23@kernel.org>
Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
---
 drivers/iio/magnetometer/ak8975.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
index 129c98c97071b4cf8efa3abf2364a3abbde129ba..b3caf5f3ebf29bf701bb214aa480fa7afb2651c6 100644
--- a/drivers/iio/magnetometer/ak8975.c
+++ b/drivers/iio/magnetometer/ak8975.c
@@ -658,6 +658,10 @@ static int wait_conversion_complete_gpio(struct ak8975_data *data,
 				 timeout_ms * USEC_PER_MSEC);
 	if (ret)
 		return ret;
+	if (val < 0) {
+		dev_err(&client->dev, "Error in reading GPIOD\n");
+		return val;
+	}
 
 	ret = i2c_smbus_read_byte_data(client, data->def->ctrl_regs[ST1]);
 	if (ret < 0)

-- 
2.47.3

Re: [PATCH v5 08/18] iio: magnetometer: ak8975: check if gpiod read was successful

[Maxwell Doose]

On Tue May 5, 2026 at 6:46 AM CDT, Joshua Crofts via B4 Relay wrote:
> From: Joshua Crofts <joshua.crofts1@gmail.com>
>
> Add a check that ensures that valid data has been read from GPIOD. If
> not, log an error and return the negative read value.
>
> Suggested-by: Jonathan Cameron <jic23@kernel.org>
> Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
> ---
>  drivers/iio/magnetometer/ak8975.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
> index 129c98c97071b4cf8efa3abf2364a3abbde129ba..b3caf5f3ebf29bf701bb214aa480fa7afb2651c6 100644
> --- a/drivers/iio/magnetometer/ak8975.c
> +++ b/drivers/iio/magnetometer/ak8975.c
> @@ -658,6 +658,10 @@ static int wait_conversion_complete_gpio(struct ak8975_data *data,
>  				 timeout_ms * USEC_PER_MSEC);
>  	if (ret)
>  		return ret;
> +	if (val < 0) {
> +		dev_err(&client->dev, "Error in reading GPIOD\n");
> +		return val;
> +	}
>  
>  	ret = i2c_smbus_read_byte_data(client, data->def->ctrl_regs[ST1]);
>  	if (ret < 0)

Small nit but I wonder if we can change:

	if (val < 0) {
		dev_err(&client->dev, "Error in reading GPIOD\n");
		return val;
	}

to something like:

	if (val < 0)
		return dev_err_probe(&client->dev, val, "Error in reading GPIOD\n");

Would be good for code density.

My rb will apply either way.

Reviewed-by: Maxwell Doose <m32285159@gmail.com>

best regards,
max

Re: [PATCH v5 08/18] iio: magnetometer: ak8975: check if gpiod read was successful

[Maxwell Doose]

On Tue, May 5, 2026 at 8:09 PM Maxwell Doose <m32285159@gmail.com> wrote:
>
> On Tue May 5, 2026 at 6:46 AM CDT, Joshua Crofts via B4 Relay wrote:
> > From: Joshua Crofts <joshua.crofts1@gmail.com>
> >
> > Add a check that ensures that valid data has been read from GPIOD. If
> > not, log an error and return the negative read value.
> >
> > Suggested-by: Jonathan Cameron <jic23@kernel.org>
> > Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
> > ---
> >  drivers/iio/magnetometer/ak8975.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
> > index 129c98c97071b4cf8efa3abf2364a3abbde129ba..b3caf5f3ebf29bf701bb214aa480fa7afb2651c6 100644
> > --- a/drivers/iio/magnetometer/ak8975.c
> > +++ b/drivers/iio/magnetometer/ak8975.c
> > @@ -658,6 +658,10 @@ static int wait_conversion_complete_gpio(struct ak8975_data *data,
> >                                timeout_ms * USEC_PER_MSEC);
> >       if (ret)
> >               return ret;
> > +     if (val < 0) {
> > +             dev_err(&client->dev, "Error in reading GPIOD\n");
> > +             return val;
> > +     }
> >
> >       ret = i2c_smbus_read_byte_data(client, data->def->ctrl_regs[ST1]);
> >       if (ret < 0)
>
> Small nit but I wonder if we can change:
>
>         if (val < 0) {
>                 dev_err(&client->dev, "Error in reading GPIOD\n");
>                 return val;
>         }
>
> to something like:
>
>         if (val < 0)
>                 return dev_err_probe(&client->dev, val, "Error in reading GPIOD\n");
>
> Would be good for code density.
>
> My rb will apply either way.
>
> Reviewed-by: Maxwell Doose <m32285159@gmail.com>
>
> best regards,
> max

Actually, timeout. Sashiko had a good potential catch here:

https://sashiko.dev/#/patchset/20260505-magnetometer-fixes-v5-0-831b9b5550fc%40gmail.com

"Does this new error path leak the pm runtime reference?
In ak8975_read_axis(), a pm runtime reference is acquired via
pm_runtime_get_sync(&data->client->dev). If this new error path returns
val back up the call chain through ak8975_start_read_axis() to
ak8975_read_axis(), the code jumps to the exit label.
The exit block unlocks the mutex and returns the error code, but it skips
calling pm_runtime_put_autosuspend()."

I'm going to withdraw my reviewed-by until I can confirm that this
isn't a problem, sorry about that.

best regards,
max

Re: [PATCH v5 08/18] iio: magnetometer: ak8975: check if gpiod read was successful

[Andy Shevchenko]

On Tue, May 05, 2026 at 08:42:12PM -0500, Maxwell Doose wrote:
> On Tue, May 5, 2026 at 8:09 PM Maxwell Doose <m32285159@gmail.com> wrote:
> > On Tue May 5, 2026 at 6:46 AM CDT, Joshua Crofts via B4 Relay wrote:

...

> Actually, timeout. Sashiko had a good potential catch here:
> 
> https://sashiko.dev/#/patchset/20260505-magnetometer-fixes-v5-0-831b9b5550fc%40gmail.com
> 
> "Does this new error path leak the pm runtime reference?
> In ak8975_read_axis(), a pm runtime reference is acquired via
> pm_runtime_get_sync(&data->client->dev). If this new error path returns
> val back up the call chain through ak8975_start_read_axis() to
> ak8975_read_axis(), the code jumps to the exit label.
> The exit block unlocks the mutex and returns the error code, but it skips
> calling pm_runtime_put_autosuspend()."

While it seems like a valid concern, it doesn't relate to this change.

> I'm going to withdraw my reviewed-by until I can confirm that this
> isn't a problem, sorry about that.

Why?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 08/18] iio: magnetometer: ak8975: check if gpiod read was successful

[Maxwell Doose]

On Wed, May 6, 2026 at 2:11 AM Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
>
> On Tue, May 05, 2026 at 08:42:12PM -0500, Maxwell Doose wrote:
> > On Tue, May 5, 2026 at 8:09 PM Maxwell Doose <m32285159@gmail.com> wrote:
> > > On Tue May 5, 2026 at 6:46 AM CDT, Joshua Crofts via B4 Relay wrote:
>
> ...
>
> > Actually, timeout. Sashiko had a good potential catch here:
> >
> > https://sashiko.dev/#/patchset/20260505-magnetometer-fixes-v5-0-831b9b5550fc%40gmail.com
> >
> > "Does this new error path leak the pm runtime reference?
> > In ak8975_read_axis(), a pm runtime reference is acquired via
> > pm_runtime_get_sync(&data->client->dev). If this new error path returns
> > val back up the call chain through ak8975_start_read_axis() to
> > ak8975_read_axis(), the code jumps to the exit label.
> > The exit block unlocks the mutex and returns the error code, but it skips
> > calling pm_runtime_put_autosuspend()."
>
> While it seems like a valid concern, it doesn't relate to this change.
>
> > I'm going to withdraw my reviewed-by until I can confirm that this
> > isn't a problem, sorry about that.
>
> Why?
>

I just wanted to make sure this wouldn't cause some crazy regression,
I think it's valid to be somewhat concerned. If you want you can
reapply my tag.

best regards,
max



> --
> With Best Regards,
> Andy Shevchenko
>
>

Re: [PATCH v5 08/18] iio: magnetometer: ak8975: check if gpiod read was successful

[Andy Shevchenko]

On Wed, May 06, 2026 at 02:13:37AM -0500, Maxwell Doose wrote:
> On Wed, May 6, 2026 at 2:11 AM Andy Shevchenko
> <andriy.shevchenko@intel.com> wrote:
> > On Tue, May 05, 2026 at 08:42:12PM -0500, Maxwell Doose wrote:
> > > On Tue, May 5, 2026 at 8:09 PM Maxwell Doose <m32285159@gmail.com> wrote:
> > > > On Tue May 5, 2026 at 6:46 AM CDT, Joshua Crofts via B4 Relay wrote:

...

> > > Actually, timeout. Sashiko had a good potential catch here:
> > >
> > > https://sashiko.dev/#/patchset/20260505-magnetometer-fixes-v5-0-831b9b5550fc%40gmail.com
> > >
> > > "Does this new error path leak the pm runtime reference?
> > > In ak8975_read_axis(), a pm runtime reference is acquired via
> > > pm_runtime_get_sync(&data->client->dev). If this new error path returns
> > > val back up the call chain through ak8975_start_read_axis() to
> > > ak8975_read_axis(), the code jumps to the exit label.
> > > The exit block unlocks the mutex and returns the error code, but it skips
> > > calling pm_runtime_put_autosuspend()."
> >
> > While it seems like a valid concern, it doesn't relate to this change.
> >
> > > I'm going to withdraw my reviewed-by until I can confirm that this
> > > isn't a problem, sorry about that.
> >
> > Why?
> 
> I just wanted to make sure this wouldn't cause some crazy regression,
> I think it's valid to be somewhat concerned.

Yes, but this was the concern even before this patch, no?



-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 08/18] iio: magnetometer: ak8975: check if gpiod read was successful

[Maxwell Doose]

On Wed, May 6, 2026 at 2:16 AM Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
>
> On Wed, May 06, 2026 at 02:13:37AM -0500, Maxwell Doose wrote:
> > On Wed, May 6, 2026 at 2:11 AM Andy Shevchenko
> > <andriy.shevchenko@intel.com> wrote:
> > > On Tue, May 05, 2026 at 08:42:12PM -0500, Maxwell Doose wrote:
> > > > On Tue, May 5, 2026 at 8:09 PM Maxwell Doose <m32285159@gmail.com> wrote:
> > > > > On Tue May 5, 2026 at 6:46 AM CDT, Joshua Crofts via B4 Relay wrote:
[snip]
> > > > I'm going to withdraw my reviewed-by until I can confirm that this
> > > > isn't a problem, sorry about that.
> > >
> > > Why?
> >
> > I just wanted to make sure this wouldn't cause some crazy regression,
> > I think it's valid to be somewhat concerned.
>
> Yes, but this was the concern even before this patch, no?
>

Going to give that a solid maybe. I must've been tired when I looked
over this again. Very sorry about that.

best regards,
max


>
>
> --
> With Best Regards,
> Andy Shevchenko
>
>

Re: [PATCH v5 08/18] iio: magnetometer: ak8975: check if gpiod read was successful

[Joshua Crofts]

On Wed, 6 May 2026 at 09:16, Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
>
> On Wed, May 06, 2026 at 02:13:37AM -0500, Maxwell Doose wrote:
> > On Wed, May 6, 2026 at 2:11 AM Andy Shevchenko
> > <andriy.shevchenko@intel.com> wrote:
> > > On Tue, May 05, 2026 at 08:42:12PM -0500, Maxwell Doose wrote:
> > > > On Tue, May 5, 2026 at 8:09 PM Maxwell Doose <m32285159@gmail.com> wrote:
> > > > > On Tue May 5, 2026 at 6:46 AM CDT, Joshua Crofts via B4 Relay wrote:
>
> ...
>
> > > > Actually, timeout. Sashiko had a good potential catch here:
> > > >
> > > > https://sashiko.dev/#/patchset/20260505-magnetometer-fixes-v5-0-831b9b5550fc%40gmail.com
> > > >
> > > > "Does this new error path leak the pm runtime reference?
> > > > In ak8975_read_axis(), a pm runtime reference is acquired via
> > > > pm_runtime_get_sync(&data->client->dev). If this new error path returns
> > > > val back up the call chain through ak8975_start_read_axis() to
> > > > ak8975_read_axis(), the code jumps to the exit label.
> > > > The exit block unlocks the mutex and returns the error code, but it skips
> > > > calling pm_runtime_put_autosuspend()."
> > >
> > > While it seems like a valid concern, it doesn't relate to this change.
> > >
> > > > I'm going to withdraw my reviewed-by until I can confirm that this
> > > > isn't a problem, sorry about that.
> > >
> > > Why?
> >
> > I just wanted to make sure this wouldn't cause some crazy regression,
> > I think it's valid to be somewhat concerned.
>
> Yes, but this was the concern even before this patch, no?

IMO yes, even the first error path would leak the pm_runtime reference.

-- 
Kind regards

CJD

Re: [PATCH v5 08/18] iio: magnetometer: ak8975: check if gpiod read was successful

[Joshua Crofts]

On Wed, 6 May 2026 at 03:09, Maxwell Doose <m32285159@gmail.com> wrote:
>
> On Tue May 5, 2026 at 6:46 AM CDT, Joshua Crofts via B4 Relay wrote:
> > From: Joshua Crofts <joshua.crofts1@gmail.com>
> >
> > Add a check that ensures that valid data has been read from GPIOD. If
> > not, log an error and return the negative read value.
> >
> > Suggested-by: Jonathan Cameron <jic23@kernel.org>
> > Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
> > ---
> >  drivers/iio/magnetometer/ak8975.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
> > index 129c98c97071b4cf8efa3abf2364a3abbde129ba..b3caf5f3ebf29bf701bb214aa480fa7afb2651c6 100644
> > --- a/drivers/iio/magnetometer/ak8975.c
> > +++ b/drivers/iio/magnetometer/ak8975.c
> > @@ -658,6 +658,10 @@ static int wait_conversion_complete_gpio(struct ak8975_data *data,
> >                                timeout_ms * USEC_PER_MSEC);
> >       if (ret)
> >               return ret;
> > +     if (val < 0) {
> > +             dev_err(&client->dev, "Error in reading GPIOD\n");
> > +             return val;
> > +     }
> >
> >       ret = i2c_smbus_read_byte_data(client, data->def->ctrl_regs[ST1]);
> >       if (ret < 0)
>
> Small nit but I wonder if we can change:
>
>         if (val < 0) {
>                 dev_err(&client->dev, "Error in reading GPIOD\n");
>                 return val;
>         }
>
> to something like:
>
>         if (val < 0)
>                 return dev_err_probe(&client->dev, val, "Error in reading GPIOD\n");

Sure, but it would go as a separate patch. Given this series is
18 parts long, I'd rather get the existing patches merged and
work on this in another series. This driver still has a few shortcomings
that should be ironed out.

-- 
Kind regards

CJD

Re: [PATCH v5 08/18] iio: magnetometer: ak8975: check if gpiod read was successful

[Jonathan Cameron]

On Wed, 6 May 2026 09:05:39 +0200
Joshua Crofts <joshua.crofts1@gmail.com> wrote:

> On Wed, 6 May 2026 at 03:09, Maxwell Doose <m32285159@gmail.com> wrote:
> >
> > On Tue May 5, 2026 at 6:46 AM CDT, Joshua Crofts via B4 Relay wrote:  
> > > From: Joshua Crofts <joshua.crofts1@gmail.com>
> > >
> > > Add a check that ensures that valid data has been read from GPIOD. If
> > > not, log an error and return the negative read value.
> > >
> > > Suggested-by: Jonathan Cameron <jic23@kernel.org>
> > > Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
> > > ---
> > >  drivers/iio/magnetometer/ak8975.c | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > >
> > > diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
> > > index 129c98c97071b4cf8efa3abf2364a3abbde129ba..b3caf5f3ebf29bf701bb214aa480fa7afb2651c6 100644
> > > --- a/drivers/iio/magnetometer/ak8975.c
> > > +++ b/drivers/iio/magnetometer/ak8975.c
> > > @@ -658,6 +658,10 @@ static int wait_conversion_complete_gpio(struct ak8975_data *data,
> > >                                timeout_ms * USEC_PER_MSEC);
> > >       if (ret)
> > >               return ret;
> > > +     if (val < 0) {
> > > +             dev_err(&client->dev, "Error in reading GPIOD\n");
> > > +             return val;
> > > +     }
> > >
> > >       ret = i2c_smbus_read_byte_data(client, data->def->ctrl_regs[ST1]);
> > >       if (ret < 0)  
> >
> > Small nit but I wonder if we can change:
> >
> >         if (val < 0) {
> >                 dev_err(&client->dev, "Error in reading GPIOD\n");
> >                 return val;
> >         }
> >
> > to something like:
> >
> >         if (val < 0)
> >                 return dev_err_probe(&client->dev, val, "Error in reading GPIOD\n");  
> 
> Sure, but it would go as a separate patch. Given this series is
> 18 parts long, I'd rather get the existing patches merged and
> work on this in another series. This driver still has a few shortcomings
> that should be ironed out.
> 

This doesn't look to only be called from probe.
dev_err_probe() is only meant for stuff that is.  Arguably
you could use it elsewhere but then it should have a different name.

Thanks,

Jonathan

Re: [PATCH v5 08/18] iio: magnetometer: ak8975: check if gpiod read was successful

[Andy Shevchenko]

On Tue, May 05, 2026 at 08:09:22PM -0500, Maxwell Doose wrote:
> On Tue May 5, 2026 at 6:46 AM CDT, Joshua Crofts via B4 Relay wrote:

...

> Small nit but I wonder if we can change:
> 
> 	if (val < 0) {
> 		dev_err(&client->dev, "Error in reading GPIOD\n");
> 		return val;
> 	}
> 
> to something like:
> 
> 	if (val < 0)
> 		return dev_err_probe(&client->dev, val, "Error in reading GPIOD\n");
> 
> Would be good for code density.

Not in this patch. IIRC we have a dedicated change for dev_err_probe()
conversion. Also note, dev_err_probe() is only applicable to the functions
that are part of the probe stage and probe stage only. If the function shared
between probe and run-time, the dev_err_probe() has not to be used.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 08/18] iio: magnetometer: ak8975: check if gpiod read was successful

[Maxwell Doose]

On Wed, May 6, 2026 at 2:08 AM Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
>
> On Tue, May 05, 2026 at 08:09:22PM -0500, Maxwell Doose wrote:
> > On Tue May 5, 2026 at 6:46 AM CDT, Joshua Crofts via B4 Relay wrote:
>
> ...
>
> > Small nit but I wonder if we can change:
> >
> >       if (val < 0) {
> >               dev_err(&client->dev, "Error in reading GPIOD\n");
> >               return val;
> >       }
> >
> > to something like:
> >
> >       if (val < 0)
> >               return dev_err_probe(&client->dev, val, "Error in reading GPIOD\n");
> >
> > Would be good for code density.
>
> Not in this patch. IIRC we have a dedicated change for dev_err_probe()
> conversion. Also note, dev_err_probe() is only applicable to the functions
> that are part of the probe stage and probe stage only. If the function shared
> between probe and run-time, the dev_err_probe() has not to be used.
>

Understood; I'll take that into account for the future, thanks for that tip.

best regards,
max



> --
> With Best Regards,
> Andy Shevchenko
>
>

Re: [PATCH v5 08/18] iio: magnetometer: ak8975: check if gpiod read was successful

[Andy Shevchenko]

On Tue, May 05, 2026 at 01:46:04PM +0200, Joshua Crofts via B4 Relay wrote:

> Add a check that ensures that valid data has been read from GPIOD. If
> not, log an error and return the negative read value.

...

See the comment on the previous patch, applies to here as well
(to some extent).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 08/18] iio: magnetometer: ak8975: check if gpiod read was successful

[Joshua Crofts]

On Wed, 6 May 2026 at 08:54, Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
>
> On Tue, May 05, 2026 at 01:46:04PM +0200, Joshua Crofts via B4 Relay wrote:
>
> > Add a check that ensures that valid data has been read from GPIOD. If
> > not, log an error and return the negative read value.
>
> ...
>
> See the comment on the previous patch, applies to here as well
> (to some extent).

Will do.

-- 
Kind regards

CJD

[PATCH v5 09/18] iio: magnetometer: ak8975: avoid using temporary variable

[Joshua Crofts via B4 Relay]

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Avoid using temporary variable in ak8975_read_axis(). With that being done,
the clamp_t() call becomes idiomatic in the driver and can be factored out
to a helper later on (and if needed).

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
---
 drivers/iio/magnetometer/ak8975.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
index b3caf5f3ebf29bf701bb214aa480fa7afb2651c6..c36d068081066ef6b42cc2f9c3c2e8978ec51623 100644
--- a/drivers/iio/magnetometer/ak8975.c
+++ b/drivers/iio/magnetometer/ak8975.c
@@ -741,7 +741,6 @@ static int ak8975_read_axis(struct iio_dev *indio_dev, int index, int *val)
 	const struct i2c_client *client = data->client;
 	const struct ak_def *def = data->def;
 	__le16 rval;
-	u16 buff;
 	int ret;
 
 	pm_runtime_get_sync(&data->client->dev);
@@ -778,8 +777,8 @@ static int ak8975_read_axis(struct iio_dev *indio_dev, int index, int *val)
 	pm_runtime_put_autosuspend(&data->client->dev);
 
 	/* Swap bytes and convert to valid range. */
-	buff = le16_to_cpu(rval);
-	*val = clamp_t(s16, buff, -def->range, def->range);
+	*val = clamp_t(s16, le16_to_cpu(rval), -def->range, def->range);
+
 	return IIO_VAL_INT;
 
 exit:

-- 
2.47.3

Re: [PATCH v5 09/18] iio: magnetometer: ak8975: avoid using temporary variable

[Jonathan Cameron]

On Tue, 05 May 2026 13:46:05 +0200
Joshua Crofts via B4 Relay <devnull+joshua.crofts1.gmail.com@kernel.org> wrote:

> From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> Avoid using temporary variable in ak8975_read_axis(). With that being done,
> the clamp_t() call becomes idiomatic in the driver and can be factored out
> to a helper later on (and if needed).
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
Independent of the last two patches (which I've left for now)
so applied this one to the testing branch of iio.git.

Jonathan

> ---
>  drivers/iio/magnetometer/ak8975.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
> index b3caf5f3ebf29bf701bb214aa480fa7afb2651c6..c36d068081066ef6b42cc2f9c3c2e8978ec51623 100644
> --- a/drivers/iio/magnetometer/ak8975.c
> +++ b/drivers/iio/magnetometer/ak8975.c
> @@ -741,7 +741,6 @@ static int ak8975_read_axis(struct iio_dev *indio_dev, int index, int *val)
>  	const struct i2c_client *client = data->client;
>  	const struct ak_def *def = data->def;
>  	__le16 rval;
> -	u16 buff;
>  	int ret;
>  
>  	pm_runtime_get_sync(&data->client->dev);
> @@ -778,8 +777,8 @@ static int ak8975_read_axis(struct iio_dev *indio_dev, int index, int *val)
>  	pm_runtime_put_autosuspend(&data->client->dev);
>  
>  	/* Swap bytes and convert to valid range. */
> -	buff = le16_to_cpu(rval);
> -	*val = clamp_t(s16, buff, -def->range, def->range);
> +	*val = clamp_t(s16, le16_to_cpu(rval), -def->range, def->range);
> +
>  	return IIO_VAL_INT;
>  
>  exit:
> 

[PATCH v5 10/18] iio: magnetometer: ak8975: drop duplicate NULL check

[Joshua Crofts via B4 Relay]

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

The gpiod_set_consumer_name() is NULL-aware, no need to perform the same
check in the caller.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
---
 drivers/iio/magnetometer/ak8975.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
index c36d068081066ef6b42cc2f9c3c2e8978ec51623..7a971de2eebf127fc8ab68300fb82f38d9476c0c 100644
--- a/drivers/iio/magnetometer/ak8975.c
+++ b/drivers/iio/magnetometer/ak8975.c
@@ -918,8 +918,7 @@ static int ak8975_probe(struct i2c_client *client)
 	eoc_gpiod = devm_gpiod_get_optional(&client->dev, NULL, GPIOD_IN);
 	if (IS_ERR(eoc_gpiod))
 		return PTR_ERR(eoc_gpiod);
-	if (eoc_gpiod)
-		gpiod_set_consumer_name(eoc_gpiod, "ak_8975");
+	gpiod_set_consumer_name(eoc_gpiod, "ak_8975");
 
 	/*
 	 * According to AK09911 datasheet, if reset GPIO is provided then

-- 
2.47.3

Re: [PATCH v5 10/18] iio: magnetometer: ak8975: drop duplicate NULL check

[Jonathan Cameron]

On Tue, 05 May 2026 13:46:06 +0200
Joshua Crofts via B4 Relay <devnull+joshua.crofts1.gmail.com@kernel.org> wrote:

> From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> The gpiod_set_consumer_name() is NULL-aware, no need to perform the same
> check in the caller.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
Applied.

> ---
>  drivers/iio/magnetometer/ak8975.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
> index c36d068081066ef6b42cc2f9c3c2e8978ec51623..7a971de2eebf127fc8ab68300fb82f38d9476c0c 100644
> --- a/drivers/iio/magnetometer/ak8975.c
> +++ b/drivers/iio/magnetometer/ak8975.c
> @@ -918,8 +918,7 @@ static int ak8975_probe(struct i2c_client *client)
>  	eoc_gpiod = devm_gpiod_get_optional(&client->dev, NULL, GPIOD_IN);
>  	if (IS_ERR(eoc_gpiod))
>  		return PTR_ERR(eoc_gpiod);
> -	if (eoc_gpiod)
> -		gpiod_set_consumer_name(eoc_gpiod, "ak_8975");
> +	gpiod_set_consumer_name(eoc_gpiod, "ak_8975");
>  
>  	/*
>  	 * According to AK09911 datasheet, if reset GPIO is provided then
> 

[PATCH v5 11/18] iio: magnetometer: ak8975: remove duplicate error message

[Joshua Crofts via B4 Relay]

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

The devm_request_irq() already prints an error message.
Remove the duplicate.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
---
 drivers/iio/magnetometer/ak8975.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
index 7a971de2eebf127fc8ab68300fb82f38d9476c0c..f58cbafb491a5e039b22fc928a6c58dd6ec0f297 100644
--- a/drivers/iio/magnetometer/ak8975.c
+++ b/drivers/iio/magnetometer/ak8975.c
@@ -584,17 +584,14 @@ static int ak8975_setup_irq(struct ak8975_data *data)
 	rc = devm_request_irq(&client->dev, irq, ak8975_irq_handler,
 			      IRQF_TRIGGER_RISING,
 			      dev_name(&client->dev), data);
-	if (rc < 0) {
-		dev_err(&client->dev, "irq %d request failed: %d\n", irq, rc);
+	if (rc < 0)
 		return rc;
-	}
 
 	data->eoc_irq = irq;
 
 	return rc;
 }
 
-
 /*
  * Perform some start-of-day setup, including reading the asa calibration
  * values and caching them.

-- 
2.47.3

Re: [PATCH v5 11/18] iio: magnetometer: ak8975: remove duplicate error message

[Jonathan Cameron]

On Tue, 05 May 2026 13:46:07 +0200
Joshua Crofts via B4 Relay <devnull+joshua.crofts1.gmail.com@kernel.org> wrote:

> From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> The devm_request_irq() already prints an error message.
> Remove the duplicate.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
Applied.

[PATCH v5 12/18] iio: magnetometer: ak8975: reduce usage of magic lengths of the buffer

[Joshua Crofts via B4 Relay]

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Reduce usage of magic lengths of the supplied buffer by replacing them
with the corresponding sizeof():s.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
---
 drivers/iio/magnetometer/ak8975.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
index f58cbafb491a5e039b22fc928a6c58dd6ec0f297..fd42e20cd145860bdd1b143409de24b7145f8252 100644
--- a/drivers/iio/magnetometer/ak8975.c
+++ b/drivers/iio/magnetometer/ak8975.c
@@ -490,8 +490,10 @@ static int ak8975_who_i_am(struct i2c_client *client,
 	 * AK8975   |  DEVICE_ID |  NA
 	 * AK8963   |  DEVICE_ID |  NA
 	 */
-	ret = i2c_smbus_read_i2c_block_data_or_emulated(
-			client, AK09912_REG_WIA1, 2, wia_val);
+	ret = i2c_smbus_read_i2c_block_data_or_emulated(client,
+							AK09912_REG_WIA1,
+							sizeof(wia_val),
+							wia_val);
 	if (ret < 0) {
 		dev_err(&client->dev, "Error reading WIA\n");
 		return ret;
@@ -610,9 +612,10 @@ static int ak8975_setup(struct i2c_client *client)
 	}
 
 	/* Get asa data and store in the device data. */
-	ret = i2c_smbus_read_i2c_block_data_or_emulated(
-			client, data->def->ctrl_regs[ASA_BASE],
-			3, data->asa);
+	ret = i2c_smbus_read_i2c_block_data_or_emulated(client,
+							data->def->ctrl_regs[ASA_BASE],
+							sizeof(data->asa),
+							data->asa);
 	if (ret < 0) {
 		dev_err(&client->dev, "Not able to read asa data\n");
 		return ret;
@@ -865,7 +868,7 @@ static void ak8975_fill_buffer(struct iio_dev *indio_dev)
 	 */
 	ret = i2c_smbus_read_i2c_block_data_or_emulated(client,
 							def->data_regs[0],
-							3 * sizeof(fval[0]),
+							sizeof(fval),
 							(u8 *)fval);
 	if (ret < 0)
 		goto unlock;

-- 
2.47.3

Re: [PATCH v5 12/18] iio: magnetometer: ak8975: reduce usage of magic lengths of the buffer

[Jonathan Cameron]

On Tue, 05 May 2026 13:46:08 +0200
Joshua Crofts via B4 Relay <devnull+joshua.crofts1.gmail.com@kernel.org> wrote:

> From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> Reduce usage of magic lengths of the supplied buffer by replacing them
> with the corresponding sizeof():s.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
Applied.

[PATCH v5 13/18] iio: magnetometer: ak8975: unify return code variable name

[Joshua Crofts via B4 Relay]

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

In one case 'rc' is used in the other 'err', the most use 'ret'.
Make the latter use the former, id est 'ret'.

While at it, drop unneeded ' < 0' checks.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
---
 drivers/iio/magnetometer/ak8975.c | 46 +++++++++++++++++++--------------------
 1 file changed, 23 insertions(+), 23 deletions(-)

diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
index fd42e20cd145860bdd1b143409de24b7145f8252..1c05e380a4d4f968f505a7d5c0acbec86ad949e1 100644
--- a/drivers/iio/magnetometer/ak8975.c
+++ b/drivers/iio/magnetometer/ak8975.c
@@ -573,8 +573,8 @@ static irqreturn_t ak8975_irq_handler(int irq, void *data)
 static int ak8975_setup_irq(struct ak8975_data *data)
 {
 	struct i2c_client *client = data->client;
-	int rc;
 	int irq;
+	int ret;
 
 	init_waitqueue_head(&data->data_ready_queue);
 	clear_bit(0, &data->flags);
@@ -583,15 +583,15 @@ static int ak8975_setup_irq(struct ak8975_data *data)
 	else
 		irq = gpiod_to_irq(data->eoc_gpiod);
 
-	rc = devm_request_irq(&client->dev, irq, ak8975_irq_handler,
-			      IRQF_TRIGGER_RISING,
-			      dev_name(&client->dev), data);
-	if (rc < 0)
-		return rc;
+	ret = devm_request_irq(&client->dev, irq, ak8975_irq_handler,
+			       IRQF_TRIGGER_RISING,
+			       dev_name(&client->dev), data);
+	if (ret)
+		return ret;
 
 	data->eoc_irq = irq;
 
-	return rc;
+	return 0;
 }
 
 /*
@@ -907,8 +907,8 @@ static int ak8975_probe(struct i2c_client *client)
 	struct iio_dev *indio_dev;
 	struct gpio_desc *eoc_gpiod;
 	struct gpio_desc *reset_gpiod;
-	int err;
 	const char *name = NULL;
+	int ret;
 
 	/*
 	 * Grab and set up the supplied GPIO.
@@ -943,9 +943,9 @@ static int ak8975_probe(struct i2c_client *client)
 	data->reset_gpiod = reset_gpiod;
 	data->eoc_irq = 0;
 
-	err = iio_read_mount_matrix(&client->dev, &data->orientation);
-	if (err)
-		return err;
+	ret = iio_read_mount_matrix(&client->dev, &data->orientation);
+	if (ret)
+		return ret;
 
 	/* id will be NULL when enumerated via ACPI */
 	data->def = i2c_get_match_data(client);
@@ -966,20 +966,20 @@ static int ak8975_probe(struct i2c_client *client)
 	if (IS_ERR(data->vid))
 		return PTR_ERR(data->vid);
 
-	err = ak8975_power_on(data);
-	if (err)
-		return err;
+	ret = ak8975_power_on(data);
+	if (ret)
+		return ret;
 
-	err = ak8975_who_i_am(client, data->def->type);
-	if (err < 0) {
+	ret = ak8975_who_i_am(client, data->def->type);
+	if (ret) {
 		dev_err(&client->dev, "Unexpected device\n");
 		goto power_off;
 	}
 	dev_dbg(&client->dev, "Asahi compass chip %s\n", name);
 
 	/* Perform some basic start-of-day setup of the device. */
-	err = ak8975_setup(client);
-	if (err < 0) {
+	ret = ak8975_setup(client);
+	if (ret) {
 		dev_err(&client->dev, "%s initialization fails\n", name);
 		goto power_off;
 	}
@@ -992,15 +992,15 @@ static int ak8975_probe(struct i2c_client *client)
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->name = name;
 
-	err = iio_triggered_buffer_setup(indio_dev, NULL, ak8975_handle_trigger,
+	ret = iio_triggered_buffer_setup(indio_dev, NULL, ak8975_handle_trigger,
 					 NULL);
-	if (err) {
+	if (ret) {
 		dev_err(&client->dev, "triggered buffer setup failed\n");
 		goto power_off;
 	}
 
-	err = iio_device_register(indio_dev);
-	if (err) {
+	ret = iio_device_register(indio_dev);
+	if (ret) {
 		dev_err(&client->dev, "device register failed\n");
 		goto cleanup_buffer;
 	}
@@ -1023,7 +1023,7 @@ static int ak8975_probe(struct i2c_client *client)
 	iio_triggered_buffer_cleanup(indio_dev);
 power_off:
 	ak8975_power_off(data);
-	return err;
+	return ret;
 }
 
 static void ak8975_remove(struct i2c_client *client)

-- 
2.47.3

Re: [PATCH v5 13/18] iio: magnetometer: ak8975: unify return code variable name

[Jonathan Cameron]

On Tue, 05 May 2026 13:46:09 +0200
Joshua Crofts via B4 Relay <devnull+joshua.crofts1.gmail.com@kernel.org> wrote:

> From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> In one case 'rc' is used in the other 'err', the most use 'ret'.
> Make the latter use the former, id est 'ret'.
> 
> While at it, drop unneeded ' < 0' checks.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
Applied.

[PATCH v5 14/18] iio: magnetometer: ak8975: switch to using managed resources

[Joshua Crofts via B4 Relay]

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Switch the driver to use managed resources (devm_*) which simplifier
error handling and allows removing ak8975_remove() method from
the driver.

Note, on error path we now also set mode to POWER_DOWN state which is
fine. Even if the device is in that mode, there is no problem to set
that mode again, it should be no-op.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Co-developed-by: Joshua Crofts <joshua.crofts1@gmail.com>
Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
---
 drivers/iio/magnetometer/ak8975.c | 77 ++++++++++++++++++++++-----------------
 1 file changed, 44 insertions(+), 33 deletions(-)

diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
index 1c05e380a4d4f968f505a7d5c0acbec86ad949e1..84ccbca758b0121a9c4930a368cb113471d389da 100644
--- a/drivers/iio/magnetometer/ak8975.c
+++ b/drivers/iio/magnetometer/ak8975.c
@@ -900,9 +900,27 @@ static irqreturn_t ak8975_handle_trigger(int irq, void *p)
 	return IRQ_HANDLED;
 }
 
+static void devm_ak8975_power_off(void *data)
+{
+	struct ak8975_data *ak = data;
+	struct device *dev = &ak->client->dev;
+
+	/* Only power down if currently active */
+	if (pm_runtime_status_suspended(dev))
+		return;
+	/*
+	 * The device may not be runtime suspended when the driver is
+	 * removed, so we soft-stop the chip before hard-stopping the
+	 * regulators.
+	 */
+	ak8975_set_mode(data, POWER_DOWN);
+	ak8975_power_off(data);
+}
+
 static int ak8975_probe(struct i2c_client *client)
 {
 	const struct i2c_device_id *id = i2c_client_get_device_id(client);
+	struct device *dev = &client->dev;
 	struct ak8975_data *data;
 	struct iio_dev *indio_dev;
 	struct gpio_desc *eoc_gpiod;
@@ -970,10 +988,21 @@ static int ak8975_probe(struct i2c_client *client)
 	if (ret)
 		return ret;
 
+	/*
+	 * Set device as active early so pm_runtime_status_suspended() works
+	 * correctly if probe fails before pm_runtime is enabled. Do not attempt
+	 * to move this line lower.
+	 */
+	pm_runtime_set_active(dev);
+
+	ret = devm_add_action_or_reset(dev, devm_ak8975_power_off, data);
+	if (ret)
+		return ret;
+
 	ret = ak8975_who_i_am(client, data->def->type);
 	if (ret) {
 		dev_err(&client->dev, "Unexpected device\n");
-		goto power_off;
+		return ret;
 	}
 	dev_dbg(&client->dev, "Asahi compass chip %s\n", name);
 
@@ -981,10 +1010,13 @@ static int ak8975_probe(struct i2c_client *client)
 	ret = ak8975_setup(client);
 	if (ret) {
 		dev_err(&client->dev, "%s initialization fails\n", name);
-		goto power_off;
+		return ret;
 	}
 
-	mutex_init(&data->lock);
+	ret = devm_mutex_init(dev, &data->lock);
+	if (ret)
+		return ret;
+
 	indio_dev->channels = ak8975_channels;
 	indio_dev->num_channels = ARRAY_SIZE(ak8975_channels);
 	indio_dev->info = &ak8975_info;
@@ -992,52 +1024,32 @@ static int ak8975_probe(struct i2c_client *client)
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->name = name;
 
-	ret = iio_triggered_buffer_setup(indio_dev, NULL, ak8975_handle_trigger,
-					 NULL);
+	ret = devm_iio_triggered_buffer_setup(dev, indio_dev, NULL,
+					      ak8975_handle_trigger, NULL);
 	if (ret) {
 		dev_err(&client->dev, "triggered buffer setup failed\n");
-		goto power_off;
+		return ret;
 	}
 
-	ret = iio_device_register(indio_dev);
+	ret = devm_iio_device_register(dev, indio_dev);
 	if (ret) {
 		dev_err(&client->dev, "device register failed\n");
-		goto cleanup_buffer;
+		return ret;
 	}
 
 	/* Enable runtime PM */
-	pm_runtime_get_noresume(&client->dev);
-	pm_runtime_set_active(&client->dev);
-	pm_runtime_enable(&client->dev);
+	ret = devm_pm_runtime_enable(dev);
+	if (ret)
+		return ret;
+
 	/*
 	 * The device comes online in 500us, so add two orders of magnitude
 	 * of delay before autosuspending: 50 ms.
 	 */
 	pm_runtime_set_autosuspend_delay(&client->dev, 50);
 	pm_runtime_use_autosuspend(&client->dev);
-	pm_runtime_put(&client->dev);
 
 	return 0;
-
-cleanup_buffer:
-	iio_triggered_buffer_cleanup(indio_dev);
-power_off:
-	ak8975_power_off(data);
-	return ret;
-}
-
-static void ak8975_remove(struct i2c_client *client)
-{
-	struct iio_dev *indio_dev = i2c_get_clientdata(client);
-	struct ak8975_data *data = iio_priv(indio_dev);
-
-	pm_runtime_get_sync(&client->dev);
-	pm_runtime_put_noidle(&client->dev);
-	pm_runtime_disable(&client->dev);
-	iio_device_unregister(indio_dev);
-	iio_triggered_buffer_cleanup(indio_dev);
-	ak8975_set_mode(data, POWER_DOWN);
-	ak8975_power_off(data);
 }
 
 static int ak8975_runtime_suspend(struct device *dev)
@@ -1131,7 +1143,6 @@ static struct i2c_driver ak8975_driver = {
 		.acpi_match_table = ak_acpi_match,
 	},
 	.probe		= ak8975_probe,
-	.remove		= ak8975_remove,
 	.id_table	= ak8975_id,
 };
 module_i2c_driver(ak8975_driver);

-- 
2.47.3

Re: [PATCH v5 14/18] iio: magnetometer: ak8975: switch to using managed resources

[Joshua Crofts]

On Tue, 5 May 2026 at 13:46, Joshua Crofts via B4 Relay
<devnull+joshua.crofts1.gmail.com@kernel.org> wrote:
>
> From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>
> Switch the driver to use managed resources (devm_*) which simplifier
> error handling and allows removing ak8975_remove() method from
> the driver.
>
> Note, on error path we now also set mode to POWER_DOWN state which is
> fine. Even if the device is in that mode, there is no problem to set
> that mode again, it should be no-op.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Co-developed-by: Joshua Crofts <joshua.crofts1@gmail.com>
> Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>

BTW, Sashiko timed out while reviewing this patch, didn't even know that's
possible.

-- 
Kind regards

CJD

Re: [PATCH v5 14/18] iio: magnetometer: ak8975: switch to using managed resources

[Andy Shevchenko]

On Tue, May 05, 2026 at 04:34:29PM +0200, Joshua Crofts wrote:
> On Tue, 5 May 2026 at 13:46, Joshua Crofts via B4 Relay
> <devnull+joshua.crofts1.gmail.com@kernel.org> wrote:
> >
> > Switch the driver to use managed resources (devm_*) which simplifier
> > error handling and allows removing ak8975_remove() method from
> > the driver.
> >
> > Note, on error path we now also set mode to POWER_DOWN state which is
> > fine. Even if the device is in that mode, there is no problem to set
> > that mode again, it should be no-op.
> >
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > Co-developed-by: Joshua Crofts <joshua.crofts1@gmail.com>
> > Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
> 
> BTW, Sashiko timed out while reviewing this patch, didn't even know that's
> possible.

You can ask Roman what to do in such a case.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 14/18] iio: magnetometer: ak8975: switch to using managed resources

[Jonathan Cameron]

On Tue, 05 May 2026 13:46:10 +0200
Joshua Crofts via B4 Relay <devnull+joshua.crofts1.gmail.com@kernel.org> wrote:

> From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> Switch the driver to use managed resources (devm_*) which simplifier
> error handling and allows removing ak8975_remove() method from
> the driver.
> 
> Note, on error path we now also set mode to POWER_DOWN state which is
> fine. Even if the device is in that mode, there is no problem to set
> that mode again, it should be no-op.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Co-developed-by: Joshua Crofts <joshua.crofts1@gmail.com>
> Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
Hi Joshua

Looks fine, but one stale comment needs an update.

Jonathan

> ---
>  drivers/iio/magnetometer/ak8975.c | 77 ++++++++++++++++++++++-----------------
>  1 file changed, 44 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
> index 1c05e380a4d4f968f505a7d5c0acbec86ad949e1..84ccbca758b0121a9c4930a368cb113471d389da 100644
> --- a/drivers/iio/magnetometer/ak8975.c
> +++ b/drivers/iio/magnetometer/ak8975.c
> @@ -900,9 +900,27 @@ static irqreturn_t ak8975_handle_trigger(int irq, void *p)
>  	return IRQ_HANDLED;
>  }
>  
> +static void devm_ak8975_power_off(void *data)
> +{
> +	struct ak8975_data *ak = data;
> +	struct device *dev = &ak->client->dev;
> +
> +	/* Only power down if currently active */
> +	if (pm_runtime_status_suspended(dev))
> +		return;
> +	/*
> +	 * The device may not be runtime suspended when the driver is

If we get here it's definitely not runtime suspended.  Hence
this comment needs a rewrite.

> +	 * removed, so we soft-stop the chip before hard-stopping the
> +	 * regulators.
> +	 */
> +	ak8975_set_mode(data, POWER_DOWN);
I went looking into how we get into different modes...
This deals with a latent bug in ak8975_setup() in that it can be in fuse mode
in some exit paths.  Arguably if we had an i2c bus fail we probably can't
change the mode anyway, but we should be trying from point of view
of no side effects. I wonder if we should fix that in a precursor to this
patch (given the amount of churn from earlier patches an obscure corner
case of the bug - I don't plan to mark it for backports either way!)

Maybe it's just not worth the bother given this is now here anyway.

> +	ak8975_power_off(data);
> +}
> +
>  static int ak8975_probe(struct i2c_client *client)
>  {
>  	const struct i2c_device_id *id = i2c_client_get_device_id(client);
> +	struct device *dev = &client->dev;
>  	struct ak8975_data *data;
>  	struct iio_dev *indio_dev;
>  	struct gpio_desc *eoc_gpiod;
> @@ -970,10 +988,21 @@ static int ak8975_probe(struct i2c_client *client)
>  	if (ret)
>  		return ret;
>  
> +	/*
> +	 * Set device as active early so pm_runtime_status_suspended() works
> +	 * correctly if probe fails before pm_runtime is enabled. Do not attempt
> +	 * to move this line lower.
> +	 */
> +	pm_runtime_set_active(dev);
> +
> +	ret = devm_add_action_or_reset(dev, devm_ak8975_power_off, data);
> +	if (ret)
> +		return ret;
> +
>  	ret = ak8975_who_i_am(client, data->def->type);
>  	if (ret) {
>  		dev_err(&client->dev, "Unexpected device\n");
> -		goto power_off;
> +		return ret;
>  	}
>  	dev_dbg(&client->dev, "Asahi compass chip %s\n", name);
>  
> @@ -981,10 +1010,13 @@ static int ak8975_probe(struct i2c_client *client)
>  	ret = ak8975_setup(client);
>  	if (ret) {
>  		dev_err(&client->dev, "%s initialization fails\n", name);
> -		goto power_off;
> +		return ret;
>  	}
>  
> -	mutex_init(&data->lock);
> +	ret = devm_mutex_init(dev, &data->lock);
> +	if (ret)
> +		return ret;
> +
>  	indio_dev->channels = ak8975_channels;
>  	indio_dev->num_channels = ARRAY_SIZE(ak8975_channels);
>  	indio_dev->info = &ak8975_info;
> @@ -992,52 +1024,32 @@ static int ak8975_probe(struct i2c_client *client)
>  	indio_dev->modes = INDIO_DIRECT_MODE;
>  	indio_dev->name = name;
>  
> -	ret = iio_triggered_buffer_setup(indio_dev, NULL, ak8975_handle_trigger,
> -					 NULL);
> +	ret = devm_iio_triggered_buffer_setup(dev, indio_dev, NULL,
> +					      ak8975_handle_trigger, NULL);
>  	if (ret) {
>  		dev_err(&client->dev, "triggered buffer setup failed\n");
> -		goto power_off;
> +		return ret;
>  	}
>  
> -	ret = iio_device_register(indio_dev);
> +	ret = devm_iio_device_register(dev, indio_dev);
>  	if (ret) {
>  		dev_err(&client->dev, "device register failed\n");
> -		goto cleanup_buffer;
> +		return ret;
>  	}
>  
>  	/* Enable runtime PM */
> -	pm_runtime_get_noresume(&client->dev);

huh. Not sure what this was doing in original code.
I guess forced an immediate power down rather that after an autosuspend
period.  I think that's fine to change - however perhaps do a bit
of analysis on whether my assumption is correct on what this did
and then mention it in the commit message.


> -	pm_runtime_set_active(&client->dev);

> -	pm_runtime_enable(&client->dev);
> +	ret = devm_pm_runtime_enable(dev);
> +	if (ret)
> +		return ret;
> +
>  	/*
>  	 * The device comes online in 500us, so add two orders of magnitude
>  	 * of delay before autosuspending: 50 ms.
>  	 */
>  	pm_runtime_set_autosuspend_delay(&client->dev, 50);
>  	pm_runtime_use_autosuspend(&client->dev);
> -	pm_runtime_put(&client->dev);
>  
>  	return 0;

[PATCH v5 15/18] iio: magnetometer: ak8975: consistently use 'data' parameter

[Joshua Crofts via B4 Relay]

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Some of the functions use 'client', some use 'data', and some use both.
Refactor the driver to consistently use 'data' in all cases.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
---
 drivers/iio/magnetometer/ak8975.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
index 84ccbca758b0121a9c4930a368cb113471d389da..d40ce26e3a47d5a5c9c81d49c128d136208a343a 100644
--- a/drivers/iio/magnetometer/ak8975.c
+++ b/drivers/iio/magnetometer/ak8975.c
@@ -474,9 +474,10 @@ static void ak8975_power_off(const struct ak8975_data *data)
  * Return 0 if the i2c device is the one we expect.
  * return a negative error number otherwise
  */
-static int ak8975_who_i_am(struct i2c_client *client,
+static int ak8975_who_i_am(const struct ak8975_data *data,
 			   enum asahi_compass_chipset type)
 {
+	struct i2c_client *client = data->client;
 	u8 wia_val[2];
 	int ret;
 
@@ -598,10 +599,9 @@ static int ak8975_setup_irq(struct ak8975_data *data)
  * Perform some start-of-day setup, including reading the asa calibration
  * values and caching them.
  */
-static int ak8975_setup(struct i2c_client *client)
+static int ak8975_setup(struct ak8975_data *data)
 {
-	struct iio_dev *indio_dev = i2c_get_clientdata(client);
-	struct ak8975_data *data = iio_priv(indio_dev);
+	struct i2c_client *client = data->client;
 	int ret;
 
 	/* Write the fused rom access mode. */
@@ -999,7 +999,7 @@ static int ak8975_probe(struct i2c_client *client)
 	if (ret)
 		return ret;
 
-	ret = ak8975_who_i_am(client, data->def->type);
+	ret = ak8975_who_i_am(data, data->def->type);
 	if (ret) {
 		dev_err(&client->dev, "Unexpected device\n");
 		return ret;
@@ -1007,7 +1007,7 @@ static int ak8975_probe(struct i2c_client *client)
 	dev_dbg(&client->dev, "Asahi compass chip %s\n", name);
 
 	/* Perform some basic start-of-day setup of the device. */
-	ret = ak8975_setup(client);
+	ret = ak8975_setup(data);
 	if (ret) {
 		dev_err(&client->dev, "%s initialization fails\n", name);
 		return ret;

-- 
2.47.3

[PATCH v5 16/18] iio: magnetometer: ak8975: unify messages with help of dev_err_probe()

[Joshua Crofts via B4 Relay]

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Unify error messages that might appear during probe phase by
switching to use dev_err_probe().

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
---
 drivers/iio/magnetometer/ak8975.c | 30 ++++++++++--------------------
 1 file changed, 10 insertions(+), 20 deletions(-)

diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
index d40ce26e3a47d5a5c9c81d49c128d136208a343a..b07c56312f7df07ea21c3f378b5721568cb66454 100644
--- a/drivers/iio/magnetometer/ak8975.c
+++ b/drivers/iio/magnetometer/ak8975.c
@@ -495,10 +495,8 @@ static int ak8975_who_i_am(const struct ak8975_data *data,
 							AK09912_REG_WIA1,
 							sizeof(wia_val),
 							wia_val);
-	if (ret < 0) {
-		dev_err(&client->dev, "Error reading WIA\n");
-		return ret;
-	}
+	if (ret < 0)
+		return dev_err_probe(&client->dev, ret, "Error reading WIA\n");
 
 	if (wia_val[0] != AK8975_DEVICE_ID)
 		return -ENODEV;
@@ -1000,18 +998,14 @@ static int ak8975_probe(struct i2c_client *client)
 		return ret;
 
 	ret = ak8975_who_i_am(data, data->def->type);
-	if (ret) {
-		dev_err(&client->dev, "Unexpected device\n");
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(dev, ret, "Unexpected device\n");
 	dev_dbg(&client->dev, "Asahi compass chip %s\n", name);
 
 	/* Perform some basic start-of-day setup of the device. */
 	ret = ak8975_setup(data);
-	if (ret) {
-		dev_err(&client->dev, "%s initialization fails\n", name);
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(dev, ret, "%s initialization fails\n", name);
 
 	ret = devm_mutex_init(dev, &data->lock);
 	if (ret)
@@ -1026,16 +1020,12 @@ static int ak8975_probe(struct i2c_client *client)
 
 	ret = devm_iio_triggered_buffer_setup(dev, indio_dev, NULL,
 					      ak8975_handle_trigger, NULL);
-	if (ret) {
-		dev_err(&client->dev, "triggered buffer setup failed\n");
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(dev, ret, "triggered buffer setup failed\n");
 
 	ret = devm_iio_device_register(dev, indio_dev);
-	if (ret) {
-		dev_err(&client->dev, "device register failed\n");
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(dev, ret, "device register failed\n");
 
 	/* Enable runtime PM */
 	ret = devm_pm_runtime_enable(dev);

-- 
2.47.3

[PATCH v5 17/18] iio: magnetometer: ak8975: use temporary variable for struct device

[Joshua Crofts via B4 Relay]

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Use temporary variable for struct device to make code neater.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
---
 drivers/iio/magnetometer/ak8975.c | 61 +++++++++++++++++++--------------------
 1 file changed, 30 insertions(+), 31 deletions(-)

diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
index b07c56312f7df07ea21c3f378b5721568cb66454..1c96df3e1ad95d268470609c61be8ee4a709675d 100644
--- a/drivers/iio/magnetometer/ak8975.c
+++ b/drivers/iio/magnetometer/ak8975.c
@@ -433,18 +433,17 @@ struct ak8975_data {
 /* Enable attached power regulator if any. */
 static int ak8975_power_on(const struct ak8975_data *data)
 {
+	struct device *dev = &data->client->dev;
 	int ret;
 
 	ret = regulator_enable(data->vdd);
 	if (ret) {
-		dev_warn(&data->client->dev,
-			 "Failed to enable specified Vdd supply\n");
+		dev_warn(dev, "Failed to enable specified Vdd supply\n");
 		return ret;
 	}
 	ret = regulator_enable(data->vid);
 	if (ret) {
-		dev_warn(&data->client->dev,
-			 "Failed to enable specified Vid supply\n");
+		dev_warn(dev, "Failed to enable specified Vid supply\n");
 		regulator_disable(data->vdd);
 		return ret;
 	}
@@ -572,6 +571,7 @@ static irqreturn_t ak8975_irq_handler(int irq, void *data)
 static int ak8975_setup_irq(struct ak8975_data *data)
 {
 	struct i2c_client *client = data->client;
+	struct device *dev = &client->dev;
 	int irq;
 	int ret;
 
@@ -582,9 +582,8 @@ static int ak8975_setup_irq(struct ak8975_data *data)
 	else
 		irq = gpiod_to_irq(data->eoc_gpiod);
 
-	ret = devm_request_irq(&client->dev, irq, ak8975_irq_handler,
-			       IRQF_TRIGGER_RISING,
-			       dev_name(&client->dev), data);
+	ret = devm_request_irq(dev, irq, ak8975_irq_handler, IRQF_TRIGGER_RISING,
+			       dev_name(dev), data);
 	if (ret)
 		return ret;
 
@@ -600,12 +599,13 @@ static int ak8975_setup_irq(struct ak8975_data *data)
 static int ak8975_setup(struct ak8975_data *data)
 {
 	struct i2c_client *client = data->client;
+	struct device *dev = &client->dev;
 	int ret;
 
 	/* Write the fused rom access mode. */
 	ret = ak8975_set_mode(data, FUSE_ROM);
 	if (ret < 0) {
-		dev_err(&client->dev, "Error in setting fuse access mode\n");
+		dev_err(dev, "Error in setting fuse access mode\n");
 		return ret;
 	}
 
@@ -615,22 +615,21 @@ static int ak8975_setup(struct ak8975_data *data)
 							sizeof(data->asa),
 							data->asa);
 	if (ret < 0) {
-		dev_err(&client->dev, "Not able to read asa data\n");
+		dev_err(dev, "Not able to read asa data\n");
 		return ret;
 	}
 
 	/* After reading fuse ROM data set power-down mode */
 	ret = ak8975_set_mode(data, POWER_DOWN);
 	if (ret < 0) {
-		dev_err(&client->dev, "Error in setting power-down mode\n");
+		dev_err(dev, "Error in setting power-down mode\n");
 		return ret;
 	}
 
 	if (data->eoc_gpiod || client->irq > 0) {
 		ret = ak8975_setup_irq(data);
 		if (ret < 0) {
-			dev_err(&client->dev,
-				"Error setting data ready interrupt\n");
+			dev_err(dev, "Error setting data ready interrupt\n");
 			return ret;
 		}
 	}
@@ -738,10 +737,11 @@ static int ak8975_read_axis(struct iio_dev *indio_dev, int index, int *val)
 	struct ak8975_data *data = iio_priv(indio_dev);
 	const struct i2c_client *client = data->client;
 	const struct ak_def *def = data->def;
+	struct device *dev = &data->client->dev;
 	__le16 rval;
 	int ret;
 
-	pm_runtime_get_sync(&data->client->dev);
+	pm_runtime_get_sync(dev);
 
 	mutex_lock(&data->lock);
 
@@ -759,20 +759,20 @@ static int ak8975_read_axis(struct iio_dev *indio_dev, int index, int *val)
 	/* Read out ST2 for release lock on measurement data. */
 	ret = i2c_smbus_read_byte_data(client, data->def->ctrl_regs[ST2]);
 	if (ret < 0) {
-		dev_err(&client->dev, "Error in reading ST2\n");
+		dev_err(dev, "Error in reading ST2\n");
 		goto exit;
 	}
 
 	if (ret & (data->def->ctrl_masks[ST2_DERR] |
 		   data->def->ctrl_masks[ST2_HOFL])) {
-		dev_err(&client->dev, "ST2 status error 0x%x\n", ret);
+		dev_err(dev, "ST2 status error 0x%x\n", ret);
 		ret = -EINVAL;
 		goto exit;
 	}
 
 	mutex_unlock(&data->lock);
 
-	pm_runtime_put_autosuspend(&data->client->dev);
+	pm_runtime_put_autosuspend(dev);
 
 	/* Swap bytes and convert to valid range. */
 	*val = clamp_t(s16, le16_to_cpu(rval), -def->range, def->range);
@@ -781,7 +781,7 @@ static int ak8975_read_axis(struct iio_dev *indio_dev, int index, int *val)
 
 exit:
 	mutex_unlock(&data->lock);
-	dev_err(&client->dev, "Error in reading axis\n");
+	dev_err(dev, "Error in reading axis\n");
 	return ret;
 }
 
@@ -931,7 +931,7 @@ static int ak8975_probe(struct i2c_client *client)
 	 * We may not have a GPIO based IRQ to scan, that is fine, we will
 	 * poll if so.
 	 */
-	eoc_gpiod = devm_gpiod_get_optional(&client->dev, NULL, GPIOD_IN);
+	eoc_gpiod = devm_gpiod_get_optional(dev, NULL, GPIOD_IN);
 	if (IS_ERR(eoc_gpiod))
 		return PTR_ERR(eoc_gpiod);
 	gpiod_set_consumer_name(eoc_gpiod, "ak_8975");
@@ -941,13 +941,12 @@ static int ak8975_probe(struct i2c_client *client)
 	 * deassert reset on ak8975_power_on() and assert reset on
 	 * ak8975_power_off().
 	 */
-	reset_gpiod = devm_gpiod_get_optional(&client->dev,
-					      "reset", GPIOD_OUT_HIGH);
+	reset_gpiod = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
 	if (IS_ERR(reset_gpiod))
 		return PTR_ERR(reset_gpiod);
 
 	/* Register with IIO */
-	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
 	if (indio_dev == NULL)
 		return -ENOMEM;
 
@@ -959,7 +958,7 @@ static int ak8975_probe(struct i2c_client *client)
 	data->reset_gpiod = reset_gpiod;
 	data->eoc_irq = 0;
 
-	ret = iio_read_mount_matrix(&client->dev, &data->orientation);
+	ret = iio_read_mount_matrix(dev, &data->orientation);
 	if (ret)
 		return ret;
 
@@ -969,16 +968,16 @@ static int ak8975_probe(struct i2c_client *client)
 		return -ENODEV;
 
 	/* If enumerated via firmware node, fix the ABI */
-	if (dev_fwnode(&client->dev))
-		name = dev_name(&client->dev);
+	if (dev_fwnode(dev))
+		name = dev_name(dev);
 	else
 		name = id->name;
 
 	/* Fetch the regulators */
-	data->vdd = devm_regulator_get(&client->dev, "vdd");
+	data->vdd = devm_regulator_get(dev, "vdd");
 	if (IS_ERR(data->vdd))
 		return PTR_ERR(data->vdd);
-	data->vid = devm_regulator_get(&client->dev, "vid");
+	data->vid = devm_regulator_get(dev, "vid");
 	if (IS_ERR(data->vid))
 		return PTR_ERR(data->vid);
 
@@ -1000,7 +999,7 @@ static int ak8975_probe(struct i2c_client *client)
 	ret = ak8975_who_i_am(data, data->def->type);
 	if (ret)
 		return dev_err_probe(dev, ret, "Unexpected device\n");
-	dev_dbg(&client->dev, "Asahi compass chip %s\n", name);
+	dev_dbg(dev, "Asahi compass chip %s\n", name);
 
 	/* Perform some basic start-of-day setup of the device. */
 	ret = ak8975_setup(data);
@@ -1036,8 +1035,8 @@ static int ak8975_probe(struct i2c_client *client)
 	 * The device comes online in 500us, so add two orders of magnitude
 	 * of delay before autosuspending: 50 ms.
 	 */
-	pm_runtime_set_autosuspend_delay(&client->dev, 50);
-	pm_runtime_use_autosuspend(&client->dev);
+	pm_runtime_set_autosuspend_delay(dev, 50);
+	pm_runtime_use_autosuspend(dev);
 
 	return 0;
 }
@@ -1052,7 +1051,7 @@ static int ak8975_runtime_suspend(struct device *dev)
 	/* Set the device in power down if it wasn't already */
 	ret = ak8975_set_mode(data, POWER_DOWN);
 	if (ret < 0) {
-		dev_err(&client->dev, "Error in setting power-down mode\n");
+		dev_err(dev, "Error in setting power-down mode\n");
 		return ret;
 	}
 	/* Next cut the regulators */
@@ -1076,7 +1075,7 @@ static int ak8975_runtime_resume(struct device *dev)
 	 */
 	ret = ak8975_set_mode(data, POWER_DOWN);
 	if (ret < 0) {
-		dev_err(&client->dev, "Error in setting power-down mode\n");
+		dev_err(dev, "Error in setting power-down mode\n");
 		return ret;
 	}
 

-- 
2.47.3

[PATCH v5 18/18] iio: magnetometer: ak8975: make use of the macros from bits.h

[Joshua Crofts via B4 Relay]

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Make use of BIT() and GENMASK() where it makes sense.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Co-developed-by: Joshua Crofts <joshua.crofts1@gmail.com>
Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
---
 drivers/iio/magnetometer/ak8975.c | 23 ++++++++---------------
 1 file changed, 8 insertions(+), 15 deletions(-)

diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
index 1c96df3e1ad95d268470609c61be8ee4a709675d..ea024390b36d4a13185ec605de9d808655cab261 100644
--- a/drivers/iio/magnetometer/ak8975.c
+++ b/drivers/iio/magnetometer/ak8975.c
@@ -45,8 +45,7 @@
 #define AK8975_REG_INFO			0x01
 
 #define AK8975_REG_ST1			0x02
-#define AK8975_REG_ST1_DRDY_SHIFT	0
-#define AK8975_REG_ST1_DRDY_MASK	(1 << AK8975_REG_ST1_DRDY_SHIFT)
+#define AK8975_REG_ST1_DRDY_MASK	BIT(0)
 
 #define AK8975_REG_HXL			0x03
 #define AK8975_REG_HXH			0x04
@@ -55,15 +54,12 @@
 #define AK8975_REG_HZL			0x07
 #define AK8975_REG_HZH			0x08
 #define AK8975_REG_ST2			0x09
-#define AK8975_REG_ST2_DERR_SHIFT	2
-#define AK8975_REG_ST2_DERR_MASK	(1 << AK8975_REG_ST2_DERR_SHIFT)
+#define AK8975_REG_ST2_DERR_MASK	BIT(2)
 
-#define AK8975_REG_ST2_HOFL_SHIFT	3
-#define AK8975_REG_ST2_HOFL_MASK	(1 << AK8975_REG_ST2_HOFL_SHIFT)
+#define AK8975_REG_ST2_HOFL_MASK	BIT(3)
 
 #define AK8975_REG_CNTL			0x0A
-#define AK8975_REG_CNTL_MODE_SHIFT	0
-#define AK8975_REG_CNTL_MODE_MASK	(0xF << AK8975_REG_CNTL_MODE_SHIFT)
+#define AK8975_REG_CNTL_MODE_MASK	GENMASK(3, 0)
 #define AK8975_REG_CNTL_MODE_POWER_DOWN	0x00
 #define AK8975_REG_CNTL_MODE_ONCE	0x01
 #define AK8975_REG_CNTL_MODE_SELF_TEST	0x08
@@ -95,8 +91,7 @@
 
 #define AK09912_REG_ST1			0x10
 
-#define AK09912_REG_ST1_DRDY_SHIFT	0
-#define AK09912_REG_ST1_DRDY_MASK	(1 << AK09912_REG_ST1_DRDY_SHIFT)
+#define AK09912_REG_ST1_DRDY_MASK	BIT(0)
 
 #define AK09912_REG_HXL			0x11
 #define AK09912_REG_HXH			0x12
@@ -107,8 +102,7 @@
 #define AK09912_REG_TMPS		0x17
 
 #define AK09912_REG_ST2			0x18
-#define AK09912_REG_ST2_HOFL_SHIFT	3
-#define AK09912_REG_ST2_HOFL_MASK	(1 << AK09912_REG_ST2_HOFL_SHIFT)
+#define AK09912_REG_ST2_HOFL_MASK	BIT(3)
 
 #define AK09912_REG_CNTL1		0x30
 
@@ -117,8 +111,7 @@
 #define AK09912_REG_CNTL_MODE_ONCE	0x01
 #define AK09912_REG_CNTL_MODE_SELF_TEST	0x10
 #define AK09912_REG_CNTL_MODE_FUSE_ROM	0x1F
-#define AK09912_REG_CNTL2_MODE_SHIFT	0
-#define AK09912_REG_CNTL2_MODE_MASK	(0x1F << AK09912_REG_CNTL2_MODE_SHIFT)
+#define AK09912_REG_CNTL2_MODE_MASK	GENMASK(4, 0)
 
 #define AK09912_REG_CNTL3		0x32
 
@@ -840,7 +833,7 @@ static const struct iio_chan_spec ak8975_channels[] = {
 	IIO_CHAN_SOFT_TIMESTAMP(3),
 };
 
-static const unsigned long ak8975_scan_masks[] = { 0x7, 0 };
+static const unsigned long ak8975_scan_masks[] = { GENMASK(2, 0), 0 };
 
 static const struct iio_info ak8975_info = {
 	.read_raw = &ak8975_read_raw,

-- 
2.47.3

Re: [PATCH v5 18/18] iio: magnetometer: ak8975: make use of the macros from bits.h

[Jonathan Cameron]

On Tue, 05 May 2026 13:46:14 +0200
Joshua Crofts via B4 Relay <devnull+joshua.crofts1.gmail.com@kernel.org> wrote:

> From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> Make use of BIT() and GENMASK() where it makes sense.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Co-developed-by: Joshua Crofts <joshua.crofts1@gmail.com>
> Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>

One small thing in here.  Feel free to just leave that for a separate patch
and for now leave it as 0x7

> @@ -840,7 +833,7 @@ static const struct iio_chan_spec ak8975_channels[] = {
>  	IIO_CHAN_SOFT_TIMESTAMP(3),
>  };
>  
> -static const unsigned long ak8975_scan_masks[] = { 0x7, 0 };
> +static const unsigned long ak8975_scan_masks[] = { GENMASK(2, 0), 0 };
Hmm. This is a fun one... It's actually 3 separate bits so please represent it
as BIT(0) | BIT(1) | BIT(2)

or maybe add an enum for the channel scan indexes?
BIT(SCAN_X) | BIT(SCAN_Y) | BIT(SCAN_Z) 
(and there will be an unused here SCAN_TIMESTAMP

	
>  
>  static const struct iio_info ak8975_info = {
>  	.read_raw = &ak8975_read_raw,
> 

Re: [PATCH v5 18/18] iio: magnetometer: ak8975: make use of the macros from bits.h

[Andy Shevchenko]

On Wed, May 06, 2026 at 06:12:16PM +0100, Jonathan Cameron wrote:
> On Tue, 05 May 2026 13:46:14 +0200
> Joshua Crofts via B4 Relay <devnull+joshua.crofts1.gmail.com@kernel.org> wrote:

...

> > Make use of BIT() and GENMASK() where it makes sense.

> One small thing in here.  Feel free to just leave that for a separate patch
> and for now leave it as 0x7

...

> > -static const unsigned long ak8975_scan_masks[] = { 0x7, 0 };
> > +static const unsigned long ak8975_scan_masks[] = { GENMASK(2, 0), 0 };
> Hmm. This is a fun one... It's actually 3 separate bits so please represent it
> as BIT(0) | BIT(1) | BIT(2)

Ah, TIL.

> or maybe add an enum for the channel scan indexes?
> BIT(SCAN_X) | BIT(SCAN_Y) | BIT(SCAN_Z) 

This looks good to me.

> (and there will be an unused here SCAN_TIMESTAMP

-- 
With Best Regards,
Andy Shevchenko