[PATCH 0/8] iio: magnetometer: ak8975: driver cleanup

[PATCH 0/8] iio: magnetometer: ak8975: driver cleanup

[Joshua Crofts via B4 Relay]

This series is a continuation of the previous ak8975 driver cleanup
effort, as most of the patches were picked.

Changes include:
- using BIT() and GENMASK() macros
- moving to devm_* resource management
- adding a scan index enum
- moving from using wait loops to iopoll functions
- various code style changes

Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
---
Andy Shevchenko (5):
      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 (3):
      iio: magnetometer: ak8975: modernize polling loops with iopoll() macros
      iio: magnetometer: ak8975: check if gpiod read was successful
      iio: magnetometer: ak8975: add scan mask index enum

 drivers/iio/magnetometer/ak8975.c | 268 +++++++++++++++++++-------------------
 1 file changed, 134 insertions(+), 134 deletions(-)
---
base-commit: 74d173f29572951629d1e0b7456b424006e51b87
change-id: 20260507-magnetometer-fixes-post-pickup-c6dabdd66f54

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

[PATCH 1/8] iio: magnetometer: ak8975: modernize polling loops with iopoll() macros

[Joshua Crofts via B4 Relay]

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

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

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

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

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

-- 
2.47.3

[PATCH 2/8] iio: magnetometer: ak8975: check if gpiod read was successful

[Joshua Crofts via B4 Relay]

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

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

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

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

-- 
2.47.3

[PATCH 3/8] 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.

Additionally, remove any pm_runtime_get/put*() function calls that
dummy cycled the counter to autosuspend the device.

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 | 74 ++++++++++++++++++++++-----------------
 1 file changed, 41 insertions(+), 33 deletions(-)

diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
index 63b6e8465f5f3873841550a1cd03ce86b95d1d67..1d8f448d5179fe9b33af989cc2f456ac91bc2f17 100644
--- a/drivers/iio/magnetometer/ak8975.c
+++ b/drivers/iio/magnetometer/ak8975.c
@@ -898,9 +898,24 @@ static irqreturn_t ak8975_handle_trigger(int irq, void *p)
 	return IRQ_HANDLED;
 }
 
+static void devm_ak8975_power_off(void *data)
+{
+	struct ak8975_data *ak = data;
+	struct device *dev = &ak->client->dev;
+
+	/* Only power down if currently active */
+	if (pm_runtime_status_suspended(dev))
+		return;
+
+	/* 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 +983,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 +1005,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 +1019,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 +1138,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 4/8] 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 | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
index 1d8f448d5179fe9b33af989cc2f456ac91bc2f17..e575d252076acc3639cfbb718a76811636fe56d2 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. */
@@ -706,12 +706,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;
@@ -744,7 +745,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 +857,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;
 
@@ -994,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;
@@ -1002,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 5/8] 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 e575d252076acc3639cfbb718a76811636fe56d2..394f930b862e82fd41c02d81bc6fdb3cfc8b729f 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 6/8] 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 | 63 +++++++++++++++++++--------------------
 1 file changed, 31 insertions(+), 32 deletions(-)

diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
index 394f930b862e82fd41c02d81bc6fdb3cfc8b729f..fde37cdcdd053bde1630c2f73ab717f9d008e2fa 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;
 		}
 	}
@@ -736,10 +735,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);
 
@@ -757,20 +757,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);
@@ -779,8 +779,8 @@ static int ak8975_read_axis(struct iio_dev *indio_dev, int index, int *val)
 
 exit:
 	mutex_unlock(&data->lock);
-	pm_runtime_put_autosuspend(&data->client->dev);
-	dev_err(&client->dev, "Error in reading axis\n");
+	pm_runtime_put_autosuspend(dev);
+	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 7/8] iio: magnetometer: ak8975: add scan mask index enum

[Joshua Crofts via B4 Relay]

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

Add an enum to explicitly define scan mask indexes for the X, Y, Z and
timestamp channels. Also, update the struct iio_chan_spec to use said
enum for the .scan_index parameter.

This prevents magic numbers from obscuring the hardware channel mapping
and improves code style.

No functional change.

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

diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
index fde37cdcdd053bde1630c2f73ab717f9d008e2fa..d11f025e146d098508f35ef31c1ccad544612535 100644
--- a/drivers/iio/magnetometer/ak8975.c
+++ b/drivers/iio/magnetometer/ak8975.c
@@ -238,6 +238,13 @@ enum ak_ctrl_mode {
 	MODE_END,
 };
 
