[PATCH v4 00/17] iio: magnetometer: ak8975: modernize and cleanup driver

[PATCH v4 00/17] 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.

The patch regarding switching to devm_* functions might have to
go through an additional revision, otherwise everything else seems
good to go.

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

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
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
v4:
 - PATCH 7-17: am Andy's changes into this series
 - PATCH 2, 5, 6: fixup Andy's changes to existing patches

Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
---
Andy Shevchenko (11):
      iio: magnetometer: ak8975: pass conversion timeouts as arguments
      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 (6):
      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: modernize polling loops with iopoll() macros

 drivers/iio/magnetometer/ak8975.c | 375 ++++++++++++++++++--------------------
 1 file changed, 181 insertions(+), 194 deletions(-)
---
base-commit: eade2b843d9b1f668fc1775f15611bb0a1999cd9
change-id: 20260423-magnetometer-fixes-18b993041763

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

[PATCH v4 01/17] 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

[PATCH v4 02/17] 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.

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

[PATCH v4 03/17] 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

[PATCH v4 04/17] 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

[PATCH v4 05/17] 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>
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 | 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

[PATCH v4 06/17] 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
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 | 45 +++++++++++++++++----------------------
 1 file changed, 19 insertions(+), 26 deletions(-)

diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
index 57f50c09cca539c3733f516a1617375e9134c349..2e750c151435da57926969a63ba9fe996d774e7a 100644
--- a/drivers/iio/magnetometer/ak8975.c
+++ b/drivers/iio/magnetometer/ak8975.c
@@ -23,6 +23,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>
 
@@ -652,18 +653,15 @@ static int ak8975_setup(struct i2c_client *client)
 static int wait_conversion_complete_gpio(struct ak8975_data *data)
 {
 	struct i2c_client *client = data->client;
-	u32 timeout_ms = AK8975_MAX_CONVERSION_TIMEOUT;
 	int ret;
+	int val;
 
 	/* Wait for the conversion to complete. */
-	while (timeout_ms) {
-		msleep(AK8975_CONVERSION_DONE_POLL_TIME);
-		if (gpiod_get_value(data->eoc_gpiod))
-			break;
-		timeout_ms -= AK8975_CONVERSION_DONE_POLL_TIME;
-	}
-	if (!timeout_ms)
-		return -ETIMEDOUT;
+	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)
+		return ret;
 
 	ret = i2c_smbus_read_byte_data(client, data->def->ctrl_regs[ST1]);
 	if (ret < 0)
@@ -675,28 +673,23 @@ static int wait_conversion_complete_gpio(struct ak8975_data *data)
 static int wait_conversion_complete_polled(struct ak8975_data *data)
 {
 	struct i2c_client *client = data->client;
-	u8 read_status;
-	u32 timeout_ms = AK8975_MAX_CONVERSION_TIMEOUT;
 	int ret;
+	int val;
 
 	/* Wait for the conversion to complete. */
-	while (timeout_ms) {
-		msleep(AK8975_CONVERSION_DONE_POLL_TIME);
-		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 -= AK8975_CONVERSION_DONE_POLL_TIME;
+	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,
+				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

[PATCH v4 07/17] iio: magnetometer: ak8975: pass conversion timeouts as arguments

[Joshua Crofts via B4 Relay]

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

Since we have switched to using macros from iopoll.h it's better to
use poll and timeout values supplied as parameters to the helper
functions. Also added local variables for poll and timeout values
to prevent magic number use.

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>
Co-developed-by: Joshua Crofts <joshua.crofts1@gmail.com>
Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
---
 drivers/iio/magnetometer/ak8975.c | 38 ++++++++++++++++++++------------------
 1 file changed, 20 insertions(+), 18 deletions(-)

diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
index 2e750c151435da57926969a63ba9fe996d774e7a..e399a6508a3e28931ec2efe07924d5f30cebf442 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.
@@ -650,7 +644,8 @@ 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, int poll_ms,
+					 int timeout_ms)
 {
 	struct i2c_client *client = data->client;
 	int ret;
@@ -658,8 +653,8 @@ 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);
+				 poll_ms * USEC_PER_MSEC,
+				 timeout_ms * USEC_PER_MSEC);
 	if (ret)
 		return ret;
 
