RE: [PATCH 4/6] dt-bindings: power: supply: add MAX77659 charger binding

[Arslanbenzer, Zeynep]

>On 17/04/2023 23:43, Arslanbenzer, Zeynep wrote:
>>> On 20/12/2022 14:22, Zeynep Arslanbenzer wrote:
>>>> This patch adds device tree binding documentation for MAX77659 charger.
>>>
>>> Do not use "This commit/patch".
>>> https://urldefense.com/v3/__https://elixir.bootlin.com/linux/v5.17.1/
>>> source/Documentation/process/submitting-patches.rst*L95__;>Iw!!A3Ni8C
>>> S0y2Y!-AnHmIThbB5Q88_dFdveEmsXlsfflRXDabf6RVE5ySRusMxP24NEfAr8RCsu26L
>>> TvoaIIMvEDr2YxDECrGlMwR8-ywvoR62rXB0W$
>>>
>>>>
>>>> Signed-off-by: Nurettin Bolucu <Nurettin.Bolucu@xxxxxxxxxx>>
>>>> Signed-off-by: Zeynep Arslanbenzer <Zeynep.Arslanbenzer@xxxxxxxxxx>>
>>>> ---
>>>>  .../power/supply/adi,max77659-charger.yaml    | 42 +++++++++++++++++++
>>>>  MAINTAINERS                                   |  1 +
>>>>  2 files changed, 43 insertions(+)
>>>>  create mode 100644
>>>> Documentation/devicetree/bindings/power/supply/adi,max77659-charger.
>>>> ya
>>>> ml
>>>>
>>>> diff --git
>>>> a/Documentation/devicetree/bindings/power/supply/adi,max77659-charger.
>>>> yaml
>>>> b/Documentation/devicetree/bindings/power/supply/adi,max77659-charger.
>>>> yaml
>>>> new file mode 100644
>>>> index 000000000000..24f8b5a2bd84
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/power/supply/adi,max77659-ch
>>>> +++ ar
>>>> +++ ger.yaml
>>>> @@ -0,0 +1,42 @@
>>>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause %YAML 1.2
>>>> +---
>>>> +$id: 
>>>> +https://urldefense.com/v3/__http://devicetree.org/schemas/power/sup
>>>> +pl 
>>>> +y/adi,max77659-charger.yaml*__;Iw!!A3Ni8CS0y2Y!-AnHmIThbB5Q88_dFdve
>>>> +Em 
>>>> +sXlsfflRXDabf6RVE5ySRusMxP24NEfAr8RCsu26LTvoaIIMvEDr2YxDECrGlMwR8-y
>>>> +wv
>>>> +oR5rKUR11$
>>>> +$schema: 
>>>> +https://urldefense.com/v3/__http://devicetree.org/meta-schemas/core
>>>> +.y 
>>>> +aml*__;Iw!!A3Ni8CS0y2Y!-AnHmIThbB5Q88_dFdveEmsXlsfflRXDabf6RVE5ySRu
>>>> +sM xP24NEfAr8RCsu26LTvoaIIMvEDr2YxDECrGlMwR8-ywvoRy_yWWBS$
>>>> +
>>>> +title: Battery charger for MAX77659 PMIC from ADI.
>>>
>>> Drop full stop.
>>>
>>>> +
>>>> +maintainers:
>>>> +  - Nurettin Bolucu <Nurettin.Bolucu@xxxxxxxxxx>>
>>>> +  - Zeynep Arslanbenzer <Zeynep.Arslanbenzer@xxxxxxxxxx>>
>>>> +
>>>> +description: |
>>>> +  This module is part of the MAX77659 MFD device. For more details
>>>> +  see Documentation/devicetree/bindings/mfd/adi,max77659.yaml.
>>>> +
>>>> +  The charger is represented as a sub-node of the PMIC node on the device tree.
>>>> +
>>>> +properties:
>>>> +  compatible:
>>>> +    const: adi,max77659-charger
>>>> +
>>>> +  reg:
>>>> +    maxItems: 1
>>>> +
>>>> +  adi,fast-charge-timer:
>>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>>> +    description: Fast-charge safety timer value (in hours).
>>>
>>> No, use suffixes for common units.
>> 
>> Hi Krzysztof,
>> 
>> Thank you for your review. The possible register values of the fast charge safety timer are described in the datasheet as follows. I was undecided about using the common unit, second, as it may affect the comprehensibility of the code. Of course, I can use second as the common unit if you think it's more appropriate.
>
>This is quite common property, so I do not understand why this one driver should have it written differently. I understand that parsing
>0/1/2/3 is easier for the machine than 0/3/5/7 but it is not easier for humans.

I referenced property-units.yaml for common units.

As I understood hours and minutes are not common units in Linux for time so I cannot use them as suffixes. Therefore, I thought I had to use "seconds" instead of "hours" or "minutes". I am totally fine if I can use " adi,fast-charge-timer-hours" and "adi,topoff-timer-minutes". 

>
>> 
>> 0b00 = Timer disabled
>> 0b01 = 3 hours
>> 0b10 = 5 hours
>> 0b11 = 7 hours
>> 
>>>
>>>> +
>>>> +  adi,fast-charge-microamp:
>>>> +    description: Fast-charge constant current value.
>>>
>>>> +
>>>> +  adi,topoff-timer:
>>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>>> +    description: Top-Off timer value (in minutes).
>>>
>>> No, use suffixes for common units.
>> 
>> Similar case:
>> 
>> 0b000 = 0 minutes 
>> 0b001 = 5 minutes 
>> 0b010 = 10 minutes 
>> 0b011 = 15 minutes 
>> 0b100 = 20 minutes 
>> 0b101 = 25 minutes 
>> 0b110 = 30 minutes 
>> 0b111 = 35 minutes
>
>Come on, this is easy to parse. Divide by 5 and where is the code
>complexity?
>
>Best regards,
>Krzysztof
>