On 2024/5/17 3:19 PM, Alexandre Belloni wrote:
On 16/05/2024 19:28:47-0700, Joseph Jang wrote:
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 presence of "alarm" in /proc/driver/rtc.
The rtctest requires the read permission on /dev/rtc0. The rtctest will be skipped if the /dev/rtc0 is not readable.
This change as to be separated. Also, I'm not sure what happened with https://lore.kernel.org/all/20230717175251.54390-1-atulpant.linux@gmail.com/
I apply above patch and seems like still cannot detect the read permission on /dev/rtc0. I guess the 'F_OK' just check the `/dev/rtc0` was there.
I share the error logs by following for your reference.
TAP version 13 1..1 # timeout set to 210 # selftests: rtc: rtctest # TAP version 13 # 1..8 # # Starting 8 tests from 1 test cases. # # RUN rtc.date_read ... # # rtctest.c:53:date_read:Expected -1 (-1) != self->fd (-1) # # date_read: Test terminated by assertion # # FAIL rtc.date_read
Not sure if we could skip the testing by following change ?
FIXTURE_SETUP(rtc) { + if (access(rtc_file, R_OK) != 0) + SKIP(return, "Skipping test since cannot access %s, perhaps miss sudo", + rtc_file) + self->fd = open(rtc_file, O_RDONLY); }
And I make sure we need root permission to access `/dev/rtc0`.
Requires commit 101ca8d05913b ("rtc: efi: Enable SET/GET WAKEUP services as optional")
Reviewed-by: Jeremy Szu jszu@nvidia.com Reviewed-by: Matthew R. Ochs mochs@nvidia.com Signed-off-by: Joseph Jang jjang@nvidia.com
tools/testing/selftests/rtc/Makefile | 2 +- tools/testing/selftests/rtc/rtctest.c | 72 +++++++++++++++++++-------- 2 files changed, 53 insertions(+), 21 deletions(-)
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..aa47b17fbd1a 100644 --- a/tools/testing/selftests/rtc/rtctest.c +++ b/tools/testing/selftests/rtc/rtctest.c @@ -8,6 +8,7 @@ #include <errno.h> #include <fcntl.h> #include <linux/rtc.h> +#include <stdbool.h> #include <stdio.h> #include <stdlib.h> #include <sys/ioctl.h> @@ -24,12 +25,17 @@ #define READ_LOOP_SLEEP_MS 11 static char *rtc_file = "/dev/rtc0"; +static char *rtc_procfs = "/proc/driver/rtc"; FIXTURE(rtc) { int fd; }; FIXTURE_SETUP(rtc) {
- if (access(rtc_file, R_OK) != 0)
SKIP(return, "Skipping test since cannot access %s, perhaps miss sudo",rtc_file);
- self->fd = open(rtc_file, O_RDONLY); }
@@ -82,6 +88,36 @@ static void nanosleep_with_retries(long ns) } } +static bool is_rtc_alarm_supported(int fd) +{
- struct rtc_param param = { 0 };
- int rc;
- char buf[1024] = { 0 };
- /* Validate kernel reflects unsupported RTC alarm state */
- param.param = RTC_PARAM_FEATURES;
- param.index = 0;
- rc = ioctl(fd, RTC_PARAM_GET, ¶m);
- if (rc < 0) {
/* Fallback to read rtc procfs */fd = open(rtc_procfs, O_RDONLY);I think I was clear on the previous thread, no new users of the procfs interface. You can carry this n your own tree but that can't be upstream.
Okay ~ If we use RTC_PARAM_GET ioctl to detect rtc feature only, not sure if that is okay for upstream ?
Thank you, Joseph.