* [PATCH v2] thermal: rcar_gen3_thermal: Fix undefined temperature if negative
@ 2020-06-09 11:45 Yoshihiro Shimoda
2020-06-10 0:13 ` Niklas Söderlund
2020-06-23 12:06 ` Amit Kucheria
0 siblings, 2 replies; 4+ messages in thread
From: Yoshihiro Shimoda @ 2020-06-09 11:45 UTC (permalink / raw)
To: rui.zhang, daniel.lezcano, amit.kucheria
Cc: niklas.soderlund+renesas, van.do.xw, dien.pham.ry, linux-pm,
linux-renesas-soc, Yoshihiro Shimoda
From: Dien Pham <dien.pham.ry@renesas.com>
As description for DIV_ROUND_CLOSEST in file include/linux/kernel.h.
"Result is undefined for negative divisors if the dividend variable
type is unsigned and for negative dividends if the divisor variable
type is unsigned."
In current code, the FIXPT_DIV uses DIV_ROUND_CLOSEST but has not
checked sign of divisor before using. It makes undefined temperature
value in case the value is negative.
This patch fixes to satisfy DIV_ROUND_CLOSEST description
and fix bug too. Note that the name "reg" is not good because it should
be the same type as rcar_gen3_thermal_read(). So, rename it with "ctemp".
Signed-off-by: Van Do <van.do.xw@renesas.com>
Signed-off-by: Dien Pham <dien.pham.ry@renesas.com>
[shimoda: minor fixes, add Fixes tag]
Fixes: 564e73d283af ("thermal: rcar_gen3_thermal: Add R-Car Gen3 thermal driver")
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
Changes from v1:
- Use int instead of long.
- Rename "reg" with "ctemp".
https://patchwork.kernel.org/patch/11593051/
drivers/thermal/rcar_gen3_thermal.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/thermal/rcar_gen3_thermal.c b/drivers/thermal/rcar_gen3_thermal.c
index 58fe7c1..49ea330 100644
--- a/drivers/thermal/rcar_gen3_thermal.c
+++ b/drivers/thermal/rcar_gen3_thermal.c
@@ -167,16 +167,16 @@ static int rcar_gen3_thermal_get_temp(void *devdata, int *temp)
{
struct rcar_gen3_thermal_tsc *tsc = devdata;
int mcelsius, val;
- u32 reg;
+ int ctemp;
/* Read register and convert to mili Celsius */
- reg = rcar_gen3_thermal_read(tsc, REG_GEN3_TEMP) & CTEMP_MASK;
+ ctemp = rcar_gen3_thermal_read(tsc, REG_GEN3_TEMP) & CTEMP_MASK;
- if (reg <= thcode[tsc->id][1])
- val = FIXPT_DIV(FIXPT_INT(reg) - tsc->coef.b1,
+ if (ctemp <= thcode[tsc->id][1])
+ val = FIXPT_DIV(FIXPT_INT(ctemp) - tsc->coef.b1,
tsc->coef.a1);
else
- val = FIXPT_DIV(FIXPT_INT(reg) - tsc->coef.b2,
+ val = FIXPT_DIV(FIXPT_INT(ctemp) - tsc->coef.b2,
tsc->coef.a2);
mcelsius = FIXPT_TO_MCELSIUS(val);
--
2.7.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] thermal: rcar_gen3_thermal: Fix undefined temperature if negative
2020-06-09 11:45 [PATCH v2] thermal: rcar_gen3_thermal: Fix undefined temperature if negative Yoshihiro Shimoda
@ 2020-06-10 0:13 ` Niklas Söderlund
2020-06-23 12:06 ` Amit Kucheria
1 sibling, 0 replies; 4+ messages in thread
From: Niklas Söderlund @ 2020-06-10 0:13 UTC (permalink / raw)
To: Yoshihiro Shimoda
Cc: rui.zhang, daniel.lezcano, amit.kucheria, van.do.xw,
dien.pham.ry, linux-pm, linux-renesas-soc
Hi Shimoda-san and Pham-san,
Thanks for your work.
On 2020-06-09 20:45:10 +0900, Yoshihiro Shimoda wrote:
> From: Dien Pham <dien.pham.ry@renesas.com>
>
> As description for DIV_ROUND_CLOSEST in file include/linux/kernel.h.
> "Result is undefined for negative divisors if the dividend variable
> type is unsigned and for negative dividends if the divisor variable
> type is unsigned."
>
> In current code, the FIXPT_DIV uses DIV_ROUND_CLOSEST but has not
> checked sign of divisor before using. It makes undefined temperature
> value in case the value is negative.
>
> This patch fixes to satisfy DIV_ROUND_CLOSEST description
> and fix bug too. Note that the name "reg" is not good because it should
> be the same type as rcar_gen3_thermal_read(). So, rename it with "ctemp".
>
> Signed-off-by: Van Do <van.do.xw@renesas.com>
> Signed-off-by: Dien Pham <dien.pham.ry@renesas.com>
> [shimoda: minor fixes, add Fixes tag]
> Fixes: 564e73d283af ("thermal: rcar_gen3_thermal: Add R-Car Gen3 thermal driver")
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Tested-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
> Changes from v1:
> - Use int instead of long.
> - Rename "reg" with "ctemp".
> https://patchwork.kernel.org/patch/11593051/
>
> drivers/thermal/rcar_gen3_thermal.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/thermal/rcar_gen3_thermal.c b/drivers/thermal/rcar_gen3_thermal.c
> index 58fe7c1..49ea330 100644
> --- a/drivers/thermal/rcar_gen3_thermal.c
> +++ b/drivers/thermal/rcar_gen3_thermal.c
> @@ -167,16 +167,16 @@ static int rcar_gen3_thermal_get_temp(void *devdata, int *temp)
> {
> struct rcar_gen3_thermal_tsc *tsc = devdata;
> int mcelsius, val;
> - u32 reg;
> + int ctemp;
>
> /* Read register and convert to mili Celsius */
> - reg = rcar_gen3_thermal_read(tsc, REG_GEN3_TEMP) & CTEMP_MASK;
> + ctemp = rcar_gen3_thermal_read(tsc, REG_GEN3_TEMP) & CTEMP_MASK;
>
> - if (reg <= thcode[tsc->id][1])
> - val = FIXPT_DIV(FIXPT_INT(reg) - tsc->coef.b1,
> + if (ctemp <= thcode[tsc->id][1])
> + val = FIXPT_DIV(FIXPT_INT(ctemp) - tsc->coef.b1,
> tsc->coef.a1);
> else
> - val = FIXPT_DIV(FIXPT_INT(reg) - tsc->coef.b2,
> + val = FIXPT_DIV(FIXPT_INT(ctemp) - tsc->coef.b2,
> tsc->coef.a2);
> mcelsius = FIXPT_TO_MCELSIUS(val);
>
> --
> 2.7.4
>
--
Regards,
Niklas Söderlund
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] thermal: rcar_gen3_thermal: Fix undefined temperature if negative
2020-06-09 11:45 [PATCH v2] thermal: rcar_gen3_thermal: Fix undefined temperature if negative Yoshihiro Shimoda
2020-06-10 0:13 ` Niklas Söderlund
@ 2020-06-23 12:06 ` Amit Kucheria
2020-06-25 2:06 ` Yoshihiro Shimoda
1 sibling, 1 reply; 4+ messages in thread
From: Amit Kucheria @ 2020-06-23 12:06 UTC (permalink / raw)
To: Yoshihiro Shimoda
Cc: Zhang Rui, Daniel Lezcano, niklas.soderlund+renesas, van.do.xw,
dien.pham.ry, Linux PM list, linux-renesas-soc
Hi,
On Tue, Jun 9, 2020 at 5:15 PM Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
>
> From: Dien Pham <dien.pham.ry@renesas.com>
>
> As description for DIV_ROUND_CLOSEST in file include/linux/kernel.h.
> "Result is undefined for negative divisors if the dividend variable
> type is unsigned and for negative dividends if the divisor variable
> type is unsigned."
>
> In current code, the FIXPT_DIV uses DIV_ROUND_CLOSEST but has not
> checked sign of divisor before using. It makes undefined temperature
> value in case the value is negative.
>
> This patch fixes to satisfy DIV_ROUND_CLOSEST description
> and fix bug too. Note that the name "reg" is not good because it should
> be the same type as rcar_gen3_thermal_read(). So, rename it with "ctemp".
>
> Signed-off-by: Van Do <van.do.xw@renesas.com>
> Signed-off-by: Dien Pham <dien.pham.ry@renesas.com>
> [shimoda: minor fixes, add Fixes tag]
> Fixes: 564e73d283af ("thermal: rcar_gen3_thermal: Add R-Car Gen3 thermal driver")
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> ---
> Changes from v1:
> - Use int instead of long.
> - Rename "reg" with "ctemp".
> https://patchwork.kernel.org/patch/11593051/
>
> drivers/thermal/rcar_gen3_thermal.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/thermal/rcar_gen3_thermal.c b/drivers/thermal/rcar_gen3_thermal.c
> index 58fe7c1..49ea330 100644
> --- a/drivers/thermal/rcar_gen3_thermal.c
> +++ b/drivers/thermal/rcar_gen3_thermal.c
> @@ -167,16 +167,16 @@ static int rcar_gen3_thermal_get_temp(void *devdata, int *temp)
> {
> struct rcar_gen3_thermal_tsc *tsc = devdata;
> int mcelsius, val;
> - u32 reg;
> + int ctemp;
Wouldn't it be better to change the variable type to fix the bug in
this patch and make a separate one for the variable rename?
Regards,
Amit
> /* Read register and convert to mili Celsius */
> - reg = rcar_gen3_thermal_read(tsc, REG_GEN3_TEMP) & CTEMP_MASK;
> + ctemp = rcar_gen3_thermal_read(tsc, REG_GEN3_TEMP) & CTEMP_MASK;
>
> - if (reg <= thcode[tsc->id][1])
> - val = FIXPT_DIV(FIXPT_INT(reg) - tsc->coef.b1,
> + if (ctemp <= thcode[tsc->id][1])
> + val = FIXPT_DIV(FIXPT_INT(ctemp) - tsc->coef.b1,
> tsc->coef.a1);
> else
> - val = FIXPT_DIV(FIXPT_INT(reg) - tsc->coef.b2,
> + val = FIXPT_DIV(FIXPT_INT(ctemp) - tsc->coef.b2,
> tsc->coef.a2);
> mcelsius = FIXPT_TO_MCELSIUS(val);
>
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH v2] thermal: rcar_gen3_thermal: Fix undefined temperature if negative
2020-06-23 12:06 ` Amit Kucheria
@ 2020-06-25 2:06 ` Yoshihiro Shimoda
0 siblings, 0 replies; 4+ messages in thread
From: Yoshihiro Shimoda @ 2020-06-25 2:06 UTC (permalink / raw)
To: Amit Kucheria
Cc: Zhang Rui, Daniel Lezcano, niklas.soderlund+renesas, Van Do,
Dien Pham, Linux PM list, linux-renesas-soc
Hi,
> From: Amit Kucheria, Sent: Tuesday, June 23, 2020 9:07 PM
<snip>
> > --- a/drivers/thermal/rcar_gen3_thermal.c
> > +++ b/drivers/thermal/rcar_gen3_thermal.c
> > @@ -167,16 +167,16 @@ static int rcar_gen3_thermal_get_temp(void *devdata, int *temp)
> > {
> > struct rcar_gen3_thermal_tsc *tsc = devdata;
> > int mcelsius, val;
> > - u32 reg;
> > + int ctemp;
>
> Wouldn't it be better to change the variable type to fix the bug in
> this patch and make a separate one for the variable rename?
I got it. I'll keep the variable name in this patch.
Best regards,
Yoshihiro Shimoda
> Regards,
> Amit
>
> > /* Read register and convert to mili Celsius */
> > - reg = rcar_gen3_thermal_read(tsc, REG_GEN3_TEMP) & CTEMP_MASK;
> > + ctemp = rcar_gen3_thermal_read(tsc, REG_GEN3_TEMP) & CTEMP_MASK;
> >
> > - if (reg <= thcode[tsc->id][1])
> > - val = FIXPT_DIV(FIXPT_INT(reg) - tsc->coef.b1,
> > + if (ctemp <= thcode[tsc->id][1])
> > + val = FIXPT_DIV(FIXPT_INT(ctemp) - tsc->coef.b1,
> > tsc->coef.a1);
> > else
> > - val = FIXPT_DIV(FIXPT_INT(reg) - tsc->coef.b2,
> > + val = FIXPT_DIV(FIXPT_INT(ctemp) - tsc->coef.b2,
> > tsc->coef.a2);
> > mcelsius = FIXPT_TO_MCELSIUS(val);
> >
> > --
> > 2.7.4
> >
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-06-25 2:06 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-09 11:45 [PATCH v2] thermal: rcar_gen3_thermal: Fix undefined temperature if negative Yoshihiro Shimoda
2020-06-10 0:13 ` Niklas Söderlund
2020-06-23 12:06 ` Amit Kucheria
2020-06-25 2:06 ` Yoshihiro Shimoda
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).