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

[Krzysztof Kozlowski]

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