[LTP] [PATCH 0/3] fix some bugs in rct02

[LTP] [PATCH 0/3] fix some bugs in rct02

[Li Wang]

Li Wang (3):
  rtc02: skip test with unsupport set RTC platform
  rtc02: reset backward 1 hour to RTC time
  rtc02: loosen the compare precision with few seconds

 testcases/kernel/device-drivers/rtc/rtc02.c | 81 ++++++++++++++++-----
 1 file changed, 64 insertions(+), 17 deletions(-)

-- 
2.35.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 1/3] rtc02: skip test with unsupport set RTC platform

[Li Wang]

Some hardware(e.g. Fujisu FX700) does not provides a feature to set the
RTC clock, thus test failed with EINVAL from ioctl(RTC_SET_TIME).

  tst_test.c:1431: TINFO: Timeout per run is 0h 05m 00s
  rtc02.c:66: TINFO: To set RTC date/time is: 2020-10-09 13:23:30
  rtc02.c:70: TFAIL: ioctl() RTC_SET_TIME: EINVAL (22)
  tst_rtctime.c:116: TWARN: open(/dev/rtc,0,7020) failed: EBUSY (16)
  tst_wallclock.c:117: TWARN: tst_rtc_settime() realtime failed: EBADF (9)

Signed-off-by: Li Wang <liwang@redhat.com>
Cc: Hiroyuki Yasuhara <hyasuhar@redhat.com>
---
 testcases/kernel/device-drivers/rtc/rtc02.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/testcases/kernel/device-drivers/rtc/rtc02.c b/testcases/kernel/device-drivers/rtc/rtc02.c
index ef83aad89..8484a0074 100644
--- a/testcases/kernel/device-drivers/rtc/rtc02.c
+++ b/testcases/kernel/device-drivers/rtc/rtc02.c
@@ -22,6 +22,7 @@
 #include "tst_wallclock.h"
 #include "tst_test.h"
 
+static int rtc_unsupport;
 static char *rtc_dev = "/dev/rtc";
 
 static char *rtctime_to_str(struct rtc_time *tm)
@@ -67,6 +68,10 @@ static void set_rtc_test(void)
 
 	ret = tst_rtc_settime(rtc_dev, &set_tm);
 	if (ret != 0) {
+		if (errno == EINVAL) {
+			rtc_unsupport = 1;
+			tst_brk(TCONF, "RTC may not support be set via ioctl(RTC_SET_TIME)");
+		}
 		tst_res(TFAIL | TERRNO, "ioctl() RTC_SET_TIME");
 		return;
 	}
@@ -99,7 +104,8 @@ static void rtc_setup(void)
 
 static void rtc_cleanup(void)
 {
-	tst_rtc_clock_restore(rtc_dev);
+	if (!rtc_unsupport)
+		tst_rtc_clock_restore(rtc_dev);
 }
 
 static struct tst_test test = {
-- 
2.35.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 2/3] rtc02: reset backward 1 hour to RTC time

[Li Wang]

If there's a limit on how much backward time can be set to RTC,
rtc02 easily get an EINVAL error as well. (Note: that limitation
may enforced by the host or by EFI firmware)

  rtc02.c:70: TFAIL: ioctl() RTC_SET_TIME: EINVAL (22)
  tst_rtctime.c:116: TWARN: open(/dev/rtc,0,21042104211) failed: EBUSY (16)
  tst_wallclock.c:117: TWARN: tst_rtc_settime() realtime failed: EBADF (9)

To show the problem by manually performing `hwclock -w`.

  # date -s "2020-10-09 13:23:30"
  Fri Oct  9 13:23:30 EDT 2020
  # hwclock -w
  hwclock: ioctl(RTC_SET_TIME) to /dev/rtc0 to set the time failed: Invalid argument

If just go with a few days backwards things work:

  # date -s "2022-03-12 13:23:30"
  Sat Mar 12 13:23:30 EST 2022
  # hwclock -w
  # echo $?
  0

To be on the safe side, here reset backward +/-1 hour (in case of zero clock).

