[PATCH v9 0/5] iio: magnetometer: ak8975: driver cleanup

[PATCH v9 0/5] iio: magnetometer: ak8975: driver cleanup

[Joshua Crofts]

This is a 4-month old series that I had in my working tree and
completely forgot about. It's a continuation of the ak8975 driver
cleanup that Andy and I worked on, all that was missing was to
implement Jonathan's powered_on bool idea. The rest of the patches
have Reviewed-by tags.

Very sorry for the long delay :(

Original cover letter contents:

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>
---
Changes in v9:
- Add powered_on bool per Jonathan's review
- Move forgotten dev_err() call to use a local dev variable
- Link to v8: https://lore.kernel.org/all/20260518-magnetometer-fixes-post-pickup-v8-0-088d610108a0@gmail.com/

---
Andy Shevchenko (4):
      iio: magnetometer: ak8975: switch to using managed resources
      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 (1):
      iio: magnetometer: ak8975: add scan mask index enum

 drivers/iio/magnetometer/ak8975.c | 213 +++++++++++++++++++-------------------
 1 file changed, 107 insertions(+), 106 deletions(-)
---
base-commit: 40288c9206c17eb66a603262e06a58d300d0f279
change-id: 20260919-ak8975-cleanup-206e303af804

Best regards,
-- 
Kind regards,
Joshua Crofts

[PATCH v9 1/5] iio: magnetometer: ak8975: switch to using managed resources

[Joshua Crofts]

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 | 80 +++++++++++++++++++++------------------
 1 file changed, 44 insertions(+), 36 deletions(-)

diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
index 8b0c07f82602..d217fdf34bce 100644
--- a/drivers/iio/magnetometer/ak8975.c
+++ b/drivers/iio/magnetometer/ak8975.c
@@ -418,6 +418,7 @@ struct ak8975_data {
 	wait_queue_head_t	data_ready_queue;
 	unsigned long		flags;
 	u8			cntl_cache;
+	bool			powered_on;
 	struct iio_mount_matrix orientation;
 	struct regulator	*vdd;
 	struct regulator	*vid;
@@ -430,7 +431,7 @@ struct ak8975_data {
 };
 
 /* Enable attached power regulator if any. */
-static int ak8975_power_on(const struct ak8975_data *data)
+static int ak8975_power_on(struct ak8975_data *data)
 {
 	int ret;
 
@@ -457,16 +458,22 @@ static int ak8975_power_on(const struct ak8975_data *data)
 	 */
 	fsleep(500);
 
+	data->powered_on = true;
 	return 0;
 }
 
 /* Disable attached power regulator if any. */
-static void ak8975_power_off(const struct ak8975_data *data)
+static void ak8975_power_off(struct ak8975_data *data)
 {
+	if (!data->powered_on)
+		return;
+
 	gpiod_set_value_cansleep(data->reset_gpiod, 1);
 
 	regulator_disable(data->vid);
 	regulator_disable(data->vdd);
+
+	data->powered_on = false;
 }
 
 /*
@@ -934,9 +941,23 @@ static const struct iio_buffer_setup_ops ak8975_buffer_setup_ops = {
 	.preenable = ak8975_buffer_preenable,
 	.postdisable = ak8975_buffer_postdisable,
 };
+
+static void devm_ak8975_power_off(void *data)
+{
+	struct ak8975_data *ak = data;
+
+	if (!ak->powered_on)
+		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;
@@ -1004,10 +1025,14 @@ static int ak8975_probe(struct i2c_client *client)
 	if (ret)
 		return ret;
 
+	ret = devm_add_action_or_reset(dev, devm_ak8975_power_off, data);
+	if (ret)
+		return ret;
+
 	ret = ak8975_who_i_am(data, 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);
 
@@ -1015,10 +1040,13 @@ static int ak8975_probe(struct i2c_client *client)
 	ret = ak8975_setup(data);
 	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;
@@ -1026,52 +1054,33 @@ 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,
-					 &ak8975_buffer_setup_ops);
+	ret = devm_iio_triggered_buffer_setup(dev, indio_dev, NULL,
+					      ak8975_handle_trigger,
+					      &ak8975_buffer_setup_ops);
 	if (ret) {
 		dev_err(&client->dev, "triggered buffer setup failed\n");
-		goto power_off;
+		return ret;
 	}
 
-	ret = iio_device_register(indio_dev);
+	pm_runtime_set_active(dev);
+	ret = devm_pm_runtime_enable(dev);
+	if (ret)
+		return ret;
+
+	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);
 	/*
 	 * 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)
@@ -1165,7 +1174,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.55.0

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

[Joshua Crofts]

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>
Reviewed-by: Nuno Sá <nuno.sa@analog.com>
Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
---
 drivers/iio/magnetometer/ak8975.c | 37 +++++++++++++------------------------
 1 file changed, 13 insertions(+), 24 deletions(-)

diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
index d217fdf34bce..59a7bb83a731 100644
--- a/drivers/iio/magnetometer/ak8975.c
+++ b/drivers/iio/magnetometer/ak8975.c
@@ -501,14 +501,10 @@ 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 != sizeof(wia_val)) {
-		dev_err(&client->dev, "Error reading WIA\n");
-		return -EIO;
-	}
+	if (ret < 0)
+		return dev_err_probe(&client->dev, ret, "Error reading WIA\n");
+	if (ret != sizeof(wia_val))
+		return dev_err_probe(&client->dev, -EIO, "Error reading WIA\n");
 
 	if (wia_val[0] != AK8975_DEVICE_ID)
 		return -ENODEV;
@@ -1030,18 +1026,15 @@ 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)
@@ -1057,10 +1050,8 @@ static int ak8975_probe(struct i2c_client *client)
 	ret = devm_iio_triggered_buffer_setup(dev, indio_dev, NULL,
 					      ak8975_handle_trigger,
 					      &ak8975_buffer_setup_ops);
-	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");
 
 	pm_runtime_set_active(dev);
 	ret = devm_pm_runtime_enable(dev);
@@ -1068,10 +1059,8 @@ static int ak8975_probe(struct i2c_client *client)
 		return ret;
 
 	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");
 
 	/*
 	 * The device comes online in 500us, so add two orders of magnitude

-- 
2.55.0

[PATCH v9 3/5] iio: magnetometer: ak8975: use temporary variable for struct device

[Joshua Crofts]

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>
Reviewed-by: Nuno Sá <nuno.sa@analog.com>
Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
---
 drivers/iio/magnetometer/ak8975.c | 65 +++++++++++++++++++--------------------
 1 file changed, 32 insertions(+), 33 deletions(-)

diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
index 59a7bb83a731..42f6892f4efd 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(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;
 	}
@@ -580,6 +579,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;
 
@@ -590,9 +590,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;
 
@@ -608,12 +607,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;
 	}
 
@@ -623,26 +623,25 @@ 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;
 	}
 	if (ret != sizeof(data->asa)) {
-		dev_err(&client->dev, "Error reading asa data\n");
+		dev_err(dev, "Error reading asa data\n");
 		return -EIO;
 	}
 
 	/* 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;
 		}
 	}
@@ -748,10 +747,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);
 
@@ -773,20 +773,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);
@@ -795,8 +795,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;
 }
 
@@ -966,7 +966,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");
@@ -976,13 +976,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;
 
@@ -994,7 +993,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;
 
@@ -1004,16 +1003,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);
 
@@ -1029,7 +1028,7 @@ static int ak8975_probe(struct i2c_client *client)
 	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);
@@ -1066,8 +1065,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;
 }
@@ -1082,7 +1081,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 */
@@ -1106,7 +1105,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.55.0

[PATCH v9 4/5] iio: magnetometer: ak8975: add scan mask index enum

[Joshua Crofts]

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>
Reviewed-by: Nuno Sá <nuno.sa@analog.com>
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 42f6892f4efd..ac556644b02d 100644
--- a/drivers/iio/magnetometer/ak8975.c
+++ b/drivers/iio/magnetometer/ak8975.c
@@ -237,6 +237,13 @@ enum ak_ctrl_mode {
 	MODE_END,
 };
 
+enum ak_chan_index {
+	AK8975_CHAN_X,
+	AK8975_CHAN_Y,
+	AK8975_CHAN_Z,
+	AK8975_CHAN_TS,
+};
+
 struct ak_def {
 	enum asahi_compass_chipset type;
 	long (*raw_to_gauss)(u16 data);
@@ -851,8 +858,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_CHAN_X),
+	AK8975_CHANNEL(Y, AK8975_CHAN_Y),
+	AK8975_CHANNEL(Z, AK8975_CHAN_Z),
+	IIO_CHAN_SOFT_TIMESTAMP(AK8975_CHAN_TS),
 };
 
 static const unsigned long ak8975_scan_masks[] = { 0x7, 0 };

-- 
2.55.0

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

[Joshua Crofts]

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>
Reviewed-by: Nuno Sá <nuno.sa@analog.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 ac556644b02d..05cdd285c5f4 100644
--- a/drivers/iio/magnetometer/ak8975.c
+++ b/drivers/iio/magnetometer/ak8975.c
@@ -44,8 +44,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
@@ -54,15 +53,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
@@ -94,8 +90,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
@@ -106,8 +101,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
 
@@ -116,8 +110,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
 
@@ -864,7 +857,10 @@ static const struct iio_chan_spec ak8975_channels[] = {
 	IIO_CHAN_SOFT_TIMESTAMP(AK8975_CHAN_TS),
 };
 
-static const unsigned long ak8975_scan_masks[] = { 0x7, 0 };
+static const unsigned long ak8975_scan_masks[] = {
+	BIT(AK8975_CHAN_X) | BIT(AK8975_CHAN_Y) | BIT(AK8975_CHAN_Z),
+	0
+};
 
 static const struct iio_info ak8975_info = {
 	.read_raw = &ak8975_read_raw,

-- 
2.55.0

Re: [PATCH v9 3/5] iio: magnetometer: ak8975: use temporary variable for struct device

[Jonathan Cameron]

On Sun, 20 Sep 2026 20:05:25 +0200
Joshua Crofts <joshua.crofts1@gmail.com> 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>
> Reviewed-by: Nuno Sá <nuno.sa@analog.com>
> Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
A couple of places in here where I'd also switch to dev_err_probe().

Jonathan

> ---
>  drivers/iio/magnetometer/ak8975.c | 65 +++++++++++++++++++--------------------
>  1 file changed, 32 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
> index 59a7bb83a731..42f6892f4efd 100644
> --- a/drivers/iio/magnetometer/ak8975.c
> +++ b/drivers/iio/magnetometer/ak8975.c

> @@ -580,6 +579,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;
>  
> @@ -590,9 +590,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;
>  
> @@ -608,12 +607,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;
>  	}
>  
> @@ -623,26 +623,25 @@ 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");

_setup is only called from probe, so the prints in here can all
be return dev_err_probe();

> +		dev_err(dev, "Not able to read asa data\n");
>  		return ret;
>  	}
>  	if (ret != sizeof(data->asa)) {
> -		dev_err(&client->dev, "Error reading asa data\n");
> +		dev_err(dev, "Error reading asa data\n");
>  		return -EIO;
>  	}
>  
>  	/* 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;
>  		}
>  	}

Re: [PATCH v9 3/5] iio: magnetometer: ak8975: use temporary variable for struct device

[Jonathan Cameron]

On Fri, 25 Sep 2026 03:21:12 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> On Sun, 20 Sep 2026 20:05:25 +0200
> Joshua Crofts <joshua.crofts1@gmail.com> 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>
> > Reviewed-by: Nuno Sá <nuno.sa@analog.com>
> > Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>  
> A couple of places in here where I'd also switch to dev_err_probe().
> 
> Jonathan
diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
index 66f105d8e42d..0e0eba116d52 100644
--- a/drivers/iio/magnetometer/ak8975.c
+++ b/drivers/iio/magnetometer/ak8975.c
@@ -613,38 +613,28 @@ static int ak8975_setup(struct ak8975_data *data)
 
        /* Write the fused rom access mode. */
        ret = ak8975_set_mode(data, FUSE_ROM);
