Skip to contentSkip to conversation

LTR301 ALS support

PATCH Superseded Kuppuswamy Sathyanarayanan 2 participants 4 messages Apr 2, 2015 IIO

v3 of 6 Superseded Latest is v6 →

lore
Share & tools
lore

Kuppuswamy Sathyanarayanan

author
#0 lore raw

v3 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.

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

 drivers/iio/light/Kconfig  |  2 +-
 drivers/iio/light/ltr501.c | 56 +++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 54 insertions(+), 4 deletions(-)
Signature · 1 line
1.9.1
author
#1 lore raw

PATCH v3 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.

Signed-off-by Kuppuswamy Sathyanarayanan <[email protected]>
---
 drivers/iio/light/Kconfig  |  2 +-
 drivers/iio/light/ltr501.c | 56 +++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 54 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 +53 -3

110 lines not on this page · raw message

Signature · 1 line
1.9.1
#2 lore raw

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

Quoted 9 lines

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.

some comments inline

Quoted 11 lines
Signed-off-by Kuppuswamy Sathyanarayanan <[email protected]>
---
 drivers/iio/light/Kconfig  |  2 +-
 drivers/iio/light/ltr501.c | 56 +++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 54 insertions(+), 4 deletions(-)
drivers/iio/light/Kconfig +0 -0
index a224afd..4b2ec51 100644

drivers/iio/light/Kconfig config LTR501 · line 162

Quoted 5 lines
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 LTR301 Ambient light sensor.

ambient
LTR-501 vs LTR301 (dash vs no dash)

Quoted 8 lines
drivers/iio/light/Kconfig +0 -0
 	 This driver can also be built as a module.  If so, the module
          will be called ltr501.
drivers/iio/light/ltr501.c +0 -0
index 62b7072..66d9a2c 100644

drivers/iio/light/ltr501.c line 51

Quoted 7 lines
drivers/iio/light/ltr501.c +4 -0
@@ -45,10 +45,17 @@
 
 #define LTR501_PS_DATA_MASK 0x7ff
 
+enum ltr_chipset {
+	LTR301,
+	LTR501,
+	LTR_MAX_CHIPS /* this must be last */

maybe use ltr501_ prefix
MAX_CHIPS is not used

drivers/iio/light/ltr501.c line 58

Quoted 7 lines
drivers/iio/light/ltr501.c +3 -0
+};
+
 struct ltr501_data {
 	struct i2c_client *client;
 	struct mutex lock_als, lock_ps;
 	u8 als_contr, ps_contr;
+	u8 chip_id;

chip_id is not used

drivers/iio/light/ltr501.c static const struct iio_chan_spec ltr501_channels[] = { · line 138

Quoted 12 lines
drivers/iio/light/ltr501.c +5 -0
 };
 
 static int ltr501_drdy(struct ltr501_data *data, u8 drdy_mask)

@@ -124,6 +131,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(3),

timestamp is at scan_index 2, not 3

Quoted 40 lines
drivers/iio/light/ltr501.c +18 -0
+};
+
 static const int ltr501_ps_gain[4][2] = {
 	{1, 0}, {0, 250000}, {0, 125000}, {0, 62500}
 };

@@ -244,10 +258,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 +278,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 = &ltr301_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);

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

Quoted 9 lines
drivers/iio/light/ltr501.c +6 -0
@@ -347,6 +377,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 -ENOSYS

is NOSYS the correct error code?
some (recent) drivers use it, but I think ENODEV would be a better choice

Quoted 42 lines
drivers/iio/light/ltr501.c +17 -3
+
 	mutex_init(&data->lock_als);
 	mutex_init(&data->lock_ps);
 

@@ -357,12 +394,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;
 	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:
+		dev_warn(&client->dev, "ltr chip invalid\n");
+		return -ENODEV;
+	}
+
 	ret = ltr501_init(data);
 	if (ret < 0)
 		return ret;

@@ -422,7 +471,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);
Signature · 2 lines
Peter Meerwald
+43-664-2444418 (mobile)
author
#3 lore raw

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

