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

Nandor Han
2021-07-14T09:55:25Z

On 7/5/21 6:29 PM, Simon Glass wrote:
> Hi Nandor,
>
> On Mon, 28 Jun 2021 at 03:44, Nandor Han <nandor.han@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@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@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@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).

Thanks Simon.

--
Regards,
Nandor