acpi: fix BDC handling in drivers/acpi/sleep/proc.c
PATCH 8/8 acpi: fix BDC handling in drivers/acpi/sleep/proc.c
From: Linus Torvalds <[email protected]>
That code is doing all the arithmetic in non-BCD mode, but the "adjust" case doesn't do a BCD_TO_BIN() on that arithmetic.
In other words, the logic you removed was certainly total crap, but there's more crap remaining inside the "if (adjust)" logic. I just suspect you never tested that part (ie using a string that starts with '+').
In fact, for the non-broken case, the old broken code *should* have been a no-op, since it did BIN_TO_BCD() followed by BCD_TO_BIN(). It then totally stupidly seemed to expect that the "adjust" case would do a BCD addition! So I don't even see why your patch should have mattered or worked!
So how about this cleanup instead? Does this work for you?
NOTE! I wanted to switch "acpi_system_alarm_seq_show()" over to this too, but that code has a strange rule:
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);and note the "& 0x3f" which is done in BCD mode. Strange. But we always write zero (which may or may not be correct). So I'd have to add a mask to the interface, and I decided it wasn't worth it until somebody talks about how that thing actually works..
---
drivers/acpi/sleep/proc.c | 66 +++++++++++++++--------------------- 1 file changed, 29 insertions(+), 37 deletions(-)
drivers/acpi/sleep/proc.c renamed from drivers/acpi/sleep/proc.c~acpi-fix-bdc-handling-in-drivers-acpi-sleep-procc +29 -37
@@ -194,6 +194,23 @@ static int get_date_field(char **p, u32return result; } +/* Read a possibly BCD register, always return binary */ +static u32 cmos_bcd_read (int offset,int rtc_control) +{ +u32 val =CMOS_READ (offset); +if (!(rtc_control &RTC_DM_BINARY ) ||RTC_ALWAYS_BCD ) +BCD_TO_BIN (val); +return val; +} + +/* Write binary value into possibly BCD register */ +static void cmos_bcd_write (u32 val,int offset,int rtc_control) +{ +if (!(rtc_control &RTC_DM_BINARY ) ||RTC_ALWAYS_BCD ) +BIN_TO_BCD (val); +CMOS_WRITE (val, offset); +} +static ssize_t acpi_system_write_alarm (struct file *file,const char __user * buffer,size_t count,loff_t * ppos)
@@ -258,35 +275,18 @@ acpi_system_write_alarm(struct file *filspin_lock_irq (&rtc_lock); rtc_control =CMOS_READ (RTC_CONTROL ); -if (!(rtc_control &RTC_DM_BINARY ) ||RTC_ALWAYS_BCD ) { -BIN_TO_BCD (yr); -BIN_TO_BCD (mo); -BIN_TO_BCD (day); -BIN_TO_BCD (hr); -BIN_TO_BCD (min); -BIN_TO_BCD (sec); - }if (adjust) { - yr +=CMOS_READ (RTC_YEAR ); - mo +=CMOS_READ (RTC_MONTH ); - day +=CMOS_READ (RTC_DAY_OF_MONTH ); - hr +=CMOS_READ (RTC_HOURS ); - min +=CMOS_READ (RTC_MINUTES ); - sec +=CMOS_READ (RTC_SECONDS ); + yr +=cmos_bcd_read (RTC_YEAR , rtc_control); + mo +=cmos_bcd_read (RTC_MONTH , rtc_control); + day +=cmos_bcd_read (RTC_DAY_OF_MONTH , rtc_control); + hr +=cmos_bcd_read (RTC_HOURS , rtc_control); + min +=cmos_bcd_read (RTC_MINUTES , rtc_control); + sec +=cmos_bcd_read (RTC_SECONDS , rtc_control); }spin_unlock_irq (&rtc_lock); -if (!(rtc_control &RTC_DM_BINARY ) ||RTC_ALWAYS_BCD ) { -BCD_TO_BIN (yr); -BCD_TO_BIN (mo); -BCD_TO_BIN (day); -BCD_TO_BIN (hr); -BCD_TO_BIN (min); -BCD_TO_BIN (sec); - } -if (sec >59 ) { min++; sec -=60 ;
@@ -307,14 +307,6 @@ acpi_system_write_alarm(struct file *fil yr++; mo -=12 ; } -if (!(rtc_control &RTC_DM_BINARY ) ||RTC_ALWAYS_BCD ) { -BIN_TO_BCD (yr); -BIN_TO_BCD (mo); -BIN_TO_BCD (day); -BIN_TO_BCD (hr); -BIN_TO_BCD (min); -BIN_TO_BCD (sec); - }spin_lock_irq (&rtc_lock);/*
@@ -326,9 +318,9 @@ acpi_system_write_alarm(struct file *filCMOS_READ (RTC_INTR_FLAGS );/* write the fields the rtc knows about */ -CMOS_WRITE (hr,RTC_HOURS_ALARM ); -CMOS_WRITE (min,RTC_MINUTES_ALARM ); -CMOS_WRITE (sec,RTC_SECONDS_ALARM ); +cmos_bcd_write (hr,RTC_HOURS_ALARM , rtc_control); +cmos_bcd_write (min,RTC_MINUTES_ALARM , rtc_control); +cmos_bcd_write (sec,RTC_SECONDS_ALARM , rtc_control);/* * If the system supports an enhanced alarm it will have non-zero
@@ -336,11 +328,11 @@ acpi_system_write_alarm(struct file *fil* to the RTC area of memory. */ if (acpi_gbl_FADT.day_alarm) -CMOS_WRITE (day, acpi_gbl_FADT.day_alarm); +cmos_bcd_write (day, acpi_gbl_FADT.day_alarm, rtc_control);if (acpi_gbl_FADT.month_alarm) -CMOS_WRITE (mo, acpi_gbl_FADT.month_alarm); +cmos_bcd_write (mo, acpi_gbl_FADT.month_alarm, rtc_control);if (acpi_gbl_FADT.century) -CMOS_WRITE (yr /100 , acpi_gbl_FADT.century); +cmos_bcd_write (yr /100 , acpi_gbl_FADT.century, rtc_control);/* enable the rtc alarm interrupt */ rtc_control |=RTC_AIE ;CMOS_WRITE (rtc_control,RTC_CONTROL );
_
By the way, another longstanding bug in this /proc/acpi/... code is:
drivers/acpi/sleep/proc.c acpi_system_write_alarm(struct file *fil · line 335
Quoted 12 lines
drivers/acpi/sleep/proc.c renamed from drivers/acpi/sleep/proc.c~acpi-fix-bdc-handling-in-drivers-acpi-sleep-procc +3
-3@@ -336,11 +328,11 @@ acpi_system_write_alarm(struct file *fil* to the RTC area of memory. */ if (acpi_gbl_FADT.day_alarm) -CMOS_WRITE (day, acpi_gbl_FADT.day_alarm); +cmos_bcd_write (day, acpi_gbl_FADT.day_alarm, rtc_control);if (acpi_gbl_FADT.month_alarm) -CMOS_WRITE (mo, acpi_gbl_FADT.month_alarm); +cmos_bcd_write (mo, acpi_gbl_FADT.month_alarm, rtc_control);if (acpi_gbl_FADT.century) -CMOS_WRITE (yr /100 , acpi_gbl_FADT.century); +cmos_bcd_write (yr /100 , acpi_gbl_FADT.century, rtc_control);
The "century" field is unrelated to the alarm. It's purely a way to distinguish 19xx vs 20xx (and 21xx, etc).
Quoted 3 lines
drivers/acpi/sleep/proc.c renamed from drivers/acpi/sleep/proc.c~acpi-fix-bdc-handling-in-drivers-acpi-sleep-procc +0
-0/* enable the rtc alarm interrupt */ rtc_control |=RTC_AIE ;CMOS_WRITE (rtc_control,RTC_CONTROL );