[PATCH v2 0/5] tmp105: Improvements and fixes

[PATCH v2 0/5] tmp105: Improvements and fixes

[Philippe Mathieu-Daudé]

Respin of Guenter fixes with:
- Use registerfields API
- Clear OS bit in WRITE path

Supersedes: <20240906132912.3826089-1-linux@roeck-us.net>

Guenter Roeck (2):
  hw/sensor/tmp105: Coding style fixes
  hw/sensor/tmp105: Lower 4 bit of limit registers are always 0

Philippe Mathieu-Daudé (3):
  hw/sensor/tmp105: Use registerfields API
  hw/sensor/tmp105: Pass 'oneshot' argument to tmp105_alarm_update()
  hw/sensor/tmp105: OS (one-shot) bit in config register always returns
    0

 hw/sensor/tmp105.c | 66 ++++++++++++++++++++++++++--------------------
 1 file changed, 37 insertions(+), 29 deletions(-)

-- 
2.45.2

[PATCH v2 1/5] hw/sensor/tmp105: Coding style fixes

[Philippe Mathieu-Daudé]

From: Guenter Roeck <linux@roeck-us.net>

Coding style asks for no space between variable and "++". The next patch
in this series will change one of those assignments. Instead of changing
just one with that patch, change all of them for consistency.

While at it, also fix other coding style problems reported by checkpatch.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/sensor/tmp105.c | 44 ++++++++++++++++++++++++--------------------
 1 file changed, 24 insertions(+), 20 deletions(-)

