On 18/04/15 06:15, Kuppuswamy Sathyanarayanan wrote:
> Added support to modify and read ALS integration time.
>
> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx>
Cool, didn't know about this regmap field stuff before. Not exactly heavily
used but looks helpful!
A few questions inline..
1) A spot where a variable name change would make it easier to follow.
2) Why are the struct reg_field not defined as const in the regmap_field
allocators? Here it gives the impression we are restricting ourselves to
one instance of this chip, but in reality they seem to actually be const
so we aren't.
messy :(
> ---
> drivers/iio/light/ltr501.c | 87 +++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 86 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iio/light/ltr501.c b/drivers/iio/light/ltr501.c
> index 84ee2b3..22769c8 100644
> --- a/drivers/iio/light/ltr501.c
> +++ b/drivers/iio/light/ltr501.c
> @@ -28,6 +28,7 @@
>
> #define LTR501_ALS_CONTR 0x80 /* ALS operation mode, SW reset */
> #define LTR501_PS_CONTR 0x81 /* PS operation mode */
> +#define LTR501_ALS_MEAS_RATE 0x85 /* ALS integ time, measurement rate*/
> #define LTR501_PART_ID 0x86
> #define LTR501_MANUFAC_ID 0x87
> #define LTR501_ALS_DATA1 0x88 /* 16-bit, little endian */
> @@ -49,11 +50,16 @@
>
> #define LTR501_REGMAP_NAME "ltr501_regmap"
>
> +static int int_time_mapping[] = {100000, 50000, 200000, 400000};
> +
> +static struct reg_field reg_it = REG_FIELD(LTR501_ALS_MEAS_RATE, 3, 4);
Naming could be clearer. The reg_it here and the regmap_field below
are different structures, but their name doesn't make this clear.
Also, why is the above not const? Obviously regmap doesn't take a const
but I can't see why not... Mark?
It is only used to initialize elements (by copying) in the regmap_field
that allocator of which it is passed to.
> +
> struct ltr501_data {
> struct i2c_client *client;
> struct mutex lock_als, lock_ps;
> u8 als_contr, ps_contr;
> struct regmap *regmap;
> + struct regmap_field *reg_it;
> };
>
> static int ltr501_drdy(struct ltr501_data *data, u8 drdy_mask)
> @@ -74,6 +80,58 @@ static int ltr501_drdy(struct ltr501_data *data, u8 drdy_mask)
> return -EIO;
> }
>
> +static int ltr501_set_it_time(struct ltr501_data *data, int it)
> +{
> + int ret, i, index = -1, status;
> +
> + for (i = 0; i < ARRAY_SIZE(int_time_mapping); i++) {
> + if (int_time_mapping[i] == it) {
> + index = i;
> + break;
> + }
> + }
> + /* Make sure integ time index is valid */
> + if (index < 0)
> + return -EINVAL;
> +
> + ret = regmap_read(data->regmap, LTR501_ALS_CONTR, &status);
> + if (ret < 0)
> + return ret;
> +
> + if (status & LTR501_CONTR_ALS_GAIN_MASK) {
> + /*
> + * 200 ms and 400 ms integ time can only be
> + * used in dynamic range 1
> + */
> + if (index > 1)
> + return -EINVAL;
> + } else
> + /* 50 ms integ time can only be used in dynamic range 2 */
> + if (index == 1)
> + return -EINVAL;
> +
> + return regmap_field_write(data->reg_it, index);
> +}
> +
> +/* read int time in micro seconds */
> +static int ltr501_read_it_time(struct ltr501_data *data, int *val, int *val2)
> +{
> + int ret, index;
> +
> + ret = regmap_field_read(data->reg_it, &index);
> + if (ret < 0)
> + return ret;
> +
> + /* Make sure integ time index is valid */
> + if (index < 0 || index >= ARRAY_SIZE(int_time_mapping))
> + return -EINVAL;
> +
> + *val2 = int_time_mapping[index];
> + *val = 0;
> +
> + return IIO_VAL_INT_PLUS_MICRO;
> +}
> +
> static int ltr501_read_als(struct ltr501_data *data, __le16 buf[2])
> {
> int ret = ltr501_drdy(data, LTR501_STATUS_ALS_RDY);
> @@ -119,7 +177,7 @@ static int ltr501_read_ps(struct ltr501_data *data)
> static const struct iio_chan_spec ltr501_channels[] = {
> LTR501_INTENSITY_CHANNEL(0, LTR501_ALS_DATA0, IIO_MOD_LIGHT_BOTH, 0),
> LTR501_INTENSITY_CHANNEL(1, LTR501_ALS_DATA1, IIO_MOD_LIGHT_IR,
> - BIT(IIO_CHAN_INFO_SCALE)),
> + BIT(IIO_CHAN_INFO_SCALE) | BIT(IIO_CHAN_INFO_INT_TIME)),
> {
> .type = IIO_PROXIMITY,
> .address = LTR501_PS_DATA,
> @@ -195,6 +253,13 @@ static int ltr501_read_raw(struct iio_dev *indio_dev,
> default:
> return -EINVAL;
> }
> + case IIO_CHAN_INFO_INT_TIME:
> + switch (chan->type) {
> + case IIO_INTENSITY:
> + return ltr501_read_it_time(data, val, val2);
> + default:
> + return -EINVAL;
> + }
> }
> return -EINVAL;
> }
> @@ -245,16 +310,30 @@ static int ltr501_write_raw(struct iio_dev *indio_dev,
> default:
> return -EINVAL;
> }
> + case IIO_CHAN_INFO_INT_TIME:
> + switch (chan->type) {
> + case IIO_INTENSITY:
> + if (val != 0)
> + return -EINVAL;
> + mutex_lock(&data->lock_als);
> + i = ltr501_set_it_time(data, val2);
> + mutex_unlock(&data->lock_als);
> + return i;
> + default:
> + return -EINVAL;
> + }
> }
> return -EINVAL;
> }
>
> static IIO_CONST_ATTR(in_proximity_scale_available, "1 0.25 0.125 0.0625");
> static IIO_CONST_ATTR(in_intensity_scale_available, "1 0.005");
> +static IIO_CONST_ATTR_INT_TIME_AVAIL("0.05 0.1 0.2 0.4");
>
> static struct attribute *ltr501_attributes[] = {
> &iio_const_attr_in_proximity_scale_available.dev_attr.attr,
> &iio_const_attr_in_intensity_scale_available.dev_attr.attr,
> + &iio_const_attr_integration_time_available.dev_attr.attr,
> NULL
> };
>
> @@ -396,6 +475,12 @@ static int ltr501_probe(struct i2c_client *client,
> mutex_init(&data->lock_als);
> mutex_init(&data->lock_ps);
>
> + data->reg_it = devm_regmap_field_alloc(&client->dev, regmap, reg_it);
> + if (IS_ERR(data->reg_it)) {
> + dev_err(&client->dev, "Integ time reg field init failed.\n");
> + return PTR_ERR(data->reg_it);
> + }
> +
> ret = regmap_read(data->regmap, LTR501_PART_ID, &partid);
> if (ret < 0)
> return ret;
>
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
