linux-kernel - Re: [PATCH v3 2/2] iio: light: veml6070: add support for integration time

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20241029211303.430c13c5@jic23-huawei>
Date: Tue, 29 Oct 2024 21:13:03 +0000
From: Jonathan Cameron <jic23@...nel.org>
To: Javier Carrasco <javier.carrasco.cruz@...il.com>
Cc: Lars-Peter Clausen <lars@...afoo.de>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, linux-iio@...r.kernel.org,
 linux-kernel@...r.kernel.org, devicetree@...r.kernel.org
Subject: Re: [PATCH v3 2/2] iio: light: veml6070: add support for
 integration time

On Mon, 28 Oct 2024 18:14:02 +0100
Javier Carrasco <javier.carrasco.cruz@...il.com> wrote:

> The integration time of the veml6070 depends on an external resistor
> (called Rset in the datasheet) and the value configured in the IT
> field of the command register, whose supported values are 1/2x, 1x,
> 2x and 4x.
> 
> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@...il.com>
Minor thing inline about not necessarily papering over wrong
settings in DT.  If someone has a genuine out of range value then we don't
want to carry on with a default. If it is a dt bug then we have no idea
what the correct value is.  Either way I think printing a shouty message
and failing the probe is the way to go.

Jonathan

> ---
>  drivers/iio/light/veml6070.c | 134 ++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 126 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/iio/light/veml6070.c b/drivers/iio/light/veml6070.c
> index d11ae00f61f8..89b3ccb51515 100644
> --- a/drivers/iio/light/veml6070.c
> +++ b/drivers/iio/light/veml6070.c
> @@ -6,7 +6,7 @@
>   *
>   * IIO driver for VEML6070 (7-bit I2C slave addresses 0x38 and 0x39)
>   *
> - * TODO: integration time, ACK signal
> + * TODO: ACK signal
>   */
>  
>  #include <linux/bitfield.h>
> @@ -15,6 +15,7 @@
>  #include <linux/mutex.h>
>  #include <linux/err.h>
>  #include <linux/delay.h>
> +#include <linux/units.h>
>  
>  #include <linux/iio/iio.h>
>  #include <linux/iio/sysfs.h>
> @@ -29,18 +30,84 @@
>  #define VEML6070_COMMAND_RSRVD	BIT(1) /* reserved, set to 1 */
>  #define VEML6070_COMMAND_SD	BIT(0) /* shutdown mode when set */
>  
> -#define VEML6070_IT_10	0x01 /* integration time 1x */
> +#define VEML6070_IT_05		0x00
> +#define VEML6070_IT_10		0x01
> +#define VEML6070_IT_20		0x02
> +#define VEML6070_IT_40		0x03
> +
> +#define VEML6070_MIN_RSET_KOHM	75
> +#define VEML6070_MIN_IT_US	15625 /* Rset = 75 kohm, IT = 1/2 */
>  
>  struct veml6070_data {
>  	struct i2c_client *client1;
>  	struct i2c_client *client2;
>  	u8 config;
>  	struct mutex lock;
> +	u32 rset;
> +	int it[4][2];
>  };
>  
> +static void veml6070_calc_it(struct device *dev, struct veml6070_data *data)
> +{
> +	int i, tmp_it;
> +
> +	data->rset = 270000;
> +	device_property_read_u32(dev, "rset-ohms", &data->rset);
> +
> +	if (data->rset < 75000) {
> +		dev_warn(dev, "Rset too low, using minimum = 75 kohms\n");
> +		data->rset = 75000;
Why are we papering over a DT issue?  Are there known DT out there that hit this
we want to keep working?

If not I'd just error out on out of range values - that way they get fixed
faster!

> +	}
> +
> +	if (data->rset > 1200000) {
> +		dev_warn(dev, "Rset too high, using maximum = 1200 kohms\n");
> +		data->rset = 1200000;
> +	}
> +
> +	/*
> +	 * convert to kohm to avoid overflows and work with the same units as
> +	 * in the datasheet and simplify UVI operations.
> +	 */
> +	data->rset /= KILO;
> +
> +	tmp_it = VEML6070_MIN_IT_US * data->rset / VEML6070_MIN_RSET_KOHM;
> +	for (i = 0; i < ARRAY_SIZE(data->it); i++) {
> +		data->it[i][0] = (tmp_it << i) / MICRO;
> +		data->it[i][1] = (tmp_it << i) % MICRO;
> +	}
> +}