public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
To: Pete Batard <pete@akeo.ie>
Cc: Leif Lindholm <leif.lindholm@linaro.org>,
	 "edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Subject: Re: [PATCH 1/1] EmbeddedPkg/Library: Add VirtualRealTimeClockLib
Date: Wed, 20 Feb 2019 08:42:51 +0100	[thread overview]
Message-ID: <CAKv+Gu-uYJ6Uu=0fR3cPPVyJdEzNyKEGnL=Pard7b+q9ANLYCw@mail.gmail.com> (raw)
In-Reply-To: <327e7ce8-b300-e239-0d47-13ee925711b0@akeo.ie>

On Fri, 15 Feb 2019 at 15:57, Pete Batard <pete@akeo.ie> wrote:
>
> On 2019-02-15 14:39, Ard Biesheuvel wrote:
> > On Fri, 15 Feb 2019 at 11:07, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> >>
> >> On Tue, 12 Feb 2019 at 19:14, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> >>>
> >>> On Mon, Feb 04, 2019 at 12:47:36PM +0000, Pete Batard wrote:
> >>>> This is designed to be used on platforms where a a real RTC is not
> >>>> available and relies on an RtcEpochSeconds variable having been set or,
> >>>> if that is not the case, falls back to using the epoch embedded at
> >>>> compilation time.
> >>>>
> >>>> Note that, in order to keep things simple for the setting of the
> >>>> compilation time variable, only GCC environments with UNIX-like shells
> >>>> and where a 'date' command is available are meant to be supported for
> >>>> now.
> >>>>
> >>>> Contributed-under: TianoCore Contribution Agreement 1.1
> >>>> Signed-off-by: Pete Batard <pete@akeo.ie>
> >>>
> >>> On the whole, this looks good to me.
> >>> One addition we'll need, so that we can build this library standalone
> >>> is an entry in EmbeddedPkg.dsc:
> >>>
> >>> diff --git a/EmbeddedPkg/EmbeddedPkg.dsc b/EmbeddedPkg/EmbeddedPkg.dsc
> >>> index 4d9e6399d5..dc5040e611 100644
> >>> --- a/EmbeddedPkg/EmbeddedPkg.dsc
> >>> +++ b/EmbeddedPkg/EmbeddedPkg.dsc
> >>> @@ -218,6 +218,7 @@ [Components.common]
> >>>     EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.inf
> >>>     EmbeddedPkg/Library/NonCoherentDmaLib/NonCoherentDmaLib.inf
> >>>     EmbeddedPkg/Library/DxeDtPlatformDtbLoaderLibDefault/DxeDtPlatformDtbLoaderLibDefault.inf
> >>> +  EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.inf
> >>>     EmbeddedPkg/EmbeddedMonotonicCounter/EmbeddedMonotonicCounter.inf
> >>>     EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClockRuntimeDxe.inf
> >>>
> >>> I don't have any strong opinions on either of Phil's suggestions, but
> >>> if you could give some feedback on those and fold the above in, this
> >>> could go in.
> >>>
> >>
> >> WIth this addition
> >>
> >> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >>
> >> Pushed as 1b261a705f94..64a17fadcb79
> >
> > OK, there is a problem with this code:
> >
> > +EFI_STATUS
> > +EFIAPI
> > +LibGetTime (
> > +  OUT EFI_TIME               *Time,
> > +  OUT EFI_TIME_CAPABILITIES  *Capabilities
> > +  )
> > +{
> > +  EFI_STATUS  Status;
> > +  UINT32      EpochSeconds;
> >
> > EpochSeconds is declared here, and updated depending on time zone and
> > DST settings. However, the resulting value is never used anywhere.
>
> You're right.
>
> Looks like I forgot to merge all the use of EpochSeconds into
> ElapsedSeconds, from the code I copy/pasted.
>
> I'm very sorry about this, as it's something I should have picked up
> before sending this patch for review.
> > It is not clear to me what the correct fix is, so Pete, could you
> > please look into this?
>
> I'll send a fix for this as soon as I have a chance. Thanks for pointing
> the mistake.
>

Any progress here?


  parent reply	other threads:[~2019-02-20  7:43 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-04 12:47 [PATCH 0/1] EmbeddedPkg/Library: Add VirtualRealTimeClockLib Pete Batard
2019-02-04 12:47 ` [PATCH 1/1] " Pete Batard
2019-02-05 19:57   ` Philippe Mathieu-Daudé
2019-02-15  0:09     ` Pete Batard
2019-02-12 18:14   ` Leif Lindholm
2019-02-15  0:12     ` Pete Batard
2019-02-15  9:57       ` Leif Lindholm
2019-02-15 10:07     ` Ard Biesheuvel
2019-02-15 13:39       ` Ard Biesheuvel
2019-02-15 14:57         ` Pete Batard
2019-02-15 15:09           ` Ard Biesheuvel
2019-02-20  7:42           ` Ard Biesheuvel [this message]
2020-05-14  7:45   ` [edk2] " Philippe Mathieu-Daudé
2020-05-14 13:14     ` Pete Batard

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAKv+Gu-uYJ6Uu=0fR3cPPVyJdEzNyKEGnL=Pard7b+q9ANLYCw@mail.gmail.com' \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox