[0/7] Enable backup switch mode on RTCs via devicetree

  From: Frieder Schrempf <frieder.schrempf@kontron.de>

Some RTC devices like the RV3028 have BSM disabled as factory default.
This makes the RTC quite useless if it is expected to preserve the
time on hardware that has a battery-buffered supply for the RTC.

Let boards that have a buffered supply for the RTC force the BSM to the
desired value via devicetree by setting the 'backup-switch-mode' property.

That way the RTC on the boards work as one would expect them to do without
any per-board intervention through userspace tools to enable BSM.

Frieder Schrempf (7):
  dt-bindings: rtc: Move RV3028 to separate binding file
  dt-bindings: rtc: Add backup-switch-mode property
  dt-bindings: rtc: microcrystal,rv3032: Add backup-switch-mode property
  rtc: Move BSM defines to separate header for DT usage
  rtc: class: Support setting backup switch mode from devicetree
  arm64: dts: imx8mm-kontron: Remove useless trickle-diode-disable from
    RTC node
  arm64: dts: imx8mm-kontron: Enable backup switch mode for RTC on OSM-S
    module

 .../bindings/rtc/microcrystal,rv3028.yaml     | 60 +++++++++++++++++++
 .../devicetree/bindings/rtc/rtc.yaml          |  7 +++
 .../devicetree/bindings/rtc/trivial-rtc.yaml  |  2 -
 .../dts/freescale/imx8mm-kontron-osm-s.dtsi   |  3 +-
 drivers/rtc/class.c                           | 14 +++++
 include/dt-bindings/rtc/rtc.h                 | 11 ++++
 include/uapi/linux/rtc.h                      |  6 +-
 7 files changed, 95 insertions(+), 8 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/rtc/microcrystal,rv3028.yaml
 create mode 100644 include/dt-bindings/rtc/rtc.h
  

Hello,

You can't do that, this breaks an important use case and it is the
reason why I didn't use device tree in the beginning. What is wrong with
setting BSM from userspace? You will anyway have to set the time and
date from userspace for it to be saved.

On 01/02/2023 15:34:22+0100, Frieder Schrempf wrote:
> From: Frieder Schrempf <frieder.schrempf@kontron.de>
> 
> Some RTC devices like the RV3028 have BSM disabled as factory default.
> This makes the RTC quite useless if it is expected to preserve the
> time on hardware that has a battery-buffered supply for the RTC.
> 
> Let boards that have a buffered supply for the RTC force the BSM to the
> desired value via devicetree by setting the 'backup-switch-mode' property.
> 
> That way the RTC on the boards work as one would expect them to do without
> any per-board intervention through userspace tools to enable BSM.
> 
> Frieder Schrempf (7):
>   dt-bindings: rtc: Move RV3028 to separate binding file
>   dt-bindings: rtc: Add backup-switch-mode property
>   dt-bindings: rtc: microcrystal,rv3032: Add backup-switch-mode property
>   rtc: Move BSM defines to separate header for DT usage
>   rtc: class: Support setting backup switch mode from devicetree
>   arm64: dts: imx8mm-kontron: Remove useless trickle-diode-disable from
>     RTC node
>   arm64: dts: imx8mm-kontron: Enable backup switch mode for RTC on OSM-S
>     module
> 
>  .../bindings/rtc/microcrystal,rv3028.yaml     | 60 +++++++++++++++++++
>  .../devicetree/bindings/rtc/rtc.yaml          |  7 +++
>  .../devicetree/bindings/rtc/trivial-rtc.yaml  |  2 -
>  .../dts/freescale/imx8mm-kontron-osm-s.dtsi   |  3 +-
>  drivers/rtc/class.c                           | 14 +++++
>  include/dt-bindings/rtc/rtc.h                 | 11 ++++
>  include/uapi/linux/rtc.h                      |  6 +-
>  7 files changed, 95 insertions(+), 8 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/rtc/microcrystal,rv3028.yaml
>  create mode 100644 include/dt-bindings/rtc/rtc.h
> 
> -- 
> 2.39.1
  

On 01.02.23 17:15, Alexandre Belloni wrote:
> Hello,
> 
> You can't do that, this breaks an important use case and it is the
> reason why I didn't use device tree in the beginning. What is wrong with
> setting BSM from userspace? You will anyway have to set the time and
> date from userspace for it to be saved.

