LTR301 ALS support
v4 cover LTR301 ALS support
Following patch adds support for Liteon 301 Ambient light sensor.
Please let me know your review comments.
v1:
Extended LTR501 driver to support both LTR301 and LTR501 device.
v2:
1. Handled device id NULL case in probe
2. Changed pr_warn to dev_warn
v3:
1. Sent v2 before by mistake. So sending the final version of v2 as v3.
v4:
1. Addressed minor comments from Peter Meerwald
2. Changed invalid chip id errno from ENOSYS to ENODEV
3. Fixed channel id index number
4. Removed unused variable.
Kuppuswamy Sathyanarayanan (1):
iio: ltr301: Add support for ltr301
drivers/iio/light/Kconfig | 2 +- drivers/iio/light/ltr501.c | 55 +++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 53 insertions(+), 4 deletions(-)
Signature · 1 line
1.9.1
PATCH v4 1/1 iio: ltr301: Add support for ltr301
Added support for Liteon 301 Ambient light sensor. Since LTR-301 and LTR-501 are register compatible(and even have same part id), LTR-501 driver has been extended to support both devices. LTR-501 is similar to LTR-301 in ALS sensing, But the only difference is, LTR-501 also supports proximity sensing.
LTR-501 - ALS + Proximity combo
LTR-301 - ALS sensor.
--- drivers/iio/light/Kconfig | 2 +- drivers/iio/light/ltr501.c | 55 +++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 53 insertions(+), 4 deletions(-)
drivers/iio/light/Kconfig +1 -1
@@ -159,7 +159,7 @@ config LTR501 select IIO_TRIGGERED_BUFFER help If you say yes here you get support for the Lite-On LTR-501ALS-01 - ambient light and proximity sensor. + ambient light and proximity sensor or LTR-301 ambient light sensor. This driver can also be built as a module. If so, the module will be called ltr501.
drivers/iio/light/ltr501.c +52 -3
109 lines not on this page · raw message
Signature · 1 line
1.9.1
Jonathan Cameron
maintainerRe: [PATCH v4 1/1] iio: ltr301: Add support for ltr301
Quoted 10 lines · Kuppuswamy Sathyanarayanan
Added support for Liteon 301 Ambient light sensor. Since LTR-301 and LTR-501 are register compatible(and even have same part id), LTR-501 driver has been extended to support both devices. LTR-501 is similar to LTR-301 in ALS sensing, But the only difference is, LTR-501 also supports proximity sensing.
LTR-501 - ALS + Proximity combo
LTR-301 - ALS sensor.Signed-off-by Kuppuswamy Sathyanarayanan <[email protected]>
Patch naming convention
iio:<name of driver>: Add support for ltr301
so here would be
iio:ltr501:Add support for ltr301.
Otherwise, looks good to me. A comment inline that it might make sense to introduced a ltr501_chip info structure and use static const struct ltr501_chip_info[2] = {
[LTR301] = {
.info = ...
....
},[LTR501] = {
}};
That way you make all the truely constant data apparently constant and loose the switch statement. It makes for an easier to review / simpler driver in the long run.
I haven't checked but this is probably what Daniel was suggesting in his review for v1 of this patch.
Jonathan
Quoted 102 lines
--- drivers/iio/light/Kconfig | 2 +- drivers/iio/light/ltr501.c | 55 +++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 53 insertions(+), 4 deletions(-)drivers/iio/light/Kconfig +1
-1@@ -159,7 +159,7 @@ config LTR501 select IIO_TRIGGERED_BUFFER help If you say yes here you get support for the Lite-On LTR-501ALS-01 - ambient light and proximity sensor. + ambient light and proximity sensor or LTR-301 ambient light sensor. This driver can also be built as a module. If so, the module will be called ltr501.drivers/iio/light/ltr501.c +36
-0@@ -45,10 +45,16 @@#define LTR501_PS_DATA_MASK 0x7ff +enum ltr_chipset { +LTR301 , +LTR501 , +}; +struct ltr501_data {struct i2c_client *client;struct mutex lock_als, lock_ps;u8 als_contr, ps_contr; +u8 chip_id; };static int ltr501_drdy (struct ltr501_data *data,u8 drdy_mask)@@ -124,6 +130,13 @@ static const struct iio_chan_spec ltr501_channels[] = { IIO_CHAN_SOFT_TIMESTAMP(3), }; +static const struct iio_chan_spec ltr301_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 )), +IIO_CHAN_SOFT_TIMESTAMP (2 ), +}; +static const int ltr501_ps_gain[4 ][2 ] = { {1 ,0 }, {0 ,250000 }, {0 ,125000 }, {0 ,62500 } };@@ -244,10 +257,19 @@ static struct attribute *ltr501_attributes[] = {NULL }; +static struct attribute *ltr301_attributes[] = { + &iio_const_attr_in_intensity_scale_available.dev_attr.attr, +NULL +}; +static const struct attribute_group ltr501_attribute_group = { .attrs = ltr501_attributes, }; +static const struct attribute_group ltr301_attribute_group = { + .attrs = ltr301_attributes, +}; +static const struct iio_info ltr501_info = { .read_raw = ltr501_read_raw, .write_raw = ltr501_write_raw,@@ -255,6 +277,13 @@ static const struct iio_info ltr501_info = { .driver_module = THIS_MODULE, }; +static const struct iio_info ltr301_info = { + .read_raw = ltr501_read_raw, + .write_raw = ltr501_write_raw, + .attrs = <r301_attribute_group, + .driver_module =THIS_MODULE , +}; +static int ltr501_write_contr (struct i2c_client *client,u8 als_val,u8 ps_val) {int ret =i2c_smbus_write_byte_data (client,LTR501_ALS_CONTR , als_val);@@ -347,6 +376,13 @@ static int ltr501_probe(struct i2c_client *client, data =iio_priv (indio_dev);i2c_set_clientdata (client, indio_dev); data->client = client; + +/* TODO: Add condition for ACPI */ +if (id) + data->chip_id = id->driver_data; +else +return -ENODEV ; +mutex_init (&data->lock_als);mutex_init (&data->lock_ps);
drivers/iio/light/ltr501.c static int ltr501_probe(struct i2c_client *client, · line 400
Quoted 10 lines
drivers/iio/light/ltr501.c +1
-2@@ -357,12 +393,24 @@ static int ltr501_probe(struct i2c_client *client,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->modes =INDIO_DIRECT_MODE ; +switch (data->chip_id) {
This could be factored out as a 'chip_info' static structure array thus allowing this case statement to be a lookup. Slightly neater as you add more devices, but at this stage, dubious whether it's worth the effort.
Quoted 27 lines
drivers/iio/light/ltr501.c +15
-1+case LTR301 : + indio_dev->info = <r301_info; + indio_dev->channels = ltr301_channels; +break ; +case LTR501 : + indio_dev->info = <r501_info; + indio_dev->channels = ltr501_channels; +break ; +default : +dev_warn (&client->dev,"ltr chip invalid\n" ); +return -ENODEV ; + } + ret =ltr501_init (data);if (ret <0 )return ret;@@ -422,7 +470,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 }, + {"ltr301" ,LTR301 }, + {"ltr501" ,LTR501 }, { } };MODULE_DEVICE_TABLE (i2c, ltr501_id);
Re: [PATCH v4 1/1] iio: ltr301: Add support for ltr301
Quoted 26 lines · Jonathan Cameron
Quoted 10 lines · Kuppuswamy Sathyanarayanan
Added support for Liteon 301 Ambient light sensor. Since LTR-301 and LTR-501 are register compatible(and even have same part id), LTR-501 driver has been extended to support both devices. LTR-501 is similar to LTR-301 in ALS sensing, But the only difference is, LTR-501 also supports proximity sensing.
LTR-501 - ALS + Proximity combo
LTR-301 - ALS sensor.Signed-off-by Kuppuswamy Sathyanarayanan <[email protected]>Patch naming convention
iio:<name of driver>: Add support for ltr301
so here would be
iio:ltr501:Add support for ltr301.Otherwise, looks good to me. A comment inline that it might make sense to introduced a
ltr501_chipinfo structure and use static const struct ltr501_chip_info[2] = {[LTR301] = { .info = ... .... },[LTR501] = {
}};
Quoted 6 lines
That way you make all the truely constant data apparently constant and loose the switch statement. It makes for an easier to review / simpler driver in the long run.
I haven't checked but this is probably what Daniel was suggesting in his review for v1 of this patch.
Hi Sathya,
I think the best approach to get this merged in would be for you to rebase your ltr301 patches on my patches for ltr559.
http://marc.info/?l=linux-kernel&m=142779827617036&w=2
Let me know if you have any questions.
Daniel.
Jonathan Cameron
maintainerRe: [PATCH v4 1/1] iio: ltr301: Add support for ltr301
Quoted 34 lines · Daniel Baluta
Quoted 26 lines · Jonathan Cameron
Quoted 10 lines · Kuppuswamy Sathyanarayanan
Added support for Liteon 301 Ambient light sensor. Since LTR-301 and LTR-501 are register compatible(and even have same part id), LTR-501 driver has been extended to support both devices. LTR-501 is similar to LTR-301 in ALS sensing, But the only difference is, LTR-501 also supports proximity sensing.
LTR-501 - ALS + Proximity combo
LTR-301 - ALS sensor.Signed-off-by Kuppuswamy Sathyanarayanan <[email protected]>Patch naming convention
iio:<name of driver>: Add support for ltr301
so here would be
iio:ltr501:Add support for ltr301.Otherwise, looks good to me. A comment inline that it might make sense to introduced a
ltr501_chipinfo structure and use static const struct ltr501_chip_info[2] = {[LTR301] = { .info = ... .... },[LTR501] = {
}};Quoted 6 lines
That way you make all the truely constant data apparently constant and loose the switch statement. It makes for an easier to review / simpler driver in the long run.
I haven't checked but this is probably what Daniel was suggesting in his review for v1 of this patch.
Quoted 7 lines
Hi Sathya,
I think the best approach to get this merged in would be for you to rebase your ltr301 patches on my patches for ltr559.
yes, that would make a lot of sense.
Quoted 4 lines
Let me know if you have any questions.
Daniel.
Re: [PATCH v4 1/1] iio: ltr301: Add support for ltr301
Quoted 34 lines · Jonathan Cameron
Quoted 33 lines · Daniel Baluta
Quoted 32 lines · Jonathan Cameron
Quoted 10 lines · Kuppuswamy Sathyanarayanan
Added support for Liteon 301 Ambient light sensor. Since LTR-301 and LTR-501 are register compatible(and even have same part id), LTR-501 driver has been extended to support both devices. LTR-501 is similar to LTR-301 in ALS sensing, But the only difference is, LTR-501 also supports proximity sensing.
LTR-501 - ALS + Proximity combo
LTR-301 - ALS sensor.Signed-off-by Kuppuswamy Sathyanarayanan <[email protected]>Patch naming convention
iio:<name of driver>: Add support for ltr301
so here would be
iio:ltr501:Add support for ltr301.Otherwise, looks good to me. A comment inline that it might make sense to introduced a
ltr501_chipinfo structure and use static const struct ltr501_chip_info[2] = {[LTR301] = { .info = ... .... },[LTR501] = {
}};That way you make all the truely constant data apparently constant and loose the switch statement. It makes for an easier to review / simpler driver in the long run.
I haven't checked but this is probably what Daniel was suggesting in his review for v1 of this patch.
Quoted 8 lines
Quoted 7 lines
Hi Sathya,
I think the best approach to get this merged in would be for you to rebase your ltr301 patches on my patches for ltr559.
yes, that would make a lot of sense.
Ok. I will rebase my patch on top of your patch.
Quoted 5 lines
Quoted 4 lines
Let me know if you have any questions.
Daniel.
Signature · 2 lines
Sathyanarayanan Kuppuswamy Android kernel developer