Re: [PATCH v1 1/1] iio: ltr501: Add light channel support

[sathyanarayanan kuppuswamy <sathyanarayanan.kuppuswamy@linux.intel.com>]

Hi Jonathan,

On 05/07/2015 03:44 PM, Jonathan Cameron wrote:
> On 01/05/15 23:39, Kuppuswamy Sathyanarayanan wrote:
>> Added support to calculate lux value from visible
>> and IR spectrum adc count values. Also added IIO_LIGHT
>> channel to enable user read the lux value directly
>> from device using illuminance input ABI.
>>
>> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> something odd going on here.
> you have changed the scan_index values, but not modified the trigger
> handler to deal with the change.  Note these only have relevant in
> buffered mode.  If you want this to only be exposed via sysfs, that
> is fine (and perhaps reasonable) then set the scan_index to -1 (iirc)
> and leave the other channels alone.
Thanks for the review. Yes, I agree that scan_index change is not
required for this feature. I intended to just add the raw read support
for this channel.
>
> Jonathan
>> ---
>>   drivers/iio/light/ltr501.c | 71 +++++++++++++++++++++++++++++++++++++++++-----
>>   1 file changed, 64 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/iio/light/ltr501.c b/drivers/iio/light/ltr501.c
>> index ca4bf47..242cf20 100644
>> --- a/drivers/iio/light/ltr501.c
>> +++ b/drivers/iio/light/ltr501.c
>> @@ -66,6 +66,9 @@
>>   
>>   #define LTR501_REGMAP_NAME "ltr501_regmap"
>>   
>> +#define LTR501_LUX_CONV(vis_coeff, vis_data, ir_coeff, ir_data) \
>> +			((vis_coeff * vis_data) - (ir_coeff * ir_data))
>> +
>>   static const int int_time_mapping[] = {100000, 50000, 200000, 400000};
>>   
>>   static const struct reg_field reg_field_it =
>> @@ -298,6 +301,29 @@ static int ltr501_ps_read_samp_period(struct ltr501_data *data, int *val)
>>   	return IIO_VAL_INT;
>>   }
>>   
>> +/* IR and visible spectrum coeff's are given in data sheet */
>> +static unsigned long ltr501_calculate_lux(u16 vis_data, u16 ir_data)
>> +{
>> +	unsigned long ratio, lux;
>> +
>> +	if (vis_data == 0)
>> +		return 0;
>> +
>> +	/* multiply numerator by 100 to avoid handling ratio < 1 */
>> +	ratio = DIV_ROUND_UP(ir_data * 100, ir_data + vis_data);
>> +
>> +	if (ratio < 45)
>> +		lux = LTR501_LUX_CONV(1774, vis_data, -1105, ir_data);
>> +	else if (ratio >= 45 && ratio < 64)
>> +		lux = LTR501_LUX_CONV(3772, vis_data, 1336, ir_data);
>> +	else if (ratio >= 64 && ratio < 85)
>> +		lux = LTR501_LUX_CONV(1690, vis_data, 169, ir_data);
>> +	else
>> +		lux = 0;
>> +
>> +	return lux / 1000;
>> +}
>> +
>>   static int ltr501_drdy(struct ltr501_data *data, u8 drdy_mask)
>>   {
>>   	int tries = 100;
>> @@ -548,11 +574,24 @@ static const struct iio_event_spec ltr501_pxs_event_spec[] = {
>>   	.num_event_specs = _evsize,\
>>   }
>>   
>> +#define LTR501_LIGHT_CHANNEL(_idx) { \
>> +	.type = IIO_LIGHT, \
>> +	.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED), \
>> +	.scan_index = 0, \
>> +	.scan_type = { \
>> +		.sign = 'u', \
>> +		.realbits = 16, \
>> +		.storagebits = 16, \
>> +		.endianness = IIO_CPU, \
>> +	}, \
>> +}
>> +
>>   static const struct iio_chan_spec ltr501_channels[] = {
>> -	LTR501_INTENSITY_CHANNEL(0, LTR501_ALS_DATA0, IIO_MOD_LIGHT_BOTH, 0,
>> +	LTR501_LIGHT_CHANNEL(0),
>> +	LTR501_INTENSITY_CHANNEL(1, LTR501_ALS_DATA0, IIO_MOD_LIGHT_BOTH, 0,
>>   				 ltr501_als_event_spec,
>>   				 ARRAY_SIZE(ltr501_als_event_spec)),
>> -	LTR501_INTENSITY_CHANNEL(1, LTR501_ALS_DATA1, IIO_MOD_LIGHT_IR,
>> +	LTR501_INTENSITY_CHANNEL(2, LTR501_ALS_DATA1, IIO_MOD_LIGHT_IR,
>>   				 BIT(IIO_CHAN_INFO_SCALE) |
>>   				 BIT(IIO_CHAN_INFO_INT_TIME) |
>>   				 BIT(IIO_CHAN_INFO_SAMP_FREQ),
>> @@ -562,7 +601,7 @@ static const struct iio_chan_spec ltr501_channels[] = {
>>   		.address = LTR501_PS_DATA,
>>   		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
>>   			BIT(IIO_CHAN_INFO_SCALE),
>> -		.scan_index = 2,
>> +		.scan_index = 3,
>>   		.scan_type = {
>>   			.sign = 'u',
>>   			.realbits = 11,
>> @@ -572,19 +611,20 @@ static const struct iio_chan_spec ltr501_channels[] = {
>>   		.event_spec = ltr501_pxs_event_spec,
>>   		.num_event_specs = ARRAY_SIZE(ltr501_pxs_event_spec),
>>   	},
>> -	IIO_CHAN_SOFT_TIMESTAMP(3),
>> +	IIO_CHAN_SOFT_TIMESTAMP(4),
>>   };
>>   
>>   static const struct iio_chan_spec ltr301_channels[] = {
>> -	LTR501_INTENSITY_CHANNEL(0, LTR501_ALS_DATA0, IIO_MOD_LIGHT_BOTH, 0,
>> +	LTR501_LIGHT_CHANNEL(0),
>> +	LTR501_INTENSITY_CHANNEL(1, LTR501_ALS_DATA0, IIO_MOD_LIGHT_BOTH, 0,
>>   				 ltr501_als_event_spec,
>>   				 ARRAY_SIZE(ltr501_als_event_spec)),
>> -	LTR501_INTENSITY_CHANNEL(1, LTR501_ALS_DATA1, IIO_MOD_LIGHT_IR,
>> +	LTR501_INTENSITY_CHANNEL(2, LTR501_ALS_DATA1, IIO_MOD_LIGHT_IR,
>>   				 BIT(IIO_CHAN_INFO_SCALE) |
>>   				 BIT(IIO_CHAN_INFO_INT_TIME) |
>>   				 BIT(IIO_CHAN_INFO_SAMP_FREQ),
>>   				 NULL, 0),
>> -	IIO_CHAN_SOFT_TIMESTAMP(2),
>> +	IIO_CHAN_SOFT_TIMESTAMP(3),
>>   };
>>   
>>   static int ltr501_read_raw(struct iio_dev *indio_dev,
>> @@ -596,6 +636,23 @@ static int ltr501_read_raw(struct iio_dev *indio_dev,
>>   	int ret, i;
>>   
>>   	switch (mask) {
>> +	case IIO_CHAN_INFO_PROCESSED:
>> +		if (iio_buffer_enabled(indio_dev))
>> +			return -EBUSY;
>> +
>> +		switch (chan->type) {
>> +		case IIO_LIGHT:
>> +			mutex_lock(&data->lock_als);
>> +			ret = ltr501_read_als(data, buf);
>> +			mutex_unlock(&data->lock_als);
>> +			if (ret < 0)
>> +				return ret;
>> +			*val = ltr501_calculate_lux(le16_to_cpu(buf[1]),
>> +						    le16_to_cpu(buf[0]));
>> +			return IIO_VAL_INT;
>> +		default:
>> +			return -EINVAL;
>> +		}
>>   	case IIO_CHAN_INFO_RAW:
>>   		if (iio_buffer_enabled(indio_dev))
>>   			return -EBUSY;
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

-- 
Sathyanarayanan Kuppuswamy
Android kernel developer