[PATCH v2 3/3] staging: iio: light: isl29018: Use standard sysfs attributes for range and scale

[Daniel Baluta]

On 02/19/2015 04:44 PM, Roberta Dobrescu wrote:
> This patch refactors the isl29018 driver code in order to use standard
> sysfs attributes for range and scale.
>
> ISL29018 light sensor uses four ranges and four ADC's resolutions
> which influence the calculated lux.
>
> This patch eliminates the resolution (adc bits) and introduces scale.
> Each range (1k, 4k, 16k or 64k) has a corresponding set of 4 scales
> (for 16, 12, 8 or 4 bits). Both range and scale can be changed by the
> user according to the corresponding set of values (exposed by the attributes
> in_illuminance_range_available and in_illuminance_scale_available).
>
> When the range is changed, the set of available scales is set accordingly and
> by default the scale used is the one for 16 adc bits. Scale can be changed
> anytime with one of the available values.
>

We should mention here that this patch renames lux_scale to calibscale 
and lux_uscale to ucalibscale.

Perhaps, we could do the renaming in a separate patch preceding this one.

>
> -static int isl29018_set_range(struct isl29018_chip *chip, unsigned long range,
> -		unsigned int *new_range)
> +/* when range is set, resolution is set by default at 16 bits */
> +static int isl29018_set_range(struct isl29018_chip *chip, unsigned long range)
>   {
> -	static const unsigned long supp_ranges[] = {1000, 4000, 16000, 64000};
> -	int i;
> -
> -	for (i = 0; i < ARRAY_SIZE(supp_ranges); ++i) {
> -		if (range <= supp_ranges[i]) {
> -			*new_range = (unsigned int)supp_ranges[i];
> +	int i, ret;
> +	unsigned long new_range;
> +	struct isl29018_scale new_scale;
> +
> +	for (i = 0; i < ARRAY_SIZE(isl29018_ranges); ++i) {
> +		if (range == isl29018_ranges[i]) {
> +			new_range = i;
> +			new_scale = isl29018_scales[i][0];
>   			break;
>   		}
>   	}
>
> -	if (i >= ARRAY_SIZE(supp_ranges))
> +	if (i >= ARRAY_SIZE(isl29018_ranges))
>   		return -EINVAL;
>
> -	return regmap_update_bits(chip->regmap, ISL29018_REG_ADD_COMMANDII,
> -			COMMANDII_RANGE_MASK, i << COMMANDII_RANGE_SHIFT);
> +	ret = regmap_update_bits(chip->regmap, ISL29018_REG_ADD_COMMANDII,
> +							 COMMANDII_RESOLUTION_MASK | COMMANDII_RANGE_MASK,
> +							 i << COMMANDII_RANGE_SHIFT);
> +	if (ret < 0)
> +		return ret;
> +
> +	chip->range = new_range;
> +	chip->scale = new_scale;
> +
> +	return 0;
>   }
>
> -static int isl29018_set_resolution(struct isl29018_chip *chip,
> -			unsigned long adcbit, unsigned int *conf_adc_bit)
> +static int isl29018_set_scale(struct isl29018_chip *chip, int scale, int uscale)
>   {
> -	static const unsigned long supp_adcbit[] = {16, 12, 8, 4};
> -	int i;
> +	int i, ret;
> +	struct isl29018_scale new_scale;
>
> -	for (i = 0; i < ARRAY_SIZE(supp_adcbit); ++i) {
> -		if (adcbit >= supp_adcbit[i]) {
> -			*conf_adc_bit = (unsigned int)supp_adcbit[i];
> +	for (i = 0; i < ARRAY_SIZE(isl29018_scales[chip->range]); ++i) {
> +		if (scale == isl29018_scales[chip->range][i].scale &&
> +			uscale == isl29018_scales[chip->range][i].uscale) {
> +			new_scale = isl29018_scales[chip->range][i];
>   			break;
>   		}
>   	}
>
> -	if (i >= ARRAY_SIZE(supp_adcbit))
> +	if (i >= ARRAY_SIZE(isl29018_scales[chip->range]))
>   		return -EINVAL;
>
> -	return regmap_update_bits(chip->regmap, ISL29018_REG_ADD_COMMANDII,
> -			COMMANDII_RESOLUTION_MASK,
> -			i << COMMANDII_RESOLUTION_SHIFT);
> +	ret = regmap_update_bits(chip->regmap, ISL29018_REG_ADD_COMMANDII,
> +							 COMMANDII_RESOLUTION_MASK,
> +							 i << COMMANDII_RESOLUTION_SHIFT);
> +	if (ret < 0)
> +		return ret;
> +
> +	chip->scale = new_scale;
> +
> +	return 0;
>   }
>

>
> @@ -321,10 +386,23 @@ static int isl29018_read_raw(struct iio_dev *indio_dev,
>   		if (!ret)
>   			ret = IIO_VAL_INT;
>   		break;
> +	case IIO_CHAN_INFO_RANGE:
> +		if (chan->type == IIO_LIGHT) {
> +			*val = 1000 << (chip->range * 2);
I don't understand this shouldn't this be:
*val = isl29018_ranges [chip->range] ?

> +			ret = IIO_VAL_INT;
> +		}
> +		break;

<snip>

22,8 +509,6 @@ enum {
>   static int isl29018_chip_init(struct isl29018_chip *chip)
>   {
>   	int status;
> -	unsigned int new_adc_bit;
> -	unsigned int new_range;
>
>   	if (chip->type == isl29035) {
>   		status = isl29035_detect(chip);
> @@ -472,15 +557,12 @@ static int isl29018_chip_init(struct isl29018_chip *chip)
>   	usleep_range(1000, 2000);	/* per data sheet, page 10 */
>
>   	/* set defaults */
> -	status = isl29018_set_range(chip, chip->range, &new_range);
> +	status = isl29018_set_range(chip, 1000 << (chip->range * 2));

The same here.


thanks,
Daniel.