[PATCH v3 05/11] thermal: armada: Add support for Armada AP806

[Baruch Siach]

Hi Miquèl,

On Mon, Dec 18, 2017 at 10:41:27AM +0100, Miquel RAYNAL wrote:
> Hello Gregory & Baruch,
> 
> On Thu, 14 Dec 2017 12:05:43 +0100
> Gregory CLEMENT <gregory.clement at free-electrons.com> wrote:
> 
> > > @@ -184,9 +214,9 @@ static int armada_get_temp(struct
> > > thermal_zone_device *thermal, div = priv->data->coef_div;
> > >  
> > >  	if (priv->data->inverted)
> > > -		*temp = ((m * reg) - b) / div;
> > > +		*temp = ((m * sample) - b) / div;
> > >  	else
> > > -		*temp = (b - (m * reg)) / div;
> > > +		*temp = (b - (m * sample)) / div;
> > >  	return 0;
> > >  }
> > >  
> > > @@ -237,6 +267,19 @@ static const struct armada_thermal_data
> > > armada380_data = { .inverted = true,
> > >  };
> > >  
> > > +static const struct armada_thermal_data armada_ap806_data = {
> > > +	.is_valid = armada_is_valid,
> > > +	.init_sensor = armada_ap806_init_sensor,
> > > +	.is_valid_bit = BIT(16),
> > > +	.temp_shift = 0,
> > > +	.temp_mask = 0x3ff,
> > > +	.coef_b = -150000,  
> > 
> > Don't you expect any side effect by storing a negative value in a
> > unsigned variable?
> 
> That is a fair question, I did not spot that.
> 
> As other values are really close to 2^32 I don't know what is the best
> option for us in this case. Should I:
> - don't care?
> - use signed values? (dangerous IMHO)
> - use a union with a signed and an unsigned value? (problem moved to
>   ->get_temp())

Another option is to use s64 type.

baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -