On 24 March 2018 at 19:24, Michael Davidsaver <mdavidsa...@gmail.com> wrote: > Add class level handling for address space size > and control register. > > Signed-off-by: Michael Davidsaver <mdavidsa...@gmail.com> > ---
> @@ -206,22 +216,15 @@ static void dsrtc_update(DSRTCState *s) > static int dsrtc_send(I2CSlave *i2c, uint8_t data) > { > DSRTCState *s = DSRTC(i2c); > + DSRTCClass *k = DSRTC_GET_CLASS(s); > > if (s->addr_byte) { > - s->ptr = data & (NVRAM_SIZE - 1); > + s->ptr = data % k->addr_size; > s->addr_byte = false; > return 0; > } > - if (s->ptr == R_CTRL) { > - /* Control register. */ > - > - /* Ensure bits 2, 3 and 6 will read back as zero. */ > - data &= 0xB3; > - > - /* Attempting to write the OSF flag to logic 1 leaves the > - value unchanged. */ > - data = (data & ~CTRL_OSF) | (data & s->nvram[s->ptr] & CTRL_OSF); > - > + if (s->ptr == k->ctrl_offset) { > + (k->ctrl_write)(s, data); > } > s->nvram[s->ptr] = data; > if (s->ptr <= R_YEAR) { > @@ -256,15 +259,41 @@ static void dsrtc_class_init(ObjectClass *klass, void > *data) > } > > static const TypeInfo dsrtc_info = { > + .abstract = true, > .name = TYPE_DSRTC, > .parent = TYPE_I2C_SLAVE, > .instance_size = sizeof(DSRTCState), > .class_init = dsrtc_class_init, > }; > > +static void ds1338_control_write(DSRTCState *s, uint8_t data) > +{ > + /* Control register. */ > + > + /* allow guest to set no-op controls for clock out pin */ > + s->nvram[R_DS1338_CTRL] = data & 0x93; This is mixing a behavioural change with an otherwise pure refactoring -- can you split them out, please? Otherwise the patch looks good. thanks -- PMM