diff mbox series

[04/14] timer: ds1338 clarify HOUR handling

Message ID 20180324192455.12254-5-mdavidsaver@gmail.com
State New
Headers show
Series Generalize Dallas/Maxim I2C RTC devices v2 | expand

Commit Message

Michael Davidsaver March 24, 2018, 7:24 p.m. UTC
Simplify and comment the translation between
registers and struct tm.

Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
---
 hw/timer/ds1338.c | 35 +++++++++++++++++------------------
 1 file changed, 17 insertions(+), 18 deletions(-)

Comments

Peter Maydell April 12, 2018, 6:04 p.m. UTC | #1
On 24 March 2018 at 19:24, Michael Davidsaver <mdavidsaver@gmail.com> wrote:
> Simplify and comment the translation between
> registers and struct tm.
>
> Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
> ---
>  hw/timer/ds1338.c | 35 +++++++++++++++++------------------
>  1 file changed, 17 insertions(+), 18 deletions(-)
>
> diff --git a/hw/timer/ds1338.c b/hw/timer/ds1338.c
> index 72a4692d60..9bcda26e51 100644
> --- a/hw/timer/ds1338.c
> +++ b/hw/timer/ds1338.c
> @@ -88,23 +88,23 @@ static void capture_current_time(DS1338State *s)
>       * which will be actually read by the data transfer operation.
>       */
>      struct tm now;
> +    bool mode12 = ARRAY_FIELD_EX32(s->nvram, HOUR, SET12);
>      qemu_get_timedate(&now, s->offset);
> +
>      s->nvram[R_SEC] = to_bcd(now.tm_sec);
>      s->nvram[R_MIN] = to_bcd(now.tm_min);
> -    if (ARRAY_FIELD_EX32(s->nvram, HOUR, SET12)) {
> -        int tmp = now.tm_hour;
> -        if (tmp % 12 == 0) {
> -            tmp += 12;
> -        }
> -        s->nvram[R_HOUR] = 0;
> +    s->nvram[R_HOUR] = 0;
> +    if (mode12) {
> +        /* map 0-23 to 1-12 am/pm */
>          ARRAY_FIELD_DP32(s->nvram, HOUR, SET12, 1);
> -        if (tmp <= 12) {
> -            ARRAY_FIELD_DP32(s->nvram, HOUR, AMPM, 0);
> -            ARRAY_FIELD_DP32(s->nvram, HOUR, HOUR12, to_bcd(tmp));
> -        } else {
> -            ARRAY_FIELD_DP32(s->nvram, HOUR, AMPM, 1);
> -            ARRAY_FIELD_DP32(s->nvram, HOUR, HOUR12, to_bcd(tmp - 12));

Oh, you're rewriting all this code from the earlier patch anyway.
In that case you should definitely just have the earlier patch
match the logic that the existing code did, so that it's easier
to review as being a no-change patch.

> +        ARRAY_FIELD_DP32(s->nvram, HOUR, AMPM, now.tm_hour >= 12u);
> +        now.tm_hour %= 12u; /* wrap 0-23 to 0-11 */
> +        if (now.tm_hour == 0u) {
> +            /* midnight/noon stored as 12 */
> +            now.tm_hour = 12u;
>          }
> +        ARRAY_FIELD_DP32(s->nvram, HOUR, HOUR12, to_bcd(now.tm_hour));
> +
>      } else {
>          ARRAY_FIELD_DP32(s->nvram, HOUR, HOUR24, to_bcd(now.tm_hour));
>      }
> @@ -182,14 +182,13 @@ static int ds1338_send(I2CSlave *i2c, uint8_t data)
>              break;
>          case R_HOUR:
>              if (FIELD_EX32(data, HOUR, SET12)) {
> -                int tmp = from_bcd(FIELD_EX32(data, HOUR, HOUR12));
> +                /* 12 hour (1-12) */
> +                /* read and wrap 1-12 -> 0-11 */
> +                now.tm_hour = from_bcd(FIELD_EX32(data, HOUR, HOUR12)) % 12u;
>                  if (FIELD_EX32(data, HOUR, AMPM)) {
> -                    tmp += 12;
> +                    now.tm_hour += 12;
>                  }
> -                if (tmp % 12 == 0) {
> -                    tmp -= 12;
> -                }
> -                now.tm_hour = tmp;
> +
>              } else {
>                  now.tm_hour = from_bcd(FIELD_EX32(data, HOUR, HOUR24));
>              }
> --
> 2.11.0
>


thanks
-- PMM
diff mbox series

Patch

diff --git a/hw/timer/ds1338.c b/hw/timer/ds1338.c
index 72a4692d60..9bcda26e51 100644
--- a/hw/timer/ds1338.c
+++ b/hw/timer/ds1338.c
@@ -88,23 +88,23 @@  static void capture_current_time(DS1338State *s)
      * which will be actually read by the data transfer operation.
      */
     struct tm now;
+    bool mode12 = ARRAY_FIELD_EX32(s->nvram, HOUR, SET12);
     qemu_get_timedate(&now, s->offset);
+
     s->nvram[R_SEC] = to_bcd(now.tm_sec);
     s->nvram[R_MIN] = to_bcd(now.tm_min);
-    if (ARRAY_FIELD_EX32(s->nvram, HOUR, SET12)) {
-        int tmp = now.tm_hour;
-        if (tmp % 12 == 0) {
-            tmp += 12;
-        }
-        s->nvram[R_HOUR] = 0;
+    s->nvram[R_HOUR] = 0;
+    if (mode12) {
+        /* map 0-23 to 1-12 am/pm */
         ARRAY_FIELD_DP32(s->nvram, HOUR, SET12, 1);
-        if (tmp <= 12) {
-            ARRAY_FIELD_DP32(s->nvram, HOUR, AMPM, 0);
-            ARRAY_FIELD_DP32(s->nvram, HOUR, HOUR12, to_bcd(tmp));
-        } else {
-            ARRAY_FIELD_DP32(s->nvram, HOUR, AMPM, 1);
-            ARRAY_FIELD_DP32(s->nvram, HOUR, HOUR12, to_bcd(tmp - 12));
+        ARRAY_FIELD_DP32(s->nvram, HOUR, AMPM, now.tm_hour >= 12u);
+        now.tm_hour %= 12u; /* wrap 0-23 to 0-11 */
+        if (now.tm_hour == 0u) {
+            /* midnight/noon stored as 12 */
+            now.tm_hour = 12u;
         }
+        ARRAY_FIELD_DP32(s->nvram, HOUR, HOUR12, to_bcd(now.tm_hour));
+
     } else {
         ARRAY_FIELD_DP32(s->nvram, HOUR, HOUR24, to_bcd(now.tm_hour));
     }
@@ -182,14 +182,13 @@  static int ds1338_send(I2CSlave *i2c, uint8_t data)
             break;
         case R_HOUR:
             if (FIELD_EX32(data, HOUR, SET12)) {
-                int tmp = from_bcd(FIELD_EX32(data, HOUR, HOUR12));
+                /* 12 hour (1-12) */
+                /* read and wrap 1-12 -> 0-11 */
+                now.tm_hour = from_bcd(FIELD_EX32(data, HOUR, HOUR12)) % 12u;
                 if (FIELD_EX32(data, HOUR, AMPM)) {
-                    tmp += 12;
+                    now.tm_hour += 12;
                 }
-                if (tmp % 12 == 0) {
-                    tmp -= 12;
-                }
-                now.tm_hour = tmp;
+
             } else {
                 now.tm_hour = from_bcd(FIELD_EX32(data, HOUR, HOUR24));
             }