Skip to contentSkip to conversation

hw/ds1338.c

PATCH In review Antoine Mathys 2 participants 9 messages Dec 12, 2012 – Dec 14, 2012

lore
Share & tools
lore
author
#1 lore raw

PATCH v4 1/6 hw/ds1338.c: Correct bug in conversion to BCD.

Signed-off-by Antoine Mathys <[email protected]>
---
 hw/ds1338.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
hw/ds1338.c +2 -2
index b576d56..faaa4a0 100644
@@ -57,9 +57,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);
 }
Signature · 1 line
1.7.10.4
author
#2 lore raw

PATCH v4 2/6 hw/ds1338.c: Add definitions for various flags in the RTC registers.

In reply to Antoine Mathys
Signed-off-by Antoine Mathys <[email protected]>
---
 hw/ds1338.c |    6 ++++++
 1 file changed, 6 insertions(+)
hw/ds1338.c +6 -0
index faaa4a0..69018bc 100644
@@ -17,6 +17,12 @@
  */
 #define NVRAM_SIZE 64
 
+/* Flags definitions */
+#define SECONDS_CH 0x80
+#define HOURS_12   0x40
+#define HOURS_PM   0x20
+#define CTRL_OSF   0x20
+
 typedef struct {
     I2CSlave i2c;
     int64_t offset;
Signature · 1 line
1.7.10.4
author
#3 lore raw

PATCH v4 3/6 hw/ds1338.c: Fix handling of HOURS register.

In reply to Antoine Mathys

Per the datasheet, the mapping between 12 and 24 hours modes is:

      0      <->  12   PM
      1-12   <->  1-12 AM
      13-23  <->  1-11 PM
Signed-off-by Antoine Mathys <[email protected]>
---
 hw/ds1338.c |   29 ++++++++++++++++++-----------
 1 file changed, 18 insertions(+), 11 deletions(-)
hw/ds1338.c +18 -11
index 69018bc..0f88720 100644
@@ -55,10 +55,15 @@ static void capture_current_time(DS1338State *s)
     qemu_get_timedate(&now, s->offset);
     s->nvram[0] = to_bcd(now.tm_sec);
     s->nvram[1] = to_bcd(now.tm_min);
-    if (s->nvram[2] & 0x40) {
-        s->nvram[2] = (to_bcd((now.tm_hour % 12)) + 1) | 0x40;
-        if (now.tm_hour >= 12) {
-            s->nvram[2] |= 0x20;
+    if (s->nvram[2] & HOURS_12) {
+        int tmp = now.tm_hour;
+        if (tmp == 0) {
+            tmp = 24;
+        }
+        if (tmp <= 12) {
+            s->nvram[2] = HOURS_12 | to_bcd(tmp);
+        } else {
+            s->nvram[2] = HOURS_12 | HOURS_PM | to_bcd(tmp - 12);
         }
     } else {
         s->nvram[2] = to_bcd(now.tm_hour);

@@ -132,16 +137,18 @@ static int ds1338_send(I2CSlave *i2c, uint8_t data)
             now.tm_min = from_bcd(data & 0x7f);
             break;
         case 2:
-            if (data & 0x40) {
-                if (data & 0x20) {
-                    data = from_bcd(data & 0x4f) + 11;
-                } else {
-                    data = from_bcd(data & 0x1f) - 1;
+            if (data & HOURS_12) {
+                int tmp = from_bcd(data & (HOURS_PM - 1));
+                if (data & HOURS_PM) {
+                    tmp += 12;
+                }
+                if (tmp == 24) {
+                    tmp = 0;
                 }
+                now.tm_hour = tmp;
             } else {
-                data = from_bcd(data);
+                now.tm_hour = from_bcd(data & (HOURS_12 - 1));
             }
-            now.tm_hour = data;
             break;
         case 3:
             now.tm_wday = from_bcd(data & 7) - 1;
Signature · 1 line
1.7.10.4
author
#4 lore raw

PATCH v4 4/6 hw/ds1338.c: Ensure state is properly initialized.

In reply to Antoine Mathys
Signed-off-by Antoine Mathys <[email protected]>
---
 hw/ds1338.c |   12 ++++++++++++
 1 file changed, 12 insertions(+)
hw/ds1338.c +12 -0
index 0f88720..d2f52fc 100644
@@ -179,6 +179,17 @@ static int ds1338_init(I2CSlave *i2c)
     return 0;
 }
 
+static void ds1338_reset(DeviceState *dev)
+{
+    DS1338State *s = FROM_I2C_SLAVE(DS1338State, I2C_SLAVE_FROM_QDEV(dev));
+
+    /* The clock is running and synchronized with the host */
+    s->offset = 0;
+    memset(s->nvram, 0, NVRAM_SIZE);
+    s->ptr = 0;
+    s->addr_byte = false;
+}
+
 static void ds1338_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);

@@ -188,6 +199,7 @@ static void ds1338_class_init(ObjectClass *klass, void *data)
     k->event = ds1338_event;
     k->recv = ds1338_recv;
     k->send = ds1338_send;
+    dc->reset = ds1338_reset;
     dc->vmsd = &vmstate_ds1338;
 }
Signature · 1 line
1.7.10.4
author
#5 lore raw

PATCH v4 5/6 hw/ds1338.c: Implement support for the control register.

In reply to Antoine Mathys
Signed-off-by Antoine Mathys <[email protected]>
---
 hw/ds1338.c |   17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)
hw/ds1338.c +13 -4
index d2f52fc..94a2f54 100644
@@ -125,7 +125,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) {

@@ -162,11 +163,19 @@ 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. */
+
+        /* Ensure bits 2, 3 and 6 will read back as zero. */
+        data &= 0xB3;
+
+        /* Attempting to write the OSF flag to logic 1 leaves the
+           value unchanged. */
+        data = (data & ~CTRL_OSF) | (data & s->nvram[s->ptr] & CTRL_OSF);
+
+        s->nvram[s->ptr] = data;
     } else {
         s->nvram[s->ptr] = data;
     }
Signature · 1 line
1.7.10.4
author
#6 lore raw

PATCH v4 6/6 hw/ds1338.c: Fix handling of DAY (wday) register.

In reply to Antoine Mathys

Per the datasheet, the DAY (wday) register is user defined. Implement this.

Signed-off-by Antoine Mathys <[email protected]>
---
 hw/ds1338.c |   15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)
