This series follows a similar approach as recently used for the veml3235
by using iio-gts to manage the scale as stated in the ABI. In its
current form, the driver exposes the hardware gain instead of the
multiplier for the raw value to obtain a value in lux.

Although this driver and the veml3235 have many similarities, there are
two main differences in this series compared to the one used to fix the
other driver:

- The veml6030 has fractional gains, which are not supported by the
  iio-gts helpers. My first attempt was adding support for them, but
  that made the whole iio-gts implementation more complex, cumbersome,
  and the risk of affecting existing clients was not negligible.
  Instead, a x8 factor has been used for the hardware gain to present
  the minimum value (x0.125) as x1, keeping linearity. The scales
  iio-gts generates are therefore right without any extra conversion,
  and they match the values provided in the different datasheets.

- This driver included a processed value for the ambient light, maybe
  because the scale did not follow the ABI and the conversion was not
  direct. To avoid breaking userspace, the functionality has been kept,
  but of course using the fixed scales. That requires using intermediate
  u64 values u64 divisions via div_u64() and do_div() to avoid overflows.

To ease the usage of the iio-gts selectors, a previous patch to support
regfields and caching has been included.

This issue has been present since the original implementation, and it
affects all devices it supports.

This series has been tested with a veml7700 (same gains as veml6030) and
a veml6035 with positive results.

Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
---
Javier Carrasco (2):
      iio: light: veml6030: extend regmap to support regfields and caching
      iio: light: veml6030: fix scale to conform to ABI

 drivers/iio/light/Kconfig    |   1 +
 drivers/iio/light/veml6030.c | 594 ++++++++++++++++++++-----------------------
 2 files changed, 282 insertions(+), 313 deletions(-)
---
base-commit: 577a66e2e634f712384c57a98f504c44ea4b47da
change-id: 20241231-veml6030-scale-8142f387e7e6

Best regards,
-- 
Javier Carrasco <javier.carrasco.cruz@gmail.com>

On 07/01/2025 22:50, Javier Carrasco wrote:
> This series follows a similar approach as recently used for the veml3235
> by using iio-gts to manage the scale as stated in the ABI. In its
> current form, the driver exposes the hardware gain instead of the
> multiplier for the raw value to obtain a value in lux.
> 
> Although this driver and the veml3235 have many similarities, there are
> two main differences in this series compared to the one used to fix the
> other driver:
> 
> - The veml6030 has fractional gains, which are not supported by the
>    iio-gts helpers. My first attempt was adding support for them, but
>    that made the whole iio-gts implementation more complex, cumbersome,
>    and the risk of affecting existing clients was not negligible.

I do agree. If one added support for fractional gains, it should be very 
very clear implementation so that even my limited capacity could handle 
it :)

>    Instead, a x8 factor has been used for the hardware gain to present
>    the minimum value (x0.125) as x1, keeping linearity. The scales
>    iio-gts generates are therefore right without any extra conversion,
>    and they match the values provided in the different datasheets.

I didn't look through the patches yet - I'm getting to there though :) 
Anyways, I assume you don't expose this HARDWAREGAIN to users?

> - This driver included a processed value for the ambient light, maybe
>    because the scale did not follow the ABI and the conversion was not
>    direct. To avoid breaking userspace, the functionality has been kept,
>    but of course using the fixed scales. That requires using intermediate
>    u64 values u64 divisions via div_u64() and do_div() to avoid overflows.
> 
> To ease the usage of the iio-gts selectors, a previous patch to support
> regfields and caching has been included.

I don't see why iio-gts would need regfields? (I have never been able to 
fully decide whether the regfields are a nice thing or not. I feel that 
in many cases regfields just add an extra layer of obfuscation while 
providing little help - but this is just my personal opinion and I'm not 
against using them. I just don't think the iio-gts needs them to be 
used. AFAIR, selectors do not need to start from zero.).

Yours,
	-- Matti

On Mon Jan 13, 2025 at 11:25 AM CET, Matti Vaittinen wrote:
> On 07/01/2025 22:50, Javier Carrasco wrote:
> > This series follows a similar approach as recently used for the veml3235
> > by using iio-gts to manage the scale as stated in the ABI. In its
> > current form, the driver exposes the hardware gain instead of the
> > multiplier for the raw value to obtain a value in lux.
> >
> > Although this driver and the veml3235 have many similarities, there are
> > two main differences in this series compared to the one used to fix the
> > other driver:
> >
> > - The veml6030 has fractional gains, which are not supported by the
> >    iio-gts helpers. My first attempt was adding support for them, but
> >    that made the whole iio-gts implementation more complex, cumbersome,
> >    and the risk of affecting existing clients was not negligible.
>
> I do agree. If one added support for fractional gains, it should be very
> very clear implementation so that even my limited capacity could handle
> it :)
>

I am working on another driver (veml6031x00, I sent a v1 with the same
flow as this one, as it inherited the gain configuration) that will
probably need some more tweaking to work with integer gains: it starts
by x0.125 like this one, but then it provides weird gains like 0.66 that
prevents a simple x8 adjustment... I will try to scale up and down instead
of adding fractional gains, though.

> >    Instead, a x8 factor has been used for the hardware gain to present
> >    the minimum value (x0.125) as x1, keeping linearity. The scales
> >    iio-gts generates are therefore right without any extra conversion,
> >    and they match the values provided in the different datasheets.
>
> I didn't look through the patches yet - I'm getting to there though :)
> Anyways, I assume you don't expose this HARDWAREGAIN to users?
>

That's right, HARDWAREGAIN is not exposed. If you believe that it should
be exposed, I am open to discuss.

> > - This driver included a processed value for the ambient light, maybe
> >    because the scale did not follow the ABI and the conversion was not
> >    direct. To avoid breaking userspace, the functionality has been kept,
> >    but of course using the fixed scales. That requires using intermediate
> >    u64 values u64 divisions via div_u64() and do_div() to avoid overflows.
> >
> > To ease the usage of the iio-gts selectors, a previous patch to support
> > regfields and caching has been included.
>
> I don't see why iio-gts would need regfields? (I have never been able to
> fully decide whether the regfields are a nice thing or not. I feel that
> in many cases regfields just add an extra layer of obfuscation while
> providing little help - but this is just my personal opinion and I'm not
> against using them. I just don't think the iio-gts needs them to be
> used. AFAIR, selectors do not need to start from zero.).
>
> Yours,
> 	-- Matti

For me, using regfields makes everything more straightforward: you
define the shifting in the configuration phase, and then you can simply
assign the selectors to the field, with the same values as the field in
question would expect for a given configuration (0 is 0, 1 is 1, and so
on). I see that easier than thinking about register-level values, that
are easier to get wrong.

Once you have defined your regfields, you don't have to think ever again
about their position. Actually, I think that iio-gts works perfect with
that approach, even if its father is not a fan of regfields ;)

Thank you for your feedback and best regards,
Javier Carrasco