2017-09-14 23:19:54

by Markus Mayer

Subject: [PATCH] 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(-)

diff --git a/drivers/thermal/broadcom/brcmstb_thermal.c b/drivers/thermal/broadcom/brcmstb_thermal.c
index 87b8e7a..1919f91 100644
--- a/drivers/thermal/broadcom/brcmstb_thermal.c
+++ b/drivers/thermal/broadcom/brcmstb_thermal.c
@@ -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

2017-09-14 23:25:38

by Markus Mayer

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

On 14 September 2017 at 16:19, Markus Mayer <[email protected]> wrote:
> 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.

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

> drivers/thermal/broadcom/brcmstb_thermal.c | 21 +++++++++++----------
> 1 file changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/thermal/broadcom/brcmstb_thermal.c b/drivers/thermal/broadcom/brcmstb_thermal.c
> index 87b8e7a..1919f91 100644
> --- a/drivers/thermal/broadcom/brcmstb_thermal.c
> +++ b/drivers/thermal/broadcom/brcmstb_thermal.c
> @@ -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
>