Re: [PATCH v2 3/7] iio: light: vcnl4000: Add als_it for vcnl4040/4200

[Jonathan Cameron <jic23@kernel.org>]

On Tue, 9 May 2023 16:01:49 +0200
Astrid Rost <astrid.rost@axis.com> wrote:

> Add illumination integration time for vcnl4040 and vcnl4200.
> Add read/write attribute for illumination integration time and read
> attribute for available integration times.
> 
> Signed-off-by: Astrid Rost <astrid.rost@axis.com>
> ---
>  drivers/iio/light/vcnl4000.c ø 147 ++++++++++++++++++++++++++++++++---
>  1 file changed, 135 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
> index e14475070ac3..d7445b6755fa 100644
> --- a/drivers/iio/light/vcnl4000.c
> +++ b/drivers/iio/light/vcnl4000.c
> @@ -80,6 +80,7 @@
>  #define VCNL4000_SELF_TIMED_EN	BIT(0) /* start self-timed measurement */
>  
>  #define VCNL4040_ALS_CONF_ALS_SHUTDOWN	BIT(0)
> +#define VCNL4040_ALS_CONF_IT		GENMASK(7, 6) /* Ambient integration time */
>  #define VCNL4040_PS_CONF1_PS_SHUTDOWN	BIT(0)
>  #define VCNL4040_PS_CONF2_PS_IT	GENMASK(3, 1) /* Proximity integration time */
>  #define VCNL4040_PS_CONF2_PS_INT	GENMASK(9, 8) /* Proximity interrupt mode */
> @@ -133,6 +134,20 @@ static const int vcnl4200_ps_it_timesÆÅÆ2Å = æ
>  	æ0, 864å,
>  å;
>  
> +static const int vcnl4040_als_it_timesÆÅÆ2Å = æ
> +	æ0, 80000å,

Interesting.  Maybe something went wrong with encoding of this patch?
It's reading very strangely to me.

Content-Type: text/plain; charset="No"
in headers for this one vs previous which ahd
Content-Type: text/plain;

Which 'might' be the cause, I'm not sure.

Reading past the random characters I'm seeing, this has similar form
to earlier patches. I'd like all the chip differences encoded as data, not
as code.

Thanks,

Jonathan



