[PATCH v0 00/14] iio: magnetometer: ak8975: Additional changes to the driver

[PATCH v0 00/14] iio: magnetometer: ak8975: Additional changes to the driver

[Andy Shevchenko]

This series is an attempt to make the driver less a rabbit hole.
It's a continuation of what Joshua Crofts started doing. Hence
it is based on his work and first three "patches" here are supposed
to be folded to his series accordingly.

I have compile-tested them, but I haven't done full double check of
the correctness from functional point of view. Joshua, please do that
before incorporating into your series. Yes, my patches are assumed to
become the part of Joshua's whatever next version of the series, that's
why mine is marked as v0.

Should not be taken directly by the maintainers, but any comments, review
are highly appreciated.

Andy Shevchenko (14):
  drivers/iio/magnetometer/ak8975.c: fixup for the IWYU change
  drivers/iio/magnetometer/ak8975.c: fixup for the errno fix
  drivers/iio/magnetometer/ak8975.c: fixup for the iopoll.h conversion
  iio: magnetometer: ak8975: Inline timeout constants
  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

 drivers/iio/magnetometer/ak8975.c | 253 +++++++++++++-----------------
 1 file changed, 112 insertions(+), 141 deletions(-)

-- 
2.50.1

[PATCH v0 01/14] drivers/iio/magnetometer/ak8975.c: fixup for the IWYU change

[Andy Shevchenko]

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/iio/magnetometer/ak8975.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
index c09db212a6cc..378940e82550 100644
--- a/drivers/iio/magnetometer/ak8975.c
+++ b/drivers/iio/magnetometer/ak8975.c
@@ -16,7 +16,6 @@
 #include <linux/i2c.h>
 #include <linux/interrupt.h>
 #include <linux/iopoll.h>
-#include <linux/irqreturn.h>
 #include <linux/minmax.h>
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
-- 
2.50.1

[PATCH v0 02/14] drivers/iio/magnetometer/ak8975.c: fixup for the errno fix

[Andy Shevchenko]

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

diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
index 378940e82550..833ee7ca9f46 100644
--- a/drivers/iio/magnetometer/ak8975.c
+++ b/drivers/iio/magnetometer/ak8975.c
@@ -693,7 +693,7 @@ static int wait_conversion_complete_polled(struct ak8975_data *data)
 	return val;
 }
 
-/* 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;
@@ -703,7 +703,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.50.1

[PATCH v0 03/14] drivers/iio/magnetometer/ak8975.c: fixup for the iopoll.h conversion

[Andy Shevchenko]

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.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 833ee7ca9f46..4cc92a15e937 100644
--- a/drivers/iio/magnetometer/ak8975.c
+++ b/drivers/iio/magnetometer/ak8975.c
@@ -657,8 +657,7 @@ static int wait_conversion_complete_gpio(struct ak8975_data *data)
 	int val;
 
 	/* Wait for the conversion to complete. */
-	ret = readx_poll_timeout(gpiod_get_value, data->eoc_gpiod, val,
-				 val != 0,
+	ret = readx_poll_timeout(gpiod_get_value, data->eoc_gpiod, val, val != 0,
 				 AK8975_CONVERSION_DONE_POLL_TIME * USEC_PER_MSEC,
 				 AK8975_MAX_CONVERSION_TIMEOUT * USEC_PER_MSEC);
 	if (ret)
-- 
2.50.1

[PATCH v0 04/14] iio: magnetometer: ak8975: Inline timeout constants

[Andy Shevchenko]

Since we have switched to using macros from iopoll.h it's better to read
when the timeout values are explicitly provided in the parameters of the
respective helpers.

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

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

diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
index 4cc92a15e937..53987f3b13d2 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>
@@ -133,13 +134,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.
@@ -658,8 +652,7 @@ static int wait_conversion_complete_gpio(struct ak8975_data *data)
 
 	/* Wait for the conversion to complete. */
 	ret = readx_poll_timeout(gpiod_get_value, data->eoc_gpiod, val, val != 0,
-				 AK8975_CONVERSION_DONE_POLL_TIME * USEC_PER_MSEC,
-				 AK8975_MAX_CONVERSION_TIMEOUT * USEC_PER_MSEC);
+				 10 * USEC_PER_MSEC, 500 * USEC_PER_MSEC);
 	if (ret)
 		return ret;
 
