LTR301 ALS support

PATCH Superseded Kuppuswamy Sathyanarayanan 3 participants 8 messages Apr 1, 2015 IIO

lore

Share & tools

lore

Kuppuswamy Sathyanarayanan

author

2015-04-01 04:06 UTC #0 lore raw

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.

Kuppuswamy Sathyanarayanan (1):
iio: ltr301: Add support for ltr301

 drivers/iio/light/Kconfig  |  2 +-
 drivers/iio/light/ltr501.c | 50 +++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 48 insertions(+), 4 deletions(-)

Signature · 1 line

1.9.1

Kuppuswamy Sathyanarayanan

author

2015-04-01 04:06 UTC #1 lore raw

PATCH 1/1 iio: ltr301: Add support for ltr301

Added support for Liteon 301 Ambient light sensor. Since LTR301 and LTR501 are register compatible(and even have same part id), LTR501 driver has been extended to support both devices. LTR501 is similar to LTR301 in ALS sensing, But the only difference is, LTR501 also supports proximity sensing.

LTR501 - ALS + Proximity combo
LTR301 - ALS sensor.

Following are the data sheets for both devices:

http://optoelectronics.liteon.com/upload/download/DS86-2013-0004/S_110_LTR-303ALS-01_DS_V1.pdf

http://optoelectronics.liteon.com/upload/download/DS86-2012-0006/S_110_LTR-501ALS-01_PrelimDS_ver1[1].pdf

Signed-off-by Kuppuswamy Sathyanarayanan <[email protected]>

---
 drivers/iio/light/Kconfig  |  2 +-
 drivers/iio/light/ltr501.c | 50 +++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 48 insertions(+), 4 deletions(-)

drivers/iio/light/Kconfig +1 -1

index a224afd..4b2ec51 100644
@@ -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 LTR301 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 +47 -3

104 lines not on this page · raw message

Signature · 1 line

1.9.1

Daniel Baluta

2015-04-01 15:10 UTC #2 lore raw

Re: [PATCH v1 1/1] iio: ltr301: Add support for ltr301

Quoted 8 lines · Kuppuswamy Sathyanarayanan

Added support for Liteon 301 Ambient light sensor. Since LTR301 and LTR501 are register compatible(and even have same part id), LTR501 driver has been extended to support both devices. LTR501 is similar to LTR301 in ALS sensing, But the only difference is, LTR501 also supports proximity sensing.
LTR501 - ALS + Proximity combo
LTR301 - ALS sensor.

As I'm adding support for LTR559 sensor, we should agree on what's the best way to this.

I suggest we should add a ltr501_chip_info for each chip type as done in [1]

drivers/iio/light/ltr501.c static int ltr501_probe(struct i2c_client *client, · line 380

Quoted 8 lines

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);

drivers/iio/light/ltr501.c +1 -0

