On 31/03/15 11:37, Daniel Baluta wrote: > This device is register compatible with LTR501, with a minor difference for > ALS control register as showed below: > > ALS Control register for LTR501: > > 7 6 5 4 3 2 1 0 > +------+------+------+------+------+------+------+------+ > | | | | | > | Reserved | Gain | SW | ALS Mode | > | | | Reset| | > +------+------+------+------+------+------+------+------+ > > ALS Control register for LTR559: > > 7 6 5 4 3 2 1 0 > +------+------+------+------+------+------+------+------+ > | | | | | > | Reserved | Gain | SW | ALS | > | | | Reset| Mode | > +------+------+------+------+------+------+------+------+ > > We handle this difference by introducing ltr501_chip_info. > > Datasheet for LTR559 is at: > http://optoelectronics.liteon.com/upload/download/DS86-2013-0003/S_110_LTR-559ALS-01_DS_V1.pdf > > Signed-off-by: Daniel Baluta <daniel.bal...@intel.com> Looks good to me. Let us know when you've tested it ;) Anyhow, one comment inline. It 'think' you can end up with a device probing succesfully with no known name...
J > --- > drivers/iio/light/ltr501.c | 196 > +++++++++++++++++++++++++++++++++++++-------- > 1 file changed, 163 insertions(+), 33 deletions(-) > > diff --git a/drivers/iio/light/ltr501.c b/drivers/iio/light/ltr501.c > index 883855a..5939cda 100644 > --- a/drivers/iio/light/ltr501.c > +++ b/drivers/iio/light/ltr501.c > @@ -45,12 +45,89 @@ > > #define LTR501_PS_DATA_MASK 0x7ff > > +#define LTR501_RESERVED_GAIN -1 > + > +enum { > + ltr501 = 0, > + ltr559, > +}; > + > +struct ltr501_gain { > + int scale; > + int uscale; > +}; > + > +static struct ltr501_gain ltr501_als_gain_tbl[] = { > + {1, 0}, > + {0, 5000}, > +}; > + > +static struct ltr501_gain ltr559_als_gain_tbl[] = { > + {1, 0}, > + {0, 500000}, > + {0, 250000}, > + {0, 125000}, > + {LTR501_RESERVED_GAIN, LTR501_RESERVED_GAIN}, > + {LTR501_RESERVED_GAIN, LTR501_RESERVED_GAIN}, > + {0, 20000}, > + {0, 10000}, > +}; > + > +static struct ltr501_gain ltr501_ps_gain_tbl[] = { > + {1, 0}, > + {0, 250000}, > + {0, 125000}, > + {0, 62500}, > +}; > + > +static struct ltr501_gain ltr559_ps_gain_tbl[] = { > + {0, 62500}, /* x16 gain */ > + {0, 31250}, /* x32 gain */ > + {0, 15625}, /* bits X1 are for x64 gain */ > + {0, 15624}, > +}; > + > +struct ltr501_chip_info { > + u8 chip_id; > + struct ltr501_gain *als_gain; > + int als_gain_tbl_size; > + struct ltr501_gain *ps_gain; > + int ps_gain_tbl_size; > + u8 als_mode_active; > + u8 als_gain_mask; > + u8 als_gain_shift; > +}; > + > struct ltr501_data { > struct i2c_client *client; > struct mutex lock_als, lock_ps; > + struct ltr501_chip_info *chip_info; > u8 als_contr, ps_contr; > }; > > +static struct ltr501_chip_info ltr501_chip_info_tbl[] = { > + [ltr501] = { > + .chip_id = 0x08, > + .als_gain = ltr501_als_gain_tbl, > + .als_gain_tbl_size = ARRAY_SIZE(ltr501_als_gain_tbl), > + .ps_gain = ltr501_ps_gain_tbl, > + .ps_gain_tbl_size = ARRAY_SIZE(ltr501_ps_gain_tbl), > + .als_mode_active = BIT(0) | BIT(1), > + .als_gain_mask = BIT(3), > + .als_gain_shift = 3, > + }, > + [ltr559] = { > + .chip_id = 0x09, > + .als_gain = ltr559_als_gain_tbl, > + .als_gain_tbl_size = ARRAY_SIZE(ltr559_als_gain_tbl), > + .ps_gain = ltr559_ps_gain_tbl, > + .ps_gain_tbl_size = ARRAY_SIZE(ltr559_ps_gain_tbl), > + .als_mode_active = BIT(1), > + .als_gain_mask = BIT(2) | BIT(3) | BIT(4), > + .als_gain_shift = 2, > + }, > +}; > + > static int ltr501_drdy(struct ltr501_data *data, u8 drdy_mask) > { > int tries = 100; > @@ -125,10 +202,6 @@ static const struct iio_chan_spec ltr501_channels[] = { > IIO_CHAN_SOFT_TIMESTAMP(3), > }; > > -static const int ltr501_ps_gain[4][2] = { > - {1, 0}, {0, 250000}, {0, 125000}, {0, 62500} > -}; > - > static int ltr501_read_raw(struct iio_dev *indio_dev, > struct iio_chan_spec const *chan, > int *val, int *val2, long mask) > @@ -166,20 +239,16 @@ static int ltr501_read_raw(struct iio_dev *indio_dev, > case IIO_CHAN_INFO_SCALE: > switch (chan->type) { > case IIO_INTENSITY: > - if (data->als_contr & LTR501_CONTR_ALS_GAIN_MASK) { > - *val = 0; > - *val2 = 5000; > - return IIO_VAL_INT_PLUS_MICRO; > - } else { > - *val = 1; > - *val2 = 0; > - return IIO_VAL_INT; > - } > + i = (data->als_contr & data->chip_info->als_gain_mask) > >> > + data->chip_info->als_gain_shift; > + *val = data->chip_info->als_gain[i].scale; > + *val2 = data->chip_info->als_gain[i].uscale; > + return IIO_VAL_INT_PLUS_MICRO; > case IIO_PROXIMITY: > i = (data->ps_contr & LTR501_CONTR_PS_GAIN_MASK) >> > LTR501_CONTR_PS_GAIN_SHIFT; > - *val = ltr501_ps_gain[i][0]; > - *val2 = ltr501_ps_gain[i][1]; > + *val = data->chip_info->ps_gain[i].scale; > + *val2 = data->chip_info->ps_gain[i].uscale; > return IIO_VAL_INT_PLUS_MICRO; > default: > return -EINVAL; > @@ -188,12 +257,13 @@ static int ltr501_read_raw(struct iio_dev *indio_dev, > return -EINVAL; > } > > -static int ltr501_get_ps_gain_index(int val, int val2) > +static int ltr501_get_gain_index(struct ltr501_gain *gain, int size, > + int val, int val2) > { > int i; > > - for (i = 0; i < ARRAY_SIZE(ltr501_ps_gain); i++) > - if (val == ltr501_ps_gain[i][0] && val2 == ltr501_ps_gain[i][1]) > + for (i = 0; i < size; i++) > + if (val == gain[i].scale && val2 == gain[i].uscale) > return i; > > return -1; > @@ -204,6 +274,7 @@ static int ltr501_write_raw(struct iio_dev *indio_dev, > int val, int val2, long mask) > { > struct ltr501_data *data = iio_priv(indio_dev); > + struct ltr501_chip_info *info = data->chip_info; > int i; > > if (iio_buffer_enabled(indio_dev)) > @@ -213,17 +284,20 @@ static int ltr501_write_raw(struct iio_dev *indio_dev, > case IIO_CHAN_INFO_SCALE: > switch (chan->type) { > case IIO_INTENSITY: > - if (val == 0 && val2 == 5000) > - data->als_contr |= LTR501_CONTR_ALS_GAIN_MASK; > - else if (val == 1 && val2 == 0) > - data->als_contr &= ~LTR501_CONTR_ALS_GAIN_MASK; > - else > + i = ltr501_get_gain_index(info->als_gain, > + info->als_gain_tbl_size, > + val, val2); > + if (i < 0) > return -EINVAL; > + data->als_contr &= ~info->als_gain_mask; > + data->als_contr |= i << info->als_gain_shift; > return i2c_smbus_write_byte_data(data->client, > LTR501_ALS_CONTR, > data->als_contr); > case IIO_PROXIMITY: > - i = ltr501_get_ps_gain_index(val, val2); > + i = ltr501_get_gain_index(info->ps_gain, > + info->ps_gain_tbl_size, > + val, val2); > if (i < 0) > return -EINVAL; > data->ps_contr &= ~LTR501_CONTR_PS_GAIN_MASK; > @@ -238,12 +312,58 @@ static int ltr501_write_raw(struct iio_dev *indio_dev, > 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 ssize_t ltr501_show_proximity_scale_avail(struct device *dev, > + struct device_attribute *attr, > + char *buf) > +{ > + struct ltr501_data *data = iio_priv(dev_to_iio_dev(dev)); > + struct ltr501_chip_info *info = data->chip_info; > + ssize_t len = 0; > + int i; > + > + for (i = 0; i < info->ps_gain_tbl_size; i++) { > + if (info->ps_gain[i].scale == LTR501_RESERVED_GAIN) > + continue; > + len += scnprintf(buf + len, PAGE_SIZE - len, "%d.%06d ", > + info->ps_gain[i].scale, > + info->ps_gain[i].uscale); > + } > + > + buf[len - 1] = '\n'; > + > + return len; > +} > + > +static ssize_t ltr501_show_intensity_scale_avail(struct device *dev, > + struct device_attribute *attr, > + char *buf) > +{ > + struct ltr501_data *data = iio_priv(dev_to_iio_dev(dev)); > + struct ltr501_chip_info *info = data->chip_info; > + ssize_t len = 0; > + int i; > + > + for (i = 0; i < info->als_gain_tbl_size; i++) { > + if (info->als_gain[i].scale == LTR501_RESERVED_GAIN) > + continue; > + len += scnprintf(buf + len, PAGE_SIZE - len, "%d.%06d ", > + info->als_gain[i].scale, > + info->als_gain[i].uscale); > + } > + > + buf[len - 1] = '\n'; > + > + return len; > +} > + > +static IIO_DEVICE_ATTR(in_proximity_scale_available, S_IRUGO, > + ltr501_show_proximity_scale_avail, NULL, 0); > +static IIO_DEVICE_ATTR(in_intensity_scale_available, S_IRUGO, > + ltr501_show_intensity_scale_avail, NULL, 0); > > 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_dev_attr_in_proximity_scale_available.dev_attr.attr, > + &iio_dev_attr_in_intensity_scale_available.dev_attr.attr, > NULL > }; > > @@ -326,7 +446,7 @@ static int ltr501_init(struct ltr501_data *data) > ret = i2c_smbus_read_byte_data(data->client, LTR501_ALS_CONTR); > if (ret < 0) > return ret; > - data->als_contr = ret | LTR501_CONTR_ACTIVE; > + data->als_contr = ret | data->chip_info->als_mode_active; > > ret = i2c_smbus_read_byte_data(data->client, LTR501_PS_CONTR); > if (ret < 0) > @@ -342,6 +462,8 @@ static int ltr501_probe(struct i2c_client *client, > { > struct ltr501_data *data; > struct iio_dev *indio_dev; > + const char *name = NULL; > + int chip_id = 0; > int ret; > > indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data)); > @@ -354,17 +476,24 @@ static int ltr501_probe(struct i2c_client *client, > mutex_init(&data->lock_als); > mutex_init(&data->lock_ps); > > + if (id) { > + name = id->name; > + chip_id = id->driver_data; > + } > + > + data->chip_info = <r501_chip_info_tbl[chip_id]; > + > ret = i2c_smbus_read_byte_data(data->client, LTR501_PART_ID); > if (ret < 0) > return ret; > - if ((ret >> 4) != 0x8) > + if ((ret >> 4) != data->chip_info->chip_id) > return -ENODEV; > > indio_dev->dev.parent = &client->dev; > indio_dev->info = <r501_info; > indio_dev->channels = ltr501_channels; > indio_dev->num_channels = ARRAY_SIZE(ltr501_channels); > - indio_dev->name = LTR501_DRV_NAME; > + indio_dev->name = name; Can name still be NULL? Needs a default value that makes sense. > indio_dev->modes = INDIO_DIRECT_MODE; > > ret = ltr501_init(data); > @@ -390,7 +519,7 @@ error_unreg_buffer: > static int ltr501_powerdown(struct ltr501_data *data) > { > return ltr501_write_contr(data->client, > - data->als_contr & ~LTR501_CONTR_ACTIVE, > + data->als_contr & > ~data->chip_info->als_mode_active, > data->ps_contr & ~LTR501_CONTR_ACTIVE); > } > > @@ -426,7 +555,8 @@ static int ltr501_resume(struct device *dev) > static SIMPLE_DEV_PM_OPS(ltr501_pm_ops, ltr501_suspend, ltr501_resume); > > static const struct i2c_device_id ltr501_id[] = { > - { "ltr501", 0 }, > + { "ltr501", ltr501}, > + { "ltr559", ltr559}, > { } > }; > MODULE_DEVICE_TABLE(i2c, ltr501_id); > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/