diff mbox series

[4.19.y-cip,02/10] thermal: rcar_gen3_thermal: Remove temperature bound

Message ID 20200825132156.7839-3-biju.das.jz@bp.renesas.com
State Accepted
Headers show
Series Add OPP/Thermal/Timer/CAN[FD] support | expand

Commit Message

Biju Das Aug. 25, 2020, 1:21 p.m. UTC
From: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

commit 0f510a2457cbbba18a98492bab1bf540be57ebd1 upstream.

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>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Link: https://lore.kernel.org/r/20200117160554.3812787-3-niklas.soderlund+renesas@ragnatech.se
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 drivers/thermal/rcar_gen3_thermal.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

Comments

Pavel Machek Aug. 26, 2020, 7:21 a.m. UTC | #1
On Tue 2020-08-25 14:21:48, Biju Das wrote:
> From: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> 
> commit 0f510a2457cbbba18a98492bab1bf540be57ebd1 upstream.
> 
> 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.

I'm tempted to ask "how was this tested?" :-).

Sometimes sensors have glitches producing very high values in normal
temperatures, and it such case clamping can be useful, but I'll assume
your sensor works ok.

Best regards,
								Pavel


> +++ b/drivers/thermal/rcar_gen3_thermal.c
> @@ -182,9 +182,7 @@ 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;
> +	/* Guaranteed operating range is -40C to 125C. */
>  
>  	/* Round value to device granularity setting */
>  	*temp = rcar_gen3_thermal_round(mcelsius);
Biju Das Aug. 26, 2020, 7:40 a.m. UTC | #2
Hi Pavel,

Thanks for the feedback.

> Subject: Re: [PATCH 4.19.y-cip 02/10] thermal: rcar_gen3_thermal: Remove
> temperature bound
>
> On Tue 2020-08-25 14:21:48, Biju Das wrote:
> > From: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> >
> > commit 0f510a2457cbbba18a98492bab1bf540be57ebd1 upstream.
> >
> > 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.
>
> I'm tempted to ask "how was this tested?" :-).
>
> Sometimes sensors have glitches producing very high values in normal
> temperatures, and it such case clamping can be useful, but I'll assume your
> sensor works ok.

Niklas,  Can you please clarify?

Cheers,
Biju



>
> > +++ b/drivers/thermal/rcar_gen3_thermal.c
> > @@ -182,9 +182,7 @@ 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;
> > +/* Guaranteed operating range is -40C to 125C. */
> >
> >  /* Round value to device granularity setting */
> >  *temp = rcar_gen3_thermal_round(mcelsius);
>
> --
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany


Renesas Electronics Europe GmbH, Geschaeftsfuehrer/President: Carsten Jauch, Sitz der Gesellschaft/Registered office: Duesseldorf, Arcadiastrasse 10, 40472 Duesseldorf, Germany, Handelsregister/Commercial Register: Duesseldorf, HRB 3708 USt-IDNr./Tax identification no.: DE 119353406 WEEE-Reg.-Nr./WEEE reg. no.: DE 14978647
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.

