All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH 0/2] Add support for Lite On LTR559 sensor
@ 2015-03-31 10:37 Daniel Baluta
  2015-03-31 10:37 ` [RFC PATCH 1/2] iio: light: ltr501: Fix alignment to match open parenthesis Daniel Baluta
  2015-03-31 10:37 ` [RFC PATCH 2/2] iio: light: Add support for LTR559 chip Daniel Baluta
  0 siblings, 2 replies; 9+ messages in thread
From: Daniel Baluta @ 2015-03-31 10:37 UTC (permalink / raw)
  To: jic23; +Cc: knaack.h, lars, pmeerw, Caniel.baluta, inux-iio, linux-kernel

First patch does some cleanups in order to make alignment style
consistent with the rest of IIO drivers. Then second patch adds
support for Lite On LTR559 ambient light and proximity sensor.

This is a RFC because the code is compile tested only. The 
chip should arrive in a few days.

Daniel Baluta (2):
  iio: light: ltr501: Fix alignment to match open parenthesis
  iio: light: Add support for LTR559 chip

 drivers/iio/light/ltr501.c | 240 +++++++++++++++++++++++++++++++++++----------
 1 file changed, 187 insertions(+), 53 deletions(-)

-- 
1.9.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [RFC PATCH 1/2] iio: light: ltr501: Fix alignment to match open parenthesis
  2015-03-31 10:37 [RFC PATCH 0/2] Add support for Lite On LTR559 sensor Daniel Baluta
