linux-kernel - Re: [PATCH 0/2] iio: light: fix scale in veml6030

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <25d5ff51-8a1c-4e34-9f0e-e11778313d8c@gmail.com>
Date: Mon, 13 Jan 2025 12:25:52 +0200
From: Matti Vaittinen <mazziesaccount@...il.com>
To: Javier Carrasco <javier.carrasco.cruz@...il.com>,
 Jonathan Cameron <jic23@...nel.org>, Lars-Peter Clausen <lars@...afoo.de>,
 Rishi Gupta <gupt21@...il.com>
Cc: linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org,
 Jonathan Cameron <Jonathan.Cameron@...wei.com>
Subject: Re: [PATCH 0/2] iio: light: fix scale in veml6030

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