On 18/10/2024 12:26:44+0800, Joseph Jang wrote:
>
>
> On 2024/6/24 9:43 AM, Joseph Jang wrote:
> >
> >
> > On 2024/6/21 3:36 AM, Alexandre Belloni wrote:
> > > On 23/05/2024 18:38:06-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 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 <kobak@xxxxxxxxxx>
> > > > Reviewed-by: Matthew R. Ochs <mochs@xxxxxxxxxx>
> > > > Signed-off-by: Joseph Jang <jjang@xxxxxxxxxx>
> > > > ---
> > > > 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/
> > >
> > > Is this change actually needed?
> >
> > If we didn't include "-I../../../../usr/include/" in rtctest Makefile,
> > we may encounter build errors like the following because rtctest default
> > look at the header file from /usr/include/linux/rtc.h which miss the
> > definition of struct rtc_param, RTC_PARAM_FEATURES and RTC_PARAM_GET.
> >
> > rtctest.c: In function ‘get_rtc_alarm_state’:
> > rtctest.c:94:15: error: variable ‘param’ has initializer but incomplete
> > type
> > 94 | struct rtc_param param = { 0 };
> > | ^~~~~~~~~
> > rtctest.c:94:35: warning: excess elements in struct initializer
> > 94 | struct rtc_param param = { 0 };
> > | ^
> > rtctest.c:94:35: note: (near initialization for ‘param’)
> > rtctest.c:94:25: error: storage size of ‘param’ isn’t known
> > 94 | struct rtc_param param = { 0 };
> > | ^~~~~
> > rtctest.c:98:22: error: ‘RTC_PARAM_FEATURES’ undeclared (first use in
> > this function)
> > 98 | param.param = RTC_PARAM_FEATURES;
> > | ^~~~~~~~~~~~~~~~~~
> > rtctest.c:98:22: note: each undeclared identifier is reported only once
> > for each function it appears in
> > rtctest.c:100:23: error: ‘RTC_PARAM_GET’ undeclared (first use in this
> > function); did you mean ‘RTC_ALM_SET’?
> > 100 | rc = ioctl(fd, RTC_PARAM_GET, ¶m);
> > | ^~~~~~~~~~~~~
> > | RTC_ALM_SET
> >
> > After adding "-I../../../../usr/include/", the rtctest will look at
> > linux kernel source header files from
> > <Linux root directory>/usr/include/linux/rtc.h to find the definition of
> > struct rtc_param, RTC_PARAM_FEATURES and RTC_PARAM_GET and fix the
> > rtctest build errors.
> >
> >
> > Thank you,
> > Joseph.
> >
> > >
> Hi Alexandre,
>
> Thank you for reviewing the kernel patch [PATCH 1/2].
> We are still not sure if we could include linux headers files from kernel
> source directory by the following change ?
>
> -CFLAGS += -O3 -Wl,-no-as-needed -Wall
> +CFLAGS += -O3 -Wl,-no-as-needed -Wall -I../../../../usr/include/
I guess this is ok, I expected Shuah to take this path too.
>
> Thank you,
> Joseph.
>
>
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
