| Message ID | 4934176F.6080900@rtr.ca |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
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 <mlord@pobox.com> > > --- 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); --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to "rtc-linux". Membership options at http://groups.google.com/group/rtc-linux . Please read http://groups.google.com/group/rtc-linux/web/checklist before submitting a driver. -~----------~----~----~----~------~----~------~--~---
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 --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to "rtc-linux". Membership options at http://groups.google.com/group/rtc-linux . Please read http://groups.google.com/group/rtc-linux/web/checklist before submitting a driver. -~----------~----~----~----~------~----~------~--~---
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" --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to "rtc-linux". Membership options at http://groups.google.com/group/rtc-linux . Please read http://groups.google.com/group/rtc-linux/web/checklist before submitting a driver. -~----------~----~----~----~------~----~------~--~---
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 --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to "rtc-linux". Membership options at http://groups.google.com/group/rtc-linux . Please read http://groups.google.com/group/rtc-linux/web/checklist before submitting a driver. -~----------~----~----~----~------~----~------~--~---
--- 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);
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 <mlord@pobox.com> --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to "rtc-linux". Membership options at http://groups.google.com/group/rtc-linux . Please read http://groups.google.com/group/rtc-linux/web/checklist before submitting a driver. -~----------~----~----~----~------~----~------~--~---