@ 2015-03-31 10:37 ` Daniel Baluta
  2015-04-09 13:32   ` Jonathan Cameron
  2015-03-31 10:37 ` [RFC PATCH 2/2] iio: light: Add support for LTR559 chip Daniel Baluta
  1 sibling, 1 reply; 9+ messages in thread
From: Daniel Baluta @ 2015-03-31 10:37 UTC (permalink / raw)
  To: jic23; +Cc: knaack.h, lars, pmeerw, Caniel.baluta, inux-iio, linux-kernel

This makes ltr501 code consistent with the coding style adopted
for the new drivers added to IIO.
We prepare the path for adding support for LTR559 chip.

Reported by checkpatch.pl

Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
---
 drivers/iio/light/ltr501.c | 46 +++++++++++++++++++++++++---------------------
 1 file changed, 25 insertions(+), 21 deletions(-)

diff --git a/drivers/iio/light/ltr501.c b/drivers/iio/light/ltr501.c
index 62b7072..883855a 100644
--- a/drivers/iio/light/ltr501.c
+++ b/drivers/iio/light/ltr501.c
@@ -58,7 +58,7 @@ static int ltr501_drdy(struct ltr501_data *data, u8 drdy_mask)
 
 	while (tries--) {
 		ret = i2c_smbus_read_byte_data(data->client,
-			LTR501_ALS_PS_STATUS);
+					       LTR501_ALS_PS_STATUS);
 		if (ret < 0)
 			return ret;
 		if ((ret & drdy_mask) == drdy_mask)
@@ -77,7 +77,8 @@ static int ltr501_read_als(struct ltr501_data *data, __le16 buf[2])
 		return ret;
 	/* always read both ALS channels in given order */
 	return i2c_smbus_read_i2c_block_data(data->client,
-		LTR501_ALS_DATA1, 2 * sizeof(__le16), (u8 *) buf);
+					     LTR501_ALS_DATA1,
+					     2 * sizeof(__le16), (u8 *)buf);
 }
 
 static int ltr501_read_ps(struct ltr501_data *data)
@@ -107,7 +108,7 @@ static int ltr501_read_ps(struct ltr501_data *data)
 static const struct iio_chan_spec ltr501_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)),
+				 BIT(IIO_CHAN_INFO_SCALE)),
 	{
 		.type = IIO_PROXIMITY,
 		.address = LTR501_PS_DATA,
@@ -129,8 +130,8 @@ static const int ltr501_ps_gain[4][2] = {
 };
 
 static int ltr501_read_raw(struct iio_dev *indio_dev,
-				struct iio_chan_spec const *chan,
-				int *val, int *val2, long mask)
+			   struct iio_chan_spec const *chan,
+			   int *val, int *val2, long mask)
 {
 	struct ltr501_data *data = iio_priv(indio_dev);
 	__le16 buf[2];
@@ -149,7 +150,7 @@ static int ltr501_read_raw(struct iio_dev *indio_dev,
 			if (ret < 0)
 				return ret;
 			*val = le16_to_cpu(chan->address == LTR501_ALS_DATA1 ?
-				buf[0] : buf[1]);
+					   buf[0] : buf[1]);
 			return IIO_VAL_INT;
 		case IIO_PROXIMITY:
 			mutex_lock(&data->lock_ps);
@@ -199,8 +200,8 @@ static int ltr501_get_ps_gain_index(int val, int val2)
 }
 
 static int ltr501_write_raw(struct iio_dev *indio_dev,
-			       struct iio_chan_spec const *chan,
-			       int val, int val2, long mask)
+			    struct iio_chan_spec const *chan,
+			    int val, int val2, long mask)
 {
 	struct ltr501_data *data = iio_priv(indio_dev);
 	int i;
@@ -219,7 +220,8 @@ static int ltr501_write_raw(struct iio_dev *indio_dev,
 			else
 				return -EINVAL;
 			return i2c_smbus_write_byte_data(data->client,
-				LTR501_ALS_CONTR, data->als_contr);
+							 LTR501_ALS_CONTR,
+							 data->als_contr);
 		case IIO_PROXIMITY:
 			i = ltr501_get_ps_gain_index(val, val2);
 			if (i < 0)
@@ -227,7 +229,8 @@ static int ltr501_write_raw(struct iio_dev *indio_dev,
 			data->ps_contr &= ~LTR501_CONTR_PS_GAIN_MASK;
 			data->ps_contr |= i << LTR501_CONTR_PS_GAIN_SHIFT;
 			return i2c_smbus_write_byte_data(data->client,
-				LTR501_PS_CONTR, data->ps_contr);
+							 LTR501_PS_CONTR,
+							 data->ps_contr);
 		default:
 			return -EINVAL;
 		}
@@ -279,7 +282,7 @@ static irqreturn_t ltr501_trigger_handler(int irq, void *p)
 
 	/* figure out which data needs to be ready */
 	if (test_bit(0, indio_dev->active_scan_mask) ||
-		test_bit(1, indio_dev->active_scan_mask))
+	    test_bit(1, indio_dev->active_scan_mask))
 		mask |= LTR501_STATUS_ALS_RDY;
 	if (test_bit(2, indio_dev->active_scan_mask))
 		mask |= LTR501_STATUS_PS_RDY;
@@ -290,7 +293,9 @@ static irqreturn_t ltr501_trigger_handler(int irq, void *p)
 
 	if (mask & LTR501_STATUS_ALS_RDY) {
 		ret = i2c_smbus_read_i2c_block_data(data->client,
-			LTR501_ALS_DATA1, sizeof(als_buf), (u8 *) als_buf);
+						    LTR501_ALS_DATA1,
+						    sizeof(als_buf),
+						    (u8 *)als_buf);
 		if (ret < 0)
 			return ret;
 		if (test_bit(0, indio_dev->active_scan_mask))
@@ -306,8 +311,7 @@ static irqreturn_t ltr501_trigger_handler(int irq, void *p)
 		buf[j++] = ret & LTR501_PS_DATA_MASK;
 	}
 
-	iio_push_to_buffers_with_timestamp(indio_dev, buf,
-		iio_get_time_ns());
+	iio_push_to_buffers_with_timestamp(indio_dev, buf, iio_get_time_ns());
 
 done:
 	iio_trigger_notify_done(indio_dev->trig);
@@ -330,11 +334,11 @@ static int ltr501_init(struct ltr501_data *data)
 	data->ps_contr = ret | LTR501_CONTR_ACTIVE;
 
 	return ltr501_write_contr(data->client, data->als_contr,
-		data->ps_contr);
+				  data->ps_contr);
 }
 
 static int ltr501_probe(struct i2c_client *client,
-			  const struct i2c_device_id *id)
+			const struct i2c_device_id *id)
 {
 	struct ltr501_data *data;
 	struct iio_dev *indio_dev;
@@ -368,7 +372,7 @@ static int ltr501_probe(struct i2c_client *client,
 		return ret;
 
 	ret = iio_triggered_buffer_setup(indio_dev, NULL,
-		ltr501_trigger_handler, NULL);
+					 ltr501_trigger_handler, NULL);
 	if (ret)
 		return ret;
 
@@ -386,8 +390,8 @@ error_unreg_buffer:
 static int ltr501_powerdown(struct ltr501_data *data)
 {
 	return ltr501_write_contr(data->client,
-		data->als_contr & ~LTR501_CONTR_ACTIVE,
-		data->ps_contr & ~LTR501_CONTR_ACTIVE);
+				  data->als_contr & ~LTR501_CONTR_ACTIVE,
+				  data->ps_contr & ~LTR501_CONTR_ACTIVE);
 }
 
 static int ltr501_remove(struct i2c_client *client)
@@ -405,14 +409,14 @@ static int ltr501_remove(struct i2c_client *client)
 static int ltr501_suspend(struct device *dev)
 {
 	struct ltr501_data *data = iio_priv(i2c_get_clientdata(
-		to_i2c_client(dev)));
+					    to_i2c_client(dev)));
 	return ltr501_powerdown(data);
 }
 
 static int ltr501_resume(struct device *dev)
 {
 	struct ltr501_data *data = iio_priv(i2c_get_clientdata(
-		to_i2c_client(dev)));
+					    to_i2c_client(dev)));
 
 	return ltr501_write_contr(data->client, data->als_contr,
 		data->ps_contr);
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [RFC PATCH 2/2] iio: light: Add support for LTR559 chip
  2015-03-31 10:37 [RFC PATCH 0/2] Add support for Lite On LTR559 sensor Daniel Baluta
  2015-03-31 10:37 ` [RFC PATCH 1/2] iio: light: ltr501: Fix alignment to match open parenthesis Daniel Baluta
