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

[PATCH v2 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>
---
Changes in v2:
- Add SEN0322 to trivial-devices.
- Use _RAW and _SCALE instead of fixed-point math.
- Refactor based on reviewer's suggestions.
- Link to v1: https://lore.kernel.org/r/20250428-iio-chemical-sen0322-v1-0-9b18363ffe42@gmail.com

---
Tóth János (2):
      dt-bindings: trivial-devices: Document SEN0322
      iio: chemical: Add driver for SEN0322

 .../devicetree/bindings/trivial-devices.yaml       |   2 +
 MAINTAINERS                                        |   6 +
 drivers/iio/chemical/Kconfig                       |  10 ++
 drivers/iio/chemical/Makefile                      |   1 +
 drivers/iio/chemical/sen0322.c                     | 167 +++++++++++++++++++++
 5 files changed, 186 insertions(+)
---
base-commit: b4432656b36e5cc1d50a1f2dc15357543add530e
change-id: 20250428-iio-chemical-sen0322-cf8fbbbe7546

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

[PATCH v2 1/2] dt-bindings: trivial-devices: 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>
---
 Documentation/devicetree/bindings/trivial-devices.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/trivial-devices.yaml b/Documentation/devicetree/bindings/trivial-devices.yaml
index 8da408107e55..a25bf2135649 100644
--- a/Documentation/devicetree/bindings/trivial-devices.yaml
+++ b/Documentation/devicetree/bindings/trivial-devices.yaml
@@ -85,6 +85,8 @@ properties:
           - devantech,srf08
             # Devantech SRF10 ultrasonic ranger
           - devantech,srf10
+            # DFRobot SEN0322 oxygen sensor
+          - dfrobot,sen0322
             # DH electronics GmbH on-board CPLD trivial SPI device
           - dh,dhcom-board
             # DA9053: flexible system level PMIC with multicore support

-- 
2.34.1

Re: [PATCH v2 1/2] dt-bindings: trivial-devices: Document SEN0322

[Krzysztof Kozlowski]

On 05/05/2025 09:52, Tóth János via B4 Relay wrote:
> 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>

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH v2 1/2] dt-bindings: trivial-devices: Document SEN0322

[Jonathan Cameron]

On Mon, 5 May 2025 10:16:06 +0200
Krzysztof Kozlowski <krzk@kernel.org> wrote:

> On 05/05/2025 09:52, Tóth János via B4 Relay wrote:
> > 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>  
> 
> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 
> Best regards,
> Krzysztof
> 
Hmm. This is fine I guess, but I'm never sure how pretty much anything
justifies being in trivial-devices.yaml given they should all be full bindings
and almost anything requires a power supply :)

Maybe we should let there be a few standard supplies for the devices
in there in the same way a single interrupt is allowed?

J

