[Qemu-devel] [PATCH 00/14] Generalize Dallas/Maxim I2C RTC devices v2

[Qemu-devel] [PATCH 00/14] Generalize Dallas/Maxim I2C RTC devices v2

[Michael Davidsaver]

This series generalizes the ds1338 model to also support the ds1375.
As previously, only the time of day registers are modeled.  This
series is largely a do-over wrt. my previous series.  This time I
started with incremental changes from the existing ds1338 model, and only
add support for the ds1375 (which I care about).

I've added a more thorough test of the time of day function, covering
reading and setting in both 12 and 24 hour mode.  This corrects two
(practically inconsequential) bugs with the handling of 12 hour mode,
and day of the week.

In an attempt to address concerns about false positive test failures
in CI builds, instead of comparing the parts of 'struct tm' seperately
I've changed the logic of the tests to compare the difference between
the expected and actual time in seconds.  The threshold is 30 seconds
when run with 'gtester -m quick', and 1 second otherwise.

Comparision of day of the week is still exact, so there is a chance of
a false positive if the test is running across midnight UTC.

Michael Davidsaver (14):
  tests: more thorough tests of ds1338
  timer: ds1338 use registerfields.h
  timer: ds1338 persist 12-hour mode selection
  timer: ds1338 clarify HOUR handling
  timer: ds1338 change write handling
  tests: ds-rtc test 12 hour mode
  timer: ds1338 fix wday_offset handling
  tests: ds-rtc test wday offset
  timer: rename ds1338 -> dsrtc
  timer: rename file ds1338.c -> ds-rtc.c
  timer: generalize ds1338
  timer: ds-rtc handle CENTURY bit
  timer: ds-rtc model ds1375
  tests: drop ds1338-test

 default-configs/arm-softmmu.mak |   2 +-
 hw/timer/Makefile.objs          |   2 +-
 hw/timer/ds-rtc.c               | 331 ++++++++++++++++++++++++++++++++++++++++
 hw/timer/ds1338.c               | 239 -----------------------------
 tests/Makefile.include          |   6 +-
 tests/ds-rtc-common.h           |  71 +++++++++
 tests/ds-rtc-current-test.c     |  66 ++++++++
 tests/ds-rtc-set-test.c         | 171 +++++++++++++++++++++
 tests/ds1338-test.c             |  75 ---------
 9 files changed, 645 insertions(+), 318 deletions(-)
 create mode 100644 hw/timer/ds-rtc.c
 delete mode 100644 hw/timer/ds1338.c
 create mode 100644 tests/ds-rtc-common.h
 create mode 100644 tests/ds-rtc-current-test.c
 create mode 100644 tests/ds-rtc-set-test.c
 delete mode 100644 tests/ds1338-test.c

-- 
2.11.0

[Qemu-devel] [PATCH 01/14] tests: more thorough tests of ds1338

[Michael Davidsaver]

Test current time and set+get round trip.
Separate current time test from set/get
tests to avoid test order issues.

Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
---
 tests/Makefile.include      |   4 ++
 tests/ds-rtc-common.h       |  67 +++++++++++++++++++++++++
 tests/ds-rtc-current-test.c |  59 ++++++++++++++++++++++
 tests/ds-rtc-set-test.c     | 117 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 247 insertions(+)
 create mode 100644 tests/ds-rtc-common.h
 create mode 100644 tests/ds-rtc-current-test.c
 create mode 100644 tests/ds-rtc-set-test.c

diff --git a/tests/Makefile.include b/tests/Makefile.include
index eb218a9539..d256095c88 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -372,6 +372,8 @@ check-qtest-sparc64-y += tests/boot-serial-test$(EXESUF)
 
 check-qtest-arm-y = tests/tmp105-test$(EXESUF)
 check-qtest-arm-y += tests/ds1338-test$(EXESUF)
+check-qtest-arm-y += tests/ds-rtc-current-test$(EXESUF)
+check-qtest-arm-y += tests/ds-rtc-set-test$(EXESUF)
 check-qtest-arm-y += tests/m25p80-test$(EXESUF)
 gcov-files-arm-y += hw/misc/tmp105.c
 check-qtest-arm-y += tests/virtio-blk-test$(EXESUF)
@@ -771,6 +773,8 @@ tests/bios-tables-test$(EXESUF): tests/bios-tables-test.o \
 tests/pxe-test$(EXESUF): tests/pxe-test.o tests/boot-sector.o $(libqos-obj-y)
 tests/tmp105-test$(EXESUF): tests/tmp105-test.o $(libqos-omap-obj-y)
 tests/ds1338-test$(EXESUF): tests/ds1338-test.o $(libqos-imx-obj-y)
+tests/ds-rtc-current-test$(EXESUF): tests/ds-rtc-current-test.o $(libqos-imx-obj-y)
+tests/ds-rtc-set-test$(EXESUF): tests/ds-rtc-set-test.o $(libqos-imx-obj-y)
 tests/m25p80-test$(EXESUF): tests/m25p80-test.o
 tests/i440fx-test$(EXESUF): tests/i440fx-test.o $(libqos-pc-obj-y)
 tests/q35-test$(EXESUF): tests/q35-test.o $(libqos-pc-obj-y)
diff --git a/tests/ds-rtc-common.h b/tests/ds-rtc-common.h
new file mode 100644
index 0000000000..c8e6c2bc5b
--- /dev/null
+++ b/tests/ds-rtc-common.h
@@ -0,0 +1,67 @@
+/* Common code for testing of Dallas/Maxim I2C bus RTC devices
+ *
+ * Copyright (c) 2018 Michael Davidsaver
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the LICENSE file in the top-level directory.
+ */
+#ifndef DSRTCCOMMON_H
+#define DSRTCCOMMON_H
+
+#include "qemu/osdep.h"
+#include "qemu/cutils.h"
+#include "libqos/i2c.h"
+
+#define IMX25_I2C_0_BASE 0x43F80000
+#define DS1338_ADDR 0x68
+
+static I2CAdapter *i2c;
+static uint8_t addr;
+static bool use_century;
+
+/* input buffer must have at least 7 elements */
+static inline time_t rtc_parse(const uint8_t *buf)
+{
+    struct tm parts;
+
+    parts.tm_sec = from_bcd(buf[0]);
+    parts.tm_min = from_bcd(buf[1]);
+    if (buf[2] & 0x40) {
+        /* 12 hour */
+        /* HOUR register is 1-12. */
+        parts.tm_hour = from_bcd(buf[2] & 0x1f);
+        g_assert_cmpuint(parts.tm_hour, >=, 1);
+        g_assert_cmpuint(parts.tm_hour, <=, 12);
+        parts.tm_hour %= 12u; /* wrap 12 -> 0 */
+        if (buf[2] & 0x20) {
+            parts.tm_hour += 12u;
+        }
+    } else {
+        /* 24 hour */
+        parts.tm_hour = from_bcd(buf[2] & 0x3f);
+    }
+    parts.tm_wday = from_bcd(buf[3]);
+    parts.tm_mday = from_bcd(buf[4]);
+    parts.tm_mon =  from_bcd((buf[5] & 0x1f) - 1u);
+    parts.tm_year = from_bcd(buf[6]);
+    if (!use_century || (buf[5] & 0x80)) {
+        parts.tm_year += 100u;
+    }
+
+    return mktimegm(&parts);
+}
+
+static time_t rtc_gettime(void)
+{
+    uint8_t buf[7];
+
+    /* zero address pointer */
+    buf[0] = 0;
+    i2c_send(i2c, addr, buf, 1);
+    /* read back current time registers */
+    i2c_recv(i2c, addr, buf, 7);
+
+    return rtc_parse(buf);
+}
+
+#endif /* DSRTCCOMMON_H */
diff --git a/tests/ds-rtc-current-test.c b/tests/ds-rtc-current-test.c
new file mode 100644
index 0000000000..6acbbed9a6
--- /dev/null
+++ b/tests/ds-rtc-current-test.c
@@ -0,0 +1,59 @@
+/* Testing of Dallas/Maxim I2C bus RTC devices
+ *
+ * Copyright (c) 2018 Michael Davidsaver
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the LICENSE file in the top-level directory.
+ */
+#include <stdio.h>
+
+#include "qemu/osdep.h"
+#include "qemu/bcd.h"
+#include "qemu/cutils.h"
+#include "libqtest.h"
+#include "libqos/libqos.h"
+#include "libqos/i2c.h"
+
+#include "ds-rtc-common.h"
+
+/* read back and compare with current system time */
+static
+void test_rtc_current(void)
+{
+    time_t expected, actual;
+    /* relax test to limit false positives when host may be overloaded.
+     * Allow larger delta when running "-m quick"
+     */
+    time_t max_delta = g_test_slow() ? 1 : 30;
+
+    actual = time(NULL);
+    /* new second may start here */
+    expected = rtc_gettime();
+    g_assert_cmpuint(expected, <=, actual + max_delta);
+    g_assert_cmpuint(expected, >=, actual);
+}
+
+int main(int argc, char *argv[])
+{
+    int ret;
+    const char *arch = qtest_get_arch();
+    QTestState *s = NULL;
+
+    g_test_init(&argc, &argv, NULL);
+
+    if (strcmp(arch, "arm") == 0) {
+        s = qtest_start("-machine imx25-pdk");
+        i2c = imx_i2c_create(s, IMX25_I2C_0_BASE);
+        addr = DS1338_ADDR;
+        use_century = false;
+
+    }
+
+    qtest_add_func("/ds-rtc-i2c/current", test_rtc_current);
+
+    ret = g_test_run();
+
+    qtest_end();
+
+    return ret;
+}
diff --git a/tests/ds-rtc-set-test.c b/tests/ds-rtc-set-test.c
new file mode 100644
index 0000000000..35e1a36281
--- /dev/null
+++ b/tests/ds-rtc-set-test.c
@@ -0,0 +1,117 @@
+/* Testing of Dallas/Maxim I2C bus RTC devices
+ *
+ * Copyright (c) 2018 Michael Davidsaver
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the LICENSE file in the top-level directory.
+ */
+#include <stdio.h>
+
+#include "qemu/osdep.h"
+#include "qemu/bcd.h"
+#include "qemu/cutils.h"
+#include "qemu/timer.h"
+#include "libqtest.h"
+#include "libqos/libqos.h"
+#include "libqos/i2c.h"
+
+#include "ds-rtc-common.h"
+
+static uint8_t test_time_24_12am[8] = {
+    0, /* address */
+    /* Wed, 22 Nov 2017 00:30:53 +0000 */
+    0x53,
+    0x30,
+    0x00, /* 12 AM in 24 hour mode */
+    0x03, /* monday is our day 1 */
+    0x22,
+    0x11 | 0x80,
+    0x17,
+};
+
+static uint8_t test_time_24_6am[8] = {
+    0, /* address */
+    /* Wed, 22 Nov 2017 06:30:53 +0000 */
+    0x53,
+    0x30,
+    0x06, /* 6 AM in 24 hour mode */
+    0x03, /* monday is our day 1 */
+    0x22,
+    0x11 | 0x80,
+    0x17,
+};
+
+static uint8_t test_time_24_12pm[8] = {
+    0, /* address */
+    /* Wed, 22 Nov 2017 12:30:53 +0000 */
+    0x53,
+    0x30,
+    0x12, /* 12 PM in 24 hour mode */
+    0x03, /* monday is our day 1 */
+    0x22,
+    0x11 | 0x80,
+    0x17,
+};
+
+static uint8_t test_time_24_6pm[8] = {
+    0, /* address */
+    /* Wed, 22 Nov 2017 18:30:53 +0000 */
+    0x53,
+    0x30,
+    0x18, /* 6 PM in 24 hour mode */
+    0x03, /* monday is our day 1 */
+    0x22,
+    0x11 | 0x80,
+    0x17,
+};
+
+/* write in and read back known time */
+static
+void test_rtc_set(const void *raw)
+{
+    /* relax test to limit false positives when host may be overloaded.
+     * Allow larger delta when running "-m quick"
+     */
+    time_t max_delta = g_test_slow() ? 1 : 30;
+
+    const uint8_t *testtime = raw;
+    time_t expected, actual;
+
+    /* skip address pointer and parse remainder */
+    expected = rtc_parse(&testtime[1]);
+
+    i2c_send(i2c, addr, testtime, 8);
+    /* host may start new second here */
+    actual = rtc_gettime();
+
+    g_assert_cmpuint(expected, <=, actual);
+    g_assert_cmpuint(expected + max_delta, >=, actual);
+}
+
+int main(int argc, char *argv[])
+{
+    int ret;
+    const char *arch = qtest_get_arch();
+    QTestState *s = NULL;
+
+    g_test_init(&argc, &argv, NULL);
+
+    if (strcmp(arch, "arm") == 0) {
+        s = qtest_start("-display none -machine imx25-pdk");
+        i2c = imx_i2c_create(s, IMX25_I2C_0_BASE);
+        addr = DS1338_ADDR;
+        use_century = false;
+
+    }
+
+    qtest_add_data_func("/ds-rtc-i2c/set24_12am", test_time_24_12am, test_rtc_set);
+    qtest_add_data_func("/ds-rtc-i2c/set24_6am", test_time_24_6am, test_rtc_set);
+    qtest_add_data_func("/ds-rtc-i2c/set24_12pm", test_time_24_12pm, test_rtc_set);
+    qtest_add_data_func("/ds-rtc-i2c/set24_6pm", test_time_24_6pm, test_rtc_set);
+
+    ret = g_test_run();
+
+    qtest_end();
+
+    return ret;
+}
-- 
2.11.0

Re: [Qemu-devel] [PATCH 01/14] tests: more thorough tests of ds1338

[Paolo Bonzini]

On 24/03/2018 20:24, Michael Davidsaver wrote:
> Test current time and set+get round trip.
> Separate current time test from set/get
> tests to avoid test order issues.

What are these issues?  You can start a different QEMU for each test.

Paolo

> Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>

Re: [Qemu-devel] [PATCH 01/14] tests: more thorough tests of ds1338

[Michael Davidsaver]

On 03/26/2018 02:18 AM, Paolo Bonzini wrote:
> On 24/03/2018 20:24, Michael Davidsaver wrote:
>> Test current time and set+get round trip.
>> Separate current time test from set/get
>> tests to avoid test order issues.
> 
> What are these issues?  You can start a different QEMU for each test.

Quite right, and this is now what I do.  The previous iteration was
clever about avoiding this for ... no particular reason.

[Qemu-devel] [PATCH 02/14] timer: ds1338 use registerfields.h

[Michael Davidsaver]

Use names for registers and bits except
for R_CTRL which will be dealt with later,
and isn't modeled anyway.

Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
---
 hw/timer/ds1338.c | 84 ++++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 58 insertions(+), 26 deletions(-)

diff --git a/hw/timer/ds1338.c b/hw/timer/ds1338.c
index 3849b74a68..b5630e214a 100644
--- a/hw/timer/ds1338.c
+++ b/hw/timer/ds1338.c
@@ -13,6 +13,7 @@
 #include "qemu/osdep.h"
 #include "qemu-common.h"
 #include "hw/i2c/i2c.h"
+#include "hw/registerfields.h"
 #include "qemu/bcd.h"
 
 /* Size of NVRAM including both the user-accessible area and the
@@ -20,15 +21,42 @@
  */
 #define NVRAM_SIZE 64
 
-/* Flags definitions */
-#define SECONDS_CH 0x80
-#define HOURS_12   0x40
-#define HOURS_PM   0x20
 #define CTRL_OSF   0x20
 
 #define TYPE_DS1338 "ds1338"
 #define DS1338(obj) OBJECT_CHECK(DS1338State, (obj), TYPE_DS1338)
 