@ 2015-03-31 10:37 ` Daniel Baluta
  2015-04-09 13:39   ` Jonathan Cameron
  1 sibling, 1 reply; 9+ messages in thread
From: Daniel Baluta @ 2015-03-31 10:37 UTC (permalink / raw)
  To: jic23; +Cc: knaack.h, lars, pmeerw, Caniel.baluta, inux-iio, linux-kernel

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.baluta@intel.com>
---
 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 = &ltr501_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 = &ltr501_info;
 	indio_dev->channels = ltr501_channels;
 	indio_dev->num_channels = ARRAY_SIZE(ltr501_channels);
-	indio_dev->name = LTR501_DRV_NAME;
+	indio_dev->name = name;
 	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);
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [RFC PATCH 1/2] iio: light: ltr501: Fix alignment to match open parenthesis
  2015-03-31 10:37 ` [RFC PATCH 1/2] iio: light: ltr501: Fix alignment to match open parenthesis Daniel Baluta
@ 2015-04-09 13:32   ` Jonathan Cameron
  0 siblings, 0 replies; 9+ messages in thread
From: Jonathan Cameron @ 2015-04-09 13:32 UTC (permalink / raw)
  To: Daniel Baluta
  Cc: knaack.h, lars, pmeerw, Caniel.baluta, inux-iio, linux-kernel

