[PATCH 0/2] thermal: rcar_{gen3_}thermal: Remove temperature bound

[PATCH 0/2] thermal: rcar_{gen3_}thermal: Remove temperature bound

[Niklas Söderlund]

Hi,

This series removes the checks that the read out temperature are within 
the guaranteed operation limits described in the hardware manual. It has 
been discussed with the hardware guys and the judgement is that it's 
better to report a best effort temperature instead of failing -EIO.

Niklas Söderlund (2):
  thermal: rcar_thermal: Remove temperature bound
  thermal: rcar_gen3_thermal: Remove temperature bound

 drivers/thermal/rcar_gen3_thermal.c | 4 ----
 drivers/thermal/rcar_thermal.c      | 7 -------
 2 files changed, 11 deletions(-)

-- 
2.24.1

[PATCH 1/2] thermal: rcar_thermal: Remove temperature bound

[Niklas Söderlund]

The hardware manual states that the operation of the sensor is not
guaranteed outside the range of -40°C to 125°C, not that the readings
are invalid. Remove the bound check and try to deliver temperature
readings even if we are outside the guaranteed operation range.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/thermal/rcar_thermal.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/thermal/rcar_thermal.c b/drivers/thermal/rcar_thermal.c
index d0873de718da9218..2ae60b27a0183db1 100644
--- a/drivers/thermal/rcar_thermal.c
+++ b/drivers/thermal/rcar_thermal.c
@@ -275,13 +275,6 @@ static int rcar_thermal_get_current_temp(struct rcar_thermal_priv *priv,
 		tmp = MCELSIUS((priv->ctemp * 5) - 60);
 	mutex_unlock(&priv->lock);
 
-	if ((tmp < MCELSIUS(-45)) || (tmp > MCELSIUS(125))) {
-		struct device *dev = rcar_priv_to_dev(priv);
-
-		dev_err(dev, "it couldn't measure temperature correctly\n");
-		return -EIO;
-	}
-
 	*temp = tmp;
 
 	return 0;
-- 
2.24.1

[PATCH 2/2] thermal: rcar_gen3_thermal: Remove temperature bound

[Niklas Söderlund]

The hardware manual states that the operation of the sensor is not
guaranteed with temperatures above 125°C, not that the readings are
invalid. Remove the bound check and try to deliver temperature readings
even if we are outside the guaranteed operation range.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/thermal/rcar_gen3_thermal.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/thermal/rcar_gen3_thermal.c b/drivers/thermal/rcar_gen3_thermal.c
index 1460cf9d9f1c397b..0f69ff676838a999 100644
--- a/drivers/thermal/rcar_gen3_thermal.c
+++ b/drivers/thermal/rcar_gen3_thermal.c
@@ -182,10 +182,6 @@ static int rcar_gen3_thermal_get_temp(void *devdata, int *temp)
 				tsc->coef.a2);
 	mcelsius = FIXPT_TO_MCELSIUS(val);
 
-	/* Make sure we are inside specifications */
-	if ((mcelsius < MCELSIUS(-40)) || (mcelsius > MCELSIUS(125)))
-		return -EIO;
-
 	/* Round value to device granularity setting */
 	*temp = rcar_gen3_thermal_round(mcelsius);
 
-- 
2.24.1

Re: [PATCH 1/2] thermal: rcar_thermal: Remove temperature bound

[Daniel Lezcano]

On 14/01/2020 23:29, Niklas Söderlund wrote:
> The hardware manual states that the operation of the sensor is not
> guaranteed outside the range of -40°C to 125°C, not that the readings
> are invalid. Remove the bound check and try to deliver temperature
> readings even if we are outside the guaranteed operation range.

And what if the sensor is returning crap in this out-of-range operation?



> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
>  drivers/thermal/rcar_thermal.c | 7 -------
>  1 file changed, 7 deletions(-)
> 
> diff --git a/drivers/thermal/rcar_thermal.c b/drivers/thermal/rcar_thermal.c
> index d0873de718da9218..2ae60b27a0183db1 100644
> --- a/drivers/thermal/rcar_thermal.c
> +++ b/drivers/thermal/rcar_thermal.c
> @@ -275,13 +275,6 @@ static int rcar_thermal_get_current_temp(struct rcar_thermal_priv *priv,
>  		tmp = MCELSIUS((priv->ctemp * 5) - 60);
>  	mutex_unlock(&priv->lock);
>  
> -	if ((tmp < MCELSIUS(-45)) || (tmp > MCELSIUS(125))) {
> -		struct device *dev = rcar_priv_to_dev(priv);
> -
> -		dev_err(dev, "it couldn't measure temperature correctly\n");
> -		return -EIO;
> -	}
> -
>  	*temp = tmp;
>  
>  	return 0;
> 


-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH 1/2] thermal: rcar_thermal: Remove temperature bound