[PATCH v2 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 get the oxygen concentration (assuming device is iio:device0) multiply
the values read from:
	/sys/bus/iio/devices/iio:device0/in_concentration_raw
	/sys/bus/iio/devices/iio:device0/in_concentration_scale

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 | 167 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 184 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..c2dfb0ff7f40
--- /dev/null
+++ b/drivers/iio/chemical/sen0322.c
@@ -0,0 +1,167 @@
+// 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/regmap.h>
+
+#include <linux/iio/iio.h>
+
+#define SEN0322_REG_DATA	0x03
+#define SEN0322_REG_COEFF	0x0A
+
+struct sen0322 {
+	struct regmap	*regmap;
+};
+
+static int sen0322_read_scale(struct sen0322 *sen0322, int *num, int *den)
+{
+	u32 val;
+	int ret;
+
+	ret = regmap_read(sen0322->regmap, SEN0322_REG_COEFF, &val);
+	if (ret < 0)
+		return ret;
+
+	if (val) {
+		*num = val;
+		*den = 100000;
+	} else {
+		*num = 209;
+		*den = 120000;
+	}
+
+	dev_dbg(regmap_get_device(sen0322->regmap), "scale: %d/%d\n",
+		*num, *den);
+
+	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(regmap_get_device(sen0322->regmap), "data: %d\n", ret);
+
+	return ret;
+}
+
+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_SCALE:
+		switch (chan->type) {
+		case IIO_CONCENTRATION:
+			ret = sen0322_read_scale(sen0322, val, val2);
+			if (ret < 0)
+				return ret;
+
+			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_channel = {
+	.type = IIO_CONCENTRATION,
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+			      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->regmap = devm_regmap_init_i2c(client, &sen0322_regmap_conf);
+	if (IS_ERR(sen0322->regmap))
+		return PTR_ERR(sen0322->regmap);
+
+	iio_dev->info = &sen0322_info;
+	iio_dev->name = "sen0322";
+	iio_dev->channels = &sen0322_channel;
+	iio_dev->num_channels = 1;
+	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" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, sen0322_of_match);
+
+static struct i2c_driver sen0322_driver = {
+	.driver = {
+		.name = "sen0322",
+		.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 v2 2/2] iio: chemical: Add driver for SEN0322

[Jonathan Cameron]

On Mon, 05 May 2025 09:52:59 +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

Checkpatch is lagging behind the times, but it is fine to use this
as a formal tag in the tag block..
> 
> To instantiate (assuming device is connected to I2C-2):
> 	echo 'sen0322 0x73' > /sys/class/i2c-dev/i2c-2/device/new_device
> 
> To get the oxygen concentration (assuming device is iio:device0) multiply
> the values read from:
> 	/sys/bus/iio/devices/iio:device0/in_concentration_raw
> 	/sys/bus/iio/devices/iio:device0/in_concentration_scale
> 
Datasheet: https://wiki.dfrobot.com/Gravity_I2C_Oxygen_Sensor_SKU_SEN0322

> Signed-off-by: Tóth János <gomba007@gmail.com>
A few other little things inline.

Nice little driver :)

Jonathan

> diff --git a/drivers/iio/chemical/sen0322.c b/drivers/iio/chemical/sen0322.c
> new file mode 100644
> index 000000000000..c2dfb0ff7f40
> --- /dev/null
> +++ b/drivers/iio/chemical/sen0322.c
> @@ -0,0 +1,167 @@
> +// 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/regmap.h>
> +
> +#include <linux/iio/iio.h>
> +
> +#define SEN0322_REG_DATA	0x03
> +#define SEN0322_REG_COEFF	0x0A
> +
> +struct sen0322 {
> +	struct regmap	*regmap;
> +};
> +
> +static int sen0322_read_scale(struct sen0322 *sen0322, int *num, int *den)
> +{
> +	u32 val;
> +	int ret;
> +
> +	ret = regmap_read(sen0322->regmap, SEN0322_REG_COEFF, &val);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (val) {
> +		*num = val;
> +		*den = 100000;
> +	} else {
> +		*num = 209;
> +		*den = 120000;

This is odd enough, that perhaps we could add a comment on why, or at least
a cross reference to where these numbers come from?
What is the special meaning of 0?

> +	}
> +
> +	dev_dbg(regmap_get_device(sen0322->regmap), "scale: %d/%d\n",
> +		*num, *den);
> +
> +	return 0;
> +}
> +
> +static int sen0322_read_data(struct sen0322 *sen0322)
> +{
> +	u8 data[4] = { 0 };

If you are only read 3 bytes, why is this 4 long?

> +	int ret;
> +
> +	ret = regmap_bulk_read(sen0322->regmap, SEN0322_REG_DATA, data, 3);

Having shortened above, use sizeof(data) for that 3 to avoid
any potential future mismatch in sizes.

> +	if (ret < 0)
> +		return ret;
> +
> +	ret = data[0] * 100 + data[1] * 10 + data[2];
> +
> +	dev_dbg(regmap_get_device(sen0322->regmap), "data: %d\n", ret);

Given you more or less directly provide this to userspace now I'd drop
the dev_dbg() as not adding any value for debugging.

> +
> +	return ret;
> +}
> +
> +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:
As the sensor only does concentration, you could either drop this
check on basis we can't get here without it or if you want 
a strong sanity check do it outside the switch statement as
	if (chan->type != IIO_CONCENTRATION)
		return -EINVAL;


> +			ret = sen0322_read_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:
> +			ret = sen0322_read_scale(sen0322, val, val2);
> +			if (ret < 0)
> +				return ret;
> +
> +			return IIO_VAL_FRACTIONAL;
> +
> +		default:
> +			return -EINVAL;
> +		}
> +
> +	default:
> +		return -EINVAL;
> +	}
> +}

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

[Tóth János]

Hi!

Thank you for the review!

> Checkpatch is lagging behind the times, but it is fine to use this
> as a formal tag in the tag block..
> > 
> Datasheet: https://wiki.dfrobot.com/Gravity_I2C_Oxygen_Sensor_SKU_SEN0322
> 
> > Signed-off-by: Tóth János <gomba007@gmail.com>

Sure.

> > +	if (val) {
> > +		*num = val;
> > +		*den = 100000;
> > +	} else {
> > +		*num = 209;
> > +		*den = 120000;
> 
> This is odd enough, that perhaps we could add a comment on why, or at least
> a cross reference to where these numbers come from?
> What is the special meaning of 0?

Okay, I'll add some explanation.

> > +	u8 data[4] = { 0 };
> 
> If you are only read 3 bytes, why is this 4 long?

It is the closest power of 2, to pacify my OCD, but you are right.

> > +	ret = regmap_bulk_read(sen0322->regmap, SEN0322_REG_DATA, data, 3);
> 
> Having shortened above, use sizeof(data) for that 3 to avoid
> any potential future mismatch in sizes.

Agreed.

> > +	dev_dbg(regmap_get_device(sen0322->regmap), "data: %d\n", ret);
> 
> Given you more or less directly provide this to userspace now I'd drop
> the dev_dbg() as not adding any value for debugging.
>

I just like to see if the function actually ran and not reading some buffered 
value stuck somewhere, but okay.

> > +	switch (mask) {
> > +	case IIO_CHAN_INFO_RAW:
> > +		switch (chan->type) {
> > +		case IIO_CONCENTRATION:
> As the sensor only does concentration, you could either drop this
> check on basis we can't get here without it or if you want 
> a strong sanity check do it outside the switch statement as
> 	if (chan->type != IIO_CONCENTRATION)
> 		return -EINVAL;

I did not want to deviate from the pattern, but yes, it will make the code
more readable.

Best regards,
János