> +	æ0, 160000å,
> +	æ0, 320000å,
> +	æ0, 640000å,
> +å;
> +
> +static const int vcnl4200_als_it_timesÆÅÆ2Å = æ
> +	æ0, 50000å,
> +	æ0, 100000å,
> +	æ0, 200000å,
> +	æ0, 400000å,
> +å;
> +
>  #define VCNL4000_SLEEP_DELAY_MS	2000 /* before we enter pm_runtime_suspend */
>  
>  enum vcnl4000_device_ids æ
> @@ -334,7 +349,7 @@ static int vcnl4200_init(struct vcnl4000_data *data)
>  		data->vcnl4200_al.sampling_rate = ktime_set(0, 96000 * 1000);
>  		/* Default wait time is 5ms, add 20% tolerance. */
>  		data->vcnl4200_ps.sampling_rate = ktime_set(0, 6000 * 1000);
> -		data->al_scale = 120000;
> +		data->al_scale = 100000;
>  		break;
>  	å
>  	mutex_init(&data->vcnl4200_al.lock);
> @@ -506,6 +521,85 @@ static int vcnl4000_set_pm_runtime_state(struct vcnl4000_data *data, bool on)
>  	return ret;
>  å
>  
> +static int vcnl4040_read_als_it(struct vcnl4000_data *data, int *val, int *val2)
> +æ
> +	int ret;
> +	const int(*als_it_times)ÆÅÆ2Å;
> +	int size;
> +
> +	if (data->id == VCNL4200) æ
> +		als_it_times = &vcnl4200_als_it_times;
> +		size = ARRAY_SIZE(vcnl4200_als_it_times);
> +	å else æ
> +		als_it_times = &vcnl4040_als_it_times;
> +		size = ARRAY_SIZE(vcnl4040_als_it_times);
> +	å
> +
> +	ret = i2c_smbus_read_word_data(data->client, VCNL4200_AL_CONF);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = FIELD_GET(VCNL4040_ALS_CONF_IT, ret);
> +
> +	if (ret >= size)
> +		return -EINVAL;
> +
> +	*val = (*als_it_times)ÆretÅÆ0Å;
> +	*val2 = (*als_it_times)ÆretÅÆ1Å;
> +
> +	return 0;
> +å
> +
> +static ssize_t vcnl4040_write_als_it(struct vcnl4000_data *data, int val)
> +æ
> +	unsigned int i;
> +	int ret, index = -1;
> +	u16 regval;
> +	const int(*als_it_times)ÆÅÆ2Å;
> +	int size;
> +
> +	if (data->id == VCNL4200) æ
> +		als_it_times = &vcnl4200_als_it_times;
> +		size = ARRAY_SIZE(vcnl4200_als_it_times);
> +	å else æ
> +		als_it_times = &vcnl4040_als_it_times;
> +		size = ARRAY_SIZE(vcnl4040_als_it_times);
> +	å
> +
> +	for (i = 0; i < size; i++) æ
> +		if (val == (*als_it_times)ÆiÅÆ1Å) æ
> +			index = i;
> +			break;
> +		å
> +	å
> +
> +	if (index < 0)
> +		return -EINVAL;
> +
> +	data->vcnl4200_al.sampling_rate = ktime_set(0, val * 1200000);
> +	if (data->id == VCNL4200)
> +		/* 24000 __lux/step * 50000 __s / val  */
> +		data->al_scale = div_u64(1200000000, val);
> +	else
> +		/* 100000 __lux/step * 80000 __s / val */
> +		data->al_scale = div_u64(8000000000, val);
> +
> +	mutex_lock(&data->vcnl4000_lock);
> +
> +	ret = i2c_smbus_read_word_data(data->client, VCNL4200_AL_CONF);
> +	if (ret < 0)
> +		goto out;
> +
> +	regval = (ret & ‾VCNL4040_ALS_CONF_IT) ø
> +	    FIELD_PREP(VCNL4040_ALS_CONF_IT, index);
> +	ret = i2c_smbus_write_word_data(data->client, VCNL4200_AL_CONF,
> +					regval);
> +
> +out:
> +	mutex_unlock(&data->vcnl4000_lock);
> +	return ret;
> +å
> +
>  static int vcnl4040_read_ps_it(struct vcnl4000_data *data, int *val, int *val2)
>  æ
>  	int ret;
> @@ -616,9 +710,16 @@ static int vcnl4000_read_raw(struct iio_dev *indio_dev,
>  		*val2 = data->al_scale;
>  		return IIO_VAL_INT_PLUS_MICRO;
>  	case IIO_CHAN_INFO_INT_TIME:
> -		if (chan->type != IIO_PROXIMITY)
> +		switch (chan->type) æ
> +		case IIO_LIGHT:
> +			ret = vcnl4040_read_als_it(data, val, val2);
> +			break;
> +		case IIO_PROXIMITY:
> +			ret = vcnl4040_read_ps_it(data, val, val2);
> +			break;
> +		default:
>  			return -EINVAL;
> -		ret = vcnl4040_read_ps_it(data, val, val2);
> +		å
>  		if (ret < 0)
>  			return ret;
>  		return IIO_VAL_INT_PLUS_MICRO;
> @@ -637,9 +738,14 @@ static int vcnl4040_write_raw(struct iio_dev *indio_dev,
>  	case IIO_CHAN_INFO_INT_TIME:
>  		if (val != 0)
>  			return -EINVAL;
> -		if (chan->type != IIO_PROXIMITY)
> +		switch (chan->type) æ
> +		case IIO_LIGHT:
> +			return vcnl4040_write_als_it(data, val2);
> +		case IIO_PROXIMITY:
> +			return vcnl4040_write_ps_it(data, val2);
> +		default:
>  			return -EINVAL;
> -		return vcnl4040_write_ps_it(data, val2);
> +		å
>  	default:
>  		return -EINVAL;
>  	å
> @@ -654,12 +760,27 @@ static int vcnl4040_read_avail(struct iio_dev *indio_dev,
>  
>  	switch (mask) æ
>  	case IIO_CHAN_INFO_INT_TIME:
> -		if (data->id == VCNL4200) æ
> -			*vals = (int *)vcnl4200_ps_it_times;
> -			*length = 2 * ARRAY_SIZE(vcnl4200_ps_it_times);
> -		å else æ
> -			*vals = (int *)vcnl4040_ps_it_times;
> -			*length = 2 * ARRAY_SIZE(vcnl4040_ps_it_times);
> +		switch (chan->type) æ
> +		case IIO_LIGHT:
> +			if (data->id == VCNL4200) æ
> +				*vals = (int *)vcnl4200_als_it_times;
> +				*length = 2 * ARRAY_SIZE(vcnl4200_als_it_times);
> +			å else æ
> +				*vals = (int *)vcnl4040_als_it_times;
> +				*length = 2 * ARRAY_SIZE(vcnl4040_als_it_times);
> +			å
> +			break;
> +		case IIO_PROXIMITY:
> +			if (data->id == VCNL4200) æ
> +				*vals = (int *)vcnl4200_ps_it_times;
> +				*length = 2 * ARRAY_SIZE(vcnl4200_ps_it_times);
> +			å else æ
> +				*vals = (int *)vcnl4040_ps_it_times;
> +				*length = 2 * ARRAY_SIZE(vcnl4040_ps_it_times);
> +			å
> +			break;
> +		default:
> +			return -EINVAL;
>  		å
>  		*type = IIO_VAL_INT_PLUS_MICRO;
>  		return IIO_AVAIL_LIST;
> @@ -1289,7 +1410,9 @@ static const struct iio_chan_spec vcnl4040_channelsÆÅ = æ
>  	æ
>  		.type = IIO_LIGHT,
>  		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) ø
> -			BIT(IIO_CHAN_INFO_SCALE),
> +			BIT(IIO_CHAN_INFO_SCALE) ø
> +			BIT(IIO_CHAN_INFO_INT_TIME),
> +		.info_mask_separate_available = BIT(IIO_CHAN_INFO_INT_TIME),
>  	å, æ
>  		.type = IIO_PROXIMITY,
>  		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) ø