[PATCH v2 0/4] iio: light: fix scale in veml6030

Javier Carrasco posted 4 patches 11 months, 1 week ago
There is a newer version of this series
drivers/iio/industrialio-gts-helper.c |   3 +-
drivers/iio/light/Kconfig             |   1 +
drivers/iio/light/veml6030.c          | 608 +++++++++++++++++-----------------
include/linux/iio/iio-gts-helper.h    |   1 +
4 files changed, 299 insertions(+), 314 deletions(-)
[PATCH v2 0/4] iio: light: fix scale in veml6030
Posted by Javier Carrasco 11 months, 1 week ago
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. In order to ease the
  calculations, iio_gts_get_total_gain() has been exported to avoid
  working directly with the scale in NANO, that would require 64-bit
  operations.

To ease the usage of the iio-gts selectors, patches 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>
---
Changes in v2:
- Rename SEL_GAIN to indicate they are in MILLI.
- Split first patch (regfields and chaching).
- Use regfield structs in chip struct instead of function pointer.
- Use total gain to derive scale, avoiding 64-bit divisions.
- Link to v1: https://lore.kernel.org/r/20250107-veml6030-scale-v1-0-1281e3ad012c@gmail.com

---
Javier Carrasco (4):
      iio: light: veml6030: extend regmap to support regfields
      iio: light: veml6030: extend regmap to support caching
      iio: gts-helper: export iio_gts_get_total_gain()
      iio: light: veml6030: fix scale to conform to ABI

 drivers/iio/industrialio-gts-helper.c |   3 +-
 drivers/iio/light/Kconfig             |   1 +
 drivers/iio/light/veml6030.c          | 608 +++++++++++++++++-----------------
 include/linux/iio/iio-gts-helper.h    |   1 +
 4 files changed, 299 insertions(+), 314 deletions(-)
---
base-commit: 577a66e2e634f712384c57a98f504c44ea4b47da
change-id: 20241231-veml6030-scale-8142f387e7e6

Best regards,
-- 
Javier Carrasco <javier.carrasco.cruz@gmail.com>
Re: [PATCH v2 0/4] iio: light: fix scale in veml6030
Posted by Jonathan Cameron 11 months ago
On Sun, 19 Jan 2025 18:31:57 +0100
Javier Carrasco <javier.carrasco.cruz@gmail.com> 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.
>   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. In order to ease the
>   calculations, iio_gts_get_total_gain() has been exported to avoid
>   working directly with the scale in NANO, that would require 64-bit
>   operations.
> 
> To ease the usage of the iio-gts selectors, patches 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>
Whilst we might consider a backport eventually.  This is a huge patch
to do that with.  Hence I'll take this the slow way for next merge window.

For now I've applied patches 1 and 2 in the interest of nibbling away
at what we will need to see again ;)

Jonathan

> ---
> Changes in v2:
> - Rename SEL_GAIN to indicate they are in MILLI.
> - Split first patch (regfields and chaching).
> - Use regfield structs in chip struct instead of function pointer.
> - Use total gain to derive scale, avoiding 64-bit divisions.
> - Link to v1: https://lore.kernel.org/r/20250107-veml6030-scale-v1-0-1281e3ad012c@gmail.com
> 
> ---
> Javier Carrasco (4):
>       iio: light: veml6030: extend regmap to support regfields
>       iio: light: veml6030: extend regmap to support caching
>       iio: gts-helper: export iio_gts_get_total_gain()
>       iio: light: veml6030: fix scale to conform to ABI
> 
>  drivers/iio/industrialio-gts-helper.c |   3 +-
>  drivers/iio/light/Kconfig             |   1 +
>  drivers/iio/light/veml6030.c          | 608 +++++++++++++++++-----------------
>  include/linux/iio/iio-gts-helper.h    |   1 +
>  4 files changed, 299 insertions(+), 314 deletions(-)
> ---
> base-commit: 577a66e2e634f712384c57a98f504c44ea4b47da
> change-id: 20241231-veml6030-scale-8142f387e7e6
> 
> Best regards,
Re: [PATCH v2 0/4] iio: light: fix scale in veml6030
Posted by Javier Carrasco 11 months ago
On Sat Jan 25, 2025 at 1:33 PM CET, Jonathan Cameron wrote:
> On Sun, 19 Jan 2025 18:31:57 +0100
> Javier Carrasco <javier.carrasco.cruz@gmail.com> 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.
> >   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. In order to ease the
> >   calculations, iio_gts_get_total_gain() has been exported to avoid
> >   working directly with the scale in NANO, that would require 64-bit
> >   operations.
> >
> > To ease the usage of the iio-gts selectors, patches 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>
> Whilst we might consider a backport eventually.  This is a huge patch
> to do that with.  Hence I'll take this the slow way for next merge window.
>
> For now I've applied patches 1 and 2 in the interest of nibbling away
> at what we will need to see again ;)
>
> Jonathan
>

Thanks, Jonathan.

Could you please tell me where you applied them, so I
can rebase onto that branch? I did not find them in linux-next/master,
iio/testing or iio/togreg. Or should they show up in one of those
within the next hours/days?

Best regards,
Javier Carrasco.