* [Qemu-devel] [PATCH 00/14] Generalize Dallas/Maxim I2C RTC devices v3
@ 2018-07-05 18:19 Michael Davidsaver
2018-07-05 18:19 ` [Qemu-devel] [PATCH 01/14] tests: more thorough tests of ds1338 Michael Davidsaver
` (13 more replies)
0 siblings, 14 replies; 31+ messages in thread
From: Michael Davidsaver @ 2018-07-05 18:19 UTC (permalink / raw)
To: Peter Maydell, Paolo Bonzini
Cc: Thomas Huth, Antoine Mathys, David Gibson, qemu-devel,
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
^ permalink raw reply [flat|nested] 31+ messages in thread
* [Qemu-devel] [PATCH 01/14] tests: more thorough tests of ds1338
2018-07-05 18:19 [Qemu-devel] [PATCH 00/14] Generalize Dallas/Maxim I2C RTC devices v3 Michael Davidsaver
@ 2018-07-05 18:19 ` Michael Davidsaver
2018-07-17 15:28 ` Peter Maydell
2018-07-05 18:19 ` [Qemu-devel] [PATCH 02/14] timer: ds1338 use registerfields.h Michael Davidsaver
` (12 subsequent siblings)
13 siblings, 1 reply; 31+ messages in thread
From: Michael Davidsaver @ 2018-07-05 18:19 UTC (permalink / raw)
To: Peter Maydell, Paolo Bonzini
Cc: Thomas Huth, Antoine Mathys, David Gibson, qemu-devel,
Michael Davidsaver
Test current time and set+get round trip.
Separate current time test from set/get tests
to avoid test needing to impose order, or to
have a magic handshaketo reset RTC to current time.
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 8859e88ffb..6ce5a9ff4d 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -378,6 +378,8 @@ check-qtest-sparc64-y += tests/boot-serial-test$(EXESUF)
check-qtest-arm-y = tests/tmp105-test$(EXESUF)
check-qtest-arm-y += tests/pca9552-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)
@@ -787,6 +789,8 @@ 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/pca9552-test$(EXESUF): tests/pca9552-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
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PATCH 02/14] timer: ds1338 use registerfields.h
2018-07-05 18:19 [Qemu-devel] [PATCH 00/14] Generalize Dallas/Maxim I2C RTC devices v3 Michael Davidsaver
2018-07-05 18:19 ` [Qemu-devel] [PATCH 01/14] tests: more thorough tests of ds1338 Michael Davidsaver
@ 2018-07-05 18:19 ` Michael Davidsaver
2018-07-06 1:35 ` David Gibson
2018-07-05 18:19 ` [Qemu-devel] [PATCH 03/14] timer: ds1338 persist 12-hour mode selection Michael Davidsaver
` (11 subsequent siblings)
13 siblings, 1 reply; 31+ messages in thread
From: Michael Davidsaver @ 2018-07-05 18:19 UTC (permalink / raw)
To: Peter Maydell, Paolo Bonzini
Cc: Thomas Huth, Antoine Mathys, David Gibson, qemu-devel,
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>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/timer/ds1338.c | 80 +++++++++++++++++++++++++++++++++++++------------------
1 file changed, 54 insertions(+), 26 deletions(-)
diff --git a/hw/timer/ds1338.c b/hw/timer/ds1338.c
index 3849b74a68..7298c5af43 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,25 @@ 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;
}
if (tmp <= 12) {
- s->nvram[2] = HOURS_12 | to_bcd(tmp);
+ s->nvram[R_HOUR] = R_HOUR_SET12_MASK | to_bcd(tmp);
} else {
- s->nvram[2] = HOURS_12 | HOURS_PM | to_bcd(tmp - 12);
+ s->nvram[R_HOUR] = R_HOUR_SET12_MASK | R_HOUR_AMPM_MASK | to_bcd(tmp - 12);
}
} else {
- s->nvram[2] = to_bcd(now.tm_hour);
+ s->nvram[R_HOUR] = 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 +169,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 +187,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 +199,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
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PATCH 03/14] timer: ds1338 persist 12-hour mode selection
2018-07-05 18:19 [Qemu-devel] [PATCH 00/14] Generalize Dallas/Maxim I2C RTC devices v3 Michael Davidsaver
2018-07-05 18:19 ` [Qemu-devel] [PATCH 01/14] tests: more thorough tests of ds1338 Michael Davidsaver
2018-07-05 18:19 ` [Qemu-devel] [PATCH 02/14] timer: ds1338 use registerfields.h Michael Davidsaver
@ 2018-07-05 18:19 ` Michael Davidsaver
2018-07-06 3:39 ` David Gibson
2018-07-05 18:19 ` [Qemu-devel] [PATCH 04/14] timer: ds1338 clarify HOUR handling Michael Davidsaver
` (10 subsequent siblings)
13 siblings, 1 reply; 31+ messages in thread
From: Michael Davidsaver @ 2018-07-05 18:19 UTC (permalink / raw)
To: Peter Maydell, Paolo Bonzini
Cc: Thomas Huth, Antoine Mathys, David Gibson, qemu-devel,
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.
This was only being done in two of three
arms of this conditional block.
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 7298c5af43..b56db5852e 100644
--- a/hw/timer/ds1338.c
+++ b/hw/timer/ds1338.c
@@ -220,10 +220,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
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PATCH 04/14] timer: ds1338 clarify HOUR handling
2018-07-05 18:19 [Qemu-devel] [PATCH 00/14] Generalize Dallas/Maxim I2C RTC devices v3 Michael Davidsaver
` (2 preceding siblings ...)
2018-07-05 18:19 ` [Qemu-devel] [PATCH 03/14] timer: ds1338 persist 12-hour mode selection Michael Davidsaver
@ 2018-07-05 18:19 ` Michael Davidsaver
2018-07-09 5:12 ` David Gibson
2018-07-05 18:19 ` [Qemu-devel] [PATCH 05/14] timer: ds1338 change write handling and fix wday_offset handling Michael Davidsaver
` (9 subsequent siblings)
13 siblings, 1 reply; 31+ messages in thread
From: Michael Davidsaver @ 2018-07-05 18:19 UTC (permalink / raw)
To: Peter Maydell, Paolo Bonzini
Cc: Thomas Huth, Antoine Mathys, David Gibson, qemu-devel,
Michael Davidsaver
Simplify and comment the translation between
registers and struct tm.
Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
---
hw/timer/ds1338.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/hw/timer/ds1338.c b/hw/timer/ds1338.c
index b56db5852e..637a0f4246 100644
--- a/hw/timer/ds1338.c
+++ b/hw/timer/ds1338.c
@@ -89,9 +89,13 @@ static void capture_current_time(DS1338State *s)
*/
struct tm now;
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)) {
+ /* 12 hour mode.
+ * map 0-23 to 1-12 am/pm
+ */
int tmp = now.tm_hour;
if (tmp % 12 == 0) {
tmp += 12;
@@ -101,7 +105,9 @@ static void capture_current_time(DS1338State *s)
} else {
s->nvram[R_HOUR] = R_HOUR_SET12_MASK | R_HOUR_AMPM_MASK | to_bcd(tmp - 12);
}
+
} else {
+ /* 24 hour mode. */
s->nvram[R_HOUR] = to_bcd(now.tm_hour);
}
s->nvram[R_WDAY] = (now.tm_wday + s->wday_offset) % 7 + 1;
@@ -178,14 +184,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
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PATCH 05/14] timer: ds1338 change write handling and fix wday_offset handling
2018-07-05 18:19 [Qemu-devel] [PATCH 00/14] Generalize Dallas/Maxim I2C RTC devices v3 Michael Davidsaver
` (3 preceding siblings ...)
2018-07-05 18:19 ` [Qemu-devel] [PATCH 04/14] timer: ds1338 clarify HOUR handling Michael Davidsaver
@ 2018-07-05 18:19 ` Michael Davidsaver
2018-07-16 4:25 ` David Gibson
2018-07-05 18:19 ` [Qemu-devel] [PATCH 06/14] tests: ds-rtc test 12 hour mode Michael Davidsaver
` (8 subsequent siblings)
13 siblings, 1 reply; 31+ messages in thread
From: Michael Davidsaver @ 2018-07-05 18:19 UTC (permalink / raw)
To: Peter Maydell, Paolo Bonzini
Cc: Thomas Huth, Antoine Mathys, David Gibson, qemu-devel,
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.
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 | 95 ++++++++++++++++++++++++++++---------------------------
1 file changed, 48 insertions(+), 47 deletions(-)
diff --git a/hw/timer/ds1338.c b/hw/timer/ds1338.c
index 637a0f4246..0134ffd72a 100644
--- a/hw/timer/ds1338.c
+++ b/hw/timer/ds1338.c
@@ -110,7 +110,10 @@ static void capture_current_time(DS1338State *s)
/* 24 hour mode. */
s->nvram[R_HOUR] = 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);
@@ -161,6 +164,46 @@ 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 = {};
+
+ /* 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));
+ }
+ 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)
{
DS1338State *s = DS1338(i2c);
@@ -170,52 +213,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. */
@@ -227,6 +225,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
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PATCH 06/14] tests: ds-rtc test 12 hour mode
2018-07-05 18:19 [Qemu-devel] [PATCH 00/14] Generalize Dallas/Maxim I2C RTC devices v3 Michael Davidsaver
` (4 preceding siblings ...)
2018-07-05 18:19 ` [Qemu-devel] [PATCH 05/14] timer: ds1338 change write handling and fix wday_offset handling Michael Davidsaver
@ 2018-07-05 18:19 ` Michael Davidsaver
2018-07-07 17:49 ` [Qemu-devel] [PATCH v2 " Michael Davidsaver
2018-07-05 18:19 ` [Qemu-devel] [PATCH 07/14] tests: ds-rtc test wday offset Michael Davidsaver
` (7 subsequent siblings)
13 siblings, 1 reply; 31+ messages in thread
From: Michael Davidsaver @ 2018-07-05 18:19 UTC (permalink / raw)
To: Peter Maydell, Paolo Bonzini
Cc: Thomas Huth, Antoine Mathys, David Gibson, qemu-devel,
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
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PATCH 07/14] tests: ds-rtc test wday offset
2018-07-05 18:19 [Qemu-devel] [PATCH 00/14] Generalize Dallas/Maxim I2C RTC devices v3 Michael Davidsaver
` (5 preceding siblings ...)
2018-07-05 18:19 ` [Qemu-devel] [PATCH 06/14] tests: ds-rtc test 12 hour mode Michael Davidsaver
@ 2018-07-05 18:19 ` Michael Davidsaver
2018-07-07 17:50 ` [Qemu-devel] [PATCH v2 " Michael Davidsaver
2018-07-05 18:19 ` [Qemu-devel] [PATCH 08/14] timer: rename ds1338 -> dsrtc Michael Davidsaver
` (6 subsequent siblings)
13 siblings, 1 reply; 31+ messages in thread
From: Michael Davidsaver @ 2018-07-05 18:19 UTC (permalink / raw)
To: Peter Maydell, Paolo Bonzini
Cc: Thomas Huth, Antoine Mathys, David Gibson, qemu-devel,
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
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PATCH 08/14] timer: rename ds1338 -> dsrtc
2018-07-05 18:19 [Qemu-devel] [PATCH 00/14] Generalize Dallas/Maxim I2C RTC devices v3 Michael Davidsaver
` (6 preceding siblings ...)
2018-07-05 18:19 ` [Qemu-devel] [PATCH 07/14] tests: ds-rtc test wday offset Michael Davidsaver
@ 2018-07-05 18:19 ` Michael Davidsaver
2018-07-05 18:19 ` [Qemu-devel] [PATCH 09/14] timer: rename file ds1338.c -> ds-rtc.c Michael Davidsaver
` (5 subsequent siblings)
13 siblings, 0 replies; 31+ messages in thread
From: Michael Davidsaver @ 2018-07-05 18:19 UTC (permalink / raw)
To: Peter Maydell, Paolo Bonzini
Cc: Thomas Huth, Antoine Mathys, David Gibson, qemu-devel,
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>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/timer/ds1338.c | 72 +++++++++++++++++++++++++++----------------------------
1 file changed, 36 insertions(+), 36 deletions(-)
diff --git a/hw/timer/ds1338.c b/hw/timer/ds1338.c
index 0134ffd72a..3c5781d53c 100644
--- a/hw/timer/ds1338.c
+++ b/hw/timer/ds1338.c
@@ -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.
@@ -119,7 +119,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
@@ -131,9 +131,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:
@@ -154,9 +154,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];
@@ -167,7 +167,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 = {};
@@ -204,9 +204,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);
@@ -226,15 +226,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;
@@ -244,28 +244,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
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PATCH 09/14] timer: rename file ds1338.c -> ds-rtc.c
2018-07-05 18:19 [Qemu-devel] [PATCH 00/14] Generalize Dallas/Maxim I2C RTC devices v3 Michael Davidsaver
` (7 preceding siblings ...)
2018-07-05 18:19 ` [Qemu-devel] [PATCH 08/14] timer: rename ds1338 -> dsrtc Michael Davidsaver
@ 2018-07-05 18:19 ` Michael Davidsaver
2018-07-05 18:19 ` [Qemu-devel] [PATCH 10/14] timer: ds1338 remove vestige of un-modeled OSF Michael Davidsaver
` (4 subsequent siblings)
13 siblings, 0 replies; 31+ messages in thread
From: Michael Davidsaver @ 2018-07-05 18:19 UTC (permalink / raw)
To: Peter Maydell, Paolo Bonzini
Cc: Thomas Huth, Antoine Mathys, David Gibson, qemu-devel,
Michael Davidsaver
Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
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 834d45cfaf..84f42b821b 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -34,7 +34,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 e16b2b913c..b3d3b80232 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_M41T80) += m41t80.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
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PATCH 10/14] timer: ds1338 remove vestige of un-modeled OSF
2018-07-05 18:19 [Qemu-devel] [PATCH 00/14] Generalize Dallas/Maxim I2C RTC devices v3 Michael Davidsaver
` (8 preceding siblings ...)
2018-07-05 18:19 ` [Qemu-devel] [PATCH 09/14] timer: rename file ds1338.c -> ds-rtc.c Michael Davidsaver
@ 2018-07-05 18:19 ` Michael Davidsaver
2018-07-16 4:26 ` David Gibson
2018-07-05 18:19 ` [Qemu-devel] [PATCH 12/14] timer: ds-rtc handle CENTURY bit Michael Davidsaver
` (3 subsequent siblings)
13 siblings, 1 reply; 31+ messages in thread
From: Michael Davidsaver @ 2018-07-05 18:19 UTC (permalink / raw)
To: Peter Maydell, Paolo Bonzini
Cc: Thomas Huth, Antoine Mathys, David Gibson, qemu-devel,
Michael Davidsaver
Oscillator stop has never been modeled, so the
Oscillator Stop Flag can never be set.
Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
---
hw/timer/ds-rtc.c | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/hw/timer/ds-rtc.c b/hw/timer/ds-rtc.c
index 3c5781d53c..126566ce1f 100644
--- a/hw/timer/ds-rtc.c
+++ b/hw/timer/ds-rtc.c
@@ -21,8 +21,6 @@
*/
#define NVRAM_SIZE 64
-#define CTRL_OSF 0x20
-
#define TYPE_DSRTC "ds1338"
#define DSRTC(obj) OBJECT_CHECK(DSRTCState, (obj), TYPE_DSRTC)
@@ -216,13 +214,11 @@ static int dsrtc_send(I2CSlave *i2c, uint8_t data)
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);
-
+ /* Allow guest to set no-op controls for clock out pin and
+ * rate select. Ignore write 1 to clear OSF. We don't model
+ * oscillator stop, so it is never set.
+ */
+ data = data & 0x93;
}
s->nvram[s->ptr] = data;
if (s->ptr <= R_YEAR) {
--
2.11.0
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PATCH 12/14] timer: ds-rtc handle CENTURY bit
2018-07-05 18:19 [Qemu-devel] [PATCH 00/14] Generalize Dallas/Maxim I2C RTC devices v3 Michael Davidsaver
` (9 preceding siblings ...)
2018-07-05 18:19 ` [Qemu-devel] [PATCH 10/14] timer: ds1338 remove vestige of un-modeled OSF Michael Davidsaver
@ 2018-07-05 18:19 ` Michael Davidsaver
2018-07-16 9:43 ` David Gibson
2018-07-05 18:20 ` [Qemu-devel] [PATCH 13/14] timer: ds-rtc model ds1375 Michael Davidsaver
` (2 subsequent siblings)
13 siblings, 1 reply; 31+ messages in thread
From: Michael Davidsaver @ 2018-07-05 18:19 UTC (permalink / raw)
To: Peter Maydell, Paolo Bonzini
Cc: Thomas Huth, Antoine Mathys, David Gibson, qemu-devel,
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
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PATCH 13/14] timer: ds-rtc model ds1375
2018-07-05 18:19 [Qemu-devel] [PATCH 00/14] Generalize Dallas/Maxim I2C RTC devices v3 Michael Davidsaver
` (10 preceding siblings ...)
2018-07-05 18:19 ` [Qemu-devel] [PATCH 12/14] timer: ds-rtc handle CENTURY bit Michael Davidsaver
@ 2018-07-05 18:20 ` Michael Davidsaver
2018-07-16 9:44 ` David Gibson
2018-07-05 18:20 ` [Qemu-devel] [PATCH 14/14] tests: drop ds1338-test Michael Davidsaver
[not found] ` <20180705182001.16537-12-mdavidsaver@gmail.com>
13 siblings, 1 reply; 31+ messages in thread
From: Michael Davidsaver @ 2018-07-05 18:20 UTC (permalink / raw)
To: Peter Maydell, Paolo Bonzini
Cc: Thomas Huth, Antoine Mathys, David Gibson, qemu-devel,
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>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
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 9abac38628..3a8ad1a00f 100644
--- a/hw/timer/ds-rtc.c
+++ b/hw/timer/ds-rtc.c
@@ -1,8 +1,9 @@
/*
- * MAXIM DS1338 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)
@@ -300,10 +302,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
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PATCH 14/14] tests: drop ds1338-test
2018-07-05 18:19 [Qemu-devel] [PATCH 00/14] Generalize Dallas/Maxim I2C RTC devices v3 Michael Davidsaver
` (11 preceding siblings ...)
2018-07-05 18:20 ` [Qemu-devel] [PATCH 13/14] timer: ds-rtc model ds1375 Michael Davidsaver
@ 2018-07-05 18:20 ` Michael Davidsaver
2018-07-17 15:23 ` Peter Maydell
[not found] ` <20180705182001.16537-12-mdavidsaver@gmail.com>
13 siblings, 1 reply; 31+ messages in thread
From: Michael Davidsaver @ 2018-07-05 18:20 UTC (permalink / raw)
To: Peter Maydell, Paolo Bonzini
Cc: Thomas Huth, Antoine Mathys, David Gibson, qemu-devel,
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 6ce5a9ff4d..8cf438260b 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -377,7 +377,6 @@ check-qtest-sparc64-y += tests/boot-serial-test$(EXESUF)
check-qtest-arm-y = tests/tmp105-test$(EXESUF)
check-qtest-arm-y += tests/pca9552-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)
@@ -788,7 +787,6 @@ 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/pca9552-test$(EXESUF): tests/pca9552-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
^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH 02/14] timer: ds1338 use registerfields.h
2018-07-05 18:19 ` [Qemu-devel] [PATCH 02/14] timer: ds1338 use registerfields.h Michael Davidsaver
@ 2018-07-06 1:35 ` David Gibson
0 siblings, 0 replies; 31+ messages in thread
From: David Gibson @ 2018-07-06 1:35 UTC (permalink / raw)
To: Michael Davidsaver
Cc: Peter Maydell, Paolo Bonzini, Thomas Huth, Antoine Mathys, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 5917 bytes --]
On Thu, Jul 05, 2018 at 11:19:49AM -0700, Michael Davidsaver 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>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> hw/timer/ds1338.c | 80 +++++++++++++++++++++++++++++++++++++------------------
> 1 file changed, 54 insertions(+), 26 deletions(-)
>
> diff --git a/hw/timer/ds1338.c b/hw/timer/ds1338.c
> index 3849b74a68..7298c5af43 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,25 @@ 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;
> }
> if (tmp <= 12) {
> - s->nvram[2] = HOURS_12 | to_bcd(tmp);
> + s->nvram[R_HOUR] = R_HOUR_SET12_MASK | to_bcd(tmp);
> } else {
> - s->nvram[2] = HOURS_12 | HOURS_PM | to_bcd(tmp - 12);
> + s->nvram[R_HOUR] = R_HOUR_SET12_MASK | R_HOUR_AMPM_MASK | to_bcd(tmp - 12);
> }
> } else {
> - s->nvram[2] = to_bcd(now.tm_hour);
> + s->nvram[R_HOUR] = 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 +169,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 +187,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 +199,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. */
--
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 --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH 03/14] timer: ds1338 persist 12-hour mode selection
2018-07-05 18:19 ` [Qemu-devel] [PATCH 03/14] timer: ds1338 persist 12-hour mode selection Michael Davidsaver
@ 2018-07-06 3:39 ` David Gibson
2018-07-06 4:35 ` Michael Davidsaver
0 siblings, 1 reply; 31+ messages in thread
From: David Gibson @ 2018-07-06 3:39 UTC (permalink / raw)
To: Michael Davidsaver
Cc: Peter Maydell, Paolo Bonzini, Thomas Huth, Antoine Mathys, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1619 bytes --]
On Thu, Jul 05, 2018 at 11:19:50AM -0700, Michael Davidsaver wrote:
11;rgb:ffff/ffff/ffff> 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.
>
> This was only being done in two of three
> arms of this conditional block.
>
> Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
This looks dubious to me, or at least the explanation of it does. The
other branch of the conditional is covering different registers in the
device, which are part of the RTC component, rather than the NVRAM
area. I wouldn't necessarily expect them to persist data as a rule
the way the rest of the block does, even if this specific bit does
need to be preserved.
> ---
> 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 7298c5af43..b56db5852e 100644
> --- a/hw/timer/ds1338.c
> +++ b/hw/timer/ds1338.c
> @@ -220,10 +220,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;
> }
--
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 --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH 03/14] timer: ds1338 persist 12-hour mode selection
2018-07-06 3:39 ` David Gibson
@ 2018-07-06 4:35 ` Michael Davidsaver
2018-07-07 17:59 ` Michael Davidsaver
2018-07-09 5:09 ` David Gibson
0 siblings, 2 replies; 31+ messages in thread
From: Michael Davidsaver @ 2018-07-06 4:35 UTC (permalink / raw)
To: David Gibson
Cc: Peter Maydell, Paolo Bonzini, Thomas Huth, Antoine Mathys, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 2329 bytes --]
On 07/05/2018 08:39 PM, David Gibson wrote:
> On Thu, Jul 05, 2018 at 11:19:50AM -0700, Michael Davidsaver wrote:
> 11;rgb:ffff/ffff/ffff> 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.
>>
>> This was only being done in two of three
>> arms of this conditional block.
>>
>> Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
>
> This looks dubious to me, or at least the explanation of it does. The
> other branch of the conditional is covering different registers in the
> device, which are part of the RTC component, rather than the NVRAM
> area. I wouldn't necessarily expect them to persist data as a rule
> the way the rest of the block does, even if this specific bit does
> need to be preserved.
The fact that the above capture_current_time() included the line
> if (s->nvram[2] & HOURS_12) {
was enough to convince me that the original author intended to persist
the 12/24 hour mode in this way. There are certainly other ways to
accomplish this, but they would involved adding to the vmstate,
which I've tried to avoid in this iteration.
Also, I though I had test coverage of this bug. That's actually how I
noticed it to begin with. But it seems my later change to allow for a
slow test runner also stopped testing readback of the 12/24 hour mode bit.
It just silently uses whichever it reads. I'll be re-issuing an updated
version which restores this check. Then you will be able to easily
see the effect of reverting 'timer: ds1338 persist 12-hour mode selection'.
>> ---
>> 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 7298c5af43..b56db5852e 100644
>> --- a/hw/timer/ds1338.c
>> +++ b/hw/timer/ds1338.c
>> @@ -220,10 +220,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;
>> }
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* [Qemu-devel] [PATCH v2 06/14] tests: ds-rtc test 12 hour mode
2018-07-05 18:19 ` [Qemu-devel] [PATCH 06/14] tests: ds-rtc test 12 hour mode Michael Davidsaver
@ 2018-07-07 17:49 ` Michael Davidsaver
2018-07-09 6:49 ` David Gibson
0 siblings, 1 reply; 31+ messages in thread
From: Michael Davidsaver @ 2018-07-07 17:49 UTC (permalink / raw)
To: Peter Maydell, Paolo Bonzini, David Gibson
Cc: Antoine Mathys, qemu-devel, Michael Davidsaver
Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
---
tests/ds-rtc-common.h | 9 ++++---
tests/ds-rtc-current-test.c | 2 +-
tests/ds-rtc-set-test.c | 58 +++++++++++++++++++++++++++++++++++++++++++--
3 files changed, 63 insertions(+), 6 deletions(-)
diff --git a/tests/ds-rtc-common.h b/tests/ds-rtc-common.h
index c8e6c2bc5b..5bc7ab32a6 100644
--- a/tests/ds-rtc-common.h
+++ b/tests/ds-rtc-common.h
@@ -20,12 +20,15 @@ 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, int *mmode)
{
struct tm parts;
parts.tm_sec = from_bcd(buf[0]);
parts.tm_min = from_bcd(buf[1]);
+ if (mmode) {
+ *mmode = !!(buf[2] & 0x40);
+ }
if (buf[2] & 0x40) {
/* 12 hour */
/* HOUR register is 1-12. */
@@ -51,7 +54,7 @@ static inline time_t rtc_parse(const uint8_t *buf)
return mktimegm(&parts);
}
-static time_t rtc_gettime(void)
+static time_t rtc_gettime(int *mmode)
{
uint8_t buf[7];
@@ -61,7 +64,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, mmode);
}
#endif /* DSRTCCOMMON_H */
diff --git a/tests/ds-rtc-current-test.c b/tests/ds-rtc-current-test.c
index 6acbbed9a6..3c15482a9d 100644
--- a/tests/ds-rtc-current-test.c
+++ b/tests/ds-rtc-current-test.c
@@ -28,7 +28,7 @@ void test_rtc_current(void)
actual = time(NULL);
/* new second may start here */
- expected = rtc_gettime();
+ expected = rtc_gettime(NULL);
g_assert_cmpuint(expected, <=, actual + max_delta);
g_assert_cmpuint(expected, >=, actual);
}
diff --git a/tests/ds-rtc-set-test.c b/tests/ds-rtc-set-test.c
index 35e1a36281..3a742e897f 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)
@@ -76,16 +124,18 @@ void test_rtc_set(const void *raw)
const uint8_t *testtime = raw;
time_t expected, actual;
+ int mode_expect, mode_actual;
/* skip address pointer and parse remainder */
- expected = rtc_parse(&testtime[1]);
+ expected = rtc_parse(&testtime[1], &mode_expect);
i2c_send(i2c, addr, testtime, 8);
/* host may start new second here */
- actual = rtc_gettime();
+ actual = rtc_gettime(&mode_actual);
g_assert_cmpuint(expected, <=, actual);
g_assert_cmpuint(expected + max_delta, >=, actual);
+ g_assert_cmpint(mode_expect, ==, mode_actual);
}
int main(int argc, char *argv[])
@@ -108,6 +158,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
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PATCH v2 07/14] tests: ds-rtc test wday offset
2018-07-05 18:19 ` [Qemu-devel] [PATCH 07/14] tests: ds-rtc test wday offset Michael Davidsaver
@ 2018-07-07 17:50 ` Michael Davidsaver
2018-07-09 6:50 ` David Gibson
0 siblings, 1 reply; 31+ messages in thread
From: Michael Davidsaver @ 2018-07-07 17:50 UTC (permalink / raw)
To: Peter Maydell, Paolo Bonzini, David Gibson
Cc: Antoine Mathys, qemu-devel, 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 5bc7ab32a6..782ea60453 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, int *mmode)
+static inline time_t rtc_parse(const uint8_t *buf, unsigned *wday, int *mmode)
{
struct tm parts;
@@ -51,10 +51,14 @@ static inline time_t rtc_parse(const uint8_t *buf, int *mmode)
parts.tm_year += 100u;
}
+ if (wday) {
+ *wday = parts.tm_wday;
+ }
+
return mktimegm(&parts);
}
-static time_t rtc_gettime(int *mmode)
+static time_t rtc_gettime(unsigned *wday, int *mmode)
{
uint8_t buf[7];
@@ -64,7 +68,7 @@ static time_t rtc_gettime(int *mmode)
/* read back current time registers */
i2c_recv(i2c, addr, buf, 7);
- return rtc_parse(buf, mmode);
+ return rtc_parse(buf, wday, mmode);
}
#endif /* DSRTCCOMMON_H */
diff --git a/tests/ds-rtc-current-test.c b/tests/ds-rtc-current-test.c
index 3c15482a9d..08d8411671 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(NULL);
+ expected = rtc_gettime(&wday_expect, NULL);
+
+ 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 3a742e897f..1004470931 100644
--- a/tests/ds-rtc-set-test.c
+++ b/tests/ds-rtc-set-test.c
@@ -124,17 +124,19 @@ void test_rtc_set(const void *raw)
const uint8_t *testtime = raw;
time_t expected, actual;
+ unsigned wday_expect, wday_actual;
int mode_expect, mode_actual;
/* skip address pointer and parse remainder */
- expected = rtc_parse(&testtime[1], &mode_expect);
+ expected = rtc_parse(&testtime[1], &wday_expect, &mode_expect);
i2c_send(i2c, addr, testtime, 8);
/* host may start new second here */
- actual = rtc_gettime(&mode_actual);
+ actual = rtc_gettime(&wday_actual, &mode_actual);
g_assert_cmpuint(expected, <=, actual);
g_assert_cmpuint(expected + max_delta, >=, actual);
+ g_assert_cmpuint(wday_expect, ==, wday_actual);
g_assert_cmpint(mode_expect, ==, mode_actual);
}
--
2.11.0
^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH 03/14] timer: ds1338 persist 12-hour mode selection
2018-07-06 4:35 ` Michael Davidsaver
@ 2018-07-07 17:59 ` Michael Davidsaver
2018-07-09 5:09 ` David Gibson
1 sibling, 0 replies; 31+ messages in thread
From: Michael Davidsaver @ 2018-07-07 17:59 UTC (permalink / raw)
To: David Gibson
Cc: Peter Maydell, Paolo Bonzini, Thomas Huth, Antoine Mathys, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1059 bytes --]
On 07/05/2018 09:35 PM, Michael Davidsaver wrote:
> Also, I though I had test coverage of this bug. That's actually how I
> noticed it to begin with. But it seems my later change to allow for a
> slow test runner also stopped testing readback of the 12/24 hour mode bit.
> It just silently uses whichever it reads. I'll be re-issuing an updated
> version which restores this check. Then you will be able to easily
> see the effect of reverting 'timer: ds1338 persist 12-hour mode selection'.
I've posted revised versions of two of the three test patches #6 and #7
(which I've hopefully posted correctly...). #6 again tests for this issue.
So you can demonstrate the problem by either applying 1,2,4-6, or just 1 and 6
to see that the issue is present in the original implementation.
The test failure should be:
> test_rtc_set: assertion failed (mode_expect == mode_actual): (1 == 0)
Which shows that a write with 12 hour mode is read back as 24 hour mode.
Similarly, omitting patch #5 will cause the tests added in #7 to fail.
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH 03/14] timer: ds1338 persist 12-hour mode selection
2018-07-06 4:35 ` Michael Davidsaver
2018-07-07 17:59 ` Michael Davidsaver
@ 2018-07-09 5:09 ` David Gibson
1 sibling, 0 replies; 31+ messages in thread
From: David Gibson @ 2018-07-09 5:09 UTC (permalink / raw)
To: Michael Davidsaver
Cc: Peter Maydell, Paolo Bonzini, Thomas Huth, Antoine Mathys, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 2997 bytes --]
On Thu, Jul 05, 2018 at 09:35:50PM -0700, Michael Davidsaver wrote:
> On 07/05/2018 08:39 PM, David Gibson wrote:
> > On Thu, Jul 05, 2018 at 11:19:50AM -0700, Michael Davidsaver wrote:
> > 11;rgb:ffff/ffff/ffff> 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.
> >>
> >> This was only being done in two of three
> >> arms of this conditional block.
> >>
> >> Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
> >
> > This looks dubious to me, or at least the explanation of it does. The
> > other branch of the conditional is covering different registers in the
> > device, which are part of the RTC component, rather than the NVRAM
> > area. I wouldn't necessarily expect them to persist data as a rule
> > the way the rest of the block does, even if this specific bit does
> > need to be preserved.
>
> The fact that the above capture_current_time() included the line
>
> > if (s->nvram[2] & HOURS_12) {
>
> was enough to convince me that the original author intended to persist
> the 12/24 hour mode in this way. There are certainly other ways to
> accomplish this, but they would involved adding to the vmstate,
> which I've tried to avoid in this iteration.
Ah, yes, I see your point. I was more unsure about whether it was
safe to also persist the other early bytes of the region, which this
patch also does. But I can't really see how it would do any harm, so:
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
>
>
> Also, I though I had test coverage of this bug. That's actually how I
> noticed it to begin with. But it seems my later change to allow for a
> slow test runner also stopped testing readback of the 12/24 hour mode bit.
> It just silently uses whichever it reads. I'll be re-issuing an updated
> version which restores this check. Then you will be able to easily
> see the effect of reverting 'timer: ds1338 persist 12-hour mode selection'.
>
>
>
> >> ---
> >> 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 7298c5af43..b56db5852e 100644
> >> --- a/hw/timer/ds1338.c
> >> +++ b/hw/timer/ds1338.c
> >> @@ -220,10 +220,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;
> >> }
> >
>
>
--
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 --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH 04/14] timer: ds1338 clarify HOUR handling
2018-07-05 18:19 ` [Qemu-devel] [PATCH 04/14] timer: ds1338 clarify HOUR handling Michael Davidsaver
@ 2018-07-09 5:12 ` David Gibson
0 siblings, 0 replies; 31+ messages in thread
From: David Gibson @ 2018-07-09 5:12 UTC (permalink / raw)
To: Michael Davidsaver
Cc: Peter Maydell, Paolo Bonzini, Thomas Huth, Antoine Mathys, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1779 bytes --]
On Thu, Jul 05, 2018 at 11:19:51AM -0700, Michael Davidsaver wrote:
> Simplify and comment the translation between
> registers and struct tm.
>
> Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
although..
[snip]
> @@ -101,7 +105,9 @@ static void capture_current_time(DS1338State *s)
> } else {
> s->nvram[R_HOUR] = R_HOUR_SET12_MASK | R_HOUR_AMPM_MASK | to_bcd(tmp - 12);
> }
> +
> } else {
I'm not real fond of blank lines before the ends of blocks.
> + /* 24 hour mode. */
> s->nvram[R_HOUR] = to_bcd(now.tm_hour);
> }
> s->nvram[R_WDAY] = (now.tm_wday + s->wday_offset) % 7 + 1;
> @@ -178,14 +184,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));
> }
--
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 --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH v2 06/14] tests: ds-rtc test 12 hour mode
2018-07-07 17:49 ` [Qemu-devel] [PATCH v2 " Michael Davidsaver
@ 2018-07-09 6:49 ` David Gibson
0 siblings, 0 replies; 31+ messages in thread
From: David Gibson @ 2018-07-09 6:49 UTC (permalink / raw)
To: Michael Davidsaver
Cc: Peter Maydell, Paolo Bonzini, Antoine Mathys, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 5821 bytes --]
On Sat, Jul 07, 2018 at 10:49:09AM -0700, Michael Davidsaver wrote:
> Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> tests/ds-rtc-common.h | 9 ++++---
> tests/ds-rtc-current-test.c | 2 +-
> tests/ds-rtc-set-test.c | 58 +++++++++++++++++++++++++++++++++++++++++++--
> 3 files changed, 63 insertions(+), 6 deletions(-)
>
> diff --git a/tests/ds-rtc-common.h b/tests/ds-rtc-common.h
> index c8e6c2bc5b..5bc7ab32a6 100644
> --- a/tests/ds-rtc-common.h
> +++ b/tests/ds-rtc-common.h
> @@ -20,12 +20,15 @@ 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, int *mmode)
> {
> struct tm parts;
>
> parts.tm_sec = from_bcd(buf[0]);
> parts.tm_min = from_bcd(buf[1]);
> + if (mmode) {
> + *mmode = !!(buf[2] & 0x40);
> + }
> if (buf[2] & 0x40) {
> /* 12 hour */
> /* HOUR register is 1-12. */
> @@ -51,7 +54,7 @@ static inline time_t rtc_parse(const uint8_t *buf)
> return mktimegm(&parts);
> }
>
> -static time_t rtc_gettime(void)
> +static time_t rtc_gettime(int *mmode)
> {
> uint8_t buf[7];
>
> @@ -61,7 +64,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, mmode);
> }
>
> #endif /* DSRTCCOMMON_H */
> diff --git a/tests/ds-rtc-current-test.c b/tests/ds-rtc-current-test.c
> index 6acbbed9a6..3c15482a9d 100644
> --- a/tests/ds-rtc-current-test.c
> +++ b/tests/ds-rtc-current-test.c
> @@ -28,7 +28,7 @@ void test_rtc_current(void)
>
> actual = time(NULL);
> /* new second may start here */
> - expected = rtc_gettime();
> + expected = rtc_gettime(NULL);
> g_assert_cmpuint(expected, <=, actual + max_delta);
> g_assert_cmpuint(expected, >=, actual);
> }
> diff --git a/tests/ds-rtc-set-test.c b/tests/ds-rtc-set-test.c
> index 35e1a36281..3a742e897f 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)
> @@ -76,16 +124,18 @@ void test_rtc_set(const void *raw)
>
> const uint8_t *testtime = raw;
> time_t expected, actual;
> + int mode_expect, mode_actual;
>
> /* skip address pointer and parse remainder */
> - expected = rtc_parse(&testtime[1]);
> + expected = rtc_parse(&testtime[1], &mode_expect);
>
> i2c_send(i2c, addr, testtime, 8);
> /* host may start new second here */
> - actual = rtc_gettime();
> + actual = rtc_gettime(&mode_actual);
>
> g_assert_cmpuint(expected, <=, actual);
> g_assert_cmpuint(expected + max_delta, >=, actual);
> + g_assert_cmpint(mode_expect, ==, mode_actual);
> }
>
> int main(int argc, char *argv[])
> @@ -108,6 +158,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();
>
--
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 --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH v2 07/14] tests: ds-rtc test wday offset
2018-07-07 17:50 ` [Qemu-devel] [PATCH v2 " Michael Davidsaver
@ 2018-07-09 6:50 ` David Gibson
0 siblings, 0 replies; 31+ messages in thread
From: David Gibson @ 2018-07-09 6:50 UTC (permalink / raw)
To: Michael Davidsaver
Cc: Peter Maydell, Paolo Bonzini, Antoine Mathys, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 3714 bytes --]
On Sat, Jul 07, 2018 at 10:50:39AM -0700, Michael Davidsaver wrote:
> Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> 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 5bc7ab32a6..782ea60453 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, int *mmode)
> +static inline time_t rtc_parse(const uint8_t *buf, unsigned *wday, int *mmode)
> {
> struct tm parts;
>
> @@ -51,10 +51,14 @@ static inline time_t rtc_parse(const uint8_t *buf, int *mmode)
> parts.tm_year += 100u;
> }
>
> + if (wday) {
> + *wday = parts.tm_wday;
> + }
> +
> return mktimegm(&parts);
> }
>
> -static time_t rtc_gettime(int *mmode)
> +static time_t rtc_gettime(unsigned *wday, int *mmode)
> {
> uint8_t buf[7];
>
> @@ -64,7 +68,7 @@ static time_t rtc_gettime(int *mmode)
> /* read back current time registers */
> i2c_recv(i2c, addr, buf, 7);
>
> - return rtc_parse(buf, mmode);
> + return rtc_parse(buf, wday, mmode);
> }
>
> #endif /* DSRTCCOMMON_H */
> diff --git a/tests/ds-rtc-current-test.c b/tests/ds-rtc-current-test.c
> index 3c15482a9d..08d8411671 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(NULL);
> + expected = rtc_gettime(&wday_expect, NULL);
> +
> + 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 3a742e897f..1004470931 100644
> --- a/tests/ds-rtc-set-test.c
> +++ b/tests/ds-rtc-set-test.c
> @@ -124,17 +124,19 @@ void test_rtc_set(const void *raw)
>
> const uint8_t *testtime = raw;
> time_t expected, actual;
> + unsigned wday_expect, wday_actual;
> int mode_expect, mode_actual;
>
> /* skip address pointer and parse remainder */
> - expected = rtc_parse(&testtime[1], &mode_expect);
> + expected = rtc_parse(&testtime[1], &wday_expect, &mode_expect);
>
> i2c_send(i2c, addr, testtime, 8);
> /* host may start new second here */
> - actual = rtc_gettime(&mode_actual);
> + actual = rtc_gettime(&wday_actual, &mode_actual);
>
> g_assert_cmpuint(expected, <=, actual);
> g_assert_cmpuint(expected + max_delta, >=, actual);
> + g_assert_cmpuint(wday_expect, ==, wday_actual);
> g_assert_cmpint(mode_expect, ==, mode_actual);
> }
>
--
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 --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH 05/14] timer: ds1338 change write handling and fix wday_offset handling
2018-07-05 18:19 ` [Qemu-devel] [PATCH 05/14] timer: ds1338 change write handling and fix wday_offset handling Michael Davidsaver
@ 2018-07-16 4:25 ` David Gibson
0 siblings, 0 replies; 31+ messages in thread
From: David Gibson @ 2018-07-16 4:25 UTC (permalink / raw)
To: Michael Davidsaver
Cc: Peter Maydell, Paolo Bonzini, Thomas Huth, Antoine Mathys, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 5605 bytes --]
On Thu, Jul 05, 2018 at 11:19:52AM -0700, Michael Davidsaver 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.
>
> 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 | 95 ++++++++++++++++++++++++++++---------------------------
> 1 file changed, 48 insertions(+), 47 deletions(-)
>
> diff --git a/hw/timer/ds1338.c b/hw/timer/ds1338.c
> index 637a0f4246..0134ffd72a 100644
> --- a/hw/timer/ds1338.c
> +++ b/hw/timer/ds1338.c
> @@ -110,7 +110,10 @@ static void capture_current_time(DS1338State *s)
> /* 24 hour mode. */
> s->nvram[R_HOUR] = 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;
> + }
This looks a bit odd - you've removed the + 1 here, but...
> 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);
> @@ -161,6 +164,46 @@ 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 = {};
> +
> + /* 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));
> + }
> + now.tm_wday = from_bcd(s->nvram[R_WDAY]) - 1u;
.. you still have the -1 here. Am I missing something?
> + 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)
> {
> DS1338State *s = DS1338(i2c);
> @@ -170,52 +213,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. */
> @@ -227,6 +225,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;
> }
--
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 --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH 10/14] timer: ds1338 remove vestige of un-modeled OSF
2018-07-05 18:19 ` [Qemu-devel] [PATCH 10/14] timer: ds1338 remove vestige of un-modeled OSF Michael Davidsaver
@ 2018-07-16 4:26 ` David Gibson
0 siblings, 0 replies; 31+ messages in thread
From: David Gibson @ 2018-07-16 4:26 UTC (permalink / raw)
To: Michael Davidsaver
Cc: Peter Maydell, Paolo Bonzini, Thomas Huth, Antoine Mathys, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1685 bytes --]
On Thu, Jul 05, 2018 at 11:19:57AM -0700, Michael Davidsaver wrote:
> Oscillator stop has never been modeled, so the
> Oscillator Stop Flag can never be set.
>
> Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> hw/timer/ds-rtc.c | 14 +++++---------
> 1 file changed, 5 insertions(+), 9 deletions(-)
>
> diff --git a/hw/timer/ds-rtc.c b/hw/timer/ds-rtc.c
> index 3c5781d53c..126566ce1f 100644
> --- a/hw/timer/ds-rtc.c
> +++ b/hw/timer/ds-rtc.c
> @@ -21,8 +21,6 @@
> */
> #define NVRAM_SIZE 64
>
> -#define CTRL_OSF 0x20
> -
> #define TYPE_DSRTC "ds1338"
> #define DSRTC(obj) OBJECT_CHECK(DSRTCState, (obj), TYPE_DSRTC)
>
> @@ -216,13 +214,11 @@ static int dsrtc_send(I2CSlave *i2c, uint8_t data)
> 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);
> -
> + /* Allow guest to set no-op controls for clock out pin and
> + * rate select. Ignore write 1 to clear OSF. We don't model
> + * oscillator stop, so it is never set.
> + */
> + data = data & 0x93;
> }
> s->nvram[s->ptr] = data;
> if (s->ptr <= R_YEAR) {
--
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 --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH 11/14] timer: generalize ds1338
[not found] ` <20180705182001.16537-12-mdavidsaver@gmail.com>
@ 2018-07-16 9:19 ` David Gibson
0 siblings, 0 replies; 31+ messages in thread
From: David Gibson @ 2018-07-16 9:19 UTC (permalink / raw)
To: Michael Davidsaver
Cc: Peter Maydell, Paolo Bonzini, Thomas Huth, Antoine Mathys, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 4729 bytes --]
On Thu, Jul 05, 2018 at 11:19:58AM -0700, Michael Davidsaver wrote:
> Add class level handling for address space size
> and control register.
>
> Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Although I don't love the name "addr_size" - just "nvram_size" seems
clearer.
> ---
> hw/timer/ds-rtc.c | 62 +++++++++++++++++++++++++++++++++++++++++++------------
> 1 file changed, 49 insertions(+), 13 deletions(-)
>
> diff --git a/hw/timer/ds-rtc.c b/hw/timer/ds-rtc.c
> index 126566ce1f..6d3a8b5586 100644
> --- a/hw/timer/ds-rtc.c
> +++ b/hw/timer/ds-rtc.c
> @@ -21,8 +21,10 @@
> */
> #define NVRAM_SIZE 64
>
> -#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 */
> @@ -38,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)
> @@ -65,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 +130,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);
> }
> @@ -205,20 +217,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. */
> -
> - /* Allow guest to set no-op controls for clock out pin and
> - * rate select. Ignore write 1 to clear OSF. We don't model
> - * oscillator stop, so it is never set.
> - */
> - data = data & 0x93;
> + if (s->ptr == k->ctrl_offset) {
> + (k->ctrl_write)(s, data);
> }
> s->nvram[s->ptr] = data;
> if (s->ptr <= R_YEAR) {
> @@ -253,15 +260,44 @@ 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 and
> + * rate select. Ignore write 1 to clear OSF. We don't model
> + * oscillator stop, so it is never set.
> + */
> + 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)
--
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 --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH 12/14] timer: ds-rtc handle CENTURY bit
2018-07-05 18:19 ` [Qemu-devel] [PATCH 12/14] timer: ds-rtc handle CENTURY bit Michael Davidsaver
@ 2018-07-16 9:43 ` David Gibson
0 siblings, 0 replies; 31+ messages in thread
From: David Gibson @ 2018-07-16 9:43 UTC (permalink / raw)
To: Michael Davidsaver
Cc: Peter Maydell, Paolo Bonzini, Thomas Huth, Antoine Mathys, qemu-devel
[-- 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 --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH 13/14] timer: ds-rtc model ds1375
2018-07-05 18:20 ` [Qemu-devel] [PATCH 13/14] timer: ds-rtc model ds1375 Michael Davidsaver
@ 2018-07-16 9:44 ` David Gibson
0 siblings, 0 replies; 31+ messages in thread
From: David Gibson @ 2018-07-16 9:44 UTC (permalink / raw)
To: Michael Davidsaver
Cc: Peter Maydell, Paolo Bonzini, Thomas Huth, Antoine Mathys, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 2419 bytes --]
On Thu, Jul 05, 2018 at 11:20:00AM -0700, Michael Davidsaver 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>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> 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 9abac38628..3a8ad1a00f 100644
> --- a/hw/timer/ds-rtc.c
> +++ b/hw/timer/ds-rtc.c
> @@ -1,8 +1,9 @@
> /*
> - * MAXIM DS1338 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)
> @@ -300,10 +302,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)
--
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 --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH 14/14] tests: drop ds1338-test
2018-07-05 18:20 ` [Qemu-devel] [PATCH 14/14] tests: drop ds1338-test Michael Davidsaver
@ 2018-07-17 15:23 ` Peter Maydell
0 siblings, 0 replies; 31+ messages in thread
From: Peter Maydell @ 2018-07-17 15:23 UTC (permalink / raw)
To: Michael Davidsaver
Cc: Paolo Bonzini, Thomas Huth, Antoine Mathys, David Gibson,
QEMU Developers
On 5 July 2018 at 19:20, Michael Davidsaver <mdavidsaver@gmail.com> wrote:
> 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
>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
thanks
-- PMM
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH 01/14] tests: more thorough tests of ds1338
2018-07-05 18:19 ` [Qemu-devel] [PATCH 01/14] tests: more thorough tests of ds1338 Michael Davidsaver
@ 2018-07-17 15:28 ` Peter Maydell
0 siblings, 0 replies; 31+ messages in thread
From: Peter Maydell @ 2018-07-17 15:28 UTC (permalink / raw)
To: Michael Davidsaver
Cc: Paolo Bonzini, Thomas Huth, Antoine Mathys, David Gibson,
QEMU Developers
On 5 July 2018 at 19:19, Michael Davidsaver <mdavidsaver@gmail.com> wrote:
> Test current time and set+get round trip.
>
> Separate current time test from set/get tests
> to avoid test needing to impose order, or to
> have a magic handshaketo reset RTC to current time.
>
> 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 8859e88ffb..6ce5a9ff4d 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -378,6 +378,8 @@ check-qtest-sparc64-y += tests/boot-serial-test$(EXESUF)
> check-qtest-arm-y = tests/tmp105-test$(EXESUF)
> check-qtest-arm-y += tests/pca9552-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)
> @@ -787,6 +789,8 @@ 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/pca9552-test$(EXESUF): tests/pca9552-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"
Headers should never include 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>
This is included for you by osdep.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) {
The makefile changes add this to check-qtest-arm-y,
so why do we need this if() ?
> + 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
Same comments as above apply to this file too.
thanks
-- PMM
^ permalink raw reply [flat|nested] 31+ messages in thread
end of thread, other threads:[~2018-07-17 15:29 UTC | newest]
Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-05 18:19 [Qemu-devel] [PATCH 00/14] Generalize Dallas/Maxim I2C RTC devices v3 Michael Davidsaver
2018-07-05 18:19 ` [Qemu-devel] [PATCH 01/14] tests: more thorough tests of ds1338 Michael Davidsaver
2018-07-17 15:28 ` Peter Maydell
2018-07-05 18:19 ` [Qemu-devel] [PATCH 02/14] timer: ds1338 use registerfields.h Michael Davidsaver
2018-07-06 1:35 ` David Gibson
2018-07-05 18:19 ` [Qemu-devel] [PATCH 03/14] timer: ds1338 persist 12-hour mode selection Michael Davidsaver
2018-07-06 3:39 ` David Gibson
2018-07-06 4:35 ` Michael Davidsaver
2018-07-07 17:59 ` Michael Davidsaver
2018-07-09 5:09 ` David Gibson
2018-07-05 18:19 ` [Qemu-devel] [PATCH 04/14] timer: ds1338 clarify HOUR handling Michael Davidsaver
2018-07-09 5:12 ` David Gibson
2018-07-05 18:19 ` [Qemu-devel] [PATCH 05/14] timer: ds1338 change write handling and fix wday_offset handling Michael Davidsaver
2018-07-16 4:25 ` David Gibson
2018-07-05 18:19 ` [Qemu-devel] [PATCH 06/14] tests: ds-rtc test 12 hour mode Michael Davidsaver
2018-07-07 17:49 ` [Qemu-devel] [PATCH v2 " Michael Davidsaver
2018-07-09 6:49 ` David Gibson
2018-07-05 18:19 ` [Qemu-devel] [PATCH 07/14] tests: ds-rtc test wday offset Michael Davidsaver
2018-07-07 17:50 ` [Qemu-devel] [PATCH v2 " Michael Davidsaver
2018-07-09 6:50 ` David Gibson
2018-07-05 18:19 ` [Qemu-devel] [PATCH 08/14] timer: rename ds1338 -> dsrtc Michael Davidsaver
2018-07-05 18:19 ` [Qemu-devel] [PATCH 09/14] timer: rename file ds1338.c -> ds-rtc.c Michael Davidsaver
2018-07-05 18:19 ` [Qemu-devel] [PATCH 10/14] timer: ds1338 remove vestige of un-modeled OSF Michael Davidsaver
2018-07-16 4:26 ` David Gibson
2018-07-05 18:19 ` [Qemu-devel] [PATCH 12/14] timer: ds-rtc handle CENTURY bit Michael Davidsaver
2018-07-16 9:43 ` David Gibson
2018-07-05 18:20 ` [Qemu-devel] [PATCH 13/14] timer: ds-rtc model ds1375 Michael Davidsaver
2018-07-16 9:44 ` David Gibson
2018-07-05 18:20 ` [Qemu-devel] [PATCH 14/14] tests: drop ds1338-test Michael Davidsaver
2018-07-17 15:23 ` Peter Maydell
[not found] ` <20180705182001.16537-12-mdavidsaver@gmail.com>
2018-07-16 9:19 ` [Qemu-devel] [PATCH 11/14] timer: generalize ds1338 David Gibson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.