| Message ID | 20220801135848.992449-6-sjg@chromium.org |
|---|---|
| State | Accepted |
| Commit | 21ddac140e3040b2693c1a5558a84c19a879c04f |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | rtc: Work around race conditions | expand |
On 8/1/22 15:58, Simon Glass wrote: > At present the sandbox timer uses localtime() which can jump around twice > a year when daylight-saving time changes. > > It would be tricky to make use of gmtime() since we still need to present > the time in local time, as seems to be required by U-Boot's RTC interface. > > The problem can only happen once, so use a loop to detect it and try > again. This should be sufficient to detect either a change in the 'second' > value, or a daylight-saving change. We can assume that the latter also > incorporates a 'second' change, so there is no need to loop more than > twice. > > Signed-off-by: Simon Glass <sjg@chromium.org> Linux systems tend to use UTC on the RTC. There is no reason for sandbox_defconfig to deviate. Please, avoid all this complication by reading the time with gmtime() instead of localtime(). Best regards Heinrich > --- > > test/dm/rtc.c | 32 +++++++++++++++++++------------- > 1 file changed, 19 insertions(+), 13 deletions(-) > > diff --git a/test/dm/rtc.c b/test/dm/rtc.c > index 50086ffcf36..bf97dbbd2f9 100644 > --- a/test/dm/rtc.c > +++ b/test/dm/rtc.c > @@ -252,6 +252,7 @@ static int dm_test_rtc_reset(struct unit_test_state *uts) > struct rtc_time now; > struct udevice *dev, *emul; > long old_base_time, base_time; > + int i; > > ut_assertok(uclass_get_device(UCLASS_RTC, 0, &dev)); > ut_assertok(dm_rtc_get(dev, &now)); > @@ -259,19 +260,24 @@ static int dm_test_rtc_reset(struct unit_test_state *uts) > ut_assertok(i2c_emul_find(dev, &emul)); > ut_assertnonnull(emul); > > - 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 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_assert(base_time - old_base_time <= 1); > + i = 0; > + do { > + old_base_time = sandbox_i2c_rtc_get_set_base_time(emul, 0); > + > + ut_asserteq(0, sandbox_i2c_rtc_get_set_base_time(emul, -1)); > + > + ut_assertok(dm_rtc_reset(dev)); > + base_time = sandbox_i2c_rtc_get_set_base_time(emul, -1); > + > + /* > + * Resetting the RTC should put the base time back to normal. > + * Allow for a one-timeadjustment in case the time flips over > + * while this test process is pre-empted (either by a second > + * or a daylight-saving change), since reset_time() in > + * i2c_rtc_emul.c reads the time from the OS. > + */ > + } while (++i < 2 && base_time != old_base_time); > + ut_asserteq(old_base_time, base_time); > > return 0; > }
Hi Heinrich, On Mon, 1 Aug 2022 at 09:00, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > On 8/1/22 15:58, Simon Glass wrote: > > At present the sandbox timer uses localtime() which can jump around twice > > a year when daylight-saving time changes. > > > > It would be tricky to make use of gmtime() since we still need to present > > the time in local time, as seems to be required by U-Boot's RTC interface. > > > > The problem can only happen once, so use a loop to detect it and try > > again. This should be sufficient to detect either a change in the 'second' > > value, or a daylight-saving change. We can assume that the latter also > > incorporates a 'second' change, so there is no need to loop more than > > twice. > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > Linux systems tend to use UTC on the RTC. There is no reason for > sandbox_defconfig to deviate. Please, avoid all this complication by > reading the time with gmtime() instead of localtime(). That is an API change...how will we show the local time in that case? I suppose we need to add this concept to rtc.h ? Regards, Simon
diff --git a/test/dm/rtc.c b/test/dm/rtc.c index 50086ffcf36..bf97dbbd2f9 100644 --- a/test/dm/rtc.c +++ b/test/dm/rtc.c @@ -252,6 +252,7 @@ static int dm_test_rtc_reset(struct unit_test_state *uts) struct rtc_time now; struct udevice *dev, *emul; long old_base_time, base_time; + int i; ut_assertok(uclass_get_device(UCLASS_RTC, 0, &dev)); ut_assertok(dm_rtc_get(dev, &now)); @@ -259,19 +260,24 @@ static int dm_test_rtc_reset(struct unit_test_state *uts) ut_assertok(i2c_emul_find(dev, &emul)); ut_assertnonnull(emul); - 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 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_assert(base_time - old_base_time <= 1); + i = 0; + do { + old_base_time = sandbox_i2c_rtc_get_set_base_time(emul, 0); + + ut_asserteq(0, sandbox_i2c_rtc_get_set_base_time(emul, -1)); + + ut_assertok(dm_rtc_reset(dev)); + base_time = sandbox_i2c_rtc_get_set_base_time(emul, -1); + + /* + * Resetting the RTC should put the base time back to normal. + * Allow for a one-timeadjustment in case the time flips over + * while this test process is pre-empted (either by a second + * or a daylight-saving change), since reset_time() in + * i2c_rtc_emul.c reads the time from the OS. + */ + } while (++i < 2 && base_time != old_base_time); + ut_asserteq(old_base_time, base_time); return 0; }
At present the sandbox timer uses localtime() which can jump around twice a year when daylight-saving time changes. It would be tricky to make use of gmtime() since we still need to present the time in local time, as seems to be required by U-Boot's RTC interface. The problem can only happen once, so use a loop to detect it and try again. This should be sufficient to detect either a change in the 'second' value, or a daylight-saving change. We can assume that the latter also incorporates a 'second' change, so there is no need to loop more than twice. Signed-off-by: Simon Glass <sjg@chromium.org> --- test/dm/rtc.c | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-)