fixing ACPI wakealarm test?

[Al Stone]

On 09/27/2017 11:41 AM, Jeffrey Hugo wrote:
> On 9/26/2017 5:33 PM, Colin Ian King wrote:
>> On 27/09/17 00:27, Al Stone wrote:
>>> So, some platforms don't really have an ACPI wake alarm, but the test is set to
>>> essentially always be run:
>>>
>>> FWTS_REGISTER("wakealarm", &wakealarm_ops, FWTS_TEST_ANYTIME, FWTS_FLAG_BATCH |
>>> FWTS_FLAG_ROOT_PRIV)
>>>
>>> Is there a recommended way to make a test more hardware specific?  Granted, if
>>> I run FWTS on the platform, I can double check and make sure /sys/class/rtc/rtc0
>>> exists (or something like that); if I run FWTS elsewhere, though, that's clearly
>>> not going to work.  I know I've added options in the past for architectures, but
>>> this is specific to an individual machine and it would seem like overkill to
>>> put in options for specific machines.
>>>
>>> Suggestions?
>>
>> Perhaps the simplest approach is to skip all the subsequent tests if
>> wakealarm_test1() fails to find a working interface.
> 
> Its been a while since I've looked at this, but if I recall correctly,
> wakealarm_tes1() always finds a "working" interface due to how the Linux kernel
> abstracts RTC.
> 
> What I've seen on our platform is that UEFI does not implement the wake alarm
> functionality, however the kernel is happy to "fake" it, until you try setting
> an actual alarm, then you start getting failures.
> 
> I'd be interested in determining a way to address this, since it would alleviate
> the burden of blacklisting the test and explaining why.

@Jeffrey: would you mind testing the patch below (it should apply to top-of-tree
for FWTS)?  Two things change with this patch: (1) assume fwts_wakealarm_get()
does not work, and the ioctl() has to to succeed, and (2) fail if the wakealarm
is not currently enabled.  That *may* be sufficient for your platform, but I
think the RTC ioctl() has to be fixed ultimately -- it's kind of strange the way
it works, as you noted, and it makes it really hard for the device to tell you
if the RTC wakealarm is working, or even exists.


diff --git a/src/lib/src/fwts_wakealarm.c b/src/lib/src/fwts_wakealarm.c
index c0550303..d725caaa 100644
--- a/src/lib/src/fwts_wakealarm.c
+++ b/src/lib/src/fwts_wakealarm.c
@@ -39,17 +39,21 @@ static const char *fwts_rtc = "/dev/rtc0";
 int fwts_wakealarm_get(fwts_framework *fw, struct rtc_time *rtc_tm)
 {
 	int fd;
-	int ret = FWTS_OK;
+	int ret = FWTS_ERROR;
+	struct rtc_wkalrm rtc_wakealarm;

 	if ((fd = open(fwts_rtc, O_RDWR)) < 0) {
 		fwts_log_error(fw, "Cannot access Real Time Clock device %s.", fwts_rtc);
 		return FWTS_ERROR;
 	}

-	if (ioctl(fd, RTC_ALM_READ, rtc_tm) < 0) {
-		fwts_log_error(fw, "Cannot read Real Time Clock Alarm with ioctl RTC_ALM_READ
%s.", fwts_rtc);
-		ret = FWTS_ERROR;
+	if (ioctl(fd, RTC_WKALM_RD, &rtc_wakealarm) < 0) {
+		if (!rtc_wakealarm.enabled) {
+			fwts_log_error(fw, "Cannot read Real Time Clock Alarm with ioctl
RTC_ALM_READ %s.", fwts_rtc);
+			ret = FWTS_ERROR;
+		}
 	}
+	ret = FWTS_OK;
 	(void)close(fd);

 	return ret;


>>
>>>
>>> BTW, I was not able to file a bug for this without registering with Ubuntu One;
>>> is that really necessary?
>>>
>> Afraid so :-(
>>
>> Colin

Hrm.  Well, that's a pain.

-- 
ciao,
al
-----------------------------------
Al Stone
Software Engineer
Linaro Enterprise Group
al.stone at linaro.org
-----------------------------------