On 31/03/15 11:37, Daniel Baluta wrote:
> This makes ltr501 code consistent with the coding style adopted
> for the new drivers added to IIO.
> We prepare the path for adding support for LTR559 chip.
> 
> Reported by checkpatch.pl
> 
> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
fine, but needs a rebase based on the powerdown on failure patch that has
gone in meanwhile.
> ---
>  drivers/iio/light/ltr501.c | 46 +++++++++++++++++++++++++---------------------
>  1 file changed, 25 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/iio/light/ltr501.c b/drivers/iio/light/ltr501.c
> index 62b7072..883855a 100644
> --- a/drivers/iio/light/ltr501.c
> +++ b/drivers/iio/light/ltr501.c
> @@ -58,7 +58,7 @@ static int ltr501_drdy(struct ltr501_data *data, u8 drdy_mask)
>  
>  	while (tries--) {
>  		ret = i2c_smbus_read_byte_data(data->client,
> -			LTR501_ALS_PS_STATUS);
> +					       LTR501_ALS_PS_STATUS);
>  		if (ret < 0)
>  			return ret;
>  		if ((ret & drdy_mask) == drdy_mask)
> @@ -77,7 +77,8 @@ static int ltr501_read_als(struct ltr501_data *data, __le16 buf[2])
>  		return ret;
>  	/* always read both ALS channels in given order */
>  	return i2c_smbus_read_i2c_block_data(data->client,
> -		LTR501_ALS_DATA1, 2 * sizeof(__le16), (u8 *) buf);
> +					     LTR501_ALS_DATA1,
> +					     2 * sizeof(__le16), (u8 *)buf);
>  }
>  
>  static int ltr501_read_ps(struct ltr501_data *data)
> @@ -107,7 +108,7 @@ static int ltr501_read_ps(struct ltr501_data *data)
>  static const struct iio_chan_spec ltr501_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)),
> +				 BIT(IIO_CHAN_INFO_SCALE)),
>  	{
>  		.type = IIO_PROXIMITY,
>  		.address = LTR501_PS_DATA,
> @@ -129,8 +130,8 @@ static const int ltr501_ps_gain[4][2] = {
>  };
>  
>  static int ltr501_read_raw(struct iio_dev *indio_dev,
> -				struct iio_chan_spec const *chan,
> -				int *val, int *val2, long mask)
> +			   struct iio_chan_spec const *chan,
> +			   int *val, int *val2, long mask)
>  {
>  	struct ltr501_data *data = iio_priv(indio_dev);
>  	__le16 buf[2];
> @@ -149,7 +150,7 @@ static int ltr501_read_raw(struct iio_dev *indio_dev,
>  			if (ret < 0)
>  				return ret;
>  			*val = le16_to_cpu(chan->address == LTR501_ALS_DATA1 ?
> -				buf[0] : buf[1]);
> +					   buf[0] : buf[1]);
>  			return IIO_VAL_INT;
>  		case IIO_PROXIMITY:
>  			mutex_lock(&data->lock_ps);
> @@ -199,8 +200,8 @@ static int ltr501_get_ps_gain_index(int val, int val2)
>  }
>  
>  static int ltr501_write_raw(struct iio_dev *indio_dev,
> -			       struct iio_chan_spec const *chan,
> -			       int val, int val2, long mask)
> +			    struct iio_chan_spec const *chan,
> +			    int val, int val2, long mask)
>  {
>  	struct ltr501_data *data = iio_priv(indio_dev);
>  	int i;
> @@ -219,7 +220,8 @@ static int ltr501_write_raw(struct iio_dev *indio_dev,
>  			else
>  				return -EINVAL;
>  			return i2c_smbus_write_byte_data(data->client,
> -				LTR501_ALS_CONTR, data->als_contr);
> +							 LTR501_ALS_CONTR,
> +							 data->als_contr);
>  		case IIO_PROXIMITY:
>  			i = ltr501_get_ps_gain_index(val, val2);
>  			if (i < 0)
> @@ -227,7 +229,8 @@ static int ltr501_write_raw(struct iio_dev *indio_dev,
>  			data->ps_contr &= ~LTR501_CONTR_PS_GAIN_MASK;
>  			data->ps_contr |= i << LTR501_CONTR_PS_GAIN_SHIFT;
>  			return i2c_smbus_write_byte_data(data->client,
> -				LTR501_PS_CONTR, data->ps_contr);
> +							 LTR501_PS_CONTR,
> +							 data->ps_contr);
>  		default:
>  			return -EINVAL;
>  		}
> @@ -279,7 +282,7 @@ static irqreturn_t ltr501_trigger_handler(int irq, void *p)
>  
>  	/* figure out which data needs to be ready */
>  	if (test_bit(0, indio_dev->active_scan_mask) ||
> -		test_bit(1, indio_dev->active_scan_mask))
> +	    test_bit(1, indio_dev->active_scan_mask))
>  		mask |= LTR501_STATUS_ALS_RDY;
>  	if (test_bit(2, indio_dev->active_scan_mask))
>  		mask |= LTR501_STATUS_PS_RDY;
> @@ -290,7 +293,9 @@ static irqreturn_t ltr501_trigger_handler(int irq, void *p)
>  
>  	if (mask & LTR501_STATUS_ALS_RDY) {
>  		ret = i2c_smbus_read_i2c_block_data(data->client,
> -			LTR501_ALS_DATA1, sizeof(als_buf), (u8 *) als_buf);
> +						    LTR501_ALS_DATA1,
> +						    sizeof(als_buf),
> +						    (u8 *)als_buf);
>  		if (ret < 0)
>  			return ret;
>  		if (test_bit(0, indio_dev->active_scan_mask))
> @@ -306,8 +311,7 @@ static irqreturn_t ltr501_trigger_handler(int irq, void *p)
>  		buf[j++] = ret & LTR501_PS_DATA_MASK;
>  	}
>  
> -	iio_push_to_buffers_with_timestamp(indio_dev, buf,
> -		iio_get_time_ns());
> +	iio_push_to_buffers_with_timestamp(indio_dev, buf, iio_get_time_ns());
>  
>  done:
>  	iio_trigger_notify_done(indio_dev->trig);
> @@ -330,11 +334,11 @@ static int ltr501_init(struct ltr501_data *data)
>  	data->ps_contr = ret | LTR501_CONTR_ACTIVE;
>  
>  	return ltr501_write_contr(data->client, data->als_contr,
> -		data->ps_contr);
> +				  data->ps_contr);
>  }
>  
>  static int ltr501_probe(struct i2c_client *client,
> -			  const struct i2c_device_id *id)
> +			const struct i2c_device_id *id)
>  {
>  	struct ltr501_data *data;
>  	struct iio_dev *indio_dev;
> @@ -368,7 +372,7 @@ static int ltr501_probe(struct i2c_client *client,
>  		return ret;
>  
>  	ret = iio_triggered_buffer_setup(indio_dev, NULL,
> -		ltr501_trigger_handler, NULL);
> +					 ltr501_trigger_handler, NULL);
>  	if (ret)
>  		return ret;
>  
> @@ -386,8 +390,8 @@ error_unreg_buffer:
>  static int ltr501_powerdown(struct ltr501_data *data)
>  {
>  	return ltr501_write_contr(data->client,
> -		data->als_contr & ~LTR501_CONTR_ACTIVE,
> -		data->ps_contr & ~LTR501_CONTR_ACTIVE);
> +				  data->als_contr & ~LTR501_CONTR_ACTIVE,
> +				  data->ps_contr & ~LTR501_CONTR_ACTIVE);
>  }
>  
>  static int ltr501_remove(struct i2c_client *client)
> @@ -405,14 +409,14 @@ static int ltr501_remove(struct i2c_client *client)
>  static int ltr501_suspend(struct device *dev)
>  {
>  	struct ltr501_data *data = iio_priv(i2c_get_clientdata(
> -		to_i2c_client(dev)));
> +					    to_i2c_client(dev)));
>  	return ltr501_powerdown(data);
>  }
>  
>  static int ltr501_resume(struct device *dev)
>  {
>  	struct ltr501_data *data = iio_priv(i2c_get_clientdata(
> -		to_i2c_client(dev)));
> +					    to_i2c_client(dev)));
>  
>  	return ltr501_write_contr(data->client, data->als_contr,
>  		data->ps_contr);
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC PATCH 2/2] iio: light: Add support for LTR559 chip
  2015-03-31 10:37 ` [RFC PATCH 2/2] iio: light: Add support for LTR559 chip Daniel Baluta
@ 2015-04-09 13:39   ` Jonathan Cameron
  2015-04-09 13:44     ` Daniel Baluta
  0 siblings, 1 reply; 9+ messages in thread
From: Jonathan Cameron @ 2015-04-09 13:39 UTC (permalink / raw)
  To: Daniel Baluta
  Cc: knaack.h, lars, pmeerw, Caniel.baluta, inux-iio, linux-kernel

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.baluta@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 = &ltr501_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 = &ltr501_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);
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC PATCH 2/2] iio: light: Add support for LTR559 chip
  2015-04-09 13:39   ` Jonathan Cameron
