Hi Niklas,
On Wed, Mar 20, 2024 at 2:22 PM Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:
> On Thu, Mar 7, 2024 at 12:03 PM Niklas Söderlund
> <niklas.soderlund+renesas@xxxxxxxxxxxx> wrote:
> > The initial driver used a formula to approximation the temperature and
> > register value reversed engineered form an out-of-tree BSP driver. This
> > was needed as the datasheet at the time did not contain any information
> > on how to do this. Later Gen3 (Rev 2.30) and Gen4 (all) now contains
> > this information.
> >
> > Update the approximation formula to use the datasheets information
> > instead of the reversed engineered one.
>
> > On an idle M3-N without fused calibration values for PTAT and THCODE the
> > old formula reports,
> >
> > zone0: 52000
> > zone1: 53000
> > zone2: 52500
> >
> > While the new formula under the same circumstances reports,
> >
> > zone0: 52500
> > zone1: 54000
> > zone2: 54000
> >
> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx>
>
> > --- a/drivers/thermal/rcar_gen3_thermal.c
> > +++ b/drivers/thermal/rcar_gen3_thermal.c
> > @@ -172,19 +165,29 @@ static int rcar_gen3_thermal_round(int temp)
> > static int rcar_gen3_thermal_get_temp(struct thermal_zone_device *tz, int *temp)
> > {
> > struct rcar_gen3_thermal_tsc *tsc = thermal_zone_device_priv(tz);
> > - int mcelsius, val;
> > - int reg;
> > + struct rcar_gen3_thermal_priv *priv = tsc->priv;
> > + const struct equation_set_coef *coef;
> > + int adj, mcelsius, reg, thcode;
> >
> > /* Read register and convert to mili Celsius */
> > reg = rcar_gen3_thermal_read(tsc, REG_GEN3_TEMP) & CTEMP_MASK;
> >
> > - if (reg <= tsc->thcode[1])
> > - val = FIXPT_DIV(FIXPT_INT(reg) - tsc->coef.b1,
> > - tsc->coef.a1);
> > - else
> > - val = FIXPT_DIV(FIXPT_INT(reg) - tsc->coef.b2,
> > - tsc->coef.a2);
> > - mcelsius = FIXPT_TO_MCELSIUS(val);
> > + if (reg < tsc->thcode[1]) {
> > + adj = priv->info->adj_below;
> > + coef = &tsc->coef.below;
> > + thcode = tsc->thcode[2];
> > + } else {
> > + adj = priv->info->adj_above;
> > + coef = &tsc->coef.above;
> > + thcode = tsc->thcode[0];
> > + }
> > +
> > + /*
> > + * The dividend can't be grown as it might overflow, instead shorten the
> > + * divisor to convert to millidegree Celsius. If we convert after the
> > + * division precision is lost to a full degree Celsius.
> > + */
> > + mcelsius = DIV_ROUND_CLOSEST(coef->a * (thcode - reg), coef->b / 1000) + adj * 1000;
>
> Don't you lose a lot of precision by pre-dividing b by 1000?
After reading PATCH 3/3: instead of calculating millidegree Celsius,
and rounding to a granularity of 0.1 degree Celsius later, perhaps it
makes more sense to calculate decidegree Celsius first (still avoiding
overflow?), and multiply by 100 later?
Then rcar_gen3_thermal_round() can be removed.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
