Skip to contentSkip to conversation

hw/ds1338.c: ensure OSF can only be cleared

PATCH In review Antoine Mathys 2 participants 2 messages Dec 2, 2012 – Dec 4, 2012

lore
Share & tools
lore
#0 lore raw

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.

---
 hw/ds1338.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
hw/ds1338.c +6 -1
index 1fb152e..f3c6bc5 100644
@@ -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;
Signature · 1 line
1.7.10.4
#1 lore raw
Quoted 2 lines · Antoine Mathys

Per the datasheet, the OSF bit in the control register can only be cleared. Attempts to set it have no effect. Implement this.

(As per comments on previous patch, I would suggest making this an "implement the control register" patch.)

Quoted 10 lines
---
 hw/ds1338.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
hw/ds1338.c +0 -0
index 1fb152e..f3c6bc5 100644

hw/ds1338.c static int ds1338_send(I2CSlave *i2c, uint8_t data) · line 167

Quoted 9 lines
hw/ds1338.c +5 -1
@@ -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);

Some constants for bit values would probably help here.

Bits 2, 3 and 6 are RAZ/WI so we should mask those out of the data written.

Quoted 7 lines
hw/ds1338.c +1 -1
+
         s->nvram[s->ptr] = data;
     } else {
         s->nvram[s->ptr] = data;
--

1.7.10.4

Shouldn't we also get the power-on-reset value of this register correct if we're going to implement it?

-- PMM