linux-kernel-mentees.lists.linuxfoundation.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 1/3] dt-bindings: vendor-prefixes: add aosong
@ 2023-11-15 12:58 Anshul Dalal
  2023-11-15 12:58 ` [PATCH v2 2/3] dt-bindings: trivial-devices: add aosong,ags02ma Anshul Dalal
  2023-11-15 12:58 ` [PATCH v2 3/3] iio: chemical: add support for Aosong AGS02MA Anshul Dalal
  0 siblings, 2 replies; 6+ messages in thread
From: Anshul Dalal @ 2023-11-15 12:58 UTC (permalink / raw)
  To: linux-kernel, linux-iio, devicetree
  Cc: Anshul Dalal, Conor Dooley, Lars-Peter Clausen, Rob Herring,
	Krzysztof Kozlowski, linux-kernel-mentees, Jonathan Cameron

Aosong Electronic Co., LTD. is a supplier for MEMS sensors such as AHT20
temperature and humidity sensor under the brand name Asair

Signed-off-by: Anshul Dalal <anshulusr@gmail.com>
---

Changes for v2:
- Changed vendor prefix from asair to aosong
---
 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 573578db9509..48d4ff635562 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -117,6 +117,8 @@ patternProperties:
     description: Andes Technology Corporation
   "^anvo,.*":
     description: Anvo-Systems Dresden GmbH
+  "^aosong,.*":
+    description: Guangzhou Aosong Electronic Co., Ltd.
   "^apm,.*":
     description: Applied Micro Circuits Corporation (APM)
   "^apple,.*":
-- 
2.42.0


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

* [PATCH v2 2/3] dt-bindings: trivial-devices: add aosong,ags02ma
  2023-11-15 12:58 [PATCH v2 1/3] dt-bindings: vendor-prefixes: add aosong Anshul Dalal
@ 2023-11-15 12:58 ` Anshul Dalal
  2023-11-15 14:38   ` Conor Dooley
  2023-11-15 12:58 ` [PATCH v2 3/3] iio: chemical: add support for Aosong AGS02MA Anshul Dalal
  1 sibling, 1 reply; 6+ messages in thread
From: Anshul Dalal @ 2023-11-15 12:58 UTC (permalink / raw)
  To: linux-kernel, linux-iio, devicetree
  Cc: Anshul Dalal, Conor Dooley, Lars-Peter Clausen, Rob Herring,
	Krzysztof Kozlowski, linux-kernel-mentees, Jonathan Cameron

Add bindings for Aosong AGS02MA TVOC sensor.

The sensor communicates over i2c with the default address 0x1a.
TVOC values can be read in the units of ppb and ug/m^3 at register 0x00.

Datasheet:
  https://asairsensors.com/wp-content/uploads/2021/09/AGS02MA.pdf
Product-Page:
  http://www.aosong.com/m/en/products-33.html

Signed-off-by: Anshul Dalal <anshulusr@gmail.com>
---

Changes for v2:
- Removed device from trivial-devices
- Added standalone binding with vdd-supply property
---
 .../bindings/iio/chemical/aosong,ags02ma.yaml | 48 +++++++++++++++++++
 1 file changed, 48 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/chemical/aosong,ags02ma.yaml

diff --git a/Documentation/devicetree/bindings/iio/chemical/aosong,ags02ma.yaml b/Documentation/devicetree/bindings/iio/chemical/aosong,ags02ma.yaml
new file mode 100644
index 000000000000..4a0278c6318c
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/chemical/aosong,ags02ma.yaml
@@ -0,0 +1,48 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/chemical/aosong,ags02ma.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Aosong AGS02MA VOC Sensor
+
+description: |
+  AGS02MA is an TVOC (Total Volatile Organic Compounds) i2c sensor with default
+  address of 0x1a.
+
+  Datasheet:
+    https://asairsensors.com/wp-content/uploads/2021/09/AGS02MA.pdf
+
+maintainers:
+  - Anshul Dalal <anshulusr@gmail.com>
+
+properties:
+  compatible:
+    enum:
+      - aosong,ags02ma
+      - asair,ags02ma
+
+  reg:
+    maxItems: 1
+
+  vdd-supply: true
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        light-sensor@1a {
+            compatible = "aosong,ags02ma";
+            reg = <0x1a>;
+            vdd-supply = <&vdd_regulator>;
+        };
+    };
-- 
2.42.0


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

* [PATCH v2 3/3] iio: chemical: add support for Aosong AGS02MA
  2023-11-15 12:58 [PATCH v2 1/3] dt-bindings: vendor-prefixes: add aosong Anshul Dalal
  2023-11-15 12:58 ` [PATCH v2 2/3] dt-bindings: trivial-devices: add aosong,ags02ma Anshul Dalal
