[PATCH 3/3] dt-bindings: w1: ds2482: Add vcc-supply property

Kryštof Černý via B4 Relay posted 3 patches 1 week ago
There is a newer version of this series
[PATCH 3/3] dt-bindings: w1: ds2482: Add vcc-supply property
Posted by Kryštof Černý via B4 Relay 1 week ago
From: Kryštof Černý <cleverline1mc@gmail.com>

Adds the newly added vcc-supply property to bindings.

Signed-off-by: Kryštof Černý <cleverline1mc@gmail.com>
---
 Documentation/devicetree/bindings/w1/maxim,ds2482.yaml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/w1/maxim,ds2482.yaml b/Documentation/devicetree/bindings/w1/maxim,ds2482.yaml
index 422becc6e1fa8d58665c5586ebdc611cd0b2c760..a6b9e0658ec858cb24b21cf64443a061bb43e4ef 100644
--- a/Documentation/devicetree/bindings/w1/maxim,ds2482.yaml
+++ b/Documentation/devicetree/bindings/w1/maxim,ds2482.yaml
@@ -25,6 +25,9 @@ properties:
   reg:
     maxItems: 1
 
+  vcc-supply:
+    description: phandle of the regulator that provides the supply voltage.
+
 required:
   - compatible
   - reg

-- 
2.39.5


Re: [PATCH 3/3] dt-bindings: w1: ds2482: Add vcc-supply property
Posted by Conor Dooley 6 days, 23 hours ago
On Fri, Nov 15, 2024 at 03:58:06PM +0100, Kryštof Černý via B4 Relay wrote:
> From: Kryštof Černý <cleverline1mc@gmail.com>
> 
> Adds the newly added vcc-supply property to bindings.

This commit message is a circular argument. You're adding it to the
binding, which of course means it is newly added.

> 
> Signed-off-by: Kryštof Černý <cleverline1mc@gmail.com>
> ---
>  Documentation/devicetree/bindings/w1/maxim,ds2482.yaml | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/w1/maxim,ds2482.yaml b/Documentation/devicetree/bindings/w1/maxim,ds2482.yaml
> index 422becc6e1fa8d58665c5586ebdc611cd0b2c760..a6b9e0658ec858cb24b21cf64443a061bb43e4ef 100644
> --- a/Documentation/devicetree/bindings/w1/maxim,ds2482.yaml
> +++ b/Documentation/devicetree/bindings/w1/maxim,ds2482.yaml
> @@ -25,6 +25,9 @@ properties:
>    reg:
>      maxItems: 1
>  
> +  vcc-supply:
> +    description: phandle of the regulator that provides the supply voltage.

"vcc-supply: true" should suffice.

> +
>  required:
>    - compatible
>    - reg
> 
> -- 
> 2.39.5
> 
> 
Re: [PATCH 3/3] dt-bindings: w1: ds2482: Add vcc-supply property
Posted by Kryštof Černý 2 days, 8 hours ago
Hello,

> On Fri, Nov 15, 2024 at 03:58:06PM +0100, Kryštof Černý via B4 Relay wrote:
>> From: Kryštof Černý <cleverline1mc@gmail.com>
>>
>> Adds the newly added vcc-supply property to bindings.
> 
> This commit message is a circular argument. You're adding it to the
> binding, which of course means it is newly added.

You are right, I will replace with "Adds the vcc-supply property to 
bindings." in the next version.

>>
>> Signed-off-by: Kryštof Černý <cleverline1mc@gmail.com>
>> ---
>>   Documentation/devicetree/bindings/w1/maxim,ds2482.yaml | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/w1/maxim,ds2482.yaml b/Documentation/devicetree/bindings/w1/maxim,ds2482.yaml
>> index 422becc6e1fa8d58665c5586ebdc611cd0b2c760..a6b9e0658ec858cb24b21cf64443a061bb43e4ef 100644
>> --- a/Documentation/devicetree/bindings/w1/maxim,ds2482.yaml
>> +++ b/Documentation/devicetree/bindings/w1/maxim,ds2482.yaml
>> @@ -25,6 +25,9 @@ properties:
>>     reg:
>>       maxItems: 1
>>   
>> +  vcc-supply:
>> +    description: phandle of the regulator that provides the supply voltage.
> 
> "vcc-supply: true" should suffice.
> 

Right, I suppose you mean to remove the description and just have 
"vcc-supply: true".
If so, could you explain why no description? Is it some standard property
or because the name is self-explanatory? If you mean to keep both, 
please reply.

>> +
>>   required:
>>     - compatible
>>     - reg
>>
>> -- 
>> 2.39.5
>>
>>

