Skip to contentSkip to conversation

thermal: add brcmstb AVS TMON driver

bug report In review Dan Carpenter 6 participants 8 messages Sep 6, 2017 – Sep 14, 2017

Subsystems (3)
Thermal Broadcom BCM7XXX Arm architecture Broadcom stb avs tmon driver
lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

Hello Brian Norris,

The patch 1e21c74eda83: "thermal: add brcmstb AVS TMON driver" from Aug 9, 2017, leads to the following static checker warning:

	drivers/thermal/broadcom/brcmstb_thermal.c:281 brcmstb_set_trips()
	warn: impossible condition '(low > ((~0 >> 1))) => (s32min-s32max > s32max)'

	drivers/thermal/broadcom/brcmstb_thermal.c:290 brcmstb_set_trips()
	warn: impossible condition '(high > ((~0 >> 1))) => (s32min-s32max > s32max)'

drivers/thermal/broadcom/brcmstb_thermal.c
274 static int brcmstb_set_trips(void *data, int low, int high) 275 {
276 struct brcmstb_thermal_priv *priv = data;
277
278 dev_dbg(priv->dev, "set trips %d <--> %d\n", low, high);
279
280 if (low) {
281 if (low > INT_MAX)

                            ^^^^^^^^^^^^^

Never true

282 low = INT_MAX;
283 avs_tmon_set_trip_temp(priv, TMON_TRIP_TYPE_LOW, low);
284 avs_tmon_trip_enable(priv, TMON_TRIP_TYPE_LOW, 1);
285 } else {
286 avs_tmon_trip_enable(priv, TMON_TRIP_TYPE_LOW, 0);
287 }
288

   289          if (high < ULONG_MAX) {
                    ^^^^^^^^^^^^^^^^

Always true

290 if (high > INT_MAX)

                            ^^^^^^^^^^^^^^

Never true

   291                          high = INT_MAX;
   292                  avs_tmon_set_trip_temp(priv, TMON_TRIP_TYPE_HIGH, high);
   293                  avs_tmon_trip_enable(priv, TMON_TRIP_TYPE_HIGH, 1);
   294          } else {
   295                  avs_tmon_trip_enable(priv, TMON_TRIP_TYPE_HIGH, 0);
   296          }
   297  
   298          return 0;
   299  }

regards,
dan carpenter

#1 lore raw
Quoted 1 line · Dan Carpenter

Hello Brian Norris,

Hi Dan! Or Dan's robots.

Quoted 1 line

The patch 1e21c74eda83: "thermal: add brcmstb AVS TMON driver" from

I'll blame Doug or Florian :)

Actually, I'm pretty sure 'low' and 'high' were 'unsigned long' in the tree where I first wrote this driver (before 'set_trips' was even merged upstream). We can probably simplify this now.

But I'll leave that to Doug.

Brian

Quoted 48 lines

Aug 9, 2017, leads to the following static checker warning:

	drivers/thermal/broadcom/brcmstb_thermal.c:281 brcmstb_set_trips()
	warn: impossible condition '(low > ((~0 >> 1))) => (s32min-s32max > s32max)'

	drivers/thermal/broadcom/brcmstb_thermal.c:290 brcmstb_set_trips()
	warn: impossible condition '(high > ((~0 >> 1))) => (s32min-s32max > s32max)'

drivers/thermal/broadcom/brcmstb_thermal.c
274 static int brcmstb_set_trips(void *data, int low, int high)

   275  {
   276          struct brcmstb_thermal_priv *priv = data;
   277  
   278          dev_dbg(priv->dev, "set trips %d <--> %d\n", low, high);
   279  
   280          if (low) {
   281                  if (low > INT_MAX)
                            ^^^^^^^^^^^^^

Never true

   282                          low = INT_MAX;
   283                  avs_tmon_set_trip_temp(priv, TMON_TRIP_TYPE_LOW, low);
   284                  avs_tmon_trip_enable(priv, TMON_TRIP_TYPE_LOW, 1);
   285          } else {
   286                  avs_tmon_trip_enable(priv, TMON_TRIP_TYPE_LOW, 0);
   287          }
   288  
   289          if (high < ULONG_MAX) {
                    ^^^^^^^^^^^^^^^^

Always true

290 if (high > INT_MAX)

                            ^^^^^^^^^^^^^^

Never true

   291                          high = INT_MAX;
   292                  avs_tmon_set_trip_temp(priv, TMON_TRIP_TYPE_HIGH, high);
   293                  avs_tmon_trip_enable(priv, TMON_TRIP_TYPE_HIGH, 1);
   294          } else {
   295                  avs_tmon_trip_enable(priv, TMON_TRIP_TYPE_HIGH, 0);
   296          }
   297  
   298          return 0;
   299  }

