Re: [PATCH 2/2] iio: veml3235: fix scale to conform to ABI

["Javier Carrasco" <javier.carrasco.cruz@gmail.com>]

On Sun Dec 22, 2024 at 2:43 PM CET, Matti Vaittinen wrote:
> On 20/12/2024 21:28, Javier Carrasco wrote:
> > The current scale is not ABI-compliant as it is just the sensor gain
> > instead of the value that acts as a multiplier to be applied to the raw
> > value (there is no offset).
> >
> > Use the iio-gts helpers to obtain the proper scale values according to
> > the gain and integration time to match the resolution tables from the
> > datasheet.
> >
> > Fixes: c5a23f80c164 ("iio: light: add support for veml3235")
> > Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
> > ---
>
> ...
>
> > +static const struct iio_itime_sel_mul veml3235_it_sel[] = {
> > +	GAIN_SCALE_ITIME_US(50000, 0, 1),
> > +	GAIN_SCALE_ITIME_US(100000, 1, 2),
> > +	GAIN_SCALE_ITIME_US(200000, 2, 4),
> > +	GAIN_SCALE_ITIME_US(400000, 3, 8),
> > +	GAIN_SCALE_ITIME_US(800000, 4, 16),
> >   };
> >
> > -static const int veml3235_scale_vals[] = { 1, 2, 4, 8 };
> > +/*
> > + * The MSB (DG) doubles the value of the rest of the field, which leads to
> > + * two possible combinations to obtain gain = 2 and gain = 4. The gain
> > + * handlding can be simplified by restricting DG = 1 to the only gain that
> > + * really requires it, gain = 8. Note that "X10" is a reserved value.
>
> Just a question - do you ensure there is no "invalid" register values? I
> think Jonathan has prefered doing this by writing known initialization
> values at probe.
>
> I *think* the GTS should survive cases where multiple bit patterns can
> be used to represent same gain/time - but I don't remember this for sure.
>
> I didn't have the time to do a proper thorough review - sorry. Still,
> what I browsed quickly looked good :) Thanks!
>
> Yours,
> 	-- Matti

Hi Matti,

Thanks for your feedback. The initial value of this register is indeed
set during the initialization (default is gain = 1). That comment is
only to explain why some combinations are missing, and why only one bit
patter per combination is required. The user does not care which one is
used, and it should be transparent to them.

I just noticed that there is a small typo in that comment (handlding
instead of handling), and I will update it for v2 after I collect more
feedback.

Best regards,
Javier Carrasco