@@ -347,6 +377,7 @@ static int ltr501_probe(struct i2c_client *client,
        data = iio_priv(indio_dev);
        i2c_set_clientdata(client, indio_dev);
        data->client = client;
+       data->chip_id = id->driver_data;

id can be NULL here if enumerated via ACPI.

drivers/iio/light/ltr501.c static int ltr501_probe(struct i2c_client *client, · line 392

Quoted 11 lines

drivers/iio/light/ltr501.c +0 -2

        mutex_init(&data->lock_als);
        mutex_init(&data->lock_ps);

@@ -357,12 +388,24 @@ static int ltr501_probe(struct i2c_client *client,
                return -ENODEV;
 
        indio_dev->dev.parent = &client->dev;
-       indio_dev->info = &ltr501_info;
-       indio_dev->channels = ltr501_channels;
        indio_dev->num_channels = ARRAY_SIZE(ltr501_channels);
        indio_dev->name = LTR501_DRV_NAME;

Name should be taken via id->name or ACPI.

drivers/iio/light/ltr501.c static int ltr501_probe(struct i2c_client *client, · line 405

Quoted 13 lines

drivers/iio/light/ltr501.c +11 -0

        indio_dev->modes = INDIO_DIRECT_MODE;
 
+       switch (data->chip_id) {
+       case LTR301:
+               indio_dev->info = &ltr301_info;
+               indio_dev->channels = ltr301_channels;
+               break;
+       case LTR501:
+               indio_dev->info = &ltr501_info;
+               indio_dev->channels = ltr501_channels;
+               break;
+       default:
+               pr_warn("ltr chip invalid\n");

We do have dev struct here, so we should use dev_warn.

Daniel.

[1] https://lkml.org/lkml/2015/3/31/198

sathyanarayanan kuppuswamy

author

2015-04-01 17:39 UTC #3 lore raw

Re: [PATCH v1 1/1] iio: ltr301: Add support for ltr301

Hi Daniel,

Thanks for the review comments.

Quoted 8 lines · Daniel Baluta

Quoted 6 lines · Kuppuswamy Sathyanarayanan
Added support for Liteon 301 Ambient light sensor. Since LTR301 and LTR501 are register compatible(and even have same part id), LTR501 driver has been extended to support both devices. LTR501 is similar to LTR301 in ALS sensing, But the only difference is, LTR501 also supports proximity sensing.

Quoted 7 lines

Quoted 2 lines
LTR501 - ALS + Proximity combo
LTR301 - ALS sensor.
As I'm adding support for LTR559 sensor, we should agree on what's the best way to this.
I suggest we should add a ltr501_chip_info for each chip type as done in [1]

Can you give me the link for your patch set ? If its just chip id , then there is no need for a special structure for it. But if you have more data to be stored then it would make sense.

Quoted 10 lines

drivers/iio/light/ltr501.c static int ltr501_probe(struct i2c_client *client, · line 380

Quoted 9 lines

  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);

drivers/iio/light/ltr501.c +1 -0

@@ -347,6 +377,7 @@ static int ltr501_probe(struct i2c_client *client,
         data = iio_priv(indio_dev);
         i2c_set_clientdata(client, indio_dev);
         data->client = client;
+       data->chip_id = id->driver_data;

id can be NULL here if enumerated via ACPI.

I will fix it.

Quoted 7 lines

drivers/iio/light/ltr501.c +0 -0

         mutex_init(&data->lock_als);
         mutex_init(&data->lock_ps);

@@ -357,12 +388,24 @@ static int ltr501_probe(struct i2c_client *client,
                 return -ENODEV;

Quoted 6 lines

drivers/iio/light/ltr501.c static int ltr501_probe(struct i2c_client *client, · line 392
Quoted 5 lines
drivers/iio/light/ltr501.c +0 -2
         indio_dev->dev.parent = &client->dev;
-       indio_dev->info = &ltr501_info;
-       indio_dev->channels = ltr501_channels;
         indio_dev->num_channels = ARRAY_SIZE(ltr501_channels);
         indio_dev->name = LTR501_DRV_NAME;
Name should be taken via id->name or ACPI.

Agree. But it needs be fixed by a separate patch.

Quoted 14 lines

Quoted 4 lines

drivers/iio/light/ltr501.c +1 -0

         indio_dev->modes = INDIO_DIRECT_MODE;
 
+       switch (data->chip_id) {

drivers/iio/light/ltr501.c static int ltr501_probe(struct i2c_client *client, · line 405

Quoted 10 lines

drivers/iio/light/ltr501.c +10 -0

+       case LTR301:
+               indio_dev->info = &ltr301_info;
+               indio_dev->channels = ltr301_channels;
+               break;
+       case LTR501:
+               indio_dev->info = &ltr501_info;
+               indio_dev->channels = ltr501_channels;
+               break;
+       default:
+               pr_warn("ltr chip invalid\n");

Quoted 1 line

We do have dev struct here, so we should use dev_warn.

ok. I will fix it.

Quoted 5 lines

Daniel.
[1] https://lkml.org/lkml/2015/3/31/198

Signature · 2 lines

Sathyanarayanan Kuppuswamy
Android kernel developer

Daniel Baluta

2015-04-01 15:34 UTC #4 lore raw

Re: [PATCH v1 1/1] iio: ltr301: Add support for ltr301

In reply to Kuppuswamy Sathyanarayanan

Quoted 6 lines · Kuppuswamy Sathyanarayanan

Added support for Liteon 301 Ambient light sensor. Since LTR301 and LTR501 are register compatible(and even have same part id), LTR501 driver has been extended to support both devices. LTR501 is similar to LTR301 in ALS sensing, But the only difference is, LTR501 also supports proximity sensing.

Quoted 8 lines

LTR501 - ALS + Proximity combo
LTR301 - ALS sensor.
Following are the data sheets for both devices:
http://optoelectronics.liteon.com/upload/download/DS86-2013-0004/S_110_LTR-303ALS-01_DS_V1.pdf
http://optoelectronics.liteon.com/upload/download/DS86-2012-0006/S_110_LTR-501ALS-01_PrelimDS_ver1[1].pdf

One more thing. The datasheet you posted is for LTR303 not LTR301. Which chip are you adding support for?

If indeed the datasheet link is correct, then although LTR303 and LTR501 are register compatbile at
a first glance I spotted at least one difference.

The ALS Control register is not the same.

(e.g ALS Gain uses 1 bit for LTR501 and 3 bits for LTR303).

thanks,
Daniel.

sathyanarayanan kuppuswamy

author

2015-04-01 17:42 UTC #5 lore raw

Re: [PATCH v1 1/1] iio: ltr301: Add support for ltr301

Hi Daniel,

Quoted 11 lines · Daniel Baluta

Quoted 9 lines · Kuppuswamy Sathyanarayanan
Added support for Liteon 301 Ambient light sensor. Since LTR301 and LTR501 are register compatible(and even have same part id), LTR501 driver has been extended to support both devices. LTR501 is similar to LTR301 in ALS sensing, But the only difference is, LTR501 also supports proximity sensing.
LTR501 - ALS + Proximity combo
LTR301 - ALS sensor.

Quoted 7 lines

Quoted 5 lines
Following are the data sheets for both devices:
http://optoelectronics.liteon.com/upload/download/DS86-2013-0004/S_110_LTR-303ALS-01_DS_V1.pdf
http://optoelectronics.liteon.com/upload/download/DS86-2012-0006/S_110_LTR-501ALS-01_PrelimDS_ver1[1].pdf
One more thing. The datasheet you posted is for LTR303 not LTR301. Which chip are you adding support for?

No. I posted link for the wrong data sheet. I will fix it once I find a proper link.

But LTR301 is register compatible with LTR501. I have verified it once again.

Quoted 12 lines

If indeed the datasheet link is correct, then although LTR303 and LTR501 are register compatbile at
a first glance I spotted at least one difference.
The ALS Control register is not the same.
(e.g ALS Gain uses 1 bit for LTR501 and 3 bits for LTR303).
thanks,
Daniel.

Signature · 2 lines

Sathyanarayanan Kuppuswamy
Android kernel developer

Daniel Baluta

2015-04-01 17:54 UTC #6 lore raw

Re: [PATCH v1 1/1] iio: ltr301: Add support for ltr301

Quoted 22 lines · sathyanarayanan kuppuswamy

Hi Daniel,
Quoted 19 lines · Daniel Baluta
Quoted 16 lines · Kuppuswamy Sathyanarayanan
Added support for Liteon 301 Ambient light sensor. Since LTR301 and LTR501 are register compatible(and even have same part id), LTR501 driver has been extended to support both devices. LTR501 is similar to LTR301 in ALS sensing, But the only difference is, LTR501 also supports proximity sensing.
LTR501 - ALS + Proximity combo
LTR301 - ALS sensor.
Following are the data sheets for both devices:
http://optoelectronics.liteon.com/upload/download/DS86-2013-0004/S_110_LTR-303ALS-01_DS_V1.pdf

Quoted 7 lines

Quoted 4 lines
Quoted 1 line
http://optoelectronics.liteon.com/upload/download/DS86-2012-0006/S_110_LTR-501ALS-01_PrelimDS_ver1[1].pdf
One more thing. The datasheet you posted is for LTR303 not LTR301. Which chip are you adding support for?
No. I posted link for the wrong data sheet. I will fix it once I find a proper link.

Oh, then it's all right.

Here is the link for my patchset:

https://lkml.org/lkml/2015/3/31/197

I guess you can take my first patch:
* iio: light: ltr501: Fix alignment to match open parenthesis

add to your patchset and rebase your changes on it.

Then when I'll receive the LTR501 device I will rebase my other patch on your changes.

thanks,
Daniel.

Daniel Baluta

2015-04-01 17:54 UTC #7 lore raw

Re: [PATCH v1 1/1] iio: ltr301: Add support for ltr301

Quoted 38 lines · Daniel Baluta

Quoted 22 lines · sathyanarayanan kuppuswamy
Hi Daniel,
Quoted 19 lines · Daniel Baluta
Quoted 16 lines · Kuppuswamy Sathyanarayanan
Added support for Liteon 301 Ambient light sensor. Since LTR301 and LTR501 are register compatible(and even have same part id), LTR501 driver has been extended to support both devices. LTR501 is similar to LTR301 in ALS sensing, But the only difference is, LTR501 also supports proximity sensing.
LTR501 - ALS + Proximity combo
LTR301 - ALS sensor.
Following are the data sheets for both devices:
http://optoelectronics.liteon.com/upload/download/DS86-2013-0004/S_110_LTR-303ALS-01_DS_V1.pdf
Quoted 7 lines
Quoted 4 lines
Quoted 1 line
http://optoelectronics.liteon.com/upload/download/DS86-2012-0006/S_110_LTR-501ALS-01_PrelimDS_ver1[1].pdf
One more thing. The datasheet you posted is for LTR303 not LTR301. Which chip are you adding support for?
No. I posted link for the wrong data sheet. I will fix it once I find a proper link.
Oh, then it's all right.
Here is the link for my patchset:
https://lkml.org/lkml/2015/3/31/197

Quoted 7 lines

I guess you can take my first patch:
* iio: light: ltr501: Fix alignment to match open parenthesis
add to your patchset and rebase your changes on it.
Then when I'll receive the LTR501 device I will rebase my other patch on your changes.

s/LTR501/LTR559