On 11/10/20 4:00 PM, Peter Maydell wrote:
> The TMP105 datasheet says that in Interrupt Mode (when TM==1) the device
> signals an alert when the temperature equals or exceeds the T_high value and
> then remains high until a device register is read or the device responds to
> the SMBUS Alert Response address, or the device is put into Shutdown Mode.
> Thereafter the Alert pin will only be re-signalled when temperature falls
> below T_low; alert can then be cleared in the same set of ways, and the
> device returns to its initial "alert when temperature goes above T_high"
> mode. (If this textual description is confusing, see figure 3 in the
> TI datasheet at https://www.ti.com/lit/gpn/tmp105 .)
>
> We were misimplementing this as a simple "always alert if temperature is
> above T_high or below T_low" condition, which gives a spurious alert on
> startup if using the "T_high = 80 degrees C, T_low = 75 degrees C" reset
> limit values.
>
> Implement the correct (hysteresis) behaviour by tracking whether we
> are currently looking for the temperature to rise over T_high or
> for it to fall below T_low. Our implementation of the comparator
> mode (TM==0) wasn't wrong, but rephrase it to match the way that
> interrupt mode is now handled for clarity.
>
> Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>
LGTM,
Reviewed-by: Cédric Le Goater <c...@kaod.org>
Thanks,
C.
> ---
> hw/misc/tmp105.c | 70 +++++++++++++++++++++++++++++++++++++++++-------
> hw/misc/tmp105.h | 7 +++++
> 2 files changed, 68 insertions(+), 9 deletions(-)
>
> diff --git a/hw/misc/tmp105.c b/hw/misc/tmp105.c
> index 0a4aad4854..d299d9b21b 100644
> --- a/hw/misc/tmp105.c
> +++ b/hw/misc/tmp105.c
> @@ -41,16 +41,40 @@ static void tmp105_alarm_update(TMP105State *s)
> return;
> }
>
> - if ((s->config >> 1) & 1) { /* TM */
> - if (s->temperature >= s->limit[1])
> - s->alarm = 1;
> - else if (s->temperature < s->limit[0])
> - s->alarm = 1;
> + if (s->config >> 1 & 1) {
> + /*
> + * TM == 1 : Interrupt mode. We signal Alert when the
> + * temperature rises above T_high, and expect the guest to clear
> + * it (eg by reading a device register).
> + */
> + if (s->detect_falling) {
> + if (s->temperature < s->limit[0]) {
> + s->alarm = 1;
> + s->detect_falling = false;
> + }
> + } else {
> + if (s->temperature >= s->limit[1]) {
> + s->alarm = 1;
> + s->detect_falling = true;
> + }
> + }
> } else {
> - if (s->temperature >= s->limit[1])
> - s->alarm = 1;
> - else if (s->temperature < s->limit[0])
> - s->alarm = 0;
> + /*
> + * TM == 0 : Comparator mode. We signal Alert when the temperature
> + * rises above T_high, and stop signalling it when the temperature
> + * falls below T_low.
> + */
> + if (s->detect_falling) {
> + if (s->temperature < s->limit[0]) {
> + s->alarm = 0;
> + s->detect_falling = false;
> + }
> + } else {
> + if (s->temperature >= s->limit[1]) {
> + s->alarm = 1;
> + s->detect_falling = true;
> + }
> + }
> }
>
> tmp105_interrupt_update(s);
> @@ -197,6 +221,29 @@ static int tmp105_post_load(void *opaque, int version_id)
> return 0;
> }
>
> +static bool detect_falling_needed(void *opaque)
> +{
> + TMP105State *s = opaque;
> +
> + /*
> + * We only need to migrate the detect_falling bool if it's set;
> + * for migration from older machines we assume that it is false
> + * (ie temperature is not out of range).
> + */
> + return s->detect_falling;
> +}
> +
> +static const VMStateDescription vmstate_tmp105_detect_falling = {
> + .name = "TMP105/detect-falling",
> + .version_id = 1,
> + .minimum_version_id = 1,
> + .needed = detect_falling_needed,
> + .fields = (VMStateField[]) {
> + VMSTATE_BOOL(detect_falling, TMP105State),
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> static const VMStateDescription vmstate_tmp105 = {
> .name = "TMP105",
> .version_id = 0,
> @@ -212,6 +259,10 @@ static const VMStateDescription vmstate_tmp105 = {
> VMSTATE_UINT8(alarm, TMP105State),
> VMSTATE_I2C_SLAVE(i2c, TMP105State),
> VMSTATE_END_OF_LIST()
> + },
> + .subsections = (const VMStateDescription*[]) {
> + &vmstate_tmp105_detect_falling,
> + NULL
> }
> };
>
> @@ -224,6 +275,7 @@ static void tmp105_reset(I2CSlave *i2c)
> s->config = 0;
> s->faults = tmp105_faultq[(s->config >> 3) & 3];
> s->alarm = 0;
> + s->detect_falling = false;
>
> s->limit[0] = 0x4b00; /* T_LOW, 75 degrees C */
> s->limit[1] = 0x5000; /* T_HIGH, 80 degrees C */
> diff --git a/hw/misc/tmp105.h b/hw/misc/tmp105.h
> index e5198fce80..7c97071ad7 100644
> --- a/hw/misc/tmp105.h
> +++ b/hw/misc/tmp105.h
> @@ -43,6 +43,13 @@ struct TMP105State {
> int16_t limit[2];
> int faults;
> uint8_t alarm;
> + /*
> + * The TMP105 initially looks for a temperature rising above T_high;
> + * once this is detected, the condition it looks for next is the
> + * temperature falling below T_low. This flag is false when initially
> + * looking for T_high, true when looking for T_low.
> + */
> + bool detect_falling;
> };
>
> #endif
>
