[PATCH 0/2] Add support for the DFRobot SEN0322 oxygen sensor

[PATCH 0/2] Add support for the DFRobot SEN0322 oxygen sensor

[Tóth János via B4 Relay]

This patchset adds a driver and the documentation for the
DFRobot SEN0322 oxygen sensor.

Signed-off-by: Tóth János <gomba007@gmail.com>
---
Tóth János (2):
      dt-bindings: iio: chemical: Document SEN0322
      iio: chemical: Add driver for SEN0322

 .../bindings/iio/chemical/dfrobot,sen0322.yaml     |  41 ++++
 MAINTAINERS                                        |   6 +
 drivers/iio/chemical/Kconfig                       |  10 +
 drivers/iio/chemical/Makefile                      |   1 +
 drivers/iio/chemical/sen0322.c                     | 238 +++++++++++++++++++++
 5 files changed, 296 insertions(+)
---
base-commit: b4432656b36e5cc1d50a1f2dc15357543add530e
change-id: 20250428-iio-chemical-sen0322-cf8fbbbe7546

Best regards,
-- 
Tóth János <gomba007@gmail.com>

[PATCH 1/2] dt-bindings: iio: chemical: Document SEN0322

[Tóth János via B4 Relay]

From: Tóth János <gomba007@gmail.com>

Add documentation for the DFRobot SEN0322 oxygen sensor.

Signed-off-by: Tóth János <gomba007@gmail.com>
---
 .../bindings/iio/chemical/dfrobot,sen0322.yaml     | 41 ++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/chemical/dfrobot,sen0322.yaml b/Documentation/devicetree/bindings/iio/chemical/dfrobot,sen0322.yaml
new file mode 100644
index 000000000000..9410d04fb91d
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/chemical/dfrobot,sen0322.yaml
@@ -0,0 +1,41 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/chemical/dfrobot,sen0322.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: DFRobot SEN0322 oxygen sensor
+
+maintainers:
+  - Tóth János <gomba007@gmail.com>
+
+description: >
+  DFRobot SEN0322 is an oxygen sensor. It supports I2C for communication.
+
+  Datasheet:
+    https://wiki.dfrobot.com/Gravity_I2C_Oxygen_Sensor_SKU_SEN0322
+
+properties:
+  compatible:
+    const: dfrobot,sen0322
+
+  reg:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      sen0322@73 {
+        compatible = "dfrobot,sen0322";
+        reg = <0x73>;
+      };
+    };

-- 
2.34.1

[PATCH 2/2] iio: chemical: Add driver for SEN0322

[Tóth János via B4 Relay]

From: Tóth János <gomba007@gmail.com>

Add support for the DFRobot SEN0322 oxygen sensor.

Datasheet:
	https://wiki.dfrobot.com/Gravity_I2C_Oxygen_Sensor_SKU_SEN0322

To instantiate (assuming device is connected to I2C-2):
	echo 'sen0322 0x73' > /sys/class/i2c-dev/i2c-2/device/new_device

To read the oxygen concentration (assuming device is iio:device0):
	cat /sys/bus/iio/devices/iio:device0/in_concentration_input

Signed-off-by: Tóth János <gomba007@gmail.com>
---
 MAINTAINERS                    |   6 ++
 drivers/iio/chemical/Kconfig   |  10 ++
 drivers/iio/chemical/Makefile  |   1 +
 drivers/iio/chemical/sen0322.c | 238 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 255 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 3cbf9ac0d83f..6fda7a2f1248 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6852,6 +6852,12 @@ L:	linux-rtc@vger.kernel.org
 S:	Maintained
 F:	drivers/rtc/rtc-sd2405al.c
 
+DFROBOT SEN0322 DRIVER
+M:	Tóth János <gomba007@gmail.com>
+L:	linux-iio@vger.kernel.org
+S:	Maintained
+F:	drivers/iio/chemical/sen0322.c
+
 DH ELECTRONICS DHSOM SOM AND BOARD SUPPORT
 M:	Christoph Niedermaier <cniedermaier@dh-electronics.com>
 M:	Marek Vasut <marex@denx.de>
diff --git a/drivers/iio/chemical/Kconfig b/drivers/iio/chemical/Kconfig
index 330fe0af946f..60a81863d123 100644
--- a/drivers/iio/chemical/Kconfig
+++ b/drivers/iio/chemical/Kconfig
@@ -166,6 +166,16 @@ config SCD4X
 	  To compile this driver as a module, choose M here: the module will
 	  be called scd4x.
 
