Hi Baruch,
On Fri, 22 Dec 2017 12:14:26 +0200
Baruch Siach <baruch@xxxxxxxxxx> wrote:
> Hi Miquèl,
>
> On Fri, Dec 22, 2017 at 10:32:21AM +0100, Miquel Raynal wrote:
> > From: Baruch Siach <baruch@xxxxxxxxxx>
> >
> > The AP806 component is integrated in the Armada 8K and 7K lines of
> > processors.
> >
> > The thermal sensor sample field on the status register is a signed
> > value. Extend armada_get_temp() and the driver structure to handle
> > signed values.
> >
> > Signed-off-by: Baruch Siach <baruch@xxxxxxxxxx>
> > [<miquel.raynal@xxxxxxxxxxxxxxxxxx>: Changes when applying over the
> > previous patches, including the register names changes, also
> > switched the coefficients values to s64 instead of unsigned long to
> > deal with negative values and used do_div instead of the
> > traditionnal '/'] Signed-off-by: Miquel Raynal
> > <miquel.raynal@xxxxxxxxxxxxxxxxxx> Reviewed-by: Gregory CLEMENT
> > <gregory.clement@xxxxxxxxxxxxxxxxxx> Tested-by: Gregory CLEMENT
> > <gregory.clement@xxxxxxxxxxxxxxxxxx> ---
>
> [..]
>
> > static int armada_get_temp(struct thermal_zone_device *thermal,
> > - int *temp)
> > + int *temperature)
> > {
> > struct armada_thermal_priv *priv = thermal->devdata;
> > - unsigned long reg;
> > - unsigned long m, b, div;
> > + u32 reg, div;
> > + s64 sample, b, m;
> > + u64 tmp;
> >
> > /* Valid check */
> > if (priv->data->is_valid && !priv->data->is_valid(priv)) {
> > @@ -178,6 +197,11 @@ static int armada_get_temp(struct
> > thermal_zone_device *thermal,
> > reg = readl_relaxed(priv->status);
> > reg = (reg >> priv->data->temp_shift) &
> > priv->data->temp_mask;
> > + if (priv->data->signed_sample)
> > + /* The most significant bit is the sign bit */
> > + sample = sign_extend32(reg,
> > fls(priv->data->temp_mask) - 1);
> > + else
> > + sample = reg;
> >
> > /* Get formula coeficients */
> > b = priv->data->coef_b;
> > @@ -185,9 +209,13 @@ static int armada_get_temp(struct
> > thermal_zone_device *thermal, div = priv->data->coef_div;
> >
> > if (priv->data->inverted)
> > - *temp = ((m * reg) - b) / div;
> > + tmp = (m * sample) - b;
> > else
> > - *temp = (b - (m * reg)) / div;
> > + tmp = b - (m * sample);
> > +
> > + do_div(tmp, div);
> > + *temperature = (int)tmp;
>
> Nitpick: why not (untested)
>
> #include <linux/math64.h>
>
> if (priv->data->inverted)
> *temp = div_s64((m * sample) - b, div);
> else
> *temp = div_s64(b - (m * sample), div);
Indeed I could also use div_s64, but the result must be unsigned anyway.
But this does all the operations on the same line, maybe this is more
readable, I will update it and send (hopefully) the last version :)
Cheers,
Miquèl
>
> baruch
>
> > +
> > return 0;
> > }
>
--
Miquel Raynal, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
