[PATCH v4 3/3] reboot-mode: read the boot mode from RTC memory

[Simon Glass]

Hi Nandor,

On Mon, 28 Jun 2021 at 03:44, Nandor Han <nandor.han at vaisala.com> wrote:
>
> On 6/26/21 9:30 PM, Simon Glass wrote:
> > Hi Nandor,
> >
> > On Thu, 10 Jun 2021 at 07:57, Nandor Han <nandor.han at vaisala.com> wrote:
> >>
> >> RTC devices could provide battery-backed memory that can be used for
> >> storing the reboot mode magic value.
> >>
> >> Add a new reboot-mode back-end that uses RTC to store the reboot-mode
> >> magic value. The driver also supports both endianness modes.
> >>
> >> Signed-off-by: Nandor Han <nandor.han at vaisala.com>
> >> ---
> >>   arch/sandbox/dts/test.dts                     |  10 ++
> >>   configs/sandbox_defconfig                     |   1 +
> >>   .../reboot-mode/reboot-mode-rtc.txt           |  22 +++
> >>   drivers/reboot-mode/Kconfig                   |   9 ++
> >>   drivers/reboot-mode/Makefile                  |   1 +
> >>   drivers/reboot-mode/reboot-mode-rtc.c         | 127 ++++++++++++++++++
> >>   include/reboot-mode/reboot-mode-rtc.h         |  16 +++
> >>   test/dm/reboot-mode.c                         |  29 ++++
> >>   8 files changed, 215 insertions(+)
> >>   create mode 100644 doc/device-tree-bindings/reboot-mode/reboot-mode-rtc.txt
> >>   create mode 100644 drivers/reboot-mode/reboot-mode-rtc.c
> >>   create mode 100644 include/reboot-mode/reboot-mode-rtc.h
> >>
> >
> > Reviewed-by: Simon Glass <sjg at chromium.org>
> >
> > But I think it would be better to put this device as a child of the
> > rtc, so it is more obvious that they are related. It also avoids the
> > phandle. What do you think?
> >
>
> Hi Simon and thanks for reviewing the code,
>    I think the idea is not bad, however, would be ok to consider the
> recommendation as improvement opportunity and merge this as
> it is for now? I'm a bit busy with some other stuff now and I could
> start implementing this some time later. At least it would help me since
> this are on the mailing list since Dec 2019 :).

Well I don't think you could change this later because it would change
the binding. I suppose you could add it as an option.

I am OK with either way (hence my reviewed-by tag). With my use of RTC
for data storage I have made it a subnode since it makes it easy to
see which parts of the RTC are used for what:

   rtc: rtc {
      compatible = "motorola,mc146818";
      u-boot,dm-pre-proper;
      reg = <0x70 2>;
      #address-cells = <1>;
      #size-cells = <0>;
      nvdata {
         u-boot,dm-vpl;
         compatible = "google,cmos-nvdata";
         reg = <0x26>;
         nvdata,types = <CROS_NV_DATA>;
      };
   };

Regards,
Simon