devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 0/4] add support for winsen MHZ19B CO2 sensor
@ 2025-04-22 15:52 Gyeyoung Baek
  2025-04-22 15:52 ` [PATCH v5 1/4] dt-bindings: add winsen to the vendor prefixes Gyeyoung Baek
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Gyeyoung Baek @ 2025-04-22 15:52 UTC (permalink / raw)
  To: jic23
  Cc: Gyeyoung Baek, linux-iio, devicetree, dlechner, nuno.sa, andy,
	robh, krzk+dt, conor+dt

v5:
 - Include the required headers explicitly.
 - Fix coding style overall.

v4:
 - Ensure buffer is aligned to the cacheline.
 - Fix coding style overall.

v3:
 - Add vin supply regulator.
 - Drop custom ABI.
 - Drop unnecessary mutex.

v2:
 - Add ABI doc.
 - Add complete struct to receive UART transmission successfully.
 - Add undersigned as a maintainer for the WINSEN MHZ19B.
 - Modify to comply with the IIO subsystem ABI as much as possible.
 - Revise the coding style overall.

Gyeyoung Baek (4):
  dt-bindings: add winsen to the vendor prefixes
  dt-bindings: add device tree support for winsen MHZ19B CO2 sensor
  iio: chemical: add support for winsen MHZ19B CO2 sensor
  MAINTAINERS: Add WINSEN MHZ19B

 .../bindings/iio/chemical/winsen,mhz19b.yaml  |  33 ++
 .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
 MAINTAINERS                                   |   6 +
 drivers/iio/chemical/Kconfig                  |  10 +
 drivers/iio/chemical/Makefile                 |   1 +
 drivers/iio/chemical/mhz19b.c                 | 311 ++++++++++++++++++
 6 files changed, 363 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/chemical/winsen,mhz19b.yaml
 create mode 100644 drivers/iio/chemical/mhz19b.c

-- 
2.34.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v5 1/4] dt-bindings: add winsen to the vendor prefixes
  2025-04-22 15:52 [PATCH v5 0/4] add support for winsen MHZ19B CO2 sensor Gyeyoung Baek
@ 2025-04-22 15:52 ` Gyeyoung Baek
  2025-04-22 15:53 ` [PATCH v5 2/4] dt-bindings: add device tree support for winsen MHZ19B CO2 sensor Gyeyoung Baek
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Gyeyoung Baek @ 2025-04-22 15:52 UTC (permalink / raw)
  To: jic23
  Cc: Gyeyoung Baek, linux-iio, devicetree, dlechner, nuno.sa, andy,
	robh, krzk+dt, conor+dt, Krzysztof Kozlowski

Add winsen to the vendor prefixes.

Signed-off-by: Gyeyoung Baek <gye976@gmail.com>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index 86f6a19b28ae..6d35549d2e4b 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -1689,6 +1689,8 @@ patternProperties:
     description: Wingtech Technology Co., Ltd.
   "^winlink,.*":
     description: WinLink Co., Ltd
+  "^winsen,.*":
+    description: Winsen Corp.
   "^winstar,.*":
     description: Winstar Display Corp.
   "^wirelesstag,.*":
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v5 2/4] dt-bindings: add device tree support for winsen MHZ19B CO2 sensor
  2025-04-22 15:52 [PATCH v5 0/4] add support for winsen MHZ19B CO2 sensor Gyeyoung Baek
  2025-04-22 15:52 ` [PATCH v5 1/4] dt-bindings: add winsen to the vendor prefixes Gyeyoung Baek
@ 2025-04-22 15:53 ` Gyeyoung Baek
  2025-04-22 15:53 ` [PATCH v5 3/4] iio: chemical: add " Gyeyoung Baek
  2025-04-22 15:53 ` [PATCH v5 4/4] MAINTAINERS: Add WINSEN MHZ19B Gyeyoung Baek
  3 siblings, 0 replies; 7+ messages in thread
From: Gyeyoung Baek @ 2025-04-22 15:53 UTC (permalink / raw)
  To: jic23
  Cc: Gyeyoung Baek, linux-iio, devicetree, dlechner, nuno.sa, andy,
	robh, krzk+dt, conor+dt, Krzysztof Kozlowski

Add device tree support for winsen MHZ19B sensor.

Signed-off-by: Gyeyoung Baek <gye976@gmail.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 .../bindings/iio/chemical/winsen,mhz19b.yaml  | 33 +++++++++++++++++++
 1 file changed, 33 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/chemical/winsen,mhz19b.yaml

diff --git a/Documentation/devicetree/bindings/iio/chemical/winsen,mhz19b.yaml b/Documentation/devicetree/bindings/iio/chemical/winsen,mhz19b.yaml
new file mode 100644
index 000000000000..2a6ddb33f163
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/chemical/winsen,mhz19b.yaml
@@ -0,0 +1,33 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/chemical/winsen,mhz19b.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MHZ19B CO2 sensor
+
+maintainers:
+  - Gyeyoung Baek <gye976@gmail.com>
+
+properties:
+  compatible:
+    const: winsen,mhz19b
+
+  vin-supply:
+    description: Regulator that provides power to the sensor
+
+required:
+  - compatible
+  - vin-supply
+
+additionalProperties: false
+
+examples:
+  - |
+    serial {
+      co2-sensor {
+        compatible = "winsen,mhz19b";
+        vin-supply = <&vdd>;
+      };
+    };
+...
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v5 3/4] iio: chemical: add support for winsen MHZ19B CO2 sensor
  2025-04-22 15:52 [PATCH v5 0/4] add support for winsen MHZ19B CO2 sensor Gyeyoung Baek
  2025-04-22 15:52 ` [PATCH v5 1/4] dt-bindings: add winsen to the vendor prefixes Gyeyoung Baek
  2025-04-22 15:53 ` [PATCH v5 2/4] dt-bindings: add device tree support for winsen MHZ19B CO2 sensor Gyeyoung Baek
@ 2025-04-22 15:53 ` Gyeyoung Baek
  2025-04-22 17:39   ` Andy Shevchenko
  2025-04-22 15:53 ` [PATCH v5 4/4] MAINTAINERS: Add WINSEN MHZ19B Gyeyoung Baek
  3 siblings, 1 reply; 7+ messages in thread
From: Gyeyoung Baek @ 2025-04-22 15:53 UTC (permalink / raw)
  To: jic23
  Cc: Gyeyoung Baek, linux-iio, devicetree, dlechner, nuno.sa, andy,
	robh, krzk+dt, conor+dt

Add support for winsen MHZ19B CO2 sensor.

Datasheet: https://www.winsen-sensor.com/d/files/infrared-gas-sensor/mh-z19b-co2-ver1_0.pdf
Signed-off-by: Gyeyoung Baek <gye976@gmail.com>
---
 drivers/iio/chemical/Kconfig  |  10 ++
 drivers/iio/chemical/Makefile |   1 +
 drivers/iio/chemical/mhz19b.c | 311 ++++++++++++++++++++++++++++++++++
 3 files changed, 322 insertions(+)
 create mode 100644 drivers/iio/chemical/mhz19b.c

diff --git a/drivers/iio/chemical/Kconfig b/drivers/iio/chemical/Kconfig
index 330fe0af946f..7742de3f9cdb 100644
--- a/drivers/iio/chemical/Kconfig
+++ b/drivers/iio/chemical/Kconfig
@@ -108,6 +108,16 @@ config IAQCORE
 	  iAQ-Core Continuous/Pulsed VOC (Volatile Organic Compounds)
 	  sensors
 
+config MHZ19B
+	tristate "Winsen MHZ19B CO2 sensor"
+	depends on SERIAL_DEV_BUS
+	help
+	  Say Y here to build Serdev interface support for the Winsen
+	  MHZ19B CO2 sensor.
+
+	  To compile this driver as a module, choose M here: the module will
+	  be called mhz19b.
+
 config PMS7003
 	tristate "Plantower PMS7003 particulate matter sensor"
 	depends on SERIAL_DEV_BUS
diff --git a/drivers/iio/chemical/Makefile b/drivers/iio/chemical/Makefile
index 4866db06bdc9..c63daebf39ac 100644
--- a/drivers/iio/chemical/Makefile
+++ b/drivers/iio/chemical/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_ENS160) += ens160_core.o
 obj-$(CONFIG_ENS160_I2C) += ens160_i2c.o
 obj-$(CONFIG_ENS160_SPI) += ens160_spi.o
 obj-$(CONFIG_IAQCORE)		+= ams-iaq-core.o