regards,
dan carpenter

#2 lore raw
Quoted 5 lines · Brian Norris
Quoted 1 line · Dan Carpenter

Hello Brian Norris,

Hi Dan! Or Dan's robots.

Quoted 7 lines
Quoted 1 line

The patch 1e21c74eda83: "thermal: add brcmstb AVS TMON driver" from

I'll blame Doug or Florian :)

Actually, I'm pretty sure 'low' and 'high' were 'unsigned long' in the tree where I first wrote this driver (before 'set_trips' was even merged upstream). We can probably simplify this now.

That is exactly what happened yes :) and Colin already sent a fix for that:

https://www.mail-archive.com/[email protected]/msg1483884.html

Quoted 53 lines

But I'll leave that to Doug.

Brian

Quoted 48 lines

Aug 9, 2017, leads to the following static checker warning:

	drivers/thermal/broadcom/brcmstb_thermal.c:281 brcmstb_set_trips()
	warn: impossible condition '(low > ((~0 >> 1))) => (s32min-s32max > s32max)'

	drivers/thermal/broadcom/brcmstb_thermal.c:290 brcmstb_set_trips()
	warn: impossible condition '(high > ((~0 >> 1))) => (s32min-s32max > s32max)'

drivers/thermal/broadcom/brcmstb_thermal.c
274 static int brcmstb_set_trips(void *data, int low, int high)

   275  {
   276          struct brcmstb_thermal_priv *priv = data;
   277  
   278          dev_dbg(priv->dev, "set trips %d <--> %d\n", low, high);
   279  
   280          if (low) {
   281                  if (low > INT_MAX)
                            ^^^^^^^^^^^^^

Never true

   282                          low = INT_MAX;
   283                  avs_tmon_set_trip_temp(priv, TMON_TRIP_TYPE_LOW, low);
   284                  avs_tmon_trip_enable(priv, TMON_TRIP_TYPE_LOW, 1);
   285          } else {
   286                  avs_tmon_trip_enable(priv, TMON_TRIP_TYPE_LOW, 0);
   287          }
   288  
   289          if (high < ULONG_MAX) {
                    ^^^^^^^^^^^^^^^^

Always true

290 if (high > INT_MAX)

                            ^^^^^^^^^^^^^^

Never true

   291                          high = INT_MAX;
   292                  avs_tmon_set_trip_temp(priv, TMON_TRIP_TYPE_HIGH, high);
   293                  avs_tmon_trip_enable(priv, TMON_TRIP_TYPE_HIGH, 1);
   294          } else {
   295                  avs_tmon_trip_enable(priv, TMON_TRIP_TYPE_HIGH, 0);
   296          }
   297  
   298          return 0;
   299  }

regards,
dan carpenter

Signature · 1 line
Florian
#3 lore raw

Hi,

Quoted 10 lines · Florian Fainelli
Quoted 9 lines · Brian Norris
Quoted 1 line · Dan Carpenter

Hello Brian Norris,

Hi Dan! Or Dan's robots.

Quoted 1 line

The patch 1e21c74eda83: "thermal: add brcmstb AVS TMON driver" from

I'll blame Doug or Florian :)

Quoted 7 lines
Quoted 3 lines

Actually, I'm pretty sure 'low' and 'high' were 'unsigned long' in the tree where I first wrote this driver (before 'set_trips' was even merged upstream). We can probably simplify this now.

That is exactly what happened yes :) and Colin already sent a fix for that:

https://www.mail-archive.com/[email protected]/msg1483884.html

Colin didn't fix all the problems. See below:

Quoted 25 lines
Quoted 25 lines