@ 2023-11-15 12:58 ` Anshul Dalal
  1 sibling, 0 replies; 6+ messages in thread
From: Anshul Dalal @ 2023-11-15 12:58 UTC (permalink / raw)
  To: linux-kernel, linux-iio, devicetree
  Cc: Anshul Dalal, Conor Dooley, Lars-Peter Clausen, Rob Herring,
	Krzysztof Kozlowski, linux-kernel-mentees, Jonathan Cameron

A simple driver for the TVOC (Total Volatile Organic Compounds)
sensor from Aosong: AGS02MA

Steps in reading the VOC sensor value over i2c:
  1. Read 5 bytes from the register `AGS02MA_TVOC_READ_REG` [0x00]
  2. The first 4 bytes are taken as the big endian sensor data with final
     byte being the CRC
  3. The CRC is verified and the value is returned over an
     `IIO_CHAN_INFO_RAW` channel as percents

Tested on Raspberry Pi Zero 2W

Datasheet:
  https://asairsensors.com/wp-content/uploads/2021/09/AGS02MA.pdf
Product-Page:
  http://www.aosong.com/m/en/products-33.html

Signed-off-by: Anshul Dalal <anshulusr@gmail.com>
---

Changes for v2:
- Fixed Kconfig not selecting CRC8 (used to be `select crc8`)
- Changed instances of asair to aosong
- Report raw readings in percents instead of ppb
- Added myself as maintainer for the device binding
---
 MAINTAINERS                    |   7 ++
 drivers/iio/chemical/Kconfig   |  11 +++
 drivers/iio/chemical/Makefile  |   1 +
 drivers/iio/chemical/ags02ma.c | 161 +++++++++++++++++++++++++++++++++
 4 files changed, 180 insertions(+)
 create mode 100644 drivers/iio/chemical/ags02ma.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 81d5fc0bba68..07b82e766dbd 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3028,6 +3028,13 @@ S:	Supported
 W:	http://www.akm.com/
 F:	drivers/iio/magnetometer/ak8974.c
 
+AOSONG AGS02MA TVOC SENSOR DRIVER
+M:	Anshul Dalal <anshulusr@gmail.com>
+L:	linux-iio@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/iio/chemical/aosong,ags02ma.yaml
+F:	drivers/iio/chemical/ags02ma.c
+
 ASC7621 HARDWARE MONITOR DRIVER
 M:	George Joseph <george.joseph@fairview5.com>
 L:	linux-hwmon@vger.kernel.org
diff --git a/drivers/iio/chemical/Kconfig b/drivers/iio/chemical/Kconfig
index c30657e10ee1..02649ab81b3c 100644
--- a/drivers/iio/chemical/Kconfig
+++ b/drivers/iio/chemical/Kconfig
@@ -5,6 +5,17 @@
 
 menu "Chemical Sensors"
 
+config AOSONG_AGS02MA
+	tristate "Aosong AGS02MA TVOC sensor driver"
+	depends on I2C
+	select CRC8
+	help
+	  Say Y here to build support for Aosong AGS02MA TVOC (Total Volatile
+	  Organic Compounds) sensor.
+
+	  To compile this driver as module, choose M here: the module will be
+	  called ags02ma.
+
 config ATLAS_PH_SENSOR
 	tristate "Atlas Scientific OEM SM sensors"
 	depends on I2C
diff --git a/drivers/iio/chemical/Makefile b/drivers/iio/chemical/Makefile
index a11e777a7a00..2f3dee8bb779 100644
--- a/drivers/iio/chemical/Makefile
+++ b/drivers/iio/chemical/Makefile
@@ -4,6 +4,7 @@
 #
 
 # When adding new entries keep the list in alphabetical order
+obj-$(CONFIG_AOSONG_AGS02MA)	+= ags02ma.o
 obj-$(CONFIG_ATLAS_PH_SENSOR)	+= atlas-sensor.o
 obj-$(CONFIG_ATLAS_EZO_SENSOR)	+= atlas-ezo-sensor.o
 obj-$(CONFIG_BME680) += bme680_core.o
diff --git a/drivers/iio/chemical/ags02ma.c b/drivers/iio/chemical/ags02ma.c
new file mode 100644
index 000000000000..e23655ffce69
--- /dev/null
+++ b/drivers/iio/chemical/ags02ma.c
@@ -0,0 +1,161 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2023 Anshul Dalal <anshulusr@gmail.com>
+ *
+ * Driver for Aosong AGS02MA
+ *
+ * Datasheet:
+ *   https://asairsensors.com/wp-content/uploads/2021/09/AGS02MA.pdf
+ * Product Page:
+ *   http://www.aosong.com/m/en/products-33.html
+ *
+ * TODO:
+ *   - Add support for ug/m^3 units of measurement
+ *   - Add support for modifying i2c address
+ */
+
+#include <linux/crc8.h>
+#include <linux/delay.h>
+#include <linux/i2c.h>
+#include <linux/iio/iio.h>
+#include <linux/module.h>
+
+#define AGS02MA_DEVICE_NAME		   "ags02ma"
+
+#define AGS02MA_TVOC_READ_REG		   0x00
+#define AGS02MA_VERSION_REG		   0x11
+
+#define AGS02MA_VERSION_PROCESSING_DELAY   30
+#define AGS02MA_TVOC_READ_PROCESSING_DELAY 1500
+
+#define AGS02MA_CRC8_INIT		   0xff
+#define AGS02MA_CRC8_POLYNOMIAL		   0x31
+#define AGS02MA_PPB_PERCENT_CONVERSION     10000000
+
+DECLARE_CRC8_TABLE(ags02ma_crc8_table);
+
+struct ags02ma_data {
+	struct i2c_client *client;
+};
+
+struct ags02ma_reading {
+	__be32 data;
+	u8 crc;
+} __packed;
+
+static u32 ags02ma_register_read(struct i2c_client *client, u8 reg, u16 delay)
+{
+	u32 ret;
+	u8 crc;
+	struct ags02ma_reading read_buffer;
+
+	ret = i2c_master_send(client, &reg, sizeof(reg));
+	if (ret < 0) {
+		dev_err(&client->dev,
+			"Failed to send data to register 0x%x: %d", reg, ret);
+		return ret;
+	}
+
+	/* Processing Delay, Check Table 7.7 in the datasheet */
+	msleep_interruptible(delay);
+
+	ret = i2c_master_recv(client, (u8 *)&read_buffer, sizeof(read_buffer));
+	if (ret < 0) {
+		dev_err(&client->dev,
+			"Failed to receive from register 0x%x: %d", reg, ret);
+		return ret;
+	}
+	ret = be32_to_cpu(read_buffer.data);
+
+	crc = crc8(ags02ma_crc8_table, (u8 *)&read_buffer.data,
+		   sizeof(read_buffer.data), AGS02MA_CRC8_INIT);
+	if (crc != read_buffer.crc) {
+		dev_err(&client->dev, "CRC error\n");
+		return -EIO;
+	}
+
+	return ret;
+}
+
+static int ags02ma_read_raw(struct iio_dev *iio_device,
+			    struct iio_chan_spec const *chan, int *val,
+			    int *val2, long mask)
+{
+	int ret;
+	struct ags02ma_data *data = iio_priv(iio_device);
+
+	if (mask == IIO_CHAN_INFO_RAW) {
+		/* The sensor reads data as ppb */
+		ret = ags02ma_register_read(data->client, AGS02MA_TVOC_READ_REG,
+					    AGS02MA_TVOC_READ_PROCESSING_DELAY);
+		if (ret < 0)
+			return ret;
+		*val = ret;
+		*val2 = AGS02MA_PPB_PERCENT_CONVERSION;
+		return IIO_VAL_FRACTIONAL;
+	} else {
+		return -EINVAL;
+	}
+}
+
+static const struct iio_info ags02ma_info = {
+	.read_raw = ags02ma_read_raw,
+};
+
+static const struct iio_chan_spec ags02ma_channels[] = {
+	{ .type = IIO_CONCENTRATION,
+	  .channel2 = IIO_MOD_VOC,
+	  .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) }
+};
+
+static int ags02ma_probe(struct i2c_client *client)
+{
+	int ret;
+	struct ags02ma_data *data;
+	struct iio_dev *indio_dev;
+	u32 version;
+
+	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	crc8_populate_msb(ags02ma_crc8_table, AGS02MA_CRC8_POLYNOMIAL);
+
+	ret = ags02ma_register_read(client, AGS02MA_VERSION_REG,
+				    AGS02MA_VERSION_PROCESSING_DELAY);
+	if (ret < 0) {
+		dev_err(&client->dev, "Failed to read device version: %d", ret);
+		return ret;
+	}
+	version = ret;
+	dev_dbg(&client->dev, "Aosong AGS02MA, Version: 0x%x", version);
+
+	data = iio_priv(indio_dev);
+	i2c_set_clientdata(client, indio_dev);
+	data->client = client;
+	indio_dev->info = &ags02ma_info;
+	indio_dev->channels = ags02ma_channels;
+	indio_dev->num_channels = ARRAY_SIZE(ags02ma_channels);
+	indio_dev->name = AGS02MA_DEVICE_NAME;
+
+	return devm_iio_device_register(&client->dev, indio_dev);
+}
+
+static const struct i2c_device_id ags02ma_id_table[] = {
+	{ AGS02MA_DEVICE_NAME, 0 },
+	{ /* Sentinel */ }
+};
+MODULE_DEVICE_TABLE(i2c, ags02ma_id_table);
+
+static struct i2c_driver ags02ma_driver = {
+	.driver = {
+		.name = AGS02MA_DEVICE_NAME,
+	},
+	.id_table = ags02ma_id_table,
+	.probe = ags02ma_probe,
+};
+module_i2c_driver(ags02ma_driver);
+
+MODULE_AUTHOR("Anshul Dalal <anshulusr@gmail.com>");
+MODULE_DESCRIPTION("Aosong AGS02MA TVOC Driver");
+MODULE_LICENSE("GPL");
-- 
2.42.0


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

* Re: [PATCH v2 2/3] dt-bindings: trivial-devices: add aosong,ags02ma
  2023-11-15 12:58 ` [PATCH v2 2/3] dt-bindings: trivial-devices: add aosong,ags02ma Anshul Dalal
