On Wed, Jun 06, 2018 at 07:35:28PM +0200, BALATON Zoltan wrote:
On Wed, 6 Jun 2018, Philippe Mathieu-Daudé wrote:
On 06/06/2018 10:31 AM, BALATON Zoltan wrote:
Basic emulation of the M41T80 serial (I2C) RTC chip. Only getting time
of day is implemented. Setting time and RTC alarm are not supported.
[...]
diff --git a/hw/timer/m41t80.c b/hw/timer/m41t80.c
new file mode 100644
index 0000000..9dbdb1b
--- /dev/null
+++ b/hw/timer/m41t80.c
@@ -0,0 +1,117 @@
+/*
+ * M41T80 serial rtc emulation
+ *
+ * Copyright (c) 2018 BALATON Zoltan
+ *
+ * This work is licensed under the GNU GPL license version 2 or later.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "qemu/timer.h"
+#include "qemu/bcd.h"
+#include "hw/i2c/i2c.h"
+
+#define TYPE_M41T80 "m41t80"
+#define M41T80(obj) OBJECT_CHECK(M41t80State, (obj), TYPE_M41T80)
+
+typedef struct M41t80State {
+ I2CSlave parent_obj;
+ int8_t addr;
+} M41t80State;
+
+static void m41t80_realize(DeviceState *dev, Error **errp)
+{
+ M41t80State *s = M41T80(dev);
+
+ s->addr = -1;
+}
+
+static int m41t80_send(I2CSlave *i2c, uint8_t data)
+{
+ M41t80State *s = M41T80(i2c);
+
+ if (s->addr < 0) {
+ s->addr = data;
+ } else {
+ s->addr++;
+ }
What about adding enum i2c_event in M41t80State and use the enum here
rather than the addr < 0? Also this wrap at INT8_MAX = 127, is this
expected?
Thanks for the review. I guess we could add enum for device bytes and the
special case -1 meaning no register address selected yet but this is a very
simple device with only 20 bytes and the datasheet also lists them by number
without naming them so I think we can also refer to them by number. Since
the device has only this 20 bytes the case with 127 should also not be a
problem as that's invalid address anyway. Or did you mean something else?
So, I'm not particularly in favour of adding extra state variables.
But is using addr < 0 safe here? You're assigning the uint8_t data to
addr - could that result in a negative value?