@@ -678,8 +671,7 @@ static int wait_conversion_complete_polled(struct ak8975_data *data)
 
 	/* Wait for the conversion to complete. */
 	ret = read_poll_timeout(i2c_smbus_read_byte_data, val, val != 0,
-				AK8975_CONVERSION_DONE_POLL_TIME * USEC_PER_MSEC,
-				AK8975_MAX_CONVERSION_TIMEOUT * USEC_PER_MSEC,
+				10 * USEC_PER_MSEC, 500 * USEC_PER_MSEC,
 				true,
 				client, data->def->ctrl_regs[ST1]);
 	if (ret)
@@ -699,7 +691,7 @@ static int wait_conversion_complete_interrupt(struct ak8975_data *data)
 
 	ret = wait_event_timeout(data->data_ready_queue,
 				 test_bit(0, &data->flags),
-				 AK8975_DATA_READY_TIMEOUT);
+				 msecs_to_jiffies(100));
 	clear_bit(0, &data->flags);
 
 	return ret > 0 ? 0 : -ETIMEDOUT;
-- 
2.50.1

Re: [PATCH v0 04/14] iio: magnetometer: ak8975: Inline timeout constants

[Jonathan Cameron]

On Mon, 27 Apr 2026 22:09:49 +0200
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> Since we have switched to using macros from iopoll.h it's better to read
> when the timeout values are explicitly provided in the parameters of the
> respective helpers.
> 
> Besides that, fix the home grown and obviously wrong in some cases the
> jiffy-based timeout.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

One comment inline on pulling the actual numbers for the 1st two
close to each other in the driver (or use local constants for them
if you prefer).
> ---
>  drivers/iio/magnetometer/ak8975.c | 16 ++++------------
>  1 file changed, 4 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
> index 4cc92a15e937..53987f3b13d2 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>
> @@ -133,13 +134,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.
> @@ -658,8 +652,7 @@ static int wait_conversion_complete_gpio(struct ak8975_data *data)
>  
>  	/* Wait for the conversion to complete. */
>  	ret = readx_poll_timeout(gpiod_get_value, data->eoc_gpiod, val, val != 0,
> -				 AK8975_CONVERSION_DONE_POLL_TIME * USEC_PER_MSEC,
> -				 AK8975_MAX_CONVERSION_TIMEOUT * USEC_PER_MSEC);
> +				 10 * USEC_PER_MSEC, 500 * USEC_PER_MSEC);
>  	if (ret)
>  		return ret;
>  
> @@ -678,8 +671,7 @@ static int wait_conversion_complete_polled(struct ak8975_data *data)
>  
>  	/* Wait for the conversion to complete. */
>  	ret = read_poll_timeout(i2c_smbus_read_byte_data, val, val != 0,
> -				AK8975_CONVERSION_DONE_POLL_TIME * USEC_PER_MSEC,
> -				AK8975_MAX_CONVERSION_TIMEOUT * USEC_PER_MSEC,
> +				10 * USEC_PER_MSEC, 500 * USEC_PER_MSEC,

I'm not that keen on these being repeated in two places given they take the same values.
Perhaps we should push them up to the caller?  That is, add them as
parameters of wait_completion_complete_gpio() and wait_completion_polled()

I don't mind if we have them hard coded into those two calls as they are right next
to each other so it should be obvious they values are the same.

Ideally I'd like to see a comment on why these numbers next to those calls.

Fine to pass them in msecs and do the scaling here.  Just name the variables to
make that clear.  poll_msecs timeout_msecs maybe?

>  				true,
>  				client, data->def->ctrl_regs[ST1]);
>  	if (ret)
> @@ -699,7 +691,7 @@ static int wait_conversion_complete_interrupt(struct ak8975_data *data)
>  
>  	ret = wait_event_timeout(data->data_ready_queue,
>  				 test_bit(0, &data->flags),
> -				 AK8975_DATA_READY_TIMEOUT);
> +				 msecs_to_jiffies(100));
>  	clear_bit(0, &data->flags);
>  
>  	return ret > 0 ? 0 : -ETIMEDOUT;

Re: [PATCH v0 04/14] iio: magnetometer: ak8975: Inline timeout constants

[Andy Shevchenko]

On Tue, Apr 28, 2026 at 05:22:12PM +0100, Jonathan Cameron wrote:
> On Mon, 27 Apr 2026 22:09:49 +0200
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

...

