On Tuesday 08 April 2008, Zhao Yakui wrote:
> + if (t->enabled)
> + if (cmos->wake_on)
> + cmos->wake_on(dev);
That looks ugly (bracketing) ... and suspicious in
terms of conformance to the RTC interface.
Shouldn't it be:
if (t->enabled) {
if (cmos->wake_on)
cmos->wake_on(dev);
} else {
if (cmos->wake_off)
cmos->wake_off(dev);
}
instead? Else I don't see what would disable the
alarm when it's supposed to be disabled. Plus, if
that's done here, I suspect it also needs to be
done in rtc_ioctl() for RTC_AIE_{ON,OFF} and *NOT*
done in cmos_{suspend,resume)() ... very odd for a
wakeup hook, since none of those presume the system
is entering a state from which it could wake!
But in general this raises the question of how
to properly hook up to ACPI. If ACPI's hook is for
normal alarm behavior (supplanting the RTC IRQ),
that's odd ... and it shouldn't fit into a hook
designed to ensure only that wake-from-sleep works.
At any rate, this patch clearly can't merge as-is.
- Dave
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
