[1/2] thermal: rcar_thermal: Remove temperature bound
diff mbox series

Message ID 20200114222945.3128250-2-niklas.soderlund+renesas@ragnatech.se
State Superseded
Delegated to: Daniel Lezcano
Headers show
Series
  • thermal: rcar_{gen3_}thermal: Remove temperature bound
Related show

Commit Message

Niklas Söderlund Jan. 14, 2020, 10:29 p.m. UTC
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(-)

Comments

Daniel Lezcano Jan. 15, 2020, 1:24 p.m. UTC | #1
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;
>
Niklas Söderlund Jan. 15, 2020, 1:45 p.m. UTC | #2
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
>
Daniel Lezcano Jan. 15, 2020, 6:15 p.m. UTC | #3
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?


[ ... ]
Niklas Söderlund Jan. 15, 2020, 6:39 p.m. UTC | #4
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
>
Daniel Lezcano Jan. 15, 2020, 6:52 p.m. UTC | #5
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
Niklas Söderlund Jan. 15, 2020, 7 p.m. UTC | #6
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
>

Patch
diff mbox series

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;