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@xxxxxxxxx>
>>>>>> ---
>>>>>>  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