+/* values stored in BCD */
+/* 00-59 */
+#define R_SEC   (0x0)
+/* 00-59 */
+#define R_MIN   (0x1)
+#define R_HOUR  (0x2)
+/* 1-7 */
+#define R_WDAY  (0x3)
+/* 0-31 */
+#define R_DATE  (0x4)
+#define R_MONTH (0x5)
+/* 0-99 */
+#define R_YEAR  (0x6)
+
+#define R_CTRL  (0x7)
+
+/* use 12 hour mode when set */
+FIELD(HOUR, SET12, 6, 1)
+/* 00-23 */
+FIELD(HOUR, HOUR24, 0, 6)
+/* PM when set */
+FIELD(HOUR, AMPM, 5, 1)
+/* 1-12 (not 0-11!) */
+FIELD(HOUR, HOUR12, 0, 5)
+
+/* 1-12 */
+FIELD(MONTH, MONTH, 0, 5)
+FIELD(MONTH, CENTURY, 7, 1)
+
+FIELD(CTRL, OSF, 5, 1)
+
 typedef struct DS1338State {
     I2CSlave parent_obj;
 
@@ -61,25 +89,29 @@ static void capture_current_time(DS1338State *s)
      */
     struct tm now;
     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] & HOURS_12) {
+    s->nvram[R_SEC] = to_bcd(now.tm_sec);
+    s->nvram[R_MIN] = to_bcd(now.tm_min);
+    if (ARRAY_FIELD_EX32(s->nvram, HOUR, SET12)) {
         int tmp = now.tm_hour;
         if (tmp % 12 == 0) {
             tmp += 12;
         }
+        s->nvram[R_HOUR] = 0;
+        ARRAY_FIELD_DP32(s->nvram, HOUR, SET12, 1);
         if (tmp <= 12) {
-            s->nvram[2] = HOURS_12 | to_bcd(tmp);
+            ARRAY_FIELD_DP32(s->nvram, HOUR, AMPM, 0);
+            ARRAY_FIELD_DP32(s->nvram, HOUR, HOUR12, to_bcd(tmp));
         } else {
-            s->nvram[2] = HOURS_12 | HOURS_PM | to_bcd(tmp - 12);
+            ARRAY_FIELD_DP32(s->nvram, HOUR, AMPM, 1);
+            ARRAY_FIELD_DP32(s->nvram, HOUR, HOUR12, to_bcd(tmp - 12));
         }
     } else {
-        s->nvram[2] = to_bcd(now.tm_hour);
+        ARRAY_FIELD_DP32(s->nvram, HOUR, HOUR24, to_bcd(now.tm_hour));
     }
-    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);
+    s->nvram[R_WDAY] = (now.tm_wday + s->wday_offset) % 7 + 1;
+    s->nvram[R_DATE] = to_bcd(now.tm_mday);
+    s->nvram[R_MONTH] = to_bcd(now.tm_mon + 1);
+    s->nvram[R_YEAR] = to_bcd(now.tm_year - 100);
 }
 
 static void inc_regptr(DS1338State *s)