> >  	/* Wait for the conversion to complete. */
> >  	ret = read_poll_timeout(i2c_smbus_read_byte_data, val, val != 0,
> > -				AK8975_CONVERSION_DONE_POLL_TIME * USEC_PER_MSEC,
> > -				AK8975_MAX_CONVERSION_TIMEOUT * USEC_PER_MSEC,
> > +				10 * USEC_PER_MSEC, 500 * USEC_PER_MSEC,
> 
> I'm not that keen on these being repeated in two places given they take the same values.
> Perhaps we should push them up to the caller?  That is, add them as
> parameters of wait_completion_complete_gpio() and wait_completion_polled()
> 
> I don't mind if we have them hard coded into those two calls as they are right next
> to each other so it should be obvious they values are the same.
> 
> Ideally I'd like to see a comment on why these numbers next to those calls.
> 
> Fine to pass them in msecs and do the scaling here.  Just name the variables to
> make that clear.  poll_msecs timeout_msecs maybe?

I think we are fine with _ms, so poll_ms, timeout_ms.

For the completeness we may also give a timeout_ms to the third one
(and convert it to jiffies there).

-- 
With Best Regards,
Andy Shevchenko

[PATCH v0 05/14] iio: magnetometer: ak8975: Avoid using temporary variable

[Andy Shevchenko]

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>
---
 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 53987f3b13d2..b70fbef1ea3c 100644
--- a/drivers/iio/magnetometer/ak8975.c
+++ b/drivers/iio/magnetometer/ak8975.c
@@ -729,7 +729,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);
@@ -766,8 +765,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.50.1

[PATCH v0 06/14] iio: magnetometer: ak8975: Drop duplicate NULL check

[Andy Shevchenko]

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>
---
 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 b70fbef1ea3c..288bdb3bf14b 100644
--- a/drivers/iio/magnetometer/ak8975.c
+++ b/drivers/iio/magnetometer/ak8975.c
@@ -906,8 +906,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.50.1

[PATCH v0 07/14] iio: magnetometer: ak8975: remove duplicate error message

[Andy Shevchenko]

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

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.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 288bdb3bf14b..dbaa64c66e4d 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.50.1

[PATCH v0 08/14] iio: magnetometer: ak8975: Reduce usage of magic lengths of the buffer

[Andy Shevchenko]

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>
---
 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 dbaa64c66e4d..7ec1d4848fca 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;
@@ -853,7 +856,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.50.1

[PATCH v0 09/14] iio: magnetometer: ak8975: Unify return code variable name

