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