[PATCH 0/3] tmp105: Improvements and fixes

[PATCH 0/3] tmp105: Improvements and fixes

[Guenter Roeck]

Fix coding style issues as well as a couple of implementation bugs.

----------------------------------------------------------------
Guenter Roeck (3):
      tmp105: Coding style fixes
      tmp105: OS (one-shot) bit in configuration register always returns 0
      tmp105: Lower 4 bit of limit registers are always 0

 hw/sensor/tmp105.c | 42 ++++++++++++++++++++++--------------------
 1 file changed, 22 insertions(+), 20 deletions(-)

[PATCH 1/3] tmp105: Coding style fixes

[Guenter Roeck]

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>
---
 hw/sensor/tmp105.c | 40 +++++++++++++++++++++-------------------
 1 file changed, 21 insertions(+), 19 deletions(-)

diff --git a/hw/sensor/tmp105.c b/hw/sensor/tmp105.c
index a8730d0b7f..77fbf2f8d3 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,10 +155,10 @@ 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;
 
@@ -175,7 +177,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 +217,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 2/3] tmp105: OS (one-shot) bit in configuration register always returns 0

[Guenter Roeck]

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

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 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 77fbf2f8d3..22728798f7 100644
--- a/hw/sensor/tmp105.c
+++ b/hw/sensor/tmp105.c
@@ -133,7 +133,7 @@ static void tmp105_read(TMP105State *s)
         break;
 
     case TMP105_REG_CONFIG:
-        s->buf[s->len++] = s->config;
+        s->buf[s->len++] = s->config & 0x7f;
         break;
 
     case TMP105_REG_T_LOW:
-- 
2.45.2

Re: [PATCH 2/3] tmp105: OS (one-shot) bit in configuration register always returns 0

[Philippe Mathieu-Daudé]

Hi Guenter,

On 6/9/24 15:29, Guenter Roeck wrote:
> Per datasheet, "ONE-SHOT (OS)", the OS bit always returns 0 when reading
> the configuration register.
> 
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
>   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 77fbf2f8d3..22728798f7 100644
> --- a/hw/sensor/tmp105.c
> +++ b/hw/sensor/tmp105.c
> @@ -133,7 +133,7 @@ static void tmp105_read(TMP105State *s)
>           break;
>   
>       case TMP105_REG_CONFIG:
> -        s->buf[s->len++] = s->config;
> +        s->buf[s->len++] = s->config & 0x7f;

LGTM but I'll post an alternative v2 clearing the bit in
the WRITE path.

>           break;
>   
>       case TMP105_REG_T_LOW:

[PATCH 3/3] tmp105: Lower 4 bit of limit registers are always 0

[Guenter Roeck]

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>
---
 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 22728798f7..5fd30f8d50 100644
--- a/hw/sensor/tmp105.c
+++ b/hw/sensor/tmp105.c
@@ -166,7 +166,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);
         break;
     }
-- 
2.45.2

Re: [PATCH 0/3] tmp105: Improvements and fixes

[Cédric Le Goater]

On 9/6/24 15:29, Guenter Roeck wrote:
> Fix coding style issues as well as a couple of implementation bugs.
> 
> ----------------------------------------------------------------
> Guenter Roeck (3):
>        tmp105: Coding style fixes
>        tmp105: OS (one-shot) bit in configuration register always returns 0
>        tmp105: Lower 4 bit of limit registers are always 0
> 
>   hw/sensor/tmp105.c | 42 ++++++++++++++++++++++--------------------
>   1 file changed, 22 insertions(+), 20 deletions(-)


For the series,

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

Thanks,

C.