@@ -670,7 +665,8 @@ 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, int poll_ms,
+					   int timeout_ms)
 {
 	struct i2c_client *client = data->client;
 	int ret;
@@ -678,8 +674,8 @@ 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,
+				poll_ms * USEC_PER_MSEC,
+				timeout_ms * USEC_PER_MSEC,
 				true,
 				client, data->def->ctrl_regs[ST1]);
 	if (ret)
@@ -693,13 +689,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,
+					      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;
@@ -708,6 +705,11 @@ 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)
 {
+	int irq_timeout_ms = 100;
+	int timeout_ms = 500;
+	int poll_ms = 10;
+	int ret;
+
 	/* Set up the device for taking a sample. */
 	int ret = ak8975_set_mode(data, MODE_ONCE);
 
@@ -718,11 +720,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, irq_timeout_ms);
 	else if (data->eoc_gpiod)
-		ret = wait_conversion_complete_gpio(data);
+		ret = wait_conversion_complete_gpio(data, poll_ms, timeout_ms);
 	else
-		ret = wait_conversion_complete_polled(data);
+		ret = wait_conversion_complete_polled(data, poll_ms, timeout_ms);
 	if (ret < 0)
 		return ret;
 

-- 
2.47.3

[PATCH v4 08/17] 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 e399a6508a3e28931ec2efe07924d5f30cebf442..00bee434432f07e8e0bd7fb2b4c97cd2995af7a7 100644
--- a/drivers/iio/magnetometer/ak8975.c
+++ b/drivers/iio/magnetometer/ak8975.c
@@ -739,7 +739,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);
@@ -776,8 +775,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

[PATCH v4 09/17] 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 00bee434432f07e8e0bd7fb2b4c97cd2995af7a7..b94b0486421873a4afa41949d956e0bb5dafdb9d 100644
--- a/drivers/iio/magnetometer/ak8975.c
+++ b/drivers/iio/magnetometer/ak8975.c
@@ -916,8 +916,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

[PATCH v4 10/17] 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 b94b0486421873a4afa41949d956e0bb5dafdb9d..c354ebaba11c86c24ebe2dbc2b20c11f18015151 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

[PATCH v4 11/17] 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 c354ebaba11c86c24ebe2dbc2b20c11f18015151..5f3e8fadf68e1806841a9561fe6aacded08c85ac 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;
@@ -863,7 +866,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

[PATCH v4 12/17] 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 5f3e8fadf68e1806841a9561fe6aacded08c85ac..44cc3db57c6882b5f8add7943d1fad594a47ed1a 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;
 }
 
 /*
@@ -905,8 +905,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.
@@ -941,9 +941,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);
@@ -964,20 +964,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;
 	}
@@ -990,15 +990,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;
 	}
@@ -1021,7 +1021,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

[PATCH v4 13/17] 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 44cc3db57c6882b5f8add7943d1fad594a47ed1a..b857f45d0e28b82c9e5be840140f85ee13b3a14b 100644
--- a/drivers/iio/magnetometer/ak8975.c
+++ b/drivers/iio/magnetometer/ak8975.c
@@ -898,9 +898,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)) {
+		/*
+		 * 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;
@@ -968,10 +986,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);
 
@@ -979,10 +1008,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;
@@ -990,52 +1022,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)
@@ -1129,7 +1141,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

[PATCH v4 14/17] 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 | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
index b857f45d0e28b82c9e5be840140f85ee13b3a14b..6fba20c613aa40b07771fe9d7ebfc29d8a2a2b7d 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. */
@@ -702,8 +702,7 @@ 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)
 {
 	int irq_timeout_ms = 100;
 	int timeout_ms = 500;
@@ -711,8 +710,7 @@ static int ak8975_start_read_axis(struct ak8975_data *data,
 	int ret;
 
 	/* Set up the device for taking a sample. */
-	int ret = ak8975_set_mode(data, MODE_ONCE);
-
+	ret = ak8975_set_mode(data, MODE_ONCE);
 	if (ret < 0) {
 		dev_err(&client->dev, "Error in setting operating mode\n");
 		return ret;
@@ -745,7 +743,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;
 
@@ -856,7 +854,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;
 
@@ -997,7 +995,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;
@@ -1005,7 +1003,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 v4 15/17] 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 6fba20c613aa40b07771fe9d7ebfc29d8a2a2b7d..861a12ba8cfcc3724573b332b3f5bc243b67b1de 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;
@@ -996,18 +994,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)
@@ -1022,16 +1016,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 v4 16/17] 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 861a12ba8cfcc3724573b332b3f5bc243b67b1de..eeaf7cd5e36dc0e09f4bc588edceeb66501b8bef 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;
 		}
 	}
