[PATCH v2 3/3] staging: iio: light: isl29018: Use standard sysfs attributes for scale and integration time
Daniel Baluta
daniel.baluta at gmail.com
Sat Mar 14 18:54:18 EET 2015
On Sat, Mar 14, 2015 at 5:06 PM, Roberta Dobrescu
<roberta.dobrescu at gmail.com> wrote:
> This patch refactors the isl29018 driver code in order to use standard
> sysfs attributes for scale and integration time.
>
> ISL29018 light sensor uses four ranges and four ADC's resolutions
> which influence the calculated lux. Adc resolution is strongly
> connected to integration time and range should be controlled by scale.
>
> This patch introduces the usage of integration time and scale instead
> of adc resolution and range.
>
> Signed-off-by: Roberta Dobrescu <roberta.dobrescu at gmail.com>
> ---
> drivers/staging/iio/light/isl29018.c | 195 +++++++++++++++++++++++++++--------
> 1 file changed, 152 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/staging/iio/light/isl29018.c b/drivers/staging/iio/light/isl29018.c
> index ffc3d1b..78cf53b 100644
> --- a/drivers/staging/iio/light/isl29018.c
> +++ b/drivers/staging/iio/light/isl29018.c
> @@ -66,6 +66,30 @@
> #define ISL29035_BOUT_SHIFT 0x07
> #define ISL29035_BOUT_MASK (0x01 << ISL29035_BOUT_SHIFT)
>
> +#define ISL29018_INT_TIME_AVAIL "0.090000 0.005630 0.000351 0.000021"
> +#define ISL29035_INT_TIME_AVAIL "0.105000 0.006500 0.000410 0.000025"
> +#define ISL29023_INT_TIME_AVAIL "0.090000 0.005600 0.000352 0.000022"
This should be in sorted order. (ISL29035 should be after ISL29023).
> +
> +static const char * const int_time_avail[] = {ISL29018_INT_TIME_AVAIL,
> + ISL29023_INT_TIME_AVAIL,
> + ISL29035_INT_TIME_AVAIL};
> +
> +static const unsigned int isl29018_adc_bits[] = {16, 12, 8, 4};
> +
> +static const struct isl29018_int_time {
> + unsigned int time;
> + unsigned int utime;
> +} isl29018_int_times[3][4] = {
> + { {0, 90000}, {0, 5630}, {0, 351}, {0, 21} },
> + { {0, 90000}, {0, 5600}, {0, 352}, {0, 22} },
> + { {0, 105000}, {0, 6500}, {0, 410}, {0, 25} },
> + };
> +
Because the integer part is always zero, we could add a vector
called isl29018_int_utime with one miscroseconds.
> +static const struct isl29018_scale {
> + unsigned int scale;
> + unsigned int uscale;
> +} isl29018_scales[4] = { {0, 15258}, {0, 61035}, {0, 244140}, {0, 976562} };
Ditto.
> +
> struct isl29018_chip {
> struct device *dev;
> struct regmap *regmap;
> @@ -73,51 +97,65 @@ struct isl29018_chip {
> int type;
> unsigned int calibscale;
> unsigned int ucalibscale;
> - unsigned int range;
> - unsigned int adc_bit;
> + struct isl29018_int_time int_time;
> + struct isl29018_scale scale;
> int prox_scheme;
> bool suspended;
> };
>
> -static int isl29018_set_range(struct isl29018_chip *chip, unsigned long range,
> - unsigned int *new_range)
> +static int isl29018_set_integration_time(struct isl29018_chip *chip,
> + unsigned int time, unsigned int utime)
> {
> - static const unsigned long supp_ranges[] = {1000, 4000, 16000, 64000};
> - int i;
> + int i, ret;
> + struct isl29018_int_time new_int_time;
>
> - for (i = 0; i < ARRAY_SIZE(supp_ranges); ++i) {
> - if (range <= supp_ranges[i]) {
> - *new_range = (unsigned int)supp_ranges[i];
> + for (i = 0; i < ARRAY_SIZE(isl29018_int_times[chip->type]); ++i) {
> + if (time == isl29018_int_times[chip->type][i].time &&
> + utime == isl29018_int_times[chip->type][i].utime) {
> + new_int_time = isl29018_int_times[chip->type][i];
> break;
> }
> }
>
> - if (i >= ARRAY_SIZE(supp_ranges))
> + if (i >= ARRAY_SIZE(isl29018_int_times[chip->type]))
> 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,
> + i << COMMANDII_RESOLUTION_SHIFT);
> + if (ret < 0)
> + return ret;
> +
> + chip->int_time = new_int_time;
> +
> + 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); ++i) {
> + if (scale == isl29018_scales[i].scale &&
> + uscale == isl29018_scales[i].uscale) {
> + new_scale = isl29018_scales[i];
> break;
> }
> }
>
> - if (i >= ARRAY_SIZE(supp_adcbit))
> + if (i >= ARRAY_SIZE(isl29018_scales))
> 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_RANGE_MASK,
> + i << COMMANDII_RANGE_SHIFT);
> + if (ret < 0)
> + return ret;
> +
> + chip->scale = new_scale;
> +
> + return 0;
> }
>
> static int isl29018_read_sensor_input(struct isl29018_chip *chip, int mode)
> @@ -155,8 +193,9 @@ static int isl29018_read_sensor_input(struct isl29018_chip *chip, int mode)
>
> static int isl29018_read_lux(struct isl29018_chip *chip, int *lux)
> {
> - int lux_data;
> - unsigned int data_x_range, lux_unshifted;
> + int lux_data, i;
> + unsigned int data_x_range;
> + unsigned int adc_bits;
>
> lux_data = isl29018_read_sensor_input(chip, COMMMAND1_OPMODE_ALS_ONCE);
>
> @@ -168,10 +207,23 @@ static int isl29018_read_lux(struct isl29018_chip *chip, int *lux)
> * ucalibscale ranges from 0-999999, so about 20 bits. Split
> * the /1,000,000 in two to reduce the risk of over/underflow.
> */
> - data_x_range = lux_data * chip->range;
> - lux_unshifted = data_x_range * chip->calibscale;
> - lux_unshifted += data_x_range / 1000 * chip->ucalibscale / 1000;
> - *lux = lux_unshifted >> chip->adc_bit;
> +
> + for (i = 0; i < ARRAY_SIZE(isl29018_int_times[chip->type]); ++i)
> + if (chip->int_time.time == isl29018_int_times[chip->type][i].time &&
> + chip->int_time.utime == isl29018_int_times[chip->type][i].utime) {
> + adc_bits = isl29018_adc_bits[i];
> + break;
> + }
> +
> + if (i >= ARRAY_SIZE(isl29018_int_times[chip->type]))
> + return -EINVAL;
> +
> + data_x_range = lux_data * chip->scale.scale;
> + data_x_range += lux_data / 1000 * chip->scale.uscale / 1000;
> + data_x_range = data_x_range << (16 - adc_bits);
> + data_x_range *= chip->calibscale;
> + data_x_range += data_x_range / 1000 * chip->ucalibscale / 1000;
> + *lux = data_x_range;
>
> return 0;
> }
> @@ -229,7 +281,23 @@ static int isl29018_read_proximity_ir(struct isl29018_chip *chip, int scheme,
> return 0;
> }
>
> -/* Sysfs interface */
> +static ssize_t show_int_time_available(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> + struct isl29018_chip *chip = iio_priv(indio_dev);
> + int i, len = 0;
> +
> + for (i = 0; i < ARRAY_SIZE(isl29018_int_times[chip->type]); ++i)
> + len += sprintf(buf + len, "%d.%06d ",
> + isl29018_int_times[chip->type][i].time,
> + isl29018_int_times[chip->type][i].utime);
> +
> + buf[len - 1] = '\n';
> +
> + return len;
> +}
> +
> /* proximity scheme */
> static ssize_t show_prox_infrared_suppression(struct device *dev,
> struct device_attribute *attr, char *buf)
> @@ -276,11 +344,24 @@ static int isl29018_write_raw(struct iio_dev *indio_dev,
> int ret = -EINVAL;
>
> mutex_lock(&chip->lock);
> - if (mask == IIO_CHAN_INFO_CALIBSCALE && chan->type == IIO_LIGHT) {
> - chip->calibscale = val;
> - /* With no write_raw_get_fmt(), val2 is a MICRO fraction. */
> - chip->ucalibscale = val2;
> - ret = 0;
> + switch (mask) {
> + case IIO_CHAN_INFO_CALIBSCALE:
> + if (chan->type == IIO_LIGHT) {
> + chip->calibscale = val;
> + chip->ucalibscale = val2;
> + ret = 0;
> + }
> + break;
> + case IIO_CHAN_INFO_INT_TIME:
> + if (chan->type == IIO_LIGHT)
> + ret = isl29018_set_integration_time(chip, val, val2);
> + break;
> + case IIO_CHAN_INFO_SCALE:
> + if (chan->type == IIO_LIGHT)
> + ret = isl29018_set_scale(chip, val, val2);
> + break;
> + default:
> + break;
> }
> mutex_unlock(&chip->lock);
>
> @@ -321,6 +402,20 @@ static int isl29018_read_raw(struct iio_dev *indio_dev,
> if (!ret)
> ret = IIO_VAL_INT;
> break;
> + case IIO_CHAN_INFO_INT_TIME:
> + if (chan->type == IIO_LIGHT) {
> + *val = chip->int_time.time;
> + *val2 = chip->int_time.utime;
> + ret = IIO_VAL_INT_PLUS_MICRO;
> + }
> + break;
> + case IIO_CHAN_INFO_SCALE:
> + if (chan->type == IIO_LIGHT) {
> + *val = chip->scale.scale;
> + *val2 = chip->scale.uscale;
> + ret = IIO_VAL_INT_PLUS_MICRO;
> + }
> + break;
> case IIO_CHAN_INFO_CALIBSCALE:
> if (chan->type == IIO_LIGHT) {
> *val = chip->calibscale;
> @@ -340,7 +435,9 @@ static int isl29018_read_raw(struct iio_dev *indio_dev,
> .indexed = 1, \
> .channel = 0, \
> .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) | \
> - BIT(IIO_CHAN_INFO_CALIBSCALE), \
> + BIT(IIO_CHAN_INFO_CALIBSCALE) | \
> + BIT(IIO_CHAN_INFO_SCALE) | \
> + BIT(IIO_CHAN_INFO_INT_TIME), \
> }
>
> #define ISL29018_IR_CHANNEL { \
> @@ -366,6 +463,10 @@ static const struct iio_chan_spec isl29023_channels[] = {
> ISL29018_IR_CHANNEL,
> };
>
> +static IIO_DEVICE_ATTR(in_illuminance_integration_time_available, S_IRUGO,
> + show_int_time_available, NULL, 0);
> +static IIO_CONST_ATTR(in_illuminance_scale_available,
> + "0.015258 0.061035 0.244140, 0.976562");
No comma after 0.244140 please.
> static IIO_DEVICE_ATTR(proximity_on_chip_ambient_infrared_suppression,
> S_IRUGO | S_IWUSR,
> show_prox_infrared_suppression,
> @@ -373,12 +474,17 @@ static IIO_DEVICE_ATTR(proximity_on_chip_ambient_infrared_suppression,
>
> #define ISL29018_DEV_ATTR(name) (&iio_dev_attr_##name.dev_attr.attr)
> #define ISL29018_CONST_ATTR(name) (&iio_const_attr_##name.dev_attr.attr)
> +
> static struct attribute *isl29018_attributes[] = {
> + ISL29018_CONST_ATTR(in_illuminance_scale_available),
> + ISL29018_DEV_ATTR(in_illuminance_integration_time_available),
> ISL29018_DEV_ATTR(proximity_on_chip_ambient_infrared_suppression),
> NULL
> };
>
> static struct attribute *isl29023_attributes[] = {
> + ISL29018_CONST_ATTR(in_illuminance_scale_available),
> + ISL29018_DEV_ATTR(in_illuminance_integration_time_available),
> NULL
> };
>
> @@ -422,8 +528,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,14 +576,19 @@ 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_scale(chip, chip->scale.scale,
> + chip->scale.uscale);
> if (status < 0) {
> dev_err(chip->dev, "Init of isl29018 fails\n");
> return status;
> }
>
> - status = isl29018_set_resolution(chip, chip->adc_bit,
> - &new_adc_bit);
> + status = isl29018_set_integration_time(chip, chip->int_time.time,
> + chip->int_time.utime);
> + if (status < 0) {
> + dev_err(chip->dev, "Init of isl29018 fails\n");
> + return status;
> + }
>
> return 0;
> }
> @@ -609,8 +718,8 @@ static int isl29018_probe(struct i2c_client *client,
> chip->type = dev_id;
> chip->calibscale = 1;
> chip->ucalibscale = 0;
> - chip->range = 1000;
> - chip->adc_bit = 16;
> + chip->int_time = isl29018_int_times[chip->type][0];
> + chip->scale = isl29018_scales[0];
> chip->suspended = false;
>
> chip->regmap = devm_regmap_init_i2c(client,
The rest looks good to me.
thanks Roberta!
Daniel.
More information about the firefly
mailing list