diff --git a/hw/sensor/tmp105.c b/hw/sensor/tmp105.c
index a8730d0b7f..ad97c9684c 100644
--- a/hw/sensor/tmp105.c
+++ b/hw/sensor/tmp105.c
@@ -29,16 +29,17 @@
 
 static void tmp105_interrupt_update(TMP105State *s)
 {
-    qemu_set_irq(s->pin, s->alarm ^ ((~s->config >> 2) & 1));	/* POL */
+    qemu_set_irq(s->pin, s->alarm ^ ((~s->config >> 2) & 1));   /* POL */
 }
 
 static void tmp105_alarm_update(TMP105State *s)
 {
-    if ((s->config >> 0) & 1) {					/* SD */
-        if ((s->config >> 7) & 1)				/* OS */
-            s->config &= ~(1 << 7);				/* OS */
-        else
+    if ((s->config >> 0) & 1) {                                 /* SD */
+        if ((s->config >> 7) & 1) {                             /* OS */
+            s->config &= ~(1 << 7);                             /* OS */
+        } else {
             return;
+        }
     }
 
     if (s->config >> 1 & 1) {
@@ -89,7 +90,8 @@ static void tmp105_get_temperature(Object *obj, Visitor *v, const char *name,
     visit_type_int(v, name, &value, errp);
 }
 
-/* Units are 0.001 centigrades relative to 0 C.  s->temperature is 8.8
+/*
+ * Units are 0.001 centigrades relative to 0 C.  s->temperature is 8.8
  * fixed point, so units are 1/256 centigrades.  A simple ratio will do.
  */
 static void tmp105_set_temperature(Object *obj, Visitor *v, const char *name,
@@ -118,30 +120,30 @@ static void tmp105_read(TMP105State *s)
 {
     s->len = 0;
 
-    if ((s->config >> 1) & 1) {					/* TM */
+    if ((s->config >> 1) & 1) {                                 /* TM */
         s->alarm = 0;
         tmp105_interrupt_update(s);
     }
 
     switch (s->pointer & 3) {
     case TMP105_REG_TEMPERATURE:
-        s->buf[s->len ++] = (((uint16_t) s->temperature) >> 8);
-        s->buf[s->len ++] = (((uint16_t) s->temperature) >> 0) &
-                (0xf0 << ((~s->config >> 5) & 3));		/* R */
+        s->buf[s->len++] = (((uint16_t) s->temperature) >> 8);
+        s->buf[s->len++] = (((uint16_t) s->temperature) >> 0) &
+                (0xf0 << ((~s->config >> 5) & 3));              /* R */
         break;
 
     case TMP105_REG_CONFIG:
-        s->buf[s->len ++] = s->config;
+        s->buf[s->len++] = s->config;
         break;
 
     case TMP105_REG_T_LOW:
-        s->buf[s->len ++] = ((uint16_t) s->limit[0]) >> 8;
-        s->buf[s->len ++] = ((uint16_t) s->limit[0]) >> 0;
+        s->buf[s->len++] = ((uint16_t) s->limit[0]) >> 8;
+        s->buf[s->len++] = ((uint16_t) s->limit[0]) >> 0;
         break;
 
     case TMP105_REG_T_HIGH:
-        s->buf[s->len ++] = ((uint16_t) s->limit[1]) >> 8;
-        s->buf[s->len ++] = ((uint16_t) s->limit[1]) >> 0;
+        s->buf[s->len++] = ((uint16_t) s->limit[1]) >> 8;
+        s->buf[s->len++] = ((uint16_t) s->limit[1]) >> 0;
         break;
     }
 }
@@ -153,18 +155,20 @@ static void tmp105_write(TMP105State *s)
         break;
 
     case TMP105_REG_CONFIG:
-        if (s->buf[0] & ~s->config & (1 << 0))			/* SD */
+        if (s->buf[0] & ~s->config & (1 << 0)) {                /* SD */
             printf("%s: TMP105 shutdown\n", __func__);
+        }
         s->config = s->buf[0];
-        s->faults = tmp105_faultq[(s->config >> 3) & 3];	/* F */
+        s->faults = tmp105_faultq[(s->config >> 3) & 3];        /* F */
         tmp105_alarm_update(s);
         break;
 
     case TMP105_REG_T_LOW:
     case TMP105_REG_T_HIGH:
-        if (s->len >= 3)
+        if (s->len >= 3) {
             s->limit[s->pointer & 1] = (int16_t)
                     ((((uint16_t) s->buf[0]) << 8) | s->buf[1]);
+        }
         tmp105_alarm_update(s);
         break;
     }
@@ -175,7 +179,7 @@ static uint8_t tmp105_rx(I2CSlave *i2c)
     TMP105State *s = TMP105(i2c);
 
     if (s->len < 2) {
-        return s->buf[s->len ++];
+        return s->buf[s->len++];
     } else {
         return 0xff;
     }
@@ -215,7 +219,7 @@ static int tmp105_post_load(void *opaque, int version_id)
 {
     TMP105State *s = opaque;
 
-    s->faults = tmp105_faultq[(s->config >> 3) & 3];		/* F */
+    s->faults = tmp105_faultq[(s->config >> 3) & 3];            /* F */
 
     tmp105_interrupt_update(s);
     return 0;
-- 
2.45.2

[PATCH v2 2/5] hw/sensor/tmp105: Use registerfields API

[Philippe Mathieu-Daudé]

To improve readability, use the registerfields API.
Define the register bits with FIELD(), and use the
FIELD_EX8() and FIELD_DP8() macros. Remove the
abbreviations in comments.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/sensor/tmp105.c | 30 +++++++++++++++++++-----------
 1 file changed, 19 insertions(+), 11 deletions(-)

diff --git a/hw/sensor/tmp105.c b/hw/sensor/tmp105.c
index ad97c9684c..150d09b278 100644
--- a/hw/sensor/tmp105.c
+++ b/hw/sensor/tmp105.c
@@ -26,23 +26,31 @@
 #include "qapi/error.h"
 #include "qapi/visitor.h"
 #include "qemu/module.h"
+#include "hw/registerfields.h"
+
+FIELD(CONFIG, SHUTDOWN_MODE,        0, 1)
+FIELD(CONFIG, THERMOSTAT_MODE,      1, 1)
+FIELD(CONFIG, POLARITY,             2, 1)
+FIELD(CONFIG, FAULT_QUEUE,          3, 2)
+FIELD(CONFIG, CONVERTER_RESOLUTION, 5, 2)
+FIELD(CONFIG, ONE_SHOT,             7, 1)
 
 static void tmp105_interrupt_update(TMP105State *s)
 {
-    qemu_set_irq(s->pin, s->alarm ^ ((~s->config >> 2) & 1));   /* POL */
+    qemu_set_irq(s->pin, s->alarm ^ FIELD_EX8(~s->config, CONFIG, POLARITY));
 }
 
 static void tmp105_alarm_update(TMP105State *s)
 {
-    if ((s->config >> 0) & 1) {                                 /* SD */
-        if ((s->config >> 7) & 1) {                             /* OS */
-            s->config &= ~(1 << 7);                             /* OS */
+    if (FIELD_EX8(s->config, CONFIG, SHUTDOWN_MODE)) {
+        if (FIELD_EX8(s->config, CONFIG, ONE_SHOT)) {
+            s->config = FIELD_DP8(s->config, CONFIG, ONE_SHOT, 0);
         } else {
             return;
         }
     }
 
-    if (s->config >> 1 & 1) {
+    if (FIELD_EX8(s->config, CONFIG, THERMOSTAT_MODE)) {
         /*
          * TM == 1 : Interrupt mode. We signal Alert when the
          * temperature rises above T_high, and expect the guest to clear
@@ -120,7 +128,7 @@ static void tmp105_read(TMP105State *s)
 {
     s->len = 0;
 
-    if ((s->config >> 1) & 1) {                                 /* TM */
+    if (FIELD_EX8(s->config, CONFIG, THERMOSTAT_MODE)) {
         s->alarm = 0;
         tmp105_interrupt_update(s);
     }
@@ -129,7 +137,7 @@ static void tmp105_read(TMP105State *s)
     case TMP105_REG_TEMPERATURE:
         s->buf[s->len++] = (((uint16_t) s->temperature) >> 8);
         s->buf[s->len++] = (((uint16_t) s->temperature) >> 0) &
-                (0xf0 << ((~s->config >> 5) & 3));              /* R */
+                (0xf0 << (FIELD_EX8(~s->config, CONFIG, CONVERTER_RESOLUTION)));
         break;
 
     case TMP105_REG_CONFIG:
@@ -155,11 +163,11 @@ static void tmp105_write(TMP105State *s)
         break;
 
     case TMP105_REG_CONFIG:
-        if (s->buf[0] & ~s->config & (1 << 0)) {                /* SD */
+        if (FIELD_EX8(s->buf[0] & ~s->config, CONFIG, SHUTDOWN_MODE)) {
             printf("%s: TMP105 shutdown\n", __func__);
         }
         s->config = s->buf[0];
-        s->faults = tmp105_faultq[(s->config >> 3) & 3];        /* F */
+        s->faults = tmp105_faultq[FIELD_EX8(s->config, CONFIG, FAULT_QUEUE)];
         tmp105_alarm_update(s);
         break;
 
@@ -219,7 +227,7 @@ static int tmp105_post_load(void *opaque, int version_id)
 {
     TMP105State *s = opaque;
 
-    s->faults = tmp105_faultq[(s->config >> 3) & 3];            /* F */
+    s->faults = tmp105_faultq[FIELD_EX8(s->config, CONFIG, FAULT_QUEUE)];
 
     tmp105_interrupt_update(s);
     return 0;
@@ -277,7 +285,7 @@ static void tmp105_reset(I2CSlave *i2c)
     s->temperature = 0;
     s->pointer = 0;
     s->config = 0;
-    s->faults = tmp105_faultq[(s->config >> 3) & 3];
+    s->faults = tmp105_faultq[FIELD_EX8(s->config, CONFIG, FAULT_QUEUE)];
     s->alarm = 0;
     s->detect_falling = false;
 
-- 
2.45.2

[PATCH v2 3/5] hw/sensor/tmp105: Pass 'oneshot' argument to tmp105_alarm_update()

[Philippe Mathieu-Daudé]

The next commit will clear the ONE_SHOT bit in the WRITE
path (to keep the READ path trivial). As a preliminary step,
pass the 'oneshot' value as argument to tmp105_alarm_update().
No logical change intended.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/sensor/tmp105.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/sensor/tmp105.c b/hw/sensor/tmp105.c
index 150d09b278..6740200aea 100644
--- a/hw/sensor/tmp105.c
+++ b/hw/sensor/tmp105.c
@@ -40,10 +40,10 @@ static void tmp105_interrupt_update(TMP105State *s)
     qemu_set_irq(s->pin, s->alarm ^ FIELD_EX8(~s->config, CONFIG, POLARITY));
 }
 
-static void tmp105_alarm_update(TMP105State *s)
+static void tmp105_alarm_update(TMP105State *s, bool one_shot)
 {
     if (FIELD_EX8(s->config, CONFIG, SHUTDOWN_MODE)) {
-        if (FIELD_EX8(s->config, CONFIG, ONE_SHOT)) {
+        if (one_shot) {
             s->config = FIELD_DP8(s->config, CONFIG, ONE_SHOT, 0);
         } else {
             return;
@@ -119,7 +119,7 @@ static void tmp105_set_temperature(Object *obj, Visitor *v, const char *name,
 
     s->temperature = (int16_t) (temp * 256 / 1000);
 
-    tmp105_alarm_update(s);
+    tmp105_alarm_update(s, false);
 }
 
 static const int tmp105_faultq[4] = { 1, 2, 4, 6 };
@@ -168,7 +168,7 @@ static void tmp105_write(TMP105State *s)
         }
         s->config = s->buf[0];
         s->faults = tmp105_faultq[FIELD_EX8(s->config, CONFIG, FAULT_QUEUE)];
-        tmp105_alarm_update(s);
+        tmp105_alarm_update(s, FIELD_EX8(s->buf[0], CONFIG, ONE_SHOT));
         break;
 
     case TMP105_REG_T_LOW:
@@ -177,7 +177,7 @@ static void tmp105_write(TMP105State *s)
             s->limit[s->pointer & 1] = (int16_t)
                     ((((uint16_t) s->buf[0]) << 8) | s->buf[1]);
         }
-        tmp105_alarm_update(s);
+        tmp105_alarm_update(s, false);
         break;
     }
 }
-- 
2.45.2

[PATCH v2 4/5] hw/sensor/tmp105: OS (one-shot) bit in config register always returns 0

[Philippe Mathieu-Daudé]

Per datasheet, "ONE-SHOT (OS)", the OS bit always returns 0 when reading
the configuration register.

Clear the ONE_SHOT bit in the WRITE path. Now than the READ path is
simpler, we can also simplify tmp105_alarm_update().

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/sensor/tmp105.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/hw/sensor/tmp105.c b/hw/sensor/tmp105.c
index 6740200aea..f5101af919 100644
--- a/hw/sensor/tmp105.c
+++ b/hw/sensor/tmp105.c
@@ -42,12 +42,8 @@ static void tmp105_interrupt_update(TMP105State *s)
 
 static void tmp105_alarm_update(TMP105State *s, bool one_shot)
 {
-    if (FIELD_EX8(s->config, CONFIG, SHUTDOWN_MODE)) {
-        if (one_shot) {
-            s->config = FIELD_DP8(s->config, CONFIG, ONE_SHOT, 0);
-        } else {
-            return;
-        }
+    if (FIELD_EX8(s->config, CONFIG, SHUTDOWN_MODE) && !one_shot) {
+        return;
     }
 
     if (FIELD_EX8(s->config, CONFIG, THERMOSTAT_MODE)) {
@@ -166,7 +162,7 @@ static void tmp105_write(TMP105State *s)
         if (FIELD_EX8(s->buf[0] & ~s->config, CONFIG, SHUTDOWN_MODE)) {
             printf("%s: TMP105 shutdown\n", __func__);
         }
-        s->config = s->buf[0];
+        s->config = FIELD_DP8(s->buf[0], CONFIG, ONE_SHOT, 0);
         s->faults = tmp105_faultq[FIELD_EX8(s->config, CONFIG, FAULT_QUEUE)];
         tmp105_alarm_update(s, FIELD_EX8(s->buf[0], CONFIG, ONE_SHOT));
         break;
-- 
2.45.2

[PATCH v2 5/5] hw/sensor/tmp105: Lower 4 bit of limit registers are always 0

[Philippe Mathieu-Daudé]

From: Guenter Roeck <linux@roeck-us.net>

Per datasheet, "HIGH AND LOW LIMIT REGISTERS", the lower 4 bit
of the limit registers are unused and always report 0.
The lower 4 bit should not be used for temperature comparisons,
so mask the unused bits before storing the limits.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/sensor/tmp105.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/sensor/tmp105.c b/hw/sensor/tmp105.c
index f5101af919..9d7b911f59 100644
--- a/hw/sensor/tmp105.c
+++ b/hw/sensor/tmp105.c
@@ -171,7 +171,7 @@ static void tmp105_write(TMP105State *s)
     case TMP105_REG_T_HIGH:
         if (s->len >= 3) {
             s->limit[s->pointer & 1] = (int16_t)
-                    ((((uint16_t) s->buf[0]) << 8) | s->buf[1]);
+                    ((((uint16_t) s->buf[0]) << 8) | (s->buf[1] & 0xf0));
         }
         tmp105_alarm_update(s, false);
         break;
-- 
2.45.2

Re: [PATCH v2 2/5] hw/sensor/tmp105: Use registerfields API

[Guenter Roeck]

On Fri, Sep 06, 2024 at 05:49:08PM +0200, Philippe Mathieu-Daudé wrote:
> To improve readability, use the registerfields API.
> Define the register bits with FIELD(), and use the
> FIELD_EX8() and FIELD_DP8() macros. Remove the
> abbreviations in comments.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

Re: [PATCH v2 3/5] hw/sensor/tmp105: Pass 'oneshot' argument to tmp105_alarm_update()

[Guenter Roeck]

On Fri, Sep 06, 2024 at 05:49:09PM +0200, Philippe Mathieu-Daudé wrote:
> The next commit will clear the ONE_SHOT bit in the WRITE
> path (to keep the READ path trivial). As a preliminary step,
> pass the 'oneshot' value as argument to tmp105_alarm_update().
> No logical change intended.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

Re: [PATCH v2 1/5] hw/sensor/tmp105: Coding style fixes

[Philippe Mathieu-Daudé]

On 6/9/24 17:49, Philippe Mathieu-Daudé wrote:
> From: Guenter Roeck <linux@roeck-us.net>
> 
> Coding style asks for no space between variable and "++". The next patch
> in this series will change one of those assignments. Instead of changing
> just one with that patch, change all of them for consistency.
> 
> While at it, also fix other coding style problems reported by checkpatch.
> 
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>

Reviewed-by: Cédric Le Goater <clg@kaod.org>

> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/sensor/tmp105.c | 44 ++++++++++++++++++++++++--------------------
>   1 file changed, 24 insertions(+), 20 deletions(-)
> 
> diff --git a/hw/sensor/tmp105.c b/hw/sensor/tmp105.c
> index a8730d0b7f..ad97c9684c 100644
> --- a/hw/sensor/tmp105.c
> +++ b/hw/sensor/tmp105.c
> @@ -29,16 +29,17 @@
>   
>   static void tmp105_interrupt_update(TMP105State *s)
>   {
> -    qemu_set_irq(s->pin, s->alarm ^ ((~s->config >> 2) & 1));	/* POL */
> +    qemu_set_irq(s->pin, s->alarm ^ ((~s->config >> 2) & 1));   /* POL */
>   }
>   
>   static void tmp105_alarm_update(TMP105State *s)
>   {
> -    if ((s->config >> 0) & 1) {					/* SD */
> -        if ((s->config >> 7) & 1)				/* OS */
> -            s->config &= ~(1 << 7);				/* OS */
> -        else
> +    if ((s->config >> 0) & 1) {                                 /* SD */
> +        if ((s->config >> 7) & 1) {                             /* OS */
> +            s->config &= ~(1 << 7);                             /* OS */
> +        } else {
>               return;
> +        }
>       }
>   
>       if (s->config >> 1 & 1) {
> @@ -89,7 +90,8 @@ static void tmp105_get_temperature(Object *obj, Visitor *v, const char *name,
>       visit_type_int(v, name, &value, errp);
>   }
>   
> -/* Units are 0.001 centigrades relative to 0 C.  s->temperature is 8.8
> +/*
> + * Units are 0.001 centigrades relative to 0 C.  s->temperature is 8.8
>    * fixed point, so units are 1/256 centigrades.  A simple ratio will do.
>    */
>   static void tmp105_set_temperature(Object *obj, Visitor *v, const char *name,
> @@ -118,30 +120,30 @@ static void tmp105_read(TMP105State *s)
>   {
>       s->len = 0;
>   
> -    if ((s->config >> 1) & 1) {					/* TM */
> +    if ((s->config >> 1) & 1) {                                 /* TM */
>           s->alarm = 0;
>           tmp105_interrupt_update(s);
>       }
>   
>       switch (s->pointer & 3) {
>       case TMP105_REG_TEMPERATURE:
> -        s->buf[s->len ++] = (((uint16_t) s->temperature) >> 8);
> -        s->buf[s->len ++] = (((uint16_t) s->temperature) >> 0) &
> -                (0xf0 << ((~s->config >> 5) & 3));		/* R */
> +        s->buf[s->len++] = (((uint16_t) s->temperature) >> 8);
> +        s->buf[s->len++] = (((uint16_t) s->temperature) >> 0) &
> +                (0xf0 << ((~s->config >> 5) & 3));              /* R */
>           break;
>   
>       case TMP105_REG_CONFIG:
> -        s->buf[s->len ++] = s->config;
> +        s->buf[s->len++] = s->config;
>           break;
>   
>       case TMP105_REG_T_LOW:
> -        s->buf[s->len ++] = ((uint16_t) s->limit[0]) >> 8;
> -        s->buf[s->len ++] = ((uint16_t) s->limit[0]) >> 0;
> +        s->buf[s->len++] = ((uint16_t) s->limit[0]) >> 8;
> +        s->buf[s->len++] = ((uint16_t) s->limit[0]) >> 0;
>           break;
>   
>       case TMP105_REG_T_HIGH:
> -        s->buf[s->len ++] = ((uint16_t) s->limit[1]) >> 8;
> -        s->buf[s->len ++] = ((uint16_t) s->limit[1]) >> 0;
> +        s->buf[s->len++] = ((uint16_t) s->limit[1]) >> 8;
> +        s->buf[s->len++] = ((uint16_t) s->limit[1]) >> 0;
>           break;
>       }
>   }
> @@ -153,18 +155,20 @@ static void tmp105_write(TMP105State *s)
>           break;
>   
>       case TMP105_REG_CONFIG:
> -        if (s->buf[0] & ~s->config & (1 << 0))			/* SD */
> +        if (s->buf[0] & ~s->config & (1 << 0)) {                /* SD */
>               printf("%s: TMP105 shutdown\n", __func__);
> +        }
>           s->config = s->buf[0];
> -        s->faults = tmp105_faultq[(s->config >> 3) & 3];	/* F */
> +        s->faults = tmp105_faultq[(s->config >> 3) & 3];        /* F */
>           tmp105_alarm_update(s);
>           break;
>   
>       case TMP105_REG_T_LOW:
>       case TMP105_REG_T_HIGH:
> -        if (s->len >= 3)
> +        if (s->len >= 3) {
>               s->limit[s->pointer & 1] = (int16_t)
>                       ((((uint16_t) s->buf[0]) << 8) | s->buf[1]);
> +        }
>           tmp105_alarm_update(s);
>           break;
>       }
> @@ -175,7 +179,7 @@ static uint8_t tmp105_rx(I2CSlave *i2c)
>       TMP105State *s = TMP105(i2c);
>   
>       if (s->len < 2) {
> -        return s->buf[s->len ++];
> +        return s->buf[s->len++];
>       } else {
>           return 0xff;
>       }
> @@ -215,7 +219,7 @@ static int tmp105_post_load(void *opaque, int version_id)
>   {
>       TMP105State *s = opaque;
>   
> -    s->faults = tmp105_faultq[(s->config >> 3) & 3];		/* F */
> +    s->faults = tmp105_faultq[(s->config >> 3) & 3];            /* F */
>   
>       tmp105_interrupt_update(s);
>       return 0;

Re: [PATCH v2 5/5] hw/sensor/tmp105: Lower 4 bit of limit registers are always 0

[Philippe Mathieu-Daudé]

On 6/9/24 17:49, Philippe Mathieu-Daudé wrote:
> From: Guenter Roeck <linux@roeck-us.net>
> 
> Per datasheet, "HIGH AND LOW LIMIT REGISTERS", the lower 4 bit
> of the limit registers are unused and always report 0.
> The lower 4 bit should not be used for temperature comparisons,
> so mask the unused bits before storing the limits.
> 
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Cédric Le Goater <clg@kaod.org>

> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/sensor/tmp105.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/sensor/tmp105.c b/hw/sensor/tmp105.c
> index f5101af919..9d7b911f59 100644
> --- a/hw/sensor/tmp105.c
> +++ b/hw/sensor/tmp105.c
> @@ -171,7 +171,7 @@ static void tmp105_write(TMP105State *s)
>       case TMP105_REG_T_HIGH:
>           if (s->len >= 3) {
>               s->limit[s->pointer & 1] = (int16_t)
> -                    ((((uint16_t) s->buf[0]) << 8) | s->buf[1]);
> +                    ((((uint16_t) s->buf[0]) << 8) | (s->buf[1] & 0xf0));
>           }
>           tmp105_alarm_update(s, false);
>           break;

Re: [PATCH v2 0/5] tmp105: Improvements and fixes

[Philippe Mathieu-Daudé]

Hi Cédric,

On 6/9/24 17:49, Philippe Mathieu-Daudé wrote:
> Respin of Guenter fixes with:
> - Use registerfields API
> - Clear OS bit in WRITE path

Since our mails crossed (you reviewed v1 while I was
posting v2), do you mind having another look at this
v2? At least patch #4 isn't yet reviewed.

Thanks,

Phil.

> Supersedes: <20240906132912.3826089-1-linux@roeck-us.net>
> 
> Guenter Roeck (2):
>    hw/sensor/tmp105: Coding style fixes
>    hw/sensor/tmp105: Lower 4 bit of limit registers are always 0
> 
> Philippe Mathieu-Daudé (3):
>    hw/sensor/tmp105: Use registerfields API
>    hw/sensor/tmp105: Pass 'oneshot' argument to tmp105_alarm_update()
>    hw/sensor/tmp105: OS (one-shot) bit in config register always returns
>      0
> 
>   hw/sensor/tmp105.c | 66 ++++++++++++++++++++++++++--------------------
>   1 file changed, 37 insertions(+), 29 deletions(-)
> 

Re: [PATCH v2 4/5] hw/sensor/tmp105: OS (one-shot) bit in config register always returns 0

[Cédric Le Goater]

On 9/6/24 17:49, Philippe Mathieu-Daudé wrote:
> Per datasheet, "ONE-SHOT (OS)", the OS bit always returns 0 when reading
> the configuration register.
> 
> Clear the ONE_SHOT bit in the WRITE path. Now than the READ path is
> simpler, we can also simplify tmp105_alarm_update().
> 
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>


Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.


> ---
>   hw/sensor/tmp105.c | 10 +++-------
>   1 file changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/sensor/tmp105.c b/hw/sensor/tmp105.c
> index 6740200aea..f5101af919 100644
> --- a/hw/sensor/tmp105.c
> +++ b/hw/sensor/tmp105.c
> @@ -42,12 +42,8 @@ static void tmp105_interrupt_update(TMP105State *s)
>   
>   static void tmp105_alarm_update(TMP105State *s, bool one_shot)
>   {
> -    if (FIELD_EX8(s->config, CONFIG, SHUTDOWN_MODE)) {
> -        if (one_shot) {
> -            s->config = FIELD_DP8(s->config, CONFIG, ONE_SHOT, 0);
> -        } else {
> -            return;
> -        }
> +    if (FIELD_EX8(s->config, CONFIG, SHUTDOWN_MODE) && !one_shot) {
> +        return;
>       }
>   
>       if (FIELD_EX8(s->config, CONFIG, THERMOSTAT_MODE)) {
> @@ -166,7 +162,7 @@ static void tmp105_write(TMP105State *s)
>           if (FIELD_EX8(s->buf[0] & ~s->config, CONFIG, SHUTDOWN_MODE)) {
>               printf("%s: TMP105 shutdown\n", __func__);
>           }
> -        s->config = s->buf[0];
> +        s->config = FIELD_DP8(s->buf[0], CONFIG, ONE_SHOT, 0);
>           s->faults = tmp105_faultq[FIELD_EX8(s->config, CONFIG, FAULT_QUEUE)];
>           tmp105_alarm_update(s, FIELD_EX8(s->buf[0], CONFIG, ONE_SHOT));
>           break;

Re: [PATCH v2 0/5] tmp105: Improvements and fixes

[Philippe Mathieu-Daudé]

On 6/9/24 17:49, Philippe Mathieu-Daudé wrote:
> Respin of Guenter fixes with:
> - Use registerfields API
> - Clear OS bit in WRITE path
> 
> Supersedes: <20240906132912.3826089-1-linux@roeck-us.net>
> 
> Guenter Roeck (2):
>    hw/sensor/tmp105: Coding style fixes
>    hw/sensor/tmp105: Lower 4 bit of limit registers are always 0
> 
> Philippe Mathieu-Daudé (3):
>    hw/sensor/tmp105: Use registerfields API
>    hw/sensor/tmp105: Pass 'oneshot' argument to tmp105_alarm_update()
>    hw/sensor/tmp105: OS (one-shot) bit in config register always returns
>      0

Series queued, thanks!