+obj-$(CONFIG_MHZ19B) += mhz19b.o
 obj-$(CONFIG_PMS7003) += pms7003.o
 obj-$(CONFIG_SCD30_CORE) += scd30_core.o
 obj-$(CONFIG_SCD30_I2C) += scd30_i2c.o
diff --git a/drivers/iio/chemical/mhz19b.c b/drivers/iio/chemical/mhz19b.c
new file mode 100644
index 000000000000..61dc94c0dd06
--- /dev/null
+++ b/drivers/iio/chemical/mhz19b.c
@@ -0,0 +1,311 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * mh-z19b CO₂ sensor driver
+ *
+ * Copyright (c) 2025 Gyeyoung Baek <gye976@gmail.com>
+ *
+ * Datasheet:
+ * https://www.winsen-sensor.com/d/files/infrared-gas-sensor/mh-z19b-co2-ver1_0.pdf
+ */
+
+#include <linux/array_size.h>
+#include <linux/completion.h>
+#include <linux/device.h>
+#include <linux/errno.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/jiffies.h>
+#include <linux/kstrtox.h>
+#include <linux/minmax.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/regulator/consumer.h>
+#include <linux/serdev.h>
+#include <linux/string.h>
+#include <linux/types.h>
+#include <linux/unaligned.h>
+
+/*
+ * Commands have following format:
+ *
+ * +------+------+-----+------+------+------+------+------+-------+
+ * | 0xFF | 0x01 | cmd | arg0 | arg1 | 0x00 | 0x00 | 0x00 | cksum |
+ * +------+------+-----+------+------+------+------+------+-------+
+ */
+#define MHZ19B_CMD_SIZE 9
+
+/* ABC logic in MHZ19B means auto calibration. */
+#define MHZ19B_ABC_LOGIC_CMD		0x79
+#define MHZ19B_READ_CO2_CMD		0x86
+#define MHZ19B_SPAN_POINT_CMD		0x88
+#define MHZ19B_ZERO_POINT_CMD		0x87
+
+#define MHZ19B_SERDEV_TIMEOUT msecs_to_jiffies(100)
+
+struct mhz19b_state {
+	struct serdev_device *serdev;
+
+	/* Must wait until the 'buf' is filled with 9 bytes.*/
+	struct completion buf_ready;
+
+	u8 buf_idx;
+	/*
+	 * Serdev receive buffer.
+	 * When data is received from the MH-Z19B,
+	 * the 'mhz19b_receive_buf' callback function is called and fills this buffer.
+	 */
+	u8 buf[MHZ19B_CMD_SIZE] __aligned(IIO_DMA_MINALIGN);
+};
+
+static u8 mhz19b_get_checksum(u8 *cmd_buf)
+{
+	u8 i, checksum = 0;
+
+/*
+ * +------+------+-----+------+------+------+------+------+-------+
+ * | 0xFF | 0x01 | cmd | arg0 | arg1 | 0x00 | 0x00 | 0x00 | cksum |
+ * +------+------+-----+------+------+------+------+------+-------+
+ *	     i:1    2      3      4      5      6      7
+ *
+ *  Sum all cmd_buf elements from index 1 to 7.
+ */
+	for (i = 1; i < 8; i++)
+		checksum += cmd_buf[i];
+
+	return -checksum;
+}
+
+static int mhz19b_serdev_cmd(struct iio_dev *indio_dev, int cmd, u16 arg)
+{
+	struct mhz19b_state *st = iio_priv(indio_dev);
+	struct serdev_device *serdev = st->serdev;
+	struct device *dev = &indio_dev->dev;
+	int ret;
+
+	/*
+	 * cmd_buf[3,4] : arg0,1
+	 * cmd_buf[8]	: checksum
+	 */
+	u8 cmd_buf[MHZ19B_CMD_SIZE] = {
+		0xFF, 0x01, cmd,
+	};
+
+	switch (cmd) {
+	case MHZ19B_ABC_LOGIC_CMD:
+		cmd_buf[3] = arg ? 0xA0 : 0;
+		break;
+	case MHZ19B_SPAN_POINT_CMD:
+		put_unaligned_be16(arg, &cmd_buf[3]);
+		break;
+	default:
+		break;
+	}
+	cmd_buf[8] = mhz19b_get_checksum(cmd_buf);
+
+	/* Write buf to uart ctrl synchronously */
+	ret = serdev_device_write(serdev, cmd_buf, MHZ19B_CMD_SIZE, 0);
+	if (ret < 0)
+		return ret;
+	if (ret != MHZ19B_CMD_SIZE)
+		return -EIO;
+
+	switch (cmd) {
+	case MHZ19B_READ_CO2_CMD:
+		ret = wait_for_completion_interruptible_timeout(&st->buf_ready,
+			MHZ19B_SERDEV_TIMEOUT);
+		if (ret < 0)
+			return ret;
+		if (!ret)
+			return -ETIMEDOUT;
+
+		if (st->buf[8] != mhz19b_get_checksum(st->buf)) {
+			dev_err(dev, "checksum err");
+			return -EINVAL;
+		}
+
+		return get_unaligned_be16(&st->buf[2]);
+	default:
+		/* No response commands. */
+		return 0;
+	}
+}
+
+static int mhz19b_read_raw(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *chan,
+			   int *val, int *val2, long mask)
+{
+	int ret;
+
+	ret = mhz19b_serdev_cmd(indio_dev, MHZ19B_READ_CO2_CMD, 0);
+	if (ret < 0)
+		return ret;
+
+	*val = ret;
+	return IIO_VAL_INT;
+}
+
+/*
+ * echo 0 > calibration_auto_enable : ABC logic off
+ * echo 1 > calibration_auto_enable : ABC logic on
+ */
+static ssize_t calibration_auto_enable_store(struct device *dev,
+					     struct device_attribute *attr,
+					     const char *buf, size_t len)
+{
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	bool enable;
+	int ret;
+
+	ret = kstrtobool(buf, &enable);
+	if (ret)
+		return ret;
+
+	ret = mhz19b_serdev_cmd(indio_dev, MHZ19B_ABC_LOGIC_CMD, enable);
+	if (ret < 0)
+		return ret;
+
+	return len;
+}
+static IIO_DEVICE_ATTR_WO(calibration_auto_enable, 0);
+
+/*
+ * echo 0 > calibration_forced_value		 : zero point calibration
+ *	(make sure the sensor has been working under 400ppm for over 20 minutes.)
+ * echo [1000 1 5000] > calibration_forced_value : span point calibration
+ *	(make sure the sensor has been working under a certain level CO₂ for over 20 minutes.)
+ */
+static ssize_t calibration_forced_value_store(struct device *dev,
+					      struct device_attribute *attr,
+					      const char *buf, size_t len)
+{
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	u16 ppm;
+	int cmd, ret;
+
+	ret = kstrtou16(buf, 0, &ppm);
+	if (ret)
+		return ret;
+
+	if (ppm) {
+		if (!in_range(ppm, 1000, 4001)) {
+			dev_dbg(&indio_dev->dev,
+				"span point ppm should be between 1000 and 5000 inclusive.");
+			return -EINVAL;
+		}
+
+		cmd = MHZ19B_SPAN_POINT_CMD;
+	} else {
+		cmd = MHZ19B_ZERO_POINT_CMD;
+	}
+
+	ret = mhz19b_serdev_cmd(indio_dev, cmd, ppm);
+	if (ret < 0)
+		return ret;
+
+	return len;
+}
+static IIO_DEVICE_ATTR_WO(calibration_forced_value, 0);
+
+static struct attribute *mhz19b_attrs[] = {
+	&iio_dev_attr_calibration_auto_enable.dev_attr.attr,
+	&iio_dev_attr_calibration_forced_value.dev_attr.attr,
+	NULL
+};
+
+static const struct attribute_group mhz19b_attr_group = {
+	.attrs = mhz19b_attrs,
+};
+
+static const struct iio_info mhz19b_info = {
+	.attrs = &mhz19b_attr_group,
+	.read_raw = mhz19b_read_raw,
+};
+
+static const struct iio_chan_spec mhz19b_channels[] = {
+	{
+		.type = IIO_CONCENTRATION,
+		.channel2 = IIO_MOD_CO2,
+		.modified = 1,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+	},
+};
+
+static size_t mhz19b_receive_buf(struct serdev_device *serdev,
+			      const u8 *data, size_t len)
+{
+	struct iio_dev *indio_dev = dev_get_drvdata(&serdev->dev);
+	struct mhz19b_state *st = iio_priv(indio_dev);
+
+	memcpy(st->buf + st->buf_idx, data, len);
+	st->buf_idx += len;
+
+	if (st->buf_idx == MHZ19B_CMD_SIZE) {
+		st->buf_idx = 0;
+		complete(&st->buf_ready);
+	}
+
+	return len;
+}
+
+static const struct serdev_device_ops mhz19b_ops = {
+	.receive_buf = mhz19b_receive_buf,
+	.write_wakeup = serdev_device_write_wakeup,
+};
+
+static int mhz19b_probe(struct serdev_device *serdev)
+{
+	int ret;
+	struct device *dev = &serdev->dev;
+	struct iio_dev *indio_dev;
+	struct mhz19b_state *st;
+
+	serdev_device_set_client_ops(serdev, &mhz19b_ops);
+	ret = devm_serdev_device_open(dev, serdev);
+	if (ret)
+		return ret;
+	serdev_device_set_baudrate(serdev, 9600);
+	serdev_device_set_flow_control(serdev, false);
+	ret = serdev_device_set_parity(serdev, SERDEV_PARITY_NONE);
+	if (ret)
+		return ret;
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
+	if (!indio_dev)
+		return ret;
+	serdev_device_set_drvdata(serdev, indio_dev);
+
+	st = iio_priv(indio_dev);
+	st->serdev = serdev;
+
+	init_completion(&st->buf_ready);
+
+	ret = devm_regulator_get_enable(dev, "vin");
+	if (ret)
+		return ret;
+
+	indio_dev->name = "mh-z19b";
+	indio_dev->channels = mhz19b_channels;
+	indio_dev->num_channels = ARRAY_SIZE(mhz19b_channels);
+	indio_dev->info = &mhz19b_info;
+
+	return devm_iio_device_register(dev, indio_dev);
+}
+
+static const struct of_device_id mhz19b_of_match[] = {
+	{ .compatible = "winsen,mhz19b", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, mhz19b_of_match);
+
+static struct serdev_device_driver mhz19b_driver = {
+	.driver = {
+		.name = "mhz19b",
+		.of_match_table = mhz19b_of_match,
+	},
+	.probe = mhz19b_probe,
+};
+module_serdev_device_driver(mhz19b_driver);
+
+MODULE_AUTHOR("Gyeyoung Baek");
+MODULE_DESCRIPTION("MH-Z19B CO2 sensor driver using serdev interface");
+MODULE_LICENSE("GPL");
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v5 4/4] MAINTAINERS: Add WINSEN MHZ19B
  2025-04-22 15:52 [PATCH v5 0/4] add support for winsen MHZ19B CO2 sensor Gyeyoung Baek
                   ` (2 preceding siblings ...)
  2025-04-22 15:53 ` [PATCH v5 3/4] iio: chemical: add " Gyeyoung Baek