@ 2015-04-09 13:44     ` Daniel Baluta
  2015-04-09 22:28         ` sathyanarayanan kuppuswamy
  0 siblings, 1 reply; 9+ messages in thread
From: Daniel Baluta @ 2015-04-09 13:44 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: knaack.h, lars, pmeerw, daniel.baluta, linux-iio, linux-kernel,
	Kuppuswamy, Sathyanarayanan



On 04/09/2015 04:39 PM, Jonathan Cameron wrote:
> 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.baluta@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...

Good point.

thanks,
Daniel.

<snip>

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

I will fix this and sync with Sathya for the LTR301 support.

thanks,
Daniel.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC PATCH 2/2] iio: light: Add support for LTR559 chip
  2015-04-09 13:44     ` Daniel Baluta
@ 2015-04-09 22:28         ` sathyanarayanan kuppuswamy
  0 siblings, 0 replies; 9+ messages in thread
From: sathyanarayanan kuppuswamy @ 2015-04-09 22:28 UTC (permalink / raw)
  To: Daniel Baluta, Jonathan Cameron
  Cc: knaack.h, lars, pmeerw, linux-iio, linux-kernel, Kuppuswamy,
	Sathyanarayanan



On 04/09/2015 06:44 AM, Daniel Baluta wrote:
>
>
> On 04/09/2015 04:39 PM, Jonathan Cameron wrote:
>> 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.baluta@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...
>
> Good point.
>
> thanks,
> Daniel.
>
> <snip>
>
>>> -    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;
>
> I will fix this and sync with Sathya for the LTR301 support.

