[Qemu-devel] [PATCH] hw/mc146818rtc.c: Fix reading and writing of time registers

[Antoine Mathys]

This patch consolidates the bit twidling involved in reading and
writing the time registers to four functions that are used consistently.

This also has the effect of fixing bug 1090558.

Signed-off-by: Antoine Mathys <address@hidden>
---
 hw/mc146818rtc.c |  163 +++++++++++++++++++++++++++++++++---------------------
 1 file changed, 99 insertions(+), 64 deletions(-)

diff --git a/hw/mc146818rtc.c b/hw/mc146818rtc.c
index 2fb11f6..646cbd0 100644
--- a/hw/mc146818rtc.c
+++ b/hw/mc146818rtc.c
@@ -86,8 +86,14 @@ typedef struct RTCState {
 
 static void rtc_set_time(RTCState *s);
 static void rtc_update_time(RTCState *s);
+
+/* data encoding / decoding */
+static inline uint8_t rtc_encode(RTCState *s, uint8_t a);
+static inline int rtc_decode(RTCState *s, uint8_t a);
+static inline uint8_t rtc_hour_encode(RTCState *s, uint8_t hour);
+static inline int rtc_hour_decode(RTCState *s, uint8_t a);
+
 static void rtc_set_cmos(RTCState *s, const struct tm *tm);
-static inline int rtc_from_bcd(RTCState *s, int a);
 static uint64_t get_next_alarm(RTCState *s);
 
 static inline bool rtc_running(RTCState *s)
@@ -254,17 +260,84 @@ static void check_update_timer(RTCState *s)
     }
 }
 
