diff mbox series

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

Message ID 20231107173100.62715-2-anshulusr@gmail.com (mailing list archive)
State Changes Requested
Headers show
Series [1/3] dt-bindings: vendor-prefixes: add asair | expand

Commit Message

Anshul Dalal Nov. 7, 2023, 5:30 p.m. UTC
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(+)

Comments

Krzysztof Kozlowski Nov. 7, 2023, 5:47 p.m. UTC | #1
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
Anshul Dalal Nov. 8, 2023, 11:54 a.m. UTC | #2
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
Krzysztof Kozlowski Nov. 8, 2023, 12:01 p.m. UTC | #3
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
Anshul Dalal Nov. 8, 2023, 12:15 p.m. UTC | #4
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
Krzysztof Kozlowski Nov. 8, 2023, 12:29 p.m. UTC | #5
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
Anshul Dalal Nov. 8, 2023, 12:53 p.m. UTC | #6
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
Krzysztof Kozlowski Nov. 9, 2023, 8:43 a.m. UTC | #7
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
diff mbox series

Patch

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)