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

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

On Tue Jun 2, 2026 at 2:38 PM CEST, Jonathan Cameron wrote:
>> >> +/*
>> >> + * 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.
>

I thought that MILLI_GAIN was already documenting what x125 is: 0.125 =
125 milli. More than the base gain of 1, it is the lowest gain you can
configure.

>>
>> >> +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),
>> >> +};
>> >
>> > ...