hw/ds1338.c +12 -3
index 94a2f54..1aefa3b 100644
@@ -26,6 +26,7 @@
 typedef struct {
     I2CSlave i2c;
     int64_t offset;
+    uint8_t wday_offset;
     uint8_t nvram[NVRAM_SIZE];
     int32_t ptr;
     bool addr_byte;

@@ -33,12 +34,13 @@ typedef struct {
 
 static const VMStateDescription vmstate_ds1338 = {
     .name = "ds1338",
-    .version_id = 1,
+    .version_id = 2,
     .minimum_version_id = 1,
     .minimum_version_id_old = 1,
     .fields = (VMStateField[]) {
         VMSTATE_I2C_SLAVE(i2c, DS1338State),
         VMSTATE_INT64(offset, DS1338State),
+        VMSTATE_UINT8_V(wday_offset, DS1338State, 2),
         VMSTATE_UINT8_ARRAY(nvram, DS1338State, NVRAM_SIZE),
         VMSTATE_INT32(ptr, DS1338State),
         VMSTATE_BOOL(addr_byte, DS1338State),

@@ -68,7 +70,7 @@ 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] = (now.tm_wday + s->wday_offset) % 7 + 1;
     s->nvram[4] = to_bcd(now.tm_mday);
     s->nvram[5] = to_bcd(now.tm_mon + 1);
     s->nvram[6] = to_bcd(now.tm_year - 100);

@@ -152,7 +154,13 @@ static int ds1338_send(I2CSlave *i2c, uint8_t data)
             }
             break;
         case 3:
-            now.tm_wday = from_bcd(data & 7) - 1;
+            {
+                /* The day field is supposed to contain a value in
+                   the range 1-7. Otherwise behavior is undefined.
+                 */
+                int user_wday = (data & 7) - 1;
+                s->wday_offset = (user_wday - now.tm_wday + 7) % 7;
+            }
             break;
         case 4:
             now.tm_mday = from_bcd(data & 0x3f);

@@ -194,6 +202,7 @@ static void ds1338_reset(DeviceState *dev)
 
     /* The clock is running and synchronized with the host */
     s->offset = 0;
+    s->wday_offset = 0;
     memset(s->nvram, 0, NVRAM_SIZE);
     s->ptr = 0;
     s->addr_byte = false;
Signature · 1 line
1.7.10.4
Reviewed-by on the v4 series
#7 lore raw
In reply to Antoine Mathys
Quoted 1 line · Antoine Mathys

Empty cover letters rather defeat their purpose, by the way. git format-patch's --cover-letter option will generate you a nice cover letter template including a summary of the patches in the series and a diffstat.

Anyway, all patches

Reviewed-by Peter Maydell <[email protected]>

and I've put them into arm-devs.next. Thanks!

A couple of formatting points for future reference, if you want to send further patches to QEMU in the future:

Your email client is sending patch emails with the 'format=flowed' attribute, which means that every line in the patch beginning with a space has an extra space added to it. That means that if I save the email or if I go and download the version from patchwork ( http://patchwork.ozlabs.org/patch/205677/ ) it won't apply. It is also wrapping long lines (as can
be seen in patch 4/6.

I fixed this up this time by running a sed command to fix up the mangled patchfiles, but it's much easier on reviewers if you can just send the patches as plain unmangled text (for example git format-patch and git send-email will send patches without mangling them.)

thanks again
-- PMM

author
#8 lore raw
Quoted 1 line · Peter Maydell

I certainly will send further patches.

As you noticed I am new to git. Thanks for your patience and advice.