+enum ak_scan_index {
+	AK8975_SCAN_X,
+	AK8975_SCAN_Y,
+	AK8975_SCAN_Z,
+	AK8975_SCAN_TS,
+};
+
 struct ak_def {
 	enum asahi_compass_chipset type;
 	long (*raw_to_gauss)(u16 data);
@@ -835,8 +842,10 @@ static const struct iio_chan_spec_ext_info ak8975_ext_info[] = {
 	}
 
 static const struct iio_chan_spec ak8975_channels[] = {
-	AK8975_CHANNEL(X, 0), AK8975_CHANNEL(Y, 1), AK8975_CHANNEL(Z, 2),
-	IIO_CHAN_SOFT_TIMESTAMP(3),
+	AK8975_CHANNEL(X, AK8975_SCAN_X),
+	AK8975_CHANNEL(Y, AK8975_SCAN_Y),
+	AK8975_CHANNEL(Z, AK8975_SCAN_Z),
+	IIO_CHAN_SOFT_TIMESTAMP(AK8975_SCAN_TS),
 };
 
 static const unsigned long ak8975_scan_masks[] = { 0x7, 0 };

-- 
2.47.3

[PATCH 8/8] 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 | 26 +++++++++++---------------
 1 file changed, 11 insertions(+), 15 deletions(-)

diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
index d11f025e146d098508f35ef31c1ccad544612535..d8d9f706f96bae3be19e8db17b9473e6e0eef4fc 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
 
@@ -848,7 +841,10 @@ static const struct iio_chan_spec ak8975_channels[] = {
 	IIO_CHAN_SOFT_TIMESTAMP(AK8975_SCAN_TS),
 };
 
-static const unsigned long ak8975_scan_masks[] = { 0x7, 0 };
+static const unsigned long ak8975_scan_masks[] = {
+	BIT(AK8975_SCAN_X) | BIT(AK8975_SCAN_Y) | BIT(AK8975_SCAN_Z),
+	0
+};
 
 static const struct iio_info ak8975_info = {
 	.read_raw = &ak8975_read_raw,

-- 
2.47.3

Re: [PATCH 0/8] iio: magnetometer: ak8975: driver cleanup

[Andy Shevchenko]

On Thu, May 07, 2026 at 04:35:49PM +0200, Joshua Crofts via B4 Relay wrote:
> This series is a continuation of the previous ak8975 driver cleanup
> effort, as most of the patches were picked.

It should be v6.

> Changes include:
> - using BIT() and GENMASK() macros
> - moving to devm_* resource management
> - adding a scan index enum
> - moving from using wait loops to iopoll functions
> - various code style changes

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 0/8] iio: magnetometer: ak8975: driver cleanup

[Joshua Crofts]

On Fri, 8 May 2026 at 09:39, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Thu, May 07, 2026 at 04:35:49PM +0200, Joshua Crofts via B4 Relay wrote:
> > This series is a continuation of the previous ak8975 driver cleanup
> > effort, as most of the patches were picked.
>
> It should be v6.

Jonathan said to either do a v6 or a new series, up to me.

-- 
Kind regards

CJD

Re: [PATCH 0/8] iio: magnetometer: ak8975: driver cleanup

[Andy Shevchenko]

On Fri, May 08, 2026 at 10:59:12AM +0200, Joshua Crofts wrote:
> On Fri, 8 May 2026 at 09:39, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > On Thu, May 07, 2026 at 04:35:49PM +0200, Joshua Crofts via B4 Relay wrote:
> > > This series is a continuation of the previous ak8975 driver cleanup
> > > effort, as most of the patches were picked.
> >
> > It should be v6.
> 
> Jonathan said to either do a v6 or a new series, up to me.

Fair enough.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 3/8] iio: magnetometer: ak8975: switch to using managed resources

[Andy Shevchenko]

On Thu, May 07, 2026 at 04:35:52PM +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.
> 
> Additionally, remove any pm_runtime_get/put*() function calls that
> dummy cycled the counter to autosuspend the device.

...

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

Is this one a correct one?
We also have pm_runtime_suspended(), which is different.