-       if (ret < 0) {
-               dev_err(dev, "Error in setting fuse access mode\n");
-               return ret;
-       }
+       if (ret < 0)
+               return dev_err_probe(dev, ret, "Error in setting fuse access mode\n");
 
        /* Get asa data and store in the device data. */
        ret = i2c_smbus_read_i2c_block_data_or_emulated(client,
                                                        data->def->ctrl_regs[ASA_BASE],
                                                        sizeof(data->asa),
                                                        data->asa);
-       if (ret < 0) {
-               dev_err(dev, "Not able to read asa data\n");
-               return ret;
-       }
-       if (ret != sizeof(data->asa)) {
-               dev_err(dev, "Error reading asa data\n");
-               return -EIO;
-       }
+       if (ret < 0)
+               return dev_err_probe(dev, ret, "Not able to read asa data\n");
+       if (ret != sizeof(data->asa))
+               return dev_err_probe(dev, ret, "Error reading asa data\n");
 
        /* After reading fuse ROM data set power-down mode */
        ret = ak8975_set_mode(data, POWER_DOWN);
-       if (ret < 0) {
-               dev_err(dev, "Error in setting power-down mode\n");
-               return ret;
-       }
+       if (ret < 0)
+               return dev_err_probe(dev, ret, "Error in setting power-down mode\n");
 
        if (data->eoc_gpiod || client->irq > 0) {
                ret = ak8975_setup_irq(data);
-               if (ret < 0) {
-                       dev_err(dev, "Error setting data ready interrupt\n");
-                       return ret;
-               }
+               if (ret < 0)
+                       return dev_err_probe(dev, ret, "Error setting data ready interrupt\n");
        }
 
        data->raw_to_gauss[0] = data->def->raw_to_gauss(data->asa[0]);