+config SEN0322
+	tristate "SEN0322 oxygen sensor"
+	depends on I2C
+	select REGMAP_I2C
+	help
+	  Say Y here to build support for the DFRobot SEN0322 oxygen sensor.
+
+	  To compile this driver as a module, choose M here: the module will
+	  be called sen0322.
+
 config SENSIRION_SGP30
 	tristate "Sensirion SGPxx gas sensors"
 	depends on I2C
diff --git a/drivers/iio/chemical/Makefile b/drivers/iio/chemical/Makefile
index 4866db06bdc9..deeff0e4e6f7 100644
--- a/drivers/iio/chemical/Makefile
+++ b/drivers/iio/chemical/Makefile
@@ -20,6 +20,7 @@ obj-$(CONFIG_SCD30_CORE) += scd30_core.o
 obj-$(CONFIG_SCD30_I2C) += scd30_i2c.o
 obj-$(CONFIG_SCD30_SERIAL) += scd30_serial.o
 obj-$(CONFIG_SCD4X) += scd4x.o
+obj-$(CONFIG_SEN0322)	+= sen0322.o
 obj-$(CONFIG_SENSEAIR_SUNRISE_CO2) += sunrise_co2.o
 obj-$(CONFIG_SENSIRION_SGP30)	+= sgp30.o
 obj-$(CONFIG_SENSIRION_SGP40)	+= sgp40.o