@@ -734,10 +733,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);
 
@@ -755,20 +755,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);
@@ -777,7 +777,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;
 }
 
@@ -927,7 +927,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");
@@ -937,13 +937,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;
 
@@ -955,7 +954,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;
 
@@ -965,16 +964,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);
 
@@ -996,7 +995,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);
@@ -1032,8 +1031,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;
 }
@@ -1048,7 +1047,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 */
@@ -1072,7 +1071,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 v4 17/17] 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 eeaf7cd5e36dc0e09f4bc588edceeb66501b8bef..ab400c107a2154b791d969f1df16f96ef139c5b2 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
 
@@ -836,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.47.3

Re: [PATCH v4 00/17] iio: magnetometer: ak8975: modernize and cleanup driver

[Andy Shevchenko]

On Mon, May 04, 2026 at 11:48:12AM +0200, Joshua Crofts via B4 Relay wrote:
> 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.
> 
> The patch regarding switching to devm_* functions might have to
> go through an additional revision, otherwise everything else seems
> good to go.
> 
> 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
> 
> 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
> 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
> v4:
>  - PATCH 7-17: am Andy's changes into this series
>  - PATCH 2, 5, 6: fixup Andy's changes to existing patches

Co-developed for those is too strong. Just use your SoB only.
You may use my Reviewed-by though.

Taking this into account, you can wait a day and send a new version with that
and --base fixed. Also it gives me and others a chance to briefly look into
the contents and see if everything looks okay.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 07/17] iio: magnetometer: ak8975: pass conversion timeouts as arguments

[Andy Shevchenko]

On Mon, May 04, 2026 at 11:48:19AM +0200, Joshua Crofts via B4 Relay wrote:

> Since we have switched to using macros from iopoll.h it's better to
> use poll and timeout values supplied as parameters to the helper
> functions. Also added local variables for poll and timeout values
> to prevent magic number use.
> 
> Besides that, fix the home grown and obviously wrong in some cases the
> jiffy-based timeout.

...

> -static int wait_conversion_complete_gpio(struct ak8975_data *data)
> +static int wait_conversion_complete_gpio(struct ak8975_data *data, int poll_ms,
> +					 int timeout_ms)

Logical split is preferred:

static int wait_conversion_complete_gpio(struct ak8975_data *data,
					 int poll_ms, int timeout_ms)

OTOH, why are they signed? Maybe

static int wait_conversion_complete_gpio(struct ak8975_data *data,
					 unsigned int poll_ms,
					 unsigned int timeout_ms)

(but please, double check that there is no possibility to have a negative
 values there)?

> +				 poll_ms * USEC_PER_MSEC,
> +				 timeout_ms * USEC_PER_MSEC);
>  	if (ret)
>  		return ret;

...

> +	int irq_timeout_ms = 100;
> +	int timeout_ms = 500;
> +	int poll_ms = 10;

Hmm... I don't think we need those, esp. taking into account the difference in
the timeout_ms for different cases.

...

>  	/* Wait for the conversion to complete. */
>  	if (data->eoc_irq)
> -		ret = wait_conversion_complete_interrupt(data);
> +		ret = wait_conversion_complete_interrupt(data, irq_timeout_ms);

		ret = wait_conversion_complete_interrupt(data, 100);

>  	else if (data->eoc_gpiod)
> -		ret = wait_conversion_complete_gpio(data);
> +		ret = wait_conversion_complete_gpio(data, poll_ms, timeout_ms);

		ret = wait_conversion_complete_gpio(data, 10, 500);

>  	else
> -		ret = wait_conversion_complete_polled(data);
> +		ret = wait_conversion_complete_polled(data, poll_ms, timeout_ms);

		ret = wait_conversion_complete_polled(data, 10, 500);

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 00/17] iio: magnetometer: ak8975: modernize and cleanup driver

[Joshua Crofts]

On Mon, 4 May 2026 at 13:01, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> Co-developed for those is too strong. Just use your SoB only.
> You may use my Reviewed-by though.
>
> Taking this into account, you can wait a day and send a new version with that
> and --base fixed. Also it gives me and others a chance to briefly look into
> the contents and see if everything looks okay.

Sounds good, the devm_* migration patch should be looked over especially,
since I have made some additional changes.