@@ -141,17 +173,17 @@ static int ds1338_send(I2CSlave *i2c, uint8_t data)
         struct tm now;
         qemu_get_timedate(&now, s->offset);
         switch(s->ptr) {
-        case 0:
+        case R_SEC:
             /* TODO: Implement CH (stop) bit.  */
             now.tm_sec = from_bcd(data & 0x7f);
             break;
-        case 1:
+        case R_MIN:
             now.tm_min = from_bcd(data & 0x7f);
             break;
-        case 2:
-            if (data & HOURS_12) {
-                int tmp = from_bcd(data & (HOURS_PM - 1));
-                if (data & HOURS_PM) {
+        case R_HOUR:
+            if (FIELD_EX32(data, HOUR, SET12)) {
+                int tmp = from_bcd(FIELD_EX32(data, HOUR, HOUR12));
+                if (FIELD_EX32(data, HOUR, AMPM)) {
                     tmp += 12;
                 }
                 if (tmp % 12 == 0) {
@@ -159,10 +191,10 @@ static int ds1338_send(I2CSlave *i2c, uint8_t data)
                 }
                 now.tm_hour = tmp;
             } else {
-                now.tm_hour = from_bcd(data & (HOURS_12 - 1));
+                now.tm_hour = from_bcd(FIELD_EX32(data, HOUR, HOUR24));
             }
             break;
-        case 3:
+        case R_WDAY:
             {
                 /* The day field is supposed to contain a value in
                    the range 1-7. Otherwise behavior is undefined.
@@ -171,18 +203,18 @@ static int ds1338_send(I2CSlave *i2c, uint8_t data)
                 s->wday_offset = (user_wday - now.tm_wday + 7) % 7;
             }
             break;
-        case 4:
+        case R_DATE:
             now.tm_mday = from_bcd(data & 0x3f);
             break;
-        case 5:
+        case R_MONTH:
             now.tm_mon = from_bcd(data & 0x1f) - 1;
             break;
-        case 6:
+        case R_YEAR:
             now.tm_year = from_bcd(data) + 100;
             break;
         }
         s->offset = qemu_timedate_diff(&now);
-    } else if (s->ptr == 7) {
+    } else if (s->ptr == R_CTRL) {
         /* Control register. */
 
         /* Ensure bits 2, 3 and 6 will read back as zero. */
-- 
2.11.0

Re: [Qemu-devel] [PATCH 02/14] timer: ds1338 use registerfields.h

[Peter Maydell]

On 24 March 2018 at 19:24, Michael Davidsaver <mdavidsaver@gmail.com> wrote:
> Use names for registers and bits except
> for R_CTRL which will be dealt with later,
> and isn't modeled anyway.
>
> Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>

> ---
>  hw/timer/ds1338.c | 84 ++++++++++++++++++++++++++++++++++++++-----------------
>  1 file changed, 58 insertions(+), 26 deletions(-)
> @@ -61,25 +89,29 @@ static void capture_current_time(DS1338State *s)
>       */
>      struct tm now;
>      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] & HOURS_12) {
> +    s->nvram[R_SEC] = to_bcd(now.tm_sec);
> +    s->nvram[R_MIN] = to_bcd(now.tm_min);
> +    if (ARRAY_FIELD_EX32(s->nvram, HOUR, SET12)) {
>          int tmp = now.tm_hour;
>          if (tmp % 12 == 0) {
>              tmp += 12;
>          }
> +        s->nvram[R_HOUR] = 0;
> +        ARRAY_FIELD_DP32(s->nvram, HOUR, SET12, 1);
>          if (tmp <= 12) {
> -            s->nvram[2] = HOURS_12 | to_bcd(tmp);
> +            ARRAY_FIELD_DP32(s->nvram, HOUR, AMPM, 0);

This is zeroing a bit that's already zero.

> +            ARRAY_FIELD_DP32(s->nvram, HOUR, HOUR12, to_bcd(tmp));
>          } else {
> -            s->nvram[2] = HOURS_12 | HOURS_PM | to_bcd(tmp - 12);
> +            ARRAY_FIELD_DP32(s->nvram, HOUR, AMPM, 1);
> +            ARRAY_FIELD_DP32(s->nvram, HOUR, HOUR12, to_bcd(tmp - 12));
>          }
>      } else {
> -        s->nvram[2] = to_bcd(now.tm_hour);
> +        ARRAY_FIELD_DP32(s->nvram, HOUR, HOUR24, to_bcd(now.tm_hour));

This else clause used to definitely set all the bits in nvram[2],
and now it doesn't (the s->nvram[R_HOUR] = 0 is only in the if {}).

For cases like this where we're setting the whole thing, I think
    s->nvram[R_HOUR] = R_HOUR_SET12_MASK | to_bcd(tmp);

    s->nvram[R_HOUR] = R_HOUR_SET12_MASK | R_HOUR_AMPM_MASK | to_bcd(tmp - 12);

    s->nvram[R_HOUR] = R_HOUR_HOUR24_MASK | to_bcm(now.tm_hour);

is clearer than individually setting each field, but you can
do the "clear and then set individual fields" approach if you like.

Otherwise

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

[Qemu-devel] [PATCH 03/14] timer: ds1338 persist 12-hour mode selection

[Michael Davidsaver]

Need to save HOUR[HOUR12] bit to keep
track of guest selection of 12-hour mode.
Write through current time registers to
achieve this.  Will be overwritten
by the next read/latch.

Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
---
 hw/timer/ds1338.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/hw/timer/ds1338.c b/hw/timer/ds1338.c
index b5630e214a..72a4692d60 100644
--- a/hw/timer/ds1338.c
+++ b/hw/timer/ds1338.c
@@ -224,10 +224,8 @@ static int ds1338_send(I2CSlave *i2c, uint8_t data)
            value unchanged. */
         data = (data & ~CTRL_OSF) | (data & s->nvram[s->ptr] & CTRL_OSF);
 
-        s->nvram[s->ptr] = data;
-    } else {
-        s->nvram[s->ptr] = data;
     }
+    s->nvram[s->ptr] = data;
     inc_regptr(s);
     return 0;
 }
-- 
2.11.0

Re: [Qemu-devel] [PATCH 03/14] timer: ds1338 persist 12-hour mode selection

[Peter Maydell]

On 24 March 2018 at 19:24, Michael Davidsaver <mdavidsaver@gmail.com> wrote:
> Need to save HOUR[HOUR12] bit to keep
> track of guest selection of 12-hour mode.
> Write through current time registers to
> achieve this.  Will be overwritten
> by the next read/latch.
>
> Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
> ---
>  hw/timer/ds1338.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/hw/timer/ds1338.c b/hw/timer/ds1338.c
> index b5630e214a..72a4692d60 100644
> --- a/hw/timer/ds1338.c
> +++ b/hw/timer/ds1338.c
> @@ -224,10 +224,8 @@ static int ds1338_send(I2CSlave *i2c, uint8_t data)
>             value unchanged. */
>          data = (data & ~CTRL_OSF) | (data & s->nvram[s->ptr] & CTRL_OSF);
>
> -        s->nvram[s->ptr] = data;
> -    } else {
> -        s->nvram[s->ptr] = data;
>      }
> +    s->nvram[s->ptr] = data;
>      inc_regptr(s);
>      return 0;

The code change here looks like a reasonable no-behaviour-change
simplification of the code, but it doesn't match up with
the description in the commit message ?

thanks
-- PMM

[Qemu-devel] [PATCH 04/14] timer: ds1338 clarify HOUR handling

[Michael Davidsaver]

Simplify and comment the translation between
registers and struct tm.

Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
---
 hw/timer/ds1338.c | 35 +++++++++++++++++------------------
 1 file changed, 17 insertions(+), 18 deletions(-)

diff --git a/hw/timer/ds1338.c b/hw/timer/ds1338.c
index 72a4692d60..9bcda26e51 100644
--- a/hw/timer/ds1338.c
+++ b/hw/timer/ds1338.c
@@ -88,23 +88,23 @@ static void capture_current_time(DS1338State *s)
      * which will be actually read by the data transfer operation.
      */
     struct tm now;
+    bool mode12 = ARRAY_FIELD_EX32(s->nvram, HOUR, SET12);
     qemu_get_timedate(&now, s->offset);
+
     s->nvram[R_SEC] = to_bcd(now.tm_sec);
     s->nvram[R_MIN] = to_bcd(now.tm_min);
-    if (ARRAY_FIELD_EX32(s->nvram, HOUR, SET12)) {
-        int tmp = now.tm_hour;
-        if (tmp % 12 == 0) {
-            tmp += 12;
-        }
-        s->nvram[R_HOUR] = 0;
+    s->nvram[R_HOUR] = 0;
+    if (mode12) {
+        /* map 0-23 to 1-12 am/pm */
         ARRAY_FIELD_DP32(s->nvram, HOUR, SET12, 1);
-        if (tmp <= 12) {
-            ARRAY_FIELD_DP32(s->nvram, HOUR, AMPM, 0);
-            ARRAY_FIELD_DP32(s->nvram, HOUR, HOUR12, to_bcd(tmp));
-        } else {
-            ARRAY_FIELD_DP32(s->nvram, HOUR, AMPM, 1);
-            ARRAY_FIELD_DP32(s->nvram, HOUR, HOUR12, to_bcd(tmp - 12));
+        ARRAY_FIELD_DP32(s->nvram, HOUR, AMPM, now.tm_hour >= 12u);
+        now.tm_hour %= 12u; /* wrap 0-23 to 0-11 */
+        if (now.tm_hour == 0u) {
+            /* midnight/noon stored as 12 */
+            now.tm_hour = 12u;
         }
+        ARRAY_FIELD_DP32(s->nvram, HOUR, HOUR12, to_bcd(now.tm_hour));
+
     } else {
         ARRAY_FIELD_DP32(s->nvram, HOUR, HOUR24, to_bcd(now.tm_hour));
     }
@@ -182,14 +182,13 @@ static int ds1338_send(I2CSlave *i2c, uint8_t data)
             break;
         case R_HOUR:
             if (FIELD_EX32(data, HOUR, SET12)) {
-                int tmp = from_bcd(FIELD_EX32(data, HOUR, HOUR12));
+                /* 12 hour (1-12) */
+                /* read and wrap 1-12 -> 0-11 */
+                now.tm_hour = from_bcd(FIELD_EX32(data, HOUR, HOUR12)) % 12u;
                 if (FIELD_EX32(data, HOUR, AMPM)) {
-                    tmp += 12;
+                    now.tm_hour += 12;
                 }
-                if (tmp % 12 == 0) {
-                    tmp -= 12;
-                }
-                now.tm_hour = tmp;
+
             } else {
                 now.tm_hour = from_bcd(FIELD_EX32(data, HOUR, HOUR24));
             }
-- 
2.11.0

Re: [Qemu-devel] [PATCH 04/14] timer: ds1338 clarify HOUR handling

[Peter Maydell]

On 24 March 2018 at 19:24, Michael Davidsaver <mdavidsaver@gmail.com> wrote:
> Simplify and comment the translation between
> registers and struct tm.
>
> Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
> ---
>  hw/timer/ds1338.c | 35 +++++++++++++++++------------------
>  1 file changed, 17 insertions(+), 18 deletions(-)
>
> diff --git a/hw/timer/ds1338.c b/hw/timer/ds1338.c
> index 72a4692d60..9bcda26e51 100644
> --- a/hw/timer/ds1338.c
> +++ b/hw/timer/ds1338.c
> @@ -88,23 +88,23 @@ static void capture_current_time(DS1338State *s)
>       * which will be actually read by the data transfer operation.
>       */
>      struct tm now;
> +    bool mode12 = ARRAY_FIELD_EX32(s->nvram, HOUR, SET12);
>      qemu_get_timedate(&now, s->offset);
> +
>      s->nvram[R_SEC] = to_bcd(now.tm_sec);
>      s->nvram[R_MIN] = to_bcd(now.tm_min);
> -    if (ARRAY_FIELD_EX32(s->nvram, HOUR, SET12)) {
> -        int tmp = now.tm_hour;
> -        if (tmp % 12 == 0) {
> -            tmp += 12;
> -        }
> -        s->nvram[R_HOUR] = 0;
> +    s->nvram[R_HOUR] = 0;
> +    if (mode12) {
> +        /* map 0-23 to 1-12 am/pm */
>          ARRAY_FIELD_DP32(s->nvram, HOUR, SET12, 1);
> -        if (tmp <= 12) {
> -            ARRAY_FIELD_DP32(s->nvram, HOUR, AMPM, 0);
> -            ARRAY_FIELD_DP32(s->nvram, HOUR, HOUR12, to_bcd(tmp));
> -        } else {
> -            ARRAY_FIELD_DP32(s->nvram, HOUR, AMPM, 1);
> -            ARRAY_FIELD_DP32(s->nvram, HOUR, HOUR12, to_bcd(tmp - 12));

Oh, you're rewriting all this code from the earlier patch anyway.
In that case you should definitely just have the earlier patch
match the logic that the existing code did, so that it's easier
to review as being a no-change patch.

> +        ARRAY_FIELD_DP32(s->nvram, HOUR, AMPM, now.tm_hour >= 12u);
> +        now.tm_hour %= 12u; /* wrap 0-23 to 0-11 */
> +        if (now.tm_hour == 0u) {
> +            /* midnight/noon stored as 12 */
> +            now.tm_hour = 12u;
>          }
> +        ARRAY_FIELD_DP32(s->nvram, HOUR, HOUR12, to_bcd(now.tm_hour));
> +
>      } else {
>          ARRAY_FIELD_DP32(s->nvram, HOUR, HOUR24, to_bcd(now.tm_hour));
>      }
> @@ -182,14 +182,13 @@ static int ds1338_send(I2CSlave *i2c, uint8_t data)
>              break;
>          case R_HOUR:
>              if (FIELD_EX32(data, HOUR, SET12)) {
> -                int tmp = from_bcd(FIELD_EX32(data, HOUR, HOUR12));
> +                /* 12 hour (1-12) */
> +                /* read and wrap 1-12 -> 0-11 */
> +                now.tm_hour = from_bcd(FIELD_EX32(data, HOUR, HOUR12)) % 12u;
>                  if (FIELD_EX32(data, HOUR, AMPM)) {
> -                    tmp += 12;
> +                    now.tm_hour += 12;
>                  }
> -                if (tmp % 12 == 0) {
> -                    tmp -= 12;
> -                }
> -                now.tm_hour = tmp;
> +
>              } else {
>                  now.tm_hour = from_bcd(FIELD_EX32(data, HOUR, HOUR24));
>              }
> --
> 2.11.0
>


thanks
-- PMM

[Qemu-devel] [PATCH 05/14] timer: ds1338 change write handling

[Michael Davidsaver]

instead of a read-modify-write, do direct translation
of device registers to struct tm members.

This new ds1338_update() is the reverse of
the existing capture_current_time().

Simplifies later handling of CENTURY bit in
similar Dallas RTC chips.

Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
---
 hw/timer/ds1338.c | 86 ++++++++++++++++++++++++++-----------------------------
 1 file changed, 40 insertions(+), 46 deletions(-)

diff --git a/hw/timer/ds1338.c b/hw/timer/ds1338.c
index 9bcda26e51..071c031563 100644
--- a/hw/timer/ds1338.c
+++ b/hw/timer/ds1338.c
@@ -159,6 +159,42 @@ static int ds1338_recv(I2CSlave *i2c)
     return res;
 }
 
+/* call after guest writes to current time registers
+ * to re-compute our offset from host time.
+ */
+static void ds1338_update(DS1338State *s)
+{
+
+    struct tm now;
+    memset(&now, 0, sizeof(now));
+
+    /* TODO: Implement CH (stop) bit?  */
+    now.tm_sec = from_bcd(s->nvram[R_SEC] & 0x7f);
+    now.tm_min = from_bcd(s->nvram[R_MIN] & 0x7f);
+    if (ARRAY_FIELD_EX32(s->nvram, HOUR, SET12)) {
+        /* 12 hour (1-12) */
+        /* read and wrap 1-12 -> 0-11 */
+        now.tm_hour = from_bcd(ARRAY_FIELD_EX32(s->nvram, HOUR, HOUR12)) % 12u;
+        if (ARRAY_FIELD_EX32(s->nvram, HOUR, AMPM)) {
+            now.tm_hour += 12;
+        }
+
+    } else {
+        now.tm_hour = from_bcd(ARRAY_FIELD_EX32(s->nvram, HOUR, HOUR24));
+    }
+    {
+        /* The day field is supposed to contain a value in
+               the range 1-7. Otherwise behavior is undefined.
+             */
+        int user_wday = (s->nvram[R_WDAY] & 7) - 1;
+        s->wday_offset = (user_wday - now.tm_wday + 7) % 7;
+    }
+    now.tm_mday = from_bcd(s->nvram[R_DATE] & 0x3f);
+    now.tm_mon = from_bcd(s->nvram[R_MONTH] & 0x1f) - 1;
+    now.tm_year = from_bcd(s->nvram[R_YEAR]) + 100;
+    s->offset = qemu_timedate_diff(&now);
+}
+
 static int ds1338_send(I2CSlave *i2c, uint8_t data)
 {
     DS1338State *s = DS1338(i2c);
@@ -168,52 +204,7 @@ static int ds1338_send(I2CSlave *i2c, uint8_t data)
         s->addr_byte = false;
         return 0;
     }
-    if (s->ptr < 7) {
-        /* Time register. */
-        struct tm now;
-        qemu_get_timedate(&now, s->offset);
-        switch(s->ptr) {
-        case R_SEC:
-            /* TODO: Implement CH (stop) bit.  */
-            now.tm_sec = from_bcd(data & 0x7f);
-            break;
-        case R_MIN:
-            now.tm_min = from_bcd(data & 0x7f);
-            break;
-        case R_HOUR:
-            if (FIELD_EX32(data, HOUR, SET12)) {
-                /* 12 hour (1-12) */
-                /* read and wrap 1-12 -> 0-11 */
-                now.tm_hour = from_bcd(FIELD_EX32(data, HOUR, HOUR12)) % 12u;
-                if (FIELD_EX32(data, HOUR, AMPM)) {
-                    now.tm_hour += 12;
-                }
-
-            } else {
-                now.tm_hour = from_bcd(FIELD_EX32(data, HOUR, HOUR24));
-            }
-            break;
-        case R_WDAY:
-            {
-                /* 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 R_DATE:
-            now.tm_mday = from_bcd(data & 0x3f);
-            break;
-        case R_MONTH:
-            now.tm_mon = from_bcd(data & 0x1f) - 1;
-            break;
-        case R_YEAR:
-            now.tm_year = from_bcd(data) + 100;
-            break;
-        }
-        s->offset = qemu_timedate_diff(&now);
-    } else if (s->ptr == R_CTRL) {
+    if (s->ptr == R_CTRL) {
         /* Control register. */
 
         /* Ensure bits 2, 3 and 6 will read back as zero. */
@@ -225,6 +216,9 @@ static int ds1338_send(I2CSlave *i2c, uint8_t data)
 
     }
     s->nvram[s->ptr] = data;
+    if (s->ptr <= R_YEAR) {
+        ds1338_update(s);
+    }
     inc_regptr(s);
     return 0;
 }
-- 
2.11.0

Re: [Qemu-devel] [PATCH 05/14] timer: ds1338 change write handling

[Peter Maydell]

On 24 March 2018 at 19:24, Michael Davidsaver <mdavidsaver@gmail.com> wrote:
> instead of a read-modify-write, do direct translation
> of device registers to struct tm members.
>
> This new ds1338_update() is the reverse of
> the existing capture_current_time().
>
> Simplifies later handling of CENTURY bit in
> similar Dallas RTC chips.
>
> Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
> ---
>  hw/timer/ds1338.c | 86 ++++++++++++++++++++++++++-----------------------------
>  1 file changed, 40 insertions(+), 46 deletions(-)
>
> diff --git a/hw/timer/ds1338.c b/hw/timer/ds1338.c
> index 9bcda26e51..071c031563 100644
> --- a/hw/timer/ds1338.c
> +++ b/hw/timer/ds1338.c
> @@ -159,6 +159,42 @@ static int ds1338_recv(I2CSlave *i2c)
>      return res;
>  }
>
> +/* call after guest writes to current time registers
> + * to re-compute our offset from host time.
> + */
> +static void ds1338_update(DS1338State *s)
> +{
> +
> +    struct tm now;
> +    memset(&now, 0, sizeof(now));

You can zero initialize a struct with just
       struct tm now = {};

so you don't need the memset.

> +
> +    /* TODO: Implement CH (stop) bit?  */
> +    now.tm_sec = from_bcd(s->nvram[R_SEC] & 0x7f);
> +    now.tm_min = from_bcd(s->nvram[R_MIN] & 0x7f);
> +    if (ARRAY_FIELD_EX32(s->nvram, HOUR, SET12)) {
> +        /* 12 hour (1-12) */
> +        /* read and wrap 1-12 -> 0-11 */
> +        now.tm_hour = from_bcd(ARRAY_FIELD_EX32(s->nvram, HOUR, HOUR12)) % 12u;
> +        if (ARRAY_FIELD_EX32(s->nvram, HOUR, AMPM)) {
> +            now.tm_hour += 12;
> +        }
> +
> +    } else {
> +        now.tm_hour = from_bcd(ARRAY_FIELD_EX32(s->nvram, HOUR, HOUR24));
> +    }
> +    {
> +        /* The day field is supposed to contain a value in
> +               the range 1-7. Otherwise behavior is undefined.
> +             */

This patch is a good opportunity to bring the format of this
comment in line with the others in the file, ie
  /* multiple lines
   * all have a star on the left
   */

> +        int user_wday = (s->nvram[R_WDAY] & 7) - 1;

I would just declare user_wday at the top of the function, and
then you can drop the local {} scope.

> +        s->wday_offset = (user_wday - now.tm_wday + 7) % 7;

This line copied from the previous code is still assuming that
'now' contains valid date information, so it won't produce the
right answer. What you need to do is
 (1) move this down to after you set s->offset,
     so that we have the new offset value, and then
 (2) call qemu_get_timedate(&now, s->offset);
     to get an updated 'now' struct with a valid now.tm_wday;
 (3) then you can do
    s->wday_offset = (user_wday - now.tm_wday + 7) % 7;

(I think that's right, anyway -- you should check my working...)

> +    }
> +    now.tm_mday = from_bcd(s->nvram[R_DATE] & 0x3f);
> +    now.tm_mon = from_bcd(s->nvram[R_MONTH] & 0x1f) - 1;
> +    now.tm_year = from_bcd(s->nvram[R_YEAR]) + 100;
> +    s->offset = qemu_timedate_diff(&now);
> +

The rest of this looks good.

thanks
-- PMM

[Qemu-devel] [PATCH 06/14] tests: ds-rtc test 12 hour mode

[Michael Davidsaver]

Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
---
 tests/ds-rtc-set-test.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/tests/ds-rtc-set-test.c b/tests/ds-rtc-set-test.c
index 35e1a36281..c48406ee2c 100644
--- a/tests/ds-rtc-set-test.c
+++ b/tests/ds-rtc-set-test.c
@@ -29,6 +29,18 @@ static uint8_t test_time_24_12am[8] = {
     0x17,
 };
 
+static uint8_t test_time_12_12am[8] = {
+    0, /* address */
+    /* Wed, 22 Nov 2017 00:30:53 +0000 */
+    0x53,
+    0x30,
+    0x52, /* 12 AM in 12 hour mode */
+    0x03, /* monday is our day 1 */
+    0x22,
+    0x11 | 0x80,
+    0x17,
+};
+
 static uint8_t test_time_24_6am[8] = {
     0, /* address */
     /* Wed, 22 Nov 2017 06:30:53 +0000 */
@@ -41,6 +53,18 @@ static uint8_t test_time_24_6am[8] = {
     0x17,
 };
 
+static uint8_t test_time_12_6am[8] = {
+    0, /* address */
+    /* Wed, 22 Nov 2017 06:30:53 +0000 */
+    0x53,
+    0x30,
+    0x46, /* 6 AM in 12 hour mode */
+    0x03, /* monday is our day 1 */
+    0x22,
+    0x11 | 0x80,
+    0x17,
+};
+
 static uint8_t test_time_24_12pm[8] = {
     0, /* address */
     /* Wed, 22 Nov 2017 12:30:53 +0000 */
@@ -53,6 +77,18 @@ static uint8_t test_time_24_12pm[8] = {
     0x17,
 };
 
+static uint8_t test_time_12_12pm[8] = {
+    0, /* address */
+    /* Wed, 22 Nov 2017 12:30:53 +0000 */
+    0x53,
+    0x30,
+    0x72, /* 12 PM in 24 hour mode */
+    0x03, /* monday is our day 1 */
+    0x22,
+    0x11 | 0x80,
+    0x17,
+};
+
 static uint8_t test_time_24_6pm[8] = {
     0, /* address */
     /* Wed, 22 Nov 2017 18:30:53 +0000 */
@@ -65,6 +101,18 @@ static uint8_t test_time_24_6pm[8] = {
     0x17,
 };
 
+static uint8_t test_time_12_6pm[8] = {
+    0, /* address */
+    /* Wed, 22 Nov 2017 18:30:53 +0000 */
+    0x53,
+    0x30,
+    0x66, /* 6 PM in 12 hour mode */
+    0x03, /* monday is our day 1 */
+    0x22,
+    0x11 | 0x80,
+    0x17,
+};
+
 /* write in and read back known time */
 static
 void test_rtc_set(const void *raw)
@@ -108,6 +156,10 @@ int main(int argc, char *argv[])
     qtest_add_data_func("/ds-rtc-i2c/set24_6am", test_time_24_6am, test_rtc_set);
     qtest_add_data_func("/ds-rtc-i2c/set24_12pm", test_time_24_12pm, test_rtc_set);
     qtest_add_data_func("/ds-rtc-i2c/set24_6pm", test_time_24_6pm, test_rtc_set);
+    qtest_add_data_func("/ds-rtc-i2c/set12_12am", test_time_12_12am, test_rtc_set);
+    qtest_add_data_func("/ds-rtc-i2c/set12_6am", test_time_12_6am, test_rtc_set);
+    qtest_add_data_func("/ds-rtc-i2c/set12_12pm", test_time_12_12pm, test_rtc_set);
+    qtest_add_data_func("/ds-rtc-i2c/set12_6pm", test_time_12_6pm, test_rtc_set);
 
     ret = g_test_run();
 
-- 
2.11.0

[Qemu-devel] [PATCH 07/14] timer: ds1338 fix wday_offset handling

[Michael Davidsaver]

Correctly handle different real weekday in
guest and host timezones.
Allow guest to use any day as start of week
(day 1).  eg. Monday instead of Sunday.

Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
---
 hw/timer/ds1338.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/hw/timer/ds1338.c b/hw/timer/ds1338.c
index 071c031563..a969b5c8ba 100644
--- a/hw/timer/ds1338.c
+++ b/hw/timer/ds1338.c
@@ -108,7 +108,10 @@ static void capture_current_time(DS1338State *s)
     } else {
         ARRAY_FIELD_DP32(s->nvram, HOUR, HOUR24, to_bcd(now.tm_hour));
     }
-    s->nvram[R_WDAY] = (now.tm_wday + s->wday_offset) % 7 + 1;
+    s->nvram[R_WDAY] = (now.tm_wday + s->wday_offset) % 7;
+    if (s->nvram[R_WDAY] == 0) {
+        s->nvram[R_WDAY] = 7;
+    }
     s->nvram[R_DATE] = to_bcd(now.tm_mday);
     s->nvram[R_MONTH] = to_bcd(now.tm_mon + 1);
     s->nvram[R_YEAR] = to_bcd(now.tm_year - 100);
@@ -182,17 +185,22 @@ static void ds1338_update(DS1338State *s)
     } else {
         now.tm_hour = from_bcd(ARRAY_FIELD_EX32(s->nvram, HOUR, HOUR24));
     }
-    {
-        /* The day field is supposed to contain a value in
-               the range 1-7. Otherwise behavior is undefined.
-             */
-        int user_wday = (s->nvram[R_WDAY] & 7) - 1;
-        s->wday_offset = (user_wday - now.tm_wday + 7) % 7;
-    }
+    now.tm_wday = from_bcd(s->nvram[R_WDAY]) - 1u;
     now.tm_mday = from_bcd(s->nvram[R_DATE] & 0x3f);
     now.tm_mon = from_bcd(s->nvram[R_MONTH] & 0x1f) - 1;
     now.tm_year = from_bcd(s->nvram[R_YEAR]) + 100;
     s->offset = qemu_timedate_diff(&now);
+
+    {
+        /* Round trip to get real wday_offset based on time delta and
+         * ref. timezone.
+         * Race if midnight (in ref. timezone) happens here.
+         */
+        int user_wday = now.tm_wday;
+        qemu_get_timedate(&now, s->offset);
+
+        s->wday_offset = (user_wday - now.tm_wday) % 7 + 1;
+    }
 }
 
 static int ds1338_send(I2CSlave *i2c, uint8_t data)
-- 
2.11.0

Re: [Qemu-devel] [PATCH 07/14] timer: ds1338 fix wday_offset handling

[Peter Maydell]

On 24 March 2018 at 19:24, Michael Davidsaver <mdavidsaver@gmail.com> wrote:
> Correctly handle different real weekday in
> guest and host timezones.
> Allow guest to use any day as start of week
> (day 1).  eg. Monday instead of Sunday.
>
> Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
> ---
>  hw/timer/ds1338.c | 24 ++++++++++++++++--------
>  1 file changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/hw/timer/ds1338.c b/hw/timer/ds1338.c
> index 071c031563..a969b5c8ba 100644
> --- a/hw/timer/ds1338.c
> +++ b/hw/timer/ds1338.c
> @@ -108,7 +108,10 @@ static void capture_current_time(DS1338State *s)
>      } else {
>          ARRAY_FIELD_DP32(s->nvram, HOUR, HOUR24, to_bcd(now.tm_hour));
>      }
> -    s->nvram[R_WDAY] = (now.tm_wday + s->wday_offset) % 7 + 1;
> +    s->nvram[R_WDAY] = (now.tm_wday + s->wday_offset) % 7;
> +    if (s->nvram[R_WDAY] == 0) {
> +        s->nvram[R_WDAY] = 7;
> +    }
>      s->nvram[R_DATE] = to_bcd(now.tm_mday);
>      s->nvram[R_MONTH] = to_bcd(now.tm_mon + 1);
>      s->nvram[R_YEAR] = to_bcd(now.tm_year - 100);
> @@ -182,17 +185,22 @@ static void ds1338_update(DS1338State *s)
>      } else {
>          now.tm_hour = from_bcd(ARRAY_FIELD_EX32(s->nvram, HOUR, HOUR24));
>      }
> -    {
> -        /* The day field is supposed to contain a value in
> -               the range 1-7. Otherwise behavior is undefined.
> -             */
> -        int user_wday = (s->nvram[R_WDAY] & 7) - 1;
> -        s->wday_offset = (user_wday - now.tm_wday + 7) % 7;
> -    }
> +    now.tm_wday = from_bcd(s->nvram[R_WDAY]) - 1u;
>      now.tm_mday = from_bcd(s->nvram[R_DATE] & 0x3f);

You don't need to set now.tm_wday -- qemu_timedate_diff()
will not use that field in the structure you hand it.

>      now.tm_mon = from_bcd(s->nvram[R_MONTH] & 0x1f) - 1;
>      now.tm_year = from_bcd(s->nvram[R_YEAR]) + 100;
>      s->offset = qemu_timedate_diff(&now);
> +
> +    {
> +        /* Round trip to get real wday_offset based on time delta and
> +         * ref. timezone.
> +         * Race if midnight (in ref. timezone) happens here.
> +         */
> +        int user_wday = now.tm_wday;
> +        qemu_get_timedate(&now, s->offset);
> +
> +        s->wday_offset = (user_wday - now.tm_wday) % 7 + 1;
> +    }
>  }

Ah, this part is fixing the bug I pointed out in the earlier patch.
You should just fold that bug fix into it.

thanks
-- PMM

[Qemu-devel] [PATCH 08/14] tests: ds-rtc test wday offset

[Michael Davidsaver]

Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
---
 tests/ds-rtc-common.h       | 10 +++++++---
 tests/ds-rtc-current-test.c |  9 ++++++++-
 tests/ds-rtc-set-test.c     |  6 ++++--
 3 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/tests/ds-rtc-common.h b/tests/ds-rtc-common.h
index c8e6c2bc5b..633131c55f 100644
--- a/tests/ds-rtc-common.h
+++ b/tests/ds-rtc-common.h
@@ -20,7 +20,7 @@ static uint8_t addr;
 static bool use_century;
 
 /* input buffer must have at least 7 elements */
-static inline time_t rtc_parse(const uint8_t *buf)
+static inline time_t rtc_parse(const uint8_t *buf, unsigned *wday)
 {
     struct tm parts;
 
@@ -48,10 +48,14 @@ static inline time_t rtc_parse(const uint8_t *buf)
         parts.tm_year += 100u;
     }
 
+    if (wday) {
+        *wday = parts.tm_wday;
+    }
+
     return mktimegm(&parts);
 }
 
-static time_t rtc_gettime(void)
+static time_t rtc_gettime(unsigned *wday)
 {
     uint8_t buf[7];
 
@@ -61,7 +65,7 @@ static time_t rtc_gettime(void)
     /* read back current time registers */
     i2c_recv(i2c, addr, buf, 7);
 
-    return rtc_parse(buf);
+    return rtc_parse(buf, wday);
 }
 
 #endif /* DSRTCCOMMON_H */
diff --git a/tests/ds-rtc-current-test.c b/tests/ds-rtc-current-test.c
index 6acbbed9a6..7dc3202261 100644
--- a/tests/ds-rtc-current-test.c
+++ b/tests/ds-rtc-current-test.c
@@ -20,17 +20,24 @@
 static
 void test_rtc_current(void)
 {
+    struct tm tm_actual;
     time_t expected, actual;
     /* relax test to limit false positives when host may be overloaded.
      * Allow larger delta when running "-m quick"
      */
     time_t max_delta = g_test_slow() ? 1 : 30;
 
+    unsigned wday_expect;
+
     actual = time(NULL);
     /* new second may start here */
-    expected = rtc_gettime();
+    expected = rtc_gettime(&wday_expect);
+
+    gmtime_r(&actual, &tm_actual);
+
     g_assert_cmpuint(expected, <=, actual + max_delta);
     g_assert_cmpuint(expected, >=, actual);
+    g_assert_cmpuint(wday_expect, ==, tm_actual.tm_wday);
 }
 
 int main(int argc, char *argv[])
diff --git a/tests/ds-rtc-set-test.c b/tests/ds-rtc-set-test.c
index c48406ee2c..12aeb2580a 100644
--- a/tests/ds-rtc-set-test.c
+++ b/tests/ds-rtc-set-test.c
@@ -124,16 +124,18 @@ void test_rtc_set(const void *raw)
 
     const uint8_t *testtime = raw;
     time_t expected, actual;
+    unsigned wday_expect, wday_actual;
 
     /* skip address pointer and parse remainder */
-    expected = rtc_parse(&testtime[1]);
+    expected = rtc_parse(&testtime[1], &wday_expect);
 
     i2c_send(i2c, addr, testtime, 8);
     /* host may start new second here */
-    actual = rtc_gettime();
+    actual = rtc_gettime(&wday_actual);
 
     g_assert_cmpuint(expected, <=, actual);
     g_assert_cmpuint(expected + max_delta, >=, actual);
+    g_assert_cmpuint(wday_expect, ==, wday_actual);
 }
 
 int main(int argc, char *argv[])
-- 
2.11.0

[Qemu-devel] [PATCH 09/14] timer: rename ds1338 -> dsrtc

[Michael Davidsaver]

Prepare to generalize with a more generic
name.

Keep device name and vmstate name "ds1338"
for compatibility.

Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
---
 hw/timer/ds1338.c | 74 +++++++++++++++++++++++++++----------------------------
 1 file changed, 37 insertions(+), 37 deletions(-)

diff --git a/hw/timer/ds1338.c b/hw/timer/ds1338.c
index a969b5c8ba..28f788dd8e 100644
--- a/hw/timer/ds1338.c
+++ b/hw/timer/ds1338.c
@@ -1,5 +1,5 @@
 /*
- * MAXIM DS1338 I2C RTC+NVRAM
+ * MAXIM DSRTC I2C RTC+NVRAM
  *
  * Copyright (c) 2009 CodeSourcery.
  * Written by Paul Brook
@@ -23,8 +23,8 @@
 
 #define CTRL_OSF   0x20
 
-#define TYPE_DS1338 "ds1338"
-#define DS1338(obj) OBJECT_CHECK(DS1338State, (obj), TYPE_DS1338)
+#define TYPE_DSRTC "ds1338"
+#define DSRTC(obj) OBJECT_CHECK(DSRTCState, (obj), TYPE_DSRTC)
 
 /* values stored in BCD */
 /* 00-59 */
@@ -57,7 +57,7 @@ FIELD(MONTH, CENTURY, 7, 1)
 
 FIELD(CTRL, OSF, 5, 1)
 
-typedef struct DS1338State {
+typedef struct DSRTCState {
     I2CSlave parent_obj;
 
     int64_t offset;
@@ -65,24 +65,24 @@ typedef struct DS1338State {
     uint8_t nvram[NVRAM_SIZE];
     int32_t ptr;
     bool addr_byte;
-} DS1338State;
+} DSRTCState;
 
-static const VMStateDescription vmstate_ds1338 = {
+static const VMStateDescription vmstate_dsrtc = {
     .name = "ds1338",
     .version_id = 2,
     .minimum_version_id = 1,
     .fields = (VMStateField[]) {
-        VMSTATE_I2C_SLAVE(parent_obj, 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),
+        VMSTATE_I2C_SLAVE(parent_obj, DSRTCState),
+        VMSTATE_INT64(offset, DSRTCState),
+        VMSTATE_UINT8_V(wday_offset, DSRTCState, 2),
+        VMSTATE_UINT8_ARRAY(nvram, DSRTCState, NVRAM_SIZE),
+        VMSTATE_INT32(ptr, DSRTCState),
+        VMSTATE_BOOL(addr_byte, DSRTCState),
         VMSTATE_END_OF_LIST()
     }
 };
 
-static void capture_current_time(DS1338State *s)
+static void capture_current_time(DSRTCState *s)
 {
     /* Capture the current time into the secondary registers
      * which will be actually read by the data transfer operation.
@@ -117,7 +117,7 @@ static void capture_current_time(DS1338State *s)
     s->nvram[R_YEAR] = to_bcd(now.tm_year - 100);
 }
 
-static void inc_regptr(DS1338State *s)
+static void inc_regptr(DSRTCState *s)
 {
     /* The register pointer wraps around after 0x3F; wraparound
      * causes the current time/date to be retransferred into
@@ -129,9 +129,9 @@ static void inc_regptr(DS1338State *s)
     }
 }
 
-static int ds1338_event(I2CSlave *i2c, enum i2c_event event)
+static int dsrtc_event(I2CSlave *i2c, enum i2c_event event)
 {
-    DS1338State *s = DS1338(i2c);
+    DSRTCState *s = DSRTC(i2c);
 
     switch (event) {
     case I2C_START_RECV:
@@ -152,9 +152,9 @@ static int ds1338_event(I2CSlave *i2c, enum i2c_event event)
     return 0;
 }
 
-static int ds1338_recv(I2CSlave *i2c)
+static int dsrtc_recv(I2CSlave *i2c)
 {
-    DS1338State *s = DS1338(i2c);
+    DSRTCState *s = DSRTC(i2c);
     uint8_t res;
 
     res  = s->nvram[s->ptr];
@@ -165,7 +165,7 @@ static int ds1338_recv(I2CSlave *i2c)
 /* call after guest writes to current time registers
  * to re-compute our offset from host time.
  */
-static void ds1338_update(DS1338State *s)
+static void dsrtc_update(DSRTCState *s)
 {
 
     struct tm now;
@@ -203,9 +203,9 @@ static void ds1338_update(DS1338State *s)
     }
 }
 
-static int ds1338_send(I2CSlave *i2c, uint8_t data)
+static int dsrtc_send(I2CSlave *i2c, uint8_t data)
 {
-    DS1338State *s = DS1338(i2c);
+    DSRTCState *s = DSRTC(i2c);
 
     if (s->addr_byte) {
         s->ptr = data & (NVRAM_SIZE - 1);
@@ -225,15 +225,15 @@ static int ds1338_send(I2CSlave *i2c, uint8_t data)
     }
     s->nvram[s->ptr] = data;
     if (s->ptr <= R_YEAR) {
-        ds1338_update(s);
+        dsrtc_update(s);
     }
     inc_regptr(s);
     return 0;
 }
 
-static void ds1338_reset(DeviceState *dev)
+static void dsrtc_reset(DeviceState *dev)
 {
-    DS1338State *s = DS1338(dev);
+    DSRTCState *s = DSRTC(dev);
 
     /* The clock is running and synchronized with the host */
     s->offset = 0;
@@ -243,28 +243,28 @@ static void ds1338_reset(DeviceState *dev)
     s->addr_byte = false;
 }
 
-static void ds1338_class_init(ObjectClass *klass, void *data)
+static void dsrtc_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     I2CSlaveClass *k = I2C_SLAVE_CLASS(klass);
 
-    k->event = ds1338_event;
-    k->recv = ds1338_recv;
-    k->send = ds1338_send;
-    dc->reset = ds1338_reset;
-    dc->vmsd = &vmstate_ds1338;
+    k->event = dsrtc_event;
+    k->recv = dsrtc_recv;
+    k->send = dsrtc_send;
+    dc->reset = dsrtc_reset;
+    dc->vmsd = &vmstate_dsrtc;
 }
 
-static const TypeInfo ds1338_info = {
-    .name          = TYPE_DS1338,
+static const TypeInfo dsrtc_info = {
+    .name          = TYPE_DSRTC,
     .parent        = TYPE_I2C_SLAVE,
-    .instance_size = sizeof(DS1338State),
-    .class_init    = ds1338_class_init,
+    .instance_size = sizeof(DSRTCState),
+    .class_init    = dsrtc_class_init,
 };
 
-static void ds1338_register_types(void)
+static void dsrtc_register_types(void)
 {
-    type_register_static(&ds1338_info);
+    type_register_static(&dsrtc_info);
 }
 
-type_init(ds1338_register_types)
+type_init(dsrtc_register_types)
-- 
2.11.0

Re: [Qemu-devel] [PATCH 09/14] timer: rename ds1338 -> dsrtc

[Peter Maydell]

On 24 March 2018 at 19:24, Michael Davidsaver <mdavidsaver@gmail.com> wrote:
> Prepare to generalize with a more generic
> name.
>
> Keep device name and vmstate name "ds1338"
> for compatibility.
>
> Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
> ---
>  hw/timer/ds1338.c | 74 +++++++++++++++++++++++++++----------------------------
>  1 file changed, 37 insertions(+), 37 deletions(-)
>
> diff --git a/hw/timer/ds1338.c b/hw/timer/ds1338.c
> index a969b5c8ba..28f788dd8e 100644
> --- a/hw/timer/ds1338.c
> +++ b/hw/timer/ds1338.c
> @@ -1,5 +1,5 @@
>  /*
> - * MAXIM DS1338 I2C RTC+NVRAM
> + * MAXIM DSRTC I2C RTC+NVRAM
>   *

I don't think the search-and-replace makes sense on this comment;
it should probably be "MAXIM DS13xx I2C RTC" or "DSxxxx" or
something, since we're trying to identify a product name/family.

Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

[Qemu-devel] [PATCH 10/14] timer: rename file ds1338.c -> ds-rtc.c

[Michael Davidsaver]

Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
---
 default-configs/arm-softmmu.mak | 2 +-
 hw/timer/Makefile.objs          | 2 +-
 hw/timer/{ds1338.c => ds-rtc.c} | 0
 3 files changed, 2 insertions(+), 2 deletions(-)
 rename hw/timer/{ds1338.c => ds-rtc.c} (100%)

diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index dd29e741c2..0afffa2a8a 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -33,7 +33,7 @@ CONFIG_SMC91C111=y
 CONFIG_ALLWINNER_EMAC=y
 CONFIG_IMX_FEC=y
 CONFIG_FTGMAC100=y
-CONFIG_DS1338=y
+CONFIG_DSRTC=y
 CONFIG_PFLASH_CFI01=y
 CONFIG_PFLASH_CFI02=y
 CONFIG_MICRODRIVE=y
diff --git a/hw/timer/Makefile.objs b/hw/timer/Makefile.objs
index 8b27a4b7ef..d4c59df1d1 100644
--- a/hw/timer/Makefile.objs
+++ b/hw/timer/Makefile.objs
@@ -3,7 +3,7 @@ common-obj-$(CONFIG_ARM_MPTIMER) += arm_mptimer.o
 common-obj-$(CONFIG_ARM_V7M) += armv7m_systick.o
 common-obj-$(CONFIG_A9_GTIMER) += a9gtimer.o
 common-obj-$(CONFIG_CADENCE) += cadence_ttc.o
-common-obj-$(CONFIG_DS1338) += ds1338.o
+common-obj-$(CONFIG_DSRTC) += ds-rtc.o
 common-obj-$(CONFIG_HPET) += hpet.o
 common-obj-$(CONFIG_I8254) += i8254_common.o i8254.o
 common-obj-$(CONFIG_M48T59) += m48t59.o
diff --git a/hw/timer/ds1338.c b/hw/timer/ds-rtc.c
similarity index 100%
rename from hw/timer/ds1338.c
rename to hw/timer/ds-rtc.c
-- 
2.11.0

Re: [Qemu-devel] [PATCH 10/14] timer: rename file ds1338.c -> ds-rtc.c

[Peter Maydell]

2018-03-24 19:24 GMT+00:00 Michael Davidsaver <mdavidsaver@gmail.com>:
> Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
> ---
>  default-configs/arm-softmmu.mak | 2 +-
>  hw/timer/Makefile.objs          | 2 +-
>  hw/timer/{ds1338.c => ds-rtc.c} | 0
>  3 files changed, 2 insertions(+), 2 deletions(-)
>  rename hw/timer/{ds1338.c => ds-rtc.c} (100%)
>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

[Qemu-devel] [PATCH 11/14] timer: generalize ds1338

[Michael Davidsaver]

Add class level handling for address space size
and control register.

Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
---
 hw/timer/ds-rtc.c | 63 ++++++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 46 insertions(+), 17 deletions(-)

diff --git a/hw/timer/ds-rtc.c b/hw/timer/ds-rtc.c
index 28f788dd8e..2df1bce3f8 100644
--- a/hw/timer/ds-rtc.c
+++ b/hw/timer/ds-rtc.c
@@ -21,10 +21,10 @@
  */
 #define NVRAM_SIZE 64
 
-#define CTRL_OSF   0x20
-
-#define TYPE_DSRTC "ds1338"
+#define TYPE_DSRTC "dsrtc"
 #define DSRTC(obj) OBJECT_CHECK(DSRTCState, (obj), TYPE_DSRTC)
+#define DSRTC_CLASS(klass) OBJECT_CLASS_CHECK(DSRTCClass, (klass), TYPE_DSRTC)
+#define DSRTC_GET_CLASS(obj) OBJECT_GET_CLASS(DSRTCClass, (obj), TYPE_DSRTC)
 
 /* values stored in BCD */
 /* 00-59 */
@@ -40,7 +40,7 @@
 /* 0-99 */
 #define R_YEAR  (0x6)
 
-#define R_CTRL  (0x7)
+#define R_DS1338_CTRL (0x7)
 
 /* use 12 hour mode when set */
 FIELD(HOUR, SET12, 6, 1)
@@ -67,6 +67,15 @@ typedef struct DSRTCState {
     bool addr_byte;
 } DSRTCState;
 
+typedef struct DSRTCClass {
+    I2CSlaveClass parent_obj;
+
+    /* actual address space size must be <= NVRAM_SIZE */
+    unsigned addr_size;
+    unsigned ctrl_offset;
+    void (*ctrl_write)(DSRTCState *s, uint8_t);
+} DSRTCClass;
+
 static const VMStateDescription vmstate_dsrtc = {
     .name = "ds1338",
     .version_id = 2,
@@ -119,11 +128,12 @@ static void capture_current_time(DSRTCState *s)
 
 static void inc_regptr(DSRTCState *s)
 {
-    /* The register pointer wraps around after 0x3F; wraparound
+    DSRTCClass *k = DSRTC_GET_CLASS(s);
+    /* The register pointer wraps around after k->addr_size-1; wraparound
      * causes the current time/date to be retransferred into
      * the secondary registers.
      */
-    s->ptr = (s->ptr + 1) & (NVRAM_SIZE - 1);
+    s->ptr = (s->ptr + 1) % k->addr_size;
     if (!s->ptr) {
         capture_current_time(s);
     }
@@ -206,22 +216,15 @@ static void dsrtc_update(DSRTCState *s)
 static int dsrtc_send(I2CSlave *i2c, uint8_t data)
 {
     DSRTCState *s = DSRTC(i2c);
+    DSRTCClass *k = DSRTC_GET_CLASS(s);
 
     if (s->addr_byte) {
-        s->ptr = data & (NVRAM_SIZE - 1);
+        s->ptr = data % k->addr_size;
         s->addr_byte = false;
         return 0;
     }
-    if (s->ptr == R_CTRL) {
-        /* 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);
-
+    if (s->ptr == k->ctrl_offset) {
+        (k->ctrl_write)(s, data);
     }
     s->nvram[s->ptr] = data;
     if (s->ptr <= R_YEAR) {
@@ -256,15 +259,41 @@ static void dsrtc_class_init(ObjectClass *klass, void *data)
 }
 
 static const TypeInfo dsrtc_info = {
+    .abstract      = true,
     .name          = TYPE_DSRTC,
     .parent        = TYPE_I2C_SLAVE,
     .instance_size = sizeof(DSRTCState),
     .class_init    = dsrtc_class_init,
 };
 
+static void ds1338_control_write(DSRTCState *s, uint8_t data)
+{
+    /* Control register. */
+
+    /* allow guest to set no-op controls for clock out pin */
+    s->nvram[R_DS1338_CTRL] = data & 0x93;
+}
+
+static void ds1338_class_init(ObjectClass *klass, void *data)
+{
+    DSRTCClass *k = DSRTC_CLASS(klass);
+
+    k->addr_size = 0x40;
+    k->ctrl_offset = R_DS1338_CTRL;
+    k->ctrl_write = ds1338_control_write;
+}
+
+static const TypeInfo ds1338_info = {
+    .name          = "ds1338",
+    .parent        = TYPE_DSRTC,
+    .class_size    = sizeof(DSRTCClass),
+    .class_init    = ds1338_class_init,
+};
+
 static void dsrtc_register_types(void)
 {
     type_register_static(&dsrtc_info);
+    type_register_static(&ds1338_info);
 }
 
 type_init(dsrtc_register_types)
-- 
2.11.0

Re: [Qemu-devel] [PATCH 11/14] timer: generalize ds1338

[Peter Maydell]

On 24 March 2018 at 19:24, Michael Davidsaver <mdavidsaver@gmail.com> wrote:
> Add class level handling for address space size
> and control register.
>
> Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
> ---

> @@ -206,22 +216,15 @@ static void dsrtc_update(DSRTCState *s)
>  static int dsrtc_send(I2CSlave *i2c, uint8_t data)
>  {
>      DSRTCState *s = DSRTC(i2c);
> +    DSRTCClass *k = DSRTC_GET_CLASS(s);
>
>      if (s->addr_byte) {
> -        s->ptr = data & (NVRAM_SIZE - 1);
> +        s->ptr = data % k->addr_size;
>          s->addr_byte = false;
>          return 0;
>      }
> -    if (s->ptr == R_CTRL) {
> -        /* 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);
> -
> +    if (s->ptr == k->ctrl_offset) {
> +        (k->ctrl_write)(s, data);
>      }
>      s->nvram[s->ptr] = data;
>      if (s->ptr <= R_YEAR) {
> @@ -256,15 +259,41 @@ static void dsrtc_class_init(ObjectClass *klass, void *data)
>  }
>
>  static const TypeInfo dsrtc_info = {
> +    .abstract      = true,
>      .name          = TYPE_DSRTC,
>      .parent        = TYPE_I2C_SLAVE,
>      .instance_size = sizeof(DSRTCState),
>      .class_init    = dsrtc_class_init,
>  };
>
> +static void ds1338_control_write(DSRTCState *s, uint8_t data)
> +{
> +    /* Control register. */
> +
> +    /* allow guest to set no-op controls for clock out pin */
> +    s->nvram[R_DS1338_CTRL] = data & 0x93;

This is mixing a behavioural change with an otherwise
pure refactoring -- can you split them out, please?

Otherwise the patch looks good.

thanks
-- PMM

[Qemu-devel] [PATCH 12/14] timer: ds-rtc handle CENTURY bit

[Michael Davidsaver]

Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
---
 hw/timer/ds-rtc.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/hw/timer/ds-rtc.c b/hw/timer/ds-rtc.c
index 2df1bce3f8..5a4df1b115 100644
--- a/hw/timer/ds-rtc.c
+++ b/hw/timer/ds-rtc.c
@@ -70,6 +70,7 @@ typedef struct DSRTCState {
 typedef struct DSRTCClass {
     I2CSlaveClass parent_obj;
 
+    bool has_century;
     /* actual address space size must be <= NVRAM_SIZE */
     unsigned addr_size;
     unsigned ctrl_offset;
@@ -91,7 +92,7 @@ static const VMStateDescription vmstate_dsrtc = {
     }
 };
 
-static void capture_current_time(DSRTCState *s)
+static void capture_current_time(DSRTCState *s, DSRTCClass *k)
 {
     /* Capture the current time into the secondary registers
      * which will be actually read by the data transfer operation.
@@ -123,25 +124,28 @@ static void capture_current_time(DSRTCState *s)
     }
     s->nvram[R_DATE] = to_bcd(now.tm_mday);
     s->nvram[R_MONTH] = to_bcd(now.tm_mon + 1);
-    s->nvram[R_YEAR] = to_bcd(now.tm_year - 100);
+    s->nvram[R_YEAR] = to_bcd(now.tm_year % 100u);
+
+    ARRAY_FIELD_DP32(s->nvram, MONTH, CENTURY,
+                     k->has_century && now.tm_year >= 100)
 }
 
-static void inc_regptr(DSRTCState *s)
+static void inc_regptr(DSRTCState *s, DSRTCClass *k)
 {
-    DSRTCClass *k = DSRTC_GET_CLASS(s);
     /* The register pointer wraps around after k->addr_size-1; wraparound
      * causes the current time/date to be retransferred into
      * the secondary registers.
      */
     s->ptr = (s->ptr + 1) % k->addr_size;
     if (!s->ptr) {
-        capture_current_time(s);
+        capture_current_time(s, k);
     }
 }
 
 static int dsrtc_event(I2CSlave *i2c, enum i2c_event event)
 {
     DSRTCState *s = DSRTC(i2c);
+    DSRTCClass *k = DSRTC_GET_CLASS(s);
 
     switch (event) {
     case I2C_START_RECV:
@@ -150,7 +154,7 @@ static int dsrtc_event(I2CSlave *i2c, enum i2c_event event)
          * START_SEND, because the guest can't get at that data
          * without going through a START_RECV which would overwrite it.
          */
-        capture_current_time(s);
+        capture_current_time(s, k);
         break;
     case I2C_START_SEND:
         s->addr_byte = true;
@@ -165,10 +169,11 @@ static int dsrtc_event(I2CSlave *i2c, enum i2c_event event)
 static int dsrtc_recv(I2CSlave *i2c)
 {
     DSRTCState *s = DSRTC(i2c);
+    DSRTCClass *k = DSRTC_GET_CLASS(s);
     uint8_t res;
 
     res  = s->nvram[s->ptr];
-    inc_regptr(s);
+    inc_regptr(s, k);
     return res;
 }
 
@@ -230,7 +235,7 @@ static int dsrtc_send(I2CSlave *i2c, uint8_t data)
     if (s->ptr <= R_YEAR) {
         dsrtc_update(s);
     }
-    inc_regptr(s);
+    inc_regptr(s, k);
     return 0;
 }
 
@@ -278,6 +283,7 @@ static void ds1338_class_init(ObjectClass *klass, void *data)
 {
     DSRTCClass *k = DSRTC_CLASS(klass);
 
+    k->has_century = false;
     k->addr_size = 0x40;
     k->ctrl_offset = R_DS1338_CTRL;
     k->ctrl_write = ds1338_control_write;
-- 
2.11.0

Re: [Qemu-devel] [PATCH 12/14] timer: ds-rtc handle CENTURY bit

[Peter Maydell]

On 24 March 2018 at 19:24, Michael Davidsaver <mdavidsaver@gmail.com> wrote:
> Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
> ---
>  hw/timer/ds-rtc.c | 22 ++++++++++++++--------
>  1 file changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/hw/timer/ds-rtc.c b/hw/timer/ds-rtc.c
> index 2df1bce3f8..5a4df1b115 100644
> --- a/hw/timer/ds-rtc.c
> +++ b/hw/timer/ds-rtc.c
> @@ -70,6 +70,7 @@ typedef struct DSRTCState {
>  typedef struct DSRTCClass {
>      I2CSlaveClass parent_obj;
>
> +    bool has_century;
>      /* actual address space size must be <= NVRAM_SIZE */
>      unsigned addr_size;
>      unsigned ctrl_offset;
> @@ -91,7 +92,7 @@ static const VMStateDescription vmstate_dsrtc = {
>      }
>  };
>
> -static void capture_current_time(DSRTCState *s)
> +static void capture_current_time(DSRTCState *s, DSRTCClass *k)
>  {
>      /* Capture the current time into the secondary registers
>       * which will be actually read by the data transfer operation.
> @@ -123,25 +124,28 @@ static void capture_current_time(DSRTCState *s)
>      }
>      s->nvram[R_DATE] = to_bcd(now.tm_mday);
>      s->nvram[R_MONTH] = to_bcd(now.tm_mon + 1);
> -    s->nvram[R_YEAR] = to_bcd(now.tm_year - 100);
> +    s->nvram[R_YEAR] = to_bcd(now.tm_year % 100u);
> +
> +    ARRAY_FIELD_DP32(s->nvram, MONTH, CENTURY,
> +                     k->has_century && now.tm_year >= 100)
>  }
>
> -static void inc_regptr(DSRTCState *s)
> +static void inc_regptr(DSRTCState *s, DSRTCClass *k)
>  {
> -    DSRTCClass *k = DSRTC_GET_CLASS(s);

I would just leave this function the way it is rather than
changing it to take the class pointer.

Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

[Qemu-devel] [PATCH 13/14] timer: ds-rtc model ds1375

[Michael Davidsaver]

differences from ds1338

* Has alarms (not modeled)
* different control register (not modeled)
* smaller address space (0x20 vs. 0x40)

Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
---
 hw/timer/ds-rtc.c | 30 ++++++++++++++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/hw/timer/ds-rtc.c b/hw/timer/ds-rtc.c
index 5a4df1b115..e5da36cae8 100644
--- a/hw/timer/ds-rtc.c
+++ b/hw/timer/ds-rtc.c
@@ -1,8 +1,9 @@
 /*
- * MAXIM DSRTC I2C RTC+NVRAM
+ * MAXIM/Dallas DS1338 and DS1375 I2C RTC+NVRAM
  *
+ * Copyright (c) 2018 Michael Davidsaver
  * Copyright (c) 2009 CodeSourcery.
- * Written by Paul Brook
+ * Written by Paul Brook, Michael Davidsaver
  *
  * This code is licensed under the GNU GPL v2.
  *
@@ -41,6 +42,7 @@
 #define R_YEAR  (0x6)
 
 #define R_DS1338_CTRL (0x7)
+#define R_DS1375_CTRL (0xe)
 
 /* use 12 hour mode when set */
 FIELD(HOUR, SET12, 6, 1)
@@ -296,10 +298,34 @@ static const TypeInfo ds1338_info = {
     .class_init    = ds1338_class_init,
 };
 
+static void ds1375_control_write(DSRTCState *s, uint8_t data)
+{
+    /* just store it, we don't model any features */
+    s->nvram[R_DS1375_CTRL] = data;
+}
+
+static void ds1375_class_init(ObjectClass *klass, void *data)
+{
+    DSRTCClass *k = DSRTC_CLASS(klass);
+
+    k->has_century = true;
+    k->addr_size = 0x20;
+    k->ctrl_offset = R_DS1375_CTRL;
+    k->ctrl_write = ds1375_control_write;
+}
+
+static const TypeInfo ds1375_info = {
+    .name          = "ds1375",
+    .parent        = TYPE_DSRTC,
+    .class_size    = sizeof(DSRTCClass),
+    .class_init    = ds1375_class_init,
+};
+
 static void dsrtc_register_types(void)
 {
     type_register_static(&dsrtc_info);
     type_register_static(&ds1338_info);
+    type_register_static(&ds1375_info);
 }
 
 type_init(dsrtc_register_types)
-- 
2.11.0

Re: [Qemu-devel] [PATCH 13/14] timer: ds-rtc model ds1375

[Peter Maydell]

On 24 March 2018 at 19:24, Michael Davidsaver <mdavidsaver@gmail.com> wrote:
> differences from ds1338
>
> * Has alarms (not modeled)
> * different control register (not modeled)
> * smaller address space (0x20 vs. 0x40)
>
> Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
> ---
>  hw/timer/ds-rtc.c | 30 ++++++++++++++++++++++++++++--
>  1 file changed, 28 insertions(+), 2 deletions(-)
>
> diff --git a/hw/timer/ds-rtc.c b/hw/timer/ds-rtc.c
> index 5a4df1b115..e5da36cae8 100644
> --- a/hw/timer/ds-rtc.c
> +++ b/hw/timer/ds-rtc.c
> @@ -1,8 +1,9 @@
>  /*
> - * MAXIM DSRTC I2C RTC+NVRAM
> + * MAXIM/Dallas DS1338 and DS1375 I2C RTC+NVRAM

Ah, if you're going to change this to this text here, you
should just leave it reading DS1338 in the earlier patch.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

[Qemu-devel] [PATCH 14/14] tests: drop ds1338-test

[Michael Davidsaver]

redundant to ds-rtc-*-test.c

Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
---
 tests/Makefile.include |  2 --
 tests/ds1338-test.c    | 75 --------------------------------------------------
 2 files changed, 77 deletions(-)
 delete mode 100644 tests/ds1338-test.c

diff --git a/tests/Makefile.include b/tests/Makefile.include
index d256095c88..04a99ea6fb 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -371,7 +371,6 @@ check-qtest-sparc64-y += tests/prom-env-test$(EXESUF)
 check-qtest-sparc64-y += tests/boot-serial-test$(EXESUF)
 
 check-qtest-arm-y = tests/tmp105-test$(EXESUF)
-check-qtest-arm-y += tests/ds1338-test$(EXESUF)
 check-qtest-arm-y += tests/ds-rtc-current-test$(EXESUF)
 check-qtest-arm-y += tests/ds-rtc-set-test$(EXESUF)
 check-qtest-arm-y += tests/m25p80-test$(EXESUF)
@@ -772,7 +771,6 @@ tests/bios-tables-test$(EXESUF): tests/bios-tables-test.o \
 	tests/boot-sector.o tests/acpi-utils.o $(libqos-obj-y)
 tests/pxe-test$(EXESUF): tests/pxe-test.o tests/boot-sector.o $(libqos-obj-y)
 tests/tmp105-test$(EXESUF): tests/tmp105-test.o $(libqos-omap-obj-y)
-tests/ds1338-test$(EXESUF): tests/ds1338-test.o $(libqos-imx-obj-y)
 tests/ds-rtc-current-test$(EXESUF): tests/ds-rtc-current-test.o $(libqos-imx-obj-y)
 tests/ds-rtc-set-test$(EXESUF): tests/ds-rtc-set-test.o $(libqos-imx-obj-y)
 tests/m25p80-test$(EXESUF): tests/m25p80-test.o
diff --git a/tests/ds1338-test.c b/tests/ds1338-test.c
deleted file mode 100644
index 742dad9113..0000000000
--- a/tests/ds1338-test.c
+++ /dev/null
@@ -1,75 +0,0 @@
-/*
- * QTest testcase for the DS1338 RTC
- *
- * Copyright (c) 2013 Jean-Christophe Dubois
- *
- *  This program is free software; you can redistribute it and/or modify it
- *  under the terms of the GNU General Public License as published by the
- *  Free Software Foundation; either version 2 of the License, or
- *  (at your option) any later version.
- *
- *  This program is distributed in the hope that it will be useful, but WITHOUT
- *  ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
- *  FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
- *  for more details.
- *
- *  You should have received a copy of the GNU General Public License along
- *  with this program; if not, see <http://www.gnu.org/licenses/>.
- */
-
-#include "qemu/osdep.h"
-#include "libqtest.h"
-#include "libqos/i2c.h"
-
-#define IMX25_I2C_0_BASE 0x43F80000
-
-#define DS1338_ADDR 0x68
-
-static I2CAdapter *i2c;
-static uint8_t addr;
-
-static inline uint8_t bcd2bin(uint8_t x)
-{
-    return ((x) & 0x0f) + ((x) >> 4) * 10;
-}
-
-static void send_and_receive(void)
-{
-    uint8_t cmd[1];
-    uint8_t resp[7];
-    time_t now = time(NULL);
-    struct tm *tm_ptr = gmtime(&now);
-
-    /* reset the index in the RTC memory */
-    cmd[0] = 0;
-    i2c_send(i2c, addr, cmd, 1);
-
-    /* retrieve the date */
-    i2c_recv(i2c, addr, resp, 7);
-
-    /* check retrieved time againt local time */
-    g_assert_cmpuint(bcd2bin(resp[4]), == , tm_ptr->tm_mday);
-    g_assert_cmpuint(bcd2bin(resp[5]), == , 1 + tm_ptr->tm_mon);
-    g_assert_cmpuint(2000 + bcd2bin(resp[6]), == , 1900 + tm_ptr->tm_year);
-}
-
-int main(int argc, char **argv)
-{
-    QTestState *s = NULL;
-    int ret;
-
-    g_test_init(&argc, &argv, NULL);
-
-    s = qtest_start("-display none -machine imx25-pdk");
-    i2c = imx_i2c_create(s, IMX25_I2C_0_BASE);
-    addr = DS1338_ADDR;
-
-    qtest_add_func("/ds1338/tx-rx", send_and_receive);
-
-    ret = g_test_run();
-
-    qtest_quit(s);
-    g_free(i2c);
-
-    return ret;
-}
-- 
2.11.0

Re: [Qemu-devel] [PATCH 00/14] Generalize Dallas/Maxim I2C RTC devices v2

[no-reply]

Hi,

This series failed docker-mingw@fedora build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

Type: series
Message-id: 20180324192455.12254-1-mdavidsaver@gmail.com
Subject: [Qemu-devel] [PATCH 00/14] Generalize Dallas/Maxim I2C RTC devices v2

=== TEST SCRIPT BEGIN ===
#!/bin/bash
set -e
git submodule update --init dtc
# Let docker tests dump environment info
export SHOW_ENV=1
export J=8
time make docker-test-mingw@fedora
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
cd5d44142f tests: drop ds1338-test
c568d03629 timer: ds-rtc model ds1375
360e76e958 timer: ds-rtc handle CENTURY bit
8e600ad679 timer: generalize ds1338
43f1db5002 timer: rename file ds1338.c -> ds-rtc.c
5ec505be2e timer: rename ds1338 -> dsrtc
628ed92468 tests: ds-rtc test wday offset
2582baf810 timer: ds1338 fix wday_offset handling
793b89687b tests: ds-rtc test 12 hour mode
3f9b2eb07b timer: ds1338 change write handling
45c4df6895 timer: ds1338 clarify HOUR handling
bb5d6cd534 timer: ds1338 persist 12-hour mode selection
8291a6b22d timer: ds1338 use registerfields.h
c785175358 tests: more thorough tests of ds1338

=== OUTPUT BEGIN ===
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into '/var/tmp/patchew-tester-tmp-bspf4r_p/src/dtc'...
Submodule path 'dtc': checked out 'e54388015af1fb4bf04d0bca99caba1074d9cc42'
  BUILD   fedora
make[1]: Entering directory '/var/tmp/patchew-tester-tmp-bspf4r_p/src'
  GEN     /var/tmp/patchew-tester-tmp-bspf4r_p/src/docker-src.2018-03-24-18.01.46.3331/qemu.tar
Cloning into '/var/tmp/patchew-tester-tmp-bspf4r_p/src/docker-src.2018-03-24-18.01.46.3331/qemu.tar.vroot'...
done.
Your branch is up-to-date with 'origin/test'.
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into '/var/tmp/patchew-tester-tmp-bspf4r_p/src/docker-src.2018-03-24-18.01.46.3331/qemu.tar.vroot/dtc'...
Submodule path 'dtc': checked out 'e54388015af1fb4bf04d0bca99caba1074d9cc42'
Submodule 'ui/keycodemapdb' (git://git.qemu.org/keycodemapdb.git) registered for path 'ui/keycodemapdb'
Cloning into '/var/tmp/patchew-tester-tmp-bspf4r_p/src/docker-src.2018-03-24-18.01.46.3331/qemu.tar.vroot/ui/keycodemapdb'...
Submodule path 'ui/keycodemapdb': checked out '6b3d716e2b6472eb7189d3220552280ef3d832ce'
tar: /var/tmp/patchew-tester-tmp-bspf4r_p/src/docker-src.2018-03-24-18.01.46.3331/qemu.tar: Wrote only 2048 of 10240 bytes
tar: Error is not recoverable: exiting now
failed to create tar file
  COPY    RUNNER
    RUN test-mingw in qemu:fedora 
tar: Unexpected EOF in archive
tar: Unexpected EOF in archive
tar: Error is not recoverable: exiting now
/var/tmp/qemu/run: line 32: prep_fail: command not found
Packages installed:
PyYAML-3.12-5.fc27.x86_64
SDL-devel-1.2.15-29.fc27.x86_64
bc-1.07.1-3.fc27.x86_64
bison-3.0.4-8.fc27.x86_64
bzip2-1.0.6-24.fc27.x86_64
ccache-3.3.6-1.fc27.x86_64
clang-5.0.1-3.fc27.x86_64
findutils-4.6.0-16.fc27.x86_64
flex-2.6.1-5.fc27.x86_64
gcc-7.3.1-5.fc27.x86_64
gcc-c++-7.3.1-5.fc27.x86_64
gettext-0.19.8.1-12.fc27.x86_64
git-2.14.3-3.fc27.x86_64
glib2-devel-2.54.3-2.fc27.x86_64
hostname-3.18-4.fc27.x86_64
libaio-devel-0.3.110-9.fc27.x86_64
libasan-7.3.1-5.fc27.x86_64
libfdt-devel-1.4.6-1.fc27.x86_64
libubsan-7.3.1-5.fc27.x86_64
llvm-5.0.1-3.fc27.x86_64
make-4.2.1-4.fc27.x86_64
mingw32-SDL-1.2.15-9.fc27.noarch
mingw32-bzip2-1.0.6-9.fc27.noarch
mingw32-curl-7.54.1-2.fc27.noarch
mingw32-glib2-2.54.1-1.fc27.noarch
mingw32-gmp-6.1.2-2.fc27.noarch
mingw32-gnutls-3.5.13-2.fc27.noarch
mingw32-gtk2-2.24.31-4.fc27.noarch
mingw32-gtk3-3.22.16-1.fc27.noarch
mingw32-libjpeg-turbo-1.5.1-3.fc27.noarch
mingw32-libpng-1.6.29-2.fc27.noarch
mingw32-libssh2-1.8.0-3.fc27.noarch
mingw32-libtasn1-4.13-1.fc27.noarch
mingw32-nettle-3.3-3.fc27.noarch
mingw32-pixman-0.34.0-3.fc27.noarch
mingw32-pkg-config-0.28-9.fc27.x86_64
mingw64-SDL-1.2.15-9.fc27.noarch
mingw64-bzip2-1.0.6-9.fc27.noarch
mingw64-curl-7.54.1-2.fc27.noarch
mingw64-glib2-2.54.1-1.fc27.noarch
mingw64-gmp-6.1.2-2.fc27.noarch
mingw64-gnutls-3.5.13-2.fc27.noarch
mingw64-gtk2-2.24.31-4.fc27.noarch
mingw64-gtk3-3.22.16-1.fc27.noarch
mingw64-libjpeg-turbo-1.5.1-3.fc27.noarch
mingw64-libpng-1.6.29-2.fc27.noarch
mingw64-libssh2-1.8.0-3.fc27.noarch
mingw64-libtasn1-4.13-1.fc27.noarch
mingw64-nettle-3.3-3.fc27.noarch
mingw64-pixman-0.34.0-3.fc27.noarch
mingw64-pkg-config-0.28-9.fc27.x86_64
nettle-devel-3.4-1.fc27.x86_64
perl-5.26.1-403.fc27.x86_64
pixman-devel-0.34.0-4.fc27.x86_64
python3-3.6.2-13.fc27.x86_64
sparse-0.5.1-2.fc27.x86_64
tar-1.29-7.fc27.x86_64
which-2.21-4.fc27.x86_64
zlib-devel-1.2.11-4.fc27.x86_64

Environment variables:
TARGET_LIST=
PACKAGES=ccache gettext git tar PyYAML sparse flex bison python3 bzip2 hostname     glib2-devel pixman-devel zlib-devel SDL-devel libfdt-devel     gcc gcc-c++ llvm clang make perl which bc findutils libaio-devel     nettle-devel libasan libubsan     mingw32-pixman mingw32-glib2 mingw32-gmp mingw32-SDL mingw32-pkg-config     mingw32-gtk2 mingw32-gtk3 mingw32-gnutls mingw32-nettle mingw32-libtasn1     mingw32-libjpeg-turbo mingw32-libpng mingw32-curl mingw32-libssh2     mingw32-bzip2     mingw64-pixman mingw64-glib2 mingw64-gmp mingw64-SDL mingw64-pkg-config     mingw64-gtk2 mingw64-gtk3 mingw64-gnutls mingw64-nettle mingw64-libtasn1     mingw64-libjpeg-turbo mingw64-libpng mingw64-curl mingw64-libssh2     mingw64-bzip2
J=8
V=
HOSTNAME=0383f38dfa78
DEBUG=
SHOW_ENV=1
PWD=/
HOME=/root
CCACHE_DIR=/var/tmp/ccache
DISTTAG=f27container
QEMU_CONFIGURE_OPTS=--python=/usr/bin/python3
FGC=f27
TEST_DIR=/tmp/qemu-test
SHLVL=1
FEATURES=mingw clang pyyaml asan dtc
PATH=/usr/lib/ccache:/usr/lib64/ccache:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
MAKEFLAGS= -j8
EXTRA_CONFIGURE_OPTS=
_=/usr/bin/env

Configure options:
--enable-werror --target-list=x86_64-softmmu,aarch64-softmmu --prefix=/tmp/qemu-test/install --python=/usr/bin/python3 --cross-prefix=x86_64-w64-mingw32- --enable-trace-backends=simple --enable-gnutls --enable-nettle --enable-curl --enable-vnc --enable-bzip2 --enable-guest-agent --with-sdlabi=1.2 --with-gtkabi=2.0

ERROR: DTC (libfdt) version >= 1.4.2 not present.
       Please install the DTC (libfdt) devel package

Traceback (most recent call last):
  File "./tests/docker/docker.py", line 407, in <module>
    sys.exit(main())
  File "./tests/docker/docker.py", line 404, in main
    return args.cmdobj.run(args, argv)
  File "./tests/docker/docker.py", line 261, in run
    return Docker().run(argv, args.keep, quiet=args.quiet)
  File "./tests/docker/docker.py", line 229, in run
    quiet=quiet)
  File "./tests/docker/docker.py", line 147, in _do_check
    return subprocess.check_call(self._command + cmd, **kwargs)
  File "/usr/lib64/python2.7/subprocess.py", line 186, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['docker', 'run', '--label', 'com.qemu.instance.uuid=fdba41202fae11e890c452540069c830', '-u', '0', '--security-opt', 'seccomp=unconfined', '--rm', '--net=none', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=8', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/root/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-bspf4r_p/src/docker-src.2018-03-24-18.01.46.3331:/var/tmp/qemu:z,ro', 'qemu:fedora', '/var/tmp/qemu/run', 'test-mingw']' returned non-zero exit status 1
make[1]: *** [tests/docker/Makefile.include:129: docker-run] Error 1
make[1]: Leaving directory '/var/tmp/patchew-tester-tmp-bspf4r_p/src'
make: *** [tests/docker/Makefile.include:163: docker-run-test-mingw@fedora] Error 2

real	1m20.833s
user	0m8.843s
sys	0m6.804s
=== OUTPUT END ===

Test command exited with code: 2


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org

Re: [Qemu-devel] [PATCH 00/14] Generalize Dallas/Maxim I2C RTC devices v2

[no-reply]

Hi,

This series failed docker-quick@centos6 build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

Type: series
Message-id: 20180324192455.12254-1-mdavidsaver@gmail.com
Subject: [Qemu-devel] [PATCH 00/14] Generalize Dallas/Maxim I2C RTC devices v2

=== TEST SCRIPT BEGIN ===
#!/bin/bash
set -e
git submodule update --init dtc
# Let docker tests dump environment info
export SHOW_ENV=1
export J=8
time make docker-test-quick@centos6
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 t [tag update]            patchew/20180323202344.70640-1-dgilbert@redhat.com -> patchew/20180323202344.70640-1-dgilbert@redhat.com
Switched to a new branch 'test'
cd5d44142f tests: drop ds1338-test
c568d03629 timer: ds-rtc model ds1375
360e76e958 timer: ds-rtc handle CENTURY bit
8e600ad679 timer: generalize ds1338
43f1db5002 timer: rename file ds1338.c -> ds-rtc.c
5ec505be2e timer: rename ds1338 -> dsrtc
628ed92468 tests: ds-rtc test wday offset
2582baf810 timer: ds1338 fix wday_offset handling
793b89687b tests: ds-rtc test 12 hour mode
3f9b2eb07b timer: ds1338 change write handling
45c4df6895 timer: ds1338 clarify HOUR handling
bb5d6cd534 timer: ds1338 persist 12-hour mode selection
8291a6b22d timer: ds1338 use registerfields.h
c785175358 tests: more thorough tests of ds1338

=== OUTPUT BEGIN ===
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into '/var/tmp/patchew-tester-tmp-9orj1if7/src/dtc'...
Submodule path 'dtc': checked out 'e54388015af1fb4bf04d0bca99caba1074d9cc42'
  BUILD   centos6
make[1]: Entering directory '/var/tmp/patchew-tester-tmp-9orj1if7/src'
  GEN     /var/tmp/patchew-tester-tmp-9orj1if7/src/docker-src.2018-03-24-18.04.07.5505/qemu.tar
Cloning into '/var/tmp/patchew-tester-tmp-9orj1if7/src/docker-src.2018-03-24-18.04.07.5505/qemu.tar.vroot'...
done.
Your branch is up-to-date with 'origin/test'.
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into '/var/tmp/patchew-tester-tmp-9orj1if7/src/docker-src.2018-03-24-18.04.07.5505/qemu.tar.vroot/dtc'...
Submodule path 'dtc': checked out 'e54388015af1fb4bf04d0bca99caba1074d9cc42'
Submodule 'ui/keycodemapdb' (git://git.qemu.org/keycodemapdb.git) registered for path 'ui/keycodemapdb'
Cloning into '/var/tmp/patchew-tester-tmp-9orj1if7/src/docker-src.2018-03-24-18.04.07.5505/qemu.tar.vroot/ui/keycodemapdb'...
Submodule path 'ui/keycodemapdb': checked out '6b3d716e2b6472eb7189d3220552280ef3d832ce'
tar: /var/tmp/patchew-tester-tmp-9orj1if7/src/docker-src.2018-03-24-18.04.07.5505/qemu.tar: Wrote only 2048 of 10240 bytes
tar: Error is not recoverable: exiting now
failed to create tar file
  COPY    RUNNER
    RUN test-quick in qemu:centos6 
tar: Unexpected EOF in archive
tar: Unexpected EOF in archive
tar: Error is not recoverable: exiting now
/var/tmp/qemu/run: line 32: prep_fail: command not found
Packages installed:
SDL-devel-1.2.14-7.el6_7.1.x86_64
bison-2.4.1-5.el6.x86_64
bzip2-devel-1.0.5-7.el6_0.x86_64
ccache-3.1.6-2.el6.x86_64
csnappy-devel-0-6.20150729gitd7bc683.el6.x86_64
flex-2.5.35-9.el6.x86_64
gcc-4.4.7-18.el6.x86_64
gettext-0.17-18.el6.x86_64
git-1.7.1-9.el6_9.x86_64
glib2-devel-2.28.8-9.el6.x86_64
libepoxy-devel-1.2-3.el6.x86_64
libfdt-devel-1.4.0-1.el6.x86_64
librdmacm-devel-1.0.21-0.el6.x86_64
lzo-devel-2.03-3.1.el6_5.1.x86_64
make-3.81-23.el6.x86_64
mesa-libEGL-devel-11.0.7-4.el6.x86_64
mesa-libgbm-devel-11.0.7-4.el6.x86_64
package g++ is not installed
pixman-devel-0.32.8-1.el6.x86_64
spice-glib-devel-0.26-8.el6.x86_64
spice-server-devel-0.12.4-16.el6.x86_64
tar-1.23-15.el6_8.x86_64
vte-devel-0.25.1-9.el6.x86_64
xen-devel-4.6.6-2.el6.x86_64
zlib-devel-1.2.3-29.el6.x86_64

Environment variables:
PACKAGES=bison     bzip2-devel     ccache     csnappy-devel     flex     g++     gcc     gettext     git     glib2-devel     libepoxy-devel     libfdt-devel     librdmacm-devel     lzo-devel     make     mesa-libEGL-devel     mesa-libgbm-devel     pixman-devel     SDL-devel     spice-glib-devel     spice-server-devel     tar     vte-devel     xen-devel     zlib-devel
HOSTNAME=53429828786d
MAKEFLAGS= -j8
J=8
CCACHE_DIR=/var/tmp/ccache
EXTRA_CONFIGURE_OPTS=
V=
SHOW_ENV=1
PATH=/usr/lib/ccache:/usr/lib64/ccache:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
PWD=/
TARGET_LIST=
SHLVL=1
HOME=/root
TEST_DIR=/tmp/qemu-test
FEATURES= dtc
DEBUG=
_=/usr/bin/env

Configure options:
--enable-werror --target-list=x86_64-softmmu,aarch64-softmmu --prefix=/tmp/qemu-test/install

ERROR: DTC (libfdt) version >= 1.4.2 not present.
       Please install the DTC (libfdt) devel package

Traceback (most recent call last):
  File "./tests/docker/docker.py", line 407, in <module>
    sys.exit(main())
  File "./tests/docker/docker.py", line 404, in main
    return args.cmdobj.run(args, argv)
  File "./tests/docker/docker.py", line 261, in run
    return Docker().run(argv, args.keep, quiet=args.quiet)
  File "./tests/docker/docker.py", line 229, in run
    quiet=quiet)
  File "./tests/docker/docker.py", line 147, in _do_check
    return subprocess.check_call(self._command + cmd, **kwargs)
  File "/usr/lib64/python2.7/subprocess.py", line 186, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['docker', 'run', '--label', 'com.qemu.instance.uuid=51b1faac2faf11e89d6b52540069c830', '-u', '0', '--security-opt', 'seccomp=unconfined', '--rm', '--net=none', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=8', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/root/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-9orj1if7/src/docker-src.2018-03-24-18.04.07.5505:/var/tmp/qemu:z,ro', 'qemu:centos6', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 1
make[1]: *** [tests/docker/Makefile.include:129: docker-run] Error 1
make[1]: Leaving directory '/var/tmp/patchew-tester-tmp-9orj1if7/src'
make: *** [tests/docker/Makefile.include:163: docker-run-test-quick@centos6] Error 2

real	1m1.235s
user	0m8.969s
sys	0m6.913s
=== OUTPUT END ===

Test command exited with code: 2


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org

Re: [Qemu-devel] [PATCH 00/14] Generalize Dallas/Maxim I2C RTC devices v2

[no-reply]

Hi,

This series failed docker-build@min-glib build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

Type: series
Message-id: 20180324192455.12254-1-mdavidsaver@gmail.com
Subject: [Qemu-devel] [PATCH 00/14] Generalize Dallas/Maxim I2C RTC devices v2

=== TEST SCRIPT BEGIN ===
#!/bin/bash
set -e
git submodule update --init dtc
# Let docker tests dump environment info
export SHOW_ENV=1
export J=8
time make docker-test-build@min-glib
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
cd5d44142f tests: drop ds1338-test
c568d03629 timer: ds-rtc model ds1375
360e76e958 timer: ds-rtc handle CENTURY bit
8e600ad679 timer: generalize ds1338
43f1db5002 timer: rename file ds1338.c -> ds-rtc.c
5ec505be2e timer: rename ds1338 -> dsrtc
628ed92468 tests: ds-rtc test wday offset
2582baf810 timer: ds1338 fix wday_offset handling
793b89687b tests: ds-rtc test 12 hour mode
3f9b2eb07b timer: ds1338 change write handling
45c4df6895 timer: ds1338 clarify HOUR handling
bb5d6cd534 timer: ds1338 persist 12-hour mode selection
8291a6b22d timer: ds1338 use registerfields.h
c785175358 tests: more thorough tests of ds1338

=== OUTPUT BEGIN ===
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into '/var/tmp/patchew-tester-tmp-8pwvp3st/src/dtc'...
Submodule path 'dtc': checked out 'e54388015af1fb4bf04d0bca99caba1074d9cc42'
  BUILD   min-glib
make[1]: Entering directory '/var/tmp/patchew-tester-tmp-8pwvp3st/src'
  GEN     /var/tmp/patchew-tester-tmp-8pwvp3st/src/docker-src.2018-03-24-18.07.16.7723/qemu.tar
Cloning into '/var/tmp/patchew-tester-tmp-8pwvp3st/src/docker-src.2018-03-24-18.07.16.7723/qemu.tar.vroot'...
done.
Your branch is up-to-date with 'origin/test'.
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into '/var/tmp/patchew-tester-tmp-8pwvp3st/src/docker-src.2018-03-24-18.07.16.7723/qemu.tar.vroot/dtc'...
Submodule path 'dtc': checked out 'e54388015af1fb4bf04d0bca99caba1074d9cc42'
Submodule 'ui/keycodemapdb' (git://git.qemu.org/keycodemapdb.git) registered for path 'ui/keycodemapdb'
Cloning into '/var/tmp/patchew-tester-tmp-8pwvp3st/src/docker-src.2018-03-24-18.07.16.7723/qemu.tar.vroot/ui/keycodemapdb'...
Submodule path 'ui/keycodemapdb': checked out '6b3d716e2b6472eb7189d3220552280ef3d832ce'
tar: /var/tmp/patchew-tester-tmp-8pwvp3st/src/docker-src.2018-03-24-18.07.16.7723/qemu.tar: Wrote only 4096 of 10240 bytes
tar: Error is not recoverable: exiting now
failed to create tar file
  COPY    RUNNER
    RUN test-build in qemu:min-glib 
tar: Unexpected EOF in archive
tar: Unexpected EOF in archive
tar: Error is not recoverable: exiting now
/var/tmp/qemu/run: line 32: prep_fail: command not found
Environment variables:
HOSTNAME=2ad891f2b17f
MAKEFLAGS= -j8
J=8
CCACHE_DIR=/var/tmp/ccache
EXTRA_CONFIGURE_OPTS=
V=
SHOW_ENV=1
PATH=/usr/lib/ccache:/usr/lib64/ccache:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
PWD=/
TARGET_LIST=
SHLVL=1
HOME=/root
TEST_DIR=/tmp/qemu-test
FEATURES= dtc
DEBUG=
_=/usr/bin/env

Configure options:
--enable-werror --target-list=x86_64-softmmu,aarch64-softmmu --prefix=/tmp/qemu-test/install

ERROR: DTC (libfdt) version >= 1.4.2 not present.
       Please install the DTC (libfdt) devel package

Traceback (most recent call last):
  File "./tests/docker/docker.py", line 407, in <module>
    sys.exit(main())
  File "./tests/docker/docker.py", line 404, in main
    return args.cmdobj.run(args, argv)
  File "./tests/docker/docker.py", line 261, in run
    return Docker().run(argv, args.keep, quiet=args.quiet)
  File "./tests/docker/docker.py", line 229, in run
    quiet=quiet)
  File "./tests/docker/docker.py", line 147, in _do_check
    return subprocess.check_call(self._command + cmd, **kwargs)
  File "/usr/lib64/python2.7/subprocess.py", line 186, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['docker', 'run', '--label', 'com.qemu.instance.uuid=c219b4d82faf11e8b99852540069c830', '-u', '0', '--security-opt', 'seccomp=unconfined', '--rm', '--net=none', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=8', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/root/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-8pwvp3st/src/docker-src.2018-03-24-18.07.16.7723:/var/tmp/qemu:z,ro', 'qemu:min-glib', '/var/tmp/qemu/run', 'test-build']' returned non-zero exit status 1
make[1]: *** [tests/docker/Makefile.include:129: docker-run] Error 1
make[1]: Leaving directory '/var/tmp/patchew-tester-tmp-8pwvp3st/src'
make: *** [tests/docker/Makefile.include:163: docker-run-test-build@min-glib] Error 2

real	0m54.355s
user	0m8.995s
sys	0m6.886s
=== OUTPUT END ===

Test command exited with code: 2


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org

[Qemu-devel] Patchew failure ? (was: Re: [PATCH 00/14] Generalize Dallas/Maxim I2C RTC devices v2)

[Thomas Huth]

On 24.03.2018 23:08, no-reply@patchew.org wrote:
> Hi,
> 
> This series failed docker-build@min-glib build test. Please find the testing commands and
> their output below. If you have Docker installed, you can probably reproduce it
> locally.
> 
> Type: series
> Message-id: 20180324192455.12254-1-mdavidsaver@gmail.com
> Subject: [Qemu-devel] [PATCH 00/14] Generalize Dallas/Maxim I2C RTC devices v2
[...]
> === OUTPUT BEGIN ===
> Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
> Cloning into '/var/tmp/patchew-tester-tmp-8pwvp3st/src/dtc'...
> Submodule path 'dtc': checked out 'e54388015af1fb4bf04d0bca99caba1074d9cc42'
>   BUILD   min-glib
> make[1]: Entering directory '/var/tmp/patchew-tester-tmp-8pwvp3st/src'
>   GEN     /var/tmp/patchew-tester-tmp-8pwvp3st/src/docker-src.2018-03-24-18.07.16.7723/qemu.tar
> Cloning into '/var/tmp/patchew-tester-tmp-8pwvp3st/src/docker-src.2018-03-24-18.07.16.7723/qemu.tar.vroot'...
> done.
> Your branch is up-to-date with 'origin/test'.
> Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
> Cloning into '/var/tmp/patchew-tester-tmp-8pwvp3st/src/docker-src.2018-03-24-18.07.16.7723/qemu.tar.vroot/dtc'...
> Submodule path 'dtc': checked out 'e54388015af1fb4bf04d0bca99caba1074d9cc42'
> Submodule 'ui/keycodemapdb' (git://git.qemu.org/keycodemapdb.git) registered for path 'ui/keycodemapdb'
> Cloning into '/var/tmp/patchew-tester-tmp-8pwvp3st/src/docker-src.2018-03-24-18.07.16.7723/qemu.tar.vroot/ui/keycodemapdb'...
> Submodule path 'ui/keycodemapdb': checked out '6b3d716e2b6472eb7189d3220552280ef3d832ce'
> tar: /var/tmp/patchew-tester-tmp-8pwvp3st/src/docker-src.2018-03-24-18.07.16.7723/qemu.tar: Wrote only 4096 of 10240 bytes
> tar: Error is not recoverable: exiting now
> failed to create tar file
>   COPY    RUNNER
>     RUN test-build in qemu:min-glib 
> tar: Unexpected EOF in archive
> tar: Unexpected EOF in archive
> tar: Error is not recoverable: exiting now
> /var/tmp/qemu/run: line 32: prep_fail: command not found
> Environment variables:
> HOSTNAME=2ad891f2b17f
> MAKEFLAGS= -j8
> J=8
> CCACHE_DIR=/var/tmp/ccache
> EXTRA_CONFIGURE_OPTS=
> V=
> SHOW_ENV=1
> PATH=/usr/lib/ccache:/usr/lib64/ccache:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
> PWD=/
> TARGET_LIST=
> SHLVL=1
> HOME=/root
> TEST_DIR=/tmp/qemu-test
> FEATURES= dtc
> DEBUG=
> _=/usr/bin/env
> 
> Configure options:
> --enable-werror --target-list=x86_64-softmmu,aarch64-softmmu --prefix=/tmp/qemu-test/install
> 
> ERROR: DTC (libfdt) version >= 1.4.2 not present.
>        Please install the DTC (libfdt) devel package

 Hi Fam,

sounds like there is currently something broken with patchew? Could you
please have a look why it fails to use libfdt?

 Thomas

Re: [Qemu-devel] Patchew failure ? (was: Re: [PATCH 00/14] Generalize Dallas/Maxim I2C RTC devices v2)

[Fam Zheng]

On Mon, 03/26 10:34, Thomas Huth wrote:
> On 24.03.2018 23:08, no-reply@patchew.org wrote:
> > Hi,
> > 
> > This series failed docker-build@min-glib build test. Please find the testing commands and
> > their output below. If you have Docker installed, you can probably reproduce it
> > locally.
> > 
> > Type: series
> > Message-id: 20180324192455.12254-1-mdavidsaver@gmail.com
> > Subject: [Qemu-devel] [PATCH 00/14] Generalize Dallas/Maxim I2C RTC devices v2
> [...]
> > === OUTPUT BEGIN ===
> > Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
> > Cloning into '/var/tmp/patchew-tester-tmp-8pwvp3st/src/dtc'...
> > Submodule path 'dtc': checked out 'e54388015af1fb4bf04d0bca99caba1074d9cc42'
> >   BUILD   min-glib
> > make[1]: Entering directory '/var/tmp/patchew-tester-tmp-8pwvp3st/src'
> >   GEN     /var/tmp/patchew-tester-tmp-8pwvp3st/src/docker-src.2018-03-24-18.07.16.7723/qemu.tar
> > Cloning into '/var/tmp/patchew-tester-tmp-8pwvp3st/src/docker-src.2018-03-24-18.07.16.7723/qemu.tar.vroot'...
> > done.
> > Your branch is up-to-date with 'origin/test'.
> > Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
> > Cloning into '/var/tmp/patchew-tester-tmp-8pwvp3st/src/docker-src.2018-03-24-18.07.16.7723/qemu.tar.vroot/dtc'...
> > Submodule path 'dtc': checked out 'e54388015af1fb4bf04d0bca99caba1074d9cc42'
> > Submodule 'ui/keycodemapdb' (git://git.qemu.org/keycodemapdb.git) registered for path 'ui/keycodemapdb'
> > Cloning into '/var/tmp/patchew-tester-tmp-8pwvp3st/src/docker-src.2018-03-24-18.07.16.7723/qemu.tar.vroot/ui/keycodemapdb'...
> > Submodule path 'ui/keycodemapdb': checked out '6b3d716e2b6472eb7189d3220552280ef3d832ce'
> > tar: /var/tmp/patchew-tester-tmp-8pwvp3st/src/docker-src.2018-03-24-18.07.16.7723/qemu.tar: Wrote only 4096 of 10240 bytes
> > tar: Error is not recoverable: exiting now
> > failed to create tar file
> >   COPY    RUNNER
> >     RUN test-build in qemu:min-glib 
> > tar: Unexpected EOF in archive
> > tar: Unexpected EOF in archive
> > tar: Error is not recoverable: exiting now
> > /var/tmp/qemu/run: line 32: prep_fail: command not found
> > Environment variables:
> > HOSTNAME=2ad891f2b17f
> > MAKEFLAGS= -j8
> > J=8
> > CCACHE_DIR=/var/tmp/ccache
> > EXTRA_CONFIGURE_OPTS=
> > V=
> > SHOW_ENV=1
> > PATH=/usr/lib/ccache:/usr/lib64/ccache:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
> > PWD=/
> > TARGET_LIST=
> > SHLVL=1
> > HOME=/root
> > TEST_DIR=/tmp/qemu-test
> > FEATURES= dtc
> > DEBUG=
> > _=/usr/bin/env
> > 
> > Configure options:
> > --enable-werror --target-list=x86_64-softmmu,aarch64-softmmu --prefix=/tmp/qemu-test/install
> > 
> > ERROR: DTC (libfdt) version >= 1.4.2 not present.
> >        Please install the DTC (libfdt) devel package

Thanks for reporting!

The image only has 1.4.0 so we need the submodule.. However, the real issue here
is -ENOSPC, and tests/docker/run has a "prep_fail" bug as above. I'll look into
those.

Fam

> 
>  Hi Fam,
> 
> sounds like there is currently something broken with patchew? Could you
> please have a look why it fails to use libfdt?
> 
>  Thomas

Re: [Qemu-devel] [PATCH 00/14] Generalize Dallas/Maxim I2C RTC devices v2

[Peter Maydell]

On 24 March 2018 at 19:24, Michael Davidsaver <mdavidsaver@gmail.com> wrote:
> This series generalizes the ds1338 model to also support the ds1375.
> As previously, only the time of day registers are modeled.  This
> series is largely a do-over wrt. my previous series.  This time I
> started with incremental changes from the existing ds1338 model, and only
> add support for the ds1375 (which I care about).
>
> I've added a more thorough test of the time of day function, covering
> reading and setting in both 12 and 24 hour mode.  This corrects two
> (practically inconsequential) bugs with the handling of 12 hour mode,
> and day of the week.
>
> In an attempt to address concerns about false positive test failures
> in CI builds, instead of comparing the parts of 'struct tm' seperately
> I've changed the logic of the tests to compare the difference between
> the expected and actual time in seconds.  The threshold is 30 seconds
> when run with 'gtester -m quick', and 1 second otherwise.
>
> Comparision of day of the week is still exact, so there is a chance of
> a false positive if the test is running across midnight UTC.

Hi; sorry it took me a while to get to reviewing this patchset.
I think I've now reviewed all the non-testcase parts of it.

thanks
-- PMM

[Qemu-devel] [PATCH 00/14] Generalize Dallas/Maxim I2C RTC devices v3

[Michael Davidsaver]

This series generalizes the ds1338 model to also support the ds1375.
As previously, only the time of day registers are modeled.  This
series (v3) is a minor iteration on the previous v2.

No changes were made to the unittests.

Summary of changes by patch name in response to comments by Peter Maydell.

= timer: ds1338 use registerfields.h

Changed as requested to be more direct translation.

= timer: ds1338 persist 12-hour mode selection

On 04/12/2018 11:03 AM, Peter Maydell wrote:
> The code change here looks like a reasonable no-behaviour-change
> simplification of the code, but it doesn't match up with
> the description in the commit message ?

While it isn't clear from the diff, the condtional
involved has 3 arms, only two of these were updating
's->nvram[]'.  The arm handling control register
writes is the one which didn't.  So attempts to set
the 12 hour mode bit were ignored, and subsequent
reads always returned 24 hour time.

So this is a behaviour change, and a minor bug fix.

= timer: ds1338 clarify HOUR handling
= timer: ds1338 fix wday_offset handling

These two are combined as requested.

= timer: generalize ds1338

The change to CTRL register handling is split out
as a seperate patch "timer: ds1338 remove vestige of un-modeled OSF"

= timer: ds-rtc handle CENTURY bit

Changed as suggested.



Michael Davidsaver (14):
  tests: more thorough tests of ds1338
  timer: ds1338 use registerfields.h
  timer: ds1338 persist 12-hour mode selection
  timer: ds1338 clarify HOUR handling
  timer: ds1338 change write handling and fix wday_offset handling
  tests: ds-rtc test 12 hour mode
  tests: ds-rtc test wday offset
  timer: rename ds1338 -> dsrtc
  timer: rename file ds1338.c -> ds-rtc.c
  timer: ds1338 remove vestige of un-modeled OSF
  timer: generalize ds1338
  timer: ds-rtc handle CENTURY bit
  timer: ds-rtc model ds1375
  tests: drop ds1338-test

 default-configs/arm-softmmu.mak |   2 +-
 hw/timer/Makefile.objs          |   2 +-
 hw/timer/ds-rtc.c               | 335 ++++++++++++++++++++++++++++++++++++++++
 hw/timer/ds1338.c               | 239 ----------------------------
 tests/Makefile.include          |   6 +-
 tests/ds-rtc-common.h           |  71 +++++++++
 tests/ds-rtc-current-test.c     |  66 ++++++++
 tests/ds-rtc-set-test.c         | 171 ++++++++++++++++++++
 tests/ds1338-test.c             |  75 ---------
 9 files changed, 649 insertions(+), 318 deletions(-)
 create mode 100644 hw/timer/ds-rtc.c
 delete mode 100644 hw/timer/ds1338.c
 create mode 100644 tests/ds-rtc-common.h
 create mode 100644 tests/ds-rtc-current-test.c
 create mode 100644 tests/ds-rtc-set-test.c
 delete mode 100644 tests/ds1338-test.c

-- 
2.11.0

[Qemu-devel] [PATCH 12/14] timer: ds-rtc handle CENTURY bit

[Michael Davidsaver]

Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/timer/ds-rtc.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/hw/timer/ds-rtc.c b/hw/timer/ds-rtc.c
index 6d3a8b5586..9abac38628 100644
--- a/hw/timer/ds-rtc.c
+++ b/hw/timer/ds-rtc.c
@@ -70,6 +70,7 @@ typedef struct DSRTCState {
 typedef struct DSRTCClass {
     I2CSlaveClass parent_obj;
 
+    bool has_century;
     /* actual address space size must be <= NVRAM_SIZE */
     unsigned addr_size;
     unsigned ctrl_offset;
@@ -93,6 +94,7 @@ static const VMStateDescription vmstate_dsrtc = {
 
 static void capture_current_time(DSRTCState *s)
 {
+    DSRTCClass *k = DSRTC_GET_CLASS(s);
     /* Capture the current time into the secondary registers
      * which will be actually read by the data transfer operation.
      */
@@ -125,7 +127,10 @@ static void capture_current_time(DSRTCState *s)
     }
     s->nvram[R_DATE] = to_bcd(now.tm_mday);
     s->nvram[R_MONTH] = to_bcd(now.tm_mon + 1);
-    s->nvram[R_YEAR] = to_bcd(now.tm_year - 100);
+    s->nvram[R_YEAR] = to_bcd(now.tm_year % 100u);
+
+    ARRAY_FIELD_DP32(s->nvram, MONTH, CENTURY,
+                     k->has_century && now.tm_year >= 100)
 }
 
 static void inc_regptr(DSRTCState *s)
@@ -282,6 +287,7 @@ static void ds1338_class_init(ObjectClass *klass, void *data)
 {
     DSRTCClass *k = DSRTC_CLASS(klass);
 
+    k->has_century = false;
     k->addr_size = 0x40;
     k->ctrl_offset = R_DS1338_CTRL;
     k->ctrl_write = ds1338_control_write;
-- 
2.11.0

Re: [Qemu-devel] [PATCH 12/14] timer: ds-rtc handle CENTURY bit

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 2032 bytes --]


On Thu, Jul 05, 2018 at 11:19:59AM -0700, Michael Davidsaver wrote:
> Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  hw/timer/ds-rtc.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/timer/ds-rtc.c b/hw/timer/ds-rtc.c
> index 6d3a8b5586..9abac38628 100644
> --- a/hw/timer/ds-rtc.c
> +++ b/hw/timer/ds-rtc.c
> @@ -70,6 +70,7 @@ typedef struct DSRTCState {
>  typedef struct DSRTCClass {
>      I2CSlaveClass parent_obj;
>  
> +    bool has_century;
>      /* actual address space size must be <= NVRAM_SIZE */
>      unsigned addr_size;
>      unsigned ctrl_offset;
> @@ -93,6 +94,7 @@ static const VMStateDescription vmstate_dsrtc = {
>  
>  static void capture_current_time(DSRTCState *s)
>  {
> +    DSRTCClass *k = DSRTC_GET_CLASS(s);
>      /* Capture the current time into the secondary registers
>       * which will be actually read by the data transfer operation.
>       */
> @@ -125,7 +127,10 @@ static void capture_current_time(DSRTCState *s)
>      }
>      s->nvram[R_DATE] = to_bcd(now.tm_mday);
>      s->nvram[R_MONTH] = to_bcd(now.tm_mon + 1);
> -    s->nvram[R_YEAR] = to_bcd(now.tm_year - 100);
> +    s->nvram[R_YEAR] = to_bcd(now.tm_year % 100u);
> +
> +    ARRAY_FIELD_DP32(s->nvram, MONTH, CENTURY,
> +                     k->has_century && now.tm_year >= 100)
>  }
>  
>  static void inc_regptr(DSRTCState *s)
> @@ -282,6 +287,7 @@ static void ds1338_class_init(ObjectClass *klass, void *data)
>  {
>      DSRTCClass *k = DSRTC_CLASS(klass);
>  
> +    k->has_century = false;
>      k->addr_size = 0x40;
>      k->ctrl_offset = R_DS1338_CTRL;
>      k->ctrl_write = ds1338_control_write;

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]