2024-05-24 01:38:41

by Joseph Jang

[permalink] [raw]
Subject: [PATCH 0/2] selftest: rtc: Add rtc feature detection and rtc file check

1. In order to make rtctest more explicit and robust, we propose to use
RTC_PARAM_GET ioctl interface to check rtc alarm feature state before
running alarm related tests.

2. The rtctest requires the read permission on /dev/rtc0. The rtctest will
be skipped if the /dev/rtc0 is not readable.

Joseph Jang (2):
selftest: rtc: Add to check rtc alarm status for alarm related test
selftest: rtc: Check if could access /dev/rtc0 before testing

tools/testing/selftests/rtc/Makefile | 2 +-
tools/testing/selftests/rtc/rtctest.c | 71 ++++++++++++++++++++++++++-
2 files changed, 71 insertions(+), 2 deletions(-)

--
2.34.1



2024-05-24 01:38:45

by Joseph Jang

[permalink] [raw]
Subject: [PATCH 2/2] selftest: rtc: Check if could access /dev/rtc0 before testing

The rtctest requires the read permission on /dev/rtc0. The rtctest will
be skipped if the /dev/rtc0 is not readable.

Reviewed-by: Koba Ko <[email protected]>
Reviewed-by: Matthew R. Ochs <[email protected]>
Signed-off-by: Joseph Jang <[email protected]>
---
tools/testing/selftests/rtc/rtctest.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/rtc/rtctest.c b/tools/testing/selftests/rtc/rtctest.c
index 2b12497eb30d..d104f5326cf4 100644
--- a/tools/testing/selftests/rtc/rtctest.c
+++ b/tools/testing/selftests/rtc/rtctest.c
@@ -483,6 +483,8 @@ __constructor_order_last(void)

int main(int argc, char **argv)
{
+ int ret = -1;
+
switch (argc) {
case 2:
rtc_file = argv[1];
@@ -494,5 +496,12 @@ int main(int argc, char **argv)
return 1;
}

- return test_harness_run(argc, argv);
+ /* Run the test if rtc_file is accessible */
+ if (access(rtc_file, R_OK) == 0)
+ ret = test_harness_run(argc, argv);
+ else
+ ksft_exit_skip("[SKIP]: Cannot access rtc file %s - Exiting\n",
+ rtc_file);
+
+ return ret;
}
--
2.34.1


2024-05-24 01:38:49

by Joseph Jang

[permalink] [raw]
Subject: [PATCH 1/2] selftest: rtc: Add to check rtc alarm status for alarm related test

In alarm_wkalm_set and alarm_wkalm_set_minute test, they use different
ioctl (RTC_ALM_SET/RTC_WKALM_SET) for alarm feature detection. They will
skip testing if RTC_ALM_SET/RTC_WKALM_SET ioctl returns an EINVAL error
code. This design may miss detecting real problems when the
efi.set_wakeup_time() return errors and then RTC_ALM_SET/RTC_WKALM_SET
ioctl returns an EINVAL error code with RTC_FEATURE_ALARM enabled.

In order to make rtctest more explicit and robust, we propose to use
RTC_PARAM_GET ioctl interface to check rtc alarm feature state before
running alarm related tests. If the kernel does not support RTC_PARAM_GET
ioctl interface, we will fallback to check the error number of
(RTC_ALM_SET/RTC_WKALM_SET) ioctl call for alarm feature detection.

Requires commit 101ca8d05913b ("rtc: efi: Enable SET/GET WAKEUP services
as optional")

Reviewed-by: Koba Ko <[email protected]>
Reviewed-by: Matthew R. Ochs <[email protected]>
Signed-off-by: Joseph Jang <[email protected]>
---
tools/testing/selftests/rtc/Makefile | 2 +-
tools/testing/selftests/rtc/rtctest.c | 64 +++++++++++++++++++++++++++
2 files changed, 65 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/rtc/Makefile b/tools/testing/selftests/rtc/Makefile
index 55198ecc04db..6e3a98fb24ba 100644
--- a/tools/testing/selftests/rtc/Makefile
+++ b/tools/testing/selftests/rtc/Makefile
@@ -1,5 +1,5 @@
# SPDX-License-Identifier: GPL-2.0
-CFLAGS += -O3 -Wl,-no-as-needed -Wall
+CFLAGS += -O3 -Wl,-no-as-needed -Wall -I../../../../usr/include/
LDLIBS += -lrt -lpthread -lm

TEST_GEN_PROGS = rtctest
diff --git a/tools/testing/selftests/rtc/rtctest.c b/tools/testing/selftests/rtc/rtctest.c
index 63ce02d1d5cc..2b12497eb30d 100644
--- a/tools/testing/selftests/rtc/rtctest.c
+++ b/tools/testing/selftests/rtc/rtctest.c
@@ -25,6 +25,12 @@

static char *rtc_file = "/dev/rtc0";

+enum rtc_alarm_state {
+ RTC_ALARM_UNKNOWN,
+ RTC_ALARM_ENABLED,
+ RTC_ALARM_DISABLED,
+};
+
FIXTURE(rtc) {
int fd;
};
@@ -82,6 +88,24 @@ static void nanosleep_with_retries(long ns)
}
}

