* [PATCH v2 1/5] hw/sensor/tmp105: Coding style fixes
2024-09-06 15:49 [PATCH v2 0/5] tmp105: Improvements and fixes Philippe Mathieu-Daudé
@ 2024-09-06 15:49 ` Philippe Mathieu-Daudé
2024-09-07 4:11 ` Philippe Mathieu-Daudé
2024-09-06 15:49 ` [PATCH v2 2/5] hw/sensor/tmp105: Use registerfields API Philippe Mathieu-Daudé
` (5 subsequent siblings)
6 siblings, 1 reply; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-09-06 15:49 UTC (permalink / raw)
To: Guenter Roeck, qemu-devel
Cc: Philippe Mathieu-Daudé, Cédric Le Goater
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
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v2 1/5] hw/sensor/tmp105: Coding style fixes
2024-09-06 15:49 ` [PATCH v2 1/5] hw/sensor/tmp105: Coding style fixes Philippe Mathieu-Daudé
@ 2024-09-07 4:11 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-09-07 4:11 UTC (permalink / raw)
To: Guenter Roeck, qemu-devel; +Cc: Cédric Le Goater
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;
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 2/5] hw/sensor/tmp105: Use registerfields API
2024-09-06 15:49 [PATCH v2 0/5] tmp105: Improvements and fixes Philippe Mathieu-Daudé
2024-09-06 15:49 ` [PATCH v2 1/5] hw/sensor/tmp105: Coding style fixes Philippe Mathieu-Daudé
@ 2024-09-06 15:49 ` Philippe Mathieu-Daudé
2024-09-06 18:50 ` Guenter Roeck
2024-09-06 15:49 ` [PATCH v2 3/5] hw/sensor/tmp105: Pass 'oneshot' argument to tmp105_alarm_update() Philippe Mathieu-Daudé
` (4 subsequent siblings)
6 siblings, 1 reply; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-09-06 15:49 UTC (permalink / raw)
To: Guenter Roeck, qemu-devel
Cc: Philippe Mathieu-Daudé, Cédric Le Goater
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
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v2 3/5] hw/sensor/tmp105: Pass 'oneshot' argument to tmp105_alarm_update()
2024-09-06 15:49 [PATCH v2 0/5] tmp105: Improvements and fixes Philippe Mathieu-Daudé
2024-09-06 15:49 ` [PATCH v2 1/5] hw/sensor/tmp105: Coding style fixes Philippe Mathieu-Daudé
2024-09-06 15:49 ` [PATCH v2 2/5] hw/sensor/tmp105: Use registerfields API Philippe Mathieu-Daudé
@ 2024-09-06 15:49 ` Philippe Mathieu-Daudé
2024-09-06 18:51 ` Guenter Roeck
2024-09-06 15:49 ` [PATCH v2 4/5] hw/sensor/tmp105: OS (one-shot) bit in config register always returns 0 Philippe Mathieu-Daudé
` (3 subsequent siblings)
6 siblings, 1 reply; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-09-06 15:49 UTC (permalink / raw)
To: Guenter Roeck, qemu-devel
Cc: Philippe Mathieu-Daudé, Cédric Le Goater
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
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v2 4/5] hw/sensor/tmp105: OS (one-shot) bit in config register always returns 0
2024-09-06 15:49 [PATCH v2 0/5] tmp105: Improvements and fixes Philippe Mathieu-Daudé
` (2 preceding siblings ...)
2024-09-06 15:49 ` [PATCH v2 3/5] hw/sensor/tmp105: Pass 'oneshot' argument to tmp105_alarm_update() Philippe Mathieu-Daudé
@ 2024-09-06 15:49 ` Philippe Mathieu-Daudé
2024-09-11 17:32 ` Cédric Le Goater
2024-09-06 15:49 ` [PATCH v2 5/5] hw/sensor/tmp105: Lower 4 bit of limit registers are always 0 Philippe Mathieu-Daudé
` (2 subsequent siblings)
6 siblings, 1 reply; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-09-06 15:49 UTC (permalink / raw)
To: Guenter Roeck, qemu-devel
Cc: Philippe Mathieu-Daudé, Cédric Le Goater
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
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v2 4/5] hw/sensor/tmp105: OS (one-shot) bit in config register always returns 0
2024-09-06 15:49 ` [PATCH v2 4/5] hw/sensor/tmp105: OS (one-shot) bit in config register always returns 0 Philippe Mathieu-Daudé
@ 2024-09-11 17:32 ` Cédric Le Goater
0 siblings, 0 replies; 13+ messages in thread
From: Cédric Le Goater @ 2024-09-11 17:32 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, Guenter Roeck, qemu-devel
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;
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 5/5] hw/sensor/tmp105: Lower 4 bit of limit registers are always 0
2024-09-06 15:49 [PATCH v2 0/5] tmp105: Improvements and fixes Philippe Mathieu-Daudé
` (3 preceding siblings ...)
2024-09-06 15:49 ` [PATCH v2 4/5] hw/sensor/tmp105: OS (one-shot) bit in config register always returns 0 Philippe Mathieu-Daudé
@ 2024-09-06 15:49 ` Philippe Mathieu-Daudé
2024-09-07 4:12 ` Philippe Mathieu-Daudé
2024-09-09 13:27 ` [PATCH v2 0/5] tmp105: Improvements and fixes Philippe Mathieu-Daudé
2024-09-12 6:50 ` Philippe Mathieu-Daudé
6 siblings, 1 reply; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-09-06 15:49 UTC (permalink / raw)
To: Guenter Roeck, qemu-devel
Cc: Philippe Mathieu-Daudé, Cédric Le Goater
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
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v2 5/5] hw/sensor/tmp105: Lower 4 bit of limit registers are always 0
2024-09-06 15:49 ` [PATCH v2 5/5] hw/sensor/tmp105: Lower 4 bit of limit registers are always 0 Philippe Mathieu-Daudé
@ 2024-09-07 4:12 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-09-07 4:12 UTC (permalink / raw)
To: Guenter Roeck, qemu-devel; +Cc: Cédric Le Goater
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;
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 0/5] tmp105: Improvements and fixes
2024-09-06 15:49 [PATCH v2 0/5] tmp105: Improvements and fixes Philippe Mathieu-Daudé
` (4 preceding siblings ...)
2024-09-06 15:49 ` [PATCH v2 5/5] hw/sensor/tmp105: Lower 4 bit of limit registers are always 0 Philippe Mathieu-Daudé
@ 2024-09-09 13:27 ` Philippe Mathieu-Daudé
2024-09-12 6:50 ` Philippe Mathieu-Daudé
6 siblings, 0 replies; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-09-09 13:27 UTC (permalink / raw)
To: Guenter Roeck, qemu-devel; +Cc: Cédric Le Goater
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(-)
>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2 0/5] tmp105: Improvements and fixes
2024-09-06 15:49 [PATCH v2 0/5] tmp105: Improvements and fixes Philippe Mathieu-Daudé
` (5 preceding siblings ...)
2024-09-09 13:27 ` [PATCH v2 0/5] tmp105: Improvements and fixes Philippe Mathieu-Daudé
@ 2024-09-12 6:50 ` Philippe Mathieu-Daudé
6 siblings, 0 replies; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-09-12 6:50 UTC (permalink / raw)
To: Guenter Roeck, qemu-devel; +Cc: Cédric Le Goater
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!
^ permalink raw reply [flat|nested] 13+ messages in thread