I will send you the re-based LTR301 patch. Please add it your patch set.
>
> thanks,
> Daniel.
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

-- 
Sathyanarayanan Kuppuswamy
Android kernel developer


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC PATCH 2/2] iio: light: Add support for LTR559 chip
@ 2015-04-09 22:28         ` sathyanarayanan kuppuswamy
  0 siblings, 0 replies; 9+ messages in thread
From: sathyanarayanan kuppuswamy @ 2015-04-09 22:28 UTC (permalink / raw)
  To: Daniel Baluta, Jonathan Cameron
  Cc: knaack.h, lars, pmeerw, linux-iio, linux-kernel, Kuppuswamy,
	Sathyanarayanan



On 04/09/2015 06:44 AM, Daniel Baluta wrote:
>
>
> On 04/09/2015 04:39 PM, Jonathan Cameron wrote:
>> 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.baluta@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...
>
> Good point.
>
> thanks,
> Daniel.
>
> <snip>
>
>>> -    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;
>
> I will fix this and sync with Sathya for the LTR301 support.

I will send you the re-based LTR301 patch. Please add it your patch set.
>
> thanks,
> Daniel.
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

-- 
Sathyanarayanan Kuppuswamy
Android kernel developer


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [RFC PATCH 1/2] iio: light: ltr501: Fix alignment to match open parenthesis
  2015-03-31 10:38 [RFC PATCH 0/2] Add support for Lite On LTR559 sensor Daniel Baluta
@ 2015-03-31 10:38 ` Daniel Baluta
  0 siblings, 0 replies; 9+ messages in thread
From: Daniel Baluta @ 2015-03-31 10:38 UTC (permalink / raw)
  To: jic23; +Cc: knaack.h, lars, pmeerw, daniel.baluta, linux-iio, linux-kernel

This makes ltr501 code consistent with the coding style adopted
for the new drivers added to IIO.
We prepare the path for adding support for LTR559 chip.

Reported by checkpatch.pl

Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
---
 drivers/iio/light/ltr501.c | 46 +++++++++++++++++++++++++---------------------
 1 file changed, 25 insertions(+), 21 deletions(-)

