Skip to contentSkip to conversation

hw/ds1338.c: Minor fixes

PATCH In review Antoine Mathys 2 participants 2 messages Dec 2, 2012 – Dec 4, 2012

lore
Share & tools
lore

Antoine Mathys

author
#0 lore raw

PATCH 2/4 hw/ds1338.c: Minor fixes

Minor fixes in the handling of the RTC registers.

Signed-off-by Antoine Mathys <[email protected]>
---
 hw/ds1338.c |   15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)
hw/ds1338.c +8 -7
index 1274b22..1fb152e 100644
@@ -62,9 +62,9 @@ static void capture_current_time(DS1338State *s)
     } else {
         s->nvram[2] = to_bcd(now.tm_hour);
     }
-    s->nvram[3] = to_bcd(now.tm_wday) + 1;
+    s->nvram[3] = to_bcd(now.tm_wday + 1);
     s->nvram[4] = to_bcd(now.tm_mday);
-    s->nvram[5] = to_bcd(now.tm_mon) + 1;
+    s->nvram[5] = to_bcd(now.tm_mon + 1);
     s->nvram[6] = to_bcd(now.tm_year - 100);
 }
 

@@ -119,7 +119,8 @@ static int ds1338_send(I2CSlave *i2c, uint8_t data)
         s->addr_byte = false;
         return 0;
     }
-    if (s->ptr < 8) {
+    if (s->ptr < 7) {
+        /* Time register. */
         struct tm now;
         qemu_get_timedate(&now, s->offset);
         switch(s->ptr) {

@@ -145,7 +146,7 @@ static int ds1338_send(I2CSlave *i2c, uint8_t data)
             }
             break;
         case 3:
-            now.tm_wday = from_bcd(data & 7) - 1;
+            now.tm_wday = from_bcd(data & 0x07) - 1;
             break;
         case 4:
             now.tm_mday = from_bcd(data & 0x3f);

@@ -156,11 +157,11 @@ static int ds1338_send(I2CSlave *i2c, uint8_t data)
         case 6:
             now.tm_year = from_bcd(data) + 100;
             break;
-        case 7:
-            /* Control register. Currently ignored.  */
-            break;
         }
         s->offset = qemu_timedate_diff(&now);
+    } else if (s->ptr == 7) {
+        /* Control register. Currently ignored.  */
+        s->nvram[s->ptr] = data;
     } else {
         s->nvram[s->ptr] = data;
     }
Signature · 1 line
1.7.10.4
#1 lore raw
Quoted 12 lines · Antoine Mathys

Minor fixes in the handling of the RTC registers.

Signed-off-by Antoine Mathys <[email protected]>
---
 hw/ds1338.c |   15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)
hw/ds1338.c +0 -0
index 1274b22..1fb152e 100644

hw/ds1338.c static void capture_current_time(DS1338State *s) · line 69

Quoted 10 lines
hw/ds1338.c +2 -2
@@ -62,9 +62,9 @@ static void capture_current_time(DS1338State *s)
     } else {
         s->nvram[2] = to_bcd(now.tm_hour);
     }
-    s->nvram[3] = to_bcd(now.tm_wday) + 1;
+    s->nvram[3] = to_bcd(now.tm_wday + 1);
     s->nvram[4] = to_bcd(now.tm_mday);
-    s->nvram[5] = to_bcd(now.tm_mon) + 1;
+    s->nvram[5] = to_bcd(now.tm_mon + 1);
     s->nvram[6] = to_bcd(now.tm_year - 100);
 }

Yep, doing arithmetic after to_bcd() is pretty much always a bug.

Quoted 12 lines
hw/ds1338.c +2 -1
@@ -119,7 +119,8 @@ static int ds1338_send(I2CSlave *i2c, uint8_t data)
         s->addr_byte = false;
         return 0;
     }
-    if (s->ptr < 8) {
+    if (s->ptr < 7) {
+        /* Time register. */
         struct tm now;
         qemu_get_timedate(&now, s->offset);
         switch(s->ptr) {

hw/ds1338.c static int ds1338_send(I2CSlave *i2c, uint8_t data) · line 149

Quoted 5 lines
hw/ds1338.c +1 -1
@@ -145,7 +146,7 @@ static int ds1338_send(I2CSlave *i2c, uint8_t data)
             }
             break;
         case 3:
-            now.tm_wday = from_bcd(data & 7) - 1;
+            now.tm_wday = from_bcd(data & 0x07) - 1;

Why bother changing this?

Quoted 5 lines
hw/ds1338.c +0 -0
             break;
         case 4:
             now.tm_mday = from_bcd(data & 0x3f);

@@ -156,11 +157,11 @@ static int ds1338_send(I2CSlave *i2c, uint8_t data)
         case 6:

hw/ds1338.c static int ds1338_send(I2CSlave *i2c, uint8_t data) · line 164

Quoted 10 lines
hw/ds1338.c +3 -3
             now.tm_year = from_bcd(data) + 100;
             break;
-        case 7:
-            /* Control register. Currently ignored.  */
-            break;
         }
         s->offset = qemu_timedate_diff(&now);
+    } else if (s->ptr == 7) {
+        /* Control register. Currently ignored.  */
+        s->nvram[s->ptr] = data;

The comment says ignored but the code isn't doing that: this change will make it read-as-written. I think you should put this movement of the control register out of the switch() in the next patch, not this one. That would leave this patch with just the to_bcd() fixes, and you could give it a more precise commit message then.

Quoted 6 lines
hw/ds1338.c +0 -1
     } else {
         s->nvram[s->ptr] = data;
     }
--

1.7.10.4

-- PMM