Signed-off-by: Li Wang <liwang@redhat.com>
Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
Cc: Eirik Fuller <efuller@redhat.com>
---
 testcases/kernel/device-drivers/rtc/rtc02.c | 27 ++++++++++++---------
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/testcases/kernel/device-drivers/rtc/rtc02.c b/testcases/kernel/device-drivers/rtc/rtc02.c
index 8484a0074..6198a5d5d 100644
--- a/testcases/kernel/device-drivers/rtc/rtc02.c
+++ b/testcases/kernel/device-drivers/rtc/rtc02.c
@@ -51,19 +51,23 @@ static int rtc_tm_cmp(struct rtc_time *set_tm, struct rtc_time *read_tm)
 
 static void set_rtc_test(void)
 {
-	struct rtc_time read_tm;
+	struct rtc_time read_tm, set_tm;
 	int ret;
 
-	struct rtc_time set_tm = {
-		.tm_sec = 30,
-		.tm_min = 23,
-		.tm_hour = 13,
-		.tm_mday = 9,
-		.tm_mon = 9,
-		.tm_year = 120,
-	};
+	/* Read current RTC Time */
+	ret = tst_rtc_gettime(rtc_dev, &read_tm);
+	if (ret != 0) {
+		tst_res(TFAIL | TERRNO, "ioctl() RTC_RD_TIME");
+		return;
+	}
+
+	/* set rtc to +/-1 hour */
+	set_tm = read_tm;
+	if (set_tm.tm_hour == 0)
+		set_tm.tm_hour += 1;
+	else
+		set_tm.tm_hour -= 1;
 
-	/* set rtc to 2020.10.9 13:23:30 */
 	tst_res(TINFO, "To set RTC date/time is: %s", rtctime_to_str(&set_tm));
 
 	ret = tst_rtc_settime(rtc_dev, &set_tm);
@@ -76,7 +80,7 @@ static void set_rtc_test(void)
 		return;
 	}
 
-	/* Read current RTC Time */
+	/* Read new RTC Time */
 	ret = tst_rtc_gettime(rtc_dev, &read_tm);
 	if (ret != 0) {
 		tst_res(TFAIL | TERRNO, "ioctl() RTC_RD_TIME");
@@ -89,7 +93,6 @@ static void set_rtc_test(void)
 		return;
 	}
 	tst_res(TPASS, "The read RTC time is consistent with set time");
-
 }
 
 static void rtc_setup(void)
-- 
2.35.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 3/3] rtc02: loosen the compare precision with few seconds

[Li Wang]

That possibly has time elapse between twice operations, especially
on VM we can't guarantee the time precisely equal, let's lose a few
seconds to make the test happy:

  tst_test.c:1433: TINFO: Timeout per run is 0h 10m 00s
  rtc02.c:66: TINFO: To set RTC date/time is: 2020-10-09 13:23:30
  rtc02.c:80: TINFO: read RTC date/time is: 2020-10-09 13:23:31
  rtc02.c:83: TFAIL: RTC SET TEST

Signed-off-by: Li Wang <liwang@redhat.com>
Cc: Eirik Fuller <efuller@redhat.com>
---
 testcases/kernel/device-drivers/rtc/rtc02.c | 46 +++++++++++++++++++--
 1 file changed, 42 insertions(+), 4 deletions(-)