diff --git a/drivers/iio/light/ltr501.c b/drivers/iio/light/ltr501.c
index 62b7072..883855a 100644
--- a/drivers/iio/light/ltr501.c
+++ b/drivers/iio/light/ltr501.c
@@ -58,7 +58,7 @@ static int ltr501_drdy(struct ltr501_data *data, u8 drdy_mask)
 
 	while (tries--) {
 		ret = i2c_smbus_read_byte_data(data->client,
-			LTR501_ALS_PS_STATUS);
+					       LTR501_ALS_PS_STATUS);
 		if (ret < 0)
 			return ret;
 		if ((ret & drdy_mask) == drdy_mask)
@@ -77,7 +77,8 @@ static int ltr501_read_als(struct ltr501_data *data, __le16 buf[2])
 		return ret;
 	/* always read both ALS channels in given order */
 	return i2c_smbus_read_i2c_block_data(data->client,
-		LTR501_ALS_DATA1, 2 * sizeof(__le16), (u8 *) buf);
+					     LTR501_ALS_DATA1,
+					     2 * sizeof(__le16), (u8 *)buf);
 }
 
 static int ltr501_read_ps(struct ltr501_data *data)
@@ -107,7 +108,7 @@ static int ltr501_read_ps(struct ltr501_data *data)
 static const struct iio_chan_spec ltr501_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)),
+				 BIT(IIO_CHAN_INFO_SCALE)),
 	{
 		.type = IIO_PROXIMITY,
 		.address = LTR501_PS_DATA,
@@ -129,8 +130,8 @@ static const int ltr501_ps_gain[4][2] = {
 };
 
 static int ltr501_read_raw(struct iio_dev *indio_dev,
-				struct iio_chan_spec const *chan,
-				int *val, int *val2, long mask)
+			   struct iio_chan_spec const *chan,
+			   int *val, int *val2, long mask)
 {
 	struct ltr501_data *data = iio_priv(indio_dev);
 	__le16 buf[2];
@@ -149,7 +150,7 @@ static int ltr501_read_raw(struct iio_dev *indio_dev,
 			if (ret < 0)
 				return ret;
 			*val = le16_to_cpu(chan->address == LTR501_ALS_DATA1 ?
-				buf[0] : buf[1]);
+					   buf[0] : buf[1]);
 			return IIO_VAL_INT;
 		case IIO_PROXIMITY:
 			mutex_lock(&data->lock_ps);
@@ -199,8 +200,8 @@ static int ltr501_get_ps_gain_index(int val, int val2)
 }
 
 static int ltr501_write_raw(struct iio_dev *indio_dev,
-			       struct iio_chan_spec const *chan,
-			       int val, int val2, long mask)
+			    struct iio_chan_spec const *chan,
+			    int val, int val2, long mask)
 {
 	struct ltr501_data *data = iio_priv(indio_dev);
 	int i;
@@ -219,7 +220,8 @@ static int ltr501_write_raw(struct iio_dev *indio_dev,
 			else
 				return -EINVAL;
 			return i2c_smbus_write_byte_data(data->client,
-				LTR501_ALS_CONTR, data->als_contr);
+							 LTR501_ALS_CONTR,
+							 data->als_contr);
 		case IIO_PROXIMITY:
 			i = ltr501_get_ps_gain_index(val, val2);
 			if (i < 0)
@@ -227,7 +229,8 @@ static int ltr501_write_raw(struct iio_dev *indio_dev,
 			data->ps_contr &= ~LTR501_CONTR_PS_GAIN_MASK;
 			data->ps_contr |= i << LTR501_CONTR_PS_GAIN_SHIFT;
 			return i2c_smbus_write_byte_data(data->client,
-				LTR501_PS_CONTR, data->ps_contr);
+							 LTR501_PS_CONTR,
+							 data->ps_contr);
 		default:
 			return -EINVAL;
 		}
@@ -279,7 +282,7 @@ static irqreturn_t ltr501_trigger_handler(int irq, void *p)
 
 	/* figure out which data needs to be ready */
 	if (test_bit(0, indio_dev->active_scan_mask) ||
-		test_bit(1, indio_dev->active_scan_mask))
+	    test_bit(1, indio_dev->active_scan_mask))
 		mask |= LTR501_STATUS_ALS_RDY;
 	if (test_bit(2, indio_dev->active_scan_mask))
 		mask |= LTR501_STATUS_PS_RDY;