But I'll leave that to Doug.

Brian

Quoted 20 lines

Aug 9, 2017, leads to the following static checker warning:

	drivers/thermal/broadcom/brcmstb_thermal.c:281 brcmstb_set_trips()
	warn: impossible condition '(low > ((~0 >> 1))) => (s32min-s32max > s32max)'

	drivers/thermal/broadcom/brcmstb_thermal.c:290 brcmstb_set_trips()
	warn: impossible condition '(high > ((~0 >> 1))) => (s32min-s32max > s32max)'

drivers/thermal/broadcom/brcmstb_thermal.c
274 static int brcmstb_set_trips(void *data, int low, int high)

   275  {
   276          struct brcmstb_thermal_priv *priv = data;
   277  
   278          dev_dbg(priv->dev, "set trips %d <--> %d\n", low, high);
   279  
   280          if (low) {
   281                  if (low > INT_MAX)
                            ^^^^^^^^^^^^^

Never true

Quoted 10 lines
Quoted 10 lines
Quoted 10 lines
   282                          low = INT_MAX;
   283                  avs_tmon_set_trip_temp(priv, TMON_TRIP_TYPE_LOW, low);
   284                  avs_tmon_trip_enable(priv, TMON_TRIP_TYPE_LOW, 1);
   285          } else {
   286                  avs_tmon_trip_enable(priv, TMON_TRIP_TYPE_LOW, 0);
   287          }
   288  
   289          if (high < ULONG_MAX) {
                    ^^^^^^^^^^^^^^^^

Always true

We didn't expect this one ^^^ and Colin didn't fix it. We expected a "max" value to turn the trip point off (in the 'else' branch). But we can never possibly reach the 'else'.

Maybe my expectations are off now (I'm not really that familiar with the current state of the thermal framework), but at any rate, we shouldn't be leaving dead code.

I'll make a quick comment on Colin's patch too, so this doesn't get lost.

Brian

Quoted 21 lines
Quoted 17 lines
Quoted 17 lines

290 if (high > INT_MAX)

                            ^^^^^^^^^^^^^^

Never true

   291                          high = INT_MAX;
   292                  avs_tmon_set_trip_temp(priv, TMON_TRIP_TYPE_HIGH, high);
   293                  avs_tmon_trip_enable(priv, TMON_TRIP_TYPE_HIGH, 1);
   294          } else {
   295                  avs_tmon_trip_enable(priv, TMON_TRIP_TYPE_HIGH, 0);
   296          }
   297  
   298          return 0;
   299  }

regards,
dan carpenter

Signature · 1 line
Florian
#4 lore raw
Quoted 12 lines · Brian Norris

Hi,

Quoted 9 lines · Florian Fainelli
Quoted 8 lines · Brian Norris
Quoted 1 line · Dan Carpenter

Hello Brian Norris,

Hi Dan! Or Dan's robots.

Quoted 1 line

The patch 1e21c74eda83: "thermal: add brcmstb AVS TMON driver" from

I'll blame Doug or Florian :)

I of course have to blame Markus ;)

Quoted 6 lines
Quoted 6 lines
Quoted 4 lines

Actually, I'm pretty sure 'low' and 'high' were 'unsigned long' in the tree where I first wrote this driver (before 'set_trips' was even merged upstream). We can probably simplify this now.

That is exactly what happened yes :) and Colin already sent a fix for

that:

Quoted 12 lines
Quoted 2 lines

https://www.mail-archive.com/[email protected]/msg1483884.html

Colin didn't fix all the problems. See below:

Quoted 7 lines
Quoted 7 lines

But I'll leave that to Doug.

Brian

Quoted 2 lines

Aug 9, 2017, leads to the following static checker warning:

Quoted 14 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
	drivers/thermal/broadcom/brcmstb_thermal.c:281 brcmstb_set_trips()
	warn: impossible condition '(low > ((~0 >> 1))) => (s32min-s32max

s32max)'

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
	drivers/thermal/broadcom/brcmstb_thermal.c:290 brcmstb_set_trips()
	warn: impossible condition '(high > ((~0 >> 1))) => (s32min-s32max

s32max)'

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines

