[PATCH v2 00/13] iio: light: Modernize al3010 and al3320a codebase

[PATCH v2 00/13] iio: light: Modernize al3010 and al3320a codebase

[David Heidelberg via B4 Relay]

This series targetting improved readability of code
and modernizing code to match AL3000a (recently upstreamed).

Except slightly improved error reporting,
there shouldn't be any function changes.

Module  before after
al3010  72 kB  58 kB
al3320a 72 kB  58 kB

Signed-off-by: David Heidelberg <david@ixit.cz>
---
Changes in v2:
- Dropped Daniel's email update.
- Dropped DRV_NAME introduction for al3000a
- Added DRV_NAME define removal for al3010 and al3320a.
- Splitted unsigned int conversion into separate patches.
- Replaced generic value with specific raw and gain variable.
- Use dev_err_probe() for error handling.
- Separated devm_add_action_or_reset move from _init back to _probe.
- Dropped copyright update.
- Link to v1: https://lore.kernel.org/r/20250308-al3010-iio-regmap-v1-0-b672535e8213@ixit.cz

---
David Heidelberg (13):
      iio: light: al3010: Use unsigned int for the indexing
      iio: light: al3320a: Use unsigned int for the indexing
      iio: light: al3010: Remove DRV_NAME definition
      iio: light: al3320a: Remove DRV_NAME definition
      iio: light: al3010: Abstract device reference in the probe function
      iio: light: al3320a: Abstract device reference in the probe function
      iio: light: al3010: Split set_pwr function into set_pwr_on and _off
      iio: light: al3320a: Split set_pwr function into set_pwr_on and _off
      iio: light: al3010: Move devm_add_action_or_reset back to _probe
      iio: light: al3010: Improve al3010_init error handling with dev_err_probe
      iio: light: al3320a: Improve error handling with dev_err_probe
      iio: light: al3010: Implement regmap support
      iio: light: al3320a: Implement regmap support

 drivers/iio/light/al3010.c  | 103 +++++++++++++++++++++------------------
 drivers/iio/light/al3320a.c | 115 +++++++++++++++++++++++---------------------
 2 files changed, 116 insertions(+), 102 deletions(-)
---
base-commit: ff7f9b199e3f4cc7d61df5a9a26a7cbb5c1492e6
change-id: 20250308-al3010-iio-regmap-038cea39f85d

Best regards,
-- 
David Heidelberg <david@ixit.cz>

[PATCH v2 01/13] iio: light: al3010: Use unsigned int for the indexing

[David Heidelberg via B4 Relay]

From: David Heidelberg <david@ixit.cz>

The integer is used as array index which cannot be negative.

Signed-off-by: David Heidelberg <david@ixit.cz>
---
 drivers/iio/light/al3010.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/light/al3010.c b/drivers/iio/light/al3010.c
