[PATCH 1/3] dt-bindings: vendor-prefixes: add asair

[PATCH 1/3] dt-bindings: vendor-prefixes: add asair

[Anshul Dalal]

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

Signed-off-by: Anshul Dalal <anshulusr@gmail.com>
---
 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..df3204f9dda6 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -143,6 +143,8 @@ patternProperties:
     description: Artesyn Embedded Technologies Inc.
   "^asahi-kasei,.*":
     description: Asahi Kasei Corp.
+  "^asair,.*":
+    description: Aosong Electronic Co., Ltd.
   "^asc,.*":
     description: All Sensors Corporation
   "^asix,.*":
-- 
2.42.0

[PATCH 2/3] dt-bindings: trivial-devices: add asair,ags02ma

[Anshul Dalal]

Add bindings for Asair AGS02MA TVOC sensor to trivial devices.

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>
---
 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 cd58179ae337..9cd67b758a88 100644
--- a/Documentation/devicetree/bindings/trivial-devices.yaml
+++ b/Documentation/devicetree/bindings/trivial-devices.yaml
@@ -47,6 +47,8 @@ properties:
           - adi,lt7182s
             # AMS iAQ-Core VOC Sensor
           - ams,iaq-core
+            # TVOC (Total Volatile Organic Compounds) i2c sensor
+          - asair,ags02ma
             # i2c serial eeprom (24cxx)
           - at,24c08
             # i2c trusted platform module (TPM)
-- 
2.42.0

[PATCH 3/3] iio: chemical: add support for Asair AGS02MA

[Anshul Dalal]

A simple driver for the TVOC (Total Volatile Organic Compounds)
sensor from Asair: 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

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>
---
 MAINTAINERS                    |   6 ++
 drivers/iio/chemical/Kconfig   |  11 +++
 drivers/iio/chemical/Makefile  |   1 +
 drivers/iio/chemical/ags02ma.c | 156 +++++++++++++++++++++++++++++++++
 4 files changed, 174 insertions(+)
 create mode 100644 drivers/iio/chemical/ags02ma.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 81d5fc0bba68..3592d83543b5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3028,6 +3028,12 @@ S:	Supported
 W:	http://www.akm.com/
 F:	drivers/iio/magnetometer/ak8974.c
 
+ASAIR AGS02MA TVOC SENSOR DRIVER
+M:	Anshul Dalal <anshulusr@gmail.com>
+L:	linux-iio@vger.kernel.org
+S:	Maintained
+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..58c57c15abdb 100644
--- a/drivers/iio/chemical/Kconfig
+++ b/drivers/iio/chemical/Kconfig
@@ -5,6 +5,17 @@
 
 menu "Chemical Sensors"
 
+config ASAIR_AGS02MA
+	tristate "Asair AGS02MA TVOC sensor driver"
+	depends on I2C
+	select crc8
+	help
+	  Say Y here to build support for Asair 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..885009f6115f 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_ASAIR_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..6590219c076e
--- /dev/null
+++ b/drivers/iio/chemical/ags02ma.c
@@ -0,0 +1,156 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2023 Anshul Dalal <anshulusr@gmail.com>
+ *
+ * Driver for Asair 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
+
+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) {
+		ret = ags02ma_register_read(data->client, AGS02MA_TVOC_READ_REG,
+					    AGS02MA_TVOC_READ_PROCESSING_DELAY);
+		if (ret < 0)
+			return ret;
+		*val = ret;
+		return IIO_VAL_INT;
+	} 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, "Asair 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("Asair AGS02MA TVOC Driver");
+MODULE_LICENSE("GPL");
-- 
2.42.0

Re: [PATCH 1/3] dt-bindings: vendor-prefixes: add asair

[Krzysztof Kozlowski]

On 07/11/2023 18:30, Anshul Dalal wrote:
> Aosong Electronic Co., LTD. is a supplier for MEMS sensors such as AHT20
> temperature and humidity sensor under the name Asair

The name of the company is Aosong Electronic or Asair? What does Asair
stands for? Sensors name? Then it is not a vendor prefix.

> 
> Signed-off-by: Anshul Dalal <anshulusr@gmail.com>
> ---
>  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..df3204f9dda6 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> @@ -143,6 +143,8 @@ patternProperties:
>      description: Artesyn Embedded Technologies Inc.
>    "^asahi-kasei,.*":
>      description: Asahi Kasei Corp.
> +  "^asair,.*":
> +    description: Aosong Electronic Co., Ltd.
>    "^asc,.*":
>      description: All Sensors Corporation
>    "^asix,.*":

Best regards,
Krzysztof

Re: [PATCH 2/3] dt-bindings: trivial-devices: add asair,ags02ma