Ok, I was already afraid there is something I missed. Can you give a
short explanation of what use case this would break?

There is nothing wrong with setting BSM from userspace. It's just the
fact that users expect BSM to be enabled in any case as there is a
battery on the board. It is much more effort to ensure that production,
user, etc. are aware of an extra step required than to let the kernel
deal with it behind the scenes.
  

Hi Alexandre,

On 01.02.23 17:26, Frieder Schrempf wrote:
> On 01.02.23 17:15, Alexandre Belloni wrote:
>> Hello,
>>
>> You can't do that, this breaks an important use case and it is the
>> reason why I didn't use device tree in the beginning. What is wrong with
>> setting BSM from userspace? You will anyway have to set the time and
>> date from userspace for it to be saved.
> 
> Ok, I was already afraid there is something I missed. Can you give a
> short explanation of what use case this would break?
> 
> There is nothing wrong with setting BSM from userspace. It's just the
> fact that users expect BSM to be enabled in any case as there is a
> battery on the board. It is much more effort to ensure that production,
> user, etc. are aware of an extra step required than to let the kernel
> deal with it behind the scenes.

Would you mind elaborating on your argument that this would break stuff?
I currently don't see how an additional optional devicetree property
would break anything.

Thanks
Frieder
  

On 13.02.23 10:18, Frieder Schrempf wrote:
> Hi Alexandre,
> 
> On 01.02.23 17:26, Frieder Schrempf wrote:
>> On 01.02.23 17:15, Alexandre Belloni wrote:
>>> Hello,
>>>
>>> You can't do that, this breaks an important use case and it is the
>>> reason why I didn't use device tree in the beginning. What is wrong with
>>> setting BSM from userspace? You will anyway have to set the time and
>>> date from userspace for it to be saved.
>>
>> Ok, I was already afraid there is something I missed. Can you give a
>> short explanation of what use case this would break?
>>
>> There is nothing wrong with setting BSM from userspace. It's just the
>> fact that users expect BSM to be enabled in any case as there is a
>> battery on the board. It is much more effort to ensure that production,
>> user, etc. are aware of an extra step required than to let the kernel
>> deal with it behind the scenes.
> 
> Would you mind elaborating on your argument that this would break stuff?
> I currently don't see how an additional optional devicetree property
> would break anything.

Ping!?
  

Hi Alexandre,

On 06.03.23 14:27, Frieder Schrempf wrote:
> On 13.02.23 10:18, Frieder Schrempf wrote:
>> Hi Alexandre,
>>
>> On 01.02.23 17:26, Frieder Schrempf wrote:
>>> On 01.02.23 17:15, Alexandre Belloni wrote:
>>>> Hello,
>>>>
>>>> You can't do that, this breaks an important use case and it is the
>>>> reason why I didn't use device tree in the beginning. What is wrong with
>>>> setting BSM from userspace? You will anyway have to set the time and
>>>> date from userspace for it to be saved.
>>>
>>> Ok, I was already afraid there is something I missed. Can you give a
>>> short explanation of what use case this would break?
>>>
>>> There is nothing wrong with setting BSM from userspace. It's just the
>>> fact that users expect BSM to be enabled in any case as there is a
>>> battery on the board. It is much more effort to ensure that production,
>>> user, etc. are aware of an extra step required than to let the kernel
>>> deal with it behind the scenes.
>>
>> Would you mind elaborating on your argument that this would break stuff?
>> I currently don't see how an additional optional devicetree property
>> would break anything.
> 
> Ping!?

It seems like you decided to ignore me for whatever reasons there are.
I'm sure we can sort it out in some way if you would respond, please.

Thanks
Frieder
  

Hello,

On 22/03/2023 14:14:50+0100, Frieder Schrempf wrote:
> On 06.03.23 14:27, Frieder Schrempf wrote:
> > On 13.02.23 10:18, Frieder Schrempf wrote:
> >> Hi Alexandre,
> >>
> >> On 01.02.23 17:26, Frieder Schrempf wrote:
> >>> On 01.02.23 17:15, Alexandre Belloni wrote:
> >>>> Hello,
> >>>>
> >>>> You can't do that, this breaks an important use case and it is the
> >>>> reason why I didn't use device tree in the beginning. What is wrong with
> >>>> setting BSM from userspace? You will anyway have to set the time and
> >>>> date from userspace for it to be saved.
> >>>
> >>> Ok, I was already afraid there is something I missed. Can you give a
> >>> short explanation of what use case this would break?
> >>>
> >>> There is nothing wrong with setting BSM from userspace. It's just the
> >>> fact that users expect BSM to be enabled in any case as there is a
> >>> battery on the board. It is much more effort to ensure that production,
> >>> user, etc. are aware of an extra step required than to let the kernel
> >>> deal with it behind the scenes.
> >>
> >> Would you mind elaborating on your argument that this would break stuff?
> >> I currently don't see how an additional optional devicetree property
> >> would break anything.
> > 
> > Ping!?
> 
> It seems like you decided to ignore me for whatever reasons there are.
> I'm sure we can sort it out in some way if you would respond, please.

