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

[Masahisa Kojima <masahisa.kojima@linaro.org>]

Hi Ard,

> 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.

Yes, I can boot and set time/date correctly.
I will submit v2 patch.

Regards,
Masahisa

On Tue, 19 Jun 2018 at 17:02, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
> 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.