Linux-Renesas-SoC Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 0/2] thermal: rcar_{gen3_}thermal: Remove temperature bound
@ 2020-01-14 22:29 Niklas Söderlund
  2020-01-14 22:29 ` [PATCH 1/2] thermal: rcar_thermal: " Niklas Söderlund
  2020-01-14 22:29 ` [PATCH 2/2] thermal: rcar_gen3_thermal: " Niklas Söderlund
  0 siblings, 2 replies; 9+ messages in thread
From: Niklas Söderlund @ 2020-01-14 22:29 UTC (permalink / raw)
  To: linux-pm, Wolfram Sang; +Cc: linux-renesas-soc, 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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/2] thermal: rcar_thermal: Remove temperature bound
  2020-01-14 22:29 [PATCH 0/2] thermal: rcar_{gen3_}thermal: Remove temperature bound Niklas Söderlund
@ 2020-01-14 22:29 ` " Niklas Söderlund
  2020-01-15 13:24   ` Daniel Lezcano
  2020-01-14 22:29 ` [PATCH 2/2] thermal: rcar_gen3_thermal: " Niklas Söderlund
  1 sibling, 1 reply; 9+ messages in thread
From: Niklas Söderlund @ 2020-01-14 22:29 UTC (permalink / raw)
  To: linux-pm, Wolfram Sang; +Cc: linux-renesas-soc, 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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 2/2] thermal: rcar_gen3_thermal: Remove temperature bound
  2020-01-14 22:29 [PATCH 0/2] thermal: rcar_{gen3_}thermal: Remove temperature bound Niklas Söderlund
  2020-01-14 22:29 ` [PATCH 1/2] thermal: rcar_thermal: " Niklas Söderlund
@ 2020-01-14 22:29 ` " Niklas Söderlund
  1 sibling, 0 replies; 9+ messages in thread
From: Niklas Söderlund @ 2020-01-14 22:29 UTC (permalink / raw)
  To: linux-pm, Wolfram Sang; +Cc: linux-renesas-soc, 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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] thermal: rcar_thermal: Remove temperature bound
  2020-01-14 22:29 ` [PATCH 1/2] thermal: rcar_thermal: " Niklas Söderlund
@ 2020-01-15 13:24   ` Daniel Lezcano
  2020-01-15 13:45     ` Niklas Söderlund
  0 siblings, 1 reply; 9+ messages in thread
From: Daniel Lezcano @ 2020-01-15 13:24 UTC (permalink / raw)
  To: Niklas Söderlund, linux-pm, Wolfram Sang; +Cc: linux-renesas-soc

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] thermal: rcar_thermal: Remove temperature bound
  2020-01-15 13:24   ` Daniel Lezcano
@ 2020-01-15 13:45     ` Niklas Söderlund
  2020-01-15 18:15       ` Daniel Lezcano
  0 siblings, 1 reply; 9+ messages in thread
From: Niklas Söderlund @ 2020-01-15 13:45 UTC (permalink / raw)
  To: Daniel Lezcano; +Cc: linux-pm, Wolfram Sang, linux-renesas-soc

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] thermal: rcar_thermal: Remove temperature bound
  2020-01-15 13:45     ` Niklas Söderlund
@ 2020-01-15 18:15       ` Daniel Lezcano
  2020-01-15 18:39         ` Niklas Söderlund
  0 siblings, 1 reply; 9+ messages in thread
From: Daniel Lezcano @ 2020-01-15 18:15 UTC (permalink / raw)
  To: Niklas Söderlund; +Cc: linux-pm, Wolfram Sang, linux-renesas-soc

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] thermal: rcar_thermal: Remove temperature bound
  2020-01-15 18:15       ` Daniel Lezcano
@ 2020-01-15 18:39         ` Niklas Söderlund
  2020-01-15 18:52           ` Daniel Lezcano
  0 siblings, 1 reply; 9+ messages in thread
From: Niklas Söderlund @ 2020-01-15 18:39 UTC (permalink / raw)
  To: Daniel Lezcano; +Cc: linux-pm, Wolfram Sang, linux-renesas-soc

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] thermal: rcar_thermal: Remove temperature bound
  2020-01-15 18:39         ` Niklas Söderlund
@ 2020-01-15 18:52           ` Daniel Lezcano
  2020-01-15 19:00             ` Niklas Söderlund
  0 siblings, 1 reply; 9+ messages in thread
From: Daniel Lezcano @ 2020-01-15 18:52 UTC (permalink / raw)
  To: Niklas Söderlund; +Cc: linux-pm, Wolfram Sang, linux-renesas-soc

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] thermal: rcar_thermal: Remove temperature bound
  2020-01-15 18:52           ` Daniel Lezcano
@ 2020-01-15 19:00             ` Niklas Söderlund
  0 siblings, 0 replies; 9+ messages in thread
From: Niklas Söderlund @ 2020-01-15 19:00 UTC (permalink / raw)
  To: Daniel Lezcano; +Cc: linux-pm, Wolfram Sang, linux-renesas-soc

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, back to index

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-14 22:29 [PATCH 0/2] thermal: rcar_{gen3_}thermal: Remove temperature bound Niklas Söderlund
2020-01-14 22:29 ` [PATCH 1/2] thermal: rcar_thermal: " Niklas Söderlund
2020-01-15 13:24   ` Daniel Lezcano
2020-01-15 13:45     ` Niklas Söderlund
2020-01-15 18:15       ` Daniel Lezcano
2020-01-15 18:39         ` Niklas Söderlund
2020-01-15 18:52           ` Daniel Lezcano
2020-01-15 19:00             ` Niklas Söderlund
2020-01-14 22:29 ` [PATCH 2/2] thermal: rcar_gen3_thermal: " Niklas Söderlund

Linux-Renesas-SoC Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-renesas-soc/0 linux-renesas-soc/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-renesas-soc linux-renesas-soc/ https://lore.kernel.org/linux-renesas-soc \
		linux-renesas-soc@vger.kernel.org
	public-inbox-index linux-renesas-soc

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-renesas-soc


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git