@@ -290,7 +293,9 @@ static irqreturn_t ltr501_trigger_handler(int irq, void *p)
 
 	if (mask & LTR501_STATUS_ALS_RDY) {
 		ret = i2c_smbus_read_i2c_block_data(data->client,
-			LTR501_ALS_DATA1, sizeof(als_buf), (u8 *) als_buf);
+						    LTR501_ALS_DATA1,
+						    sizeof(als_buf),
+						    (u8 *)als_buf);
 		if (ret < 0)
 			return ret;
 		if (test_bit(0, indio_dev->active_scan_mask))
@@ -306,8 +311,7 @@ static irqreturn_t ltr501_trigger_handler(int irq, void *p)
 		buf[j++] = ret & LTR501_PS_DATA_MASK;
 	}
 
-	iio_push_to_buffers_with_timestamp(indio_dev, buf,
-		iio_get_time_ns());
+	iio_push_to_buffers_with_timestamp(indio_dev, buf, iio_get_time_ns());
 
 done:
 	iio_trigger_notify_done(indio_dev->trig);
@@ -330,11 +334,11 @@ static int ltr501_init(struct ltr501_data *data)
 	data->ps_contr = ret | LTR501_CONTR_ACTIVE;
 
 	return ltr501_write_contr(data->client, data->als_contr,
-		data->ps_contr);
+				  data->ps_contr);
 }
 
 static int ltr501_probe(struct i2c_client *client,
-			  const struct i2c_device_id *id)
+			const struct i2c_device_id *id)
 {
 	struct ltr501_data *data;
 	struct iio_dev *indio_dev;
@@ -368,7 +372,7 @@ static int ltr501_probe(struct i2c_client *client,
 		return ret;
 
 	ret = iio_triggered_buffer_setup(indio_dev, NULL,
-		ltr501_trigger_handler, NULL);
+					 ltr501_trigger_handler, NULL);
 	if (ret)
 		return ret;
 
@@ -386,8 +390,8 @@ error_unreg_buffer:
 static int ltr501_powerdown(struct ltr501_data *data)
 {
 	return ltr501_write_contr(data->client,
-		data->als_contr & ~LTR501_CONTR_ACTIVE,
-		data->ps_contr & ~LTR501_CONTR_ACTIVE);
+				  data->als_contr & ~LTR501_CONTR_ACTIVE,
+				  data->ps_contr & ~LTR501_CONTR_ACTIVE);
 }
 
 static int ltr501_remove(struct i2c_client *client)
@@ -405,14 +409,14 @@ static int ltr501_remove(struct i2c_client *client)
 static int ltr501_suspend(struct device *dev)
 {
 	struct ltr501_data *data = iio_priv(i2c_get_clientdata(
-		to_i2c_client(dev)));
+					    to_i2c_client(dev)));
 	return ltr501_powerdown(data);
 }
 
 static int ltr501_resume(struct device *dev)
 {
 	struct ltr501_data *data = iio_priv(i2c_get_clientdata(
-		to_i2c_client(dev)));
+					    to_i2c_client(dev)));
 
 	return ltr501_write_contr(data->client, data->als_contr,
 		data->ps_contr);
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2015-04-09 22:28 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-31 10:37 [RFC PATCH 0/2] Add support for Lite On LTR559 sensor Daniel Baluta
2015-03-31 10:37 ` [RFC PATCH 1/2] iio: light: ltr501: Fix alignment to match open parenthesis Daniel Baluta
2015-04-09 13:32   ` Jonathan Cameron
2015-03-31 10:37 ` [RFC PATCH 2/2] iio: light: Add support for LTR559 chip Daniel Baluta
2015-04-09 13:39   ` Jonathan Cameron
2015-04-09 13:44     ` Daniel Baluta
2015-04-09 22:28       ` sathyanarayanan kuppuswamy
2015-04-09 22:28         ` sathyanarayanan kuppuswamy
2015-03-31 10:38 [RFC PATCH 0/2] Add support for Lite On LTR559 sensor Daniel Baluta
2015-03-31 10:38 ` [RFC PATCH 1/2] iio: light: ltr501: Fix alignment to match open parenthesis Daniel Baluta

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.