@ 2025-04-22 15:53 ` Gyeyoung Baek
  3 siblings, 0 replies; 7+ messages in thread
From: Gyeyoung Baek @ 2025-04-22 15:53 UTC (permalink / raw)
  To: jic23
  Cc: Gyeyoung Baek, linux-iio, devicetree, dlechner, nuno.sa, andy,
	robh, krzk+dt, conor+dt

Add undersigned as a maintainer for the WINSEN MHZ19B.

Signed-off-by: Gyeyoung Baek <gye976@gmail.com>
---
 MAINTAINERS | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 01079a189c93..4a0089db6670 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -26015,6 +26015,12 @@ M:	David Härdeman <david@hardeman.nu>
 S:	Maintained
 F:	drivers/media/rc/winbond-cir.c
 
+WINSEN MHZ19B
+M:	Gyeyoung Baek <gye976@gmail.com>
+S:	Maintained
+F:	Documentation/devicetree/bindings/iio/chemical/winsen,mhz19b.yaml
+F:	drivers/iio/chemical/mhz19b.c
+
 WINSYSTEMS EBC-C384 WATCHDOG DRIVER
 L:	linux-watchdog@vger.kernel.org
 S:	Orphan
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v5 3/4] iio: chemical: add support for winsen MHZ19B CO2 sensor
  2025-04-22 15:53 ` [PATCH v5 3/4] iio: chemical: add " Gyeyoung Baek