> +		return;
> +
> +	/* 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 0/8] iio: magnetometer: ak8975: driver cleanup

[Jonathan Cameron]

On Fri, 8 May 2026 12:19:04 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> On Fri, May 08, 2026 at 10:59:12AM +0200, Joshua Crofts wrote:
> > On Fri, 8 May 2026 at 09:39, Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:  
> > >
> > > On Thu, May 07, 2026 at 04:35:49PM +0200, Joshua Crofts via B4 Relay wrote:  
> > > > This series is a continuation of the previous ak8975 driver cleanup
> > > > effort, as most of the patches were picked.  
> > >
> > > It should be v6.  
> > 
> > Jonathan said to either do a v6 or a new series, up to me.  
> 
> Fair enough.
> 
Oops. Wasn't my intent.  v6 would be normal for 'some patches' of a series
that are outstanding. Ah well never mind.. 

J

Re: [PATCH 0/8] iio: magnetometer: ak8975: driver cleanup

[Joshua Crofts]

On Fri, 8 May 2026 at 15:34, Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Fri, 8 May 2026 12:19:04 +0300
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
>
> > On Fri, May 08, 2026 at 10:59:12AM +0200, Joshua Crofts wrote:
> > > On Fri, 8 May 2026 at 09:39, Andy Shevchenko
> > > <andriy.shevchenko@linux.intel.com> wrote:
> > > >
> > > > On Thu, May 07, 2026 at 04:35:49PM +0200, Joshua Crofts via B4 Relay wrote:
> > > > > This series is a continuation of the previous ak8975 driver cleanup
> > > > > effort, as most of the patches were picked.
> > > >
> > > > It should be v6.
> > >
> > > Jonathan said to either do a v6 or a new series, up to me.
> >
> > Fair enough.
> >
> Oops. Wasn't my intent.  v6 would be normal for 'some patches' of a series
> that are outstanding. Ah well never mind..

Oh well.

-- 
Kind regards

CJD

Re: [PATCH 3/8] iio: magnetometer: ak8975: switch to using managed resources

[Joshua Crofts]

On Fri, 8 May 2026 at 11:58, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Thu, May 07, 2026 at 04:35:52PM +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.
> >
> > Additionally, remove any pm_runtime_get/put*() function calls that
> > dummy cycled the counter to autosuspend the device.
>
> ...
>
> > +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))
>
> Is this one a correct one?
> We also have pm_runtime_suspended(), which is different.

As we only require the status of the device and not the power.disable_depth,
pm_runtime_status_suspended() seems adequate.

-- 
Kind regards

CJD

Re: [PATCH 3/8] iio: magnetometer: ak8975: switch to using managed resources

[Andy Shevchenko]

On Fri, May 08, 2026 at 03:51:06PM +0200, Joshua Crofts wrote:
> On Fri, 8 May 2026 at 11:58, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Thu, May 07, 2026 at 04:35:52PM +0200, Joshua Crofts via B4 Relay wrote:

...

> > > +     /* Only power down if currently active */
> > > +     if (pm_runtime_status_suspended(dev))
> >
> > Is this one a correct one?
> > We also have pm_runtime_suspended(), which is different.
> 
> As we only require the status of the device and not the power.disable_depth,
> pm_runtime_status_suspended() seems adequate.

Last time I have checked the code on this I remember that probably this one
doesn't guarantee that immediately after returning to the caller the device is
still in a suspend status.

The difference between two as documentation said is in "...if runtime PM
is enabled for @dev..." for the pm_runtime_suspended(). So the
pm_runtime_suspended() is stricter in that sense.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 3/8] iio: magnetometer: ak8975: switch to using managed resources

[Joshua Crofts]

On Sat, 9 May 2026 at 08:52, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Fri, May 08, 2026 at 03:51:06PM +0200, Joshua Crofts wrote:
> > On Fri, 8 May 2026 at 11:58, Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> > > On Thu, May 07, 2026 at 04:35:52PM +0200, Joshua Crofts via B4 Relay wrote:
>
> ...
>
> > > > +     /* Only power down if currently active */
> > > > +     if (pm_runtime_status_suspended(dev))
> > >
> > > Is this one a correct one?
> > > We also have pm_runtime_suspended(), which is different.
> >
> > As we only require the status of the device and not the power.disable_depth,
> > pm_runtime_status_suspended() seems adequate.
>
> Last time I have checked the code on this I remember that probably this one
> doesn't guarantee that immediately after returning to the caller the device is
> still in a suspend status.
>
> The difference between two as documentation said is in "...if runtime PM
> is enabled for @dev..." for the pm_runtime_suspended(). So the
> pm_runtime_suspended() is stricter in that sense.

Okay, if you prefer pm_runtime_suspended() then I have no issue with it.
I can amend the series version this time.

-- 
Kind regards

CJD

Re: [PATCH 3/8] iio: magnetometer: ak8975: switch to using managed resources

[Andy Shevchenko]

On Sat, May 09, 2026 at 09:47:25AM +0200, Joshua Crofts wrote:
> On Sat, 9 May 2026 at 08:52, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Fri, May 08, 2026 at 03:51:06PM +0200, Joshua Crofts wrote:
> > > On Fri, 8 May 2026 at 11:58, Andy Shevchenko
> > > <andriy.shevchenko@linux.intel.com> wrote:
> > > > On Thu, May 07, 2026 at 04:35:52PM +0200, Joshua Crofts via B4 Relay wrote:

...

> > > > > +     /* Only power down if currently active */
> > > > > +     if (pm_runtime_status_suspended(dev))
> > > >
> > > > Is this one a correct one?
> > > > We also have pm_runtime_suspended(), which is different.
> > >
> > > As we only require the status of the device and not the power.disable_depth,
> > > pm_runtime_status_suspended() seems adequate.
> >
> > Last time I have checked the code on this I remember that probably this one
> > doesn't guarantee that immediately after returning to the caller the device is
> > still in a suspend status.
> >
> > The difference between two as documentation said is in "...if runtime PM
> > is enabled for @dev..." for the pm_runtime_suspended(). So the
> > pm_runtime_suspended() is stricter in that sense.
> 
> Okay, if you prefer pm_runtime_suspended() then I have no issue with it.
> I can amend the series version this time.

It's not about my preference (I am fine with either call), it's about
understanding the code flow and what does fit better in _this_ case.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/8] iio: magnetometer: ak8975: modernize polling loops with iopoll() macros

[Nuno Sá]

On Thu, 2026-05-07 at 16:35 +0200, Joshua Crofts via B4 Relay 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
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
> ---

Reviewed-by: Nuno Sá <nuno.sa@analog.com>

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