drivers/thermal/broadcom/brcmstb_thermal.c
274 static int brcmstb_set_trips(void *data, int low, int high)

   275  {
   276          struct brcmstb_thermal_priv *priv = data;
   277
   278          dev_dbg(priv->dev, "set trips %d <--> %d\n", low,

high);

Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
   279
   280          if (low) {
   281                  if (low > INT_MAX)
                            ^^^^^^^^^^^^^

Never true

   282                          low = INT_MAX;
   283                  avs_tmon_set_trip_temp(priv,

TMON_TRIP_TYPE_LOW, low);

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

284 avs_tmon_trip_enable(priv,

TMON_TRIP_TYPE_LOW, 1);

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
   285          } else {
   286                  avs_tmon_trip_enable(priv,

TMON_TRIP_TYPE_LOW, 0);

Quoted 9 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
   287          }
   288
   289          if (high < ULONG_MAX) {
                    ^^^^^^^^^^^^^^^^

Always true

We didn't expect this one ^^^ and Colin didn't fix it. We expected a "max" value to turn the trip point off (in the 'else' branch). But we can never possibly reach the 'else'.

This actually appears to be the heart of the matter.

As stated above, the set_trips patch API that this logic was developed for was different from the version of the patch ultimately merged upstream and the brcmstb_thermal driver logic was not corrected before being submitted upstream.

The key change appears to be as follows:

Old API - unsigned long ints and "If there is no trip point above or below the current temperature, the passed trip temperature will be ULONG_MAX or 0 respectively.

New API - ints and "If there is no trip point above or below the current temperature, the passed trip temperature will be -INT_MAX or INT_MAX respectively."

So there are latent logic errors in addition to the "dead code" errors flagged here.

Quoted 16 lines

Maybe my expectations are off now (I'm not really that familiar with the current state of the thermal framework), but at any rate, we shouldn't be leaving dead code.

I'll make a quick comment on Colin's patch too, so this doesn't get lost.

Brian

Quoted 6 lines
Quoted 6 lines
Quoted 6 lines

290 if (high > INT_MAX)

                            ^^^^^^^^^^^^^^

Never true

   291                          high = INT_MAX;
   292                  avs_tmon_set_trip_temp(priv,

TMON_TRIP_TYPE_HIGH, high);

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

293 avs_tmon_trip_enable(priv,

TMON_TRIP_TYPE_HIGH, 1);

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
   294          } else {
   295                  avs_tmon_trip_enable(priv,

TMON_TRIP_TYPE_HIGH, 0);

Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
   296          }
   297
   298          return 0;
   299  }
Quoted 6 lines
Quoted 6 lines
Quoted 2 lines
Quoted 2 lines

regards,
dan carpenter

--
Florian

I'll leave it to Markus to correct the logic, unless he wants to punt it back to me :).

Thanks!
Doug

#5 lore raw
Quoted 8 lines · Doug Berger
Quoted 7 lines · Brian Norris

Hi,

Quoted 4 lines · Florian Fainelli
Quoted 3 lines · Brian Norris
Quoted 1 line · Dan Carpenter

Hello Brian Norris,

Quoted 7 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines

Hi Dan! Or Dan's robots.

Quoted 1 line

The patch 1e21c74eda83: "thermal: add brcmstb AVS TMON driver" from

I'll blame Doug or Florian :)

I of course have to blame Markus ;)

Tried fleeing to the other side of the continent, but it caught up with me anyway. ;-)

Quoted 109 lines
Quoted 6 lines
Quoted 6 lines
Quoted 4 lines

Actually, I'm pretty sure 'low' and 'high' were 'unsigned long' in the tree where I first wrote this driver (before 'set_trips' was even merged upstream). We can probably simplify this now.

That is exactly what happened yes :) and Colin already sent a fix for

that:

Quoted 12 lines
Quoted 2 lines

https://www.mail-archive.com/[email protected]/msg1483884.html

Colin didn't fix all the problems. See below:

Quoted 7 lines
Quoted 7 lines

But I'll leave that to Doug.

Brian

Quoted 2 lines

Aug 9, 2017, leads to the following static checker warning:

Quoted 14 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
   drivers/thermal/broadcom/brcmstb_thermal.c:281 brcmstb_set_trips()
   warn: impossible condition '(low > ((~0 >> 1))) => (s32min-s32max

s32max)'

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
   drivers/thermal/broadcom/brcmstb_thermal.c:290 brcmstb_set_trips()
   warn: impossible condition '(high > ((~0 >> 1))) => (s32min-s32max

s32max)'

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines

drivers/thermal/broadcom/brcmstb_thermal.c
274 static int brcmstb_set_trips(void *data, int low, int high)

   275  {
   276          struct brcmstb_thermal_priv *priv = data;
   277
   278          dev_dbg(priv->dev, "set trips %d <--> %d\n", low,

high);

Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
   279
   280          if (low) {
   281                  if (low > INT_MAX)
                            ^^^^^^^^^^^^^

Never true

   282                          low = INT_MAX;
   283                  avs_tmon_set_trip_temp(priv,

TMON_TRIP_TYPE_LOW, low);

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

284 avs_tmon_trip_enable(priv,

TMON_TRIP_TYPE_LOW, 1);

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
   285          } else {
   286                  avs_tmon_trip_enable(priv,

TMON_TRIP_TYPE_LOW, 0);

Quoted 9 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
   287          }
   288
   289          if (high < ULONG_MAX) {
                    ^^^^^^^^^^^^^^^^

Always true

We didn't expect this one ^^^ and Colin didn't fix it. We expected a "max" value to turn the trip point off (in the 'else' branch). But we can never possibly reach the 'else'.

This actually appears to be the heart of the matter.

As stated above, the set_trips patch API that this logic was developed for was different from the version of the patch ultimately merged upstream and the brcmstb_thermal driver logic was not corrected before being submitted upstream.

The key change appears to be as follows:

Old API - unsigned long ints and "If there is no trip point above or below the current temperature, the passed trip temperature will be ULONG_MAX or 0 respectively.

New API - ints and "If there is no trip point above or below the current temperature, the passed trip temperature will be -INT_MAX or INT_MAX respectively."

So there are latent logic errors in addition to the "dead code" errors flagged here.

Quoted 16 lines

Maybe my expectations are off now (I'm not really that familiar with the current state of the thermal framework), but at any rate, we shouldn't be leaving dead code.

I'll make a quick comment on Colin's patch too, so this doesn't get lost.

Brian

Quoted 6 lines
Quoted 6 lines
Quoted 6 lines

290 if (high > INT_MAX)

                            ^^^^^^^^^^^^^^

Never true

   291                          high = INT_MAX;
   292                  avs_tmon_set_trip_temp(priv,

TMON_TRIP_TYPE_HIGH, high);

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

293 avs_tmon_trip_enable(priv,

TMON_TRIP_TYPE_HIGH, 1);

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
   294          } else {
   295                  avs_tmon_trip_enable(priv,

TMON_TRIP_TYPE_HIGH, 0);

Quoted 10 lines
Quoted 10 lines
Quoted 8 lines
Quoted 8 lines
   296          }
   297
   298          return 0;
   299  }

regards,
dan carpenter

Quoted 5 lines
Quoted 2 lines
Quoted 2 lines

--
Florian

I'll leave it to Markus to correct the logic, unless he wants to punt it back to me :).

I'll fix it up when I get back home next week.

Cheers,
-Markus

#6 lore raw

PATCH 1/1 thermal: brcmstb: disable trip points properly when needed

From: Markus Mayer <[email protected]>

The code checking for low and high temperature points was still based on an earlier implementation of the driver. It wasn't working properly with the data types currently being used.

We fix this by disabling the high trip point if our high temperature is INT_MAX and disabling the low trip point if our low temperature is -INT_MAX (or lower).

Signed-off-by Markus Mayer <[email protected]>

---

Here is my patch for the issue described. Please let me know if I should be squashing this into the driver patch and re-submit the entire series.

 drivers/thermal/broadcom/brcmstb_thermal.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)
drivers/thermal/broadcom/brcmstb_thermal.c +11 -10
index 87b8e7a..1919f91 100644
@@ -277,22 +277,23 @@ static int brcmstb_set_trips(void *data, int low, int high)
 
 	dev_dbg(priv->dev, "set trips %d <--> %d\n", low, high);
 
