[patch 2.6.26-rc7] rtc_read_alarm() handles wraparound

Previous thread: [PATCH] uio-howto.tmpl: use unique output names by Mike Frysinger on Sunday, June 22, 2008 - 11:18 pm. (2 messages)

Next thread: [ANNOUNCE] Position Statement on Linux Kernel Modules by Greg KH on Monday, June 23, 2008 - 1:01 am. (8 messages)
To: lkml <linux-kernel@...>
Cc: <rtc-linux@...>, Mark Lord <lkml@...>
Date: Sunday, June 22, 2008 - 11:42 pm

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 <dbrownell@users.sourceforge.net>
---
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.
+ *
+ ...

To: Len Brown <lenb@...>, Alessandro Zummo <a.zummo@...>
Cc: David Brownell <david-b@...>, lkml <linux-kernel@...>, <rtc-linux@...>, <linux-acpi@...>
Date: Monday, December 1, 2008 - 12:57 pm

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>

--- 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, rt...

To: Mark Lord <lkml@...>
Cc: Alessandro Zummo <a.zummo@...>, David Brownell <david-b@...>, lkml <linux-kernel@...>, <rtc-linux@...>, <linux-acpi@...>
Date: Monday, December 1, 2008 - 2:18 pm

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

--

To: Len Brown <lenb@...>
Cc: Alessandro Zummo <a.zummo@...>, David Brownell <david-b@...>, lkml <linux-kernel@...>, <rtc-linux@...>, <linux-acpi@...>
Date: Monday, December 1, 2008 - 6:15 pm

..

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

--

To: <rtc-linux@...>
Cc: <lkml@...>, Len Brown <lenb@...>, Alessandro Zummo <a.zummo@...>, David Brownell <david-b@...>, lkml <linux-kernel@...>, <linux-acpi@...>
Date: Monday, December 1, 2008 - 6:32 pm

On Mon, 01 Dec 2008 17:15:46 -0500

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

--

To: Len Brown <lenb@...>
Cc: Alessandro Zummo <a.zummo@...>, David Brownell <david-b@...>, lkml <linux-kernel@...>, <rtc-linux@...>, <linux-acpi@...>
Date: Monday, December 1, 2008 - 6:13 pm

Len Brown wrote:
..

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 <mlord@pobox.com>

--- 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_MONT...

To: Len Brown <lenb@...>, Alessandro Zummo <a.zummo@...>
Cc: David Brownell <david-b@...>, lkml <linux-kernel@...>, <rtc-linux@...>, <linux-acpi@...>
Date: Monday, December 1, 2008 - 1:02 pm

..

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,
--

To: <rtc-linux@...>
Cc: <david-b@...>, lkml <linux-kernel@...>, Mark Lord <lkml@...>, <akpm@...>
Date: Wednesday, June 25, 2008 - 4:59 am

On Sun, 22 Jun 2008 20:42:13 -0700

Acked-by: Alessandro Zummo <a.zummo@towertech.it>

--

Best regards,

Alessandro Zummo,
Tower Technologies - Torino, Italy

http://www.towertech.it

--

Previous thread: [PATCH] uio-howto.tmpl: use unique output names by Mike Frysinger on Sunday, June 22, 2008 - 11:18 pm. (2 messages)

Next thread: [ANNOUNCE] Position Statement on Linux Kernel Modules by Greg KH on Monday, June 23, 2008 - 1:01 am. (8 messages)