Thank you for review,
Kryštof Černý
Re: [PATCH 3/3] dt-bindings: w1: ds2482: Add vcc-supply property
Posted by Krzysztof Kozlowski 2 days, 8 hours ago
On 20/11/2024 09:34, Kryštof Černý wrote:
> Hello,
> 
>> On Fri, Nov 15, 2024 at 03:58:06PM +0100, Kryštof Černý via B4 Relay wrote:
>>> From: Kryštof Černý <cleverline1mc@gmail.com>
>>>
>>> Adds the newly added vcc-supply property to bindings.
>>
>> This commit message is a circular argument. You're adding it to the
>> binding, which of course means it is newly added.
> 
> You are right, I will replace with "Adds the vcc-supply property to 
> bindings." in the next version.

No, please say why, e.g. because it was missing and device has it
according to datasheet.

> 
>>>
>>> Signed-off-by: Kryštof Černý <cleverline1mc@gmail.com>
>>> ---
>>>   Documentation/devicetree/bindings/w1/maxim,ds2482.yaml | 3 +++
>>>   1 file changed, 3 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/w1/maxim,ds2482.yaml b/Documentation/devicetree/bindings/w1/maxim,ds2482.yaml
>>> index 422becc6e1fa8d58665c5586ebdc611cd0b2c760..a6b9e0658ec858cb24b21cf64443a061bb43e4ef 100644
>>> --- a/Documentation/devicetree/bindings/w1/maxim,ds2482.yaml
>>> +++ b/Documentation/devicetree/bindings/w1/maxim,ds2482.yaml
>>> @@ -25,6 +25,9 @@ properties:
>>>     reg:
>>>       maxItems: 1
>>>   
>>> +  vcc-supply:
>>> +    description: phandle of the regulator that provides the supply voltage.
>>
>> "vcc-supply: true" should suffice.
>>
> 
> Right, I suppose you mean to remove the description and just have 
> "vcc-supply: true".
> If so, could you explain why no description? Is it some standard property
> or because the name is self-explanatory? If you mean to keep both, 
> please reply.

It's almost self-explanatory and your description does not give any more
information. git grep for existing code - you will find also examples
which give actual information, e.g. detailed PIN name and accepted voltages.



Best regards,
Krzysztof
Re: [PATCH 3/3] dt-bindings: w1: ds2482: Add vcc-supply property
Posted by Kryštof Černý 1 day, 18 hours ago
Hello,

> On 20/11/2024 09:34, Kryštof Černý wrote:
>> Hello,
>>
>>> On Fri, Nov 15, 2024 at 03:58:06PM +0100, Kryštof Černý via B4 Relay wrote:
>>>> From: Kryštof Černý <cleverline1mc@gmail.com>
>>>>
>>>> Adds the newly added vcc-supply property to bindings.
>>>
>>> This commit message is a circular argument. You're adding it to the
>>> binding, which of course means it is newly added.
>>
>> You are right, I will replace with "Adds the vcc-supply property to
>> bindings." in the next version.
> 
> No, please say why, e.g. because it was missing and device has it
> according to datasheet.

Right, what about:

Adds the optional vcc-supply property to bindings, informs if the device 
needs a regulator to be turned on for its operation.


>>
>>>>
>>>> Signed-off-by: Kryštof Černý <cleverline1mc@gmail.com>
>>>> ---
>>>>    Documentation/devicetree/bindings/w1/maxim,ds2482.yaml | 3 +++
>>>>    1 file changed, 3 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/w1/maxim,ds2482.yaml b/Documentation/devicetree/bindings/w1/maxim,ds2482.yaml
>>>> index 422becc6e1fa8d58665c5586ebdc611cd0b2c760..a6b9e0658ec858cb24b21cf64443a061bb43e4ef 100644
>>>> --- a/Documentation/devicetree/bindings/w1/maxim,ds2482.yaml
>>>> +++ b/Documentation/devicetree/bindings/w1/maxim,ds2482.yaml
>>>> @@ -25,6 +25,9 @@ properties:
>>>>      reg:
>>>>        maxItems: 1
>>>>    
>>>> +  vcc-supply:
>>>> +    description: phandle of the regulator that provides the supply voltage.
>>>
>>> "vcc-supply: true" should suffice.
>>>
>>
>> Right, I suppose you mean to remove the description and just have
>> "vcc-supply: true".
>> If so, could you explain why no description? Is it some standard property
>> or because the name is self-explanatory? If you mean to keep both,
>> please reply.
> 
> It's almost self-explanatory and your description does not give any more
> information. git grep for existing code - you will find also examples
> which give actual information, e.g. detailed PIN name and accepted voltages.
> 

Thanks for the tip, best description I have come up with:

   vcc-supply:
     description:
       Regulator that drives the VCC pin (2.9-5.5 V). Depending on the 
hardware
       design, it may set the vcc voltage of 3 wire 1-Wire bus variant.