diff --git a/testcases/kernel/device-drivers/rtc/rtc02.c b/testcases/kernel/device-drivers/rtc/rtc02.c
index 6198a5d5d..a008971d5 100644
--- a/testcases/kernel/device-drivers/rtc/rtc02.c
+++ b/testcases/kernel/device-drivers/rtc/rtc02.c
@@ -41,10 +41,48 @@ static char *rtctime_to_str(struct rtc_time *tm)
 
 static int rtc_tm_cmp(struct rtc_time *set_tm, struct rtc_time *read_tm)
 {
-	return !((set_tm->tm_sec == read_tm->tm_sec)
-		&& (set_tm->tm_min == read_tm->tm_min)
-		&& (set_tm->tm_hour == read_tm->tm_hour)
-		&& (set_tm->tm_mday == read_tm->tm_mday)
+	int delta = read_tm->tm_sec - set_tm->tm_sec;
+
+	/*
+	 * To handle the normal and special situations:
+	 * 1#
+	 *       set_tm:  2022-04-28 13:00:50
+	 *       read_tm: 2022-04-28 13:00:50
+	 * 2#
+	 *       set_tm:  2022-04-28 13:00:50
+	 *       read_tm: 2022-04-28 13:00:51
+	 * 3#
+	 *       set_tm:  2022-04-28 13:00:59
+	 *       read_tm: 2022-04-28 13:01:00
+	 * 4#
+	 *       set_tm:  2022-04-28 13:59:59
+	 *       read_tm: 2022-04-28 14:00:00
+	 *
+	 * Note: as we have avoided testing around the zero
+	 * clock, so it's impossible to hit situation 5#
+	 *       set_tm:  2022-04-28 23:59:59
+	 *       read_tm: 2022-04-29 00:00:00
+	 */
+
+	/* 1~3 */
+	if (set_tm->tm_hour == read_tm->tm_hour) {
+		if (set_tm->tm_min == read_tm->tm_min - 1)
+			delta += 60;
+		else if (set_tm->tm_min != read_tm->tm_min)
+			return 1;
+	}
+
+	/* 4 */
+	if ((set_tm->tm_hour == read_tm->tm_hour - 1) &&
+			(set_tm->tm_min == read_tm->tm_min + 59))
+		delta += 60;
+	else if ((set_tm->tm_hour != read_tm->tm_hour))
+		return 1;
+
+	if (delta < 0 || delta > 3)
+		return 1;
+
+	return !((set_tm->tm_mday == read_tm->tm_mday)
 		&& (set_tm->tm_mon == read_tm->tm_mon)
 		&& (set_tm->tm_year == read_tm->tm_year));
 }
-- 
2.35.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/3] rtc02: skip test with unsupport set RTC platform

[Cyril Hrubis]

Hi!
> Some hardware(e.g. Fujisu FX700) does not provides a feature to set the
> RTC clock, thus test failed with EINVAL from ioctl(RTC_SET_TIME).
> 
>   tst_test.c:1431: TINFO: Timeout per run is 0h 05m 00s
>   rtc02.c:66: TINFO: To set RTC date/time is: 2020-10-09 13:23:30
>   rtc02.c:70: TFAIL: ioctl() RTC_SET_TIME: EINVAL (22)
>   tst_rtctime.c:116: TWARN: open(/dev/rtc,0,7020) failed: EBUSY (16)
>   tst_wallclock.c:117: TWARN: tst_rtc_settime() realtime failed: EBADF (9)
> 
> Signed-off-by: Li Wang <liwang@redhat.com>
> Cc: Hiroyuki Yasuhara <hyasuhar@redhat.com>
> ---
>  testcases/kernel/device-drivers/rtc/rtc02.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/testcases/kernel/device-drivers/rtc/rtc02.c b/testcases/kernel/device-drivers/rtc/rtc02.c
> index ef83aad89..8484a0074 100644
> --- a/testcases/kernel/device-drivers/rtc/rtc02.c
> +++ b/testcases/kernel/device-drivers/rtc/rtc02.c
> @@ -22,6 +22,7 @@
>  #include "tst_wallclock.h"
>  #include "tst_test.h"
>  
> +static int rtc_unsupport;
>  static char *rtc_dev = "/dev/rtc";
>  
>  static char *rtctime_to_str(struct rtc_time *tm)
> @@ -67,6 +68,10 @@ static void set_rtc_test(void)
>  
>  	ret = tst_rtc_settime(rtc_dev, &set_tm);
>  	if (ret != 0) {
> +		if (errno == EINVAL) {
> +			rtc_unsupport = 1;
> +			tst_brk(TCONF, "RTC may not support be set via ioctl(RTC_SET_TIME)");
> +		}

I wonder if there is a different way how to figure out setting RTC is
not supported, this may potentionaly mask away a breakage in a driver
that is supposed to be able to set time like this.

>  		tst_res(TFAIL | TERRNO, "ioctl() RTC_SET_TIME");
>  		return;
>  	}
> @@ -99,7 +104,8 @@ static void rtc_setup(void)
>  
>  static void rtc_cleanup(void)
>  {
> -	tst_rtc_clock_restore(rtc_dev);
> +	if (!rtc_unsupport)
> +		tst_rtc_clock_restore(rtc_dev);
>  }
>  
>  static struct tst_test test = {
> -- 
> 2.35.1
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 2/3] rtc02: reset backward 1 hour to RTC time

