Hi Nandor,
On Mon, 28 Jun 2021 at 03:44, Nandor Han <nandor....@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....@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....@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 <s...@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