@ 2025-04-22 17:39   ` Andy Shevchenko
  2025-04-23 15:59     ` Gyeyoung Baek
  0 siblings, 1 reply; 7+ messages in thread
From: Andy Shevchenko @ 2025-04-22 17:39 UTC (permalink / raw)
  To: Gyeyoung Baek
  Cc: jic23, linux-iio, devicetree, dlechner, nuno.sa, robh, krzk+dt,
	conor+dt

On Wed, Apr 23, 2025 at 12:53:01AM +0900, Gyeyoung Baek wrote:
> Add support for winsen MHZ19B CO2 sensor.

Winsen (name capitalisation)?

...

> +static ssize_t calibration_forced_value_store(struct device *dev,
> +					      struct device_attribute *attr,
> +					      const char *buf, size_t len)
> +{
> +	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> +	u16 ppm;
> +	int cmd, ret;
> +
> +	ret = kstrtou16(buf, 0, &ppm);
> +	if (ret)
> +		return ret;
> +
> +	if (ppm) {
> +		if (!in_range(ppm, 1000, 4001)) {
> +			dev_dbg(&indio_dev->dev,
> +				"span point ppm should be between 1000 and 5000 inclusive.");
> +			return -EINVAL;
> +		}

I proposed to define the _MIN and _MAX constants for the range and use them in
the parameters. Any objection?

> +		cmd = MHZ19B_SPAN_POINT_CMD;
> +	} else {
> +		cmd = MHZ19B_ZERO_POINT_CMD;
> +	}
> +
> +	ret = mhz19b_serdev_cmd(indio_dev, cmd, ppm);
> +	if (ret < 0)
> +		return ret;
> +
> +	return len;
> +}