[Krzysztof Kozlowski]

On 07/11/2023 18:30, Anshul Dalal wrote:
> Add bindings for Asair AGS02MA TVOC sensor to trivial devices.
> 
> 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>
> ---
>  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 cd58179ae337..9cd67b758a88 100644
> --- a/Documentation/devicetree/bindings/trivial-devices.yaml
> +++ b/Documentation/devicetree/bindings/trivial-devices.yaml
> @@ -47,6 +47,8 @@ properties:
>            - adi,lt7182s
>              # AMS iAQ-Core VOC Sensor
>            - ams,iaq-core
> +            # TVOC (Total Volatile Organic Compounds) i2c sensor
> +          - asair,ags02ma

I think you miss VDD supply.

Best regards,
Krzysztof

Re: [PATCH 1/3] dt-bindings: vendor-prefixes: add asair

[Anshul Dalal]

On 11/7/23 23:16, Krzysztof Kozlowski wrote:
> On 07/11/2023 18:30, Anshul Dalal wrote:
>> Aosong Electronic Co., LTD. is a supplier for MEMS sensors such as AHT20
>> temperature and humidity sensor under the name Asair
> 
> The name of the company is Aosong Electronic or Asair? What does Asair
> stands for? Sensors name? Then it is not a vendor prefix.
> 

Asair is the brand name under which Aosong Electronics sells their
products. I thought Asair would be the better choice since
all their products use that as the manufacturer name instead of Aosong
as well as all the retail suppliers of their sensors.

>>
>> Signed-off-by: Anshul Dalal <anshulusr@gmail.com>
>> ---
>>  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..df3204f9dda6 100644
>> --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
>> +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
>> @@ -143,6 +143,8 @@ patternProperties:
>>      description: Artesyn Embedded Technologies Inc.
>>    "^asahi-kasei,.*":
>>      description: Asahi Kasei Corp.
>> +  "^asair,.*":
>> +    description: Aosong Electronic Co., Ltd.
>>    "^asc,.*":
>>      description: All Sensors Corporation
>>    "^asix,.*":
> 
> Best regards,
> Krzysztof
> 

Re: [PATCH 1/3] dt-bindings: vendor-prefixes: add asair

[Krzysztof Kozlowski]

On 08/11/2023 01:32, Anshul Dalal wrote:
> On 11/7/23 23:16, Krzysztof Kozlowski wrote:
>> On 07/11/2023 18:30, Anshul Dalal wrote:
>>> Aosong Electronic Co., LTD. is a supplier for MEMS sensors such as AHT20
>>> temperature and humidity sensor under the name Asair
>>
>> The name of the company is Aosong Electronic or Asair? What does Asair
>> stands for? Sensors name? Then it is not a vendor prefix.
>>
> 
> Asair is the brand name under which Aosong Electronics sells their
> products. I thought Asair would be the better choice since
> all their products use that as the manufacturer name instead of Aosong
> as well as all the retail suppliers of their sensors.

What if Aosong sells new product, e.g. power supply, under name Aosong.
Would you create new prefix? One vendor should have one prefix, so in
that case all other products not sold under Asair should be with "asair".

Best regards,
Krzysztof

Re: [PATCH 2/3] dt-bindings: trivial-devices: add asair,ags02ma

[Anshul Dalal]

Hello Krzysztof,

On 11/7/23 23:17, Krzysztof Kozlowski wrote:
> On 07/11/2023 18:30, Anshul Dalal wrote:
>> Add bindings for Asair AGS02MA TVOC sensor to trivial devices.
>>
>> 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>
>> ---
>>  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 cd58179ae337..9cd67b758a88 100644
>> --- a/Documentation/devicetree/bindings/trivial-devices.yaml
>> +++ b/Documentation/devicetree/bindings/trivial-devices.yaml
>> @@ -47,6 +47,8 @@ properties:
>>            - adi,lt7182s
>>              # AMS iAQ-Core VOC Sensor
>>            - ams,iaq-core
>> +            # TVOC (Total Volatile Organic Compounds) i2c sensor
>> +          - asair,ags02ma
> 
> I think you miss VDD supply.

I am sorry but I'm not sure what you meant. Are you referring to the
addition of some information in the commit description?

Best regards,
Anshul

Re: [PATCH 2/3] dt-bindings: trivial-devices: add asair,ags02ma

[Krzysztof Kozlowski]

