1. Patchew
  2. linux
  3. [PATCH 0/6] drivers: mfd: Add MAX77659 MFD and related device drivers
  4. View patch

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

Zeynep Arslanbenzer posted 6 patches 3 years, 3 months ago
[PATCH 4/6] dt-bindings: power: supply: add MAX77659 charger binding
Posted by Zeynep Arslanbenzer 3 years, 3 months ago 
This patch adds device tree binding documentation for MAX77659 charger.

Signed-off-by: Nurettin Bolucu <Nurettin.Bolucu@analog.com>
Signed-off-by: Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>
---
 .../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.yaml

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-charger.yaml
@@ -0,0 +1,42 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/power/supply/adi,max77659-charger.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Battery charger for MAX77659 PMIC from ADI.
+
+maintainers:
+  - Nurettin Bolucu <Nurettin.Bolucu@analog.com>
+  - Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>
+
+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).
+
+  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).
+
+required:
+  - compatible
+
+additionalProperties: false
+
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index b3e307163063..5cb8fa452f2d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12646,6 +12646,7 @@ M:	Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>
 L:	linux-kernel@vger.kernel.org
 S:	Maintained
 F:	Documentation/devicetree/bindings/mfd/adi,max77659.yaml
+F:	Documentation/devicetree/bindings/power/supply/adi,max77659-charger.yaml
 F:	drivers/mfd/max77659.c
 F:	drivers/power/supply/max77659-charger.c
 F:	include/linux/mfd/max77659.h
-- 
2.25.1
Re: [PATCH 4/6] dt-bindings: power: supply: add MAX77659 charger binding
Posted by Krzysztof Kozlowski 3 years, 3 months ago 
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://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95

> 
> Signed-off-by: Nurettin Bolucu <Nurettin.Bolucu@analog.com>
> Signed-off-by: Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>
> ---
>  .../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.yaml
> 
> 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-charger.yaml
> @@ -0,0 +1,42 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/power/supply/adi,max77659-charger.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Battery charger for MAX77659 PMIC from ADI.

Drop full stop.

> +
> +maintainers:
> +  - Nurettin Bolucu <Nurettin.Bolucu@analog.com>
> +  - Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>
> +
> +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.

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


Best regards,
Krzysztof
RE: [PATCH 4/6] dt-bindings: power: supply: add MAX77659 charger binding
Posted by Arslanbenzer, Zeynep 2 years, 11 months ago 
>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!!A3Ni8CS0y2Y!-AnHmIThbB5Q88_dFdveEmsXlsfflRXDabf6RVE5ySRusMxP24NEfAr8RCsu26LTvoaIIMvEDr2YxDECrGlMwR8-ywvoR62rXB0W$ 
>
>> 
>> Signed-off-by: Nurettin Bolucu <Nurettin.Bolucu@analog.com>>
>> Signed-off-by: Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>>
>> ---
>>  .../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-char
>> +++ 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/suppl
>> +y/adi,max77659-charger.yaml*__;Iw!!A3Ni8CS0y2Y!-AnHmIThbB5Q88_dFdveEm
>> +sXlsfflRXDabf6RVE5ySRusMxP24NEfAr8RCsu26LTvoaIIMvEDr2YxDECrGlMwR8-ywv
>> +oR5rKUR11$
>> +$schema: 
>> +https://urldefense.com/v3/__http://devicetree.org/meta-schemas/core.y
>> +aml*__;Iw!!A3Ni8CS0y2Y!-AnHmIThbB5Q88_dFdveEmsXlsfflRXDabf6RVE5ySRusM
>> +xP24NEfAr8RCsu26LTvoaIIMvEDr2YxDECrGlMwR8-ywvoRy_yWWBS$
>> +
>> +title: Battery charger for MAX77659 PMIC from ADI.
>
>Drop full stop.
>
>> +
>> +maintainers:
>> +  - Nurettin Bolucu <Nurettin.Bolucu@analog.com>>
>> +  - Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>>
>> +
>> +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.

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

>
>
>Best regards,
>Krzysztof

Best regards,
Zeynep
Re: [PATCH 4/6] dt-bindings: power: supply: add MAX77659 charger binding
Posted by Krzysztof Kozlowski 2 years, 11 months ago 
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!!A3Ni8CS0y2Y!-AnHmIThbB5Q88_dFdveEmsXlsfflRXDabf6RVE5ySRusMxP24NEfAr8RCsu26LTvoaIIMvEDr2YxDECrGlMwR8-ywvoR62rXB0W$ 
>>
>>>
>>> Signed-off-by: Nurettin Bolucu <Nurettin.Bolucu@analog.com>>
>>> Signed-off-by: Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>>
>>> ---
>>>  .../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-char
>>> +++ 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/suppl
>>> +y/adi,max77659-charger.yaml*__;Iw!!A3Ni8CS0y2Y!-AnHmIThbB5Q88_dFdveEm
>>> +sXlsfflRXDabf6RVE5ySRusMxP24NEfAr8RCsu26LTvoaIIMvEDr2YxDECrGlMwR8-ywv
>>> +oR5rKUR11$
>>> +$schema: 
>>> +https://urldefense.com/v3/__http://devicetree.org/meta-schemas/core.y
>>> +aml*__;Iw!!A3Ni8CS0y2Y!-AnHmIThbB5Q88_dFdveEmsXlsfflRXDabf6RVE5ySRusM
>>> +xP24NEfAr8RCsu26LTvoaIIMvEDr2YxDECrGlMwR8-ywvoRy_yWWBS$
>>> +
>>> +title: Battery charger for MAX77659 PMIC from ADI.
>>
>> Drop full stop.
>>
>>> +
>>> +maintainers:
>>> +  - Nurettin Bolucu <Nurettin.Bolucu@analog.com>>
>>> +  - Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>>
>>> +
>>> +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.

> 
> 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
RE: [PATCH 4/6] dt-bindings: power: supply: add MAX77659 charger binding
Posted by Arslanbenzer, Zeynep 2 years, 11 months ago 
>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@analog.com>>
>>>> Signed-off-by: Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>>
>>>> ---
>>>>  .../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@analog.com>>
>>>> +  - Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>>
>>>> +
>>>> +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
>
Re: [PATCH 4/6] dt-bindings: power: supply: add MAX77659 charger binding
Posted by Krzysztof Kozlowski 2 years, 11 months ago 
On 18/04/2023 11:41, Arslanbenzer, Zeynep wrote:
>>>> 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". 

Wrap your replies to match mailing list style.

Yeah... indeed... because it should not be in DTS in the first place.
It's not a property of the hardware, so the same as with other drivers -
we don't store it in DT. Check other maxim drivers.

> 
>>
>>>
>>> 0b00 = Timer disabled
>>> 0b01 = 3 hours
>>> 0b10 = 5 hours
>>> 0b11 = 7 hours
Best regards,
Krzysztof

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.