-	if (low) {
-		if (low > INT_MAX)
-			low = INT_MAX;
+	/*
+	 * Disable low-temp if "low" is too small. As per thermal framework
+	 * API, we use -INT_MAX rather than INT_MIN.
+	 */
+	if (low <= -INT_MAX) {
+		avs_tmon_trip_enable(priv, TMON_TRIP_TYPE_LOW, 0);
+	} else {
 		avs_tmon_set_trip_temp(priv, TMON_TRIP_TYPE_LOW, low);
 		avs_tmon_trip_enable(priv, TMON_TRIP_TYPE_LOW, 1);
-	} else {
-		avs_tmon_trip_enable(priv, TMON_TRIP_TYPE_LOW, 0);
 	}
 
-	if (high < ULONG_MAX) {
-		if (high > INT_MAX)
-			high = INT_MAX;
+	/* Disable high-temp if "high" is too big. */
+	if (high == INT_MAX) {
+		avs_tmon_trip_enable(priv, TMON_TRIP_TYPE_HIGH, 0);
+	} else {
 		avs_tmon_set_trip_temp(priv, TMON_TRIP_TYPE_HIGH, high);
 		avs_tmon_trip_enable(priv, TMON_TRIP_TYPE_HIGH, 1);
-	} else {
-		avs_tmon_trip_enable(priv, TMON_TRIP_TYPE_HIGH, 0);
 	}
 
 	return 0;
Signature · 1 line
2.7.4
#7 lore raw

Re: [PATCH] thermal: brcmstb: disable trip points properly when needed

Quoted 10 lines · Markus Mayer

From: Markus Mayer <[email protected]>

The code checking for low and high temperature points was still based on an earlier implementation of the driver. It wasn't working properly with the data types currently being used.

We fix this by disabling the high trip point if our high temperature is INT_MAX and disabling the low trip point if our low temperature is -INT_MAX (or lower).

Quoted 5 lines
Signed-off-by Markus Mayer <[email protected]>

---

Here is my patch for the issue described. Please let me know if I should be squashing this into the driver patch and re-submit the entire series.

Looks like my attempt at adding this patch to the existing thread may not have worked out so well.

Here's a link: https://marc.info/?l=linux-pm&m=150472721929919&w=2

Quoted 44 lines
 drivers/thermal/broadcom/brcmstb_thermal.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)
drivers/thermal/broadcom/brcmstb_thermal.c +11 -11
index 87b8e7a..1919f91 100644
@@ -277,22 +277,23 @@ static int brcmstb_set_trips(void *data, int low, int high)
 
        dev_dbg(priv->dev, "set trips %d <--> %d\n", low, high);
 
-       if (low) {
-               if (low > INT_MAX)
-                       low = INT_MAX;
+       /*
+        * Disable low-temp if "low" is too small. As per thermal framework
+        * API, we use -INT_MAX rather than INT_MIN.
+        */
+       if (low <= -INT_MAX) {
+               avs_tmon_trip_enable(priv, TMON_TRIP_TYPE_LOW, 0);
+       } else {
                avs_tmon_set_trip_temp(priv, TMON_TRIP_TYPE_LOW, low);
                avs_tmon_trip_enable(priv, TMON_TRIP_TYPE_LOW, 1);
-       } else {
-               avs_tmon_trip_enable(priv, TMON_TRIP_TYPE_LOW, 0);
        }
 
-       if (high < ULONG_MAX) {
-               if (high > INT_MAX)
-                       high = INT_MAX;
+       /* Disable high-temp if "high" is too big. */
+       if (high == INT_MAX) {
+               avs_tmon_trip_enable(priv, TMON_TRIP_TYPE_HIGH, 0);
+       } else {
                avs_tmon_set_trip_temp(priv, TMON_TRIP_TYPE_HIGH, high);
                avs_tmon_trip_enable(priv, TMON_TRIP_TYPE_HIGH, 1);
-       } else {
-               avs_tmon_trip_enable(priv, TMON_TRIP_TYPE_HIGH, 0);
        }
 
        return 0;
--

2.7.4