Would be glad for feedback if you think it's a good idea to use it, or
just keep "vcc-supply: true".

> 
> Best regards,
> Krzysztof

Thank you,
Kryštof Černý
Re: [PATCH 3/3] dt-bindings: w1: ds2482: Add vcc-supply property
Posted by Krzysztof Kozlowski 1 day, 9 hours ago
On 20/11/2024 23:53, Kryštof Černý wrote:
> Hello,
> 
>> On 20/11/2024 09:34, Kryštof Černý wrote:
>>> Hello,
>>>
>>>> On Fri, Nov 15, 2024 at 03:58:06PM +0100, Kryštof Černý via B4 Relay wrote:
>>>>> From: Kryštof Černý <cleverline1mc@gmail.com>
>>>>>
>>>>> Adds the newly added vcc-supply property to bindings.
>>>>
>>>> This commit message is a circular argument. You're adding it to the
>>>> binding, which of course means it is newly added.
>>>
>>> You are right, I will replace with "Adds the vcc-supply property to
>>> bindings." in the next version.
>>
>> No, please say why, e.g. because it was missing and device has it
>> according to datasheet.
> 
> Right, what about:
> 
> Adds the optional vcc-supply property to bindings, informs if the device 
> needs a regulator to be turned on for its operation

It does not inform at all. All devices needs power, don't they? And what
operation?



Best regards,
Krzysztof
Re: [PATCH 3/3] dt-bindings: w1: ds2482: Add vcc-supply property
Posted by Kryštof Černý 1 day, 1 hour ago

Dne 21.11.2024 v 8:43 Krzysztof Kozlowski napsal(a):
> On 20/11/2024 23:53, Kryštof Černý wrote:
>> Hello,
>>
>>> On 20/11/2024 09:34, Kryštof Černý wrote:
>>>> Hello,
>>>>
>>>>> On Fri, Nov 15, 2024 at 03:58:06PM +0100, Kryštof Černý via B4 Relay wrote:
>>>>>> From: Kryštof Černý <cleverline1mc@gmail.com>
>>>>>>
>>>>>> Adds the newly added vcc-supply property to bindings.
>>>>>
>>>>> This commit message is a circular argument. You're adding it to the
>>>>> binding, which of course means it is newly added.
>>>>
>>>> You are right, I will replace with "Adds the vcc-supply property to
>>>> bindings." in the next version.
>>>
>>> No, please say why, e.g. because it was missing and device has it
>>> according to datasheet.
>>
>> Right, what about:
>>
>> Adds the optional vcc-supply property to bindings, informs if the device
>> needs a regulator to be turned on for its operation
> 
> It does not inform at all. All devices needs power, don't they? And what
> operation?

This is probably the best I can do:

	ds2482 has a VCC pin, accepting 2.9-5.5 V. Allow optionally specifying 
a voltage regulator to power the chip.

If you don't find this satisfactory, please provide a suggestion.

> Best regards,
> Krzysztof

Thank you for your patience,
Kryštof Černý
Re: [PATCH 3/3] dt-bindings: w1: ds2482: Add vcc-supply property
Posted by Krzysztof Kozlowski 1 day, 1 hour ago
On 21/11/2024 16:56, Kryštof Černý wrote:
> 
> 
> Dne 21.11.2024 v 8:43 Krzysztof Kozlowski napsal(a):
>> On 20/11/2024 23:53, Kryštof Černý wrote:
>>> Hello,
>>>
>>>> On 20/11/2024 09:34, Kryštof Černý wrote:
>>>>> Hello,
>>>>>
>>>>>> On Fri, Nov 15, 2024 at 03:58:06PM +0100, Kryštof Černý via B4 Relay wrote:
>>>>>>> From: Kryštof Černý <cleverline1mc@gmail.com>
>>>>>>>
>>>>>>> Adds the newly added vcc-supply property to bindings.
>>>>>>
>>>>>> This commit message is a circular argument. You're adding it to the
>>>>>> binding, which of course means it is newly added.
>>>>>
>>>>> You are right, I will replace with "Adds the vcc-supply property to
>>>>> bindings." in the next version.
>>>>
>>>> No, please say why, e.g. because it was missing and device has it
>>>> according to datasheet.
>>>
>>> Right, what about:
>>>
>>> Adds the optional vcc-supply property to bindings, informs if the device
>>> needs a regulator to be turned on for its operation
>>
>> It does not inform at all. All devices needs power, don't they? And what
>> operation?
> 
> This is probably the best I can do:
> 
> 	ds2482 has a VCC pin, accepting 2.9-5.5 V. Allow optionally specifying 
> a voltage regulator to power the chip.

Keep only first sentence. Second is obvious, no need to repeat schema in
free form text (and schema defines it is optional).

Best regards,
Krzysztof