[Qemu-devel] [PATCH] hw/ds1338.c: implement clock enable/disable (CH bit)
Signed-off-by: Antoine Mathys barsa...@gmail.com
---
hw/ds1338.c | 156 ---
1 file changed, 95 insertions(+), 61 deletions(-)
diff --git a/hw/ds1338.c b/hw/ds1338.c
index 1da0f96..5a93fb6 100644
--- a/hw/ds1338.c
+++ b/hw/ds1338.c
@@ -48,17 +48,32 @@ static const VMStateDescription vmstate_ds1338 = {
}
};
-static void capture_current_time(DS1338State *s)
+/* This mask is used to clear the read as zero bits in the RTC registers */
+static const uint8_t nvram_mask[8] = {
+0xff, 0x7f, 0x7f, 0x7, 0x3f, 0x1f, 0xff, 0xb3
+};
+
+
+static int compute_wday(int y, int m, int d)
{
-/* Capture the current time into the secondary registers
- * which will be actually read by the data transfer operation.
- */
-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);
+static int t[12] = {0, 3, 2, 5, 0, 3, 5, 1, 4, 6, 2, 4};
+
+if (m 2) {
+y--;
+}
+return (y + y/4 - y/100 + y/400 + t[m] + d) % 7;
+}
+
+/* Write TM to the RTC registers. */
+static void write_time(DS1338State *s, const struct tm *tm)
+{
+/* Preserve the CH flag. */
+s-nvram[0] = SECONDS_CH;
+s-nvram[0] |= to_bcd(tm-tm_sec);
+
+s-nvram[1] = to_bcd(tm-tm_min);
if (s-nvram[2] HOURS_12) {
-int tmp = now.tm_hour;
+int tmp = tm-tm_hour;
if (tmp % 12 == 0) {
tmp += 12;
}
@@ -68,12 +83,50 @@ static void capture_current_time(DS1338State *s)
s-nvram[2] = HOURS_12 | HOURS_PM | to_bcd(tmp - 12);
}
} else {
-s-nvram[2] = to_bcd(now.tm_hour);
+s-nvram[2] = to_bcd(tm-tm_hour);
+}
+s-nvram[3] = (tm-tm_wday + s-wday_offset) % 7 + 1;
+s-nvram[4] = to_bcd(tm-tm_mday);
+s-nvram[5] = to_bcd(tm-tm_mon + 1);
+s-nvram[6] = to_bcd(tm-tm_year - 100);
+}
+
+/* Read TM from the RTC registers. */
+static void read_time(DS1338State *s, struct tm *tm)
+{
+tm-tm_sec = from_bcd(s-nvram[0] 0x7f);
+tm-tm_min = from_bcd(s-nvram[1] 0x7f);
+if (s-nvram[2] HOURS_12) {
+int tmp = from_bcd(s-nvram[2] (HOURS_PM - 1));
+if (s-nvram[2] HOURS_PM) {
+tmp += 12;
+}
+if (tmp % 12 == 0) {
+tmp -= 12;
+}
+tm-tm_hour = tmp;
+} else {
+tm-tm_hour = from_bcd(s-nvram[2] (HOURS_12 - 1));
+}
+tm-tm_mday = from_bcd(s-nvram[4] 0x3f);
+tm-tm_mon = from_bcd(s-nvram[5] 0x1f) - 1;
+tm-tm_year = from_bcd(s-nvram[6]) + 100;
+tm-tm_wday = compute_wday(tm-tm_year + 1900, tm-tm_mon, tm-tm_mday);
+}
+
+static bool clock_running(DS1338State *s)
+{
+return !(s-nvram[0] SECONDS_CH);
+}
+
+static void capture_current_time(DS1338State *s)
+{
+if (clock_running(s)) {
+/* Write current time. */
+struct tm tmp;
+qemu_get_timedate(tmp, s-offset);
+write_time(s, tmp);
}
-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);
}
static void inc_regptr(DS1338State *s)
@@ -129,65 +182,46 @@ static int ds1338_send(I2CSlave *i2c, uint8_t data)
}
if (s-ptr 7) {
/* Time register. */
-struct tm now;
-qemu_get_timedate(now, s-offset);
-switch(s-ptr) {
-case 0:
-/* TODO: Implement CH (stop) bit. */
-now.tm_sec = from_bcd(data 0x7f);
-break;
-case 1:
-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) {
-tmp += 12;
-}
-if (tmp % 12 == 0) {
-tmp -= 12;
-}
-now.tm_hour = tmp;
-} else {
-now.tm_hour = from_bcd(data (HOURS_12 - 1));
-}
-break;
-case 3:
-{
-/* 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;
+bool was_running = clock_running(s);
+
+capture_current_time(s);
+
+s-nvram[s-ptr] = data nvram_mask[s-ptr];
+
+if (clock_running(s)) {
+/* Read the new time */
+struct tm tmp;
+int user_wday;
+
+read_time(s, tmp);
+s-offset = qemu_timedate_diff(tmp);
+
+/* The day field is supposed to contain a value in
+ the range 1-7. Otherwise behavior is undefined.
+*/
+user_wday = (s-nvram[3] 7) - 1;
+s-wday_offset = (user_wday - tmp.tm_wday + 7
Re: [Qemu-devel] [PATCH] Fix conversion between 12 hours and 24 hours modes.
On 02/15/2013 12:24 PM, Andreas Färber wrote: The expected answer would've been take guest X and do Y to see Z, or better to extend the existing qtest cases to prove something was broken before and fixed afterwards and to avoid the same bug being reintroduced later. If we are talking about adding a test case in order to have some guarantee that what works after a fix keeps working in the future, that's fine. And I am willing to add such tests for the DS1338 implementation (once it is finished). But demanding a test case that the code passes with the fix but fails without, in order to prove that something was broken before, is only reasonable if the bug was found through testing in the first place. It is inappropriate for a bug found in a code review. Not only do you not need a test case to prove the bug exists, but reverse-engineering a test-case can be a significant undertaking. Paolo tried to do that unsuccessfully in the case of bug 1090558 and I had no reason to think I could do better. This does not strike me as a very productive use of developer time anyway. And your suggestion that it is better to leave a known bug unpatched until someone can conjure up a test case is ridiculous. I don't see how that attitude help users, in the short or long term. If you don't nuance your position you are only going to discourage much needed code reviews. I don't see what good can come of that.
Re: [Qemu-devel] [PATCH] hw/mc146818rtc.c: Fix reading and writing of time registers
On 02/14/2013 10:30 AM, Paolo Bonzini wrote: Nice. Do you have a testcase? No, but the refactoring makes the code very easy to audit. Please also test this patch with the two rtc-test patches at http://thread.gmane.org/gmane.comp.emulators.qemu/188244. I did and the tests pass.
Re: [Qemu-devel] [PATCH] Fix conversion between 12 hours and 24 hours modes.
First, the ds1338 code was in a poor state and never handled the 12 hour clock correctly. My first patch failed to fully fix the problem so I had to write a second one, but at no point did Peter or I introduce a regression, quite the opposite. Second, I don't know where you got the idea that I refuse to write test cases. I just didn't have one ready or in the works at the time. Third, bug 1090558 in mc146818rtc is a good example of a bug which was not due to insufficient testing, but to poorly structured code. There is no point worrying about unit testing if you accept code of such low quality. This goes for the tests too. For instance cmos_get_date_time() in tests/rtc-test.c doesn't work correctly in 12 hour mode. Fourth, I am not interested in the PC architecture, I only wrote a fix for bug 1090558 because Paolo asked me to. It is nice to see that fixing your crappy code makes me not a nice guy who is making things worse. But don't worry, I'll focus on ARM from now on.
[Qemu-devel] [PATCH] hw/mc146818rtc.c: Fix reading and writing of time registers
This patch consolidates the bit twidling involved in reading and
writing the time registers to four functions that are used consistently.
This also has the effect of fixing bug 1090558.
Signed-off-by: Antoine Mathys barsa...@gmail.com
---
hw/mc146818rtc.c | 163 +-
1 file changed, 99 insertions(+), 64 deletions(-)
diff --git a/hw/mc146818rtc.c b/hw/mc146818rtc.c
index 2fb11f6..646cbd0 100644
--- a/hw/mc146818rtc.c
+++ b/hw/mc146818rtc.c
@@ -86,8 +86,14 @@ typedef struct RTCState {
static void rtc_set_time(RTCState *s);
static void rtc_update_time(RTCState *s);
+
+/* data encoding / decoding */
+static inline uint8_t rtc_encode(RTCState *s, uint8_t a);
+static inline int rtc_decode(RTCState *s, uint8_t a);
+static inline uint8_t rtc_hour_encode(RTCState *s, uint8_t hour);
+static inline int rtc_hour_decode(RTCState *s, uint8_t a);
+
static void rtc_set_cmos(RTCState *s, const struct tm *tm);
-static inline int rtc_from_bcd(RTCState *s, int a);
static uint64_t get_next_alarm(RTCState *s);
static inline bool rtc_running(RTCState *s)
@@ -254,17 +260,84 @@ static void check_update_timer(RTCState *s)
}
}
-static inline uint8_t convert_hour(RTCState *s, uint8_t hour)
+/* data encoding / decoding */
+
+static inline uint8_t rtc_encode(RTCState *s, uint8_t a)
+{
+if (s-cmos_data[RTC_REG_B] REG_B_DM) {
+return a;
+} else {
+return to_bcd(a);
+}
+}
+
+static inline int rtc_decode(RTCState *s, uint8_t a)
+{
+if ((a 0xc0) == 0xc0) {
+return -1;
+}
+if (s-cmos_data[RTC_REG_B] REG_B_DM) {
+return a;
+} else {
+return from_bcd(a);
+}
+}
+
+/*
+ Note: The next two functions implement the following mapping between
+ the 12 hour and 24 hour formats:
+
+ 0-12 AM
+ 1-11 -1 - 11 AM
+ 12 -12 PM
+ 13-23-1 - 11 PM
+*/
+
+static inline uint8_t rtc_hour_encode(RTCState *s, uint8_t hour)
+{
+uint8_t tmp;
+
+if (s-cmos_data[RTC_REG_B] REG_B_24H) {
+/* 24 hour format */
+tmp = rtc_encode(s, hour);
+} else {
+/* 12 hour format */
+uint8_t h = (hour % 12) ? (hour % 12) : 12;
+tmp = rtc_encode(s, h);
+if (hour = 12) {
+tmp |= 0x80;
+}
+}
+return tmp;
+}
+
+static inline int rtc_hour_decode(RTCState *s, uint8_t a)
{
-if (!(s-cmos_data[RTC_REG_B] REG_B_24H)) {
+uint8_t hour;
+
+/* Note: in 12 hour mode we clear bit 7 before calling
+ rtc_decode(), hence we cannot rely on the later to check the
+ don't care condition. While we could skip this check in 24 hour
+ mode it is simpler to do it in any case. */
+if ((a 0xc0) == 0xc0) {
+return -1;
+}
+
+if (s-cmos_data[RTC_REG_B] REG_B_24H) {
+/* 24 hour format */
+hour = rtc_decode(s, a);
+} else {
+/* 12 hour format */
+hour = rtc_decode(s, a 0x7f);
hour %= 12;
-if (s-cmos_data[RTC_HOURS] 0x80) {
+if (a 0x80) {
hour += 12;
}
}
return hour;
}
+
static uint64_t get_next_alarm(RTCState *s)
{
int32_t alarm_sec, alarm_min, alarm_hour, cur_hour, cur_min, cur_sec;
@@ -272,15 +345,13 @@ static uint64_t get_next_alarm(RTCState *s)
rtc_update_time(s);
-alarm_sec = rtc_from_bcd(s, s-cmos_data[RTC_SECONDS_ALARM]);
-alarm_min = rtc_from_bcd(s, s-cmos_data[RTC_MINUTES_ALARM]);
-alarm_hour = rtc_from_bcd(s, s-cmos_data[RTC_HOURS_ALARM]);
-alarm_hour = alarm_hour == -1 ? -1 : convert_hour(s, alarm_hour);
+alarm_sec = rtc_decode(s, s-cmos_data[RTC_SECONDS_ALARM]);
+alarm_min = rtc_decode(s, s-cmos_data[RTC_MINUTES_ALARM]);
+alarm_hour = rtc_hour_decode(s, s-cmos_data[RTC_HOURS_ALARM]);
-cur_sec = rtc_from_bcd(s, s-cmos_data[RTC_SECONDS]);
-cur_min = rtc_from_bcd(s, s-cmos_data[RTC_MINUTES]);
-cur_hour = rtc_from_bcd(s, s-cmos_data[RTC_HOURS]);
-cur_hour = convert_hour(s, cur_hour);
+cur_sec = rtc_decode(s, s-cmos_data[RTC_SECONDS]);
+cur_min = rtc_decode(s, s-cmos_data[RTC_MINUTES]);
+cur_hour = rtc_hour_decode(s, s-cmos_data[RTC_HOURS]);
if (alarm_hour == -1) {
alarm_hour = cur_hour;
@@ -486,44 +557,17 @@ static void cmos_ioport_write(void *opaque, hwaddr addr,
}
}
-static inline int rtc_to_bcd(RTCState *s, int a)
-{
-if (s-cmos_data[RTC_REG_B] REG_B_DM) {
-return a;
-} else {
-return ((a / 10) 4) | (a % 10);
-}
-}
-
-static inline int rtc_from_bcd(RTCState *s, int a)
-{
-if ((a 0xc0) == 0xc0) {
-return -1;
-}
-if (s-cmos_data[RTC_REG_B] REG_B_DM) {
-return a;
-} else {
-return ((a 4) * 10) + (a 0x0f);
-}
-}
-
static void rtc_get_time(RTCState *s, struct tm *tm)
{
-tm-tm_sec = rtc_from_bcd(s, s-cmos_data[RTC_SECONDS]);
-tm-tm_min = rtc_from_bcd(s, s-cmos_data[RTC_MINUTES]);
-tm-tm_hour
Re: [Qemu-devel] [PATCH v4 0/6] hw/ds1338.c
On 12/13/2012 03:09 PM, Peter Maydell wrote: I certainly will send further patches. As you noticed I am new to git. Thanks for your patience and advice.
[Qemu-devel] [Bug 1090558] [NEW] hw/mc146818: error reading RTC_HOURS_ALARM
Public bug reported: get_next_alarm() doesn't read the RTC_HOURS_ALARM field correctly. - Bit 7 must be masked before conversion from BCD. - Care must be taken to check the don't care condition before masking. - The PM bit must be read from RTC_HOURS_ALARM, not from RTC_HOURS (as is done in convert_hour()). Seen in commit e376a788ae130454ad5e797f60cb70d0308babb6. ** Affects: qemu Importance: Undecided Status: New -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1090558 Title: hw/mc146818: error reading RTC_HOURS_ALARM Status in QEMU: New Bug description: get_next_alarm() doesn't read the RTC_HOURS_ALARM field correctly. - Bit 7 must be masked before conversion from BCD. - Care must be taken to check the don't care condition before masking. - The PM bit must be read from RTC_HOURS_ALARM, not from RTC_HOURS (as is done in convert_hour()). Seen in commit e376a788ae130454ad5e797f60cb70d0308babb6. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1090558/+subscriptions
[Qemu-devel] [PATCH] Fix conversion between 12 hours and 24 hours modes.
The proper mapping between 24 hours and 12 hours modes is:
0 12 AM
1-111-11 AM
12 12 PM
13-23 1-11 PM
Fix code accordingly.
Signed-off-by: Antoine Mathys barsa...@gmail.com
---
hw/ds1338.c |8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/hw/ds1338.c b/hw/ds1338.c
index 1aefa3b..9e6b490 100644
--- a/hw/ds1338.c
+++ b/hw/ds1338.c
@@ -59,8 +59,8 @@ static void capture_current_time(DS1338State *s)
s-nvram[1] = to_bcd(now.tm_min);
if (s-nvram[2] HOURS_12) {
int tmp = now.tm_hour;
-if (tmp == 0) {
-tmp = 24;
+if (tmp % 12 == 0) {
+tmp += 12;
}
if (tmp = 12) {
s-nvram[2] = HOURS_12 | to_bcd(tmp);
@@ -145,8 +145,8 @@ static int ds1338_send(I2CSlave *i2c, uint8_t data)
if (data HOURS_PM) {
tmp += 12;
}
-if (tmp == 24) {
-tmp = 0;
+if (tmp % 12 == 0) {
+tmp -= 12;
}
now.tm_hour = tmp;
} else {
--
1.7.10.4
Re: [Qemu-devel] [PATCH v2 4/6] hw/ds1338.c: Ensure state is properly initialized.
On 12/12/2012 01:05 PM, Peter Maydell wrote: I think we need to reset ptr and addr_byte too... The initial value of the register pointer is unspecified. addr_byte is only used when receiving user data. It is written in ds1338_event() then read in ds1338_send(). Setting it in ds1338_reset() is unnecessary and would break encapsulation.
Re: [Qemu-devel] [PATCH v2 4/6] hw/ds1338.c: Ensure state is properly initialized.
It's true that it probably doesn't make much difference, but both the addr_byte and ptr are part of the migration state listed in the vmstate struct. I think it is cleaner and more straightforward to reason about if resetting the device returns it to an exact known state. The state may be undefined according to the specification but that doesn't mean that our implementation has to leave it undefined. I don't understand your point about encapsulation -- these are both fields of the device state, and ds1338_reset() is a method of the device and has every right to access them. You are right. While it is not strictly necessary it is cleaner (and safer) to set every field to a reasonable value. In addition some guest code probably incorrectly assumes that the pointer is initially at zero so that wouldn't hurt. Would it be ok to reply with a new version of this patch only, instead of resending the whole series ?
[Qemu-devel] [PATCH v3 0/6] hw/ds1338.c
Re: [Qemu-devel] [PATCH v3 2/6] hw/ds1338.c: Add definitions for various flags in, the RTC registers.
Signed-off-by: Antoine Mathys barsa...@gmail.com
---
hw/ds1338.c |6 ++
1 file changed, 6 insertions(+)
diff --git a/hw/ds1338.c b/hw/ds1338.c
index faaa4a0..69018bc 100644
--- a/hw/ds1338.c
+++ b/hw/ds1338.c
@@ -17,6 +17,12 @@
*/
#define NVRAM_SIZE 64
+/* Flags definitions */
+#define SECONDS_CH 0x80
+#define HOURS_12 0x40
+#define HOURS_PM 0x20
+#define CTRL_OSF 0x20
+
typedef struct {
I2CSlave i2c;
int64_t offset;
--
1.7.10.4
Re: [Qemu-devel] [PATCH v3 1/6] hw/ds1338.c: Correct bug in conversion to BCD.
Signed-off-by: Antoine Mathys barsa...@gmail.com
---
hw/ds1338.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/ds1338.c b/hw/ds1338.c
index b576d56..faaa4a0 100644
--- a/hw/ds1338.c
+++ b/hw/ds1338.c
@@ -57,9 +57,9 @@ static void capture_current_time(DS1338State *s)
} else {
s-nvram[2] = to_bcd(now.tm_hour);
}
-s-nvram[3] = to_bcd(now.tm_wday) + 1;
+s-nvram[3] = to_bcd(now.tm_wday + 1);
s-nvram[4] = to_bcd(now.tm_mday);
-s-nvram[5] = to_bcd(now.tm_mon) + 1;
+s-nvram[5] = to_bcd(now.tm_mon + 1);
s-nvram[6] = to_bcd(now.tm_year - 100);
}
--
1.7.10.4
Re: [Qemu-devel] [PATCH v3 5/6] hw/ds1338.c: Implement support for the control register.
Signed-off-by: Antoine Mathys barsa...@gmail.com
---
hw/ds1338.c | 17 +
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/hw/ds1338.c b/hw/ds1338.c
index d2f52fc..94a2f54 100644
--- a/hw/ds1338.c
+++ b/hw/ds1338.c
@@ -125,7 +125,8 @@ static int ds1338_send(I2CSlave *i2c, uint8_t data)
s-addr_byte = false;
return 0;
}
-if (s-ptr 8) {
+if (s-ptr 7) {
+/* Time register. */
struct tm now;
qemu_get_timedate(now, s-offset);
switch(s-ptr) {
@@ -162,11 +163,19 @@ static int ds1338_send(I2CSlave *i2c, uint8_t data)
case 6:
now.tm_year = from_bcd(data) + 100;
break;
-case 7:
-/* Control register. Currently ignored. */
-break;
}
s-offset = qemu_timedate_diff(now);
+} else if (s-ptr == 7) {
+/* Control register. */
+
+/* Ensure bits 2, 3 and 6 will read back as zero. */
+data = 0xB3;
+
+/* Attempting to write the OSF flag to logic 1 leaves the
+ value unchanged. */
+data = (data ~CTRL_OSF) | (data s-nvram[s-ptr] CTRL_OSF);
+
+s-nvram[s-ptr] = data;
} else {
s-nvram[s-ptr] = data;
}
--
1.7.10.4
Re: [Qemu-devel] [PATCH v3 3/6] hw/ds1338.c: Fix handling of HOURS register.
Per the datasheet, the mapping between 12 and 24 hours modes is:
0 - 12 PM
1-12 - 1-12 AM
13-23 - 1-11 PM
Signed-off-by: Antoine Mathys barsa...@gmail.com
---
hw/ds1338.c | 29 ++---
1 file changed, 18 insertions(+), 11 deletions(-)
diff --git a/hw/ds1338.c b/hw/ds1338.c
index 69018bc..0f88720 100644
--- a/hw/ds1338.c
+++ b/hw/ds1338.c
@@ -55,10 +55,15 @@ static void capture_current_time(DS1338State *s)
qemu_get_timedate(now, s-offset);
s-nvram[0] = to_bcd(now.tm_sec);
s-nvram[1] = to_bcd(now.tm_min);
-if (s-nvram[2] 0x40) {
-s-nvram[2] = (to_bcd((now.tm_hour % 12)) + 1) | 0x40;
-if (now.tm_hour = 12) {
-s-nvram[2] |= 0x20;
+if (s-nvram[2] HOURS_12) {
+int tmp = now.tm_hour;
+if (tmp == 0) {
+tmp = 24;
+}
+if (tmp = 12) {
+s-nvram[2] = HOURS_12 | to_bcd(tmp);
+} else {
+s-nvram[2] = HOURS_12 | HOURS_PM | to_bcd(tmp - 12);
}
} else {
s-nvram[2] = to_bcd(now.tm_hour);
@@ -132,16 +137,18 @@ static int ds1338_send(I2CSlave *i2c, uint8_t data)
now.tm_min = from_bcd(data 0x7f);
break;
case 2:
-if (data 0x40) {
-if (data 0x20) {
-data = from_bcd(data 0x4f) + 11;
-} else {
-data = from_bcd(data 0x1f) - 1;
+if (data HOURS_12) {
+int tmp = from_bcd(data (HOURS_PM - 1));
+if (data HOURS_PM) {
+tmp += 12;
+}
+if (tmp == 24) {
+tmp = 0;
}
+now.tm_hour = tmp;
} else {
-data = from_bcd(data);
+now.tm_hour = from_bcd(data (HOURS_12 - 1));
}
-now.tm_hour = data;
break;
case 3:
now.tm_wday = from_bcd(data 7) - 1;
--
1.7.10.4
Re: [Qemu-devel] [PATCH v3 6/6] hw/ds1338.c: Fix handling of DAY (wday) register.
Per the datasheet, the DAY (wday) register is user defined. Implement this.
Signed-off-by: Antoine Mathys barsa...@gmail.com
---
hw/ds1338.c | 15 ---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/hw/ds1338.c b/hw/ds1338.c
index 94a2f54..1aefa3b 100644
--- a/hw/ds1338.c
+++ b/hw/ds1338.c
@@ -26,6 +26,7 @@
typedef struct {
I2CSlave i2c;
int64_t offset;
+uint8_t wday_offset;
uint8_t nvram[NVRAM_SIZE];
int32_t ptr;
bool addr_byte;
@@ -33,12 +34,13 @@ typedef struct {
static const VMStateDescription vmstate_ds1338 = {
.name = ds1338,
-.version_id = 1,
+.version_id = 2,
.minimum_version_id = 1,
.minimum_version_id_old = 1,
.fields = (VMStateField[]) {
VMSTATE_I2C_SLAVE(i2c, DS1338State),
VMSTATE_INT64(offset, DS1338State),
+VMSTATE_UINT8_V(wday_offset, DS1338State, 2),
VMSTATE_UINT8_ARRAY(nvram, DS1338State, NVRAM_SIZE),
VMSTATE_INT32(ptr, DS1338State),
VMSTATE_BOOL(addr_byte, DS1338State),
@@ -68,7 +70,7 @@ static void capture_current_time(DS1338State *s)
} else {
s-nvram[2] = to_bcd(now.tm_hour);
}
-s-nvram[3] = to_bcd(now.tm_wday + 1);
+s-nvram[3] = (now.tm_wday + s-wday_offset) % 7 + 1;
s-nvram[4] = to_bcd(now.tm_mday);
s-nvram[5] = to_bcd(now.tm_mon + 1);
s-nvram[6] = to_bcd(now.tm_year - 100);
@@ -152,7 +154,13 @@ static int ds1338_send(I2CSlave *i2c, uint8_t data)
}
break;
case 3:
-now.tm_wday = from_bcd(data 7) - 1;
+{
+/* The day field is supposed to contain a value in
+ the range 1-7. Otherwise behavior is undefined.
+ */
+int user_wday = (data 7) - 1;
+s-wday_offset = (user_wday - now.tm_wday + 7) % 7;
+}
break;
case 4:
now.tm_mday = from_bcd(data 0x3f);
@@ -194,6 +202,7 @@ static void ds1338_reset(DeviceState *dev)
/* The clock is running and synchronized with the host */
s-offset = 0;
+s-wday_offset = 0;
memset(s-nvram, 0, NVRAM_SIZE);
s-ptr = 0;
s-addr_byte = false;
--
1.7.10.4
Re: [Qemu-devel] [PATCH v3 4/6] hw/ds1338.c: Ensure state is properly initialized.
Signed-off-by: Antoine Mathys barsa...@gmail.com
---
hw/ds1338.c | 51 ++-
1 file changed, 38 insertions(+), 13 deletions(-)
diff --git a/hw/ds1338.c b/hw/ds1338.c
index b576d56..d2f52fc 100644
--- a/hw/ds1338.c
+++ b/hw/ds1338.c
@@ -17,6 +17,12 @@
*/
#define NVRAM_SIZE 64
+/* Flags definitions */
+#define SECONDS_CH 0x80
+#define HOURS_12 0x40
+#define HOURS_PM 0x20
+#define CTRL_OSF 0x20
+
typedef struct {
I2CSlave i2c;
int64_t offset;
@@ -49,17 +55,22 @@ static void capture_current_time(DS1338State *s)
qemu_get_timedate(now, s-offset);
s-nvram[0] = to_bcd(now.tm_sec);
s-nvram[1] = to_bcd(now.tm_min);
-if (s-nvram[2] 0x40) {
-s-nvram[2] = (to_bcd((now.tm_hour % 12)) + 1) | 0x40;
-if (now.tm_hour = 12) {
-s-nvram[2] |= 0x20;
+if (s-nvram[2] HOURS_12) {
+int tmp = now.tm_hour;
+if (tmp == 0) {
+tmp = 24;
+}
+if (tmp = 12) {
+s-nvram[2] = HOURS_12 | to_bcd(tmp);
+} else {
+s-nvram[2] = HOURS_12 | HOURS_PM | to_bcd(tmp - 12);
}
} else {
s-nvram[2] = to_bcd(now.tm_hour);
}
-s-nvram[3] = to_bcd(now.tm_wday) + 1;
+s-nvram[3] = to_bcd(now.tm_wday + 1);
s-nvram[4] = to_bcd(now.tm_mday);
-s-nvram[5] = to_bcd(now.tm_mon) + 1;
+s-nvram[5] = to_bcd(now.tm_mon + 1);
s-nvram[6] = to_bcd(now.tm_year - 100);
}
@@ -126,16 +137,18 @@ static int ds1338_send(I2CSlave *i2c, uint8_t data)
now.tm_min = from_bcd(data 0x7f);
break;
case 2:
-if (data 0x40) {
-if (data 0x20) {
-data = from_bcd(data 0x4f) + 11;
-} else {
-data = from_bcd(data 0x1f) - 1;
+if (data HOURS_12) {
+int tmp = from_bcd(data (HOURS_PM - 1));
+if (data HOURS_PM) {
+tmp += 12;
}
+if (tmp == 24) {
+tmp = 0;
+}
+now.tm_hour = tmp;
} else {
-data = from_bcd(data);
+now.tm_hour = from_bcd(data (HOURS_12 - 1));
}
-now.tm_hour = data;
break;
case 3:
now.tm_wday = from_bcd(data 7) - 1;
@@ -166,6 +179,17 @@ static int ds1338_init(I2CSlave *i2c)
return 0;
}
+static void ds1338_reset(DeviceState *dev)
+{
+DS1338State *s = FROM_I2C_SLAVE(DS1338State, I2C_SLAVE_FROM_QDEV(dev));
+
+/* The clock is running and synchronized with the host */
+s-offset = 0;
+memset(s-nvram, 0, NVRAM_SIZE);
+s-ptr = 0;
+s-addr_byte = false;
+}
+
static void ds1338_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
@@ -175,6 +199,7 @@ static void ds1338_class_init(ObjectClass *klass,
void *data)
k-event = ds1338_event;
k-recv = ds1338_recv;
k-send = ds1338_send;
+dc-reset = ds1338_reset;
dc-vmsd = vmstate_ds1338;
}
--
1.7.10.4
Re: [Qemu-devel] [PATCH v3 4/6] hw/ds1338.c: Ensure state is properly initialized.
Oops. There was a problem in the patch. Resending the series.
[Qemu-devel] [PATCH v4 0/6] hw/ds1338.c
[Qemu-devel] [PATCH v4 1/6] hw/ds1338.c: Correct bug in conversion to BCD.
Signed-off-by: Antoine Mathys barsa...@gmail.com
---
hw/ds1338.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/ds1338.c b/hw/ds1338.c
index b576d56..faaa4a0 100644
--- a/hw/ds1338.c
+++ b/hw/ds1338.c
@@ -57,9 +57,9 @@ static void capture_current_time(DS1338State *s)
} else {
s-nvram[2] = to_bcd(now.tm_hour);
}
-s-nvram[3] = to_bcd(now.tm_wday) + 1;
+s-nvram[3] = to_bcd(now.tm_wday + 1);
s-nvram[4] = to_bcd(now.tm_mday);
-s-nvram[5] = to_bcd(now.tm_mon) + 1;
+s-nvram[5] = to_bcd(now.tm_mon + 1);
s-nvram[6] = to_bcd(now.tm_year - 100);
}
--
1.7.10.4
[Qemu-devel] [PATCH v4 2/6] hw/ds1338.c: Add definitions for various flags in the RTC registers.
Signed-off-by: Antoine Mathys barsa...@gmail.com
---
hw/ds1338.c |6 ++
1 file changed, 6 insertions(+)
diff --git a/hw/ds1338.c b/hw/ds1338.c
index faaa4a0..69018bc 100644
--- a/hw/ds1338.c
+++ b/hw/ds1338.c
@@ -17,6 +17,12 @@
*/
#define NVRAM_SIZE 64
+/* Flags definitions */
+#define SECONDS_CH 0x80
+#define HOURS_12 0x40
+#define HOURS_PM 0x20
+#define CTRL_OSF 0x20
+
typedef struct {
I2CSlave i2c;
int64_t offset;
--
1.7.10.4
[Qemu-devel] [PATCH v4 3/6] hw/ds1338.c: Fix handling of HOURS register.
Per the datasheet, the mapping between 12 and 24 hours modes is:
0 - 12 PM
1-12 - 1-12 AM
13-23 - 1-11 PM
Signed-off-by: Antoine Mathys barsa...@gmail.com
---
hw/ds1338.c | 29 ++---
1 file changed, 18 insertions(+), 11 deletions(-)
diff --git a/hw/ds1338.c b/hw/ds1338.c
index 69018bc..0f88720 100644
--- a/hw/ds1338.c
+++ b/hw/ds1338.c
@@ -55,10 +55,15 @@ static void capture_current_time(DS1338State *s)
qemu_get_timedate(now, s-offset);
s-nvram[0] = to_bcd(now.tm_sec);
s-nvram[1] = to_bcd(now.tm_min);
-if (s-nvram[2] 0x40) {
-s-nvram[2] = (to_bcd((now.tm_hour % 12)) + 1) | 0x40;
-if (now.tm_hour = 12) {
-s-nvram[2] |= 0x20;
+if (s-nvram[2] HOURS_12) {
+int tmp = now.tm_hour;
+if (tmp == 0) {
+tmp = 24;
+}
+if (tmp = 12) {
+s-nvram[2] = HOURS_12 | to_bcd(tmp);
+} else {
+s-nvram[2] = HOURS_12 | HOURS_PM | to_bcd(tmp - 12);
}
} else {
s-nvram[2] = to_bcd(now.tm_hour);
@@ -132,16 +137,18 @@ static int ds1338_send(I2CSlave *i2c, uint8_t data)
now.tm_min = from_bcd(data 0x7f);
break;
case 2:
-if (data 0x40) {
-if (data 0x20) {
-data = from_bcd(data 0x4f) + 11;
-} else {
-data = from_bcd(data 0x1f) - 1;
+if (data HOURS_12) {
+int tmp = from_bcd(data (HOURS_PM - 1));
+if (data HOURS_PM) {
+tmp += 12;
+}
+if (tmp == 24) {
+tmp = 0;
}
+now.tm_hour = tmp;
} else {
-data = from_bcd(data);
+now.tm_hour = from_bcd(data (HOURS_12 - 1));
}
-now.tm_hour = data;
break;
case 3:
now.tm_wday = from_bcd(data 7) - 1;
--
1.7.10.4
[Qemu-devel] [PATCH v4 4/6] hw/ds1338.c: Ensure state is properly initialized.
Signed-off-by: Antoine Mathys barsa...@gmail.com
---
hw/ds1338.c | 12
1 file changed, 12 insertions(+)
diff --git a/hw/ds1338.c b/hw/ds1338.c
index 0f88720..d2f52fc 100644
--- a/hw/ds1338.c
+++ b/hw/ds1338.c
@@ -179,6 +179,17 @@ static int ds1338_init(I2CSlave *i2c)
return 0;
}
+static void ds1338_reset(DeviceState *dev)
+{
+DS1338State *s = FROM_I2C_SLAVE(DS1338State, I2C_SLAVE_FROM_QDEV(dev));
+
+/* The clock is running and synchronized with the host */
+s-offset = 0;
+memset(s-nvram, 0, NVRAM_SIZE);
+s-ptr = 0;
+s-addr_byte = false;
+}
+
static void ds1338_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
@@ -188,6 +199,7 @@ static void ds1338_class_init(ObjectClass *klass,
void *data)
k-event = ds1338_event;
k-recv = ds1338_recv;
k-send = ds1338_send;
+dc-reset = ds1338_reset;
dc-vmsd = vmstate_ds1338;
}
--
1.7.10.4
[Qemu-devel] [PATCH v4 6/6] hw/ds1338.c: Fix handling of DAY (wday) register.
Per the datasheet, the DAY (wday) register is user defined. Implement this.
Signed-off-by: Antoine Mathys barsa...@gmail.com
---
hw/ds1338.c | 15 ---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/hw/ds1338.c b/hw/ds1338.c
index 94a2f54..1aefa3b 100644
--- a/hw/ds1338.c
+++ b/hw/ds1338.c
@@ -26,6 +26,7 @@
typedef struct {
I2CSlave i2c;
int64_t offset;
+uint8_t wday_offset;
uint8_t nvram[NVRAM_SIZE];
int32_t ptr;
bool addr_byte;
@@ -33,12 +34,13 @@ typedef struct {
static const VMStateDescription vmstate_ds1338 = {
.name = ds1338,
-.version_id = 1,
+.version_id = 2,
.minimum_version_id = 1,
.minimum_version_id_old = 1,
.fields = (VMStateField[]) {
VMSTATE_I2C_SLAVE(i2c, DS1338State),
VMSTATE_INT64(offset, DS1338State),
+VMSTATE_UINT8_V(wday_offset, DS1338State, 2),
VMSTATE_UINT8_ARRAY(nvram, DS1338State, NVRAM_SIZE),
VMSTATE_INT32(ptr, DS1338State),
VMSTATE_BOOL(addr_byte, DS1338State),
@@ -68,7 +70,7 @@ static void capture_current_time(DS1338State *s)
} else {
s-nvram[2] = to_bcd(now.tm_hour);
}
-s-nvram[3] = to_bcd(now.tm_wday + 1);
+s-nvram[3] = (now.tm_wday + s-wday_offset) % 7 + 1;
s-nvram[4] = to_bcd(now.tm_mday);
s-nvram[5] = to_bcd(now.tm_mon + 1);
s-nvram[6] = to_bcd(now.tm_year - 100);
@@ -152,7 +154,13 @@ static int ds1338_send(I2CSlave *i2c, uint8_t data)
}
break;
case 3:
-now.tm_wday = from_bcd(data 7) - 1;
+{
+/* The day field is supposed to contain a value in
+ the range 1-7. Otherwise behavior is undefined.
+ */
+int user_wday = (data 7) - 1;
+s-wday_offset = (user_wday - now.tm_wday + 7) % 7;
+}
break;
case 4:
now.tm_mday = from_bcd(data 0x3f);
@@ -194,6 +202,7 @@ static void ds1338_reset(DeviceState *dev)
/* The clock is running and synchronized with the host */
s-offset = 0;
+s-wday_offset = 0;
memset(s-nvram, 0, NVRAM_SIZE);
s-ptr = 0;
s-addr_byte = false;
--
1.7.10.4
[Qemu-devel] [PATCH v4 5/6] hw/ds1338.c: Implement support for the control register.
Signed-off-by: Antoine Mathys barsa...@gmail.com
---
hw/ds1338.c | 17 +
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/hw/ds1338.c b/hw/ds1338.c
index d2f52fc..94a2f54 100644
--- a/hw/ds1338.c
+++ b/hw/ds1338.c
@@ -125,7 +125,8 @@ static int ds1338_send(I2CSlave *i2c, uint8_t data)
s-addr_byte = false;
return 0;
}
-if (s-ptr 8) {
+if (s-ptr 7) {
+/* Time register. */
struct tm now;
qemu_get_timedate(now, s-offset);
switch(s-ptr) {
@@ -162,11 +163,19 @@ static int ds1338_send(I2CSlave *i2c, uint8_t data)
case 6:
now.tm_year = from_bcd(data) + 100;
break;
-case 7:
-/* Control register. Currently ignored. */
-break;
}
s-offset = qemu_timedate_diff(now);
+} else if (s-ptr == 7) {
+/* Control register. */
+
+/* Ensure bits 2, 3 and 6 will read back as zero. */
+data = 0xB3;
+
+/* Attempting to write the OSF flag to logic 1 leaves the
+ value unchanged. */
+data = (data ~CTRL_OSF) | (data s-nvram[s-ptr] CTRL_OSF);
+
+s-nvram[s-ptr] = data;
} else {
s-nvram[s-ptr] = data;
}
--
1.7.10.4
[Qemu-devel] [PATCH v2 0/6] hw/ds1338.c
This is the second version of my series of patch to hw/ds1338.c It should address the various points that were made. The main difference with the previous version is that I dropped some stuff that will only be useful once we implement clock enable/disable.
[Qemu-devel] [PATCH v2 1/6] hw/ds1338.c: Correct bug in conversion to BCD.
Signed-off-by: Antoine Mathys barsa...@gmail.com
---
hw/ds1338.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/ds1338.c b/hw/ds1338.c
index b576d56..faaa4a0 100644
--- a/hw/ds1338.c
+++ b/hw/ds1338.c
@@ -57,9 +57,9 @@ static void capture_current_time(DS1338State *s)
} else {
s-nvram[2] = to_bcd(now.tm_hour);
}
-s-nvram[3] = to_bcd(now.tm_wday) + 1;
+s-nvram[3] = to_bcd(now.tm_wday + 1);
s-nvram[4] = to_bcd(now.tm_mday);
-s-nvram[5] = to_bcd(now.tm_mon) + 1;
+s-nvram[5] = to_bcd(now.tm_mon + 1);
s-nvram[6] = to_bcd(now.tm_year - 100);
}
--
1.7.10.4
[Qemu-devel] [PATCH v2 2/6] hw/ds1338.c: Add definitions for various flags in the RTC registers.
Signed-off-by: Antoine Mathys barsa...@gmail.com
---
hw/ds1338.c |6 ++
1 file changed, 6 insertions(+)
diff --git a/hw/ds1338.c b/hw/ds1338.c
index faaa4a0..69018bc 100644
--- a/hw/ds1338.c
+++ b/hw/ds1338.c
@@ -17,6 +17,12 @@
*/
#define NVRAM_SIZE 64
+/* Flags definitions */
+#define SECONDS_CH 0x80
+#define HOURS_12 0x40
+#define HOURS_PM 0x20
+#define CTRL_OSF 0x20
+
typedef struct {
I2CSlave i2c;
int64_t offset;
--
1.7.10.4
[Qemu-devel] [PATCH v2 3/6] hw/ds1338.c: Fix handling of HOURS register.
Per the datasheet, the mapping between 12 and 24 hours modes is:
0 - 12 PM
1-12 - 1-12 AM
13-23 - 1-11 PM
Signed-off-by: Antoine Mathys barsa...@gmail.com
---
hw/ds1338.c | 29 ++---
1 file changed, 18 insertions(+), 11 deletions(-)
diff --git a/hw/ds1338.c b/hw/ds1338.c
index 69018bc..0f88720 100644
--- a/hw/ds1338.c
+++ b/hw/ds1338.c
@@ -55,10 +55,15 @@ static void capture_current_time(DS1338State *s)
qemu_get_timedate(now, s-offset);
s-nvram[0] = to_bcd(now.tm_sec);
s-nvram[1] = to_bcd(now.tm_min);
-if (s-nvram[2] 0x40) {
-s-nvram[2] = (to_bcd((now.tm_hour % 12)) + 1) | 0x40;
-if (now.tm_hour = 12) {
-s-nvram[2] |= 0x20;
+if (s-nvram[2] HOURS_12) {
+int tmp = now.tm_hour;
+if (tmp == 0) {
+tmp = 24;
+}
+if (tmp = 12) {
+s-nvram[2] = HOURS_12 | to_bcd(tmp);
+} else {
+s-nvram[2] = HOURS_12 | HOURS_PM | to_bcd(tmp - 12);
}
} else {
s-nvram[2] = to_bcd(now.tm_hour);
@@ -132,16 +137,18 @@ static int ds1338_send(I2CSlave *i2c, uint8_t data)
now.tm_min = from_bcd(data 0x7f);
break;
case 2:
-if (data 0x40) {
-if (data 0x20) {
-data = from_bcd(data 0x4f) + 11;
-} else {
-data = from_bcd(data 0x1f) - 1;
+if (data HOURS_12) {
+int tmp = from_bcd(data (HOURS_PM - 1));
+if (data HOURS_PM) {
+tmp += 12;
+}
+if (tmp == 24) {
+tmp = 0;
}
+now.tm_hour = tmp;
} else {
-data = from_bcd(data);
+now.tm_hour = from_bcd(data (HOURS_12 - 1));
}
-now.tm_hour = data;
break;
case 3:
now.tm_wday = from_bcd(data 7) - 1;
--
1.7.10.4
[Qemu-devel] [PATCH v2 4/6] hw/ds1338.c: Ensure state is properly initialized.
Signed-off-by: Antoine Mathys barsa...@gmail.com
---
hw/ds1338.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/hw/ds1338.c b/hw/ds1338.c
index 0f88720..b4d5b74 100644
--- a/hw/ds1338.c
+++ b/hw/ds1338.c
@@ -179,6 +179,15 @@ static int ds1338_init(I2CSlave *i2c)
return 0;
}
+static void ds1338_reset(DeviceState *dev)
+{
+DS1338State *s = FROM_I2C_SLAVE(DS1338State, I2C_SLAVE_FROM_QDEV(dev));
+
+/* The clock is running and synchronized with the host */
+s-offset = 0;
+memset(s-nvram, 0, NVRAM_SIZE);
+}
+
static void ds1338_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
@@ -188,6 +197,7 @@ static void ds1338_class_init(ObjectClass *klass,
void *data)
k-event = ds1338_event;
k-recv = ds1338_recv;
k-send = ds1338_send;
+dc-reset = ds1338_reset;
dc-vmsd = vmstate_ds1338;
}
--
1.7.10.4
[Qemu-devel] [PATCH v2 5/6] hw/ds1338.c: Implement support for the control register.
The previous patch has the side effect of clearing the control register.
That's already its proper power-on-reset value.
Signed-off-by: Antoine Mathys barsa...@gmail.com
---
hw/ds1338.c | 17 +
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/hw/ds1338.c b/hw/ds1338.c
index 5a6234b..319c341 100644
--- a/hw/ds1338.c
+++ b/hw/ds1338.c
@@ -125,7 +125,8 @@ static int ds1338_send(I2CSlave *i2c, uint8_t data)
s-addr_byte = false;
return 0;
}
-if (s-ptr 8) {
+if (s-ptr 7) {
+/* Time register. */
struct tm now;
qemu_get_timedate(now, s-offset);
switch(s-ptr) {
@@ -162,11 +163,19 @@ static int ds1338_send(I2CSlave *i2c, uint8_t data)
case 6:
now.tm_year = from_bcd(data) + 100;
break;
-case 7:
-/* Control register. Currently ignored. */
-break;
}
s-offset = qemu_timedate_diff(now);
+} else if (s-ptr == 7) {
+/* Control register. */
+
+/* Ensure bits 2, 3 and 6 will read back as zero. */
+data = 0xB3;
+
+/* Attempting to write the OSF flag to logic 1 leaves the
+ value unchanged. */
+data = (data ~CTRL_OSF) | (data s-nvram[s-ptr] CTRL_OSF);
+
+s-nvram[s-ptr] = data;
} else {
s-nvram[s-ptr] = data;
}
--
1.7.10.4
[Qemu-devel] [PATCH v2 6/6] hw/ds1338.c: Fix handling of DAY (wday) register.
Per the datasheet, the DAY (wday) register is user defined. Implement this.
Signed-off-by: Antoine Mathys barsa...@gmail.com
---
hw/ds1338.c | 15 ---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/hw/ds1338.c b/hw/ds1338.c
index 319c341..c5cee99 100644
--- a/hw/ds1338.c
+++ b/hw/ds1338.c
@@ -26,6 +26,7 @@
typedef struct {
I2CSlave i2c;
int64_t offset;
+uint8_t wday_offset;
uint8_t nvram[NVRAM_SIZE];
int32_t ptr;
bool addr_byte;
@@ -33,12 +34,13 @@ typedef struct {
static const VMStateDescription vmstate_ds1338 = {
.name = ds1338,
-.version_id = 1,
+.version_id = 2,
.minimum_version_id = 1,
.minimum_version_id_old = 1,
.fields = (VMStateField[]) {
VMSTATE_I2C_SLAVE(i2c, DS1338State),
VMSTATE_INT64(offset, DS1338State),
+VMSTATE_UINT8_V(wday_offset, DS1338State, 2),
VMSTATE_UINT8_ARRAY(nvram, DS1338State, NVRAM_SIZE),
VMSTATE_INT32(ptr, DS1338State),
VMSTATE_BOOL(addr_byte, DS1338State),
@@ -68,7 +70,7 @@ static void capture_current_time(DS1338State *s)
} else {
s-nvram[2] = to_bcd(now.tm_hour);
}
-s-nvram[3] = to_bcd(now.tm_wday + 1);
+s-nvram[3] = (now.tm_wday + s-wday_offset) % 7 + 1;
s-nvram[4] = to_bcd(now.tm_mday);
s-nvram[5] = to_bcd(now.tm_mon + 1);
s-nvram[6] = to_bcd(now.tm_year - 100);
@@ -152,7 +154,13 @@ static int ds1338_send(I2CSlave *i2c, uint8_t data)
}
break;
case 3:
-now.tm_wday = from_bcd(data 7) - 1;
+{
+/* The day field is supposed to contain a value in
+ the range 1-7. Otherwise behavior is undefined.
+ */
+int user_wday = (data 7) - 1;
+s-wday_offset = (user_wday - now.tm_wday + 7) % 7;
+}
break;
case 4:
now.tm_mday = from_bcd(data 0x3f);
@@ -194,6 +202,7 @@ static void ds1338_reset(DeviceState *dev)
/* The clock is running and synchronized with the host */
s-offset = 0;
+s-wday_offset = 0;
memset(s-nvram, 0, NVRAM_SIZE);
}
--
1.7.10.4
Re: [Qemu-devel] [PATCH 1/4] hw/ds1338.c: Fix handling of HOURS register.
On 12/04/2012 06:42 PM, Peter Maydell wrote: This looks good as far as the logic goes, but I think we could use some symbolic constants for the 12-hour and PM bits rather than all the literal 0x20 0x40 0x60. thanks -- PMM I refrained from using symbolic constants for three reasons: 1. You need to refer to the datasheet anyway to understand what is going on. 2. The code is short and it is easy to keep track of a few constants. 3. Symbolic names, which can be quite long, tend to make the code harder rather than easier to read It is largely a matter of preference though and I'm not opposed to the idea. I would appreciate naming suggestions though. I propose not to have symbolic constants for the registers and to use CH, OSF, MODE12, PM for the various bits, provided such short names are acceptable. I'd really hate something like REG_HOURS_MODE12.
[Qemu-devel] [PATCH 5/?] hw/ds1338.c: Fix handling of DATE (wday) register
Per the datasheet, the DATE (wday) register is user defined. Implement this.
Signed-off-by: Antoine Mathys barsa...@gmail.com
---
hw/ds1338.c | 14 --
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/hw/ds1338.c b/hw/ds1338.c
index 8f85635..c502934 100644
--- a/hw/ds1338.c
+++ b/hw/ds1338.c
@@ -20,6 +20,7 @@
typedef struct {
I2CSlave i2c;
int64_t offset;
+uint8_t wday_offset;
uint8_t nvram[NVRAM_SIZE];
int32_t ptr;
bool addr_byte;
@@ -33,6 +34,7 @@ static const VMStateDescription vmstate_ds1338 = {
.fields = (VMStateField[]) {
VMSTATE_I2C_SLAVE(i2c, DS1338State),
VMSTATE_INT64(offset, DS1338State),
+VMSTATE_UINT8(wday_offset, DS1338State),
VMSTATE_UINT8_ARRAY(nvram, DS1338State, NVRAM_SIZE),
VMSTATE_INT32(ptr, DS1338State),
VMSTATE_BOOL(addr_byte, DS1338State),
@@ -62,7 +64,7 @@ static void write_time(DS1338State *s, const struct tm
*tm)
} else {
s-nvram[2] = to_bcd(tm-tm_hour);
}
-s-nvram[3] = to_bcd(tm-tm_wday + 1);
+s-nvram[3] = to_bcd((tm-tm_wday + s-wday_offset) % 7 + 1);
s-nvram[4] = to_bcd(tm-tm_mday);
s-nvram[5] = to_bcd(tm-tm_mon + 1);
s-nvram[6] = to_bcd(tm-tm_year - 100);
@@ -164,7 +166,12 @@ static int ds1338_send(I2CSlave *i2c, uint8_t data)
}
break;
case 3:
-now.tm_wday = from_bcd(data 0x07) - 1;
+{
+int user_wday = from_bcd(data 0x07) - 1;
+if ((user_wday = 0) (user_wday = 6)) {
+s-wday_offset = (user_wday - now.tm_wday + 7) % 7;
+}
+}
break;
case 4:
now.tm_mday = from_bcd(data 0x3f);
@@ -194,6 +201,9 @@ static int ds1338_send(I2CSlave *i2c, uint8_t data)
static int ds1338_init(I2CSlave *i2c)
{
+DS1338State *s = FROM_I2C_SLAVE(DS1338State, i2c);
+s-wday_offset = 0;
+
return 0;
}
--
1.7.10.4
[Qemu-devel] [PATCH 1/4] hw/ds1338.c: Fix handling of HOURS register.
Per the datasheet, the mapping between 12 and 24 hours modes is:
0 - 12 PM
1-12 - 1-12 AM
13-23 - 1-11 PM
Signed-off-by: Antoine Mathys barsa...@gmail.com
---
hw/ds1338.c | 23 +++
1 file changed, 15 insertions(+), 8 deletions(-)
diff --git a/hw/ds1338.c b/hw/ds1338.c
index b576d56..1274b22 100644
--- a/hw/ds1338.c
+++ b/hw/ds1338.c
@@ -50,9 +50,14 @@ static void capture_current_time(DS1338State *s)
s-nvram[0] = to_bcd(now.tm_sec);
s-nvram[1] = to_bcd(now.tm_min);
if (s-nvram[2] 0x40) {
-s-nvram[2] = (to_bcd((now.tm_hour % 12)) + 1) | 0x40;
-if (now.tm_hour = 12) {
-s-nvram[2] |= 0x20;
+int tmp = now.tm_hour;
+if (tmp == 0) {
+tmp = 24;
+}
+if (tmp = 12) {
+s-nvram[2] = 0x40 | to_bcd(tmp);
+} else {
+s-nvram[2] = 0x60 | to_bcd(tmp - 12);
}
} else {
s-nvram[2] = to_bcd(now.tm_hour);
@@ -127,15 +132,17 @@ static int ds1338_send(I2CSlave *i2c, uint8_t data)
break;
case 2:
if (data 0x40) {
+int tmp = from_bcd(data 0x1f);
if (data 0x20) {
-data = from_bcd(data 0x4f) + 11;
-} else {
-data = from_bcd(data 0x1f) - 1;
+tmp += 12;
+}
+if (tmp == 24) {
+tmp = 0;
}
+now.tm_hour = tmp;
} else {
-data = from_bcd(data);
+now.tm_hour = from_bcd(data 0x3f);
}
-now.tm_hour = data;
break;
case 3:
now.tm_wday = from_bcd(data 7) - 1;
--
1.7.10.4
[Qemu-devel] [PATCH 2/4] hw/ds1338.c: Minor fixes
Minor fixes in the handling of the RTC registers.
Signed-off-by: Antoine Mathys barsa...@gmail.com
---
hw/ds1338.c | 15 ---
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/hw/ds1338.c b/hw/ds1338.c
index 1274b22..1fb152e 100644
--- a/hw/ds1338.c
+++ b/hw/ds1338.c
@@ -62,9 +62,9 @@ static void capture_current_time(DS1338State *s)
} else {
s-nvram[2] = to_bcd(now.tm_hour);
}
-s-nvram[3] = to_bcd(now.tm_wday) + 1;
+s-nvram[3] = to_bcd(now.tm_wday + 1);
s-nvram[4] = to_bcd(now.tm_mday);
-s-nvram[5] = to_bcd(now.tm_mon) + 1;
+s-nvram[5] = to_bcd(now.tm_mon + 1);
s-nvram[6] = to_bcd(now.tm_year - 100);
}
@@ -119,7 +119,8 @@ static int ds1338_send(I2CSlave *i2c, uint8_t data)
s-addr_byte = false;
return 0;
}
-if (s-ptr 8) {
+if (s-ptr 7) {
+/* Time register. */
struct tm now;
qemu_get_timedate(now, s-offset);
switch(s-ptr) {
@@ -145,7 +146,7 @@ static int ds1338_send(I2CSlave *i2c, uint8_t data)
}
break;
case 3:
-now.tm_wday = from_bcd(data 7) - 1;
+now.tm_wday = from_bcd(data 0x07) - 1;
break;
case 4:
now.tm_mday = from_bcd(data 0x3f);
@@ -156,11 +157,11 @@ static int ds1338_send(I2CSlave *i2c, uint8_t data)
case 6:
now.tm_year = from_bcd(data) + 100;
break;
-case 7:
-/* Control register. Currently ignored. */
-break;
}
s-offset = qemu_timedate_diff(now);
+} else if (s-ptr == 7) {
+/* Control register. Currently ignored. */
+s-nvram[s-ptr] = data;
} else {
s-nvram[s-ptr] = data;
}
--
1.7.10.4
[Qemu-devel] [PATCH 3/4] hw/ds1338.c: ensure OSF can only be cleared
Per the datasheet, the OSF bit in the control register can only be
cleared. Attempts to set it have no effect. Implement this.
Signed-off-by: Antoine Mathys barsa...@gmail.com
---
hw/ds1338.c |7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/hw/ds1338.c b/hw/ds1338.c
index 1fb152e..f3c6bc5 100644
--- a/hw/ds1338.c
+++ b/hw/ds1338.c
@@ -160,7 +160,12 @@ static int ds1338_send(I2CSlave *i2c, uint8_t data)
}
s-offset = qemu_timedate_diff(now);
} else if (s-ptr == 7) {
-/* Control register. Currently ignored. */
+/* Control register. */
+
+/* Attempting to write the OSF flag to logic 1 leaves the
+ value unchanged. */
+data = (data 0xDF) | (data s-nvram[s-ptr] 0x20);
+
s-nvram[s-ptr] = data;
} else {
s-nvram[s-ptr] = data;
--
1.7.10.4
[Qemu-devel] [PATCH 4/4] hw/ds1338.c: Handle stuck bits and preserve CH
Preserve the CH bit when updating the time.
Per the datasheet, some bits in the first eight registers must always
read back as zero. These registers can be written in two ways:
1. By ourselves when we update the registers with the current time.
2. By user request
Even though (1) is not supposed to set these bits it is safer to check
anyway. In order not to duplicate this logic I thus chose to enforce it
at read time.
Note that currently we don't preserve what the user sends in (2). We
will have to once we support stopping the clock (CH bit).
Signed-off-by: Antoine Mathys barsa...@gmail.com
---
hw/ds1338.c | 50 ++
1 file changed, 34 insertions(+), 16 deletions(-)
diff --git a/hw/ds1338.c b/hw/ds1338.c
index f3c6bc5..8f85635 100644
--- a/hw/ds1338.c
+++ b/hw/ds1338.c
@@ -40,17 +40,17 @@ static const VMStateDescription vmstate_ds1338 = {
}
};
-static void capture_current_time(DS1338State *s)
+/* Write TM to the RTC registers. */
+static void write_time(DS1338State *s, const struct tm *tm)
{
-/* Capture the current time into the secondary registers
- * which will be actually read by the data transfer operation.
- */
-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);
+/* Preserve the CH flag. */
+s-nvram[0] = 0x80;
+s-nvram[0] |= to_bcd(tm-tm_sec);
+
+s-nvram[1] = to_bcd(tm-tm_min);
+
if (s-nvram[2] 0x40) {
-int tmp = now.tm_hour;
+int tmp = tm-tm_hour;
if (tmp == 0) {
tmp = 24;
}
@@ -60,12 +60,30 @@ static void capture_current_time(DS1338State *s)
s-nvram[2] = 0x60 | to_bcd(tmp - 12);
}
} else {
-s-nvram[2] = to_bcd(now.tm_hour);
+s-nvram[2] = to_bcd(tm-tm_hour);
}
-s-nvram[3] = to_bcd(now.tm_wday + 1);
-s-nvram[4] = to_bcd(now.tm_mday);
-s-nvram[5] = to_bcd(now.tm_mon + 1);
-s-nvram[6] = to_bcd(now.tm_year - 100);
+s-nvram[3] = to_bcd(tm-tm_wday + 1);
+s-nvram[4] = to_bcd(tm-tm_mday);
+s-nvram[5] = to_bcd(tm-tm_mon + 1);
+s-nvram[6] = to_bcd(tm-tm_year - 100);
+}
+
+static void update_read_buffer(DS1338State *s)
+{
+/* Capture the current time into the secondary registers
+ * which will be actually read by the data transfer operation.
+ */
+struct tm now;
+qemu_get_timedate(now, s-offset);
+write_time(s, now);
+
+/* Ensure specified bits read as zero. */
+s-nvram[1] = 0x7F;
+s-nvram[2] = 0x7F;
+s-nvram[3] = 0x07;
+s-nvram[4] = 0x3F;
+s-nvram[5] = 0x1F;
+s-nvram[7] = 0xB3;
}
static void inc_regptr(DS1338State *s)
@@ -76,7 +94,7 @@ static void inc_regptr(DS1338State *s)
*/
s-ptr = (s-ptr + 1) (NVRAM_SIZE - 1);
if (!s-ptr) {
-capture_current_time(s);
+update_read_buffer(s);
}
}
@@ -91,7 +109,7 @@ static void ds1338_event(I2CSlave *i2c, enum
i2c_event event)
* START_SEND, because the guest can't get at that data
* without going through a START_RECV which would overwrite it.
*/
-capture_current_time(s);
+update_read_buffer(s);
break;
case I2C_START_SEND:
s-addr_byte = true;
--
1.7.10.4
