[Qemu-devel] [PATCH 3/4] hw/ds1338.c: ensure OSF can only be cleared

[Qemu-devel] [PATCH 3/4] hw/ds1338.c: ensure OSF can only be cleared

[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.

Signed-off-by: Antoine Mathys <barsamin@gmail.com>
---
  hw/ds1338.c |    7 ++++++-
  1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/hw/ds1338.c b/hw/ds1338.c
index 1fb152e..f3c6bc5 100644
--- a/hw/ds1338.c
+++ b/hw/ds1338.c
@@ -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;
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH 3/4] hw/ds1338.c: ensure OSF can only be cleared

[Peter Maydell]

On 2 December 2012 17:19, Antoine Mathys <barsamin@gmail.com> wrote:
> 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.)

> Signed-off-by: Antoine Mathys <barsamin@gmail.com>
> ---
>  hw/ds1338.c |    7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/hw/ds1338.c b/hw/ds1338.c
> index 1fb152e..f3c6bc5 100644
> --- a/hw/ds1338.c
> +++ b/hw/ds1338.c
> @@ -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.

> +
>          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