> 
> > ---
> >  drivers/iio/magnetometer/ak8975.c | 65 +++++++++++++++++++--------------------
> >  1 file changed, 32 insertions(+), 33 deletions(-)
> > 
> > diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
> > index 59a7bb83a731..42f6892f4efd 100644
> > --- a/drivers/iio/magnetometer/ak8975.c
> > +++ b/drivers/iio/magnetometer/ak8975.c  
> 
> > @@ -580,6 +579,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;
> >  
> > @@ -590,9 +590,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;
> >  
> > @@ -608,12 +607,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;
> >  	}
> >  
> > @@ -623,26 +623,25 @@ 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");  
> 
> _setup is only called from probe, so the prints in here can all
> be return dev_err_probe();
> 
> > +		dev_err(dev, "Not able to read asa data\n");
> >  		return ret;
> >  	}
> >  	if (ret != sizeof(data->asa)) {
> > -		dev_err(&client->dev, "Error reading asa data\n");
> > +		dev_err(dev, "Error reading asa data\n");
> >  		return -EIO;
> >  	}
> >  
> >  	/* 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;
> >  		}
> >  	}  
> 
> 
> 

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

[Jonathan Cameron]

On Sun, 20 Sep 2026 20:05:22 +0200
Joshua Crofts <joshua.crofts1@gmail.com> wrote:

> This is a 4-month old series that I had in my working tree and
> completely forgot about. It's a continuation of the ak8975 driver
> cleanup that Andy and I worked on, all that was missing was to
> implement Jonathan's powered_on bool idea. The rest of the patches
> have Reviewed-by tags.
> 
> Very sorry for the long delay :(
> 
> Original cover letter contents:
> 
> 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>
Applied with the tweaks called out in patch 3 (and a commit message
to mention that was rolled in).

Thanks,

Jonathan

> ---
> Changes in v9:
> - Add powered_on bool per Jonathan's review
> - Move forgotten dev_err() call to use a local dev variable
> - Link to v8: https://lore.kernel.org/all/20260518-magnetometer-fixes-post-pickup-v8-0-088d610108a0@gmail.com/
> 
> ---
> Andy Shevchenko (4):
>       iio: magnetometer: ak8975: switch to using managed resources
>       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 (1):
>       iio: magnetometer: ak8975: add scan mask index enum
> 
>  drivers/iio/magnetometer/ak8975.c | 213 +++++++++++++++++++-------------------
>  1 file changed, 107 insertions(+), 106 deletions(-)
> ---
> base-commit: 40288c9206c17eb66a603262e06a58d300d0f279
> change-id: 20260919-ak8975-cleanup-206e303af804
> 
> Best regards,