index 7cbb8b203300907a88f4a0ab87da89cabdd087f3..4c2fd88ab32cd73f4735b0fa3014af084037c94d 100644
--- a/drivers/iio/light/al3010.c
+++ b/drivers/iio/light/al3010.c
@@ -145,7 +145,7 @@ static int al3010_write_raw(struct iio_dev *indio_dev,
 			    int val2, long mask)
 {
 	struct al3010_data *data = iio_priv(indio_dev);
-	int i;
+	unsigned int i;
 
 	switch (mask) {
 	case IIO_CHAN_INFO_SCALE:

-- 
2.49.0

[PATCH v2 02/13] iio: light: al3320a: Use unsigned int for the indexing

[David Heidelberg via B4 Relay]

From: David Heidelberg <david@ixit.cz>

The integer is used as array index which cannot be negative.

Signed-off-by: David Heidelberg <david@ixit.cz>
---
 drivers/iio/light/al3320a.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/light/al3320a.c b/drivers/iio/light/al3320a.c
index 497ea3fe337775b07efdfc56c80beb1aa55e394c..bceda71517c8180dff76db311aa3591ab9846156 100644
--- a/drivers/iio/light/al3320a.c
+++ b/drivers/iio/light/al3320a.c
@@ -163,7 +163,7 @@ static int al3320a_write_raw(struct iio_dev *indio_dev,
 			     int val2, long mask)
 {
 	struct al3320a_data *data = iio_priv(indio_dev);
-	int i;
+	unsigned int i;
 
 	switch (mask) {
 	case IIO_CHAN_INFO_SCALE:

-- 
2.49.0

[PATCH v2 03/13] iio: light: al3010: Remove DRV_NAME definition

[David Heidelberg via B4 Relay]

From: David Heidelberg <david@ixit.cz>

The driver name should be passed directly.

Signed-off-by: David Heidelberg <david@ixit.cz>
---
 drivers/iio/light/al3010.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/light/al3010.c b/drivers/iio/light/al3010.c
index 4c2fd88ab32cd73f4735b0fa3014af084037c94d..7fe91049b55e57558aef69d088d168437a6819ec 100644
--- a/drivers/iio/light/al3010.c
+++ b/drivers/iio/light/al3010.c
@@ -22,8 +22,6 @@
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
 
-#define AL3010_DRV_NAME "al3010"
-
 #define AL3010_REG_SYSTEM		0x00
 #define AL3010_REG_DATA_LOW		0x0c
 #define AL3010_REG_CONFIG		0x10
@@ -184,7 +182,7 @@ static int al3010_probe(struct i2c_client *client)
 	data->client = client;
 
 	indio_dev->info = &al3010_info;
-	indio_dev->name = AL3010_DRV_NAME;
+	indio_dev->name = "al3010";
 	indio_dev->channels = al3010_channels;
 	indio_dev->num_channels = ARRAY_SIZE(al3010_channels);
 	indio_dev->modes = INDIO_DIRECT_MODE;
@@ -224,7 +222,7 @@ MODULE_DEVICE_TABLE(of, al3010_of_match);
 
 static struct i2c_driver al3010_driver = {
 	.driver = {
-		.name = AL3010_DRV_NAME,
+		.name = "al3010",
 		.of_match_table = al3010_of_match,
 		.pm = pm_sleep_ptr(&al3010_pm_ops),
 	},

-- 
2.49.0

[PATCH v2 04/13] iio: light: al3320a: Remove DRV_NAME definition

[David Heidelberg via B4 Relay]

From: David Heidelberg <david@ixit.cz>

The driver name should be passed directly.

Signed-off-by: David Heidelberg <david@ixit.cz>
---
 drivers/iio/light/al3320a.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/light/al3320a.c b/drivers/iio/light/al3320a.c
index bceda71517c8180dff76db311aa3591ab9846156..93416f3bd7fbb0a75bef17949725d0d40b9b93ea 100644
--- a/drivers/iio/light/al3320a.c
+++ b/drivers/iio/light/al3320a.c
@@ -20,8 +20,6 @@
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
 
-#define AL3320A_DRV_NAME "al3320a"
-
 #define AL3320A_REG_CONFIG		0x00
 #define AL3320A_REG_STATUS		0x01
 #define AL3320A_REG_INT			0x02
@@ -202,7 +200,7 @@ static int al3320a_probe(struct i2c_client *client)
 	data->client = client;
 
 	indio_dev->info = &al3320a_info;
-	indio_dev->name = AL3320A_DRV_NAME;
+	indio_dev->name = "al3320a";
 	indio_dev->channels = al3320a_channels;
 	indio_dev->num_channels = ARRAY_SIZE(al3320a_channels);
 	indio_dev->modes = INDIO_DIRECT_MODE;
@@ -255,7 +253,7 @@ MODULE_DEVICE_TABLE(acpi, al3320a_acpi_match);
 
 static struct i2c_driver al3320a_driver = {
 	.driver = {
-		.name = AL3320A_DRV_NAME,
+		.name = "al3320a",
 		.of_match_table = al3320a_of_match,
 		.pm = pm_sleep_ptr(&al3320a_pm_ops),
 		.acpi_match_table = al3320a_acpi_match,

-- 
2.49.0

[PATCH v2 05/13] iio: light: al3010: Abstract device reference in the probe function

[David Heidelberg via B4 Relay]

From: David Heidelberg <david@ixit.cz>

Introduce a local variable reducing redundancy and improving readability.

No functional changes.

Signed-off-by: David Heidelberg <david@ixit.cz>
---
 drivers/iio/light/al3010.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/light/al3010.c b/drivers/iio/light/al3010.c
index 7fe91049b55e57558aef69d088d168437a6819ec..c8f528f3636a2eb0f0c9586da64a6560d4b42e29 100644
--- a/drivers/iio/light/al3010.c
+++ b/drivers/iio/light/al3010.c
@@ -170,10 +170,11 @@ static const struct iio_info al3010_info = {
 static int al3010_probe(struct i2c_client *client)
 {
 	struct al3010_data *data;
+	struct device *dev = &client->dev;
 	struct iio_dev *indio_dev;
 	int ret;
 
-	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
 	if (!indio_dev)
 		return -ENOMEM;
 
@@ -189,11 +190,11 @@ static int al3010_probe(struct i2c_client *client)
 
 	ret = al3010_init(data);
 	if (ret < 0) {
-		dev_err(&client->dev, "al3010 chip init failed\n");
+		dev_err(dev, "al3010 chip init failed\n");
 		return ret;
 	}
 
-	return devm_iio_device_register(&client->dev, indio_dev);
+	return devm_iio_device_register(dev, indio_dev);
 }
 
 static int al3010_suspend(struct device *dev)

-- 
2.49.0

[PATCH v2 06/13] iio: light: al3320a: Abstract device reference in the probe function

[David Heidelberg via B4 Relay]

From: David Heidelberg <david@ixit.cz>

Introduce a local variable reducing redundancy and improving readability.

No functional changes.

Signed-off-by: David Heidelberg <david@ixit.cz>
---
 drivers/iio/light/al3320a.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/light/al3320a.c b/drivers/iio/light/al3320a.c
index 93416f3bd7fbb0a75bef17949725d0d40b9b93ea..9817cfe8ae18a8e27c82e7362481ebd32a41f8ec 100644
--- a/drivers/iio/light/al3320a.c
+++ b/drivers/iio/light/al3320a.c
@@ -188,10 +188,11 @@ static const struct iio_info al3320a_info = {
 static int al3320a_probe(struct i2c_client *client)
 {
 	struct al3320a_data *data;
+	struct device *dev = &client->dev;
 	struct iio_dev *indio_dev;
 	int ret;
 
-	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
 	if (!indio_dev)
 		return -ENOMEM;
 
@@ -207,17 +208,15 @@ static int al3320a_probe(struct i2c_client *client)
 
 	ret = al3320a_init(data);
 	if (ret < 0) {
-		dev_err(&client->dev, "al3320a chip init failed\n");
+		dev_err(dev, "al3320a chip init failed\n");
 		return ret;
 	}
 
-	ret = devm_add_action_or_reset(&client->dev,
-					al3320a_set_pwr_off,
-					data);
+	ret = devm_add_action_or_reset(dev, al3320a_set_pwr_off, data);
 	if (ret < 0)
 		return ret;
 
-	return devm_iio_device_register(&client->dev, indio_dev);
+	return devm_iio_device_register(dev, indio_dev);
 }
 
 static int al3320a_suspend(struct device *dev)

-- 
2.49.0

[PATCH v2 07/13] iio: light: al3010: Split set_pwr function into set_pwr_on and _off

[David Heidelberg via B4 Relay]

From: David Heidelberg <david@ixit.cz>

Simplifies later conversion to the regmap framework.

Signed-off-by: David Heidelberg <david@ixit.cz>
---
 drivers/iio/light/al3010.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/light/al3010.c b/drivers/iio/light/al3010.c
index c8f528f3636a2eb0f0c9586da64a6560d4b42e29..8c004a9239aef246a8c6f6c3f4acd6b760ee8249 100644
--- a/drivers/iio/light/al3010.c
+++ b/drivers/iio/light/al3010.c
@@ -67,24 +67,25 @@ static const struct attribute_group al3010_attribute_group = {
 	.attrs = al3010_attributes,
 };
 
-static int al3010_set_pwr(struct i2c_client *client, bool pwr)
+static int al3010_set_pwr_on(struct i2c_client *client)
 {
-	u8 val = pwr ? AL3010_CONFIG_ENABLE : AL3010_CONFIG_DISABLE;
-	return i2c_smbus_write_byte_data(client, AL3010_REG_SYSTEM, val);
+	return i2c_smbus_write_byte_data(client, AL3010_REG_SYSTEM,
+					 AL3010_CONFIG_ENABLE);
 }
 
 static void al3010_set_pwr_off(void *_data)
 {
 	struct al3010_data *data = _data;
 
-	al3010_set_pwr(data->client, false);
+	i2c_smbus_write_byte_data(data->client, AL3010_REG_SYSTEM,
+				  AL3010_CONFIG_DISABLE);
 }
 
 static int al3010_init(struct al3010_data *data)
 {
 	int ret;
 
-	ret = al3010_set_pwr(data->client, true);
+	ret = al3010_set_pwr_on(data->client);
 	if (ret < 0)
 		return ret;
 
@@ -199,12 +200,15 @@ static int al3010_probe(struct i2c_client *client)
 
 static int al3010_suspend(struct device *dev)
 {
-	return al3010_set_pwr(to_i2c_client(dev), false);
+	struct al3010_data *data = iio_priv(dev_get_drvdata(dev));
+
+	al3010_set_pwr_off(data);
+	return 0;
 }
 
 static int al3010_resume(struct device *dev)
 {
-	return al3010_set_pwr(to_i2c_client(dev), true);
+	return al3010_set_pwr_on(to_i2c_client(dev));
 }
 
 static DEFINE_SIMPLE_DEV_PM_OPS(al3010_pm_ops, al3010_suspend, al3010_resume);

-- 
2.49.0

[PATCH v2 08/13] iio: light: al3320a: Split set_pwr function into set_pwr_on and _off

[David Heidelberg via B4 Relay]

From: David Heidelberg <david@ixit.cz>

Simplifies later conversion to the regmap framework.

Signed-off-by: David Heidelberg <david@ixit.cz>
---
 drivers/iio/light/al3320a.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/light/al3320a.c b/drivers/iio/light/al3320a.c
index 9817cfe8ae18a8e27c82e7362481ebd32a41f8ec..1b2b0359ed5dad5e00d2fe584f8f3495c13c997e 100644
--- a/drivers/iio/light/al3320a.c
+++ b/drivers/iio/light/al3320a.c
@@ -80,24 +80,23 @@ static const struct attribute_group al3320a_attribute_group = {
 	.attrs = al3320a_attributes,
 };
 
-static int al3320a_set_pwr(struct i2c_client *client, bool pwr)
+static int al3320a_set_pwr_on(struct i2c_client *client)
 {
-	u8 val = pwr ? AL3320A_CONFIG_ENABLE : AL3320A_CONFIG_DISABLE;
-	return i2c_smbus_write_byte_data(client, AL3320A_REG_CONFIG, val);
+	return i2c_smbus_write_byte_data(client, AL3320A_REG_CONFIG, AL3320A_CONFIG_ENABLE);
 }
 
 static void al3320a_set_pwr_off(void *_data)
 {
 	struct al3320a_data *data = _data;
 
-	al3320a_set_pwr(data->client, false);
+	i2c_smbus_write_byte_data(data->client, AL3320A_REG_CONFIG, AL3320A_CONFIG_DISABLE);
 }
 
 static int al3320a_init(struct al3320a_data *data)
 {
 	int ret;
 
-	ret = al3320a_set_pwr(data->client, true);
+	ret = al3320a_set_pwr_on(data->client);
 
 	if (ret < 0)
 		return ret;
@@ -221,12 +220,15 @@ static int al3320a_probe(struct i2c_client *client)
 
 static int al3320a_suspend(struct device *dev)
 {
-	return al3320a_set_pwr(to_i2c_client(dev), false);
+	struct al3320a_data *data = iio_priv(dev_get_drvdata(dev));
+
+	al3320a_set_pwr_off(data);
+	return 0;
 }
 
 static int al3320a_resume(struct device *dev)
 {
-	return al3320a_set_pwr(to_i2c_client(dev), true);
+	return al3320a_set_pwr_on(to_i2c_client(dev));
 }
 
 static DEFINE_SIMPLE_DEV_PM_OPS(al3320a_pm_ops, al3320a_suspend,

-- 
2.49.0

[PATCH v2 09/13] iio: light: al3010: Move devm_add_action_or_reset back to _probe

[David Heidelberg via B4 Relay]

From: David Heidelberg <david@ixit.cz>

In a preparation to the regmap transition.

Improve error handling using dev_err_probe().

Signed-off-by: David Heidelberg <david@ixit.cz>
---
 drivers/iio/light/al3010.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/light/al3010.c b/drivers/iio/light/al3010.c
index 8c004a9239aef246a8c6f6c3f4acd6b760ee8249..8098c92c9572befe92d00ef0785ded5e1a08d587 100644
--- a/drivers/iio/light/al3010.c
+++ b/drivers/iio/light/al3010.c
@@ -89,12 +89,6 @@ static int al3010_init(struct al3010_data *data)
 	if (ret < 0)
 		return ret;
 
-	ret = devm_add_action_or_reset(&data->client->dev,
-				       al3010_set_pwr_off,
-				       data);
-	if (ret < 0)
-		return ret;
-
 	ret = i2c_smbus_write_byte_data(data->client, AL3010_REG_CONFIG,
 					FIELD_PREP(AL3010_GAIN_MASK,
 						   AL3XXX_RANGE_3));
@@ -195,6 +189,10 @@ static int al3010_probe(struct i2c_client *client)
 		return ret;
 	}
 
+	ret = devm_add_action_or_reset(dev, al3010_set_pwr_off, data);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to add action\n");
+
 	return devm_iio_device_register(dev, indio_dev);
 }
 

-- 
2.49.0

[PATCH v2 10/13] iio: light: al3010: Improve al3010_init error handling with dev_err_probe

[David Heidelberg via B4 Relay]

From: David Heidelberg <david@ixit.cz>

Slight simplification of the code.

Signed-off-by: David Heidelberg <david@ixit.cz>
---
 drivers/iio/light/al3010.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/light/al3010.c b/drivers/iio/light/al3010.c
index 8098c92c9572befe92d00ef0785ded5e1a08d587..af7ed028259837f2232f30072b87cc0da7c77f37 100644
--- a/drivers/iio/light/al3010.c
+++ b/drivers/iio/light/al3010.c
@@ -184,10 +184,8 @@ static int al3010_probe(struct i2c_client *client)
 	indio_dev->modes = INDIO_DIRECT_MODE;
 
 	ret = al3010_init(data);
-	if (ret < 0) {
-		dev_err(dev, "al3010 chip init failed\n");
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to init ALS\n");
 
 	ret = devm_add_action_or_reset(dev, al3010_set_pwr_off, data);
 	if (ret)

-- 
2.49.0

[PATCH v2 11/13] iio: light: al3320a: Improve error handling with dev_err_probe

[David Heidelberg via B4 Relay]

From: David Heidelberg <david@ixit.cz>

Slight simplification of the code.

Signed-off-by: David Heidelberg <david@ixit.cz>
---
 drivers/iio/light/al3320a.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/light/al3320a.c b/drivers/iio/light/al3320a.c
index 1b2b0359ed5dad5e00d2fe584f8f3495c13c997e..1943e6f34a70b00b5d732dbf4ae6ccb4376303b7 100644
--- a/drivers/iio/light/al3320a.c
+++ b/drivers/iio/light/al3320a.c
@@ -206,14 +206,12 @@ static int al3320a_probe(struct i2c_client *client)
 	indio_dev->modes = INDIO_DIRECT_MODE;
 
 	ret = al3320a_init(data);
-	if (ret < 0) {
-		dev_err(dev, "al3320a chip init failed\n");
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to init ALS\n");
 
 	ret = devm_add_action_or_reset(dev, al3320a_set_pwr_off, data);
-	if (ret < 0)
-		return ret;
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to add action\n");
 
 	return devm_iio_device_register(dev, indio_dev);
 }

-- 
2.49.0

[PATCH v2 12/13] iio: light: al3010: Implement regmap support

[David Heidelberg via B4 Relay]

From: David Heidelberg <david@ixit.cz>

Modernize and make driver a bit cleaner.

After the regmap implementation, the compiler is able to produce
much smaller module.

Size before: 72 kB
Size after:  58 kB

Signed-off-by: David Heidelberg <david@ixit.cz>
---
 drivers/iio/light/al3010.c | 70 ++++++++++++++++++++++++++--------------------
 1 file changed, 39 insertions(+), 31 deletions(-)

diff --git a/drivers/iio/light/al3010.c b/drivers/iio/light/al3010.c
index af7ed028259837f2232f30072b87cc0da7c77f37..04d9d49ef20930f47b642ef118514be6e016368a 100644
--- a/drivers/iio/light/al3010.c
+++ b/drivers/iio/light/al3010.c
@@ -17,6 +17,7 @@
 #include <linux/bitfield.h>
 #include <linux/i2c.h>
 #include <linux/module.h>
+#include <linux/regmap.h>
 #include <linux/mod_devicetable.h>
 
 #include <linux/iio/iio.h>
@@ -44,8 +45,14 @@ static const int al3010_scales[][2] = {
 	{0, 1187200}, {0, 296800}, {0, 74200}, {0, 18600}
 };
 
+static const struct regmap_config al3010_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = AL3010_REG_CONFIG,
+};
+
 struct al3010_data {
-	struct i2c_client *client;
+	struct regmap *regmap;
 };
 
 static const struct iio_chan_spec al3010_channels[] = {
@@ -67,35 +74,32 @@ static const struct attribute_group al3010_attribute_group = {
 	.attrs = al3010_attributes,
 };
 
-static int al3010_set_pwr_on(struct i2c_client *client)
+static int al3010_set_pwr_on(struct al3010_data *data)
 {
-	return i2c_smbus_write_byte_data(client, AL3010_REG_SYSTEM,
-					 AL3010_CONFIG_ENABLE);
+	return regmap_write(data->regmap, AL3010_REG_SYSTEM, AL3010_CONFIG_ENABLE);
 }
 
 static void al3010_set_pwr_off(void *_data)
 {
 	struct al3010_data *data = _data;
+	struct device *dev = regmap_get_device(data->regmap);
+	int ret;
 
-	i2c_smbus_write_byte_data(data->client, AL3010_REG_SYSTEM,
-				  AL3010_CONFIG_DISABLE);
+	ret = regmap_write(data->regmap, AL3010_REG_SYSTEM, AL3010_CONFIG_DISABLE);
+	if (ret)
+		dev_err(dev, "failed to write system register\n");
 }
 
 static int al3010_init(struct al3010_data *data)
 {
 	int ret;
 
-	ret = al3010_set_pwr_on(data->client);
-	if (ret < 0)
-		return ret;
-
-	ret = i2c_smbus_write_byte_data(data->client, AL3010_REG_CONFIG,
-					FIELD_PREP(AL3010_GAIN_MASK,
-						   AL3XXX_RANGE_3));
-	if (ret < 0)
+	ret = al3010_set_pwr_on(data);
+	if (ret)
 		return ret;
 
-	return 0;
+	return regmap_write(data->regmap, AL3010_REG_CONFIG,
+			    FIELD_PREP(AL3010_GAIN_MASK, AL3XXX_RANGE_3));
 }
 
 static int al3010_read_raw(struct iio_dev *indio_dev,
@@ -103,7 +107,7 @@ static int al3010_read_raw(struct iio_dev *indio_dev,
 			   int *val2, long mask)
 {
 	struct al3010_data *data = iio_priv(indio_dev);
-	int ret;
+	int ret, gain, raw;
 
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
@@ -112,21 +116,21 @@ static int al3010_read_raw(struct iio_dev *indio_dev,
 		 * - low byte of output is stored at AL3010_REG_DATA_LOW
 		 * - high byte of output is stored at AL3010_REG_DATA_LOW + 1
 		 */
-		ret = i2c_smbus_read_word_data(data->client,
-					       AL3010_REG_DATA_LOW);
-		if (ret < 0)
+		ret = regmap_read(data->regmap, AL3010_REG_DATA_LOW, &raw);
+		if (ret)
 			return ret;
-		*val = ret;
+
+		*val = raw;
+
 		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_SCALE:
-		ret = i2c_smbus_read_byte_data(data->client,
-					       AL3010_REG_CONFIG);
-		if (ret < 0)
+		ret = regmap_read(data->regmap, AL3010_REG_CONFIG, &gain);
+		if (ret)
 			return ret;
 
-		ret = FIELD_GET(AL3010_GAIN_MASK, ret);
-		*val = al3010_scales[ret][0];
-		*val2 = al3010_scales[ret][1];
+		gain = FIELD_GET(AL3010_GAIN_MASK, gain);
+		*val = al3010_scales[gain][0];
+		*val2 = al3010_scales[gain][1];
 
 		return IIO_VAL_INT_PLUS_MICRO;
 	}
@@ -147,9 +151,8 @@ static int al3010_write_raw(struct iio_dev *indio_dev,
 			    val2 != al3010_scales[i][1])
 				continue;
 
-			return i2c_smbus_write_byte_data(data->client,
-					AL3010_REG_CONFIG,
-					FIELD_PREP(AL3010_GAIN_MASK, i));
+			return regmap_write(data->regmap, AL3010_REG_CONFIG,
+					    FIELD_PREP(AL3010_GAIN_MASK, i));
 		}
 		break;
 	}
@@ -175,7 +178,10 @@ static int al3010_probe(struct i2c_client *client)
 
 	data = iio_priv(indio_dev);
 	i2c_set_clientdata(client, indio_dev);
-	data->client = client;
+	data->regmap = devm_regmap_init_i2c(client, &al3010_regmap_config);
+	if (IS_ERR(data->regmap))
+		return dev_err_probe(dev, PTR_ERR(data->regmap),
+				     "cannot allocate regmap\n");
 
 	indio_dev->info = &al3010_info;
 	indio_dev->name = "al3010";
@@ -204,7 +210,9 @@ static int al3010_suspend(struct device *dev)
 
 static int al3010_resume(struct device *dev)
 {
-	return al3010_set_pwr_on(to_i2c_client(dev));
+	struct al3010_data *data = iio_priv(dev_get_drvdata(dev));
+
+	return al3010_set_pwr_on(data);
 }
 
 static DEFINE_SIMPLE_DEV_PM_OPS(al3010_pm_ops, al3010_suspend, al3010_resume);

-- 
2.49.0

[PATCH v2 13/13] iio: light: al3320a: Implement regmap support

[David Heidelberg via B4 Relay]

From: David Heidelberg <david@ixit.cz>

Modernize and make driver a bit cleaner.

After the regmap implementation, the compiler is able to produce
much smaller module.

Size before: 72 kB
Size after:  58 kB

Signed-off-by: David Heidelberg <david@ixit.cz>
---
 drivers/iio/light/al3320a.c | 82 +++++++++++++++++++++++++--------------------
 1 file changed, 46 insertions(+), 36 deletions(-)

diff --git a/drivers/iio/light/al3320a.c b/drivers/iio/light/al3320a.c
index 1943e6f34a70b00b5d732dbf4ae6ccb4376303b7..dc9534195e1db9366a5307b84eeab96b99f16b96 100644
--- a/drivers/iio/light/al3320a.c
+++ b/drivers/iio/light/al3320a.c
@@ -15,6 +15,7 @@
 #include <linux/bitfield.h>
 #include <linux/i2c.h>
 #include <linux/module.h>
+#include <linux/regmap.h>
 #include <linux/mod_devicetable.h>
 
 #include <linux/iio/iio.h>
@@ -57,8 +58,14 @@ static const int al3320a_scales[][2] = {
 	{0, 512000}, {0, 128000}, {0, 32000}, {0, 10000}
 };
 
+static const struct regmap_config al3320a_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = AL3320A_REG_HIGH_THRESH_HIGH,
+};
+
 struct al3320a_data {
-	struct i2c_client *client;
+	struct regmap *regmap;
 };
 
 static const struct iio_chan_spec al3320a_channels[] = {
@@ -80,44 +87,42 @@ static const struct attribute_group al3320a_attribute_group = {
 	.attrs = al3320a_attributes,
 };
 
-static int al3320a_set_pwr_on(struct i2c_client *client)
+static int al3320a_set_pwr_on(struct al3320a_data *data)
 {
-	return i2c_smbus_write_byte_data(client, AL3320A_REG_CONFIG, AL3320A_CONFIG_ENABLE);
+	return regmap_write(data->regmap, AL3320A_REG_CONFIG, AL3320A_CONFIG_ENABLE);
 }
 
 static void al3320a_set_pwr_off(void *_data)
 {
 	struct al3320a_data *data = _data;
+	struct device *dev = regmap_get_device(data->regmap);
+	int ret;
 
-	i2c_smbus_write_byte_data(data->client, AL3320A_REG_CONFIG, AL3320A_CONFIG_DISABLE);
+	ret = regmap_write(data->regmap, AL3320A_REG_CONFIG, AL3320A_CONFIG_DISABLE);
+	if (ret)
+		dev_err(dev, "failed to write system register\n");
 }
 
 static int al3320a_init(struct al3320a_data *data)
 {
 	int ret;
 
-	ret = al3320a_set_pwr_on(data->client);
-
-	if (ret < 0)
-		return ret;
-
-	ret = i2c_smbus_write_byte_data(data->client, AL3320A_REG_CONFIG_RANGE,
-					FIELD_PREP(AL3320A_GAIN_MASK,
-						   AL3320A_RANGE_3));
-	if (ret < 0)
+	ret = al3320a_set_pwr_on(data);
+	if (ret)
 		return ret;
 
-	ret = i2c_smbus_write_byte_data(data->client, AL3320A_REG_MEAN_TIME,
-					AL3320A_DEFAULT_MEAN_TIME);
-	if (ret < 0)
+	ret = regmap_write(data->regmap, AL3320A_REG_CONFIG_RANGE,
+			   FIELD_PREP(AL3320A_GAIN_MASK, AL3320A_RANGE_3));
+	if (ret)
 		return ret;
 
-	ret = i2c_smbus_write_byte_data(data->client, AL3320A_REG_WAIT,
-					AL3320A_DEFAULT_WAIT_TIME);
-	if (ret < 0)
+	ret = regmap_write(data->regmap, AL3320A_REG_MEAN_TIME,
+			   AL3320A_DEFAULT_MEAN_TIME);
+	if (ret)
 		return ret;
 
-	return 0;
+	return regmap_write(data->regmap, AL3320A_REG_WAIT,
+			    AL3320A_DEFAULT_WAIT_TIME);
 }
 
 static int al3320a_read_raw(struct iio_dev *indio_dev,
@@ -125,7 +130,7 @@ static int al3320a_read_raw(struct iio_dev *indio_dev,
 			    int *val2, long mask)
 {
 	struct al3320a_data *data = iio_priv(indio_dev);
-	int ret;
+	int ret, gain, raw;
 
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
@@ -134,21 +139,21 @@ static int al3320a_read_raw(struct iio_dev *indio_dev,
 		 * - low byte of output is stored at AL3320A_REG_DATA_LOW
 		 * - high byte of output is stored at AL3320A_REG_DATA_LOW + 1
 		 */
-		ret = i2c_smbus_read_word_data(data->client,
-					       AL3320A_REG_DATA_LOW);
-		if (ret < 0)
+		ret = regmap_read(data->regmap, AL3320A_REG_DATA_LOW, &raw);
+		if (ret)
 			return ret;
-		*val = ret;
+
+		*val = raw;
+
 		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_SCALE:
-		ret = i2c_smbus_read_byte_data(data->client,
-					       AL3320A_REG_CONFIG_RANGE);
-		if (ret < 0)
+		ret = regmap_read(data->regmap, AL3320A_REG_CONFIG_RANGE, &gain);
+		if (ret)
 			return ret;
 
-		ret = FIELD_GET(AL3320A_GAIN_MASK, ret);
-		*val = al3320a_scales[ret][0];
-		*val2 = al3320a_scales[ret][1];
+		gain = FIELD_GET(AL3320A_GAIN_MASK, gain);
+		*val = al3320a_scales[gain][0];
+		*val2 = al3320a_scales[gain][1];
 
 		return IIO_VAL_INT_PLUS_MICRO;
 	}
@@ -169,9 +174,8 @@ static int al3320a_write_raw(struct iio_dev *indio_dev,
 			    val2 != al3320a_scales[i][1])
 				continue;
 
-			return i2c_smbus_write_byte_data(data->client,
-					AL3320A_REG_CONFIG_RANGE,
-					FIELD_PREP(AL3320A_GAIN_MASK, i));
+			return regmap_write(data->regmap, AL3320A_REG_CONFIG_RANGE,
+					    FIELD_PREP(AL3320A_GAIN_MASK, i));
 		}
 		break;
 	}
@@ -197,7 +201,11 @@ static int al3320a_probe(struct i2c_client *client)
 
 	data = iio_priv(indio_dev);
 	i2c_set_clientdata(client, indio_dev);
-	data->client = client;
+
+	data->regmap = devm_regmap_init_i2c(client, &al3320a_regmap_config);
+	if (IS_ERR(data->regmap))
+		return dev_err_probe(dev, PTR_ERR(data->regmap),
+				     "cannot allocate regmap\n");
 
 	indio_dev->info = &al3320a_info;
 	indio_dev->name = "al3320a";
@@ -226,7 +234,9 @@ static int al3320a_suspend(struct device *dev)
 
 static int al3320a_resume(struct device *dev)
 {
-	return al3320a_set_pwr_on(to_i2c_client(dev));
+	struct al3320a_data *data = iio_priv(dev_get_drvdata(dev));
+
+	return al3320a_set_pwr_on(data);
 }
 
 static DEFINE_SIMPLE_DEV_PM_OPS(al3320a_pm_ops, al3320a_suspend,

-- 
2.49.0

Re: [PATCH v2 01/13] iio: light: al3010: Use unsigned int for the indexing

[Jonathan Cameron]

On Wed, 19 Mar 2025 21:59:40 +0100
David Heidelberg via B4 Relay <devnull+david.ixit.cz@kernel.org> wrote:

> From: David Heidelberg <david@ixit.cz>
> 
> The integer is used as array index which cannot be negative.
> 
> Signed-off-by: David Heidelberg <david@ixit.cz>
I do find it hard to care about these, but you are making
other changes so fair enough I guess.

Applied patches 1-2

> ---
>  drivers/iio/light/al3010.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/light/al3010.c b/drivers/iio/light/al3010.c
> index 7cbb8b203300907a88f4a0ab87da89cabdd087f3..4c2fd88ab32cd73f4735b0fa3014af084037c94d 100644
> --- a/drivers/iio/light/al3010.c
> +++ b/drivers/iio/light/al3010.c
> @@ -145,7 +145,7 @@ static int al3010_write_raw(struct iio_dev *indio_dev,
>  			    int val2, long mask)
>  {
>  	struct al3010_data *data = iio_priv(indio_dev);
> -	int i;
> +	unsigned int i;
>  
>  	switch (mask) {
>  	case IIO_CHAN_INFO_SCALE:
> 

Re: [PATCH v2 03/13] iio: light: al3010: Remove DRV_NAME definition

[Jonathan Cameron]

On Wed, 19 Mar 2025 21:59:42 +0100
David Heidelberg via B4 Relay <devnull+david.ixit.cz@kernel.org> wrote:

> From: David Heidelberg <david@ixit.cz>
> 
> The driver name should be passed directly.
> 
> Signed-off-by: David Heidelberg <david@ixit.cz>
Also things I'd generally not worry too much about in existing code.
Given they are part of a larger series though I'll take them.
Applied 3-4

> ---
>  drivers/iio/light/al3010.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iio/light/al3010.c b/drivers/iio/light/al3010.c
> index 4c2fd88ab32cd73f4735b0fa3014af084037c94d..7fe91049b55e57558aef69d088d168437a6819ec 100644
> --- a/drivers/iio/light/al3010.c
> +++ b/drivers/iio/light/al3010.c
> @@ -22,8 +22,6 @@
>  #include <linux/iio/iio.h>
>  #include <linux/iio/sysfs.h>
>  
> -#define AL3010_DRV_NAME "al3010"
> -
>  #define AL3010_REG_SYSTEM		0x00
>  #define AL3010_REG_DATA_LOW		0x0c
>  #define AL3010_REG_CONFIG		0x10
> @@ -184,7 +182,7 @@ static int al3010_probe(struct i2c_client *client)
>  	data->client = client;
>  
>  	indio_dev->info = &al3010_info;
> -	indio_dev->name = AL3010_DRV_NAME;
> +	indio_dev->name = "al3010";
>  	indio_dev->channels = al3010_channels;
>  	indio_dev->num_channels = ARRAY_SIZE(al3010_channels);
>  	indio_dev->modes = INDIO_DIRECT_MODE;
> @@ -224,7 +222,7 @@ MODULE_DEVICE_TABLE(of, al3010_of_match);
>  
>  static struct i2c_driver al3010_driver = {
>  	.driver = {
> -		.name = AL3010_DRV_NAME,
> +		.name = "al3010",
>  		.of_match_table = al3010_of_match,
>  		.pm = pm_sleep_ptr(&al3010_pm_ops),
>  	},
> 

Re: [PATCH v2 05/13] iio: light: al3010: Abstract device reference in the probe function

[Jonathan Cameron]

On Wed, 19 Mar 2025 21:59:44 +0100
David Heidelberg via B4 Relay <devnull+david.ixit.cz@kernel.org> wrote:

> From: David Heidelberg <david@ixit.cz>
> 
> Introduce a local variable reducing redundancy and improving readability.
> 
> No functional changes.
> 
> Signed-off-by: David Heidelberg <david@ixit.cz>
Applied 5-6

Re: [PATCH v2 07/13] iio: light: al3010: Split set_pwr function into set_pwr_on and _off

[Jonathan Cameron]

On Wed, 19 Mar 2025 21:59:46 +0100
David Heidelberg via B4 Relay <devnull+david.ixit.cz@kernel.org> wrote:

> From: David Heidelberg <david@ixit.cz>
> 
> Simplifies later conversion to the regmap framework.
> 
> Signed-off-by: David Heidelberg <david@ixit.cz>
Applied 7-8

Re: [PATCH v2 09/13] iio: light: al3010: Move devm_add_action_or_reset back to _probe

[Jonathan Cameron]

On Wed, 19 Mar 2025 21:59:48 +0100
David Heidelberg via B4 Relay <devnull+david.ixit.cz@kernel.org> wrote:

> From: David Heidelberg <david@ixit.cz>
> 
> In a preparation to the regmap transition.
I'm not following the reasoning for this patch.

The setup action being unwound is done in _init()
and if we happen to get an error on the write immediately after
where it currently is, we no longer power down the device.



> 
> Improve error handling using dev_err_probe().
> 
> Signed-off-by: David Heidelberg <david@ixit.cz>
> ---
>  drivers/iio/light/al3010.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/iio/light/al3010.c b/drivers/iio/light/al3010.c
> index 8c004a9239aef246a8c6f6c3f4acd6b760ee8249..8098c92c9572befe92d00ef0785ded5e1a08d587 100644
> --- a/drivers/iio/light/al3010.c
> +++ b/drivers/iio/light/al3010.c
> @@ -89,12 +89,6 @@ static int al3010_init(struct al3010_data *data)
>  	if (ret < 0)
>  		return ret;
>  
> -	ret = devm_add_action_or_reset(&data->client->dev,
> -				       al3010_set_pwr_off,
> -				       data);
> -	if (ret < 0)
> -		return ret;
> -
>  	ret = i2c_smbus_write_byte_data(data->client, AL3010_REG_CONFIG,
>  					FIELD_PREP(AL3010_GAIN_MASK,
>  						   AL3XXX_RANGE_3));
> @@ -195,6 +189,10 @@ static int al3010_probe(struct i2c_client *client)
>  		return ret;
>  	}
>  
> +	ret = devm_add_action_or_reset(dev, al3010_set_pwr_off, data);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to add action\n");
> +
>  	return devm_iio_device_register(dev, indio_dev);
>  }
>  
> 

Re: [PATCH v2 10/13] iio: light: al3010: Improve al3010_init error handling with dev_err_probe

[Jonathan Cameron]

On Wed, 19 Mar 2025 21:59:49 +0100
David Heidelberg via B4 Relay <devnull+david.ixit.cz@kernel.org> wrote:

> From: David Heidelberg <david@ixit.cz>
> 
> Slight simplification of the code.
> 
> Signed-off-by: David Heidelberg <david@ixit.cz>
This one is fine but given I've not picked up 9 I can't pick this up either
for this version.

Thanks,

Jonathan

> ---
>  drivers/iio/light/al3010.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iio/light/al3010.c b/drivers/iio/light/al3010.c
> index 8098c92c9572befe92d00ef0785ded5e1a08d587..af7ed028259837f2232f30072b87cc0da7c77f37 100644
> --- a/drivers/iio/light/al3010.c
> +++ b/drivers/iio/light/al3010.c
> @@ -184,10 +184,8 @@ static int al3010_probe(struct i2c_client *client)
>  	indio_dev->modes = INDIO_DIRECT_MODE;
>  
>  	ret = al3010_init(data);
> -	if (ret < 0) {
> -		dev_err(dev, "al3010 chip init failed\n");
> -		return ret;
> -	}
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to init ALS\n");
>  
>  	ret = devm_add_action_or_reset(dev, al3010_set_pwr_off, data);
>  	if (ret)
> 

Re: [PATCH v2 11/13] iio: light: al3320a: Improve error handling with dev_err_probe

[Jonathan Cameron]

On Wed, 19 Mar 2025 21:59:50 +0100
David Heidelberg via B4 Relay <devnull+david.ixit.cz@kernel.org> wrote:

> From: David Heidelberg <david@ixit.cz>
> 
> Slight simplification of the code.
> 
> Signed-off-by: David Heidelberg <david@ixit.cz>
> ---
>  drivers/iio/light/al3320a.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/iio/light/al3320a.c b/drivers/iio/light/al3320a.c
> index 1b2b0359ed5dad5e00d2fe584f8f3495c13c997e..1943e6f34a70b00b5d732dbf4ae6ccb4376303b7 100644
> --- a/drivers/iio/light/al3320a.c
> +++ b/drivers/iio/light/al3320a.c
> @@ -206,14 +206,12 @@ static int al3320a_probe(struct i2c_client *client)
>  	indio_dev->modes = INDIO_DIRECT_MODE;
>  
>  	ret = al3320a_init(data);
> -	if (ret < 0) {
> -		dev_err(dev, "al3320a chip init failed\n");
> -		return ret;
> -	}
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to init ALS\n");
>  
>  	ret = devm_add_action_or_reset(dev, al3320a_set_pwr_off, data);
Seeing this here made me look. Seems that the error path I highlighted in
patch 9 is here as well. I'd push this devm registration down into init
where it can immediately follow the power up.

> -	if (ret < 0)
> -		return ret;
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to add action\n");
>  
>  	return devm_iio_device_register(dev, indio_dev);
>  }
> 

Re: [PATCH v2 12/13] iio: light: al3010: Implement regmap support

[Jonathan Cameron]

On Wed, 19 Mar 2025 21:59:51 +0100
David Heidelberg via B4 Relay <devnull+david.ixit.cz@kernel.org> wrote:

> From: David Heidelberg <david@ixit.cz>
> 
> Modernize and make driver a bit cleaner.
> 
> After the regmap implementation, the compiler is able to produce
> much smaller module.
> 
> Size before: 72 kB
> Size after:  58 kB
> 
> Signed-off-by: David Heidelberg <david@ixit.cz>
Just one real comment on your code. 
Also a passing musing that you can feel free to ignore!

Jonathan


>  static void al3010_set_pwr_off(void *_data)
>  {
>  	struct al3010_data *data = _data;
> +	struct device *dev = regmap_get_device(data->regmap);
> +	int ret;
>  
> -	i2c_smbus_write_byte_data(data->client, AL3010_REG_SYSTEM,
> -				  AL3010_CONFIG_DISABLE);
> +	ret = regmap_write(data->regmap, AL3010_REG_SYSTEM, AL3010_CONFIG_DISABLE);
> +	if (ret)
> +		dev_err(dev, "failed to write system register\n");

Unrelated to your patch, but I wonder if there is appetite for
		regmap_err(data->regmap, "failed to write system register\n");

A lot of drivers carry a local dev pointer because the regmap_get_device() stuff
is a bit clunky. They only use it in many paths for prints like this.

>  }


> @@ -204,7 +210,9 @@ static int al3010_suspend(struct device *dev)
>  
>  static int al3010_resume(struct device *dev)
>  {
> -	return al3010_set_pwr_on(to_i2c_client(dev));
> +	struct al3010_data *data = iio_priv(dev_get_drvdata(dev));

Why this change?

> +
> +	return al3010_set_pwr_on(data);
>  }
>  
>  static DEFINE_SIMPLE_DEV_PM_OPS(al3010_pm_ops, al3010_suspend, al3010_resume);
> 

Re: [PATCH v2 12/13] iio: light: al3010: Implement regmap support

[Jonathan Cameron]

On Sun, 30 Mar 2025 18:45:38 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> On Wed, 19 Mar 2025 21:59:51 +0100
> David Heidelberg via B4 Relay <devnull+david.ixit.cz@kernel.org> wrote:
> 
> > From: David Heidelberg <david@ixit.cz>
> > 
> > Modernize and make driver a bit cleaner.
> > 
> > After the regmap implementation, the compiler is able to produce
> > much smaller module.
> > 
> > Size before: 72 kB
> > Size after:  58 kB
> > 
> > Signed-off-by: David Heidelberg <david@ixit.cz>  
> Just one real comment on your code. 
> Also a passing musing that you can feel free to ignore!
> 
> Jonathan
> 
> 
> >  static void al3010_set_pwr_off(void *_data)
> >  {
> >  	struct al3010_data *data = _data;
> > +	struct device *dev = regmap_get_device(data->regmap);
> > +	int ret;
> >  
> > -	i2c_smbus_write_byte_data(data->client, AL3010_REG_SYSTEM,
> > -				  AL3010_CONFIG_DISABLE);
> > +	ret = regmap_write(data->regmap, AL3010_REG_SYSTEM, AL3010_CONFIG_DISABLE);
> > +	if (ret)
> > +		dev_err(dev, "failed to write system register\n");  
> 
> Unrelated to your patch, but I wonder if there is appetite for
> 		regmap_err(data->regmap, "failed to write system register\n");
> 
> A lot of drivers carry a local dev pointer because the regmap_get_device() stuff
> is a bit clunky. They only use it in many paths for prints like this.
> 
> >  }  
> 
> 
> > @@ -204,7 +210,9 @@ static int al3010_suspend(struct device *dev)
> >  
> >  static int al3010_resume(struct device *dev)
> >  {
> > -	return al3010_set_pwr_on(to_i2c_client(dev));
> > +	struct al3010_data *data = iio_priv(dev_get_drvdata(dev));  
> 
> Why this change?
Ah. Obvious.  Ignore me.
> 
> > +
> > +	return al3010_set_pwr_on(data);
> >  }
> >  
> >  static DEFINE_SIMPLE_DEV_PM_OPS(al3010_pm_ops, al3010_suspend, al3010_resume);
> >   
> 

Re: [PATCH v2 13/13] iio: light: al3320a: Implement regmap support

[Jonathan Cameron]

On Wed, 19 Mar 2025 21:59:52 +0100
David Heidelberg via B4 Relay <devnull+david.ixit.cz@kernel.org> wrote:

> From: David Heidelberg <david@ixit.cz>
> 
> Modernize and make driver a bit cleaner.
> 
> After the regmap implementation, the compiler is able to produce
> much smaller module.
> 
> Size before: 72 kB
> Size after:  58 kB
> 
> Signed-off-by: David Heidelberg <david@ixit.cz>

LGTM

Re: [PATCH v2 09/13] iio: light: al3010: Move devm_add_action_or_reset back to _probe

[David Heidelberg]

On 30/03/2025 19:34, Jonathan Cameron wrote:
> On Wed, 19 Mar 2025 21:59:48 +0100
> David Heidelberg via B4 Relay <devnull+david.ixit.cz@kernel.org> wrote:
> 
>> From: David Heidelberg <david@ixit.cz>
>>
>> In a preparation to the regmap transition.
> I'm not following the reasoning for this patch.
> 
> The setup action being unwound is done in _init()
> and if we happen to get an error on the write immediately after
> where it currently is, we no longer power down the device.

Good point, thank you.

In next series I kept the devm_add_action in place and moved it for 
al3000a and a3320a to the right place.

David>
> 
> 
>>
>> Improve error handling using dev_err_probe().
>>
>> Signed-off-by: David Heidelberg <david@ixit.cz>
>> ---
>>   drivers/iio/light/al3010.c | 10 ++++------
>>   1 file changed, 4 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/iio/light/al3010.c b/drivers/iio/light/al3010.c
>> index 8c004a9239aef246a8c6f6c3f4acd6b760ee8249..8098c92c9572befe92d00ef0785ded5e1a08d587 100644
>> --- a/drivers/iio/light/al3010.c
>> +++ b/drivers/iio/light/al3010.c
>> @@ -89,12 +89,6 @@ static int al3010_init(struct al3010_data *data)
>>   	if (ret < 0)
>>   		return ret;
>>   
>> -	ret = devm_add_action_or_reset(&data->client->dev,
>> -				       al3010_set_pwr_off,
>> -				       data);
>> -	if (ret < 0)
>> -		return ret;
>> -
>>   	ret = i2c_smbus_write_byte_data(data->client, AL3010_REG_CONFIG,
>>   					FIELD_PREP(AL3010_GAIN_MASK,
>>   						   AL3XXX_RANGE_3));
>> @@ -195,6 +189,10 @@ static int al3010_probe(struct i2c_client *client)
>>   		return ret;
>>   	}
>>   
>> +	ret = devm_add_action_or_reset(dev, al3010_set_pwr_off, data);
>> +	if (ret)
>> +		return dev_err_probe(dev, ret, "failed to add action\n");
>> +
>>   	return devm_iio_device_register(dev, indio_dev);
>>   }
>>   
>>
> 

-- 
David Heidelberg