[Andy Shevchenko]

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>
---
 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 7ec1d4848fca..53158ffd173b 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;
 }
 
 /*
@@ -895,8 +895,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.
@@ -931,9 +931,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);
@@ -954,20 +954,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;
 	}
@@ -980,15 +980,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;
 	}
@@ -1011,7 +1011,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.50.1

[PATCH v0 10/14] iio: magnetometer: ak8975: switch to using managed resources

[Andy Shevchenko]

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>
---
 drivers/iio/magnetometer/ak8975.c | 59 ++++++++++++++-----------------
 1 file changed, 26 insertions(+), 33 deletions(-)

diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
index 53158ffd173b..d666d9d171bd 100644
--- a/drivers/iio/magnetometer/ak8975.c
+++ b/drivers/iio/magnetometer/ak8975.c
@@ -888,9 +888,16 @@ static irqreturn_t ak8975_handle_trigger(int irq, void *p)
 	return IRQ_HANDLED;
 }
 
+static void devm_ak8975_power_off(void *data)
+{
+	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;
@@ -958,10 +965,14 @@ static int ak8975_probe(struct i2c_client *client)
 	if (ret)
 		return ret;
 
+	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);
 
@@ -969,10 +980,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;
@@ -980,52 +994,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_set_active_enabled(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)
@@ -1119,7 +1113,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.50.1

Re: [PATCH v0 10/14] iio: magnetometer: ak8975: switch to using managed resources

[Jonathan Cameron]

On Mon, 27 Apr 2026 22:09:55 +0200
Andy Shevchenko <andriy.shevchenko@linux.intel.com> 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>

One day I'll have time to write up some notes on patterns that work
for runtime pm + devm. I don't think think this one does - note original
code didn't work either as would underflow on regulators in some paths
and hence print warnings.
> ---
>  drivers/iio/magnetometer/ak8975.c | 59 ++++++++++++++-----------------
>  1 file changed, 26 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
> index 53158ffd173b..d666d9d171bd 100644
> --- a/drivers/iio/magnetometer/ak8975.c
> +++ b/drivers/iio/magnetometer/ak8975.c
> @@ -888,9 +888,16 @@ static irqreturn_t ak8975_handle_trigger(int irq, void *p)
>  	return IRQ_HANDLED;
>  }
>  
> +static void devm_ak8975_power_off(void *data)
> +{
> +	ak8975_set_mode(data, POWER_DOWN);

Add a comment somewhere appropriate on what we are undoing with this.

It's a 'might not be runtime suspended' at time of remove I think
rather than the mode being set at that point in probe.

If it is suspended, we shouldn't call the regulator disables
in power_off.  So maybe gate this whole thing on
whether runtime suspended.  If you do that be careful with what
state we are in until runtime PM is enabled.  Maybe you already have that
covered elsewhere.


> +	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;
> @@ -958,10 +965,14 @@ static int ak8975_probe(struct i2c_client *client)
>  	if (ret)
>  		return ret;

If following comments above and below you'd need to set the state
to active here.  If you go that way, make sure to add a comment to
say why it is up here (and hopefully prevent someone thinking they can
move it down).

>  
> +	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);
>  
> @@ -969,10 +980,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;
> @@ -980,52 +994,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_set_active_enabled(dev);

This hits the race with whether we can query if the device is disabled mentioned
above. Dance where we need to be able to check that before we turn runtime pm on
so that we can correctly do regulator disables if we error out before here.

> +	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;
> -}

[PATCH v0 11/14] iio: magnetometer: ak8975: Consistently use 'data' parameter

[Andy Shevchenko]

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>
---
 drivers/iio/magnetometer/ak8975.c | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
index d666d9d171bd..b734a5939f73 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. */
@@ -697,12 +697,13 @@ static int wait_conversion_complete_interrupt(struct ak8975_data *data)
 	return ret > 0 ? 0 : -ETIMEDOUT;
 }
 
-static int ak8975_start_read_axis(struct ak8975_data *data,
-				  const struct i2c_client *client)
+static int ak8975_start_read_axis(struct ak8975_data *data)
 {
-	/* Set up the device for taking a sample. */
-	int ret = ak8975_set_mode(data, MODE_ONCE);
+	struct i2c_client *client = data->client;
+	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;
@@ -735,7 +736,7 @@ static int ak8975_read_axis(struct iio_dev *indio_dev, int index, int *val)
 
 	mutex_lock(&data->lock);
 
-	ret = ak8975_start_read_axis(data, client);
+	ret = ak8975_start_read_axis(data);
 	if (ret)
 		goto exit;
 
@@ -846,7 +847,7 @@ static void ak8975_fill_buffer(struct iio_dev *indio_dev)
 
 	mutex_lock(&data->lock);
 
-	ret = ak8975_start_read_axis(data, client);
+	ret = ak8975_start_read_axis(data);
 	if (ret)
 		goto unlock;
 
@@ -969,7 +970,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;
@@ -977,7 +978,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.50.1

[PATCH v0 12/14] iio: magnetometer: ak8975: Unify messages with help of dev_err_probe()

[Andy Shevchenko]

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>
---
 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 b734a5939f73..c2eb7b0d0da8 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;
@@ -971,18 +969,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)
@@ -997,16 +991,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_set_active_enabled(dev);
-- 
2.50.1

Re: [PATCH v0 12/14] iio: magnetometer: ak8975: Unify messages with help of dev_err_probe()

[Joshua Crofts]

On Mon, 27 Apr 2026 at 22:14, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> +       if (ret)
> +               return dev_err_probe(dev, ret, "%s initialization fails\n", name);

Minor typo, better to have "initialization failed", but that was already
in the original codebase.

> +       if (ret)
> +               return dev_err_probe(dev, ret, "device register failed\n");

Same here, "failed to register device" would probably be better imo.

-- 
Kind regards

CJD

Re: [PATCH v0 12/14] iio: magnetometer: ak8975: Unify messages with help of dev_err_probe()

[Andy Shevchenko]

On Tue, Apr 28, 2026 at 11:21:29AM +0200, Joshua Crofts wrote:
> On Mon, 27 Apr 2026 at 22:14, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:

...