+static enum rtc_alarm_state get_rtc_alarm_state(int fd)
+{
+ struct rtc_param param = { 0 };
+ int rc;
+
+ /* Validate kernel reflects unsupported RTC alarm state */
+ param.param = RTC_PARAM_FEATURES;
+ param.index = 0;
+ rc = ioctl(fd, RTC_PARAM_GET, &param);
+ if (rc < 0)
+ return RTC_ALARM_UNKNOWN;
+
+ if ((param.uvalue & _BITUL(RTC_FEATURE_ALARM)) == 0)
+ return RTC_ALARM_DISABLED;
+
+ return RTC_ALARM_ENABLED;
+}
+
TEST_F_TIMEOUT(rtc, date_read_loop, READ_LOOP_DURATION_SEC + 2) {
int rc;
long iter_count = 0;
@@ -197,11 +221,16 @@ TEST_F(rtc, alarm_alm_set) {
fd_set readfds;
time_t secs, new;
int rc;
+ enum rtc_alarm_state alarm_state = RTC_ALARM_UNKNOWN;

if (self->fd == -1 && errno == ENOENT)
SKIP(return, "Skipping test since %s does not exist", rtc_file);
ASSERT_NE(-1, self->fd);

+ alarm_state = get_rtc_alarm_state(self->fd);
+ if (alarm_state == RTC_ALARM_DISABLED)
+ SKIP(return, "Skipping test since alarms are not supported.");
+
rc = ioctl(self->fd, RTC_RD_TIME, &tm);
ASSERT_NE(-1, rc);

@@ -210,6 +239,11 @@ TEST_F(rtc, alarm_alm_set) {

rc = ioctl(self->fd, RTC_ALM_SET, &tm);
if (rc == -1) {
+ /*
+ * Report error if rtc alarm was enabled. Fallback to check ioctl
+ * error number if rtc alarm state is unknown.
+ */
+ ASSERT_EQ(RTC_ALARM_UNKNOWN, alarm_state);
ASSERT_EQ(EINVAL, errno);
TH_LOG("skip alarms are not supported.");
return;
@@ -255,11 +289,16 @@ TEST_F(rtc, alarm_wkalm_set) {
fd_set readfds;
time_t secs, new;
int rc;
+ enum rtc_alarm_state alarm_state = RTC_ALARM_UNKNOWN;

if (self->fd == -1 && errno == ENOENT)
SKIP(return, "Skipping test since %s does not exist", rtc_file);
ASSERT_NE(-1, self->fd);

+ alarm_state = get_rtc_alarm_state(self->fd);
+ if (alarm_state == RTC_ALARM_DISABLED)
+ SKIP(return, "Skipping test since alarms are not supported.");
+
rc = ioctl(self->fd, RTC_RD_TIME, &alarm.time);
ASSERT_NE(-1, rc);

@@ -270,6 +309,11 @@ TEST_F(rtc, alarm_wkalm_set) {

rc = ioctl(self->fd, RTC_WKALM_SET, &alarm);
if (rc == -1) {
+ /*
+ * Report error if rtc alarm was enabled. Fallback to check ioctl
+ * error number if rtc alarm state is unknown.
+ */
+ ASSERT_EQ(RTC_ALARM_UNKNOWN, alarm_state);
ASSERT_EQ(EINVAL, errno);
TH_LOG("skip alarms are not supported.");
return;
@@ -307,11 +351,16 @@ TEST_F_TIMEOUT(rtc, alarm_alm_set_minute, 65) {
fd_set readfds;
time_t secs, new;
int rc;
+ enum rtc_alarm_state alarm_state = RTC_ALARM_UNKNOWN;

if (self->fd == -1 && errno == ENOENT)
SKIP(return, "Skipping test since %s does not exist", rtc_file);
ASSERT_NE(-1, self->fd);

+ alarm_state = get_rtc_alarm_state(self->fd);
+ if (alarm_state == RTC_ALARM_DISABLED)
+ SKIP(return, "Skipping test since alarms are not supported.");
+
rc = ioctl(self->fd, RTC_RD_TIME, &tm);
ASSERT_NE(-1, rc);

@@ -320,6 +369,11 @@ TEST_F_TIMEOUT(rtc, alarm_alm_set_minute, 65) {

rc = ioctl(self->fd, RTC_ALM_SET, &tm);
if (rc == -1) {
+ /*
+ * Report error if rtc alarm was enabled. Fallback to check ioctl
+ * error number if rtc alarm state is unknown.
+ */
+ ASSERT_EQ(RTC_ALARM_UNKNOWN, alarm_state);
ASSERT_EQ(EINVAL, errno);
TH_LOG("skip alarms are not supported.");
return;
@@ -365,11 +419,16 @@ TEST_F_TIMEOUT(rtc, alarm_wkalm_set_minute, 65) {
fd_set readfds;
time_t secs, new;
int rc;
+ enum rtc_alarm_state alarm_state = RTC_ALARM_UNKNOWN;

if (self->fd == -1 && errno == ENOENT)
SKIP(return, "Skipping test since %s does not exist", rtc_file);
ASSERT_NE(-1, self->fd);

+ alarm_state = get_rtc_alarm_state(self->fd);
+ if (alarm_state == RTC_ALARM_DISABLED)
+ SKIP(return, "Skipping test since alarms are not supported.");
+
rc = ioctl(self->fd, RTC_RD_TIME, &alarm.time);
ASSERT_NE(-1, rc);

@@ -380,6 +439,11 @@ TEST_F_TIMEOUT(rtc, alarm_wkalm_set_minute, 65) {

rc = ioctl(self->fd, RTC_WKALM_SET, &alarm);
if (rc == -1) {
+ /*
+ * Report error if rtc alarm was enabled. Fallback to check ioctl
+ * error number if rtc alarm state is unknown.
+ */
+ ASSERT_EQ(RTC_ALARM_UNKNOWN, alarm_state);
ASSERT_EQ(EINVAL, errno);
TH_LOG("skip alarms are not supported.");
return;
--
2.34.1