@ 2023-11-15 14:38   ` Conor Dooley
  2023-11-15 15:59     ` Anshul Dalal
  0 siblings, 1 reply; 6+ messages in thread
From: Conor Dooley @ 2023-11-15 14:38 UTC (permalink / raw)
  To: Anshul Dalal
  Cc: linux-kernel, linux-iio, devicetree, Conor Dooley,
	Lars-Peter Clausen, Rob Herring, Krzysztof Kozlowski,
	linux-kernel-mentees, Jonathan Cameron

[-- Attachment #1: Type: text/plain, Size: 2642 bytes --]

On Wed, Nov 15, 2023 at 06:28:07PM +0530, Anshul Dalal wrote:
> Add bindings for Aosong AGS02MA TVOC sensor.
> 
> The sensor communicates over i2c with the default address 0x1a.
> TVOC values can be read in the units of ppb and ug/m^3 at register 0x00.
> 
> Datasheet:
>   https://asairsensors.com/wp-content/uploads/2021/09/AGS02MA.pdf
> Product-Page:
>   http://www.aosong.com/m/en/products-33.html
> 
> Signed-off-by: Anshul Dalal <anshulusr@gmail.com>
> ---
> 
> Changes for v2:
> - Removed device from trivial-devices

Your $subject still says "trivial-devices" though, so please fix that in
your next submission.

> - Added standalone binding with vdd-supply property
> ---
>  .../bindings/iio/chemical/aosong,ags02ma.yaml | 48 +++++++++++++++++++
>  1 file changed, 48 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/chemical/aosong,ags02ma.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/chemical/aosong,ags02ma.yaml b/Documentation/devicetree/bindings/iio/chemical/aosong,ags02ma.yaml
> new file mode 100644
> index 000000000000..4a0278c6318c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/chemical/aosong,ags02ma.yaml
> @@ -0,0 +1,48 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/chemical/aosong,ags02ma.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Aosong AGS02MA VOC Sensor
> +
> +description: |
> +  AGS02MA is an TVOC (Total Volatile Organic Compounds) i2c sensor with default
> +  address of 0x1a.
> +
> +  Datasheet:
> +    https://asairsensors.com/wp-content/uploads/2021/09/AGS02MA.pdf
> +
> +maintainers:
> +  - Anshul Dalal <anshulusr@gmail.com>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - aosong,ags02ma
> +      - asair,ags02ma

Why do you have two compatibles for the same device? Please document and
use only one of these. The aoson website says:
"Guangzhou ASAIR Electronic Co., Ltd"
so I suspect "asair" is a more apt vendor prefix.


Thanks,
Conor.

> +
> +  reg:
> +    maxItems: 1
> +
> +  vdd-supply: true
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        light-sensor@1a {
> +            compatible = "aosong,ags02ma";
> +            reg = <0x1a>;
> +            vdd-supply = <&vdd_regulator>;
> +        };
> +    };
> -- 
> 2.42.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

* Re: [PATCH v2 2/3] dt-bindings: trivial-devices: add aosong,ags02ma
  2023-11-15 14:38   ` Conor Dooley
@ 2023-11-15 15:59     ` Anshul Dalal
  2023-11-15 19:40       ` Conor Dooley
  0 siblings, 1 reply; 6+ messages in thread
From: Anshul Dalal @ 2023-11-15 15:59 UTC (permalink / raw)
  To: Conor Dooley
  Cc: linux-kernel, linux-iio, devicetree, Conor Dooley,
	Lars-Peter Clausen, Rob Herring, Krzysztof Kozlowski,
	linux-kernel-mentees, Jonathan Cameron

On 11/15/23 20:08, Conor Dooley wrote:
> On Wed, Nov 15, 2023 at 06:28:07PM +0530, Anshul Dalal wrote:
>> Add bindings for Aosong AGS02MA TVOC sensor.
>>
>> The sensor communicates over i2c with the default address 0x1a.
>> TVOC values can be read in the units of ppb and ug/m^3 at register 0x00.
>>
>> Datasheet:
>>   https://asairsensors.com/wp-content/uploads/2021/09/AGS02MA.pdf
>> Product-Page:
>>   http://www.aosong.com/m/en/products-33.html
>>
>> Signed-off-by: Anshul Dalal <anshulusr@gmail.com>
>> ---
>>
>> Changes for v2:
>> - Removed device from trivial-devices
> 
> Your $subject still says "trivial-devices" though, so please fix that in
> your next submission.
> 
>> - Added standalone binding with vdd-supply property
>> ---
>>  .../bindings/iio/chemical/aosong,ags02ma.yaml | 48 +++++++++++++++++++
>>  1 file changed, 48 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/iio/chemical/aosong,ags02ma.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/iio/chemical/aosong,ags02ma.yaml b/Documentation/devicetree/bindings/iio/chemical/aosong,ags02ma.yaml
>> new file mode 100644
>> index 000000000000..4a0278c6318c
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/iio/chemical/aosong,ags02ma.yaml
>> @@ -0,0 +1,48 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/iio/chemical/aosong,ags02ma.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Aosong AGS02MA VOC Sensor
>> +
>> +description: |
>> +  AGS02MA is an TVOC (Total Volatile Organic Compounds) i2c sensor with default
>> +  address of 0x1a.
>> +
>> +  Datasheet:
>> +    https://asairsensors.com/wp-content/uploads/2021/09/AGS02MA.pdf
>> +
>> +maintainers:
>> +  - Anshul Dalal <anshulusr@gmail.com>
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - aosong,ags02ma
>> +      - asair,ags02ma
> 
> Why do you have two compatibles for the same device? Please document and
> use only one of these. The aoson website says:
> "Guangzhou ASAIR Electronic Co., Ltd"

I'm sorry but I couldn't find any reference to such name on
http://www.aosong.com/en/
In the contact page (http://www.aosong.com/en/article-34.html),
I could only find references to "Guangzhou Aosong Electronic Co., Ltd."

> so I suspect "asair" is a more apt vendor prefix.
> 

Asair is the brand name under which Guangzhou Aosong Electronics sells
their products [0]. After a prior discussion with Krzysztof [1], I
thought it would be the best to go with Aosong instead.

The rationale being since a vendor can only have a single prefix, if
Aosong were to sell some new product under the name Aosong, it would
then have to have the same vendor-prefix as other products under the
Asair brand.

[0]: https://www.linkedin.com/company/asairsensor/
[1]:
https://lore.kernel.org/lkml/1f62ab04-1679-44b1-b95e-e3954afbfa88@linaro.org/
Best regards,
Anshul

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

* Re: [PATCH v2 2/3] dt-bindings: trivial-devices: add aosong,ags02ma
  2023-11-15 15:59     ` Anshul Dalal