> > +       if (ret)
> > +               return dev_err_probe(dev, ret, "%s initialization fails\n", name);
> 
> Minor typo, better to have "initialization failed", but that was already
> in the original codebase.
> 
> > +       if (ret)
> > +               return dev_err_probe(dev, ret, "device register failed\n");
> 
> Same here, "failed to register device" would probably be better imo.

Hmm... Not sure about these. I would leave as is and do a separate change.
(In principle I agree that messages may be improved.)

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v0 12/14] iio: magnetometer: ak8975: Unify messages with help of dev_err_probe()

[Joshua Crofts]

On Tue, 28 Apr 2026 at 11:47, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> Hmm... Not sure about these. I would leave as is and do a separate change.
> (In principle I agree that messages may be improved.)

Okay, in another patch series then.

-- 
Kind regards

CJD

[PATCH v0 13/14] iio: magnetometer: ak8975: Use temporary variable for struct device

[Andy Shevchenko]

Use temporary variable for struct device to make code neater.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.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 c2eb7b0d0da8..e6ca30b36965 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;
 		}
 	}
@@ -727,10 +726,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);
 
@@ -748,20 +748,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);
@@ -770,7 +770,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;
 }
 
@@ -909,7 +909,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");
@@ -919,13 +919,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;
 
@@ -937,7 +936,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;
 
@@ -947,16 +946,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);
 
@@ -971,7 +970,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);
@@ -1007,8 +1006,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;
 }
@@ -1023,7 +1022,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 */
@@ -1047,7 +1046,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.50.1

[PATCH v0 14/14] iio: magnetometer: ak8975: Make use of the macros from bits.h

[Andy Shevchenko]

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

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/iio/magnetometer/ak8975.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
index e6ca30b36965..4e53c61c8d69 100644
--- a/drivers/iio/magnetometer/ak8975.c
+++ b/drivers/iio/magnetometer/ak8975.c
@@ -46,7 +46,7 @@
 
 #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
@@ -56,14 +56,14 @@
 #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
@@ -96,7 +96,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
@@ -108,7 +108,7 @@
 
 #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
 
@@ -118,7 +118,7 @@
 #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
 
@@ -829,7 +829,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.50.1

Re: [PATCH v0 14/14] iio: magnetometer: ak8975: Make use of the macros from bits.h

[Joshua Crofts]

On Mon, 27 Apr 2026 at 22:14, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> Make use of BIT() and GENMASK() where it makes sense.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/iio/magnetometer/ak8975.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
> index e6ca30b36965..4e53c61c8d69 100644
> --- a/drivers/iio/magnetometer/ak8975.c
> +++ b/drivers/iio/magnetometer/ak8975.c
> @@ -46,7 +46,7 @@
>
>  #define AK8975_REG_ST1                 0x02
>  #define AK8975_REG_ST1_DRDY_SHIFT      0

Might as well remove these types of macros, as they are
otherwise unused in the code after this patch. Either that
or incorporate them into the BIT() and GENMASK()
macros to prevent magic numbers.

> -#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
> @@ -56,14 +56,14 @@
>  #define AK8975_REG_HZH                 0x08
>  #define AK8975_REG_ST2                 0x09
>  #define AK8975_REG_ST2_DERR_SHIFT      2

Here.

> -#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

Here.

> -#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

Here.

> -#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
> @@ -96,7 +96,7 @@
>  #define AK09912_REG_ST1                        0x10
>
>  #define AK09912_REG_ST1_DRDY_SHIFT     0

Here.

> -#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
> @@ -108,7 +108,7 @@
>
>  #define AK09912_REG_ST2                        0x18
>  #define AK09912_REG_ST2_HOFL_SHIFT     3

Here.

> -#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
>
> @@ -118,7 +118,7 @@
>  #define AK09912_REG_CNTL_MODE_SELF_TEST        0x10
>  #define AK09912_REG_CNTL_MODE_FUSE_ROM 0x1F
>  #define AK09912_REG_CNTL2_MODE_SHIFT   0

Here.

> -#define AK09912_REG_CNTL2_MODE_MASK    (0x1F << AK09912_REG_CNTL2_MODE_SHIFT)
> +#define AK09912_REG_CNTL2_MODE_MASK    GENMASK(4, 0)

-- 
Kind regards

CJD

Re: [PATCH v0 14/14] iio: magnetometer: ak8975: Make use of the macros from bits.h

[Andy Shevchenko]