-- 
Kind regards

CJD

Re: [PATCH v4 13/17] iio: magnetometer: ak8975: switch to using managed resources

[Andy Shevchenko]

On Mon, May 04, 2026 at 11:48:25AM +0200, Joshua Crofts via B4 Relay 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.

...

> +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)) {

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

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 06/17] iio: magnetometer: ak8975: modernize polling loops with iopoll() macros

[Andy Shevchenko]

On Mon, May 04, 2026 at 11:48:18AM +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.

...

>  static int wait_conversion_complete_gpio(struct ak8975_data *data)
>  {
>  	struct i2c_client *client = data->client;
> -	u32 timeout_ms = AK8975_MAX_CONVERSION_TIMEOUT;

It seems logically to shuffle the order of the series and put the patch that
adds a parameter to the wait_*() custom calls before this one. It may mean
the new patch (as split from the current one by me, you can use there my
SoB/Co-developed-by though).

>  	int ret;

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 07/17] iio: magnetometer: ak8975: pass conversion timeouts as arguments

[Joshua Crofts]

On Mon, 4 May 2026 at 13:10, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Mon, May 04, 2026 at 11:48:19AM +0200, Joshua Crofts via B4 Relay wrote:
>
> > Since we have switched to using macros from iopoll.h it's better to
> > use poll and timeout values supplied as parameters to the helper
> > functions. Also added local variables for poll and timeout values
> > to prevent magic number use.
> >
> > Besides that, fix the home grown and obviously wrong in some cases the
> > jiffy-based timeout.
>
> ...
>
> > -static int wait_conversion_complete_gpio(struct ak8975_data *data)
> > +static int wait_conversion_complete_gpio(struct ak8975_data *data, int poll_ms,
> > +                                      int timeout_ms)
>
> Logical split is preferred:
>
> static int wait_conversion_complete_gpio(struct ak8975_data *data,
>                                          int poll_ms, int timeout_ms)

Fair enough.

> OTOH, why are they signed? Maybe

> static int wait_conversion_complete_gpio(struct ak8975_data *data,
>                                          unsigned int poll_ms,
>                                          unsigned int timeout_ms)
>
> (but please, double check that there is no possibility to have a negative
>  values there)?

Well, considering each wait_ function is called exactly once in the code and
accepts only the hardcoded ms values (which are all positive), I think we can
change it to unsigned.

> Hmm... I don't think we need those, esp. taking into account the difference in
> the timeout_ms for different cases.

Okay, this was suggested by Jonathan on your v0 version, but I guess there
is no need now that there are poll and timeout parameters added in the function
(or maybe the suggestions were for the parameters only and I
misinterpreted them).

-- 
Kind regards

CJD

Re: [PATCH v4 13/17] iio: magnetometer: ak8975: switch to using managed resources

[Joshua Crofts]