Thanks for your comments.

Quoted 20 lines
Quoted 9 lines

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.

some comments inline

Quoted 8 lines

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

---
 drivers/iio/light/Kconfig  |  2 +-
 drivers/iio/light/ltr501.c | 56
+++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 54 insertions(+), 4 deletions(-)
Quoted 13 lines

drivers/iio/light/Kconfig config LTR501 · line 162

Quoted 10 lines
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.

ambient
LTR-501 vs LTR301 (dash vs no dash)

Ok. I will go with LTR-301. I will fix it in next set.

Quoted 12 lines
Quoted 12 lines
drivers/iio/light/Kconfig +0 -0
 	 This driver can also be built as a module.  If so, the module
          will be called ltr501.
drivers/iio/light/ltr501.c +0 -0
index 62b7072..66d9a2c 100644
@@ -45,10 +45,17 @@
 
 #define LTR501_PS_DATA_MASK 0x7ff
Quoted 7 lines

drivers/iio/light/ltr501.c line 51

Quoted 4 lines
drivers/iio/light/ltr501.c +4 -0
+enum ltr_chipset {
+	LTR301,
+	LTR501,
+	LTR_MAX_CHIPS /* this must be last */

maybe use ltr501_ prefix
MAX_CHIPS is not used

I added it for future use. May be I can remove it.

Quoted 10 lines

drivers/iio/light/ltr501.c line 58

Quoted 8 lines
drivers/iio/light/ltr501.c +3 -0
+};
+
 struct ltr501_data {
 	struct i2c_client *client;
 	struct mutex lock_als, lock_ps;
 	u8 als_contr, ps_contr;
+	u8 chip_id;

chip_id is not used

Its used in probe.

Quoted 63 lines
Quoted 5 lines
drivers/iio/light/ltr501.c +0 -0
 };
 
 static int ltr501_drdy(struct ltr501_data *data, u8 drdy_mask)

drivers/iio/light/ltr501.c static const struct iio_chan_spec ltr501_channels[] · line 139

Quoted 9 lines
drivers/iio/light/ltr501.c +5 -0
@@ -124,6 +131,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(3),

timestamp is at scan_index 2, not 3

Quoted 46 lines
drivers/iio/light/ltr501.c +2 -0
+};
+
 static const int ltr501_ps_gain[4][2] = {
 	{1, 0}, {0, 250000}, {0, 125000}, {0, 62500}

};

drivers/iio/light/ltr501.c +16 -0
@@ -244,10 +258,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 +278,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 = &ltr301_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);

drivers/iio/light/ltr501.c +1 -0
@@ -347,6 +377,13 @@ static int ltr501_probe(struct i2c_client *client,
 	data = iio_priv(indio_dev);
 	i2c_set_clientdata(client, indio_dev);
 	data->client = client;
+
Quoted 8 lines

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

Quoted 5 lines
drivers/iio/light/ltr501.c +5 -0
+	/* TODO: Add condition for ACPI */
+	if (id)
+		data->chip_id = id->driver_data;
+	else
+		return -ENOSYS

is NOSYS the correct error code?
some (recent) drivers use it, but I think ENODEV would be a better choice

I can change it to ENODEV.

Quoted 53 lines
Quoted 34 lines
drivers/iio/light/ltr501.c +15 -2
+
 	mutex_init(&data->lock_als);
 	mutex_init(&data->lock_ps);
 

@@ -357,12 +394,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;
 	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:
+		dev_warn(&client->dev, "ltr chip invalid\n");
+		return -ENODEV;
+	}
+
 	ret = ltr501_init(data);
 	if (ret < 0)
 		return ret;

drivers/iio/light/ltr501.c static int ltr501_resume(struct device *dev) · line 478

Quoted 9 lines
drivers/iio/light/ltr501.c +2 -1
@@ -422,7 +471,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);

--

Peter Meerwald
+43-664-2444418 (mobile)
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html