-static inline uint8_t convert_hour(RTCState *s, uint8_t hour)
+/* data encoding / decoding */
+
+static inline uint8_t rtc_encode(RTCState *s, uint8_t a)
+{
+    if (s->cmos_data[RTC_REG_B] & REG_B_DM) {
+        return a;
+    } else {
+        return to_bcd(a);
+    }
+}
+
+static inline int rtc_decode(RTCState *s, uint8_t a)
+{
+    if ((a & 0xc0) == 0xc0) {
+        return -1;
+    }
+    if (s->cmos_data[RTC_REG_B] & REG_B_DM) {
+        return a;
+    } else {
+        return from_bcd(a);
+    }
+}
+
+/*
+  Note: The next two functions implement the following mapping between
+  the 12 hour and 24 hour formats:
+
+  0        <->    12     AM
+  1-11     <->    1 - 11 AM
+  12       <->    12     PM
+  13-23    <->    1 - 11 PM
+*/
+
+static inline uint8_t rtc_hour_encode(RTCState *s, uint8_t hour)
+{
+    uint8_t tmp;
+
+    if (s->cmos_data[RTC_REG_B] & REG_B_24H) {
+        /* 24 hour format */
+        tmp = rtc_encode(s, hour);
+    } else {
+        /* 12 hour format */
+        uint8_t h = (hour % 12) ? (hour % 12) : 12;
+        tmp = rtc_encode(s, h);
+        if (hour >= 12) {
+            tmp |= 0x80;
+        }
+    }
+    return tmp;
+}
+
+static inline int rtc_hour_decode(RTCState *s, uint8_t a)
 {
-    if (!(s->cmos_data[RTC_REG_B] & REG_B_24H)) {
+    uint8_t hour;
+
+    /* Note: in 12 hour mode we clear bit 7 before calling
+       rtc_decode(), hence we cannot rely on the later to check the
+       don't care condition. While we could skip this check in 24 hour
+       mode it is simpler to do it in any case. */
+    if ((a & 0xc0) == 0xc0) {
+        return -1;
+    }
+
+    if (s->cmos_data[RTC_REG_B] & REG_B_24H) {
+        /* 24 hour format */
+        hour = rtc_decode(s, a);
+    } else {
+        /* 12 hour format */
+        hour = rtc_decode(s, a & 0x7f);
         hour %= 12;
-        if (s->cmos_data[RTC_HOURS] & 0x80) {
+        if (a & 0x80) {
             hour += 12;
         }
     }
     return hour;
 }
 
+
 static uint64_t get_next_alarm(RTCState *s)
 {
     int32_t alarm_sec, alarm_min, alarm_hour, cur_hour, cur_min, cur_sec;
@@ -272,15 +345,13 @@ static uint64_t get_next_alarm(RTCState *s)
 
     rtc_update_time(s);
 
-    alarm_sec = rtc_from_bcd(s, s->cmos_data[RTC_SECONDS_ALARM]);
-    alarm_min = rtc_from_bcd(s, s->cmos_data[RTC_MINUTES_ALARM]);
-    alarm_hour = rtc_from_bcd(s, s->cmos_data[RTC_HOURS_ALARM]);
-    alarm_hour = alarm_hour == -1 ? -1 : convert_hour(s, alarm_hour);
+    alarm_sec = rtc_decode(s, s->cmos_data[RTC_SECONDS_ALARM]);
+    alarm_min = rtc_decode(s, s->cmos_data[RTC_MINUTES_ALARM]);
+    alarm_hour = rtc_hour_decode(s, s->cmos_data[RTC_HOURS_ALARM]);
 
-    cur_sec = rtc_from_bcd(s, s->cmos_data[RTC_SECONDS]);
-    cur_min = rtc_from_bcd(s, s->cmos_data[RTC_MINUTES]);
-    cur_hour = rtc_from_bcd(s, s->cmos_data[RTC_HOURS]);
-    cur_hour = convert_hour(s, cur_hour);
+    cur_sec = rtc_decode(s, s->cmos_data[RTC_SECONDS]);
+    cur_min = rtc_decode(s, s->cmos_data[RTC_MINUTES]);
+    cur_hour = rtc_hour_decode(s, s->cmos_data[RTC_HOURS]);
 
     if (alarm_hour == -1) {
         alarm_hour = cur_hour;
@@ -486,44 +557,17 @@ static void cmos_ioport_write(void *opaque, hwaddr addr,
     }
 }
 
-static inline int rtc_to_bcd(RTCState *s, int a)
-{
-    if (s->cmos_data[RTC_REG_B] & REG_B_DM) {
-        return a;
-    } else {
-        return ((a / 10) << 4) | (a % 10);
-    }
-}
-
-static inline int rtc_from_bcd(RTCState *s, int a)
-{
-    if ((a & 0xc0) == 0xc0) {
-        return -1;
-    }
-    if (s->cmos_data[RTC_REG_B] & REG_B_DM) {
-        return a;
-    } else {
-        return ((a >> 4) * 10) + (a & 0x0f);
-    }
-}
-
 static void rtc_get_time(RTCState *s, struct tm *tm)
 {
-    tm->tm_sec = rtc_from_bcd(s, s->cmos_data[RTC_SECONDS]);
-    tm->tm_min = rtc_from_bcd(s, s->cmos_data[RTC_MINUTES]);
-    tm->tm_hour = rtc_from_bcd(s, s->cmos_data[RTC_HOURS] & 0x7f);
-    if (!(s->cmos_data[RTC_REG_B] & REG_B_24H)) {
-        tm->tm_hour %= 12;
-        if (s->cmos_data[RTC_HOURS] & 0x80) {
-            tm->tm_hour += 12;
-        }
-    }
-    tm->tm_wday = rtc_from_bcd(s, s->cmos_data[RTC_DAY_OF_WEEK]) - 1;
-    tm->tm_mday = rtc_from_bcd(s, s->cmos_data[RTC_DAY_OF_MONTH]);
-    tm->tm_mon = rtc_from_bcd(s, s->cmos_data[RTC_MONTH]) - 1;
+    tm->tm_sec = rtc_decode(s, s->cmos_data[RTC_SECONDS]);
+    tm->tm_min = rtc_decode(s, s->cmos_data[RTC_MINUTES]);
+    tm->tm_hour = rtc_hour_decode(s, s->cmos_data[RTC_HOURS]);
+    tm->tm_wday = rtc_decode(s, s->cmos_data[RTC_DAY_OF_WEEK]) - 1;
+    tm->tm_mday = rtc_decode(s, s->cmos_data[RTC_DAY_OF_MONTH]);
+    tm->tm_mon = rtc_decode(s, s->cmos_data[RTC_MONTH]) - 1;
     tm->tm_year =
-        rtc_from_bcd(s, s->cmos_data[RTC_YEAR]) + s->base_year +
-        rtc_from_bcd(s, s->cmos_data[RTC_CENTURY]) * 100 - 1900;
+        rtc_decode(s, s->cmos_data[RTC_YEAR]) + s->base_year +
+        rtc_decode(s, s->cmos_data[RTC_CENTURY]) * 100 - 1900;
 }
 
 static void rtc_set_time(RTCState *s)
@@ -541,24 +585,15 @@ static void rtc_set_cmos(RTCState *s, const struct tm *tm)
 {
     int year;
 
-    s->cmos_data[RTC_SECONDS] = rtc_to_bcd(s, tm->tm_sec);
-    s->cmos_data[RTC_MINUTES] = rtc_to_bcd(s, tm->tm_min);
-    if (s->cmos_data[RTC_REG_B] & REG_B_24H) {
-        /* 24 hour format */
-        s->cmos_data[RTC_HOURS] = rtc_to_bcd(s, tm->tm_hour);
-    } else {
-        /* 12 hour format */
-        int h = (tm->tm_hour % 12) ? tm->tm_hour % 12 : 12;
-        s->cmos_data[RTC_HOURS] = rtc_to_bcd(s, h);
-        if (tm->tm_hour >= 12)
-            s->cmos_data[RTC_HOURS] |= 0x80;
-    }
-    s->cmos_data[RTC_DAY_OF_WEEK] = rtc_to_bcd(s, tm->tm_wday + 1);
-    s->cmos_data[RTC_DAY_OF_MONTH] = rtc_to_bcd(s, tm->tm_mday);
-    s->cmos_data[RTC_MONTH] = rtc_to_bcd(s, tm->tm_mon + 1);
+    s->cmos_data[RTC_SECONDS] = rtc_encode(s, tm->tm_sec);
+    s->cmos_data[RTC_MINUTES] = rtc_encode(s, tm->tm_min);
+    s->cmos_data[RTC_HOURS] = rtc_hour_encode(s, tm->tm_hour);
+    s->cmos_data[RTC_DAY_OF_WEEK] = rtc_encode(s, tm->tm_wday + 1);
+    s->cmos_data[RTC_DAY_OF_MONTH] = rtc_encode(s, tm->tm_mday);
+    s->cmos_data[RTC_MONTH] = rtc_encode(s, tm->tm_mon + 1);
     year = tm->tm_year + 1900 - s->base_year;
-    s->cmos_data[RTC_YEAR] = rtc_to_bcd(s, year % 100);
-    s->cmos_data[RTC_CENTURY] = rtc_to_bcd(s, year / 100);
+    s->cmos_data[RTC_YEAR] = rtc_encode(s, year % 100);
+    s->cmos_data[RTC_CENTURY] = rtc_encode(s, year / 100);
 }
 
 static void rtc_update_time(RTCState *s)
-- 
1.7.10.4