Re: [PATCH 2/3] dt-bindings: rtc: add chargeable flag for rx8130

[Bastian Krause <bst@pengutronix.de>]

On 8/25/20 5:32 PM, Alexandre Belloni wrote:
> Hi,
> 
> On 24/08/2020 15:32:22+0200, Bastian Krause wrote:
>> On 8/24/20 1:31 PM, Bastian Krause wrote:
>>>
>>> On 4/15/20 8:56 PM, Alexandre Belloni wrote:
>>>> On 15/04/2020 18:37:00+0200, Bastian Krause wrote:
>>>>> Signed-off-by: Bastian Krause <bst@pengutronix.de>
>>>>> ---
>>>>>  Documentation/devicetree/bindings/rtc/rtc-ds1307.txt | 3 +++
>>>>>  1 file changed, 3 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/rtc/rtc-ds1307.txt b/Documentation/devicetree/bindings/rtc/rtc-ds1307.txt
>>>>> index 66f0a31ae9ce..987a0c9e0cd7 100644
>>>>> --- a/Documentation/devicetree/bindings/rtc/rtc-ds1307.txt
>>>>> +++ b/Documentation/devicetree/bindings/rtc/rtc-ds1307.txt
>>>>> @@ -34,6 +34,9 @@ Optional properties:
>>>>>  - trickle-diode-disable : ds1339, ds1340 and ds 1388 only
>>>>>  	Do not use internal trickle charger diode
>>>>>  	Should be given if internal trickle charger diode should be disabled
>>>>> +- aux-voltage-chargeable: rx8130 only
>>>>> +	Epsons's rx8130 supports a backup battery/supercap.
>>>>> +	This flag tells	whether the battery/supercap is chargeable or not.
>>>>>  
>>>>
>>>> I think we should make that a generic property and this should supersede
>>>> trickle-diode-disable which is a bit wonky as I would prefer the default
>>>> to be disabled instead of enabled with the current semantics.
>>>
>>> Alright, I think I know how to transform the RTC drivers.
>>>
>>> One question about the DTs though:
>>>
>>> This means we should remove "trickle-diode-disable" from all upstream
>>> DTs and add "aux-voltage-chargeable" to all upstream DTs that use a RTC
>>> compatible whose driver care in their probe function for
>>> "trickle-diode-disable", right?
>>
>> Sorry, forget that.
>>
>> Here's the situation:
>>
>> Currently there is a switch to explicitly disable charging, so the
>> default is to charge. We cannot introduce another boolean switch to turn
>> that the other way around, because that would change the default and
>> break backwards compatibility.
>>
>> The only way I can think of is to introduce "aux-voltage-chargeable" not
>> as a boolean switch but as an integer, without any default. If this
>> property is not available, the drivers should simply do what they did
>> prior to this change (look for the legacy trickle-diode-disable, use the
>> default they used before).
>>
>> Are you okay with that?
>>
> 
> I agree boolean should be avoided in RTC drivers because we need a way
> to express "don't change this value".

Alright.

>> Some more context:
>>
>> I originally tried to add a chargeable flag for rx8130. Prior to this
>> patch, there was no need to set "trickle-diode-disable" for this,
>> because the driver did not pass the chargeable flag to the RTC. With the
>> patch the default would have been to charge as long as
>> "trickle-diode-disable" is not there. So there's a change in behavior.
>>
> 
> Yes, IIRC, my point was simply to move the documentation for
> aux-voltage-chargeable to the generice rtc binding documentation,
> Documentation/devicetree/bindings/rtc/rtc.yaml
> 
> For now, you sould keep support for trickle-diode-disable but it has to be
> superseded by aux-voltage-chargeable if present. Is that more clear?

Yes, thanks for the clarification.

Should I set the deprecated flag for trickle-diode-disable in the
dt-binding yaml?

Regards,
Bastian

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |