| Message ID | 8c404c8c4ee1bfd2e4d079877d481094f797df8f.1528291908.git.balaton@eik.bme.hu (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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. > > Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> > --- > MAINTAINERS | 1 + > default-configs/ppc-softmmu.mak | 1 + > hw/timer/Makefile.objs | 1 + > hw/timer/m41t80.c | 117 ++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 120 insertions(+) > create mode 100644 hw/timer/m41t80.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index 41cd373..9e13bc1 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -826,6 +826,7 @@ M: BALATON Zoltan <balaton@eik.bme.hu> > L: qemu-ppc@nongnu.org > S: Maintained > F: hw/ide/sii3112.c > +F: hw/timer/m41t80.c > > SH4 Machines > ------------ > diff --git a/default-configs/ppc-softmmu.mak b/default-configs/ppc-softmmu.mak > index 7d0dc2f..9fbaadc 100644 > --- a/default-configs/ppc-softmmu.mak > +++ b/default-configs/ppc-softmmu.mak > @@ -27,6 +27,7 @@ CONFIG_SM501=y > CONFIG_IDE_SII3112=y > CONFIG_I2C=y > CONFIG_BITBANG_I2C=y > +CONFIG_M41T80=y > > # For Macs > CONFIG_MAC=y > diff --git a/hw/timer/Makefile.objs b/hw/timer/Makefile.objs > index 8b27a4b..e16b2b9 100644 > --- a/hw/timer/Makefile.objs > +++ b/hw/timer/Makefile.objs > @@ -6,6 +6,7 @@ common-obj-$(CONFIG_CADENCE) += cadence_ttc.o > common-obj-$(CONFIG_DS1338) += ds1338.o > common-obj-$(CONFIG_HPET) += hpet.o > common-obj-$(CONFIG_I8254) += i8254_common.o i8254.o > +common-obj-$(CONFIG_M41T80) += m41t80.o > common-obj-$(CONFIG_M48T59) += m48t59.o > ifeq ($(CONFIG_ISA_BUS),y) > common-obj-$(CONFIG_M48T59) += m48t59-isa.o > 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? > + return 0; > +} > + > +static int m41t80_recv(I2CSlave *i2c) > +{ > + M41t80State *s = M41T80(i2c); > + struct tm now; > + qemu_timeval tv; int8_t addr; > + > + if (s->addr < 0) { > + s->addr = 0; > + } addr = s->addr++; So you can simply use 'addr' bellow. > + if (s->addr >= 1 && s->addr <= 7) { > + qemu_get_timedate(&now, -1); > + } > + switch (s->addr++) { > + case 0: > + qemu_gettimeofday(&tv); > + return to_bcd(tv.tv_usec / 10000); > + case 1: > + return to_bcd(now.tm_sec); > + case 2: > + return to_bcd(now.tm_min); > + case 3: > + return to_bcd(now.tm_hour); > + case 4: > + return to_bcd(now.tm_wday); > + case 5: > + return to_bcd(now.tm_mday); > + case 6: > + return to_bcd(now.tm_mon + 1); > + case 7: > + return to_bcd(now.tm_year % 100); > + case 8 ... 19: > + qemu_log_mask(LOG_UNIMP, "\n%s: unimplemented register: %d\n", > + __func__, s->addr - 1); > + return 0; > + default: > + qemu_log_mask(LOG_GUEST_ERROR, "\n%s: invalid register: %d", > + __func__, s->addr - 1); > + return 0; > + } > +} > + > +static int m41t80_event(I2CSlave *i2c, enum i2c_event event) > +{ > + M41t80State *s = M41T80(i2c); > + > + if (event == I2C_START_SEND) { > + s->addr = -1; > + } > + return 0; > +} > + > +static void m41t80_class_init(ObjectClass *klass, void *data) > +{ > + DeviceClass *dc = DEVICE_CLASS(klass); > + I2CSlaveClass *sc = I2C_SLAVE_CLASS(klass); > + > + dc->realize = m41t80_realize; > + sc->send = m41t80_send; > + sc->recv = m41t80_recv; > + sc->event = m41t80_event; > +} > + > +static const TypeInfo m41t80_info = { > + .name = TYPE_M41T80, > + .parent = TYPE_I2C_SLAVE, > + .instance_size = sizeof(M41t80State), > + .class_init = m41t80_class_init, > +}; > + > +static void m41t80_register_types(void) > +{ > + type_register_static(&m41t80_info); > +} > + > +type_init(m41t80_register_types) >
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? >> + return 0; >> +} >> + >> +static int m41t80_recv(I2CSlave *i2c) >> +{ >> + M41t80State *s = M41T80(i2c); >> + struct tm now; >> + qemu_timeval tv; > > int8_t addr; > >> + >> + if (s->addr < 0) { >> + s->addr = 0; >> + } > > addr = s->addr++; > > So you can simply use 'addr' bellow. But that would mean numbers in switch cases below would be +1 compared to how they are listed in the register map in the datasheet but now they match so I'd leave it like this. (In case someone wants to check this with the datasheet, the one I could find on-line says the range of register number 4 (holding day of week) is 1-7 but U-Boot and clients expect 0-6 so I think it's a bug in the datasheet and clients were tested on hardware.) Regards, BALATON Zoltan >> + if (s->addr >= 1 && s->addr <= 7) { >> + qemu_get_timedate(&now, -1); >> + } >> + switch (s->addr++) { >> + case 0: >> + qemu_gettimeofday(&tv); >> + return to_bcd(tv.tv_usec / 10000); >> + case 1: >> + return to_bcd(now.tm_sec); >> + case 2: >> + return to_bcd(now.tm_min); >> + case 3: >> + return to_bcd(now.tm_hour); >> + case 4: >> + return to_bcd(now.tm_wday); >> + case 5: >> + return to_bcd(now.tm_mday); >> + case 6: >> + return to_bcd(now.tm_mon + 1); >> + case 7: >> + return to_bcd(now.tm_year % 100); >> + case 8 ... 19: >> + qemu_log_mask(LOG_UNIMP, "\n%s: unimplemented register: %d\n", >> + __func__, s->addr - 1); >> + return 0; >> + default: >> + qemu_log_mask(LOG_GUEST_ERROR, "\n%s: invalid register: %d", >> + __func__, s->addr - 1); >> + return 0; >> + } >> +} >> + >> +static int m41t80_event(I2CSlave *i2c, enum i2c_event event) >> +{ >> + M41t80State *s = M41T80(i2c); >> + >> + if (event == I2C_START_SEND) { >> + s->addr = -1; >> + } >> + return 0; >> +} >> + >> +static void m41t80_class_init(ObjectClass *klass, void *data) >> +{ >> + DeviceClass *dc = DEVICE_CLASS(klass); >> + I2CSlaveClass *sc = I2C_SLAVE_CLASS(klass); >> + >> + dc->realize = m41t80_realize; >> + sc->send = m41t80_send; >> + sc->recv = m41t80_recv; >> + sc->event = m41t80_event; >> +} >> + >> +static const TypeInfo m41t80_info = { >> + .name = TYPE_M41T80, >> + .parent = TYPE_I2C_SLAVE, >> + .instance_size = sizeof(M41t80State), >> + .class_init = m41t80_class_init, >> +}; >> + >> +static void m41t80_register_types(void) >> +{ >> + type_register_static(&m41t80_info); >> +} >> + >> +type_init(m41t80_register_types) >> > >
On 06/06/2018 03:31 PM, 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. > > Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> > --- > MAINTAINERS | 1 + > default-configs/ppc-softmmu.mak | 1 + > hw/timer/Makefile.objs | 1 + > hw/timer/m41t80.c | 117 ++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 120 insertions(+) > create mode 100644 hw/timer/m41t80.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index 41cd373..9e13bc1 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -826,6 +826,7 @@ M: BALATON Zoltan <balaton@eik.bme.hu> > L: qemu-ppc@nongnu.org > S: Maintained > F: hw/ide/sii3112.c > +F: hw/timer/m41t80.c > > SH4 Machines > ------------ > diff --git a/default-configs/ppc-softmmu.mak b/default-configs/ppc-softmmu.mak > index 7d0dc2f..9fbaadc 100644 > --- a/default-configs/ppc-softmmu.mak > +++ b/default-configs/ppc-softmmu.mak > @@ -27,6 +27,7 @@ CONFIG_SM501=y > CONFIG_IDE_SII3112=y > CONFIG_I2C=y > CONFIG_BITBANG_I2C=y > +CONFIG_M41T80=y > > # For Macs > CONFIG_MAC=y > diff --git a/hw/timer/Makefile.objs b/hw/timer/Makefile.objs > index 8b27a4b..e16b2b9 100644 > --- a/hw/timer/Makefile.objs > +++ b/hw/timer/Makefile.objs > @@ -6,6 +6,7 @@ common-obj-$(CONFIG_CADENCE) += cadence_ttc.o > common-obj-$(CONFIG_DS1338) += ds1338.o > common-obj-$(CONFIG_HPET) += hpet.o > common-obj-$(CONFIG_I8254) += i8254_common.o i8254.o > +common-obj-$(CONFIG_M41T80) += m41t80.o > common-obj-$(CONFIG_M48T59) += m48t59.o > ifeq ($(CONFIG_ISA_BUS),y) > common-obj-$(CONFIG_M48T59) += m48t59-isa.o > 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++; > + } > + return 0; > +} > + > +static int m41t80_recv(I2CSlave *i2c) > +{ > + M41t80State *s = M41T80(i2c); > + struct tm now; > + qemu_timeval tv; > + > + if (s->addr < 0) { > + s->addr = 0; > + } > + if (s->addr >= 1 && s->addr <= 7) { > + qemu_get_timedate(&now, -1); > + } > + switch (s->addr++) { you could use some define to name the registers : > + case 0: > + qemu_gettimeofday(&tv); > + return to_bcd(tv.tv_usec / 10000);> + case 1: > + return to_bcd(now.tm_sec); > + case 2: > + return to_bcd(now.tm_min); > + case 3: > + return to_bcd(now.tm_hour); There is an interesting century bit in specs. > + case 4: > + return to_bcd(now.tm_wday); > + case 5: > + return to_bcd(now.tm_mday); > + case 6: > + return to_bcd(now.tm_mon + 1); > + case 7: > + return to_bcd(now.tm_year % 100); > + case 8 ... 19: > + qemu_log_mask(LOG_UNIMP, "\n%s: unimplemented register: %d\n", is the beginning \n needed ? Thanks, C. > + __func__, s->addr - 1); > + return 0; > + default: > + qemu_log_mask(LOG_GUEST_ERROR, "\n%s: invalid register: %d", > + __func__, s->addr - 1); > + return 0; > + } > +} > + > +static int m41t80_event(I2CSlave *i2c, enum i2c_event event) > +{ > + M41t80State *s = M41T80(i2c); > + > + if (event == I2C_START_SEND) { > + s->addr = -1; > + } > + return 0; > +} > + > +static void m41t80_class_init(ObjectClass *klass, void *data) > +{ > + DeviceClass *dc = DEVICE_CLASS(klass); > + I2CSlaveClass *sc = I2C_SLAVE_CLASS(klass); > + > + dc->realize = m41t80_realize; > + sc->send = m41t80_send; > + sc->recv = m41t80_recv; > + sc->event = m41t80_event; > +} > + > +static const TypeInfo m41t80_info = { > + .name = TYPE_M41T80, > + .parent = TYPE_I2C_SLAVE, > + .instance_size = sizeof(M41t80State), > + .class_init = m41t80_class_init, > +}; > + > +static void m41t80_register_types(void) > +{ > + type_register_static(&m41t80_info); > +} > + > +type_init(m41t80_register_types) >
On Fri, 8 Jun 2018, Cédric Le Goater wrote: > On 06/06/2018 03:31 PM, 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. >> >> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> >> --- >> MAINTAINERS | 1 + >> default-configs/ppc-softmmu.mak | 1 + >> hw/timer/Makefile.objs | 1 + >> hw/timer/m41t80.c | 117 ++++++++++++++++++++++++++++++++++++++++ >> 4 files changed, 120 insertions(+) >> create mode 100644 hw/timer/m41t80.c >> >> diff --git a/MAINTAINERS b/MAINTAINERS >> index 41cd373..9e13bc1 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -826,6 +826,7 @@ M: BALATON Zoltan <balaton@eik.bme.hu> >> L: qemu-ppc@nongnu.org >> S: Maintained >> F: hw/ide/sii3112.c >> +F: hw/timer/m41t80.c >> >> SH4 Machines >> ------------ >> diff --git a/default-configs/ppc-softmmu.mak b/default-configs/ppc-softmmu.mak >> index 7d0dc2f..9fbaadc 100644 >> --- a/default-configs/ppc-softmmu.mak >> +++ b/default-configs/ppc-softmmu.mak >> @@ -27,6 +27,7 @@ CONFIG_SM501=y >> CONFIG_IDE_SII3112=y >> CONFIG_I2C=y >> CONFIG_BITBANG_I2C=y >> +CONFIG_M41T80=y >> >> # For Macs >> CONFIG_MAC=y >> diff --git a/hw/timer/Makefile.objs b/hw/timer/Makefile.objs >> index 8b27a4b..e16b2b9 100644 >> --- a/hw/timer/Makefile.objs >> +++ b/hw/timer/Makefile.objs >> @@ -6,6 +6,7 @@ common-obj-$(CONFIG_CADENCE) += cadence_ttc.o >> common-obj-$(CONFIG_DS1338) += ds1338.o >> common-obj-$(CONFIG_HPET) += hpet.o >> common-obj-$(CONFIG_I8254) += i8254_common.o i8254.o >> +common-obj-$(CONFIG_M41T80) += m41t80.o >> common-obj-$(CONFIG_M48T59) += m48t59.o >> ifeq ($(CONFIG_ISA_BUS),y) >> common-obj-$(CONFIG_M48T59) += m48t59-isa.o >> 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++; >> + } >> + return 0; >> +} >> + >> +static int m41t80_recv(I2CSlave *i2c) >> +{ >> + M41t80State *s = M41T80(i2c); >> + struct tm now; >> + qemu_timeval tv; >> + >> + if (s->addr < 0) { >> + s->addr = 0; >> + } >> + if (s->addr >= 1 && s->addr <= 7) { >> + qemu_get_timedate(&now, -1); >> + } >> + switch (s->addr++) { > > you could use some define to name the registers : This was also suggested by Philippe Mathieu-Daudé and my answer to that was that I don't feel like I want to come up with names the datasheet does not have either. I think this device is simple enough with just 20 consecutively numbered registers that appear only in these switch cases by number as in the datasheet table so that I don't want to make it more difficult to read by encrypting these numbers behind some arbitrary defines without a good reason. They are also so simple that it's clear from the usually one line implementation what they do so that's also not a good reason to name them. >> + case 0: >> + qemu_gettimeofday(&tv); >> + return to_bcd(tv.tv_usec / 10000);> + case 1: >> + return to_bcd(now.tm_sec); >> + case 2: >> + return to_bcd(now.tm_min); >> + case 3: >> + return to_bcd(now.tm_hour); > > There is an interesting century bit in specs. Which I could not figure out how should work and guests seem to be happy without it so I did not try to implement it. >> + case 4: >> + return to_bcd(now.tm_wday); >> + case 5: >> + return to_bcd(now.tm_mday); >> + case 6: >> + return to_bcd(now.tm_mon + 1); >> + case 7: >> + return to_bcd(now.tm_year % 100); >> + case 8 ... 19: >> + qemu_log_mask(LOG_UNIMP, "\n%s: unimplemented register: %d\n", > > is the beginning \n needed ? Probably not, maybe left there due to previous debug logs I've removed. I'll drop the beginning \n-s in next version. > Thanks, > > C. Thanks for the review, BALATON Zoltan
On 06/08/2018 06:16 PM, BALATON Zoltan wrote: > On Fri, 8 Jun 2018, Cédric Le Goater wrote: >> On 06/06/2018 03:31 PM, 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. >>> >>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> >>> --- >>> MAINTAINERS | 1 + >>> default-configs/ppc-softmmu.mak | 1 + >>> hw/timer/Makefile.objs | 1 + >>> hw/timer/m41t80.c | 117 ++++++++++++++++++++++++++++++++++++++++ >>> 4 files changed, 120 insertions(+) >>> create mode 100644 hw/timer/m41t80.c >>> >>> diff --git a/MAINTAINERS b/MAINTAINERS >>> index 41cd373..9e13bc1 100644 >>> --- a/MAINTAINERS >>> +++ b/MAINTAINERS >>> @@ -826,6 +826,7 @@ M: BALATON Zoltan <balaton@eik.bme.hu> >>> L: qemu-ppc@nongnu.org >>> S: Maintained >>> F: hw/ide/sii3112.c >>> +F: hw/timer/m41t80.c >>> >>> SH4 Machines >>> ------------ >>> diff --git a/default-configs/ppc-softmmu.mak b/default-configs/ppc-softmmu.mak >>> index 7d0dc2f..9fbaadc 100644 >>> --- a/default-configs/ppc-softmmu.mak >>> +++ b/default-configs/ppc-softmmu.mak >>> @@ -27,6 +27,7 @@ CONFIG_SM501=y >>> CONFIG_IDE_SII3112=y >>> CONFIG_I2C=y >>> CONFIG_BITBANG_I2C=y >>> +CONFIG_M41T80=y >>> >>> # For Macs >>> CONFIG_MAC=y >>> diff --git a/hw/timer/Makefile.objs b/hw/timer/Makefile.objs >>> index 8b27a4b..e16b2b9 100644 >>> --- a/hw/timer/Makefile.objs >>> +++ b/hw/timer/Makefile.objs >>> @@ -6,6 +6,7 @@ common-obj-$(CONFIG_CADENCE) += cadence_ttc.o >>> common-obj-$(CONFIG_DS1338) += ds1338.o >>> common-obj-$(CONFIG_HPET) += hpet.o >>> common-obj-$(CONFIG_I8254) += i8254_common.o i8254.o >>> +common-obj-$(CONFIG_M41T80) += m41t80.o >>> common-obj-$(CONFIG_M48T59) += m48t59.o >>> ifeq ($(CONFIG_ISA_BUS),y) >>> common-obj-$(CONFIG_M48T59) += m48t59-isa.o >>> 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++; >>> + } >>> + return 0; >>> +} >>> + >>> +static int m41t80_recv(I2CSlave *i2c) >>> +{ >>> + M41t80State *s = M41T80(i2c); >>> + struct tm now; >>> + qemu_timeval tv; >>> + >>> + if (s->addr < 0) { >>> + s->addr = 0; >>> + } >>> + if (s->addr >= 1 && s->addr <= 7) { >>> + qemu_get_timedate(&now, -1); >>> + } >>> + switch (s->addr++) { >> >> you could use some define to name the registers : > > This was also suggested by Philippe Mathieu-Daudé and my answer to that was that I don't feel like I want to come up with names the datasheet does not have either. I think this device is simple enough with just 20 consecutively numbered registers that appear only in these switch cases by number as in the datasheet table so that I don't want to make it more difficult to read by encrypting these numbers behind some arbitrary defines without a good reason. They are also so simple that it's clear from the usually one line implementation what they do so that's also not a good reason to name them. OK. It's fine with me but you might get some inspiration from Linux for the names :) >>> + case 0: >>> + qemu_gettimeofday(&tv); >>> + return to_bcd(tv.tv_usec / 10000);> + case 1: >>> + return to_bcd(now.tm_sec); >>> + case 2: >>> + return to_bcd(now.tm_min); >>> + case 3: >>> + return to_bcd(now.tm_hour); >> >> There is an interesting century bit in specs. > > Which I could not figure out how should work and guests seem to be happy without it so I did not try to implement it. yes. It seems that Linux simply ignores it. Let's forget it. Thanks, C. >>> + case 4: >>> + return to_bcd(now.tm_wday); >>> + case 5: >>> + return to_bcd(now.tm_mday); >>> + case 6: >>> + return to_bcd(now.tm_mon + 1); >>> + case 7: >>> + return to_bcd(now.tm_year % 100); >>> + case 8 ... 19: >>> + qemu_log_mask(LOG_UNIMP, "\n%s: unimplemented register: %d\n", >> >> is the beginning \n needed ? > > Probably not, maybe left there due to previous debug logs I've removed. I'll drop the beginning \n-s in next version. > >> Thanks, >> >> C. > > Thanks for the review, > BALATON Zoltan
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?
On Wed, 13 Jun 2018, David Gibson wrote: > 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? Why wouldn't it be safe with the expected values for register address between 0-19? If the guest sends garbage values over 127 it will either result in invalid register or unselected register and lead to an error when trying to read/write that register so I don't see what other problem this may cause. The addr < 0 is to check if no address was selected before (on creating the device and when sending first value from host addr is set to -1. In this case first write will set register address, then subsequent reads/writes increment register address as the datasheet says). Regards, BALATON Zoltan
On Wed, Jun 13, 2018 at 10:50:59AM +0200, BALATON Zoltan wrote: > On Wed, 13 Jun 2018, David Gibson wrote: > > 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? > > Why wouldn't it be safe with the expected values for register address > between 0-19? If the guest sends garbage values over 127 it will either > result in invalid register or unselected register and lead to an error when > trying to read/write that register so I don't see what other problem this > may cause. Ok, but where is that enforced? > The addr < 0 is to check if no address was selected before (on creating the > device and when sending first value from host addr is set to -1. In this > case first write will set register address, then subsequent reads/writes > increment register address as the datasheet says). > > Regards, > BALATON Zoltan
On Wed, 13 Jun 2018, David Gibson wrote: > On Wed, Jun 13, 2018 at 10:50:59AM +0200, BALATON Zoltan wrote: >> On Wed, 13 Jun 2018, David Gibson wrote: >>> 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? >> >> Why wouldn't it be safe with the expected values for register address >> between 0-19? If the guest sends garbage values over 127 it will either >> result in invalid register or unselected register and lead to an error when >> trying to read/write that register so I don't see what other problem this >> may cause. > > Ok, but where is that enforced? I don't see the problem. The addr register selects the register to read or write. It is set by the first write when the device is accessed the first time (this is denoted by addr == -1 (or really any negative value). The device has 20 registers and trying to read any register outside addr between 0-19 will result in returning 0 and logging a warning about invalid register in m41t80_recv. What could fail here when guest sends garbage? It will set addr to invalid value and try to read non-exitent register and get an error just like for any other nonexistent value of addr (or start to read from register 0 if it manages to set a negative value). All writes of registers are ignored currently (except setting addr by the first write). What should be enforced here? Regards, BALATON Zoltan
On Wed, Jun 13, 2018 at 04:13:57PM +0200, BALATON Zoltan wrote: > On Wed, 13 Jun 2018, David Gibson wrote: > > On Wed, Jun 13, 2018 at 10:50:59AM +0200, BALATON Zoltan wrote: > > > On Wed, 13 Jun 2018, David Gibson wrote: > > > > 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? > > > > > > Why wouldn't it be safe with the expected values for register address > > > between 0-19? If the guest sends garbage values over 127 it will either > > > result in invalid register or unselected register and lead to an error when > > > trying to read/write that register so I don't see what other problem this > > > may cause. > > > > Ok, but where is that enforced? > > I don't see the problem. The addr register selects the register to read or > write. It is set by the first write when the device is accessed the first > time (this is denoted by addr == -1 (or really any negative value). The > device has 20 registers and trying to read any register outside addr between > 0-19 will result in returning 0 and logging a warning about invalid register > in m41t80_recv. What could fail here when guest sends garbage? It will set > addr to invalid value and try to read non-exitent register and get an error > just like for any other nonexistent value of addr (or start to read from > register 0 if it manages to set a negative value). All writes of registers > are ignored currently (except setting addr by the first write). What should > be enforced here? Ah, I see your point. I mean strictly we should match the hardware behaviour if you write garbage addresses here, but really I don't think it matters much. Ok, it should be fine.
On Thu, 14 Jun 2018, David Gibson wrote: > On Wed, Jun 13, 2018 at 04:13:57PM +0200, BALATON Zoltan wrote: >> I don't see the problem. The addr register selects the register to read or >> write. It is set by the first write when the device is accessed the first >> time (this is denoted by addr == -1 (or really any negative value). The >> device has 20 registers and trying to read any register outside addr between >> 0-19 will result in returning 0 and logging a warning about invalid register >> in m41t80_recv. What could fail here when guest sends garbage? It will set >> addr to invalid value and try to read non-exitent register and get an error >> just like for any other nonexistent value of addr (or start to read from >> register 0 if it manages to set a negative value). All writes of registers >> are ignored currently (except setting addr by the first write). What should >> be enforced here? > > Ah, I see your point. I mean strictly we should match the hardware > behaviour if you write garbage addresses here, but really I don't > think it matters much. Problem is like usual I have no idea what the real hardware does. I've only seen the datasheet, never seen real device and have no way to test. All the clients I've tested seem to be OK with the current emulation so unless someone has more info on how this should work I think we can live with this version and then fix it later if found to be needed. Regards, BALATON Zoltan
On Thu, Jun 14, 2018 at 09:54:41AM +0200, BALATON Zoltan wrote: > On Thu, 14 Jun 2018, David Gibson wrote: > > On Wed, Jun 13, 2018 at 04:13:57PM +0200, BALATON Zoltan wrote: > > > I don't see the problem. The addr register selects the register to read or > > > write. It is set by the first write when the device is accessed the first > > > time (this is denoted by addr == -1 (or really any negative value). The > > > device has 20 registers and trying to read any register outside addr between > > > 0-19 will result in returning 0 and logging a warning about invalid register > > > in m41t80_recv. What could fail here when guest sends garbage? It will set > > > addr to invalid value and try to read non-exitent register and get an error > > > just like for any other nonexistent value of addr (or start to read from > > > register 0 if it manages to set a negative value). All writes of registers > > > are ignored currently (except setting addr by the first write). What should > > > be enforced here? > > > > Ah, I see your point. I mean strictly we should match the hardware > > behaviour if you write garbage addresses here, but really I don't > > think it matters much. > > Problem is like usual I have no idea what the real hardware does. I've only > seen the datasheet, never seen real device and have no way to test. All the > clients I've tested seem to be OK with the current emulation so unless > someone has more info on how this should work I think we can live with this > version and then fix it later if found to be needed. Ok, fair enough.
diff --git a/MAINTAINERS b/MAINTAINERS index 41cd373..9e13bc1 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -826,6 +826,7 @@ M: BALATON Zoltan <balaton@eik.bme.hu> L: qemu-ppc@nongnu.org S: Maintained F: hw/ide/sii3112.c +F: hw/timer/m41t80.c SH4 Machines ------------ diff --git a/default-configs/ppc-softmmu.mak b/default-configs/ppc-softmmu.mak index 7d0dc2f..9fbaadc 100644 --- a/default-configs/ppc-softmmu.mak +++ b/default-configs/ppc-softmmu.mak @@ -27,6 +27,7 @@ CONFIG_SM501=y CONFIG_IDE_SII3112=y CONFIG_I2C=y CONFIG_BITBANG_I2C=y +CONFIG_M41T80=y # For Macs CONFIG_MAC=y diff --git a/hw/timer/Makefile.objs b/hw/timer/Makefile.objs index 8b27a4b..e16b2b9 100644 --- a/hw/timer/Makefile.objs +++ b/hw/timer/Makefile.objs @@ -6,6 +6,7 @@ common-obj-$(CONFIG_CADENCE) += cadence_ttc.o common-obj-$(CONFIG_DS1338) += ds1338.o common-obj-$(CONFIG_HPET) += hpet.o common-obj-$(CONFIG_I8254) += i8254_common.o i8254.o +common-obj-$(CONFIG_M41T80) += m41t80.o common-obj-$(CONFIG_M48T59) += m48t59.o ifeq ($(CONFIG_ISA_BUS),y) common-obj-$(CONFIG_M48T59) += m48t59-isa.o 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++; + } + return 0; +} + +static int m41t80_recv(I2CSlave *i2c) +{ + M41t80State *s = M41T80(i2c); + struct tm now; + qemu_timeval tv; + + if (s->addr < 0) { + s->addr = 0; + } + if (s->addr >= 1 && s->addr <= 7) { + qemu_get_timedate(&now, -1); + } + switch (s->addr++) { + case 0: + qemu_gettimeofday(&tv); + return to_bcd(tv.tv_usec / 10000); + case 1: + return to_bcd(now.tm_sec); + case 2: + return to_bcd(now.tm_min); + case 3: + return to_bcd(now.tm_hour); + case 4: + return to_bcd(now.tm_wday); + case 5: + return to_bcd(now.tm_mday); + case 6: + return to_bcd(now.tm_mon + 1); + case 7: + return to_bcd(now.tm_year % 100); + case 8 ... 19: + qemu_log_mask(LOG_UNIMP, "\n%s: unimplemented register: %d\n", + __func__, s->addr - 1); + return 0; + default: + qemu_log_mask(LOG_GUEST_ERROR, "\n%s: invalid register: %d", + __func__, s->addr - 1); + return 0; + } +} + +static int m41t80_event(I2CSlave *i2c, enum i2c_event event) +{ + M41t80State *s = M41T80(i2c); + + if (event == I2C_START_SEND) { + s->addr = -1; + } + return 0; +} + +static void m41t80_class_init(ObjectClass *klass, void *data) +{ + DeviceClass *dc = DEVICE_CLASS(klass); + I2CSlaveClass *sc = I2C_SLAVE_CLASS(klass); + + dc->realize = m41t80_realize; + sc->send = m41t80_send; + sc->recv = m41t80_recv; + sc->event = m41t80_event; +} + +static const TypeInfo m41t80_info = { + .name = TYPE_M41T80, + .parent = TYPE_I2C_SLAVE, + .instance_size = sizeof(M41t80State), + .class_init = m41t80_class_init, +}; + +static void m41t80_register_types(void) +{ + type_register_static(&m41t80_info); +} + +type_init(m41t80_register_types)
Basic emulation of the M41T80 serial (I2C) RTC chip. Only getting time of day is implemented. Setting time and RTC alarm are not supported. Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> --- MAINTAINERS | 1 + default-configs/ppc-softmmu.mak | 1 + hw/timer/Makefile.objs | 1 + hw/timer/m41t80.c | 117 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 120 insertions(+) create mode 100644 hw/timer/m41t80.c