From: akpm@linux-foundation.org
To: lenb@kernel.org
Cc: linux-acpi@vger.kernel.org, akpm@linux-foundation.org,
torvalds@linux-foundation.org, borntraeger@de.ibm.com,
david-b@pacbell.net, lkml@rtr.ca
Subject: [patch 8/8] acpi: fix BDC handling in drivers/acpi/sleep/proc.c
Date: Tue, 02 Oct 2007 13:24:11 -0700 [thread overview]
Message-ID: <200710022024.l92KOBXq020474@imap1.linux-foundation.org> (raw)
From: Linus Torvalds <torvalds@linux-foundation.org>
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..
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mark Lord <lkml@rtr.ca>
Cc: David Brownell <david-b@pacbell.net>
Cc: <borntraeger@de.ibm.com>
Cc: Len Brown <lenb@kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---
drivers/acpi/sleep/proc.c | 66 +++++++++++++++---------------------
1 file changed, 29 insertions(+), 37 deletions(-)
diff -puN drivers/acpi/sleep/proc.c~acpi-fix-bdc-handling-in-drivers-acpi-sleep-procc drivers/acpi/sleep/proc.c
--- a/drivers/acpi/sleep/proc.c~acpi-fix-bdc-handling-in-drivers-acpi-sleep-procc
+++ a/drivers/acpi/sleep/proc.c
@@ -194,6 +194,23 @@ static int get_date_field(char **p, u32
return 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 *fil
spin_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 *fil
CMOS_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);
_
next reply other threads:[~2007-10-02 20:25 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-10-02 20:24 akpm [this message]
2007-10-03 15:58 ` [patch 8/8] acpi: fix BDC handling in drivers/acpi/sleep/proc.c David Brownell
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=200710022024.l92KOBXq020474@imap1.linux-foundation.org \
--to=akpm@linux-foundation.org \
--cc=borntraeger@de.ibm.com \
--cc=david-b@pacbell.net \
--cc=lenb@kernel.org \
--cc=linux-acpi@vger.kernel.org \
--cc=lkml@rtr.ca \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox