Re: [PATCH v4 2/4] iio: light: add support for veml6031x00 ALS series

[Jonathan Cameron]

> >> +/*
> >> + * The gain selector encodes (PD_D4 << 2) | GAIN to identify each gain setting.
> >> + * Gains are multiplied by 8 to work with integers. The values in the iio-gts
> >> + * tables don't need corrections because the maximum value of the scale refers
> >> + * to GAIN = x1, and the rest of the values are obtained from the resulting
> >> + * linear function.
> >> + * TODO: add support for MILLI_GAIN_X165 and MILLI_GAIN_X660
> >> + */
> >> +#define VEML6031X00_SEL_MILLI_GAIN_X125  0x07
> >> +#define VEML6031X00_SEL_MILLI_GAIN_X250  0x04
> >> +#define VEML6031X00_SEL_MILLI_GAIN_X500  0x03
> >> +#define VEML6031X00_SEL_MILLI_GAIN_X1000 0x00
> >> +#define VEML6031X00_SEL_MILLI_GAIN_X2000 0x01  
> >
> > Not sure if these one-time use definitions improve or not the readability
> > of the code. Up to Jonathan.
> >  
> 
> I prefer these definitions, and a similar pattern is used in multiple
> drivers in IIO, but I have no strong feelings about it.

Looking again at this, what do the numbers in the defines actually mean?
Seems a bit odd to have the base gain of 1 being called X125.
Maybe a comment on that would be useful.  I don't mind either way
on defines for this but if that number is useful to have I'd rather
have a define than a comment on each line.

> 
> >> +static const struct iio_gain_sel_pair veml6031x00_gain_sel[] = {
> >> +	GAIN_SCALE_GAIN(1, VEML6031X00_SEL_MILLI_GAIN_X125),
> >> +	GAIN_SCALE_GAIN(2, VEML6031X00_SEL_MILLI_GAIN_X250),
> >> +	GAIN_SCALE_GAIN(4, VEML6031X00_SEL_MILLI_GAIN_X500),
> >> +	GAIN_SCALE_GAIN(8, VEML6031X00_SEL_MILLI_GAIN_X1000),
> >> +	GAIN_SCALE_GAIN(16, VEML6031X00_SEL_MILLI_GAIN_X2000),
> >> +};  
> >
> > ...