Re: [PATCH 2/8] iio: magnetometer: ak8975: check if gpiod read was successful

[Nuno Sá]

On Thu, 2026-05-07 at 16:35 +0200, Joshua Crofts via B4 Relay wrote:
> From: Joshua Crofts <joshua.crofts1@gmail.com>
> 
> Add a check that ensures that valid data has been read from GPIOD. If
> not, log an error and return the negative read value.
> 
> Suggested-by: Jonathan Cameron <jic23@kernel.org>
> Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
> ---

Reviewed-by: Nuno Sá <nuno.sa@analog.com>

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

Re: [PATCH 3/8] iio: magnetometer: ak8975: switch to using managed resources

[Nuno Sá]

On Thu, 2026-05-07 at 16:35 +0200, Joshua Crofts via B4 Relay wrote:
> From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> Switch the driver to use managed resources (devm_*) which simplifier
> error handling and allows removing ak8975_remove() method from
> the driver.
> 
> Note, on error path we now also set mode to POWER_DOWN state which is
> fine. Even if the device is in that mode, there is no problem to set
> that mode again, it should be no-op.
> 
> Additionally, remove any pm_runtime_get/put*() function calls that
> dummy cycled the counter to autosuspend the device.
> 
> 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 | 74 ++++++++++++++++++++++-----------------
>  1 file changed, 41 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
> index
> 63b6e8465f5f3873841550a1cd03ce86b95d1d67..1d8f448d5179fe9b33af989cc2f456ac91bc2f17
> 100644
> --- a/drivers/iio/magnetometer/ak8975.c
> +++ b/drivers/iio/magnetometer/ak8975.c
> @@ -898,9 +898,24 @@ static irqreturn_t ak8975_handle_trigger(int irq, void *p)
>  	return IRQ_HANDLED;
>  }
>  
> +static void devm_ak8975_power_off(void *data)
> +{
> +	struct ak8975_data *ak = data;
> +	struct device *dev = &ak->client->dev;
> +
> +	/* Only power down if currently active */
> +	if (pm_runtime_status_suspended(dev))
> +		return;
> +
> +	/* 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 +983,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;

This looks a bit hackish to me. Why don't we just register this powerdown action
after enabling PM?
> +
>  	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 +1005,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 +1019,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;

Maybe it would make sense to move this before devm_iio_device_register(). At the
point we register the device, userspace can start to interact with the device where
we have pm calls? Not that it is a problem (I think) but makes sense to me to enable
PM before exposing the device.

- Nuno Sá

> +
>  	/*
>  	 * 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 +1138,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);

Re: [PATCH 4/8] iio: magnetometer: ak8975: consistently use 'data' parameter

[Nuno Sá]

On Thu, 2026-05-07 at 16:35 +0200, Joshua Crofts via B4 Relay wrote:
> 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>
> ---

Reviewed-by: Nuno Sá <nuno.sa@analog.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
> 1d8f448d5179fe9b33af989cc2f456ac91bc2f17..e575d252076acc3639cfbb718a76811636fe56d2
> 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. */
> @@ -706,12 +706,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;
> @@ -744,7 +745,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 +857,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;
>  
> @@ -994,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;
> @@ -1002,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;

Re: [PATCH 5/8] iio: magnetometer: ak8975: unify messages with help of dev_err_probe()

[Nuno Sá]

On Thu, 2026-05-07 at 16:35 +0200, Joshua Crofts via B4 Relay wrote:
> 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>
> ---

Reviewed-by: Nuno Sá <nuno.sa@analog.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
> e575d252076acc3639cfbb718a76811636fe56d2..394f930b862e82fd41c02d81bc6fdb3cfc8b729f
> 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);

Re: [PATCH 6/8] iio: magnetometer: ak8975: use temporary variable for struct device

[Nuno Sá]

On Thu, 2026-05-07 at 16:35 +0200, Joshua Crofts via B4 Relay wrote:
> 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>
> ---

Reviewed-by: Nuno Sá <nuno.sa@analog.com>

>  drivers/iio/magnetometer/ak8975.c | 63 +++++++++++++++++++--------------------
>  1 file changed, 31 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
> index
> 394f930b862e82fd41c02d81bc6fdb3cfc8b729f..fde37cdcdd053bde1630c2f73ab717f9d008e2fa
> 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;
>  		}
>  	}
> @@ -736,10 +735,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);
>  
> @@ -757,20 +757,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);
> @@ -779,8 +779,8 @@ static int ak8975_read_axis(struct iio_dev *indio_dev, int
> index, int *val)
>  
>  exit:
>  	mutex_unlock(&data->lock);
> -	pm_runtime_put_autosuspend(&data->client->dev);
> -	dev_err(&client->dev, "Error in reading axis\n");
> +	pm_runtime_put_autosuspend(dev);
> +	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;
>  	}
>  

Re: [PATCH 7/8] iio: magnetometer: ak8975: add scan mask index enum

[Nuno Sá]