diff --git a/drivers/iio/chemical/sen0322.c b/drivers/iio/chemical/sen0322.c
new file mode 100644
index 000000000000..5f1f4528401e
--- /dev/null
+++ b/drivers/iio/chemical/sen0322.c
@@ -0,0 +1,238 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for the DFRobot SEN0322 oxygen sensor.
+ *
+ * Datasheet:
+ *	https://wiki.dfrobot.com/Gravity_I2C_Oxygen_Sensor_SKU_SEN0322
+ *
+ * Possible I2C slave addresses:
+ *	0x70
+ *	0x71
+ *	0x72
+ *	0x73
+ *
+ * Copyright (C) 2025 Tóth János <gomba007@gmail.com>
+ */
+
+#include <linux/i2c.h>
+#include <linux/math64.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+
+#include <linux/iio/iio.h>
+
+#define DRIVER_NAME "sen0322"
+
+#define SEN0322_REG_DATA	0x03
+#define SEN0322_REG_COEFF	0x0A
+
+#define FIXED_FRAC_BITS		18
+#define FIXED_INT(x)		((fixed_t)((x) << FIXED_FRAC_BITS))
+
+typedef u32 fixed_t;
+
+struct sen0322 {
+	struct i2c_client	*client;
+	struct regmap		*regmap;
+	fixed_t			coeff;
+};
+
+static fixed_t fixed_mul(fixed_t a, fixed_t b)
+{
+	u64 tmp;
+
+	tmp = (u64)a * (u64)b;
+	tmp = (tmp >> FIXED_FRAC_BITS) + ((tmp >> FIXED_FRAC_BITS) & 1);
+
+	if (tmp > U32_MAX)
+		return (fixed_t)U32_MAX;
+	else
+		return (fixed_t)tmp;
+}
+
+static fixed_t fixed_div(fixed_t a, fixed_t b)
+{
+	u64 tmp;
+
+	tmp = (uint64_t)a << FIXED_FRAC_BITS;
+	tmp += (b >> 1);
+
+	return (fixed_t)(div_u64(tmp, b));
+}
+
+static int sen0322_read_coeff(struct sen0322 *sen0322)
+{
+	u32 val;
+	int ret;
+
+	ret = regmap_read(sen0322->regmap, SEN0322_REG_COEFF, &val);
+	if (ret < 0)
+		return ret;
+
+	if (val)
+		sen0322->coeff = fixed_div(FIXED_INT(val), FIXED_INT(1000));
+	else
+		sen0322->coeff = fixed_div(FIXED_INT(209), FIXED_INT(1200));
+
+	dev_dbg(&sen0322->client->dev, "coeff: %08X\n", sen0322->coeff);
+
+	return 0;
+}
+
+static int sen0322_read_data(struct sen0322 *sen0322)
+{
+	u8 data[4] = { 0 };
+	int ret;
+
+	ret = regmap_bulk_read(sen0322->regmap, SEN0322_REG_DATA, data, 3);
+	if (ret < 0)
+		return ret;
+
+	ret = data[0] * 100 +  data[1] * 10 + data[2];
+
+	dev_dbg(&sen0322->client->dev, "raw data: %d\n", ret);
+
+	return ret;
+}
+
+static int sen0322_read_prep_data(struct sen0322 *sen0322)
+{
+	fixed_t val;
+	int ret;
+
+	if (!sen0322->coeff) {
+		ret = sen0322_read_coeff(sen0322);
+		if (ret < 0)
+			return ret;
+	}
+
+	ret = sen0322_read_data(sen0322);
+	if (ret < 0)
+		return ret;
+
+	val = fixed_mul(sen0322->coeff, FIXED_INT(ret));
+
+	dev_dbg(&sen0322->client->dev, "prep data: %08X\n", val);
+
+	return val >> FIXED_FRAC_BITS;
+}
+
+static int sen0322_read_raw(struct iio_dev *iio_dev,
+			    const struct iio_chan_spec *chan,
+			    int *val, int *val2, long mask)
+{
+	struct sen0322 *sen0322 = iio_priv(iio_dev);
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		switch (chan->type) {
+		case IIO_CONCENTRATION:
+			ret = sen0322_read_data(sen0322);
+			if (ret < 0)
+				return ret;
+
+			*val = ret;
+			return IIO_VAL_INT;
+
+		default:
+			return -EINVAL;
+		}
+
+	case IIO_CHAN_INFO_PROCESSED:
+		switch (chan->type) {
+		case IIO_CONCENTRATION:
+			ret = sen0322_read_prep_data(sen0322);
+			if (ret < 0)
+				return ret;
+
+			*val = ret;
+			return IIO_VAL_INT;
+
+		default:
+			return -EINVAL;
+		}
+
+	case IIO_CHAN_INFO_SCALE:
+		switch (chan->type) {
+		case IIO_CONCENTRATION:
+			*val = 1;
+			*val2 = 100;
+			return IIO_VAL_FRACTIONAL;
+
+		default:
+			return -EINVAL;
+		}
+
+	default:
+		return -EINVAL;
+	}
+}
+
+static const struct iio_info sen0322_info = {
+	.read_raw = sen0322_read_raw,
+};
+
+static const struct regmap_config sen0322_regmap_conf = {
+	.reg_bits = 8,
+	.val_bits = 8,
+};
+
+static const struct iio_chan_spec sen0322_channels[] = {
+	{
+		.type = IIO_CONCENTRATION,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+				      BIT(IIO_CHAN_INFO_PROCESSED) |
+				      BIT(IIO_CHAN_INFO_SCALE),
+	},
+};
+
+static int sen0322_probe(struct i2c_client *client)
+{
+	struct sen0322 *sen0322;
+	struct iio_dev *iio_dev;
+
+	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
+		return -ENODEV;
+
+	iio_dev = devm_iio_device_alloc(&client->dev, sizeof(*sen0322));
+	if (!iio_dev)
+		return -ENOMEM;
+
+	sen0322 = iio_priv(iio_dev);
+	sen0322->client = client;
+	sen0322->coeff = 0;
+
+	sen0322->regmap = devm_regmap_init_i2c(client, &sen0322_regmap_conf);
+	if (IS_ERR(sen0322->regmap))
+		return PTR_ERR(sen0322->regmap);
+
+	i2c_set_clientdata(client, sen0322);
+
+	iio_dev->info = &sen0322_info;
+	iio_dev->name = DRIVER_NAME;
+	iio_dev->channels = sen0322_channels;
+	iio_dev->num_channels = ARRAY_SIZE(sen0322_channels);
+	iio_dev->modes = INDIO_DIRECT_MODE;
+
+	return devm_iio_device_register(&client->dev, iio_dev);
+}
+
+static const struct of_device_id sen0322_of_match[] = {
+	{ .compatible = "dfrobot,sen0322" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, sen0322_of_match);
+
+static struct i2c_driver sen0322_driver = {
+	.driver = {
+		.name = DRIVER_NAME,
+		.of_match_table = sen0322_of_match,
+	},
+	.probe = sen0322_probe,
+};
+module_i2c_driver(sen0322_driver);
+
+MODULE_AUTHOR("Tóth János <gomba007@gmail.com>");
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("SEN0322 oxygen sensor driver");

-- 
2.34.1

Re: [PATCH 1/2] dt-bindings: iio: chemical: Document SEN0322

[Krzysztof Kozlowski]

On 28/04/2025 12:50, Tóth János via B4 Relay wrote:
> +
> +description: >
> +  DFRobot SEN0322 is an oxygen sensor. It supports I2C for communication.
> +
> +  Datasheet:
> +    https://wiki.dfrobot.com/Gravity_I2C_Oxygen_Sensor_SKU_SEN0322
> +
> +properties:
> +  compatible:
> +    const: dfrobot,sen0322
> +
> +  reg:
> +    maxItems: 1

No other properties like supplies or configuration? If so, this could go
to trivial-devices.

> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      sen0322@73 {

Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation

Choose something from above or similar devices.



Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: iio: chemical: Document SEN0322

[Tóth János]

Hi!

> No other properties like supplies or configuration? If so, this could go
> to trivial-devices.

I don't think so, I'll add it as a trivial-device then.

> > +      sen0322@73 {
> 
> Node names should be generic. See also an explanation and list of
> examples (not exhaustive) in DT specification:
> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
> 
> Choose something from above or similar devices.

Noted, thanks!

Best regards,
János

Re: [PATCH 1/2] dt-bindings: iio: chemical: Document SEN0322

[Jonathan Cameron]

On Mon, 28 Apr 2025 16:45:32 +0200
Tóth János <gomba007@gmail.com> wrote:

> Hi!
> 
> > No other properties like supplies or configuration? If so, this could go
> > to trivial-devices.  
> 
> I don't think so, I'll add it as a trivial-device then.

vcc-supply?

It is very common to see later boards have controllable supplies
(or someone to enable that on an existing board for power saving) so
good and trivial to support just turning the supply on when we probe
the driver and off when we remove it.


> 
> > > +      sen0322@73 {  
> > 
> > Node names should be generic. See also an explanation and list of
> > examples (not exhaustive) in DT specification:
> > https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
> > 
> > Choose something from above or similar devices.  
> 
> Noted, thanks!
> 
> Best regards,
> János
> 
> 

Re: [PATCH 2/2] iio: chemical: Add driver for SEN0322

[Jonathan Cameron]

On Mon, 28 Apr 2025 12:50:14 +0200
Tóth János via B4 Relay <devnull+gomba007.gmail.com@kernel.org> wrote:

> From: Tóth János <gomba007@gmail.com>
> 
> Add support for the DFRobot SEN0322 oxygen sensor.
> 
> Datasheet:
> 	https://wiki.dfrobot.com/Gravity_I2C_Oxygen_Sensor_SKU_SEN0322
> 
> To instantiate (assuming device is connected to I2C-2):
> 	echo 'sen0322 0x73' > /sys/class/i2c-dev/i2c-2/device/new_device
> 
> To read the oxygen concentration (assuming device is iio:device0):
> 	cat /sys/bus/iio/devices/iio:device0/in_concentration_input
> 
> Signed-off-by: Tóth János <gomba007@gmail.com>

Hi Tóth

Nice little driver.  Main questions are around the userspace ABI and why
we have both _RAW and _PROCESSED reported. There are few reasons we
let drivers do that and I don't see what reason applies here.

Mostly it just confuses userspace by providing multiple ways to read the
same thing.

Jonathan

> diff --git a/drivers/iio/chemical/sen0322.c b/drivers/iio/chemical/sen0322.c
> new file mode 100644
> index 000000000000..5f1f4528401e
> --- /dev/null
> +++ b/drivers/iio/chemical/sen0322.c
> @@ -0,0 +1,238 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Driver for the DFRobot SEN0322 oxygen sensor.
> + *
> + * Datasheet:
> + *	https://wiki.dfrobot.com/Gravity_I2C_Oxygen_Sensor_SKU_SEN0322
> + *
> + * Possible I2C slave addresses:
> + *	0x70
> + *	0x71
> + *	0x72
> + *	0x73
> + *
> + * Copyright (C) 2025 Tóth János <gomba007@gmail.com>
> + */
> +
> +#include <linux/i2c.h>
> +#include <linux/math64.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +
> +#include <linux/iio/iio.h>
> +
> +#define DRIVER_NAME "sen0322"
> +
> +#define SEN0322_REG_DATA	0x03
> +#define SEN0322_REG_COEFF	0x0A
> +
> +#define FIXED_FRAC_BITS		18
> +#define FIXED_INT(x)		((fixed_t)((x) << FIXED_FRAC_BITS))
> +
> +typedef u32 fixed_t;
> +
> +struct sen0322 {
> +	struct i2c_client	*client;
What do you need client for after probe?

There is a function to get the struct device from the regmap.

> +	struct regmap		*regmap;
> +	fixed_t			coeff;
> +};
> +
> +static fixed_t fixed_mul(fixed_t a, fixed_t b)
> +{
> +	u64 tmp;
> +
> +	tmp = (u64)a * (u64)b;
> +	tmp = (tmp >> FIXED_FRAC_BITS) + ((tmp >> FIXED_FRAC_BITS) & 1);

These need some comments.  It's moderately fiddly fixed point maths
and there are many ways to do that.

> +
> +	if (tmp > U32_MAX)
> +		return (fixed_t)U32_MAX;
> +	else
> +		return (fixed_t)tmp;
> +}
> +
> +static fixed_t fixed_div(fixed_t a, fixed_t b)
> +{
> +	u64 tmp;
> +
> +	tmp = (uint64_t)a << FIXED_FRAC_BITS;
> +	tmp += (b >> 1);
> +
> +	return (fixed_t)(div_u64(tmp, b));
> +}
> +
> +static int sen0322_read_coeff(struct sen0322 *sen0322)
> +{
> +	u32 val;
> +	int ret;
> +
> +	ret = regmap_read(sen0322->regmap, SEN0322_REG_COEFF, &val);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (val)
> +		sen0322->coeff = fixed_div(FIXED_INT(val), FIXED_INT(1000));
> +	else
> +		sen0322->coeff = fixed_div(FIXED_INT(209), FIXED_INT(1200));

This second one is just a number. Why not just put the constant here?

> +
> +	dev_dbg(&sen0322->client->dev, "coeff: %08X\n", sen0322->coeff);
> +
> +	return 0;
> +}
> +
> +static int sen0322_read_data(struct sen0322 *sen0322)
> +{
> +	u8 data[4] = { 0 };
> +	int ret;
> +
> +	ret = regmap_bulk_read(sen0322->regmap, SEN0322_REG_DATA, data, 3);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = data[0] * 100 +  data[1] * 10 + data[2];
> +
> +	dev_dbg(&sen0322->client->dev, "raw data: %d\n", ret);
> +
> +	return ret;
> +}
> +
> +static int sen0322_read_prep_data(struct sen0322 *sen0322)
> +{
> +	fixed_t val;
> +	int ret;
> +
> +	if (!sen0322->coeff) {
> +		ret = sen0322_read_coeff(sen0322);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	ret = sen0322_read_data(sen0322);
> +	if (ret < 0)
> +		return ret;
> +
> +	val = fixed_mul(sen0322->coeff, FIXED_INT(ret));
Superficially looks like you could compute a correct _SCALE and
make this maths a userspace problem?

> +
> +	dev_dbg(&sen0322->client->dev, "prep data: %08X\n", val);
> +
> +	return val >> FIXED_FRAC_BITS;
> +}
> +
> +static int sen0322_read_raw(struct iio_dev *iio_dev,
> +			    const struct iio_chan_spec *chan,
> +			    int *val, int *val2, long mask)
> +{
> +	struct sen0322 *sen0322 = iio_priv(iio_dev);
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		switch (chan->type) {
> +		case IIO_CONCENTRATION:

You need a strong reason to provide both _RAW and _PROCESSED.
What was your thinking here? 

As a general rule, if the conversion is linear, then we provide
_RAW and _SCALE. If it's non linear then _PROCESSED.

The _RAW + _SCALE thing is for 2 reasons.
1 - userspace is better at maths as it has floating point easily
    available.
2 - if we ever add buffered capture then _RAW tends to be of a defined
    number of bits whereas processed is more complex.

> +			ret = sen0322_read_data(sen0322);
> +			if (ret < 0)
> +				return ret;
> +
> +			*val = ret;
> +			return IIO_VAL_INT;
> +
> +		default:
> +			return -EINVAL;
> +		}
> +
> +	case IIO_CHAN_INFO_PROCESSED:
> +		switch (chan->type) {
> +		case IIO_CONCENTRATION:
> +			ret = sen0322_read_prep_data(sen0322);
> +			if (ret < 0)
> +				return ret;
> +
> +			*val = ret;
> +			return IIO_VAL_INT;
> +
> +		default:
> +			return -EINVAL;
> +		}
> +
> +	case IIO_CHAN_INFO_SCALE:
> +		switch (chan->type) {
> +		case IIO_CONCENTRATION:
> +			*val = 1;
> +			*val2 = 100;

Given above you use the coeff in the calculation of processed
I don't understand what this scale is indicating.
Scale only applies to _RAW channels.

> +			return IIO_VAL_FRACTIONAL;
> +
> +		default:
> +			return -EINVAL;
> +		}
> +
> +	default:
> +		return -EINVAL;
> +	}
> +}

> +static const struct iio_chan_spec sen0322_channels[] = {
> +	{
> +		.type = IIO_CONCENTRATION,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> +				      BIT(IIO_CHAN_INFO_PROCESSED) |
> +				      BIT(IIO_CHAN_INFO_SCALE),
> +	},
> +};
This doesn't need to be an array. Can just use one structure
and pass the address plus an explicit 1 for the number of channels
below.   Quite a few drivers do it like this though and I don't mind
much.

> +
> +static int sen0322_probe(struct i2c_client *client)
> +{
> +	struct sen0322 *sen0322;
> +	struct iio_dev *iio_dev;
> +
> +	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
> +		return -ENODEV;
> +
> +	iio_dev = devm_iio_device_alloc(&client->dev, sizeof(*sen0322));
> +	if (!iio_dev)
> +		return -ENOMEM;
> +
> +	sen0322 = iio_priv(iio_dev);
> +	sen0322->client = client;
> +	sen0322->coeff = 0;
> +
> +	sen0322->regmap = devm_regmap_init_i2c(client, &sen0322_regmap_conf);
> +	if (IS_ERR(sen0322->regmap))
> +		return PTR_ERR(sen0322->regmap);
> +
> +	i2c_set_clientdata(client, sen0322);

I don't immediately see where this is used. If it's not then drop setting it.

> +
> +	iio_dev->info = &sen0322_info;
> +	iio_dev->name = DRIVER_NAME;

As below. I'd rather see the name here as a string.

> +	iio_dev->channels = sen0322_channels;
> +	iio_dev->num_channels = ARRAY_SIZE(sen0322_channels);
> +	iio_dev->modes = INDIO_DIRECT_MODE;
> +
> +	return devm_iio_device_register(&client->dev, iio_dev);
> +}
> +
> +static const struct of_device_id sen0322_of_match[] = {
> +	{ .compatible = "dfrobot,sen0322" },
> +	{ /* sentinel */ }

No real need for the comment.

> +};
> +MODULE_DEVICE_TABLE(of, sen0322_of_match);
> +
> +static struct i2c_driver sen0322_driver = {
> +	.driver = {

> +		.name = DRIVER_NAME,
I'd rather see the string directly here.  There is no reason
why the iio_dev->name above would always match this so in general
it is easier to just see the strings in each place rather than
under a define.

> +		.of_match_table = sen0322_of_match,
> +	},
> +	.probe = sen0322_probe,
> +};
> +module_i2c_driver(sen0322_driver);
> +
> +MODULE_AUTHOR("Tóth János <gomba007@gmail.com>");
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("SEN0322 oxygen sensor driver");
> 

Re: [PATCH 2/2] iio: chemical: Add driver for SEN0322

[Tóth János]

Hi Jonathan!

Thank you for the review and the explanation! I've misunderstood the purpose of
_SCALE. I'll rewrite the driver to use only _RAW and _SCALE.

Regards,
János

Re: [PATCH 1/2] dt-bindings: iio: chemical: Document SEN0322

[Tóth János]

Hi!

> > > No other properties like supplies or configuration? If so, this could go
> > > to trivial-devices.  
> > 
> > I don't think so, I'll add it as a trivial-device then.
> 
> vcc-supply?

It has no switchable VCC supply.

Regards,
János

Re: [PATCH 1/2] dt-bindings: iio: chemical: Document SEN0322

[Jonathan Cameron]

On Mon, 5 May 2025 08:32:02 +0200
Tóth János <gomba007@gmail.com> wrote:

> Hi!
> 
> > > > No other properties like supplies or configuration? If so, this could go
> > > > to trivial-devices.    
> > > 
> > > I don't think so, I'll add it as a trivial-device then.  
> > 
> > vcc-supply?  
> 
> It has no switchable VCC supply.

Your board may have no switchable vcc-supply. In general someone else's
board with this part in use may well have a switchable vcc-supply.
Ideally the DT binding should support that and the driver should just
turn it on at probe.  A stub / fake regulator will be provided by
the regulator core so there is no need for special handling of boards
that don't have switchable vcc-supply - they will just work.

Jonathan

> 
> Regards,
> János
>