On Mon, 4 May 2026 at 13:13, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Mon, May 04, 2026 at 11:48:25AM +0200, Joshua Crofts via B4 Relay 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.
>
> ...
>
> > +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)) {
>
>         if (pm_runtime_status_suspended(dev))
>                 return;
>
> ?

Good point.

-- 
Kind regards

CJD

Re: [PATCH v4 07/17] iio: magnetometer: ak8975: pass conversion timeouts as arguments

[Joshua Crofts]

On Mon, 4 May 2026 at 11:48, Joshua Crofts via B4 Relay
<devnull+joshua.crofts1.gmail.com@kernel.org> wrote:
> @@ -708,6 +705,11 @@ 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)
>  {
> +       int irq_timeout_ms = 100;
> +       int timeout_ms = 500;
> +       int poll_ms = 10;
> +       int ret;
> +
>         /* Set up the device for taking a sample. */
>         int ret = ak8975_set_mode(data, MODE_ONCE);

BTW, also noticed a redefinition error here.

-- 
Kind regards

CJD

Re: [PATCH v4 07/17] iio: magnetometer: ak8975: pass conversion timeouts as arguments

[Andy Shevchenko]

On Mon, May 04, 2026 at 01:25:11PM +0200, Joshua Crofts wrote:
> On Mon, 4 May 2026 at 13:10, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Mon, May 04, 2026 at 11:48:19AM +0200, Joshua Crofts via B4 Relay wrote:

...

> > Hmm... I don't think we need those, esp. taking into account the difference in
> > the timeout_ms for different cases.
> 
> Okay, this was suggested by Jonathan on your v0 version, but I guess there
> is no need now that there are poll and timeout parameters added in the function
> (or maybe the suggestions were for the parameters only and I
> misinterpreted them).

The idea is to see them on an average one page in the text editor, all of them
at once. As long as they are quite close to each other, the temporary variables
are not needed. Moreover, I found them a nit harmful in terms of readability as
one will need to jump over the code to see the values, while with explicit
numbers it's all being concentrated in a small code stanza.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 07/17] iio: magnetometer: ak8975: pass conversion timeouts as arguments

[Joshua Crofts]

On Mon, 4 May 2026 at 16:06, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> > Okay, this was suggested by Jonathan on your v0 version, but I guess there
> > is no need now that there are poll and timeout parameters added in the function
> > (or maybe the suggestions were for the parameters only and I
> > misinterpreted them).
>
> The idea is to see them on an average one page in the text editor, all of them
> at once. As long as they are quite close to each other, the temporary variables
> are not needed. Moreover, I found them a nit harmful in terms of readability as
> one will need to jump over the code to see the values, while with explicit
> numbers it's all being concentrated in a small code stanza.

You've convinced me :)

-- 
Kind regards

CJD

Re: [PATCH v4 06/17] iio: magnetometer: ak8975: modernize polling loops with iopoll() macros

[Jonathan Cameron]

On Mon, 04 May 2026 11:48:18 +0200
Joshua Crofts via B4 Relay <devnull+joshua.crofts1.gmail.com@kernel.org> wrote:

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

One question inline on whether it is a good idea to be paranoid about
gpiod_get_value() potentially returning < 0 to indicate an error.
Right now that is treated as success.

> ---
>  drivers/iio/magnetometer/ak8975.c | 45 +++++++++++++++++----------------------
>  1 file changed, 19 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
> index 57f50c09cca539c3733f516a1617375e9134c349..2e750c151435da57926969a63ba9fe996d774e7a 100644
> --- a/drivers/iio/magnetometer/ak8975.c
> +++ b/drivers/iio/magnetometer/ak8975.c
> @@ -23,6 +23,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>
>  
> @@ -652,18 +653,15 @@ static int ak8975_setup(struct i2c_client *client)
>  static int wait_conversion_complete_gpio(struct ak8975_data *data)
>  {
>  	struct i2c_client *client = data->client;
> -	u32 timeout_ms = AK8975_MAX_CONVERSION_TIMEOUT;
>  	int ret;
> +	int val;
>  
>  	/* Wait for the conversion to complete. */
> -	while (timeout_ms) {
> -		msleep(AK8975_CONVERSION_DONE_POLL_TIME);
> -		if (gpiod_get_value(data->eoc_gpiod))
> -			break;
> -		timeout_ms -= AK8975_CONVERSION_DONE_POLL_TIME;
> -	}
> -	if (!timeout_ms)
> -		return -ETIMEDOUT;
> +	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)
> +		return ret;

Should we check val as well?   It might be negative if gpiod_get_value()
returned an error..  Obviously the original code didn't so this would be an
improvement rather than maintaining what that was doing.


>  
>  	ret = i2c_smbus_read_byte_data(client, data->def->ctrl_regs[ST1]);
>  	if (ret < 0)
> @@ -675,28 +673,23 @@ static int wait_conversion_complete_gpio(struct ak8975_data *data)
>  static int wait_conversion_complete_polled(struct ak8975_data *data)
>  {
>  	struct i2c_client *client = data->client;
> -	u8 read_status;
> -	u32 timeout_ms = AK8975_MAX_CONVERSION_TIMEOUT;
>  	int ret;
> +	int val;
>  
>  	/* Wait for the conversion to complete. */
> -	while (timeout_ms) {
> -		msleep(AK8975_CONVERSION_DONE_POLL_TIME);
> -		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 -= AK8975_CONVERSION_DONE_POLL_TIME;
> +	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,
> +				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 */
> 

Re: [PATCH v4 07/17] iio: magnetometer: ak8975: pass conversion timeouts as arguments

[Jonathan Cameron]

On Mon, 4 May 2026 16:11:47 +0200
Joshua Crofts <joshua.crofts1@gmail.com> wrote:

> On Mon, 4 May 2026 at 16:06, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > > Okay, this was suggested by Jonathan on your v0 version, but I guess there
> > > is no need now that there are poll and timeout parameters added in the function
> > > (or maybe the suggestions were for the parameters only and I
> > > misinterpreted them).  
> >
> > The idea is to see them on an average one page in the text editor, all of them
> > at once. As long as they are quite close to each other, the temporary variables
> > are not needed. Moreover, I found them a nit harmful in terms of readability as
> > one will need to jump over the code to see the values, while with explicit
> > numbers it's all being concentrated in a small code stanza.  
> 
> You've convinced me :)
> 
Also agree given they are so close together and we have that different
timeout nearby.

J

Re: [PATCH v4 07/17] iio: magnetometer: ak8975: pass conversion timeouts as arguments

[Jonathan Cameron]

On Mon, 4 May 2026 13:33:10 +0200
Joshua Crofts <joshua.crofts1@gmail.com> wrote:

> On Mon, 4 May 2026 at 11:48, Joshua Crofts via B4 Relay
> <devnull+joshua.crofts1.gmail.com@kernel.org> wrote:
> > @@ -708,6 +705,11 @@ 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)
> >  {
> > +       int irq_timeout_ms = 100;
> > +       int timeout_ms = 500;
> > +       int poll_ms = 10;
> > +       int ret;
> > +
> >         /* Set up the device for taking a sample. */
> >         int ret = ak8975_set_mode(data, MODE_ONCE);  
> 
> BTW, also noticed a redefinition error here.
Given that's one that should have resulted in a build fail
please make sure you build after applying each patch in the
series.  Need that for bisection to work and because I might
pick up a partial set.

Thanks,

J
> 

Re: [PATCH v4 00/17] iio: magnetometer: ak8975: modernize and cleanup driver

[Jonathan Cameron]

On Mon, 4 May 2026 13:13:17 +0200
Joshua Crofts <joshua.crofts1@gmail.com> wrote:

> On Mon, 4 May 2026 at 13:01, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > Co-developed for those is too strong. Just use your SoB only.
> > You may use my Reviewed-by though.
> >
> > Taking this into account, you can wait a day and send a new version with that
> > and --base fixed. Also it gives me and others a chance to briefly look into
> > the contents and see if everything looks okay.  
> 
> Sounds good, the devm_* migration patch should be looked over especially,
> since I have made some additional changes.
> 

I looked through them and only that one question on the read_poll stuff
and whether we should check for errors from the function being checked
on each poll

J

Re: [PATCH v4 06/17] iio: magnetometer: ak8975: modernize polling loops with iopoll() macros

[Andy Shevchenko]

On Mon, May 04, 2026 at 04:06:06PM +0100, Jonathan Cameron wrote:
> On Mon, 04 May 2026 11:48:18 +0200
> Joshua Crofts via B4 Relay <devnull+joshua.crofts1.gmail.com@kernel.org> wrote:

...

> One question inline on whether it is a good idea to be paranoid about
> gpiod_get_value() potentially returning < 0 to indicate an error.
> Right now that is treated as success.

> > +	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)
> > +		return ret;
> 
> Should we check val as well?   It might be negative if gpiod_get_value()
> returned an error..  Obviously the original code didn't so this would be an
> improvement rather than maintaining what that was doing.

I agree that this would be a good addition, but since it's a behavioural change
I think the separate patch should be made (perhaps after readx_*() conversion
as it makes it easier to do).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 06/17] iio: magnetometer: ak8975: modernize polling loops with iopoll() macros

[Joshua Crofts]

On Tue, 5 May 2026 at 06:58, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Mon, May 04, 2026 at 04:06:06PM +0100, Jonathan Cameron wrote:
> > On Mon, 04 May 2026 11:48:18 +0200
> > Joshua Crofts via B4 Relay <devnull+joshua.crofts1.gmail.com@kernel.org> wrote:
>
> ...
>
> > One question inline on whether it is a good idea to be paranoid about
> > gpiod_get_value() potentially returning < 0 to indicate an error.
> > Right now that is treated as success.
>
> > > +   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)
> > > +           return ret;
> >
> > Should we check val as well?   It might be negative if gpiod_get_value()
> > returned an error..  Obviously the original code didn't so this would be an
> > improvement rather than maintaining what that was doing.
>
> I agree that this would be a good addition, but since it's a behavioural change
> I think the separate patch should be made (perhaps after readx_*() conversion
> as it makes it easier to do).