Otherwise LGTM, thanks!

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v5 3/4] iio: chemical: add support for winsen MHZ19B CO2 sensor
  2025-04-22 17:39   ` Andy Shevchenko
@ 2025-04-23 15:59     ` Gyeyoung Baek
  0 siblings, 0 replies; 7+ messages in thread
From: Gyeyoung Baek @ 2025-04-23 15:59 UTC (permalink / raw)
  To: andy
  Cc: Gyeyoung Baek, linux-iio, devicetree, dlechner, nuno.sa, jic23,
	robh, krzk+dt, conor+dt

> > +	if (ppm) {
> > +		if (!in_range(ppm, 1000, 4001)) {
> > +			dev_dbg(&indio_dev->dev,
> > +				"span point ppm should be between 1000 and 5000 inclusive.");
> > +			return -EINVAL;
> > +		}
>
> I proposed to define the _MIN and _MAX constants for the range and use them in
> the parameters. Any objection?

I'm sorry, my Gmail didn’t show your replies for some reason, so I missed your previous review. 
I only noticed it after checking iio lore directly.

It seems more readable to use _MIN and _MAX. I'll use those, thanks!

---

> > +
> > +	return len;
> > +}
> 
> Otherwise LGTM, thanks!

I'll send new patch based on your other review comments.
thanks.

--
Regards,
Gyeyoung

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2025-04-23 16:00 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-22 15:52 [PATCH v5 0/4] add support for winsen MHZ19B CO2 sensor Gyeyoung Baek
2025-04-22 15:52 ` [PATCH v5 1/4] dt-bindings: add winsen to the vendor prefixes Gyeyoung Baek
2025-04-22 15:53 ` [PATCH v5 2/4] dt-bindings: add device tree support for winsen MHZ19B CO2 sensor Gyeyoung Baek
2025-04-22 15:53 ` [PATCH v5 3/4] iio: chemical: add " Gyeyoung Baek
2025-04-22 17:39   ` Andy Shevchenko
2025-04-23 15:59     ` Gyeyoung Baek
2025-04-22 15:53 ` [PATCH v5 4/4] MAINTAINERS: Add WINSEN MHZ19B Gyeyoung Baek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).