While 0e36a9a4a788e4e92407774df76c545910810d35 made sure that active
alarms were never returned with invalid "wildcard" fields (negative),
it can still report (wrongly) that the alarm triggers in the past.
Example, if it's now 10am, an alarm firing at 5am will be triggered
TOMORROW not today. (Which may also be next month or next year...)
This updates that alarm handling in three ways:
* Handle alarm rollover in the common cases of RTCs that don't
support matching on all date fields.
* Skip the invalid-field logic when it's not needed.
* Minor bugfix ... tm_isdst should be ignored, it's one of the
fields Linux doesn't maintain.
A warning is emitted for some of the unhandled rollover cases, but
the possible combinations are a bit too numerous to handle every
bit of potential hardware and firmware braindamage.
Signed-off-by: David Brownell <[email protected]>
---
drivers/rtc/interface.c | 102 ++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 91 insertions(+), 11 deletions(-)
--- a/drivers/rtc/interface.c 2008-06-21 19:51:24.000000000 -0700
+++ b/drivers/rtc/interface.c 2008-06-22 13:05:57.000000000 -0700
@@ -126,12 +126,25 @@ int rtc_read_alarm(struct rtc_device *rt
int err;
struct rtc_time before, now;
int first_time = 1;
+ unsigned long t_now, t_alm;
+ enum { none, day, month, year } missing = none;
+ unsigned days;
- /* The lower level RTC driver may not be capable of filling
- * in all fields of the rtc_time struct (eg. rtc-cmos),
- * and so might instead return -1 in some fields.
- * We deal with that here by grabbing a current RTC timestamp
- * and using values from that for any missing (-1) values.
+ /* The lower level RTC driver may return -1 in some fields,
+ * creating invalid alarm->time values, for reasons like:
+ *
+ * - The hardware may not be capable of filling them in;
+ * many alarms match only on time-of-day fields, not
+ * day/month/year calendar data.
+ *
+ * - Some hardware uses illegal values as "wildcard" match
+ * values, which non-Linux firmware (like a BIOS) may try
+ * to set up as e.g. "alarm 15 minutes after each hour".
+ * Linux uses only oneshot alarms.
+ *
+ * When we see that here, we deal with it by using values from
+ * a current RTC timestamp for any missing (-1) values. The
+ * RTC driver prevents "periodic alarm" modes.
*
* But this can be racey, because some fields of the RTC timestamp
* may have wrapped in the interval since we read the RTC alarm,
@@ -174,6 +187,10 @@ int rtc_read_alarm(struct rtc_device *rt
if (!alarm->enabled)
return 0;
+ /* full-function RTCs won't have such missing fields */
+ if (rtc_valid_tm(&alarm->time) == 0)
+ return 0;
+
/* get the "after" timestamp, to detect wrapped fields */
err = rtc_read_time(rtc, &now);
if (err < 0)
@@ -183,22 +200,85 @@ int rtc_read_alarm(struct rtc_device *rt
} while ( before.tm_min != now.tm_min
|| before.tm_hour != now.tm_hour
|| before.tm_mon != now.tm_mon
- || before.tm_year != now.tm_year
- || before.tm_isdst != now.tm_isdst);
+ || before.tm_year != now.tm_year);
- /* Fill in any missing alarm fields using the timestamp */
+ /* Fill in the missing alarm fields using the timestamp; we
+ * know there's at least one since alarm->time is invalid.
+ */
if (alarm->time.tm_sec == -1)
alarm->time.tm_sec = now.tm_sec;
if (alarm->time.tm_min == -1)
alarm->time.tm_min = now.tm_min;
if (alarm->time.tm_hour == -1)
alarm->time.tm_hour = now.tm_hour;
- if (alarm->time.tm_mday == -1)
+
+ /* For simplicity, only support date rollover for now */
+ if (alarm->time.tm_mday == -1) {
alarm->time.tm_mday = now.tm_mday;
- if (alarm->time.tm_mon == -1)
+ missing = day;
+ }
+ if (alarm->time.tm_mon == -1) {
alarm->time.tm_mon = now.tm_mon;
- if (alarm->time.tm_year == -1)
+ if (missing == none)
+ missing = month;
+ }
+ if (alarm->time.tm_year == -1) {
alarm->time.tm_year = now.tm_year;
+ if (missing == none)
+ missing = year;
+ }
+
+ /* with luck, no rollover is needed */
+ rtc_tm_to_time(&now, &t_now);
+ rtc_tm_to_time(&alarm->time, &t_alm);
+ if (t_now < t_alm)
+ goto done;
+
+ switch (missing) {
+
+ /* 24 hour rollover ... if it's now 10am Monday, an alarm that
+ * that will trigger at 5am will do so at 5am Tuesday, which
+ * could also be in the next month or year. This is a common
+ * case, especially for PCs.
+ */
+ case day:
+ dev_dbg(&rtc->dev, "alarm rollover: %s\n", "day");
+ t_alm += 24 * 60 * 60;
+ rtc_time_to_tm(t_alm, &alarm->time);
+ break;
+
+ /* Month rollover ... if it's the 31th, an alarm on the 3rd will
+ * be next month. An alarm matching on the 30th, 29th, or 28th
+ * may end up in the month after that! Many newer PCs support
+ * this type of alarm.
+ */
+ case month:
+ dev_dbg(&rtc->dev, "alarm rollover: %s\n", "month");
+ do {
+ if (alarm->time.tm_mon < 11)
+ alarm->time.tm_mon++;
+ else {
+ alarm->time.tm_mon = 0;
+ alarm->time.tm_year++;
+ }
+ days = rtc_month_days(alarm->time.tm_mon,
+ alarm->time.tm_year);
+ } while (days < alarm->time.tm_mday);
+ break;
+
+ /* Year rollover ... easy except for leap years! */
+ case year:
+ dev_dbg(&rtc->dev, "alarm rollover: %s\n", "year");
+ do {
+ alarm->time.tm_year++;
+ } while (!rtc_valid_tm(&alarm->time));
+ break;
+
+ default:
+ dev_warn(&rtc->dev, "alarm rollover not handled\n");
+ }
+
+done:
return 0;
}
EXPORT_SYMBOL_GPL(rtc_read_alarm);
On Sun, 22 Jun 2008 20:42:13 -0700
David Brownell <[email protected]> wrote:
> While 0e36a9a4a788e4e92407774df76c545910810d35 made sure that active
> alarms were never returned with invalid "wildcard" fields (negative),
> it can still report (wrongly) that the alarm triggers in the past.
>
> Example, if it's now 10am, an alarm firing at 5am will be triggered
> TOMORROW not today. (Which may also be next month or next year...)
>
> This updates that alarm handling in three ways:
>
> * Handle alarm rollover in the common cases of RTCs that don't
> support matching on all date fields.
>
> * Skip the invalid-field logic when it's not needed.
>
> * Minor bugfix ... tm_isdst should be ignored, it's one of the
> fields Linux doesn't maintain.
>
> A warning is emitted for some of the unhandled rollover cases, but
> the possible combinations are a bit too numerous to handle every
> bit of potential hardware and firmware braindamage.
>
> Signed-off-by: David Brownell <[email protected]>
Acked-by: Alessandro Zummo <[email protected]>
--
Best regards,
Alessandro Zummo,
Tower Technologies - Torino, Italy
http://www.towertech.it
Fix month wrap issue with readback from /proc/acpi/alarm
This bug has been around *forever*.
$ echo '2008-12-01 10:36:20' > /proc/acpi/alarm
$ cat /proc/acpi/alarm
2008-11-01 10:36:20
Note how the readback above shows the month incorrectly.
But with this patch applied, it shows the correct month (12).
Patch applies/works on kernels 2.6.25.* through 2.6.27.*,
and probably on earlier kernels as well.
Probably best to queue it up for the next major cycle.
Signed-off-by: Mark Lord <[email protected]>
--- linux-2.6.25/drivers/acpi/sleep/proc.c 2008-06-09 14:27:19.000000000 -0400
+++ linux/drivers/acpi/sleep/proc.c 2008-11-30 18:08:31.000000000 -0500
@@ -84,12 +84,15 @@
#define HAVE_ACPI_LEGACY_ALARM
#endif
+static u32 cmos_bcd_read(int offset, int rtc_control);
+
#ifdef HAVE_ACPI_LEGACY_ALARM
static int acpi_system_alarm_seq_show(struct seq_file *seq, void *offset)
{
u32 sec, min, hr;
u32 day, mo, yr, cent = 0;
+ u32 today = 0;
unsigned char rtc_control = 0;
unsigned long flags;
@@ -97,38 +100,32 @@
spin_lock_irqsave(&rtc_lock, flags);
- sec = CMOS_READ(RTC_SECONDS_ALARM);
- min = CMOS_READ(RTC_MINUTES_ALARM);
- hr = CMOS_READ(RTC_HOURS_ALARM);
rtc_control = CMOS_READ(RTC_CONTROL);
+ sec = cmos_bcd_read(RTC_SECONDS_ALARM, rtc_control);
+ min = cmos_bcd_read(RTC_MINUTES_ALARM, rtc_control);
+ hr = cmos_bcd_read(RTC_HOURS_ALARM, rtc_control);
/* If we ever get an FACP with proper values... */
- if (acpi_gbl_FADT.day_alarm)
+ if (acpi_gbl_FADT.day_alarm) {
/* ACPI spec: only low 6 its should be cared */
day = CMOS_READ(acpi_gbl_FADT.day_alarm) & 0x3F;
- else
- day = CMOS_READ(RTC_DAY_OF_MONTH);
+ if (!(rtc_control & RTC_DM_BINARY) || RTC_ALWAYS_BCD)
+ BCD_TO_BIN(day);
+ } else
+ day = cmos_bcd_read(RTC_DAY_OF_MONTH, rtc_control);
if (acpi_gbl_FADT.month_alarm)
- mo = CMOS_READ(acpi_gbl_FADT.month_alarm);
- else
- mo = CMOS_READ(RTC_MONTH);
+ mo = cmos_bcd_read(acpi_gbl_FADT.month_alarm, rtc_control);
+ else {
+ mo = cmos_bcd_read(RTC_MONTH, rtc_control);
+ today = cmos_bcd_read(RTC_DAY_OF_MONTH, rtc_control);
+ }
if (acpi_gbl_FADT.century)
- cent = CMOS_READ(acpi_gbl_FADT.century);
+ cent = cmos_bcd_read(acpi_gbl_FADT.century, rtc_control);
- yr = CMOS_READ(RTC_YEAR);
+ yr = cmos_bcd_read(RTC_YEAR, rtc_control);
spin_unlock_irqrestore(&rtc_lock, flags);
- if (!(rtc_control & RTC_DM_BINARY) || RTC_ALWAYS_BCD) {
- BCD_TO_BIN(sec);
- BCD_TO_BIN(min);
- BCD_TO_BIN(hr);
- BCD_TO_BIN(day);
- BCD_TO_BIN(mo);
- BCD_TO_BIN(yr);
- BCD_TO_BIN(cent);
- }
-
/* we're trusting the FADT (see above) */
if (!acpi_gbl_FADT.century)
/* If we're not trusting the FADT, we should at least make it
@@ -153,6 +150,20 @@
else
yr += cent * 100;
+ /*
+ * Show correct dates for alarms up to a month into the future.
+ * This solves issues for nearly all situations with the common
+ * 30-day alarm clocks in PC hardware.
+ */
+ if (day < today) {
+ if (mo < 12) {
+ mo += 1;
+ } else {
+ mo = 1;
+ yr += 1;
+ }
+ }
+
seq_printf(seq, "%4.4u-", yr);
(mo > 12) ? seq_puts(seq, "**-") : seq_printf(seq, "%2.2u-", mo);
(day > 31) ? seq_puts(seq, "** ") : seq_printf(seq, "%2.2u ", day);
Mark Lord wrote:
> Fix month wrap issue with readback from /proc/acpi/alarm
> This bug has been around *forever*.
>
> $ echo '2008-12-01 10:36:20' > /proc/acpi/alarm
> $ cat /proc/acpi/alarm
> 2008-11-01 10:36:20
>
> Note how the readback above shows the month incorrectly.
> But with this patch applied, it shows the correct month (12).
..
I should add, that the above test requires that the alarm
be set for any day of the *next* month from the current month.
My MythTV box does a readback test any time it programs a wakeup,
and noticed the bug over this past weekend (2008-11-30).
>
> Patch applies/works on kernels 2.6.25.* through 2.6.27.*,
> and probably on earlier kernels as well.
>
> Probably best to queue it up for the next major cycle.
>
> Signed-off-by: Mark Lord <[email protected]>
>
> --- linux-2.6.25/drivers/acpi/sleep/proc.c 2008-06-09
> 14:27:19.000000000 -0400
> +++ linux/drivers/acpi/sleep/proc.c 2008-11-30 18:08:31.000000000 -0500
> @@ -84,12 +84,15 @@
> #define HAVE_ACPI_LEGACY_ALARM
> #endif
>
> +static u32 cmos_bcd_read(int offset, int rtc_control);
> +
> #ifdef HAVE_ACPI_LEGACY_ALARM
>
> static int acpi_system_alarm_seq_show(struct seq_file *seq, void *offset)
> {
> u32 sec, min, hr;
> u32 day, mo, yr, cent = 0;
> + u32 today = 0;
> unsigned char rtc_control = 0;
> unsigned long flags;
>
> @@ -97,38 +100,32 @@
>
> spin_lock_irqsave(&rtc_lock, flags);
>
> - sec = CMOS_READ(RTC_SECONDS_ALARM);
> - min = CMOS_READ(RTC_MINUTES_ALARM);
> - hr = CMOS_READ(RTC_HOURS_ALARM);
> rtc_control = CMOS_READ(RTC_CONTROL);
> + sec = cmos_bcd_read(RTC_SECONDS_ALARM, rtc_control);
> + min = cmos_bcd_read(RTC_MINUTES_ALARM, rtc_control);
> + hr = cmos_bcd_read(RTC_HOURS_ALARM, rtc_control);
>
> /* If we ever get an FACP with proper values... */
> - if (acpi_gbl_FADT.day_alarm)
> + if (acpi_gbl_FADT.day_alarm) {
> /* ACPI spec: only low 6 its should be cared */
> day = CMOS_READ(acpi_gbl_FADT.day_alarm) & 0x3F;
> - else
> - day = CMOS_READ(RTC_DAY_OF_MONTH);
> + if (!(rtc_control & RTC_DM_BINARY) || RTC_ALWAYS_BCD)
> + BCD_TO_BIN(day);
> + } else
> + day = cmos_bcd_read(RTC_DAY_OF_MONTH, rtc_control);
> if (acpi_gbl_FADT.month_alarm)
> - mo = CMOS_READ(acpi_gbl_FADT.month_alarm);
> - else
> - mo = CMOS_READ(RTC_MONTH);
> + mo = cmos_bcd_read(acpi_gbl_FADT.month_alarm, rtc_control);
> + else {
> + mo = cmos_bcd_read(RTC_MONTH, rtc_control);
> + today = cmos_bcd_read(RTC_DAY_OF_MONTH, rtc_control);
> + }
> if (acpi_gbl_FADT.century)
> - cent = CMOS_READ(acpi_gbl_FADT.century);
> + cent = cmos_bcd_read(acpi_gbl_FADT.century, rtc_control);
>
> - yr = CMOS_READ(RTC_YEAR);
> + yr = cmos_bcd_read(RTC_YEAR, rtc_control);
>
> spin_unlock_irqrestore(&rtc_lock, flags);
>
> - if (!(rtc_control & RTC_DM_BINARY) || RTC_ALWAYS_BCD) {
> - BCD_TO_BIN(sec);
> - BCD_TO_BIN(min);
> - BCD_TO_BIN(hr);
> - BCD_TO_BIN(day);
> - BCD_TO_BIN(mo);
> - BCD_TO_BIN(yr);
> - BCD_TO_BIN(cent);
> - }
> -
> /* we're trusting the FADT (see above) */
> if (!acpi_gbl_FADT.century)
> /* If we're not trusting the FADT, we should at least make it
> @@ -153,6 +150,20 @@
> else
> yr += cent * 100;
>
> + /*
> + * Show correct dates for alarms up to a month into the future.
> + * This solves issues for nearly all situations with the common
> + * 30-day alarm clocks in PC hardware.
> + */
> + if (day < today) {
> + if (mo < 12) {
> + mo += 1;
> + } else {
> + mo = 1;
> + yr += 1;
> + }
> + }
> +
> seq_printf(seq, "%4.4u-", yr);
> (mo > 12) ? seq_puts(seq, "**-") : seq_printf(seq, "%2.2u-", mo);
> (day > 31) ? seq_puts(seq, "** ") : seq_printf(seq, "%2.2u ", day);
On Mon, 1 Dec 2008, Mark Lord wrote:
> Fix month wrap issue with readback from /proc/acpi/alarm
> This bug has been around *forever*.
>
> $ echo '2008-12-01 10:36:20' > /proc/acpi/alarm
> $ cat /proc/acpi/alarm
> 2008-11-01 10:36:20
>
> Note how the readback above shows the month incorrectly.
> But with this patch applied, it shows the correct month (12).
>
> Patch applies/works on kernels 2.6.25.* through 2.6.27.*,
> and probably on earlier kernels as well.
We've had some BCD_TO_BIN() changes here in 2.6.28 already,
does 2.6.28-rc still have this bug?
I'd check myself, but I don't have a /proc/acpi/alarm due to this:
#if defined(CONFIG_RTC_DRV_CMOS) || defined(CONFIG_RTC_DRV_CMOS_MODULE) ||
!defined(CONFIG_X86)
/* use /sys/class/rtc/rtcX/wakealarm instead; it's not ACPI-specific */
#else
#define HAVE_ACPI_LEGACY_ALARM
#endif
thanks,
-Len
Len Brown wrote:
..
> We've had some BCD_TO_BIN() changes here in 2.6.28 already,
> does 2.6.28-rc still have this bug?
..
Yes, bug still there. Here is a version of the patch for 2.6.28-rc6
---snip---
Fix month wrap issue with readback from /proc/acpi/alarm
This bug has been around *forever*.
$ echo '2008-12-01 10:36:20' > /proc/acpi/alarm
$ cat /proc/acpi/alarm
2008-11-01 10:36:20
Note how the readback above shows the month incorrectly.
But with this patch applied, it shows the correct month (12).
This patch applies/works on 2.6.28-rc6.
Probably best to queue it up for the next major cycle.
Signed-off-by: Mark Lord <[email protected]>
--- linux/drivers/acpi/sleep/proc.c 2008-11-20 18:19:22.000000000 -0500
+++ linux-2.6.28-rc6/drivers/acpi/sleep/proc.c 2008-12-01 17:07:51.000000000 -0500
@@ -84,12 +84,15 @@
#define HAVE_ACPI_LEGACY_ALARM
#endif
+static u32 cmos_bcd_read(int offset, int rtc_control);
+
#ifdef HAVE_ACPI_LEGACY_ALARM
static int acpi_system_alarm_seq_show(struct seq_file *seq, void *offset)
{
u32 sec, min, hr;
u32 day, mo, yr, cent = 0;
+ u32 today = 0;
unsigned char rtc_control = 0;
unsigned long flags;
@@ -97,38 +100,32 @@
spin_lock_irqsave(&rtc_lock, flags);
- sec = CMOS_READ(RTC_SECONDS_ALARM);
- min = CMOS_READ(RTC_MINUTES_ALARM);
- hr = CMOS_READ(RTC_HOURS_ALARM);
rtc_control = CMOS_READ(RTC_CONTROL);
+ sec = cmos_bcd_read(RTC_SECONDS_ALARM, rtc_control);
+ min = cmos_bcd_read(RTC_MINUTES_ALARM, rtc_control);
+ hr = cmos_bcd_read(RTC_HOURS_ALARM, rtc_control);
/* If we ever get an FACP with proper values... */
- if (acpi_gbl_FADT.day_alarm)
+ if (acpi_gbl_FADT.day_alarm) {
/* ACPI spec: only low 6 its should be cared */
day = CMOS_READ(acpi_gbl_FADT.day_alarm) & 0x3F;
- else
- day = CMOS_READ(RTC_DAY_OF_MONTH);
+ if (!(rtc_control & RTC_DM_BINARY) || RTC_ALWAYS_BCD)
+ day = bcd2bin(day);
+ } else
+ day = cmos_bcd_read(RTC_DAY_OF_MONTH, rtc_control);
if (acpi_gbl_FADT.month_alarm)
- mo = CMOS_READ(acpi_gbl_FADT.month_alarm);
- else
- mo = CMOS_READ(RTC_MONTH);
+ mo = cmos_bcd_read(acpi_gbl_FADT.month_alarm, rtc_control);
+ else {
+ mo = cmos_bcd_read(RTC_MONTH, rtc_control);
+ today = cmos_bcd_read(RTC_DAY_OF_MONTH, rtc_control);
+ }
if (acpi_gbl_FADT.century)
- cent = CMOS_READ(acpi_gbl_FADT.century);
+ cent = cmos_bcd_read(acpi_gbl_FADT.century, rtc_control);
- yr = CMOS_READ(RTC_YEAR);
+ yr = cmos_bcd_read(RTC_YEAR, rtc_control);
spin_unlock_irqrestore(&rtc_lock, flags);
- if (!(rtc_control & RTC_DM_BINARY) || RTC_ALWAYS_BCD) {
- sec = bcd2bin(sec);
- min = bcd2bin(min);
- hr = bcd2bin(hr);
- day = bcd2bin(day);
- mo = bcd2bin(mo);
- yr = bcd2bin(yr);
- cent = bcd2bin(cent);
- }
-
/* we're trusting the FADT (see above) */
if (!acpi_gbl_FADT.century)
/* If we're not trusting the FADT, we should at least make it
@@ -153,6 +150,20 @@
else
yr += cent * 100;
+ /*
+ * Show correct dates for alarms up to a month into the future.
+ * This solves issues for nearly all situations with the common
+ * 30-day alarm clocks in PC hardware.
+ */
+ if (day < today) {
+ if (mo < 12) {
+ mo += 1;
+ } else {
+ mo = 1;
+ yr += 1;
+ }
+ }
+
seq_printf(seq, "%4.4u-", yr);
(mo > 12) ? seq_puts(seq, "**-") : seq_printf(seq, "%2.2u-", mo);
(day > 31) ? seq_puts(seq, "** ") : seq_printf(seq, "%2.2u ", day);
Len Brown wrote:
> I don't have a /proc/acpi/alarm due to this:
>
> #if defined(CONFIG_RTC_DRV_CMOS) || defined(CONFIG_RTC_DRV_CMOS_MODULE) ||
> !defined(CONFIG_X86)
> /* use /sys/class/rtc/rtcX/wakealarm instead; it's not ACPI-specific */
> #else
> #define HAVE_ACPI_LEGACY_ALARM
> #endif
..
Yeah.. bit of a nuisance that.
I regularly apply this patch to my own kernels:
--- old/drivers/acpi/sleep/proc.c 2008-01-06 16:39:45.000000000 -0500
+++ linux/drivers/acpi/sleep/proc.c 2008-01-06 17:35:35.000000000 -0500
@@ -78,11 +78,11 @@
}
#endif /* CONFIG_ACPI_PROCFS */
-#if defined(CONFIG_RTC_DRV_CMOS) || defined(CONFIG_RTC_DRV_CMOS_MODULE) || !defined(CONFIG_X86)
+//#if defined(CONFIG_RTC_DRV_CMOS) || defined(CONFIG_RTC_DRV_CMOS_MODULE) || !defined(CONFIG_X86)
/* use /sys/class/rtc/rtcX/wakealarm instead; it's not ACPI-specific */
-#else
+//#else
#define HAVE_ACPI_LEGACY_ALARM
-#endif
+//#endif
#ifdef HAVE_ACPI_LEGACY_ALARM
On Mon, 01 Dec 2008 17:15:46 -0500
Mark Lord <[email protected]> wrote:
> Yeah.. bit of a nuisance that.
> I regularly apply this patch to my own kernels:
While it will certainly work, maybe it's time to switch
to /sys/class/rtc/rtcX/wakealarm .
--
Best regards,
Alessandro Zummo,
Tower Technologies - Torino, Italy
http://www.towertech.it
On Mon, Dec 01, 2008 at 12:02:06 -0500, Mark Lord wrote:
> Mark Lord wrote:
>> Fix month wrap issue with readback from /proc/acpi/alarm
>> This bug has been around *forever*.
>>
>> $ echo '2008-12-01 10:36:20' > /proc/acpi/alarm
>> $ cat /proc/acpi/alarm
>> 2008-11-01 10:36:20
>>
>> Note how the readback above shows the month incorrectly.
>> But with this patch applied, it shows the correct month (12).
> ..
>
> I should add, that the above test requires that the alarm
> be set for any day of the *next* month from the current month.
> My MythTV box does a readback test any time it programs a wakeup,
> and noticed the bug over this past weekend (2008-11-30).
Why not just use the new RTC drivers and /sys/class/rtc/rtc0/wakealarm?
MythTV already provides seconds since epoch for the wakeup time, so you
can use this value without converting it:
$ cat /usr/local/bin/myth-setwaketime
#!/bin/sh
SYSFS_WAKE_FILE="/sys/class/rtc/rtc0/wakealarm"
echo -n "Wakeup time is "
date -d @$1
if ! test -w "$SYSFS_WAKE_FILE" ; then
exit 1
fi
echo 0 > "$SYSFS_WAKE_FILE"
echo "$1" > "$SYSFS_WAKE_FILE"
Tino Keitel wrote:
> On Mon, Dec 01, 2008 at 12:02:06 -0500, Mark Lord wrote:
>> Mark Lord wrote:
>>> Fix month wrap issue with readback from /proc/acpi/alarm
>>> This bug has been around *forever*.
>>>
>>> $ echo '2008-12-01 10:36:20' > /proc/acpi/alarm
>>> $ cat /proc/acpi/alarm
>>> 2008-11-01 10:36:20
>>>
>>> Note how the readback above shows the month incorrectly.
>>> But with this patch applied, it shows the correct month (12).
>> ..
>>
>> I should add, that the above test requires that the alarm
>> be set for any day of the *next* month from the current month.
>> My MythTV box does a readback test any time it programs a wakeup,
>> and noticed the bug over this past weekend (2008-11-30).
>
> Why not just use the new RTC drivers and /sys/class/rtc/rtc0/wakealarm?
..
That question has nothing to do with fixing the bug in
the /proc/acpi/alarm code. The bug is still there whether
I personally use a different mechanism or not! :)
But in answer to your question, the new RTC drivers *also* fail
for some other reason. They did work up until the HPET integration
(back in 2.6.26 ?), but have not worked since.
My MythTV box always tries the new ones, and if the readback test
fails to agree with what was attempted, it then falls back to
the old yet more reliable /proc/acpi/alarm.
I haven't really had time to find the exact breakage point and patch it
in the new RTC code yet (I put about 10 hours into it back then,
with no useful result), but the old code does have a useful fix.
Thus this thread.
Cheers
Alessandro Zummo wrote:
> On Mon, 01 Dec 2008 17:15:46 -0500
> Mark Lord <[email protected]> wrote:
>
>> Yeah.. bit of a nuisance that.
>> I regularly apply this patch to my own kernels:
>
> While it will certainly work, maybe it's time to switch
> to /sys/class/rtc/rtcX/wakealarm .
..
Perhaps, but doing so would still leave the but in /proc/acpi/alarm,
so we need to fix it regardless.
Cheers
(repost)
Fix month wrap issue with readback from /proc/acpi/alarm
This bug has been around *forever*.
$ echo '2008-12-01 10:36:20' > /proc/acpi/alarm
$ cat /proc/acpi/alarm
2008-11-01 10:36:20
Note how the readback above shows the month incorrectly
when the alarm is set in the *next* calendar month.
But with this patch applied, it shows the correct month (12).
This patch applies/works on 2.6.28-rc6.
Probably best to queue it up for the next major cycle.
Signed-off-by: Mark Lord <[email protected]>
--- old/drivers/acpi/sleep/proc.c 2008-11-20 18:19:22.000000000 -0500
+++ rc6/drivers/acpi/sleep/proc.c 2008-12-01 17:07:51.000000000 -0500
@@ -84,12 +84,15 @@
#define HAVE_ACPI_LEGACY_ALARM
#endif
+static u32 cmos_bcd_read(int offset, int rtc_control);
+
#ifdef HAVE_ACPI_LEGACY_ALARM
static int acpi_system_alarm_seq_show(struct seq_file *seq, void *offset)
{
u32 sec, min, hr;
u32 day, mo, yr, cent = 0;
+ u32 today = 0;
unsigned char rtc_control = 0;
unsigned long flags;
@@ -97,38 +100,32 @@
spin_lock_irqsave(&rtc_lock, flags);
- sec = CMOS_READ(RTC_SECONDS_ALARM);
- min = CMOS_READ(RTC_MINUTES_ALARM);
- hr = CMOS_READ(RTC_HOURS_ALARM);
rtc_control = CMOS_READ(RTC_CONTROL);
+ sec = cmos_bcd_read(RTC_SECONDS_ALARM, rtc_control);
+ min = cmos_bcd_read(RTC_MINUTES_ALARM, rtc_control);
+ hr = cmos_bcd_read(RTC_HOURS_ALARM, rtc_control);
/* If we ever get an FACP with proper values... */
- if (acpi_gbl_FADT.day_alarm)
+ if (acpi_gbl_FADT.day_alarm) {
/* ACPI spec: only low 6 its should be cared */
day = CMOS_READ(acpi_gbl_FADT.day_alarm) & 0x3F;
- else
- day = CMOS_READ(RTC_DAY_OF_MONTH);
+ if (!(rtc_control & RTC_DM_BINARY) || RTC_ALWAYS_BCD)
+ day = bcd2bin(day);
+ } else
+ day = cmos_bcd_read(RTC_DAY_OF_MONTH, rtc_control);
if (acpi_gbl_FADT.month_alarm)
- mo = CMOS_READ(acpi_gbl_FADT.month_alarm);
- else
- mo = CMOS_READ(RTC_MONTH);
+ mo = cmos_bcd_read(acpi_gbl_FADT.month_alarm, rtc_control);
+ else {
+ mo = cmos_bcd_read(RTC_MONTH, rtc_control);
+ today = cmos_bcd_read(RTC_DAY_OF_MONTH, rtc_control);
+ }
if (acpi_gbl_FADT.century)
- cent = CMOS_READ(acpi_gbl_FADT.century);
+ cent = cmos_bcd_read(acpi_gbl_FADT.century, rtc_control);
- yr = CMOS_READ(RTC_YEAR);
+ yr = cmos_bcd_read(RTC_YEAR, rtc_control);
spin_unlock_irqrestore(&rtc_lock, flags);
- if (!(rtc_control & RTC_DM_BINARY) || RTC_ALWAYS_BCD) {
- sec = bcd2bin(sec);
- min = bcd2bin(min);
- hr = bcd2bin(hr);
- day = bcd2bin(day);
- mo = bcd2bin(mo);
- yr = bcd2bin(yr);
- cent = bcd2bin(cent);
- }
-
/* we're trusting the FADT (see above) */
if (!acpi_gbl_FADT.century)
/* If we're not trusting the FADT, we should at least make it
@@ -153,6 +150,20 @@
else
yr += cent * 100;
+ /*
+ * Show correct dates for alarms up to a month into the future.
+ * This solves issues for nearly all situations with the common
+ * 30-day alarm clocks in PC hardware.
+ */
+ if (day < today) {
+ if (mo < 12) {
+ mo += 1;
+ } else {
+ mo = 1;
+ yr += 1;
+ }
+ }
+
seq_printf(seq, "%4.4u-", yr);
(mo > 12) ? seq_puts(seq, "**-") : seq_printf(seq, "%2.2u-", mo);
(day > 31) ? seq_puts(seq, "** ") : seq_printf(seq, "%2.2u ", day);
> (repost)
thanks for the refresh, Mark.
applied to acpi-test.
--
Len Brown, Intel Open Source Technology Center