Good point, val should definitely be checked. Will send a fix this afternoon.

-- 
Kind regards

CJD

Re: [PATCH v4 06/17] iio: magnetometer: ak8975: modernize polling loops with iopoll() macros

[Andy Shevchenko]

On Tue, May 05, 2026 at 09:07:30AM +0200, Joshua Crofts wrote:
> On Tue, 5 May 2026 at 06:58, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Mon, May 04, 2026 at 04:06:06PM +0100, Jonathan Cameron wrote:
> > > On Mon, 04 May 2026 11:48:18 +0200
> > > Joshua Crofts via B4 Relay <devnull+joshua.crofts1.gmail.com@kernel.org> wrote:

...

> > > One question inline on whether it is a good idea to be paranoid about
> > > gpiod_get_value() potentially returning < 0 to indicate an error.
> > > Right now that is treated as success.
> >
> > > > +   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)
> > > > +           return ret;
> > >
> > > Should we check val as well?   It might be negative if gpiod_get_value()
> > > returned an error..  Obviously the original code didn't so this would be an
> > > improvement rather than maintaining what that was doing.
> >
> > I agree that this would be a good addition, but since it's a behavioural change
> > I think the separate patch should be made (perhaps after readx_*() conversion
> > as it makes it easier to do).
> 
> Good point, val should definitely be checked. Will send a fix this afternoon.