On Tue, Apr 28, 2026 at 09:23:28AM +0200, Joshua Crofts wrote:
> On Mon, 27 Apr 2026 at 22:14, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > Make use of BIT() and GENMASK() where it makes sense.

...

> >  #define AK8975_REG_ST1_DRDY_SHIFT      0
> 
> Might as well remove these types of macros, as they are
> otherwise unused in the code after this patch. Either that
> or incorporate them into the BIT() and GENMASK()
> macros to prevent magic numbers.

Good catch! Please, adjust this patch accordingly in your local branch before
sending out.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v0 00/14] iio: magnetometer: ak8975: Additional changes to the driver

[Andy Shevchenko]

On Mon, Apr 27, 2026 at 10:09:45PM +0200, Andy Shevchenko wrote:
> This series is an attempt to make the driver less a rabbit hole.

FWIW, the more refactoring and cleaning up may be
- moving to guard()() - requires preparatory refactoring
- splitting chip_info array to individual data, getting rid of .type field

When I looked into them, I understood that my time slot for this driver
is fully consumed (and even more) already, so I leave it to the motivated
people.

> It's a continuation of what Joshua Crofts started doing. Hence
> it is based on his work and first three "patches" here are supposed
> to be folded to his series accordingly.
> 
> I have compile-tested them, but I haven't done full double check of
> the correctness from functional point of view. Joshua, please do that
> before incorporating into your series. Yes, my patches are assumed to
> become the part of Joshua's whatever next version of the series, that's
> why mine is marked as v0.
> 
> Should not be taken directly by the maintainers, but any comments, review
> are highly appreciated.


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v0 00/14] iio: magnetometer: ak8975: Additional changes to the driver

[Joshua Crofts]

On Mon, 27 Apr 2026 at 22:14, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> This series is an attempt to make the driver less a rabbit hole.
> It's a continuation of what Joshua Crofts started doing. Hence
> it is based on his work and first three "patches" here are supposed
> to be folded to his series accordingly.
>
> I have compile-tested them, but I haven't done full double check of
> the correctness from functional point of view. Joshua, please do that
> before incorporating into your series. Yes, my patches are assumed to
> become the part of Joshua's whatever next version of the series, that's
> why mine is marked as v0.
>
> Should not be taken directly by the maintainers, but any comments, review
> are highly appreciated.

When you said a few changes, I didn't expect an 11-part series :). I'll go over
them and am them to my branch. I might check out the guard()() and chip_info
changes.

-- 
Kind regards

CJD

Re: [PATCH v0 00/14] iio: magnetometer: ak8975: Additional changes to the driver

[Andy Shevchenko]

On Tue, Apr 28, 2026 at 08:37:22AM +0200, Joshua Crofts wrote:
> On Mon, 27 Apr 2026 at 22:14, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > This series is an attempt to make the driver less a rabbit hole.
> > It's a continuation of what Joshua Crofts started doing. Hence
> > it is based on his work and first three "patches" here are supposed
> > to be folded to his series accordingly.
> >
> > I have compile-tested them, but I haven't done full double check of
> > the correctness from functional point of view. Joshua, please do that
> > before incorporating into your series. Yes, my patches are assumed to
> > become the part of Joshua's whatever next version of the series, that's
> > why mine is marked as v0.
> >
> > Should not be taken directly by the maintainers, but any comments, review
> > are highly appreciated.
> 
> When you said a few changes, I didn't expect an 11-part series :). I'll go over
> them and am them to my branch. I might check out the guard()() and chip_info
> changes.

On top of that also one may consider to switching to use reset-gpio instead of
poking "reset" GPIO directly. But this sounds like too many for one go, let's
make this 17 patch series go (6 yours + 11 mine).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v0 00/14] iio: magnetometer: ak8975: Additional changes to the driver

[Joshua Crofts]

On Tue, 28 Apr 2026 at 09:03, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Tue, Apr 28, 2026 at 08:37:22AM +0200, Joshua Crofts wrote:
> > On Mon, 27 Apr 2026 at 22:14, Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> > >
> > > This series is an attempt to make the driver less a rabbit hole.
> > > It's a continuation of what Joshua Crofts started doing. Hence
> > > it is based on his work and first three "patches" here are supposed
> > > to be folded to his series accordingly.
> > >
> > > I have compile-tested them, but I haven't done full double check of
> > > the correctness from functional point of view. Joshua, please do that
> > > before incorporating into your series. Yes, my patches are assumed to
> > > become the part of Joshua's whatever next version of the series, that's
> > > why mine is marked as v0.
> > >
> > > Should not be taken directly by the maintainers, but any comments, review
> > > are highly appreciated.
> >
> > When you said a few changes, I didn't expect an 11-part series :). I'll go over
> > them and am them to my branch. I might check out the guard()() and chip_info
> > changes.
>
> On top of that also one may consider to switching to use reset-gpio instead of
> poking "reset" GPIO directly. But this sounds like too many for one go, let's
> make this 17 patch series go (6 yours + 11 mine).

