On 01/08/2022 21.13, Simon Glass wrote: > Hi Heinrich, > > On Mon, 1 Aug 2022 at 08:58, Heinrich Schuchardt <xypron.g...@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.g...@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...@gmail.com> >>>>> Reported-by: Tom Rini <tr...@konsulko.com> >>>>> Suggested-by: Rasmus Villemoes <rasmus.villem...@prevas.dk> >>>>> Signed-off-by: Simon Glass <s...@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.
Who's "we"? I'm with Heinrich. I'd much rather U-Boot showed the time actually stored in the RTC, and if we emulate an RTC, that RTC should emulate what a real RTC does, namely store UTC. If you're worried about that that might confuse somebody, there's no harm adding " UTC" or "+00" or whatever to the output-for-humans (wherever that is). Especially when doing anything else causes weird and still-fragile hacks to be sprinkled throughout the testing code (that DST hack is ugly, and the "only twice a year" isn't accurate, because something on the host may also change /etc/localtime at any time). Rasmus PS: I loved working in Iceland. Apart from the beautiful nature, their timezone is UTC+0 all year round, so all questions of UTC vs localtime vs DST were moot :)