On 08/11/2023 12:54, Anshul Dalal wrote:
> 
> Hello Krzysztof,
> 
> On 11/7/23 23:17, Krzysztof Kozlowski wrote:
>> On 07/11/2023 18:30, Anshul Dalal wrote:
>>> Add bindings for Asair AGS02MA TVOC sensor to trivial devices.
>>>
>>> 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>
>>> ---
>>>  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 cd58179ae337..9cd67b758a88 100644
>>> --- a/Documentation/devicetree/bindings/trivial-devices.yaml
>>> +++ b/Documentation/devicetree/bindings/trivial-devices.yaml
>>> @@ -47,6 +47,8 @@ properties:
>>>            - adi,lt7182s
>>>              # AMS iAQ-Core VOC Sensor
>>>            - ams,iaq-core
>>> +            # TVOC (Total Volatile Organic Compounds) i2c sensor
>>> +          - asair,ags02ma
>>
>> I think you miss VDD supply.
> 
> I am sorry but I'm not sure what you meant. Are you referring to the
> addition of some information in the commit description?

I meant that your device might not be trivial. Your device takes VDD
supply, which is now not described in the bindings. Do you want to say
that VDD supply in all possible designs is hard-wired to
non-controllable regulator supply?

Best regards,
Krzysztof

Re: [PATCH 2/3] dt-bindings: trivial-devices: add asair,ags02ma

[Anshul Dalal]

On 11/8/23 17:31, Krzysztof Kozlowski wrote:
> On 08/11/2023 12:54, Anshul Dalal wrote:
>>
>> Hello Krzysztof,
>>
>> On 11/7/23 23:17, Krzysztof Kozlowski wrote:
>>> On 07/11/2023 18:30, Anshul Dalal wrote:
>>>> Add bindings for Asair AGS02MA TVOC sensor to trivial devices.
>>>>
>>>> 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>
>>>> ---
>>>>  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 cd58179ae337..9cd67b758a88 100644
>>>> --- a/Documentation/devicetree/bindings/trivial-devices.yaml
>>>> +++ b/Documentation/devicetree/bindings/trivial-devices.yaml
>>>> @@ -47,6 +47,8 @@ properties:
>>>>            - adi,lt7182s
>>>>              # AMS iAQ-Core VOC Sensor
>>>>            - ams,iaq-core
>>>> +            # TVOC (Total Volatile Organic Compounds) i2c sensor
>>>> +          - asair,ags02ma
>>>
>>> I think you miss VDD supply.
>>
>> I am sorry but I'm not sure what you meant. Are you referring to the
>> addition of some information in the commit description?
> 
> I meant that your device might not be trivial. Your device takes VDD
> supply, which is now not described in the bindings. Do you want to say
> that VDD supply in all possible designs is hard-wired to
> non-controllable regulator supply?

I can't speak for all possible designs but for testing this driver I had
just connected the VDD pin to 5V out of the Raspberry Pi. I have since
verified 3.3V to also work.
Could you explain why `vdd-supply` is a property or point me to further
sources. Wouldn't almost all devices have a VDD/VCC pin for power in?

Best Regards,
Anshul

Re: [PATCH 2/3] dt-bindings: trivial-devices: add asair,ags02ma

[Krzysztof Kozlowski]

On 08/11/2023 13:15, Anshul Dalal wrote:
> On 11/8/23 17:31, Krzysztof Kozlowski wrote:
>> On 08/11/2023 12:54, Anshul Dalal wrote:
>>>
>>> Hello Krzysztof,
>>>
>>> On 11/7/23 23:17, Krzysztof Kozlowski wrote:
>>>> On 07/11/2023 18:30, Anshul Dalal wrote:
>>>>> Add bindings for Asair AGS02MA TVOC sensor to trivial devices.
>>>>>
>>>>> 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>
>>>>> ---
>>>>>  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 cd58179ae337..9cd67b758a88 100644
>>>>> --- a/Documentation/devicetree/bindings/trivial-devices.yaml
>>>>> +++ b/Documentation/devicetree/bindings/trivial-devices.yaml
>>>>> @@ -47,6 +47,8 @@ properties:
>>>>>            - adi,lt7182s
>>>>>              # AMS iAQ-Core VOC Sensor
>>>>>            - ams,iaq-core
>>>>> +            # TVOC (Total Volatile Organic Compounds) i2c sensor
>>>>> +          - asair,ags02ma
>>>>
>>>> I think you miss VDD supply.
>>>
>>> I am sorry but I'm not sure what you meant. Are you referring to the
>>> addition of some information in the commit description?
>>
>> I meant that your device might not be trivial. Your device takes VDD
>> supply, which is now not described in the bindings. Do you want to say
>> that VDD supply in all possible designs is hard-wired to
>> non-controllable regulator supply?
> 
> I can't speak for all possible designs but for testing this driver I had
> just connected the VDD pin to 5V out of the Raspberry Pi. I have since
> verified 3.3V to also work.
> Could you explain why `vdd-supply` is a property or point me to further
> sources. Wouldn't almost all devices have a VDD/VCC pin for power in?