[Niklas Söderlund]

Hi Daniel,

Thanks for your feedback.

On 2020-01-15 14:24:30 +0100, Daniel Lezcano wrote:
> On 14/01/2020 23:29, Niklas Söderlund wrote:
> > The hardware manual states that the operation of the sensor is not
> > guaranteed outside the range of -40°C to 125°C, not that the readings
> > are invalid. Remove the bound check and try to deliver temperature
> > readings even if we are outside the guaranteed operation range.
> 
> And what if the sensor is returning crap in this out-of-range operation?

I'm not sure what is worse, reporting an untrue (but still outside the 
guaranteed operation range) extreme temperature or failing with -EIO.  
The view of the hardware guys is that it's better to report what the 
sensor indicates then to return -EIO.

> 
> 
> 
> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > ---
> >  drivers/thermal/rcar_thermal.c | 7 -------
> >  1 file changed, 7 deletions(-)
> > 
> > diff --git a/drivers/thermal/rcar_thermal.c b/drivers/thermal/rcar_thermal.c
> > index d0873de718da9218..2ae60b27a0183db1 100644
> > --- a/drivers/thermal/rcar_thermal.c
> > +++ b/drivers/thermal/rcar_thermal.c
> > @@ -275,13 +275,6 @@ static int rcar_thermal_get_current_temp(struct rcar_thermal_priv *priv,
> >  		tmp = MCELSIUS((priv->ctemp * 5) - 60);
> >  	mutex_unlock(&priv->lock);
> >  
> > -	if ((tmp < MCELSIUS(-45)) || (tmp > MCELSIUS(125))) {
> > -		struct device *dev = rcar_priv_to_dev(priv);
> > -
> > -		dev_err(dev, "it couldn't measure temperature correctly\n");
> > -		return -EIO;
> > -	}
> > -
> >  	*temp = tmp;
> >  
> >  	return 0;
> > 
> 
> 
> -- 
>  <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
> 
> Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
> <http://twitter.com/#!/linaroorg> Twitter |
> <http://www.linaro.org/linaro-blog/> Blog
> 

-- 
Regards,
Niklas Söderlund

Re: [PATCH 1/2] thermal: rcar_thermal: Remove temperature bound

[Daniel Lezcano]

On 15/01/2020 14:45, Niklas Söderlund wrote:
> Hi Daniel,
> 
> Thanks for your feedback.
> 
> On 2020-01-15 14:24:30 +0100, Daniel Lezcano wrote:
>> On 14/01/2020 23:29, Niklas Söderlund wrote:
>>> The hardware manual states that the operation of the sensor is not
>>> guaranteed outside the range of -40°C to 125°C, not that the readings
>>> are invalid. Remove the bound check and try to deliver temperature
>>> readings even if we are outside the guaranteed operation range.
>>
>> And what if the sensor is returning crap in this out-of-range operation?
> 
> I'm not sure what is worse, reporting an untrue (but still outside the 
> guaranteed operation range) extreme temperature or failing with -EIO.  
> The view of the hardware guys is that it's better to report what the 
> sensor indicates then to return -EIO.

I don't get the point.

What happens if we read the sensor while it is above or below the limits?


[ ... ]



-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH 1/2] thermal: rcar_thermal: Remove temperature bound

[Niklas Söderlund]

Hi Daniel,

On 2020-01-15 19:15:11 +0100, Daniel Lezcano wrote:
> On 15/01/2020 14:45, Niklas Söderlund wrote:
> > Hi Daniel,
> > 
> > Thanks for your feedback.
> > 
> > On 2020-01-15 14:24:30 +0100, Daniel Lezcano wrote:
> >> On 14/01/2020 23:29, Niklas Söderlund wrote:
> >>> The hardware manual states that the operation of the sensor is not
> >>> guaranteed outside the range of -40°C to 125°C, not that the readings
> >>> are invalid. Remove the bound check and try to deliver temperature
> >>> readings even if we are outside the guaranteed operation range.
> >>
> >> And what if the sensor is returning crap in this out-of-range operation?
> > 
> > I'm not sure what is worse, reporting an untrue (but still outside the 
> > guaranteed operation range) extreme temperature or failing with -EIO.  
> > The view of the hardware guys is that it's better to report what the 
> > sensor indicates then to return -EIO.
> 
> I don't get the point.
> 
> What happens if we read the sensor while it is above or below the limits?

