| Message ID | 20230201143431.863784-1-frieder@fris.de (mailing list archive) |
|---|---|
| Headers | show |
| Series | Enable backup switch mode on RTCs via devicetree | expand |
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.
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