| Message ID | 50BB8D3D.8050205@gmail.com |
|---|---|
| State | New |
| Headers | show |
On 2 December 2012 17:17, Antoine Mathys <barsamin@gmail.com> wrote: > Minor fixes in the handling of the RTC registers. > > Signed-off-by: Antoine Mathys <barsamin@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); > } Yep, doing arithmetic after to_bcd() is pretty much always a bug. > > @@ -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; Why bother changing this? > 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; The comment says ignored but the code isn't doing that: this change will make it read-as-written. I think you should put this movement of the control register out of the switch() in the next patch, not this one. That would leave this patch with just the to_bcd() fixes, and you could give it a more precise commit message then. > } else { > s->nvram[s->ptr] = data; > } > -- > 1.7.10.4 > -- PMM
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; }
Minor fixes in the handling of the RTC registers. Signed-off-by: Antoine Mathys <barsamin@gmail.com> --- hw/ds1338.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-)