@ 2023-11-15 19:40       ` Conor Dooley
  0 siblings, 0 replies; 6+ messages in thread
From: Conor Dooley @ 2023-11-15 19:40 UTC (permalink / raw)
  To: Anshul Dalal
  Cc: linux-kernel, linux-iio, devicetree, Conor Dooley,
	Lars-Peter Clausen, Rob Herring, Krzysztof Kozlowski,
	linux-kernel-mentees, Jonathan Cameron

[-- Attachment #1: Type: text/plain, Size: 3300 bytes --]

On Wed, Nov 15, 2023 at 09:29:20PM +0530, Anshul Dalal wrote:
> On 11/15/23 20:08, Conor Dooley wrote:
> > On Wed, Nov 15, 2023 at 06:28:07PM +0530, Anshul Dalal wrote:
> >> Add bindings for Aosong AGS02MA TVOC sensor.
> >>
> >> The sensor communicates over i2c with the default address 0x1a.
> >> TVOC values can be read in the units of ppb and ug/m^3 at register 0x00.
> >>
> >> Datasheet:
> >>   https://asairsensors.com/wp-content/uploads/2021/09/AGS02MA.pdf
> >> Product-Page:
> >>   http://www.aosong.com/m/en/products-33.html
> >>
> >> Signed-off-by: Anshul Dalal <anshulusr@gmail.com>
> >> ---
> >>
> >> Changes for v2:
> >> - Removed device from trivial-devices
> > 
> > Your $subject still says "trivial-devices" though, so please fix that in
> > your next submission.
> > 
> >> - Added standalone binding with vdd-supply property
> >> ---
> >>  .../bindings/iio/chemical/aosong,ags02ma.yaml | 48 +++++++++++++++++++
> >>  1 file changed, 48 insertions(+)
> >>  create mode 100644 Documentation/devicetree/bindings/iio/chemical/aosong,ags02ma.yaml
> >>
> >> diff --git a/Documentation/devicetree/bindings/iio/chemical/aosong,ags02ma.yaml b/Documentation/devicetree/bindings/iio/chemical/aosong,ags02ma.yaml
> >> new file mode 100644
> >> index 000000000000..4a0278c6318c
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/iio/chemical/aosong,ags02ma.yaml
> >> @@ -0,0 +1,48 @@
> >> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> >> +%YAML 1.2
> >> +---
> >> +$id: http://devicetree.org/schemas/iio/chemical/aosong,ags02ma.yaml#
> >> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >> +
> >> +title: Aosong AGS02MA VOC Sensor
> >> +
> >> +description: |
> >> +  AGS02MA is an TVOC (Total Volatile Organic Compounds) i2c sensor with default
> >> +  address of 0x1a.
> >> +
> >> +  Datasheet:
> >> +    https://asairsensors.com/wp-content/uploads/2021/09/AGS02MA.pdf
> >> +
> >> +maintainers:
> >> +  - Anshul Dalal <anshulusr@gmail.com>
> >> +
> >> +properties:
> >> +  compatible:
> >> +    enum:
> >> +      - aosong,ags02ma
> >> +      - asair,ags02ma
> > 
> > Why do you have two compatibles for the same device? Please document and
> > use only one of these. The aoson website says:
> > "Guangzhou ASAIR Electronic Co., Ltd"
> 
> I'm sorry but I couldn't find any reference to such name on
> http://www.aosong.com/en/
> In the contact page (http://www.aosong.com/en/article-34.html),
> I could only find references to "Guangzhou Aosong Electronic Co., Ltd."

On the page you linked in your original submission:
www.aosong.com/m/en/products-33.html
(scroll down)

> > so I suspect "asair" is a more apt vendor prefix.
> > 
> 
> Asair is the brand name under which Guangzhou Aosong Electronics sells
> their products [0]. After a prior discussion with Krzysztof [1], I
> thought it would be the best to go with Aosong instead.
> 
> The rationale being since a vendor can only have a single prefix, if
> Aosong were to sell some new product under the name Aosong, it would
> then have to have the same vendor-prefix as other products under the
> Asair brand.

Go with Aosong then, sure. But please drop the "asair" prefixed
compatible from your binding.

Cheers,
Conor,

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

end of thread, other threads:[~2023-11-15 19:40 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-15 12:58 [PATCH v2 1/3] dt-bindings: vendor-prefixes: add aosong Anshul Dalal
2023-11-15 12:58 ` [PATCH v2 2/3] dt-bindings: trivial-devices: add aosong,ags02ma Anshul Dalal
2023-11-15 14:38   ` Conor Dooley
2023-11-15 15:59     ` Anshul Dalal
2023-11-15 19:40       ` Conor Dooley
2023-11-15 12:58 ` [PATCH v2 3/3] iio: chemical: add support for Aosong AGS02MA Anshul Dalal

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).