I do what I can with the time I have.

There are 2 issues:
 - the first one is that this is encoding device configuration in the
   device tree which is forbidden. BSM is not really hardware related.
The worse that could happen is that the backup voltage is not present
and so the RTC will never switch to the backup source.

 - the second one is why I got to a userspace solution. There are RTC
   where it is crucial to be able to change BSM dynamically. Those RTCs
have a standby mode: they will only draw current from the backup source
once they have seen VDD once. This is useful when you install a battery
in a product and this products stays on the shelf for a while before
being used. However, if your production line needs to powerup the device
to flash it or perform tests, the RTC will get out of standby mode and
you need a way to get it back to standby. This is possible with the
current interface, I'm not going to have a second interface.

Regards,
  

On 22.03.23 23:19, Alexandre Belloni wrote:
> Hello,
> 
> On 22/03/2023 14:14:50+0100, Frieder Schrempf wrote:
>> On 06.03.23 14:27, Frieder Schrempf wrote:
>>> On 13.02.23 10:18, Frieder Schrempf wrote:
>>>> Hi Alexandre,
>>>>
>>>> On 01.02.23 17:26, Frieder Schrempf wrote:
>>>>> On 01.02.23 17:15, Alexandre Belloni wrote:
>>>>>> Hello,
>>>>>>
>>>>>> You can't do that, this breaks an important use case and it is the
>>>>>> reason why I didn't use device tree in the beginning. What is wrong with
>>>>>> setting BSM from userspace? You will anyway have to set the time and
>>>>>> date from userspace for it to be saved.
>>>>>
>>>>> Ok, I was already afraid there is something I missed. Can you give a
>>>>> short explanation of what use case this would break?
>>>>>
>>>>> There is nothing wrong with setting BSM from userspace. It's just the
>>>>> fact that users expect BSM to be enabled in any case as there is a
>>>>> battery on the board. It is much more effort to ensure that production,
>>>>> user, etc. are aware of an extra step required than to let the kernel
>>>>> deal with it behind the scenes.
>>>>
>>>> Would you mind elaborating on your argument that this would break stuff?
>>>> I currently don't see how an additional optional devicetree property
>>>> would break anything.
>>>
>>> Ping!?
>>
>> It seems like you decided to ignore me for whatever reasons there are.
>> I'm sure we can sort it out in some way if you would respond, please.
> 
> I do what I can with the time I have.

Thanks for taking the time! I know that maintainers are usually
chronically overloaded. Still I got a bit worried after ~7 weeks that I
wont get a reply at all.

> 
> There are 2 issues:
>  - the first one is that this is encoding device configuration in the
>    device tree which is forbidden. BSM is not really hardware related.
> The worse that could happen is that the backup voltage is not present
> and so the RTC will never switch to the backup source.

This is an argument that I was expecting to hear in the first place. I
think this is kind of a grey area as the BSM feature is definitely
related to the hardware implementation of the V_DD and V_BACKUP supply
voltages, but at the same time it also might reflect device configuration.

> 
>  - the second one is why I got to a userspace solution. There are RTC
>    where it is crucial to be able to change BSM dynamically. Those RTCs
> have a standby mode: they will only draw current from the backup source
> once they have seen VDD once. This is useful when you install a battery
> in a product and this products stays on the shelf for a while before
> being used. However, if your production line needs to powerup the device
> to flash it or perform tests, the RTC will get out of standby mode and
> you need a way to get it back to standby. This is possible with the
> current interface, I'm not going to have a second interface.

Thanks for pointing that out. The userspace solution is definitely
useful and necessary and I would never argue against it. What I'm
proposing is not really a second interface but a way to set the default
mode at boot time. If you really think this is too much, then I will
need to scratch this approach.