RE: [PATCH v2 5/8] dt-bindings: power: supply: max77658: Add ADI MAX77658 Battery

[Arslanbenzer, Zeynep]

On Wed, 22 Mar 2023, Krzysztof Kozlowski wrote:
>On 22/03/2023 06:56, Zeynep Arslanbenzer wrote:
>> Add ADI MAX77658 power supply devicetree document.
>> 
>> Signed-off-by: Nurettin Bolucu <Nurettin.Bolucu@xxxxxxxxxx>
>> Signed-off-by: Zeynep Arslanbenzer <Zeynep.Arslanbenzer@xxxxxxxxxx>
>> ---
>>  .../power/supply/adi,max77658-battery.yaml    | 58 +++++++++++++++++++
>>  1 file changed, 58 insertions(+)
>>  create mode 100644 
>> Documentation/devicetree/bindings/power/supply/adi,max77658-battery.ya
>> ml
>> 
>> diff --git 
>> a/Documentation/devicetree/bindings/power/supply/adi,max77658-battery.
>> yaml 
>> b/Documentation/devicetree/bindings/power/supply/adi,max77658-battery.
>> yaml
>> new file mode 100644
>> index 000000000000..0b696f7c4d1b
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/power/supply/adi,max77658-batt
>> +++ ery.yaml
>> @@ -0,0 +1,58 @@
>> +# 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,max77658-battery.yaml*__;Iw!!A3Ni8CS0y2Y!7jzMr8UalEjjYfmquE6Iqt
>> +SndU7f-c9va789cC2VmSpvstAZ-AokoftF1vX_ZdeLxGuE455k4EMaG0BdyEAEeqCT4rs
>> +zrkvmwS9F$
>> +$schema: 
>> +https://urldefense.com/v3/__http://devicetree.org/meta-schemas/core.y
>> +aml*__;Iw!!A3Ni8CS0y2Y!7jzMr8UalEjjYfmquE6IqtSndU7f-c9va789cC2VmSpvst
>> +AZ-AokoftF1vX_ZdeLxGuE455k4EMaG0BdyEAEeqCT4rszromzOD1g$
>> +
>> +title: Battery for MAX77658 PMIC from ADI.
>
>Implement all previous comments, not just some.
>
>
>> +
>> +maintainers:
>> +  - Nurettin Bolucu <Nurettin.Bolucu@xxxxxxxxxx>
>> +  - Zeynep Arslanbenzer <Zeynep.Arslanbenzer@xxxxxxxxxx>
>> +
>> +description: |
>> +  This module is part of the MAX77658 MFD device. For more details
>> +  see Documentation/devicetree/bindings/mfd/adi,max77658.yaml.
>> +
>> +  The fuel gauge is represented as a sub-node of the PMIC node on the device tree.
>> +
>> +properties:
>> +  compatible:
>> +    const:
>> +      adi,max77658-battery
>
>It's one line.
>
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  adi,valrt-min-microvolt:
>> +    description: Minimum voltage value that triggers the alarm.
>> +
>> +  adi,valrt-max-microvolt:
>> +    description: Maximum voltage value that triggers the alarm.
>
>Use the same syntax as battery.yaml
>
>> +
>> +  adi,salrt-min-percent:
>> +    description: Minimum percentage of battery that triggers the alarm.
>> +
>> +  adi,salrt-max-percent:
>> +    description: Maximum percentage of battery that triggers the alarm.
>
>That's not suitable for DT. Do not encode policies into DT.
>
>> +
>> +  adi,ialrt-min-microamp:
>> +    description: Minimum current value that triggers the alarm.
>> +
>> +  adi,ialrt-max-microamp:
>> +    description: Maximum current value that triggers the alarm.
>> +
>> +  monitored-battery:
>> +    description: >
>> +      phandle to a "simple-battery" compatible node.
>> +
>> +      This property must be a phandle to a node using the format 
>> + described
>
>You already said it above.
>
>> +      in battery.yaml, with the following properties being required:
>> +      - alert-celsius
>> +
>> +required:
>> +  - compatible
>
>Why reg and monitored-batter are not required?
>
If no monitored-battery information is supplied, we set default values for alert-celsius. The reg property is the I2C address of the device and it is part of the parent schema. Therefore, both are not required in this file.

Best regards,
Zeynep