[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