| Message ID | 20200114222945.3128250-2-niklas.soderlund+renesas@ragnatech.se (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Daniel Lezcano |
| Headers | show |
| Series | thermal: rcar_{gen3_}thermal: Remove temperature bound | expand |
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; >
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 >
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? [ ... ]
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 >
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
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 >
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;
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(-)