Fair enough, perhaps another time.

BTW, if I'm sending your patches to the list, I should also add a Signed-off-by
tag to the end, right?

-- 
Kind regards

CJD

Re: [PATCH v0 00/14] iio: magnetometer: ak8975: Additional changes to the driver

[Jonathan Cameron]

On Tue, 28 Apr 2026 09:14:55 +0200
Joshua Crofts <joshua.crofts1@gmail.com> wrote:

> On Tue, 28 Apr 2026 at 09:03, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > On Tue, Apr 28, 2026 at 08:37:22AM +0200, Joshua Crofts wrote:  
> > > On Mon, 27 Apr 2026 at 22:14, Andy Shevchenko
> > > <andriy.shevchenko@linux.intel.com> wrote:  
> > > >
> > > > This series is an attempt to make the driver less a rabbit hole.
> > > > It's a continuation of what Joshua Crofts started doing. Hence
> > > > it is based on his work and first three "patches" here are supposed
> > > > to be folded to his series accordingly.
> > > >
> > > > I have compile-tested them, but I haven't done full double check of
> > > > the correctness from functional point of view. Joshua, please do that
> > > > before incorporating into your series. Yes, my patches are assumed to
> > > > become the part of Joshua's whatever next version of the series, that's
> > > > why mine is marked as v0.
> > > >
> > > > Should not be taken directly by the maintainers, but any comments, review
> > > > are highly appreciated.  
> > >
> > > When you said a few changes, I didn't expect an 11-part series :). I'll go over
> > > them and am them to my branch. I might check out the guard()() and chip_info
> > > changes.  
> >
> > On top of that also one may consider to switching to use reset-gpio instead of
> > poking "reset" GPIO directly. But this sounds like too many for one go, let's
> > make this 17 patch series go (6 yours + 11 mine).  
> 
> Fair enough, perhaps another time.
> 
> BTW, if I'm sending your patches to the list, I should also add a Signed-off-by
> tag to the end, right?
> 
Yes.  You are 'handling' the patches, so need to add your sign off.

Re: [PATCH v0 00/14] iio: magnetometer: ak8975: Additional changes to the driver

[Joshua Crofts]

On Tue, 28 Apr 2026 at 18:16, Jonathan Cameron <jic23@kernel.org> wrote:
> Yes.  You are 'handling' the patches, so need to add your sign off.

Thanks for the clarification.

-- 
Kind regards

CJD

Re: [PATCH v0 00/14] iio: magnetometer: ak8975: Additional changes to the driver

[Jonathan Cameron]

On Mon, 27 Apr 2026 22:09:45 +0200
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> This series is an attempt to make the driver less a rabbit hole.
> It's a continuation of what Joshua Crofts started doing. Hence
> it is based on his work and first three "patches" here are supposed
> to be folded to his series accordingly.
> 
> I have compile-tested them, but I haven't done full double check of
> the correctness from functional point of view. Joshua, please do that
> before incorporating into your series. Yes, my patches are assumed to
> become the part of Joshua's whatever next version of the series, that's
> why mine is marked as v0.
> 
> Should not be taken directly by the maintainers, but any comments, review
> are highly appreciated.
> 
A few brief comments but mostly looks good in isolation. I'll wait
for the fused set with the fixups applied to take a closer look.

Thanks Andy

J
> Andy Shevchenko (14):
>   drivers/iio/magnetometer/ak8975.c: fixup for the IWYU change
>   drivers/iio/magnetometer/ak8975.c: fixup for the errno fix
>   drivers/iio/magnetometer/ak8975.c: fixup for the iopoll.h conversion
>   iio: magnetometer: ak8975: Inline timeout constants
>   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
> 
>  drivers/iio/magnetometer/ak8975.c | 253 +++++++++++++-----------------
>  1 file changed, 112 insertions(+), 141 deletions(-)
>