View/Reply Online (#5258): https://lists.cip-project.org/g/cip-dev/message/5258
Mute This Topic: https://lists.cip-project.org/mt/76406706/4520428
Group Owner: cip-dev+owner@lists.cip-project.org
Unsubscribe: https://lists.cip-project.org/g/cip-dev/leave/8129116/1171672734/xyzzy  [patchwork-cip-dev@patchwork.kernel.org]
-=-=-=-=-=-=-=-=-=-=-=-
Niklas Söderlund Aug. 26, 2020, 8:45 a.m. UTC | #3
Hi Biju,

On 2020-08-26 07:40:46 +0000, Biju Das wrote:
> Hi Pavel,
> 
> Thanks for the feedback.
> 
> > Subject: Re: [PATCH 4.19.y-cip 02/10] thermal: rcar_gen3_thermal: Remove
> > temperature bound
> >
> > On Tue 2020-08-25 14:21:48, Biju Das wrote:
> > > From: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > >
> > > commit 0f510a2457cbbba18a98492bab1bf540be57ebd1 upstream.
> > >
> > > 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.
> >
> > I'm tempted to ask "how was this tested?" :-).
> >
> > Sometimes sensors have glitches producing very high values in normal
> > temperatures, and it such case clamping can be useful, but I'll assume your
> > sensor works ok.
> 
> Niklas,  Can you please clarify?

IIRC I was asked to remove the clamp as the hardware team had tested the 
sensors in a wider range and it was judged better to report a reading 
even if it was not guaranteed then to clamp it. I have not been able 
myself to test this change myself and rely on the report from the 
hardware team.

> 
> Cheers,
> Biju
> 
> 
> 
> >
> > > +++ b/drivers/thermal/rcar_gen3_thermal.c
> > > @@ -182,9 +182,7 @@ 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;
> > > +/* Guaranteed operating range is -40C to 125C. */
> > >
> > >  /* Round value to device granularity setting */
> > >  *temp = rcar_gen3_thermal_round(mcelsius);
> >
> > --
> > DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> 
> 
> Renesas Electronics Europe GmbH, Geschaeftsfuehrer/President: Carsten Jauch, Sitz der Gesellschaft/Registered office: Duesseldorf, Arcadiastrasse 10, 40472 Duesseldorf, Germany, Handelsregister/Commercial Register: Duesseldorf, HRB 3708 USt-IDNr./Tax identification no.: DE 119353406 WEEE-Reg.-Nr./WEEE reg. no.: DE 14978647
Biju Das Aug. 26, 2020, 9:16 a.m. UTC | #4
Hi Niklas,

Thanks for your feedback.

Cheers,
Biju

> -----Original Message-----
> From: Niklas <niklas.soderlund@ragnatech.se>
> Sent: 26 August 2020 09:46
> To: Biju Das <biju.das.jz@bp.renesas.com>
> Cc: Pavel Machek <pavel@denx.de>; cip-dev@lists.cip-project.org; Nobuhiro
> Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>; Chris Paterson
> <Chris.Paterson2@renesas.com>; Prabhakar Mahadev Lad
> <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Subject: Re: [PATCH 4.19.y-cip 02/10] thermal: rcar_gen3_thermal: Remove
> temperature bound
>
> Hi Biju,
>
> On 2020-08-26 07:40:46 +0000, Biju Das wrote:
> > Hi Pavel,
> >
> > Thanks for the feedback.
> >
> > > Subject: Re: [PATCH 4.19.y-cip 02/10] thermal: rcar_gen3_thermal:
> > > Remove temperature bound
> > >
> > > On Tue 2020-08-25 14:21:48, Biju Das wrote:
> > > > From: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > > >
> > > > commit 0f510a2457cbbba18a98492bab1bf540be57ebd1 upstream.
> > > >
> > > > 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.
> > >
> > > I'm tempted to ask "how was this tested?" :-).
> > >
> > > Sometimes sensors have glitches producing very high values in normal
> > > temperatures, and it such case clamping can be useful, but I'll
> > > assume your sensor works ok.
> >
> > Niklas,  Can you please clarify?
>
> IIRC I was asked to remove the clamp as the hardware team had tested the
> sensors in a wider range and it was judged better to report a reading even if
> it was not guaranteed then to clamp it. I have not been able myself to test
> this change myself and rely on the report from the hardware team.
>
> >
> > Cheers,
> > Biju
> >
> >
> >
> > >
> > > > +++ b/drivers/thermal/rcar_gen3_thermal.c
> > > > @@ -182,9 +182,7 @@ 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;
> > > > +/* Guaranteed operating range is -40C to 125C. */
> > > >
> > > >  /* Round value to device granularity setting */  *temp =
> > > > rcar_gen3_thermal_round(mcelsius);
> > >
> > > --
> > > DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> > > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell,
> > > Germany
> >
> >
> > Renesas Electronics Europe GmbH, Geschaeftsfuehrer/President: Carsten
> > Jauch, Sitz der Gesellschaft/Registered office: Duesseldorf,
> > Arcadiastrasse 10, 40472 Duesseldorf, Germany,
> > Handelsregister/Commercial Register: Duesseldorf, HRB 3708
> > USt-IDNr./Tax identification no.: DE 119353406 WEEE-Reg.-Nr./WEEE reg.
> > no.: DE 14978647
>
> --
> Regards,
> Niklas Söderlund


Renesas Electronics Europe GmbH, Geschaeftsfuehrer/President: Carsten Jauch, Sitz der Gesellschaft/Registered office: Duesseldorf, Arcadiastrasse 10, 40472 Duesseldorf, Germany, Handelsregister/Commercial Register: Duesseldorf, HRB 3708 USt-IDNr./Tax identification no.: DE 119353406 WEEE-Reg.-Nr./WEEE reg. no.: DE 14978647
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.

View/Reply Online (#5260): https://lists.cip-project.org/g/cip-dev/message/5260
Mute This Topic: https://lists.cip-project.org/mt/76406706/4520428
Group Owner: cip-dev+owner@lists.cip-project.org
Unsubscribe: https://lists.cip-project.org/g/cip-dev/leave/8129116/1171672734/xyzzy  [patchwork-cip-dev@patchwork.kernel.org]
-=-=-=-=-=-=-=-=-=-=-=-
diff mbox series

Patch

diff --git a/drivers/thermal/rcar_gen3_thermal.c b/drivers/thermal/rcar_gen3_thermal.c
index 42ec69d29540..e7a0c7fc9897 100644
--- a/drivers/thermal/rcar_gen3_thermal.c
+++ b/drivers/thermal/rcar_gen3_thermal.c
@@ -182,9 +182,7 @@  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;
+	/* Guaranteed operating range is -40C to 125C. */
 
 	/* Round value to device granularity setting */
 	*temp = rcar_gen3_thermal_round(mcelsius);