Re: [PATCH 4/4] iio: light: veml6070: add support for integration time

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

On 17/10/2024 23:39, Javier Carrasco 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.
> 
> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
> ---
>  drivers/iio/light/veml6070.c | 109 ++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 107 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/light/veml6070.c b/drivers/iio/light/veml6070.c
> index d11ae00f61f8..87dc3295a656 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>
> @@ -29,15 +29,78 @@
>  #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 it[4][2];
>  };
>  
> +static void veml6070_calc_it(struct device *dev, struct veml6070_data *data)
> +{
> +	u32 rset, tmp_it;
> +	int i, ret;
> +
> +	ret = device_property_read_u32(dev, "rset-kohms", &rset);
> +	if (ret) {
> +		dev_warn(dev, "no Rset specified, using default 300 kohms\n");
> +		rset = 300;
> +	}
> +
> +	if (rset < 75) {
> +		dev_warn(dev, "Rset too low, using minimum = 75 kohms\n");
> +		rset = 75;
> +	}
> +
> +	if (rset > 1200) {
> +		dev_warn(dev, "Rset too high, using maximum = 1200 kohms\n");
> +		rset = 1200;
> +	}
> +
> +	tmp_it = VEML6070_MIN_IT_US * (rset / VEML6070_MIN_RSET_KOHM);
> +	for (i = 0; i < ARRAY_SIZE(data->it); i++) {
> +		data->it[i][0] = (tmp_it << i) / 1000000;
> +		data->it[i][1] = (tmp_it << i) % 1000000;
> +	}
> +}
> +
> +static int veml6070_get_it(struct veml6070_data *data, int *val, int *val2)
> +{
> +	int it_idx = FIELD_GET(VEML6070_COMMAND_IT, data->config);
> +
> +	*val = data->it[it_idx][0];
> +	*val2 = data->it[it_idx][1];
> +
> +	return IIO_VAL_INT_PLUS_MICRO;
> +}
> +
> +static int veml6070_set_it(struct veml6070_data *data, int val, int val2)
> +{
> +	int it_idx;
> +
> +	for (it_idx = 0; it_idx < ARRAY_SIZE(data->it); it_idx++) {
> +		if (data->it[it_idx][0] == val && data->it[it_idx][1] == val2)
> +			break;
> +	}
> +
> +	if (it_idx >= ARRAY_SIZE(data->it))
> +		return -EINVAL;
> +
> +	data->config = (data->config & ~VEML6070_COMMAND_IT) |
> +		FIELD_PREP(VEML6070_COMMAND_IT, it_idx);
> +
> +	return i2c_smbus_write_byte(data->client1, data->config);
> +}
> +
>  static int veml6070_read(struct veml6070_data *data)
>  {
>  	int ret;
> @@ -81,10 +144,14 @@ static const struct iio_chan_spec veml6070_channels[] = {
>  		.modified = 1,
>  		.channel2 = IIO_MOD_LIGHT_UV,
>  		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> +		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_INT_TIME),
> +		.info_mask_shared_by_all_available = BIT(IIO_CHAN_INFO_INT_TIME),
>  	},
>  	{
>  		.type = IIO_UVINDEX,
>  		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
> +		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_INT_TIME),
> +		.info_mask_shared_by_all_available = BIT(IIO_CHAN_INFO_INT_TIME),
>  	}
>  };
>  
> @@ -127,6 +194,40 @@ static int veml6070_read_raw(struct iio_dev *indio_dev,
>  		else
>  			*val = ret;
>  		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_INT_TIME:
> +		return veml6070_get_it(data, val, val2);
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int veml6070_read_avail(struct iio_dev *indio_dev,
> +			       struct iio_chan_spec const *chan,
> +			       const int **vals, int *type, int *length,
> +			       long mask)
> +{
> +	struct veml6070_data *data = iio_priv(indio_dev);
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_INT_TIME:
> +		*vals = (int *)data->it;
> +		*length = 2 * ARRAY_SIZE(data->it);
> +		*type = IIO_VAL_INT_PLUS_MICRO;
> +		return IIO_AVAIL_LIST;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int veml6070_write_raw(struct iio_dev *indio_dev,
> +			      struct iio_chan_spec const *chan,
> +			      int val, int val2, long mask)
> +{
> +	struct veml6070_data *data = iio_priv(indio_dev);
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_INT_TIME:
> +		return veml6070_set_it(data, val, val2);
>  	default:
>  		return -EINVAL;
>  	}
> @@ -134,6 +235,8 @@ static int veml6070_read_raw(struct iio_dev *indio_dev,
>  

I just noticed that the processed data needs to be updated to take into
account different integration times because in its current form it
provides fixed values for Rset = 270kohm, IT=1T. I will add it to v2,
which is required anyway to address changes in the dt-bindings.