[v2,3/8] dt-bindings: rtc: isl12022: add bindings for battery alarm trip levels

  The isl12022 has a built-in support for monitoring the voltage of the
backup battery, and setting bits in the status register when that
voltage drops below two predetermined levels (usually 85% and 75% of
the nominal voltage). However, since it can operate at wide range of
battery voltages (2.5V - 5.5V), one must configure those trip levels
according to which battery is used on a given board.

Add bindings for defining these two trip levels. While the register
and bit names suggest that they should correspond to 85% and 75% of
the nominal battery voltage, the data sheet also says

  There are total of 7 levels that could be selected for the first
  alarm. Any of the of levels could be selected as the first alarm
  with no reference as to nominal Battery voltage level.

Hence this provides the hardware designer the ability to choose values
based on the discharge characteristics of the battery chosen for the
given product, rather than just having one battery-microvolt property
and having the driver choose levels close to 0.85/0.75 times that.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 .../devicetree/bindings/rtc/intersil,isl12022.yaml | 14 ++++++++++++++
 1 file changed, 14 insertions(+)
  

On 13/06/2023 15:00, Rasmus Villemoes wrote:
> The isl12022 has a built-in support for monitoring the voltage of the
> backup battery, and setting bits in the status register when that
> voltage drops below two predetermined levels (usually 85% and 75% of
> the nominal voltage). However, since it can operate at wide range of
> battery voltages (2.5V - 5.5V), one must configure those trip levels
> according to which battery is used on a given board.
> 
> Add bindings for defining these two trip levels. While the register
> and bit names suggest that they should correspond to 85% and 75% of
> the nominal battery voltage, the data sheet also says
> 
>   There are total of 7 levels that could be selected for the first
>   alarm. Any of the of levels could be selected as the first alarm
>   with no reference as to nominal Battery voltage level.
> 
> Hence this provides the hardware designer the ability to choose values
> based on the discharge characteristics of the battery chosen for the
> given product, rather than just having one battery-microvolt property
> and having the driver choose levels close to 0.85/0.75 times that.
> 
> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> ---
>  .../devicetree/bindings/rtc/intersil,isl12022.yaml | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/rtc/intersil,isl12022.yaml b/Documentation/devicetree/bindings/rtc/intersil,isl12022.yaml
> index 7c1e638d657a..d5d3a687a34d 100644
> --- a/Documentation/devicetree/bindings/rtc/intersil,isl12022.yaml
> +++ b/Documentation/devicetree/bindings/rtc/intersil,isl12022.yaml
> @@ -22,6 +22,18 @@ properties:
>    interrupts:
>      maxItems: 1
>  
> +  isil,trip-level85-microvolt:

Why encoding level85 in the property name? Your commit msg (datasheet)
suggests this is quite flexible, so why it cannot be just list of two
trip levels - for first and second interrupt?

> +    description: |

Do not need '|' unless you need to preserve formatting.

Best regards,
Krzysztof
  

On 13/06/2023 21.09, Krzysztof Kozlowski wrote:
> On 13/06/2023 15:00, Rasmus Villemoes wrote:
>> The isl12022 has a built-in support for monitoring the voltage of the
>> backup battery, and setting bits in the status register when that
>> voltage drops below two predetermined levels (usually 85% and 75% of
>> the nominal voltage). However, since it can operate at wide range of
>> battery voltages (2.5V - 5.5V), one must configure those trip levels
>> according to which battery is used on a given board.
>>
>> Add bindings for defining these two trip levels. While the register
>> and bit names suggest that they should correspond to 85% and 75% of
>> the nominal battery voltage, the data sheet also says
>>
>>   There are total of 7 levels that could be selected for the first
>>   alarm. Any of the of levels could be selected as the first alarm
>>   with no reference as to nominal Battery voltage level.
>>
>> Hence this provides the hardware designer the ability to choose values
>> based on the discharge characteristics of the battery chosen for the
>> given product, rather than just having one battery-microvolt property
>> and having the driver choose levels close to 0.85/0.75 times that.
>>
>> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
>> ---
>>  .../devicetree/bindings/rtc/intersil,isl12022.yaml | 14 ++++++++++++++
>>  1 file changed, 14 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/rtc/intersil,isl12022.yaml b/Documentation/devicetree/bindings/rtc/intersil,isl12022.yaml
>> index 7c1e638d657a..d5d3a687a34d 100644
>> --- a/Documentation/devicetree/bindings/rtc/intersil,isl12022.yaml
>> +++ b/Documentation/devicetree/bindings/rtc/intersil,isl12022.yaml
>> @@ -22,6 +22,18 @@ properties:
>>    interrupts:
>>      maxItems: 1
>>  
>> +  isil,trip-level85-microvolt:
> 
> Why encoding level85 in the property name? Your commit msg (datasheet)
> suggests this is quite flexible, so why it cannot be just list of two
> trip levels - for first and second interrupt?

Yeah, so I did consider just making it a two-element array
isil,trip-levels-microvolt. But then I didn't know how to express the
enum constraint, i.e. that the first must be one of the 2125000, ...,
4675000 values and the second one of the 1875000, ..., 4125000 ones. Is
that possible, without providing a list of 49 possible pairs? Or is it
sufficient to just write this out in prose?

I'm also happy to use other names for these. I just chose to use the 85
and 75 nomenclature because that matches the field names.

>> +    description: |
> 
> Do not need '|' unless you need to preserve formatting.

OK.

Rasmus
  

On 13/06/2023 21:51, Rasmus Villemoes wrote:
>>> diff --git a/Documentation/devicetree/bindings/rtc/intersil,isl12022.yaml b/Documentation/devicetree/bindings/rtc/intersil,isl12022.yaml
>>> index 7c1e638d657a..d5d3a687a34d 100644
>>> --- a/Documentation/devicetree/bindings/rtc/intersil,isl12022.yaml
>>> +++ b/Documentation/devicetree/bindings/rtc/intersil,isl12022.yaml
>>> @@ -22,6 +22,18 @@ properties:
>>>    interrupts:
>>>      maxItems: 1
>>>  
>>> +  isil,trip-level85-microvolt:
>>
>> Why encoding level85 in the property name? Your commit msg (datasheet)
>> suggests this is quite flexible, so why it cannot be just list of two
>> trip levels - for first and second interrupt?
> 
> Yeah, so I did consider just making it a two-element array
> isil,trip-levels-microvolt. But then I didn't know how to express the
> enum constraint, i.e. that the first must be one of the 2125000, ...,
> 4675000 values and the second one of the 1875000, ..., 4125000 ones. Is
> that possible, without providing a list of 49 possible pairs? Or is it
> sufficient to just write this out in prose?

items:
  - enum: [ a, b, c ]
  - enum: [ f, d, e ]


Best regards,
Krzysztof