Most of the devices have such pin. For most of the devices we include it
in the bindings.

git grep regulator_get -- drivers/iio/
git grep vdd -- drivers/iio/

If you do not describe it in the bindings, then your device will have to
be supplied by always-on regulators (and marking controllable regulator
as always-on because of this is not correct).

If you are unsure and you just work on some sensor not used for final
product, I think it's fine as is, so to add the regulator later if ever
needed.

Best regards,
Krzysztof

Re: [PATCH 2/3] dt-bindings: trivial-devices: add asair,ags02ma

[Anshul Dalal]

On 11/8/23 17:59, Krzysztof Kozlowski wrote:
> On 08/11/2023 13:15, Anshul Dalal wrote:
>> On 11/8/23 17:31, Krzysztof Kozlowski wrote:
>>> On 08/11/2023 12:54, Anshul Dalal wrote:
>>>>
>>>> Hello Krzysztof,
>>>>
>>>> On 11/7/23 23:17, Krzysztof Kozlowski wrote:
>>>>> On 07/11/2023 18:30, Anshul Dalal wrote:
>>>>>> Add bindings for Asair AGS02MA TVOC sensor to trivial devices.
>>>>>>
>>>>>> 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>
>>>>>> ---
>>>>>>  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 cd58179ae337..9cd67b758a88 100644
>>>>>> --- a/Documentation/devicetree/bindings/trivial-devices.yaml
>>>>>> +++ b/Documentation/devicetree/bindings/trivial-devices.yaml
>>>>>> @@ -47,6 +47,8 @@ properties:
>>>>>>            - adi,lt7182s
>>>>>>              # AMS iAQ-Core VOC Sensor
>>>>>>            - ams,iaq-core
>>>>>> +            # TVOC (Total Volatile Organic Compounds) i2c sensor
>>>>>> +          - asair,ags02ma
>>>>>
>>>>> I think you miss VDD supply.
>>>>
>>>> I am sorry but I'm not sure what you meant. Are you referring to the
>>>> addition of some information in the commit description?
>>>
>>> I meant that your device might not be trivial. Your device takes VDD
>>> supply, which is now not described in the bindings. Do you want to say
>>> that VDD supply in all possible designs is hard-wired to
>>> non-controllable regulator supply?
>>
>> I can't speak for all possible designs but for testing this driver I had
>> just connected the VDD pin to 5V out of the Raspberry Pi. I have since
>> verified 3.3V to also work.
>> Could you explain why `vdd-supply` is a property or point me to further
>> sources. Wouldn't almost all devices have a VDD/VCC pin for power in?
> 
> Most of the devices have such pin. For most of the devices we include it
> in the bindings.
> 
> git grep regulator_get -- drivers/iio/
> git grep vdd -- drivers/iio/
> 
> If you do not describe it in the bindings, then your device will have to
> be supplied by always-on regulators (and marking controllable regulator
> as always-on because of this is not correct).
>
Thanks for the explanation Krzysztof. If I understand correctly, having
an vdd-supply in the binding indicates that the device may be powered by
a controllable power source instead of one that is always on.

> If you are unsure and you just work on some sensor not used for final
> product, I think it's fine as is, so to add the regulator later if ever
> needed.

No problem, I can add the required properties in the next patch version.
I need to make a few changes to the driver anyways.

Just out of curiosity, the sensor supports reassigning of the i2c
address by writing to the 0x21 register from the default address of
0x1a. Is there some way to represent this in the binding.
For future reference, is there some exhaustive list or specification
document for all the allowed properties.

Thanks for the help,
Anshul

Re: [PATCH 2/3] dt-bindings: trivial-devices: add asair,ags02ma

[Krzysztof Kozlowski]

On 08/11/2023 13:53, Anshul Dalal wrote:
>> If you are unsure and you just work on some sensor not used for final
>> product, I think it's fine as is, so to add the regulator later if ever
>> needed.
> 
> No problem, I can add the required properties in the next patch version.
> I need to make a few changes to the driver anyways.
> 
> Just out of curiosity, the sensor supports reassigning of the i2c
> address by writing to the 0x21 register from the default address of
> 0x1a. 

You can list allowed addresses, but we rarely do it.

> Is there some way to represent this in the binding.
> For future reference, is there some exhaustive list or specification
> document for all the allowed properties.

Any property could be allowed so the is no list of them.

dt-extract-props -v Documentation/devicetree/bindings/

Best regards,
Krzysztof