[Cyril Hrubis]

Hi!
Looks good.

Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/3] rtc02: skip test with unsupport set RTC platform

[Li Wang]

[-- Attachment #1.1: Type: text/plain, Size: 426 bytes --]

On Fri, Apr 29, 2022 at 7:05 PM Cyril Hrubis <chrubis@suse.cz> wrote:


>
> I wonder if there is a different way how to figure out setting RTC is
> not supported, this may potentionaly mask away a breakage in a driver
> that is supposed to be able to set time like this.
>

Yes, that is what I really hope to know as well.

Not sure if Hiroyuki Yasuhara (already on CC list) can provide more info
here.


-- 
Regards,
Li Wang

[-- Attachment #1.2: Type: text/html, Size: 1131 bytes --]

[-- Attachment #2: Type: text/plain, Size: 60 bytes --]


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 3/3] rtc02: loosen the compare precision with few seconds

[Cyril Hrubis]

Hi!
> That possibly has time elapse between twice operations, especially
> on VM we can't guarantee the time precisely equal, let's lose a few
> seconds to make the test happy:
> 
>   tst_test.c:1433: TINFO: Timeout per run is 0h 10m 00s
>   rtc02.c:66: TINFO: To set RTC date/time is: 2020-10-09 13:23:30
>   rtc02.c:80: TINFO: read RTC date/time is: 2020-10-09 13:23:31
>   rtc02.c:83: TFAIL: RTC SET TEST
> 
> Signed-off-by: Li Wang <liwang@redhat.com>
> Cc: Eirik Fuller <efuller@redhat.com>
> ---
>  testcases/kernel/device-drivers/rtc/rtc02.c | 46 +++++++++++++++++++--
>  1 file changed, 42 insertions(+), 4 deletions(-)
> 
> diff --git a/testcases/kernel/device-drivers/rtc/rtc02.c b/testcases/kernel/device-drivers/rtc/rtc02.c
> index 6198a5d5d..a008971d5 100644
> --- a/testcases/kernel/device-drivers/rtc/rtc02.c
> +++ b/testcases/kernel/device-drivers/rtc/rtc02.c
> @@ -41,10 +41,48 @@ static char *rtctime_to_str(struct rtc_time *tm)
>  
>  static int rtc_tm_cmp(struct rtc_time *set_tm, struct rtc_time *read_tm)
>  {
> -	return !((set_tm->tm_sec == read_tm->tm_sec)
> -		&& (set_tm->tm_min == read_tm->tm_min)
> -		&& (set_tm->tm_hour == read_tm->tm_hour)
> -		&& (set_tm->tm_mday == read_tm->tm_mday)
> +	int delta = read_tm->tm_sec - set_tm->tm_sec;
> +
> +	/*
> +	 * To handle the normal and special situations:
> +	 * 1#
> +	 *       set_tm:  2022-04-28 13:00:50
> +	 *       read_tm: 2022-04-28 13:00:50
> +	 * 2#
> +	 *       set_tm:  2022-04-28 13:00:50
> +	 *       read_tm: 2022-04-28 13:00:51
> +	 * 3#
> +	 *       set_tm:  2022-04-28 13:00:59
> +	 *       read_tm: 2022-04-28 13:01:00
> +	 * 4#
> +	 *       set_tm:  2022-04-28 13:59:59
> +	 *       read_tm: 2022-04-28 14:00:00
> +	 *
> +	 * Note: as we have avoided testing around the zero
> +	 * clock, so it's impossible to hit situation 5#
> +	 *       set_tm:  2022-04-28 23:59:59
> +	 *       read_tm: 2022-04-29 00:00:00
> +	 */
> +
> +	/* 1~3 */
> +	if (set_tm->tm_hour == read_tm->tm_hour) {
> +		if (set_tm->tm_min == read_tm->tm_min - 1)
> +			delta += 60;
> +		else if (set_tm->tm_min != read_tm->tm_min)
> +			return 1;
> +	}
> +
> +	/* 4 */
> +	if ((set_tm->tm_hour == read_tm->tm_hour - 1) &&
> +			(set_tm->tm_min == read_tm->tm_min + 59))
> +		delta += 60;
> +	else if ((set_tm->tm_hour != read_tm->tm_hour))
> +		return 1;
> +
> +	if (delta < 0 || delta > 3)
> +		return 1;
> +
> +	return !((set_tm->tm_mday == read_tm->tm_mday)
>  		&& (set_tm->tm_mon == read_tm->tm_mon)
>  		&& (set_tm->tm_year == read_tm->tm_year));

Wouldn't it be easier to convert both dates into a 64bit timestamp and
compare the timestamps?

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 3/3] rtc02: loosen the compare precision with few seconds

[Li Wang]

[-- Attachment #1.1: Type: text/plain, Size: 2089 bytes --]

On Thu, May 5, 2022 at 10:03 PM Cyril Hrubis <chrubis@suse.cz> wrote:


> > +      * To handle the normal and special situations:
> > +      * 1#
> > +      *       set_tm:  2022-04-28 13:00:50
> > +      *       read_tm: 2022-04-28 13:00:50
> > +      * 2#
> > +      *       set_tm:  2022-04-28 13:00:50
> > +      *       read_tm: 2022-04-28 13:00:51
> > +      * 3#
> > +      *       set_tm:  2022-04-28 13:00:59
> > +      *       read_tm: 2022-04-28 13:01:00
> > +      * 4#
> > +      *       set_tm:  2022-04-28 13:59:59
> > +      *       read_tm: 2022-04-28 14:00:00
> > +      *
> > +      * Note: as we have avoided testing around the zero
> > +      * clock, so it's impossible to hit situation 5#
> > +      *       set_tm:  2022-04-28 23:59:59
> > +      *       read_tm: 2022-04-29 00:00:00
> > +      */
> > +
> > +     /* 1~3 */
> > +     if (set_tm->tm_hour == read_tm->tm_hour) {
> > +             if (set_tm->tm_min == read_tm->tm_min - 1)
> > +                     delta += 60;
> > +             else if (set_tm->tm_min != read_tm->tm_min)
> > +                     return 1;
> > +     }
> > +
> > +     /* 4 */
> > +     if ((set_tm->tm_hour == read_tm->tm_hour - 1) &&
> > +                     (set_tm->tm_min == read_tm->tm_min + 59))
> > +             delta += 60;
> > +     else if ((set_tm->tm_hour != read_tm->tm_hour))
> > +             return 1;
> > +
> > +     if (delta < 0 || delta > 3)
> > +             return 1;
> > +
> > +     return !((set_tm->tm_mday == read_tm->tm_mday)
> >               && (set_tm->tm_mon == read_tm->tm_mon)
> >               && (set_tm->tm_year == read_tm->tm_year));
>
> Wouldn't it be easier to convert both dates into a 64bit timestamp and
> compare the timestamps?
>

Yeah, this is a good thought. And we don't even need to convert all dates
because we have avoided the zero clock setting. IOW, just take the
hours/min/sec to be compared should be enough.
Update process:
  patch1/3: drop this one until we find a good way to detect hardware
support.
  patch2/3: pushed.
  patch3/3: will send a V2.

-- 
Regards,
Li Wang

[-- Attachment #1.2: Type: text/html, Size: 3601 bytes --]

[-- Attachment #2: Type: text/plain, Size: 60 bytes --]


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v2] rtc02: loosen the compare precision with few seconds

[Li Wang]

That possibly has time elapse between twice operations, especially
on VM we can't guarantee the time precisely equal, let's lose a few
seconds to make the test happy:

  tst_test.c:1433: TINFO: Timeout per run is 0h 10m 00s
  rtc02.c:66: TINFO: To set RTC date/time is: 2020-10-09 13:23:30
  rtc02.c:80: TINFO: read RTC date/time is: 2020-10-09 13:23:31
  rtc02.c:83: TFAIL: RTC SET TEST

Signed-off-by: Li Wang <liwang@redhat.com>
Cc: Eirik Fuller <efuller@redhat.com>
Cc: Cyril Hrubis <chrubis@suse.cz>
---
 testcases/kernel/device-drivers/rtc/rtc02.c | 41 +++++++++++++++++++--
 1 file changed, 37 insertions(+), 4 deletions(-)

diff --git a/testcases/kernel/device-drivers/rtc/rtc02.c b/testcases/kernel/device-drivers/rtc/rtc02.c
index 0705357bb..8b03b0a90 100644
--- a/testcases/kernel/device-drivers/rtc/rtc02.c
+++ b/testcases/kernel/device-drivers/rtc/rtc02.c
@@ -40,10 +40,43 @@ static char *rtctime_to_str(struct rtc_time *tm)
 
 static int rtc_tm_cmp(struct rtc_time *set_tm, struct rtc_time *read_tm)
 {
-	return !((set_tm->tm_sec == read_tm->tm_sec)
-		&& (set_tm->tm_min == read_tm->tm_min)
-		&& (set_tm->tm_hour == read_tm->tm_hour)
-		&& (set_tm->tm_mday == read_tm->tm_mday)
+	long delta, seconds1, seconds2;
+
+	/*
+	 * Convert hour/min/sec into seconds to handle the normal
+	 * and special situations:
+	 * 1#
+	 *       set_tm:  2022-04-28 13:00:50
+	 *       read_tm: 2022-04-28 13:00:50
+	 * 2#
+	 *       set_tm:  2022-04-28 13:00:50
+	 *       read_tm: 2022-04-28 13:00:51
+	 * 3#
+	 *       set_tm:  2022-04-28 13:00:59
+	 *       read_tm: 2022-04-28 13:01:00
+	 * 4#
+	 *       set_tm:  2022-04-28 13:59:59
+	 *       read_tm: 2022-04-28 14:00:00
+	 *
+	 * Note: as we have avoided testing around the zero
+	 * clock, so it's impossible to hit situation 5#
+	 *       set_tm:  2022-04-28 23:59:59
+	 *       read_tm: 2022-04-29 00:00:00
+	 */
+	if (!(set_tm->tm_sec == read_tm->tm_sec)
+		|| !(set_tm->tm_min == read_tm->tm_min)
+		|| !(set_tm->tm_hour == read_tm->tm_hour)) {
+
+		seconds1 = (set_tm->tm_hour  * 3600) + (set_tm->tm_min  * 60) + set_tm->tm_sec;
+		seconds2 = (read_tm->tm_hour * 3600) + (read_tm->tm_min * 60) + read_tm->tm_sec;
+
+		delta = seconds2 - seconds1;
+
+		if (delta < 0 || delta > 3)
+			return 1;
+	}
+
+	return !((set_tm->tm_mday == read_tm->tm_mday)
 		&& (set_tm->tm_mon == read_tm->tm_mon)
 		&& (set_tm->tm_year == read_tm->tm_year));
 }
-- 
2.31.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2] rtc02: loosen the compare precision with few seconds

[Li Wang]

[-- Attachment #1.1: Type: text/plain, Size: 718 bytes --]

On Sun, May 8, 2022 at 11:06 AM Li Wang <liwang@redhat.com> wrote:


> +       if (!(set_tm->tm_sec == read_tm->tm_sec)
> +               || !(set_tm->tm_min == read_tm->tm_min)
> +               || !(set_tm->tm_hour == read_tm->tm_hour)) {
>

nit: I should use 'A != B' directly but not '!(A==B)' in the expression.
That looks strange anyway.



> +
> +               seconds1 = (set_tm->tm_hour  * 3600) + (set_tm->tm_min  *
> 60) + set_tm->tm_sec;
> +               seconds2 = (read_tm->tm_hour * 3600) + (read_tm->tm_min *
> 60) + read_tm->tm_sec;
> +
> +               delta = seconds2 - seconds1;
> +
> +               if (delta < 0 || delta > 3)
>

Better to print seconds value if fails.

-- 
Regards,
Li Wang

[-- Attachment #1.2: Type: text/html, Size: 1702 bytes --]

[-- Attachment #2: Type: text/plain, Size: 60 bytes --]


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2] rtc02: loosen the compare precision with few seconds

[Cyril Hrubis]

Hi!
>  static int rtc_tm_cmp(struct rtc_time *set_tm, struct rtc_time *read_tm)
>  {
> -	return !((set_tm->tm_sec == read_tm->tm_sec)
> -		&& (set_tm->tm_min == read_tm->tm_min)
> -		&& (set_tm->tm_hour == read_tm->tm_hour)
> -		&& (set_tm->tm_mday == read_tm->tm_mday)
> +	long delta, seconds1, seconds2;
> +
> +	/*
> +	 * Convert hour/min/sec into seconds to handle the normal
> +	 * and special situations:
> +	 * 1#
> +	 *       set_tm:  2022-04-28 13:00:50
> +	 *       read_tm: 2022-04-28 13:00:50
> +	 * 2#
> +	 *       set_tm:  2022-04-28 13:00:50
> +	 *       read_tm: 2022-04-28 13:00:51
> +	 * 3#
> +	 *       set_tm:  2022-04-28 13:00:59
> +	 *       read_tm: 2022-04-28 13:01:00
> +	 * 4#
> +	 *       set_tm:  2022-04-28 13:59:59
> +	 *       read_tm: 2022-04-28 14:00:00
> +	 *
> +	 * Note: as we have avoided testing around the zero
> +	 * clock, so it's impossible to hit situation 5#
> +	 *       set_tm:  2022-04-28 23:59:59
> +	 *       read_tm: 2022-04-29 00:00:00
> +	 */
> +	if (!(set_tm->tm_sec == read_tm->tm_sec)
> +		|| !(set_tm->tm_min == read_tm->tm_min)
> +		|| !(set_tm->tm_hour == read_tm->tm_hour)) {
> +
> +		seconds1 = (set_tm->tm_hour  * 3600) + (set_tm->tm_min  * 60) + set_tm->tm_sec;
> +		seconds2 = (read_tm->tm_hour * 3600) + (read_tm->tm_min * 60) + read_tm->tm_sec;
> +
> +		delta = seconds2 - seconds1;
> +
> +		if (delta < 0 || delta > 3)
> +			return 1;
> +	}
> +
> +	return !((set_tm->tm_mday == read_tm->tm_mday)
>  		&& (set_tm->tm_mon == read_tm->tm_mon)
>  		&& (set_tm->tm_year == read_tm->tm_year));
>  }

I would have done this a bit differently, first chek for day, mon, year
then do the calculation as:

	if (set_tm->tm_year != read_tm->tm_year)
		return 1;

	if (set_tm->tm_mon != read_tm->tm_mon)
		return 1;

	if (set_tm->tm_mday != read_tm->tm_mday)
		return 1;

	seconds1 = ....
	seconds2 = ....
	delta = ...

	if (delta < 0 || delta > 3)
		return 1;

	return 0;


I find this a bit clearer to read.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2] rtc02: loosen the compare precision with few seconds

[Li Wang]

[-- Attachment #1.1: Type: text/plain, Size: 971 bytes --]

Cyril Hrubis <chrubis@suse.cz> wrote:


>
> I would have done this a bit differently, first chek for day, mon, year
> then do the calculation as:
>
>         if (set_tm->tm_year != read_tm->tm_year)
>                 return 1;
>
>         if (set_tm->tm_mon != read_tm->tm_mon)
>                 return 1;
>
>         if (set_tm->tm_mday != read_tm->tm_mday)
>                 return 1;
>
>
I slightly want to keep rtc_time comparison before converting
into seconds here, because the time change rarely happens,
we don't need to use seconds every time.

+       if ((set_tm->tm_sec != read_tm->tm_sec)
+               || (set_tm->tm_min != read_tm->tm_min)
+               || (set_tm->tm_hour != read_tm->tm_hour)) {


>         seconds1 = ....
>         seconds2 = ....
>         delta = ...
>
>         if (delta < 0 || delta > 3)
>                 return 1;
>

+ }


>
>         return 0;
>
>
> I find this a bit clearer to read.
>

Fair enough!

-- 
Regards,
Li Wang

[-- Attachment #1.2: Type: text/html, Size: 2363 bytes --]

[-- Attachment #2: Type: text/plain, Size: 60 bytes --]


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp