* [PATCH v4 0/1] LTR301 ALS support @ 2015-04-07 2:16 Kuppuswamy Sathyanarayanan 2015-04-07 2:16 ` [PATCH v4 1/1] iio: ltr301: Add support for ltr301 Kuppuswamy Sathyanarayanan 0 siblings, 1 reply; 6+ messages in thread From: Kuppuswamy Sathyanarayanan @ 2015-04-07 2:16 UTC (permalink / raw) To: jic23, pmeerw; +Cc: linux-iio, srinivas.pandruvada, sathyanarayanan.kuppuswamy 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(-) -- 1.9.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v4 1/1] iio: ltr301: Add support for ltr301 2015-04-07 2:16 [PATCH v4 0/1] LTR301 ALS support Kuppuswamy Sathyanarayanan @ 2015-04-07 2:16 ` Kuppuswamy Sathyanarayanan 2015-04-09 10:14 ` Jonathan Cameron 0 siblings, 1 reply; 6+ messages in thread From: Kuppuswamy Sathyanarayanan @ 2015-04-07 2:16 UTC (permalink / raw) To: jic23, pmeerw; +Cc: linux-iio, srinivas.pandruvada, sathyanarayanan.kuppuswamy 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 <sathyanarayanan.kuppuswamy@linux.intel.com> --- drivers/iio/light/Kconfig | 2 +- drivers/iio/light/ltr501.c | 55 +++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 53 insertions(+), 4 deletions(-) diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig index a224afd..215e4a3 100644 --- a/drivers/iio/light/Kconfig +++ b/drivers/iio/light/Kconfig @@ -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. diff --git a/drivers/iio/light/ltr501.c b/drivers/iio/light/ltr501.c index 62b7072..5cb0427 100644 --- a/drivers/iio/light/ltr501.c +++ b/drivers/iio/light/ltr501.c @@ -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); @@ -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) { + 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); -- 1.9.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v4 1/1] iio: ltr301: Add support for ltr301 2015-04-07 2:16 ` [PATCH v4 1/1] iio: ltr301: Add support for ltr301 Kuppuswamy Sathyanarayanan @ 2015-04-09 10:14 ` Jonathan Cameron 2015-04-09 12:20 ` Daniel Baluta 0 siblings, 1 reply; 6+ messages in thread From: Jonathan Cameron @ 2015-04-09 10:14 UTC (permalink / raw) To: Kuppuswamy Sathyanarayanan, pmeerw; +Cc: linux-iio, srinivas.pandruvada On 07/04/15 03:16, Kuppuswamy Sathyanarayanan wrote: > 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 <sathyanarayanan.kuppuswamy@linux.intel.com> 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 > --- > drivers/iio/light/Kconfig | 2 +- > drivers/iio/light/ltr501.c | 55 +++++++++++++++++++++++++++++++++++++++++++--- > 2 files changed, 53 insertions(+), 4 deletions(-) > > diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig > index a224afd..215e4a3 100644 > --- a/drivers/iio/light/Kconfig > +++ b/drivers/iio/light/Kconfig > @@ -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. > diff --git a/drivers/iio/light/ltr501.c b/drivers/iio/light/ltr501.c > index 62b7072..5cb0427 100644 > --- a/drivers/iio/light/ltr501.c > +++ b/drivers/iio/light/ltr501.c > @@ -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); > > @@ -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. > + 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); > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4 1/1] iio: ltr301: Add support for ltr301 2015-04-09 10:14 ` Jonathan Cameron @ 2015-04-09 12:20 ` Daniel Baluta 2015-04-09 13:41 ` Jonathan Cameron 0 siblings, 1 reply; 6+ messages in thread From: Daniel Baluta @ 2015-04-09 12:20 UTC (permalink / raw) To: Kuppuswamy Sathyanarayanan Cc: Peter Meerwald, linux-iio, Srinivas Pandruvada, Jonathan Cameron On Thu, Apr 9, 2015 at 1:14 PM, Jonathan Cameron <jic23@kernel.org> wrote: > On 07/04/15 03:16, Kuppuswamy Sathyanarayanan wrote: >> 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 <sathyanarayanan.kuppuswamy@linux.intel.com> > 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. 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. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4 1/1] iio: ltr301: Add support for ltr301 2015-04-09 12:20 ` Daniel Baluta @ 2015-04-09 13:41 ` Jonathan Cameron 2015-04-09 22:27 ` sathyanarayanan kuppuswamy 0 siblings, 1 reply; 6+ messages in thread From: Jonathan Cameron @ 2015-04-09 13:41 UTC (permalink / raw) To: Daniel Baluta, Kuppuswamy Sathyanarayanan Cc: Peter Meerwald, linux-iio, Srinivas Pandruvada On 09/04/15 13:20, Daniel Baluta wrote: > On Thu, Apr 9, 2015 at 1:14 PM, Jonathan Cameron <jic23@kernel.org> wrote: >> On 07/04/15 03:16, Kuppuswamy Sathyanarayanan wrote: >>> 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 <sathyanarayanan.kuppuswamy@linux.intel.com> >> 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. > > 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 > yes, that would make a lot of sense. > Let me know if you have any questions. > > Daniel. > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4 1/1] iio: ltr301: Add support for ltr301 2015-04-09 13:41 ` Jonathan Cameron @ 2015-04-09 22:27 ` sathyanarayanan kuppuswamy 0 siblings, 0 replies; 6+ messages in thread From: sathyanarayanan kuppuswamy @ 2015-04-09 22:27 UTC (permalink / raw) To: Jonathan Cameron, Daniel Baluta Cc: Peter Meerwald, linux-iio, Srinivas Pandruvada On 04/09/2015 06:41 AM, Jonathan Cameron wrote: > On 09/04/15 13:20, Daniel Baluta wrote: >> On Thu, Apr 9, 2015 at 1:14 PM, Jonathan Cameron <jic23@kernel.org> wrote: >>> On 07/04/15 03:16, Kuppuswamy Sathyanarayanan wrote: >>>> 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 <sathyanarayanan.kuppuswamy@linux.intel.com> >>> 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. >> 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 >> > yes, that would make a lot of sense. Ok. I will rebase my patch on top of your patch. >> Let me know if you have any questions. >> >> Daniel. >> > -- Sathyanarayanan Kuppuswamy Android kernel developer ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-04-09 22:26 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-04-07 2:16 [PATCH v4 0/1] LTR301 ALS support Kuppuswamy Sathyanarayanan 2015-04-07 2:16 ` [PATCH v4 1/1] iio: ltr301: Add support for ltr301 Kuppuswamy Sathyanarayanan 2015-04-09 10:14 ` Jonathan Cameron 2015-04-09 12:20 ` Daniel Baluta 2015-04-09 13:41 ` Jonathan Cameron 2015-04-09 22:27 ` sathyanarayanan kuppuswamy
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.