The manual describes the read outs as being +/- 2°C from the true 
temperature for the guaranteed operating range. Outside this range the 
difference between the true temperature and the read out temperature 
might be larger than 2°C.

> 
> 
> [ ... ]
> 
> 
> 
> -- 
>  <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
> 
> Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
> <http://twitter.com/#!/linaroorg> Twitter |
> <http://www.linaro.org/linaro-blog/> Blog
> 

-- 
Regards,
Niklas Söderlund

Re: [PATCH 1/2] thermal: rcar_thermal: Remove temperature bound

[Daniel Lezcano]

On 15/01/2020 19:39, Niklas Söderlund wrote:
> Hi Daniel,
> 
> On 2020-01-15 19:15:11 +0100, Daniel Lezcano wrote:
>> On 15/01/2020 14:45, Niklas Söderlund wrote:
>>> Hi Daniel,
>>>
>>> Thanks for your feedback.
>>>
>>> On 2020-01-15 14:24:30 +0100, Daniel Lezcano wrote:
>>>> On 14/01/2020 23:29, Niklas Söderlund wrote:
>>>>> The hardware manual states that the operation of the sensor is not
>>>>> guaranteed outside the range of -40°C to 125°C, not that the readings
>>>>> are invalid. Remove the bound check and try to deliver temperature
>>>>> readings even if we are outside the guaranteed operation range.
>>>>
>>>> And what if the sensor is returning crap in this out-of-range operation?
>>>
>>> I'm not sure what is worse, reporting an untrue (but still outside the 
>>> guaranteed operation range) extreme temperature or failing with -EIO.  
>>> The view of the hardware guys is that it's better to report what the 
>>> sensor indicates then to return -EIO.
>>
>> I don't get the point.
>>
>> What happens if we read the sensor while it is above or below the limits?
> 
> The manual describes the read outs as being +/- 2°C from the true 
> temperature for the guaranteed operating range. Outside this range the 
> difference between the true temperature and the read out temperature 
> might be larger than 2°C.

Ok, I see the point now. I guess in any case the SoC won't be working
very well outside of these operating range also.

It would be interesting to add in the core code a warning when reaching
the sensor's operating range. Please replace the code deletion in your
patches by a comment keeping the range values for reference if we add a
sensor boundaries property later.

Thanks
  -- Daniel

-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH 1/2] thermal: rcar_thermal: Remove temperature bound

[Niklas Söderlund]

On 2020-01-15 19:52:25 +0100, Daniel Lezcano wrote:
> On 15/01/2020 19:39, Niklas Söderlund wrote:
> > Hi Daniel,
> > 
> > On 2020-01-15 19:15:11 +0100, Daniel Lezcano wrote:
> >> On 15/01/2020 14:45, Niklas Söderlund wrote:
> >>> Hi Daniel,
> >>>
> >>> Thanks for your feedback.
> >>>
> >>> On 2020-01-15 14:24:30 +0100, Daniel Lezcano wrote:
> >>>> On 14/01/2020 23:29, Niklas Söderlund wrote:
> >>>>> The hardware manual states that the operation of the sensor is not
> >>>>> guaranteed outside the range of -40°C to 125°C, not that the readings
> >>>>> are invalid. Remove the bound check and try to deliver temperature
> >>>>> readings even if we are outside the guaranteed operation range.
> >>>>
> >>>> And what if the sensor is returning crap in this out-of-range operation?
> >>>
> >>> I'm not sure what is worse, reporting an untrue (but still outside the 
> >>> guaranteed operation range) extreme temperature or failing with -EIO.  
> >>> The view of the hardware guys is that it's better to report what the 
> >>> sensor indicates then to return -EIO.
> >>
> >> I don't get the point.
> >>
> >> What happens if we read the sensor while it is above or below the limits?
> > 
> > The manual describes the read outs as being +/- 2°C from the true 
> > temperature for the guaranteed operating range. Outside this range the 
> > difference between the true temperature and the read out temperature 
> > might be larger than 2°C.
> 
> Ok, I see the point now. I guess in any case the SoC won't be working
> very well outside of these operating range also.
> 
> It would be interesting to add in the core code a warning when reaching
> the sensor's operating range. Please replace the code deletion in your
> patches by a comment keeping the range values for reference if we add a
> sensor boundaries property later.

Thanks, will do and post a v2.

> 
> Thanks
>   -- Daniel
> 
> -- 
>  <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
> 
> Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
> <http://twitter.com/#!/linaroorg> Twitter |
> <http://www.linaro.org/linaro-blog/> Blog
> 

-- 
Regards,
Niklas Söderlund