[Qemu-devel] [PATCH v4 0/6] hw/ds1338.c

[Qemu-devel] [PATCH v4 0/6] hw/ds1338.c

[Antoine Mathys]

[Qemu-devel] [PATCH v4 1/6] hw/ds1338.c: Correct bug in conversion to BCD.

[Antoine Mathys]

Signed-off-by: Antoine Mathys <barsamin@gmail.com>
---
  hw/ds1338.c |    4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/ds1338.c b/hw/ds1338.c
index b576d56..faaa4a0 100644
--- a/hw/ds1338.c
+++ b/hw/ds1338.c
@@ -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);
  }

-- 
1.7.10.4

[Qemu-devel] [PATCH v4 2/6] hw/ds1338.c: Add definitions for various flags in the RTC registers.

[Antoine Mathys]

Signed-off-by: Antoine Mathys <barsamin@gmail.com>
---
  hw/ds1338.c |    6 ++++++
  1 file changed, 6 insertions(+)

diff --git a/hw/ds1338.c b/hw/ds1338.c
index faaa4a0..69018bc 100644
--- a/hw/ds1338.c
+++ b/hw/ds1338.c
@@ -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;
-- 
1.7.10.4

[Qemu-devel] [PATCH v4 3/6] hw/ds1338.c: Fix handling of HOURS register.

[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 <barsamin@gmail.com>
---
  hw/ds1338.c |   29 ++++++++++++++++++-----------
  1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/hw/ds1338.c b/hw/ds1338.c
index 69018bc..0f88720 100644
--- a/hw/ds1338.c
+++ b/hw/ds1338.c
@@ -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;
-- 
1.7.10.4

[Qemu-devel] [PATCH v4 4/6] hw/ds1338.c: Ensure state is properly initialized.

[Antoine Mathys]

Signed-off-by: Antoine Mathys <barsamin@gmail.com>
---
  hw/ds1338.c |   12 ++++++++++++
  1 file changed, 12 insertions(+)

diff --git a/hw/ds1338.c b/hw/ds1338.c
index 0f88720..d2f52fc 100644
--- a/hw/ds1338.c
+++ b/hw/ds1338.c
@@ -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;
  }

-- 
1.7.10.4

[Qemu-devel] [PATCH v4 5/6] hw/ds1338.c: Implement support for the control register.

[Antoine Mathys]

Signed-off-by: Antoine Mathys <barsamin@gmail.com>
---
  hw/ds1338.c |   17 +++++++++++++----
  1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/hw/ds1338.c b/hw/ds1338.c
index d2f52fc..94a2f54 100644
--- a/hw/ds1338.c
+++ b/hw/ds1338.c
@@ -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;
      }
-- 
1.7.10.4

[Qemu-devel] [PATCH v4 6/6] hw/ds1338.c: Fix handling of DAY (wday) register.

[Antoine Mathys]

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

Signed-off-by: Antoine Mathys <barsamin@gmail.com>
---
  hw/ds1338.c |   15 ++++++++++++---
  1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/hw/ds1338.c b/hw/ds1338.c
index 94a2f54..1aefa3b 100644
--- a/hw/ds1338.c
+++ b/hw/ds1338.c
@@ -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;
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH v4 0/6] hw/ds1338.c

[Peter Maydell]

On 12 December 2012 22:56, Antoine Mathys <barsamin@gmail.com> wrote:
>

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 <peter.maydell@linaro.org>
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

Re: [Qemu-devel] [PATCH v4 0/6] hw/ds1338.c

[Antoine Mathys]

On 12/13/2012 03:09 PM, Peter Maydell wrote:
>
I certainly will send further patches.

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