Re: [PATCH] Silicon/NXP/Pcf8563RealTimeClockLib: add rtc device initialization

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

On 19 June 2018 at 03:09, Masahisa Kojima <masahisa.kojima@linaro.org> wrote:
> Hi Ard,
>
> This issue can be divided into following two issues.
>
> 1) Assertion occurs during boot time in DEBUG build, it depends on
>     the uninitialized RTC device state(~70%)
> 2) SetTime() fails if GetTime() fails
>
> For issue 1), we should update pcf8563 driver, ignore retrieved data
> and return error when the VL bit is 1.
>

Well, if returning an error is preventing us from using the shell
command, I'd rather work around it.


> For issue 2), it is better to update shell command as your comment.
> But situation is a little complicated.
> UEFI shell command can only set DATE or TIME separately.
> To set DATE, current procedure is like following.
>  1) GetTime(retrieve both DATE and TIME)
>  2) update only DATE
>  3) Write back to RTC both DATE and TIME
> So, current uefi shell can not set DATE/TIME without GetTime().
> One possible solution is adding new command to set both DATE and TIME.
> But it will change user experience and I'm not sure it is acceptable.
> What do you think?
>

Yes, that makes it a bit complicated.

So could we simply return EPOCH_BASE/1/1 0:0:0 in case the VL bit is
set? That allows the time/date commands to work right? Then, it is up
to the user to set the correct time and data, and the VL bit will
remain set until they do so.



> On Tue, 19 Jun 2018 at 08:56, Masahisa Kojima
> <masahisa.kojima@linaro.org> wrote:
>>
>> On Tue, 19 Jun 2018 at 00:51, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> >
>> > On 18 June 2018 at 16:29, Masahisa Kojima <masahisa.kojima@linaro.org> wrote:
>> > > Hi Ard,
>> > >
>> > > Thank you for comment.
>> > >
>> > >> Wouldn't it be better to return EFI_DEVICE_ERROR in this case?
>> > >
>> > > It is first option I come up with to fix this issue.
>> > > But edk2/ShellPkg/Library/UefiShellLevel2CommandsLib/TimeDate.c::CheckAndSetTime()
>> > > performs GetTime()->update with user specified time->SetTime(),
>> > > If GetTime() failes, SetTime() never called and user can not set time.
>> > >  # It really depends on the RTC device, we failed to set time in 70% devices.
>> > >
>> > > Another place to perform this dummy time/date setting is inside of
>> > > LibRtcInitialize().
>> > > Current error occurs in setting time, and I prefer to add this process
>> > > in GetTime().
>> > >
>> >
>> > I think we should fix the shell command instead. Setting the time
>> > should be possible even if getting the time file, precisely for
>> > situations like this one.
>>
>> OK, I agree with you.