On Thu, 2026-05-07 at 16:35 +0200, Joshua Crofts via B4 Relay wrote:
> From: Joshua Crofts <joshua.crofts1@gmail.com>
> 
> Add an enum to explicitly define scan mask indexes for the X, Y, Z and
> timestamp channels. Also, update the struct iio_chan_spec to use said
> enum for the .scan_index parameter.
> 
> This prevents magic numbers from obscuring the hardware channel mapping
> and improves code style.
> 
> No functional change.
> 
> Suggested-by: Jonathan Cameron <jic23@kernel.org>
> Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
> ---

Reviewed-by: Nuno Sá <nuno.sa@analog.com>

>  drivers/iio/magnetometer/ak8975.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
> index
> fde37cdcdd053bde1630c2f73ab717f9d008e2fa..d11f025e146d098508f35ef31c1ccad544612535
> 100644
> --- a/drivers/iio/magnetometer/ak8975.c
> +++ b/drivers/iio/magnetometer/ak8975.c
> @@ -238,6 +238,13 @@ enum ak_ctrl_mode {
>  	MODE_END,
>  };
>  
> +enum ak_scan_index {
> +	AK8975_SCAN_X,
> +	AK8975_SCAN_Y,
> +	AK8975_SCAN_Z,
> +	AK8975_SCAN_TS,
> +};
> +
>  struct ak_def {
>  	enum asahi_compass_chipset type;
>  	long (*raw_to_gauss)(u16 data);
> @@ -835,8 +842,10 @@ static const struct iio_chan_spec_ext_info ak8975_ext_info[] =
> {
>  	}
>  
>  static const struct iio_chan_spec ak8975_channels[] = {
> -	AK8975_CHANNEL(X, 0), AK8975_CHANNEL(Y, 1), AK8975_CHANNEL(Z, 2),
> -	IIO_CHAN_SOFT_TIMESTAMP(3),
> +	AK8975_CHANNEL(X, AK8975_SCAN_X),
> +	AK8975_CHANNEL(Y, AK8975_SCAN_Y),
> +	AK8975_CHANNEL(Z, AK8975_SCAN_Z),
> +	IIO_CHAN_SOFT_TIMESTAMP(AK8975_SCAN_TS),
>  };
>  
>  static const unsigned long ak8975_scan_masks[] = { 0x7, 0 };

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

[Nuno Sá]

On Thu, 2026-05-07 at 16:35 +0200, Joshua Crofts via B4 Relay wrote:
> From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> Make use of BIT() and GENMASK() where it makes sense.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Co-developed-by: Joshua Crofts <joshua.crofts1@gmail.com>
> Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
> ---

Small nit, anyways:

Reviewed-by: Nuno Sá <nuno.sa@analog.com>

>  drivers/iio/magnetometer/ak8975.c | 26 +++++++++++---------------
>  1 file changed, 11 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
> index
> d11f025e146d098508f35ef31c1ccad544612535..d8d9f706f96bae3be19e8db17b9473e6e0eef4fc
> 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
>  
> @@ -848,7 +841,10 @@ static const struct iio_chan_spec ak8975_channels[] = {
>  	IIO_CHAN_SOFT_TIMESTAMP(AK8975_SCAN_TS),
>  };
>  
> -static const unsigned long ak8975_scan_masks[] = { 0x7, 0 };
> +static const unsigned long ak8975_scan_masks[] = {
> +	BIT(AK8975_SCAN_X) | BIT(AK8975_SCAN_Y) | BIT(AK8975_SCAN_Z),

Small nit: But maybe GENMASK().

> +	0
> +};
>  
>  static const struct iio_info ak8975_info = {
>  	.read_raw = &ak8975_read_raw,

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

[Joshua Crofts]

On Sat, 9 May 2026 at 11:08, Nuno Sá <noname.nuno@gmail.com> wrote:
>
> On Thu, 2026-05-07 at 16:35 +0200, Joshua Crofts via B4 Relay wrote:
> > From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> >
> > Make use of BIT() and GENMASK() where it makes sense.
> >
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > Co-developed-by: Joshua Crofts <joshua.crofts1@gmail.com>
> > Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
> > ---
>
> Small nit, anyways:
>
> Reviewed-by: Nuno Sá <nuno.sa@analog.com>
>
> >  drivers/iio/magnetometer/ak8975.c | 26 +++++++++++---------------
> >  1 file changed, 11 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
> > index
> > d11f025e146d098508f35ef31c1ccad544612535..d8d9f706f96bae3be19e8db17b9473e6e0eef4fc
> > 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
> >
> > @@ -848,7 +841,10 @@ static const struct iio_chan_spec ak8975_channels[] = {
> >       IIO_CHAN_SOFT_TIMESTAMP(AK8975_SCAN_TS),
> >  };
> >
> > -static const unsigned long ak8975_scan_masks[] = { 0x7, 0 };
> > +static const unsigned long ak8975_scan_masks[] = {
> > +     BIT(AK8975_SCAN_X) | BIT(AK8975_SCAN_Y) | BIT(AK8975_SCAN_Z),
>
> Small nit: But maybe GENMASK().

Well, actually it was decided to add an enum and just do a bitwise
OR of the three BIT() macros for better readability.

https://lore.kernel.org/linux-iio/20260506181216.049f87f8@jic23-huawei/

-- 
Kind regards

CJD

Re: [PATCH 3/8] iio: magnetometer: ak8975: switch to using managed resources

[Joshua Crofts]

On Sat, 9 May 2026 at 11:02, Nuno Sá <noname.nuno@gmail.com> wrote:
>
> On Thu, 2026-05-07 at 16:35 +0200, Joshua Crofts via B4 Relay wrote:
> > From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> >
> > Switch the driver to use managed resources (devm_*) which simplifier
> > error handling and allows removing ak8975_remove() method from
> > the driver.
> >
> > Note, on error path we now also set mode to POWER_DOWN state which is
> > fine. Even if the device is in that mode, there is no problem to set
> > that mode again, it should be no-op.
> >
> > Additionally, remove any pm_runtime_get/put*() function calls that
> > dummy cycled the counter to autosuspend the device.
> >
> > 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 | 74 ++++++++++++++++++++++-----------------
> >  1 file changed, 41 insertions(+), 33 deletions(-)
> >
> > diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
> > index
> > 63b6e8465f5f3873841550a1cd03ce86b95d1d67..1d8f448d5179fe9b33af989cc2f456ac91bc2f17
> > 100644
> > --- a/drivers/iio/magnetometer/ak8975.c
> > +++ b/drivers/iio/magnetometer/ak8975.c
> > @@ -898,9 +898,24 @@ static irqreturn_t ak8975_handle_trigger(int irq, void *p)
> >       return IRQ_HANDLED;
> >  }
> >
> > +static void devm_ak8975_power_off(void *data)
> > +{
> > +     struct ak8975_data *ak = data;
> > +     struct device *dev = &ak->client->dev;
> > +
> > +     /* Only power down if currently active */
> > +     if (pm_runtime_status_suspended(dev))
> > +             return;
> > +
> > +     /* 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 +983,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;
>
> This looks a bit hackish to me. Why don't we just register this powerdown action
> after enabling PM?

This is to prevent a resource leak. If we register the power_off
function at the end,
any potential probe() failures will leave the regulators on.

> > +
> >       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 +1005,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 +1019,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;
>
> Maybe it would make sense to move this before devm_iio_device_register(). At the
> point we register the device, userspace can start to interact with the device where
> we have pm calls? Not that it is a problem (I think) but makes sense to me to enable
> PM before exposing the device.

Sure, I agree with this.

-- 
Kind regards

CJD

Re: [PATCH 3/8] iio: magnetometer: ak8975: switch to using managed resources

[Andy Shevchenko]

On Sat, May 09, 2026 at 10:03:38AM +0100, Nuno Sá wrote:
> On Thu, 2026-05-07 at 16:35 +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.
> > 
> > Additionally, remove any pm_runtime_get/put*() function calls that
> > dummy cycled the counter to autosuspend the device.

...

> Maybe it would make sense to move this before devm_iio_device_register(). At the
> point we register the device, userspace can start to interact with the device where
> we have pm calls? Not that it is a problem (I think) but makes sense to me to enable
> PM before exposing the device.

When you called devm_iio_device_register(), it's already user space interaction
there, so device has to be prepared for that. Do we guarantee that device is
power enabled at that point?

-- 
With Best Regards,
Andy Shevchenko

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

[Andy Shevchenko]

On Sat, May 09, 2026 at 10:09:03AM +0100, Nuno Sá wrote:
> On Thu, 2026-05-07 at 16:35 +0200, Joshua Crofts via B4 Relay wrote:

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

...

> > -static const unsigned long ak8975_scan_masks[] = { 0x7, 0 };
> > +static const unsigned long ak8975_scan_masks[] = {
> > +	BIT(AK8975_SCAN_X) | BIT(AK8975_SCAN_Y) | BIT(AK8975_SCAN_Z),
> 
> Small nit: But maybe GENMASK().

Jonathan explained that this is *not* a bitmask while looks and feels like it.
So it is his request to not do a GENMASK() here.

> > +	0
> > +};

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 3/8] iio: magnetometer: ak8975: switch to using managed resources

[Joshua Crofts]

On Sat, 9 May 2026 at 19:15, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Sat, May 09, 2026 at 10:03:38AM +0100, Nuno Sá wrote:
> > On Thu, 2026-05-07 at 16:35 +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.
> > >
> > > Additionally, remove any pm_runtime_get/put*() function calls that
> > > dummy cycled the counter to autosuspend the device.
>
> ...
>
> > Maybe it would make sense to move this before devm_iio_device_register(). At the
> > point we register the device, userspace can start to interact with the device where
> > we have pm calls? Not that it is a problem (I think) but makes sense to me to enable
> > PM before exposing the device.
>
> When you called devm_iio_device_register(), it's already user space interaction
> there, so device has to be prepared for that. Do we guarantee that device is
> power enabled at that point?

Okay, I'm all for moving devm_pm_runtime_enable() before
devm_iio_device_register(),
it makes sense to set up pm_runtime before opening the driver to
userspace. I'll push
out a (hopefully final) fix today so we can wrap up this series.

-- 
Kind regards

CJD

Re: [PATCH 3/8] iio: magnetometer: ak8975: switch to using managed resources

[Jonathan Cameron]

On Mon, 11 May 2026 09:04:43 +0200
Joshua Crofts <joshua.crofts1@gmail.com> wrote:

> On Sat, 9 May 2026 at 19:15, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > On Sat, May 09, 2026 at 10:03:38AM +0100, Nuno Sá wrote:  
> > > On Thu, 2026-05-07 at 16:35 +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.
> > > >
> > > > Additionally, remove any pm_runtime_get/put*() function calls that
> > > > dummy cycled the counter to autosuspend the device.  
> >
> > ...
> >  
> > > Maybe it would make sense to move this before devm_iio_device_register(). At the
> > > point we register the device, userspace can start to interact with the device where
> > > we have pm calls? Not that it is a problem (I think) but makes sense to me to enable
> > > PM before exposing the device.  
> >
> > When you called devm_iio_device_register(), it's already user space interaction
> > there, so device has to be prepared for that. Do we guarantee that device is
> > power enabled at that point?  

We should guarantee a couple of things.

1) If runtime PM is not turned on at all  - maybe not even built into kernel.
   The power should be on.  Note that has nothing to with devm_pm_runtime_enable()
2) Any calls that do need the power on and occur after or or in parallel with
   runtime PM being enabled ensure they have asked for the power to be on.   

Should all be race free if that combination is true.

Jonathan

> 
> Okay, I'm all for moving devm_pm_runtime_enable() before
> devm_iio_device_register(),
> it makes sense to set up pm_runtime before opening the driver to
> userspace. I'll push
> out a (hopefully final) fix today so we can wrap up this series.
> 

Re: [PATCH 3/8] iio: magnetometer: ak8975: switch to using managed resources

[Jonathan Cameron]

On Sat, 9 May 2026 10:54:18 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> On Sat, May 09, 2026 at 09:47:25AM +0200, Joshua Crofts wrote:
> > On Sat, 9 May 2026 at 08:52, Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:  
> > > On Fri, May 08, 2026 at 03:51:06PM +0200, Joshua Crofts wrote:  
> > > > On Fri, 8 May 2026 at 11:58, Andy Shevchenko
> > > > <andriy.shevchenko@linux.intel.com> wrote:  
> > > > > On Thu, May 07, 2026 at 04:35:52PM +0200, Joshua Crofts via B4 Relay wrote:  
> 
> ...
> 
> > > > > > +     /* Only power down if currently active */
> > > > > > +     if (pm_runtime_status_suspended(dev))  
> > > > >
> > > > > Is this one a correct one?
> > > > > We also have pm_runtime_suspended(), which is different.  
> > > >
> > > > As we only require the status of the device and not the power.disable_depth,
> > > > pm_runtime_status_suspended() seems adequate.  
> > >
> > > Last time I have checked the code on this I remember that probably this one
> > > doesn't guarantee that immediately after returning to the caller the device is
> > > still in a suspend status.
> > >
> > > The difference between two as documentation said is in "...if runtime PM
> > > is enabled for @dev..." for the pm_runtime_suspended(). So the
> > > pm_runtime_suspended() is stricter in that sense.  
> > 
> > Okay, if you prefer pm_runtime_suspended() then I have no issue with it.
> > I can amend the series version this time.  
> 
> It's not about my preference (I am fine with either call), it's about
> understanding the code flow and what does fit better in _this_ case.
> 

https://sashiko.dev/#/patchset/20260509191907.24734-1-macroalpha82%40gmail.com

See patch 5 review.  I haven't checked the details but looks like this
dance might not work and we need to figure out something more clever.

This particular example 'might' be fine as we don't end up with
pm_runtime_set_suspended_action() being registered.

May be fine but needs more careful analysis :(

I'm a little concerned we get some stale state because in the 'early'
set active approach used here we don't undo the pm_runtime_set_active()
anywhere.

Re: [PATCH 3/8] iio: magnetometer: ak8975: switch to using managed resources

[Nuno Sá]

On Sat, May 09, 2026 at 03:32:59PM +0200, Joshua Crofts wrote:
> On Sat, 9 May 2026 at 11:02, Nuno Sá <noname.nuno@gmail.com> wrote:
> >
> > On Thu, 2026-05-07 at 16:35 +0200, Joshua Crofts via B4 Relay wrote:
> > > From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > >
> > > Switch the driver to use managed resources (devm_*) which simplifier
> > > error handling and allows removing ak8975_remove() method from
> > > the driver.
> > >
> > > Note, on error path we now also set mode to POWER_DOWN state which is
> > > fine. Even if the device is in that mode, there is no problem to set
> > > that mode again, it should be no-op.
> > >
> > > Additionally, remove any pm_runtime_get/put*() function calls that
> > > dummy cycled the counter to autosuspend the device.
> > >
> > > 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 | 74 ++++++++++++++++++++++-----------------
> > >  1 file changed, 41 insertions(+), 33 deletions(-)
> > >
> > > diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
> > > index
> > > 63b6e8465f5f3873841550a1cd03ce86b95d1d67..1d8f448d5179fe9b33af989cc2f456ac91bc2f17
> > > 100644
> > > --- a/drivers/iio/magnetometer/ak8975.c
> > > +++ b/drivers/iio/magnetometer/ak8975.c
> > > @@ -898,9 +898,24 @@ static irqreturn_t ak8975_handle_trigger(int irq, void *p)
> > >       return IRQ_HANDLED;
> > >  }
> > >
> > > +static void devm_ak8975_power_off(void *data)
> > > +{
> > > +     struct ak8975_data *ak = data;
> > > +     struct device *dev = &ak->client->dev;
> > > +
> > > +     /* Only power down if currently active */
> > > +     if (pm_runtime_status_suspended(dev))
> > > +             return;
> > > +
> > > +     /* 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 +983,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;
> >
> > This looks a bit hackish to me. Why don't we just register this powerdown action
> > after enabling PM?
> 
> This is to prevent a resource leak. If we register the power_off
> function at the end,
> any potential probe() failures will leave the regulators on.

Okay it does make sense! I now see the action is registered after
ak8975_power_on() which what makes sense. Still would like to avoid that
weird pm_runtime_set_active() call. It would be nice if we could
guarantee through PM that the device get's suspended on unbind. Oh well,
I guess this is ok.

- Nuno Sá

> 
> > > +
> > >       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 +1005,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 +1019,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;
> >
> > Maybe it would make sense to move this before devm_iio_device_register(). At the
> > point we register the device, userspace can start to interact with the device where
> > we have pm calls? Not that it is a problem (I think) but makes sense to me to enable
> > PM before exposing the device.
> 
> Sure, I agree with this.
> 
> -- 
> Kind regards
> 
> CJD

Re: [PATCH 3/8] iio: magnetometer: ak8975: switch to using managed resources

[Joshua Crofts]

On Tue, 12 May 2026 at 10:06, Nuno Sá <noname.nuno@gmail.com> wrote:
> > This is to prevent a resource leak. If we register the power_off
> > function at the end,
> > any potential probe() failures will leave the regulators on.
>
> Okay it does make sense! I now see the action is registered after
> ak8975_power_on() which what makes sense. Still would like to avoid that
> weird pm_runtime_set_active() call. It would be nice if we could
> guarantee through PM that the device get's suspended on unbind. Oh well,
> I guess this is ok.

FYI, there is a new version where I addressed some of your comments.

https://lore.kernel.org/linux-iio/20260511-magnetometer-fixes-post-pickup-v7-3-9d910faa28b6@gmail.com/T/#u

-- 
Kind regards

CJD

Re: [PATCH 3/8] iio: magnetometer: ak8975: switch to using managed resources

[Andy Shevchenko]

On Tue, May 12, 2026 at 10:12:35AM +0200, Joshua Crofts wrote:
> On Tue, 12 May 2026 at 10:06, Nuno Sá <noname.nuno@gmail.com> wrote:
> > > This is to prevent a resource leak. If we register the power_off
> > > function at the end,
> > > any potential probe() failures will leave the regulators on.
> >
> > Okay it does make sense! I now see the action is registered after
> > ak8975_power_on() which what makes sense. Still would like to avoid that
> > weird pm_runtime_set_active() call. It would be nice if we could
> > guarantee through PM that the device get's suspended on unbind. Oh well,
> > I guess this is ok.
> 
> FYI, there is a new version where I addressed some of your comments.

Btw, can we rearrange the patches that the most controversial one(s?) will go
at the end (as much as possible) of the new series? Then we can work on it with
a less burden and unneeded dragging of the 'good' patches. Jonathan, what do
you think?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 3/8] iio: magnetometer: ak8975: switch to using managed resources

[Jonathan Cameron]

On Tue, 12 May 2026 11:23:37 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> On Tue, May 12, 2026 at 10:12:35AM +0200, Joshua Crofts wrote:
> > On Tue, 12 May 2026 at 10:06, Nuno Sá <noname.nuno@gmail.com> wrote:  
> > > > This is to prevent a resource leak. If we register the power_off
> > > > function at the end,
> > > > any potential probe() failures will leave the regulators on.  
> > >
> > > Okay it does make sense! I now see the action is registered after
> > > ak8975_power_on() which what makes sense. Still would like to avoid that
> > > weird pm_runtime_set_active() call. It would be nice if we could
> > > guarantee through PM that the device get's suspended on unbind. Oh well,
> > > I guess this is ok.  
> > 
> > FYI, there is a new version where I addressed some of your comments.  
> 
> Btw, can we rearrange the patches that the most controversial one(s?) will go
> at the end (as much as possible) of the new series? Then we can work on it with
> a less burden and unneeded dragging of the 'good' patches. Jonathan, what do
> you think?
> 

Definitely. I'm swamped (or as it's otherwise known neglecting the other
stuff I supposedly maintain). So anything that allows me to pick up partial
sets would be most welcome.

Jonathan