It's better to integrate it into the series either as a fix (should be first in
the series) or as just an improvement after readx_*() conversion patch.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 06/17] iio: magnetometer: ak8975: modernize polling loops with iopoll() macros

[Joshua Crofts]

On Tue, 5 May 2026 at 09:21, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Tue, May 05, 2026 at 09:07:30AM +0200, Joshua Crofts wrote:
> > On Tue, 5 May 2026 at 06:58, Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> > > On Mon, May 04, 2026 at 04:06:06PM +0100, Jonathan Cameron wrote:
> > > > On Mon, 04 May 2026 11:48:18 +0200
> > > > Joshua Crofts via B4 Relay <devnull+joshua.crofts1.gmail.com@kernel.org> wrote:
>
> ...
>
> > > > One question inline on whether it is a good idea to be paranoid about
> > > > gpiod_get_value() potentially returning < 0 to indicate an error.
> > > > Right now that is treated as success.
> > >
> > > > > +   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)
> > > > > +           return ret;
> > > >
> > > > Should we check val as well?   It might be negative if gpiod_get_value()
> > > > returned an error..  Obviously the original code didn't so this would be an
> > > > improvement rather than maintaining what that was doing.
> > >
> > > I agree that this would be a good addition, but since it's a behavioural change
> > > I think the separate patch should be made (perhaps after readx_*() conversion
> > > as it makes it easier to do).
> >
> > Good point, val should definitely be checked. Will send a fix this afternoon.
>
> It's better to integrate it into the series either as a fix (should be first in
> the series) or as just an improvement after readx_*() conversion patch.

Better to do it as an improvement after readx_(), rebasing would be easier. I do
agree with it being added as a separate patch though.

-- 
Kind regards

CJD

Re: [PATCH v4 06/17] iio: magnetometer: ak8975: modernize polling loops with iopoll() macros

[Joshua Crofts]

On Tue, 5 May 2026 at 09:26, Joshua Crofts <joshua.crofts1@gmail.com> wrote:
> > It's better to integrate it into the series either as a fix (should be first in
> > the series) or as just an improvement after readx_*() conversion patch.
>
> Better to do it as an improvement after readx_(), rebasing would be easier. I do
> agree with it being added as a separate patch though.

Before I forget, is it better to return the negative 'val' or -EINVAL?

-- 
Kind regards

CJD

Re: [PATCH v4 06/17] iio: magnetometer: ak8975: modernize polling loops with iopoll() macros

[Andy Shevchenko]

On Tue, May 05, 2026 at 10:07:57AM +0200, Joshua Crofts wrote:
> On Tue, 5 May 2026 at 09:26, Joshua Crofts <joshua.crofts1@gmail.com> wrote:
> > > It's better to integrate it into the series either as a fix (should be first in
> > > the series) or as just an improvement after readx_*() conversion patch.
> >
> > Better to do it as an improvement after readx_(), rebasing would be easier. I do
> > agree with it being added as a separate patch though.
> 
> Before I forget, is it better to return the negative 'val' or -EINVAL?

The (negative) contents of val. Shadowing the error code must be justified.

-- 
With Best Regards,
Andy Shevchenko