[PATCH] dm: rtc: Avoid a race in the rtc_reset test

[Simon Glass]

Hi Heinrich,

On Mon, 1 Aug 2022 at 08:58, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>
> On 8/1/22 15:59, Simon Glass wrote:
> > Hi Heinrich,
> >
> > On Mon, 1 Aug 2022 at 02:11, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> >>
> >> On 7/31/22 20:27, Simon Glass wrote:
> >>> Since resetting the RTC on sandbox causes it to read the base time from
> >>> the system, we cannot rely on this being unchanged since it was last read.
> >>> Allow for a one-second delay.
> >>>
> >>> Fixes: https://source.denx.de/u-boot/u-boot/-/issues/4
> >>> Reported-by: Bin Meng <bmeng.cn at gmail.com>
> >>> Reported-by: Tom Rini <trini at konsulko.com>
> >>> Suggested-by: Rasmus Villemoes <rasmus.villemoes at prevas.dk>
> >>> Signed-off-by: Simon Glass <sjg at chromium.org>
> >>> ---
> >>>
> >>>    test/dm/rtc.c | 11 ++++++++---
> >>>    1 file changed, 8 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/test/dm/rtc.c b/test/dm/rtc.c
> >>> index c7f9f8f0ce7..403bf5c640a 100644
> >>> --- a/test/dm/rtc.c
> >>> +++ b/test/dm/rtc.c
> >>> @@ -245,16 +245,21 @@ static int dm_test_rtc_reset(struct unit_test_state *uts)
> >>>        ut_assertok(dm_rtc_get(dev, &now));
> >>>
> >>>        ut_assertok(i2c_emul_find(dev, &emul));
> >>> -     ut_assert(emul != NULL);
> >>> +     ut_assertnonnull(emul);
> >>
> >> This is an unrelated change. It would be preferable to describe it in
> >> the commit message.
> >>
> >>>
> >>>        old_base_time = sandbox_i2c_rtc_get_set_base_time(emul, 0);
> >>>
> >>>        ut_asserteq(0, sandbox_i2c_rtc_get_set_base_time(emul, -1));
> >>>
> >>> -     /* Resetting the RTC should put he base time back to normal */
> >>> +     /*
> >>> +      * Resetting the RTC should put the base time back to normal. Allow for
> >>> +      * a one-second adjustment in case the time flips over while this
> >>> +      * test process is pre-empted, since reset_time() in i2c_rtc_emul.c
> >>> +      * reads the time from the OS.
> >>> +      */
> >>>        ut_assertok(dm_rtc_reset(dev));
> >>>        base_time = sandbox_i2c_rtc_get_set_base_time(emul, -1);
> >>> -     ut_asserteq(old_base_time, base_time);
> >>> +     ut_assert(base_time - old_base_time <= 1);
> >>
> >> If the operating system uses daylight saving time, this may still fail
> >> (very rarely).
> >>
> >> How about using gmtime() instead of localtime()? But that would be a
> >> separate patch.
> >
> > I'm not sure how to do this, as U-Boot expects local time. Of course
> > we could enhance the rtc API to support both (and use gmtime for the
>
> What makes you think that U-Boot expects local time? U-Boot has no
> notion of time zones. Linux systems tend to use UTC on the RTC. Why
> should sandbox_defconfig deviate?
>
> $ sudo hwclock --show -v
> hwclock from util-linux 2.38
> System Time: 1659365754.792540
> Trying to open: /dev/rtc0
> Using the rtc interface to the clock.
> Assuming hardware clock is kept in UTC time.

Well the thing is, we want to show local times in U-Boot. As I
mentioned, we could expand the API to allow control over that, e.g. to
show local times but use UTC in tests?

>
> If you dual boot into Windows, you should better adjust Windows to use UTC.

I'm not the world's most dedicated Windows user.

Regards,
Simon


>
> Best regards
>
> Heinrich
>
> > test).
> >
> > I've got an idea how to deal with daylight savings, and an alternative
> > approach for this patch, so will send a series.
> >
> >>
> >> Reviewed-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
> >>
> >>>
> >>>        return 0;
> >>>    }
> >>
> >
> > Regards,
> > Simon
>