All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/3] staging: iio: light: isl29018: Remove non-standard sysfs attributes
@ 2015-04-16 19:20 Roberta Dobrescu
  2015-04-16 19:20 ` [PATCH v2 1/3] " Roberta Dobrescu
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Roberta Dobrescu @ 2015-04-16 19:20 UTC (permalink / raw)
  To: linux-iio; +Cc: jic23, knaack.h, lars, pmeerw, daniel.baluta, Roberta Dobrescu

This set of patches removes non-standard sysfs attributes used
by isl29018 driver.

Integration time is used instead of adc resolution since these are
strongly connected. Also, full scale range is controlled by scale.
isl29018 driver code is refactored in order to use these standard attributes.

Additionally, it renames lux_scale to calibscale and lux_uscale to
ucalibscale since these parameters are used for hardware applied calibration.

The purpose of these patches is to change the isl29018 driver to use
standard IIO attributes in order to be moved out of staging.

Changes since v1:
	* Use integration time for controlling adc resolution
	* Use scale to control range instead of introducing IIO_CHAN_INFO_RANGE

Roberta Dobrescu (3):
  staging: iio: light: isl29018: Remove non-standard sysfs attributes
  staging: iio: light: isl29018: Rename lux_scale to calibscale
  staging: iio: light: isl29018: Use standard sysfs attributes for scale
    and integration time

 drivers/staging/iio/light/isl29018.c | 294 +++++++++++++++++++----------------
 1 file changed, 163 insertions(+), 131 deletions(-)

-- 
1.9.1


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

* [PATCH v2 1/3] staging: iio: light: isl29018: Remove non-standard sysfs attributes
  2015-04-16 19:20 [PATCH v2 0/3] staging: iio: light: isl29018: Remove non-standard sysfs attributes Roberta Dobrescu
@ 2015-04-16 19:20 ` Roberta Dobrescu
  2015-04-26 19:46   ` Jonathan Cameron
  2015-04-16 19:20 ` [PATCH v2 2/3] staging: iio: light: isl29018: Rename lux_scale to calibscale Roberta Dobrescu
  2015-04-16 19:20 ` [PATCH v2 3/3] staging: iio: light: isl29018: Use standard sysfs attributes for scale and integration time Roberta Dobrescu
  2 siblings, 1 reply; 12+ messages in thread
From: Roberta Dobrescu @ 2015-04-16 19:20 UTC (permalink / raw)
  To: linux-iio; +Cc: jic23, knaack.h, lars, pmeerw, daniel.baluta, Roberta Dobrescu

This patch removes non-standard sysfs attributes range, range_available,
adc_resolution and adc_resolution_available. It also removes the
corresponding show and store functions.

This is in preparation for using standard IIO attributes in order to move
the code out of staging.

Signed-off-by: Roberta Dobrescu <roberta.dobrescu@gmail.com>
---
 drivers/staging/iio/light/isl29018.c | 94 ------------------------------------
 1 file changed, 94 deletions(-)

diff --git a/drivers/staging/iio/light/isl29018.c b/drivers/staging/iio/light/isl29018.c
index a348918..d3d0611 100644
--- a/drivers/staging/iio/light/isl29018.c
+++ b/drivers/staging/iio/light/isl29018.c
@@ -230,87 +230,6 @@ static int isl29018_read_proximity_ir(struct isl29018_chip *chip, int scheme,
 }
 
 /* Sysfs interface */
-/* range */
-static ssize_t show_range(struct device *dev,
-			struct device_attribute *attr, char *buf)
-{
-	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-	struct isl29018_chip *chip = iio_priv(indio_dev);
-
-	return sprintf(buf, "%u\n", chip->range);
-}
-
-static ssize_t store_range(struct device *dev,
-		struct device_attribute *attr, const char *buf, size_t count)
-{
-	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-	struct isl29018_chip *chip = iio_priv(indio_dev);
-	int status;
-	unsigned long lval;
-	unsigned int new_range;
-
-	if (kstrtoul(buf, 10, &lval))
-		return -EINVAL;
-
-	if (!(lval == 1000UL || lval == 4000UL ||
-			lval == 16000UL || lval == 64000UL)) {
-		dev_err(dev, "The range is not supported\n");
-		return -EINVAL;
-	}
-
-	mutex_lock(&chip->lock);
-	status = isl29018_set_range(chip, lval, &new_range);
-	if (status < 0) {
-		mutex_unlock(&chip->lock);
-		dev_err(dev,
-			"Error in setting max range with err %d\n", status);
-		return status;
-	}
-	chip->range = new_range;
-	mutex_unlock(&chip->lock);
-
-	return count;
-}
-
-/* resolution */
-static ssize_t show_resolution(struct device *dev,
-			struct device_attribute *attr, char *buf)
-{
-	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-	struct isl29018_chip *chip = iio_priv(indio_dev);
-
-	return sprintf(buf, "%u\n", chip->adc_bit);
-}
-
-static ssize_t store_resolution(struct device *dev,
-		struct device_attribute *attr, const char *buf, size_t count)
-{
-	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-	struct isl29018_chip *chip = iio_priv(indio_dev);
-	int status;
-	unsigned int val;
-	unsigned int new_adc_bit;
-
-	if (kstrtouint(buf, 10, &val))
-		return -EINVAL;
-	if (!(val == 4 || val == 8 || val == 12 || val == 16)) {
-		dev_err(dev, "The resolution is not supported\n");
-		return -EINVAL;
-	}
-
-	mutex_lock(&chip->lock);
-	status = isl29018_set_resolution(chip, val, &new_adc_bit);
-	if (status < 0) {
-		mutex_unlock(&chip->lock);
-		dev_err(dev, "Error in setting resolution\n");
-		return status;
-	}
-	chip->adc_bit = new_adc_bit;
-	mutex_unlock(&chip->lock);
-
-	return count;
-}
-
 /* proximity scheme */
 static ssize_t show_prox_infrared_suppression(struct device *dev,
 			struct device_attribute *attr, char *buf)
@@ -447,11 +366,6 @@ static const struct iio_chan_spec isl29023_channels[] = {
 	ISL29018_IR_CHANNEL,
 };
 
-static IIO_DEVICE_ATTR(range, S_IRUGO | S_IWUSR, show_range, store_range, 0);
-static IIO_CONST_ATTR(range_available, "1000 4000 16000 64000");
-static IIO_CONST_ATTR(adc_resolution_available, "4 8 12 16");
-static IIO_DEVICE_ATTR(adc_resolution, S_IRUGO | S_IWUSR,
-					show_resolution, store_resolution, 0);
 static IIO_DEVICE_ATTR(proximity_on_chip_ambient_infrared_suppression,
 					S_IRUGO | S_IWUSR,
 					show_prox_infrared_suppression,
@@ -460,19 +374,11 @@ static IIO_DEVICE_ATTR(proximity_on_chip_ambient_infrared_suppression,
 #define ISL29018_DEV_ATTR(name) (&iio_dev_attr_##name.dev_attr.attr)
 #define ISL29018_CONST_ATTR(name) (&iio_const_attr_##name.dev_attr.attr)
 static struct attribute *isl29018_attributes[] = {
-	ISL29018_DEV_ATTR(range),
-	ISL29018_CONST_ATTR(range_available),
-	ISL29018_DEV_ATTR(adc_resolution),
-	ISL29018_CONST_ATTR(adc_resolution_available),
 	ISL29018_DEV_ATTR(proximity_on_chip_ambient_infrared_suppression),
 	NULL
 };
 
 static struct attribute *isl29023_attributes[] = {
-	ISL29018_DEV_ATTR(range),
-	ISL29018_CONST_ATTR(range_available),
-	ISL29018_DEV_ATTR(adc_resolution),
-	ISL29018_CONST_ATTR(adc_resolution_available),
 	NULL
 };
 
-- 
1.9.1

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

* [PATCH v2 2/3] staging: iio: light: isl29018: Rename lux_scale to calibscale
  2015-04-16 19:20 [PATCH v2 0/3] staging: iio: light: isl29018: Remove non-standard sysfs attributes Roberta Dobrescu
  2015-04-16 19:20 ` [PATCH v2 1/3] " Roberta Dobrescu
@ 2015-04-16 19:20 ` Roberta Dobrescu
  2015-04-26 19:46   ` Jonathan Cameron
  2015-04-16 19:20 ` [PATCH v2 3/3] staging: iio: light: isl29018: Use standard sysfs attributes for scale and integration time Roberta Dobrescu
  2 siblings, 1 reply; 12+ messages in thread
From: Roberta Dobrescu @ 2015-04-16 19:20 UTC (permalink / raw)
  To: linux-iio; +Cc: jic23, knaack.h, lars, pmeerw, daniel.baluta, Roberta Dobrescu

This patch renames lux_scale to calibscale and lux_uscale to
ucalibscale.

This is done in order to avoid confusion since these parameters are
used for hardware applied calibration.

Signed-off-by: Roberta Dobrescu <roberta.dobrescu@gmail.com>
---
 drivers/staging/iio/light/isl29018.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/iio/light/isl29018.c b/drivers/staging/iio/light/isl29018.c
index d3d0611..ffc3d1b 100644
--- a/drivers/staging/iio/light/isl29018.c
+++ b/drivers/staging/iio/light/isl29018.c
@@ -71,8 +71,8 @@ struct isl29018_chip {
 	struct regmap		*regmap;
 	struct mutex		lock;
 	int			type;
-	unsigned int		lux_scale;
-	unsigned int		lux_uscale;
+	unsigned int		calibscale;
+	unsigned int		ucalibscale;
 	unsigned int		range;
 	unsigned int		adc_bit;
 	int			prox_scheme;
@@ -165,12 +165,12 @@ static int isl29018_read_lux(struct isl29018_chip *chip, int *lux)
 
 	/* To support fractional scaling, separate the unshifted lux
 	 * into two calculations: int scaling and micro-scaling.
-	 * lux_uscale ranges from 0-999999, so about 20 bits.  Split
+	 * ucalibscale ranges from 0-999999, so about 20 bits.  Split
 	 * the /1,000,000 in two to reduce the risk of over/underflow.
 	 */
 	data_x_range = lux_data * chip->range;
-	lux_unshifted = data_x_range * chip->lux_scale;
-	lux_unshifted += data_x_range / 1000 * chip->lux_uscale / 1000;
+	lux_unshifted = data_x_range * chip->calibscale;
+	lux_unshifted += data_x_range / 1000 * chip->ucalibscale / 1000;
 	*lux = lux_unshifted >> chip->adc_bit;
 
 	return 0;
@@ -277,9 +277,9 @@ static int isl29018_write_raw(struct iio_dev *indio_dev,
 
 	mutex_lock(&chip->lock);
 	if (mask == IIO_CHAN_INFO_CALIBSCALE && chan->type == IIO_LIGHT) {
-		chip->lux_scale = val;
+		chip->calibscale = val;
 		/* With no write_raw_get_fmt(), val2 is a MICRO fraction. */
-		chip->lux_uscale = val2;
+		chip->ucalibscale = val2;
 		ret = 0;
 	}
 	mutex_unlock(&chip->lock);
@@ -323,8 +323,8 @@ static int isl29018_read_raw(struct iio_dev *indio_dev,
 		break;
 	case IIO_CHAN_INFO_CALIBSCALE:
 		if (chan->type == IIO_LIGHT) {
-			*val = chip->lux_scale;
-			*val2 = chip->lux_uscale;
+			*val = chip->calibscale;
+			*val2 = chip->ucalibscale;
 			ret = IIO_VAL_INT_PLUS_MICRO;
 		}
 		break;
@@ -607,8 +607,8 @@ static int isl29018_probe(struct i2c_client *client,
 	mutex_init(&chip->lock);
 
 	chip->type = dev_id;
-	chip->lux_scale = 1;
-	chip->lux_uscale = 0;
+	chip->calibscale = 1;
+	chip->ucalibscale = 0;
 	chip->range = 1000;
 	chip->adc_bit = 16;
 	chip->suspended = false;
-- 
1.9.1

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

* [PATCH v2 3/3] staging: iio: light: isl29018: Use standard sysfs attributes for scale and integration time
  2015-04-16 19:20 [PATCH v2 0/3] staging: iio: light: isl29018: Remove non-standard sysfs attributes Roberta Dobrescu
  2015-04-16 19:20 ` [PATCH v2 1/3] " Roberta Dobrescu
  2015-04-16 19:20 ` [PATCH v2 2/3] staging: iio: light: isl29018: Rename lux_scale to calibscale Roberta Dobrescu
@ 2015-04-16 19:20 ` Roberta Dobrescu
  2015-04-26 19:51   ` Jonathan Cameron
  2015-06-27  9:34   ` Hartmut Knaack
  2 siblings, 2 replies; 12+ messages in thread
From: Roberta Dobrescu @ 2015-04-16 19:20 UTC (permalink / raw)
  To: linux-iio; +Cc: jic23, knaack.h, lars, pmeerw, daniel.baluta, Roberta Dobrescu

This patch refactors the isl29018 driver code in order to use standard
sysfs attributes for scale and integration time.

ISL29018 light sensor uses four ranges and four ADC's resolutions
which influence the calculated lux. Adc resolution is strongly
connected to integration time and range should be controlled by scale.

This patch introduces the usage of integration time and scale instead
of adc resolution and range.

Signed-off-by: Roberta Dobrescu <roberta.dobrescu@gmail.com>
---
 drivers/staging/iio/light/isl29018.c | 224 +++++++++++++++++++++++++++--------
 1 file changed, 175 insertions(+), 49 deletions(-)

diff --git a/drivers/staging/iio/light/isl29018.c b/drivers/staging/iio/light/isl29018.c
index ffc3d1b..435d417 100644
--- a/drivers/staging/iio/light/isl29018.c
+++ b/drivers/staging/iio/light/isl29018.c
@@ -66,6 +66,39 @@
 #define ISL29035_BOUT_SHIFT		0x07
 #define ISL29035_BOUT_MASK		(0x01 << ISL29035_BOUT_SHIFT)
 
+#define ISL29018_INT_TIME_AVAIL		"0.090000 0.005630 0.000351 0.000021"
+#define ISL29023_INT_TIME_AVAIL		"0.090000 0.005600 0.000352 0.000022"
+#define ISL29035_INT_TIME_AVAIL		"0.105000 0.006500 0.000410 0.000025"
+
+static const char * const int_time_avail[] = {
+	ISL29018_INT_TIME_AVAIL,
+	ISL29023_INT_TIME_AVAIL,
+	ISL29035_INT_TIME_AVAIL,
+};
+
+enum isl29018_int_time {
+	ISL29018_INT_TIME_16,
+	ISL29018_INT_TIME_12,
+	ISL29018_INT_TIME_8,
+	ISL29018_INT_TIME_4,
+};
+
+static const unsigned int isl29018_int_utimes[3][4] = {
+	{90000, 5630, 351, 21},
+	{90000, 5600, 352, 22},
+	{105000, 6500, 410, 25},
+};
+
+static const struct isl29018_scale {
+	unsigned int scale;
+	unsigned int uscale;
+} isl29018_scales[4][4] = {
+	{ {0, 15258}, {0, 61035}, {0, 244140}, {0, 976562} },
+	{ {0, 244140}, {0, 976562}, {3, 906250}, {15, 625000} },
+	{ {3, 906250}, {15, 625000}, {62, 500000}, {250, 0} },
+	{ {62, 500000}, {250, 0}, {1000, 0}, {4000, 0} }
+};
+
 struct isl29018_chip {
 	struct device		*dev;
 	struct regmap		*regmap;
@@ -73,51 +106,75 @@ struct isl29018_chip {
 	int			type;
 	unsigned int		calibscale;
 	unsigned int		ucalibscale;
-	unsigned int		range;
-	unsigned int		adc_bit;
+	unsigned int		int_time;
+	struct isl29018_scale	scale;
 	int			prox_scheme;
 	bool			suspended;
 };
 
-static int isl29018_set_range(struct isl29018_chip *chip, unsigned long range,
-		unsigned int *new_range)
+static int isl29018_set_integration_time(struct isl29018_chip *chip,
+					 unsigned int utime)
 {
-	static const unsigned long supp_ranges[] = {1000, 4000, 16000, 64000};
-	int i;
-
-	for (i = 0; i < ARRAY_SIZE(supp_ranges); ++i) {
-		if (range <= supp_ranges[i]) {
-			*new_range = (unsigned int)supp_ranges[i];
+	int i, ret;
+	unsigned int int_time, new_int_time;
+	struct isl29018_scale new_scale;
+
+	for (i = 0; i < ARRAY_SIZE(isl29018_int_utimes[chip->type]); ++i) {
+		if (utime == isl29018_int_utimes[chip->type][i]) {
+			new_int_time = i;
+			new_scale = isl29018_scales[new_int_time][0];
 			break;
 		}
 	}
 
-	if (i >= ARRAY_SIZE(supp_ranges))
+	if (i >= ARRAY_SIZE(isl29018_int_utimes[chip->type]))
 		return -EINVAL;
 
-	return regmap_update_bits(chip->regmap, ISL29018_REG_ADD_COMMANDII,
-			COMMANDII_RANGE_MASK, i << COMMANDII_RANGE_SHIFT);
+	ret = regmap_update_bits(chip->regmap, ISL29018_REG_ADD_COMMANDII,
+				 COMMANDII_RESOLUTION_MASK,
+				 i << COMMANDII_RESOLUTION_SHIFT);
+	if (ret < 0)
+		return ret;
+
+	/* keep the same range when integration time changes */
+	int_time = chip->int_time;
+	for (i = 0; i < ARRAY_SIZE(isl29018_scales[int_time]); ++i) {
+		if (chip->scale.scale == isl29018_scales[int_time][i].scale &&
+		    chip->scale.uscale == isl29018_scales[int_time][i].uscale) {
+			chip->scale = isl29018_scales[new_int_time][i];
+			break;
+		}
+	}
+	chip->int_time = new_int_time;
+
+	return 0;
 }
 
-static int isl29018_set_resolution(struct isl29018_chip *chip,
-			unsigned long adcbit, unsigned int *conf_adc_bit)
+static int isl29018_set_scale(struct isl29018_chip *chip, int scale, int uscale)
 {
-	static const unsigned long supp_adcbit[] = {16, 12, 8, 4};
-	int i;
+	int i, ret;
+	struct isl29018_scale new_scale;
 
-	for (i = 0; i < ARRAY_SIZE(supp_adcbit); ++i) {
-		if (adcbit >= supp_adcbit[i]) {
-			*conf_adc_bit = (unsigned int)supp_adcbit[i];
+	for (i = 0; i < ARRAY_SIZE(isl29018_scales[chip->int_time]); ++i) {
+		if (scale == isl29018_scales[chip->int_time][i].scale &&
+		    uscale == isl29018_scales[chip->int_time][i].uscale) {
+			new_scale = isl29018_scales[chip->int_time][i];
 			break;
 		}
 	}
 
-	if (i >= ARRAY_SIZE(supp_adcbit))
+	if (i >= ARRAY_SIZE(isl29018_scales[chip->int_time]))
 		return -EINVAL;
 
-	return regmap_update_bits(chip->regmap, ISL29018_REG_ADD_COMMANDII,
-			COMMANDII_RESOLUTION_MASK,
-			i << COMMANDII_RESOLUTION_SHIFT);
+	ret = regmap_update_bits(chip->regmap, ISL29018_REG_ADD_COMMANDII,
+				 COMMANDII_RANGE_MASK,
+				 i << COMMANDII_RANGE_SHIFT);
+	if (ret < 0)
+		return ret;
+
+	chip->scale = new_scale;
+
+	return 0;
 }
 
 static int isl29018_read_sensor_input(struct isl29018_chip *chip, int mode)
@@ -156,22 +213,17 @@ static int isl29018_read_sensor_input(struct isl29018_chip *chip, int mode)
 static int isl29018_read_lux(struct isl29018_chip *chip, int *lux)
 {
 	int lux_data;
-	unsigned int data_x_range, lux_unshifted;
+	unsigned int data_x_range;
 
 	lux_data = isl29018_read_sensor_input(chip, COMMMAND1_OPMODE_ALS_ONCE);
 
 	if (lux_data < 0)
 		return lux_data;
 
-	/* To support fractional scaling, separate the unshifted lux
-	 * into two calculations: int scaling and micro-scaling.
-	 * ucalibscale ranges from 0-999999, so about 20 bits.  Split
-	 * the /1,000,000 in two to reduce the risk of over/underflow.
-	 */
-	data_x_range = lux_data * chip->range;
-	lux_unshifted = data_x_range * chip->calibscale;
-	lux_unshifted += data_x_range / 1000 * chip->ucalibscale / 1000;
-	*lux = lux_unshifted >> chip->adc_bit;
+	data_x_range = lux_data * chip->scale.scale +
+		       lux_data * chip->scale.uscale / 1000000;
+	*lux = data_x_range * chip->calibscale +
+	       data_x_range * chip->ucalibscale / 1000000;
 
 	return 0;
 }
@@ -229,7 +281,39 @@ static int isl29018_read_proximity_ir(struct isl29018_chip *chip, int scheme,
 	return 0;
 }
 
-/* Sysfs interface */
+static ssize_t show_scale_available(struct device *dev,
+			struct device_attribute *attr, char *buf)
+{
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct isl29018_chip *chip = iio_priv(indio_dev);
+	int i, len = 0;
+
+	for (i = 0; i < ARRAY_SIZE(isl29018_scales[chip->int_time]); ++i)
+		len += sprintf(buf + len, "%d.%06d ",
+			       isl29018_scales[chip->int_time][i].scale,
+			       isl29018_scales[chip->int_time][i].uscale);
+
+	buf[len - 1] = '\n';
+
+	return len;
+}
+
+static ssize_t show_int_time_available(struct device *dev,
+			struct device_attribute *attr, char *buf)
+{
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct isl29018_chip *chip = iio_priv(indio_dev);
+	int i, len = 0;
+
+	for (i = 0; i < ARRAY_SIZE(isl29018_int_utimes[chip->type]); ++i)
+		len += sprintf(buf + len, "0.%06d ",
+			       isl29018_int_utimes[chip->type][i]);
+
+	buf[len - 1] = '\n';
+
+	return len;
+}
+
 /* proximity scheme */
 static ssize_t show_prox_infrared_suppression(struct device *dev,
 			struct device_attribute *attr, char *buf)
@@ -276,11 +360,26 @@ static int isl29018_write_raw(struct iio_dev *indio_dev,
 	int ret = -EINVAL;
 
 	mutex_lock(&chip->lock);
-	if (mask == IIO_CHAN_INFO_CALIBSCALE && chan->type == IIO_LIGHT) {
-		chip->calibscale = val;
-		/* With no write_raw_get_fmt(), val2 is a MICRO fraction. */
-		chip->ucalibscale = val2;
-		ret = 0;
+	switch (mask) {
+	case IIO_CHAN_INFO_CALIBSCALE:
+		if (chan->type == IIO_LIGHT) {
+			chip->calibscale = val;
+			chip->ucalibscale = val2;
+			ret = 0;
+		}
+		break;
+	case IIO_CHAN_INFO_INT_TIME:
+		if (chan->type == IIO_LIGHT)
+			if (val != 0)
+				return -EINVAL;
+			ret = isl29018_set_integration_time(chip, val2);
+		break;
+	case IIO_CHAN_INFO_SCALE:
+		if (chan->type == IIO_LIGHT)
+			ret = isl29018_set_scale(chip, val, val2);
+		break;
+	default:
+		break;
 	}
 	mutex_unlock(&chip->lock);
 
@@ -321,6 +420,20 @@ static int isl29018_read_raw(struct iio_dev *indio_dev,
 		if (!ret)
 			ret = IIO_VAL_INT;
 		break;
+	case IIO_CHAN_INFO_INT_TIME:
+		if (chan->type == IIO_LIGHT) {
+			*val = 0;
+			*val2 = isl29018_int_utimes[chip->type][chip->int_time];
+			ret = IIO_VAL_INT_PLUS_MICRO;
+		}
+		break;
+	case IIO_CHAN_INFO_SCALE:
+		if (chan->type == IIO_LIGHT) {
+			*val = chip->scale.scale;
+			*val2 = chip->scale.uscale;
+			ret = IIO_VAL_INT_PLUS_MICRO;
+		}
+		break;
 	case IIO_CHAN_INFO_CALIBSCALE:
 		if (chan->type == IIO_LIGHT) {
 			*val = chip->calibscale;
@@ -340,7 +453,9 @@ static int isl29018_read_raw(struct iio_dev *indio_dev,
 	.indexed = 1,							\
 	.channel = 0,							\
 	.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |		\
-	BIT(IIO_CHAN_INFO_CALIBSCALE),					\
+	BIT(IIO_CHAN_INFO_CALIBSCALE) |					\
+	BIT(IIO_CHAN_INFO_SCALE) |					\
+	BIT(IIO_CHAN_INFO_INT_TIME),					\
 }
 
 #define ISL29018_IR_CHANNEL {						\
@@ -366,19 +481,27 @@ static const struct iio_chan_spec isl29023_channels[] = {
 	ISL29018_IR_CHANNEL,
 };
 
+static IIO_DEVICE_ATTR(in_illuminance_integration_time_available, S_IRUGO,
+		       show_int_time_available, NULL, 0);
+static IIO_DEVICE_ATTR(in_illuminance_scale_available, S_IRUGO,
+		      show_scale_available, NULL, 0);
 static IIO_DEVICE_ATTR(proximity_on_chip_ambient_infrared_suppression,
 					S_IRUGO | S_IWUSR,
 					show_prox_infrared_suppression,
 					store_prox_infrared_suppression, 0);
 
 #define ISL29018_DEV_ATTR(name) (&iio_dev_attr_##name.dev_attr.attr)
-#define ISL29018_CONST_ATTR(name) (&iio_const_attr_##name.dev_attr.attr)
+
 static struct attribute *isl29018_attributes[] = {
+	ISL29018_DEV_ATTR(in_illuminance_scale_available),
+	ISL29018_DEV_ATTR(in_illuminance_integration_time_available),
 	ISL29018_DEV_ATTR(proximity_on_chip_ambient_infrared_suppression),
 	NULL
 };
 
 static struct attribute *isl29023_attributes[] = {
+	ISL29018_DEV_ATTR(in_illuminance_scale_available),
+	ISL29018_DEV_ATTR(in_illuminance_integration_time_available),
 	NULL
 };
 
@@ -422,8 +545,6 @@ enum {
 static int isl29018_chip_init(struct isl29018_chip *chip)
 {
 	int status;
-	unsigned int new_adc_bit;
-	unsigned int new_range;
 
 	if (chip->type == isl29035) {
 		status = isl29035_detect(chip);
@@ -472,14 +593,19 @@ static int isl29018_chip_init(struct isl29018_chip *chip)
 	usleep_range(1000, 2000);	/* per data sheet, page 10 */
 
 	/* set defaults */
-	status = isl29018_set_range(chip, chip->range, &new_range);
+	status = isl29018_set_scale(chip, chip->scale.scale,
+				    chip->scale.uscale);
 	if (status < 0) {
 		dev_err(chip->dev, "Init of isl29018 fails\n");
 		return status;
 	}
 
-	status = isl29018_set_resolution(chip, chip->adc_bit,
-						&new_adc_bit);
+	status = isl29018_set_integration_time(chip,
+			isl29018_int_utimes[chip->type][chip->int_time]);
+	if (status < 0) {
+		dev_err(chip->dev, "Init of isl29018 fails\n");
+		return status;
+	}
 
 	return 0;
 }
@@ -609,8 +735,8 @@ static int isl29018_probe(struct i2c_client *client,
 	chip->type = dev_id;
 	chip->calibscale = 1;
 	chip->ucalibscale = 0;
-	chip->range = 1000;
-	chip->adc_bit = 16;
+	chip->int_time = ISL29018_INT_TIME_16;
+	chip->scale = isl29018_scales[chip->int_time][0];
 	chip->suspended = false;
 
 	chip->regmap = devm_regmap_init_i2c(client,
-- 
1.9.1

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

* Re: [PATCH v2 1/3] staging: iio: light: isl29018: Remove non-standard sysfs attributes
  2015-04-16 19:20 ` [PATCH v2 1/3] " Roberta Dobrescu
@ 2015-04-26 19:46   ` Jonathan Cameron
  0 siblings, 0 replies; 12+ messages in thread
From: Jonathan Cameron @ 2015-04-26 19:46 UTC (permalink / raw)
  To: Roberta Dobrescu, linux-iio; +Cc: knaack.h, lars, pmeerw, daniel.baluta

On 16/04/15 20:20, Roberta Dobrescu wrote:
> This patch removes non-standard sysfs attributes range, range_available,
> adc_resolution and adc_resolution_available. It also removes the
> corresponding show and store functions.
> 
> This is in preparation for using standard IIO attributes in order to move
> the code out of staging.
> 
> Signed-off-by: Roberta Dobrescu <roberta.dobrescu@gmail.com>
Applied to the togreg branch of iio.git.  Initially pushed out as testing for
the autobuilders to play with it.

Thanks,

Jonathan
> ---
>  drivers/staging/iio/light/isl29018.c | 94 ------------------------------------
>  1 file changed, 94 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/isl29018.c b/drivers/staging/iio/light/isl29018.c
> index a348918..d3d0611 100644
> --- a/drivers/staging/iio/light/isl29018.c
> +++ b/drivers/staging/iio/light/isl29018.c
> @@ -230,87 +230,6 @@ static int isl29018_read_proximity_ir(struct isl29018_chip *chip, int scheme,
>  }
>  
>  /* Sysfs interface */
> -/* range */
> -static ssize_t show_range(struct device *dev,
> -			struct device_attribute *attr, char *buf)
> -{
> -	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> -	struct isl29018_chip *chip = iio_priv(indio_dev);
> -
> -	return sprintf(buf, "%u\n", chip->range);
> -}
> -
> -static ssize_t store_range(struct device *dev,
> -		struct device_attribute *attr, const char *buf, size_t count)
> -{
> -	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> -	struct isl29018_chip *chip = iio_priv(indio_dev);
> -	int status;
> -	unsigned long lval;
> -	unsigned int new_range;
> -
> -	if (kstrtoul(buf, 10, &lval))
> -		return -EINVAL;
> -
> -	if (!(lval == 1000UL || lval == 4000UL ||
> -			lval == 16000UL || lval == 64000UL)) {
> -		dev_err(dev, "The range is not supported\n");
> -		return -EINVAL;
> -	}
> -
> -	mutex_lock(&chip->lock);
> -	status = isl29018_set_range(chip, lval, &new_range);
> -	if (status < 0) {
> -		mutex_unlock(&chip->lock);
> -		dev_err(dev,
> -			"Error in setting max range with err %d\n", status);
> -		return status;
> -	}
> -	chip->range = new_range;
> -	mutex_unlock(&chip->lock);
> -
> -	return count;
> -}
> -
> -/* resolution */
> -static ssize_t show_resolution(struct device *dev,
> -			struct device_attribute *attr, char *buf)
> -{
> -	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> -	struct isl29018_chip *chip = iio_priv(indio_dev);
> -
> -	return sprintf(buf, "%u\n", chip->adc_bit);
> -}
> -
> -static ssize_t store_resolution(struct device *dev,
> -		struct device_attribute *attr, const char *buf, size_t count)
> -{
> -	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> -	struct isl29018_chip *chip = iio_priv(indio_dev);
> -	int status;
> -	unsigned int val;
> -	unsigned int new_adc_bit;
> -
> -	if (kstrtouint(buf, 10, &val))
> -		return -EINVAL;
> -	if (!(val == 4 || val == 8 || val == 12 || val == 16)) {
> -		dev_err(dev, "The resolution is not supported\n");
> -		return -EINVAL;
> -	}
> -
> -	mutex_lock(&chip->lock);
> -	status = isl29018_set_resolution(chip, val, &new_adc_bit);
> -	if (status < 0) {
> -		mutex_unlock(&chip->lock);
> -		dev_err(dev, "Error in setting resolution\n");
> -		return status;
> -	}
> -	chip->adc_bit = new_adc_bit;
> -	mutex_unlock(&chip->lock);
> -
> -	return count;
> -}
> -
>  /* proximity scheme */
>  static ssize_t show_prox_infrared_suppression(struct device *dev,
>  			struct device_attribute *attr, char *buf)
> @@ -447,11 +366,6 @@ static const struct iio_chan_spec isl29023_channels[] = {
>  	ISL29018_IR_CHANNEL,
>  };
>  
> -static IIO_DEVICE_ATTR(range, S_IRUGO | S_IWUSR, show_range, store_range, 0);
> -static IIO_CONST_ATTR(range_available, "1000 4000 16000 64000");
> -static IIO_CONST_ATTR(adc_resolution_available, "4 8 12 16");
> -static IIO_DEVICE_ATTR(adc_resolution, S_IRUGO | S_IWUSR,
> -					show_resolution, store_resolution, 0);
>  static IIO_DEVICE_ATTR(proximity_on_chip_ambient_infrared_suppression,
>  					S_IRUGO | S_IWUSR,
>  					show_prox_infrared_suppression,
> @@ -460,19 +374,11 @@ static IIO_DEVICE_ATTR(proximity_on_chip_ambient_infrared_suppression,
>  #define ISL29018_DEV_ATTR(name) (&iio_dev_attr_##name.dev_attr.attr)
>  #define ISL29018_CONST_ATTR(name) (&iio_const_attr_##name.dev_attr.attr)
>  static struct attribute *isl29018_attributes[] = {
> -	ISL29018_DEV_ATTR(range),
> -	ISL29018_CONST_ATTR(range_available),
> -	ISL29018_DEV_ATTR(adc_resolution),
> -	ISL29018_CONST_ATTR(adc_resolution_available),
>  	ISL29018_DEV_ATTR(proximity_on_chip_ambient_infrared_suppression),
>  	NULL
>  };
>  
>  static struct attribute *isl29023_attributes[] = {
> -	ISL29018_DEV_ATTR(range),
> -	ISL29018_CONST_ATTR(range_available),
> -	ISL29018_DEV_ATTR(adc_resolution),
> -	ISL29018_CONST_ATTR(adc_resolution_available),
>  	NULL
>  };
>  
> 


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

* Re: [PATCH v2 2/3] staging: iio: light: isl29018: Rename lux_scale to calibscale
  2015-04-16 19:20 ` [PATCH v2 2/3] staging: iio: light: isl29018: Rename lux_scale to calibscale Roberta Dobrescu
@ 2015-04-26 19:46   ` Jonathan Cameron
  0 siblings, 0 replies; 12+ messages in thread
From: Jonathan Cameron @ 2015-04-26 19:46 UTC (permalink / raw)
  To: Roberta Dobrescu, linux-iio; +Cc: knaack.h, lars, pmeerw, daniel.baluta

On 16/04/15 20:20, Roberta Dobrescu wrote:
> This patch renames lux_scale to calibscale and lux_uscale to
> ucalibscale.
> 
> This is done in order to avoid confusion since these parameters are
> used for hardware applied calibration.
> 
> Signed-off-by: Roberta Dobrescu <roberta.dobrescu@gmail.com>
Applied to the togreg branch of iio.git - initially pushed out as testing
for the autobuilders to play with it.

Thanks,

Jonathan
> ---
>  drivers/staging/iio/light/isl29018.c | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/isl29018.c b/drivers/staging/iio/light/isl29018.c
> index d3d0611..ffc3d1b 100644
> --- a/drivers/staging/iio/light/isl29018.c
> +++ b/drivers/staging/iio/light/isl29018.c
> @@ -71,8 +71,8 @@ struct isl29018_chip {
>  	struct regmap		*regmap;
>  	struct mutex		lock;
>  	int			type;
> -	unsigned int		lux_scale;
> -	unsigned int		lux_uscale;
> +	unsigned int		calibscale;
> +	unsigned int		ucalibscale;
>  	unsigned int		range;
>  	unsigned int		adc_bit;
>  	int			prox_scheme;
> @@ -165,12 +165,12 @@ static int isl29018_read_lux(struct isl29018_chip *chip, int *lux)
>  
>  	/* To support fractional scaling, separate the unshifted lux
>  	 * into two calculations: int scaling and micro-scaling.
> -	 * lux_uscale ranges from 0-999999, so about 20 bits.  Split
> +	 * ucalibscale ranges from 0-999999, so about 20 bits.  Split
>  	 * the /1,000,000 in two to reduce the risk of over/underflow.
>  	 */
>  	data_x_range = lux_data * chip->range;
> -	lux_unshifted = data_x_range * chip->lux_scale;
> -	lux_unshifted += data_x_range / 1000 * chip->lux_uscale / 1000;
> +	lux_unshifted = data_x_range * chip->calibscale;
> +	lux_unshifted += data_x_range / 1000 * chip->ucalibscale / 1000;
>  	*lux = lux_unshifted >> chip->adc_bit;
>  
>  	return 0;
> @@ -277,9 +277,9 @@ static int isl29018_write_raw(struct iio_dev *indio_dev,
>  
>  	mutex_lock(&chip->lock);
>  	if (mask == IIO_CHAN_INFO_CALIBSCALE && chan->type == IIO_LIGHT) {
> -		chip->lux_scale = val;
> +		chip->calibscale = val;
>  		/* With no write_raw_get_fmt(), val2 is a MICRO fraction. */
> -		chip->lux_uscale = val2;
> +		chip->ucalibscale = val2;
>  		ret = 0;
>  	}
>  	mutex_unlock(&chip->lock);
> @@ -323,8 +323,8 @@ static int isl29018_read_raw(struct iio_dev *indio_dev,
>  		break;
>  	case IIO_CHAN_INFO_CALIBSCALE:
>  		if (chan->type == IIO_LIGHT) {
> -			*val = chip->lux_scale;
> -			*val2 = chip->lux_uscale;
> +			*val = chip->calibscale;
> +			*val2 = chip->ucalibscale;
>  			ret = IIO_VAL_INT_PLUS_MICRO;
>  		}
>  		break;
> @@ -607,8 +607,8 @@ static int isl29018_probe(struct i2c_client *client,
>  	mutex_init(&chip->lock);
>  
>  	chip->type = dev_id;
> -	chip->lux_scale = 1;
> -	chip->lux_uscale = 0;
> +	chip->calibscale = 1;
> +	chip->ucalibscale = 0;
>  	chip->range = 1000;
>  	chip->adc_bit = 16;
>  	chip->suspended = false;
> 


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

* Re: [PATCH v2 3/3] staging: iio: light: isl29018: Use standard sysfs attributes for scale and integration time
  2015-04-16 19:20 ` [PATCH v2 3/3] staging: iio: light: isl29018: Use standard sysfs attributes for scale and integration time Roberta Dobrescu
@ 2015-04-26 19:51   ` Jonathan Cameron
  2015-04-27 16:03     ` Rhyland Klein
  2015-06-27  9:34   ` Hartmut Knaack
  1 sibling, 1 reply; 12+ messages in thread
From: Jonathan Cameron @ 2015-04-26 19:51 UTC (permalink / raw)
  To: Roberta Dobrescu, linux-iio
  Cc: knaack.h, lars, pmeerw, daniel.baluta, Rhyland Klein

On 16/04/15 20:20, Roberta Dobrescu wrote:
> This patch refactors the isl29018 driver code in order to use standard
> sysfs attributes for scale and integration time.
> 
> ISL29018 light sensor uses four ranges and four ADC's resolutions
> which influence the calculated lux. Adc resolution is strongly
> connected to integration time and range should be controlled by scale.
> 
> This patch introduces the usage of integration time and scale instead
> of adc resolution and range.
> 
> Signed-off-by: Roberta Dobrescu <roberta.dobrescu@gmail.com>
Looks good to me,  Applied to the togreg branch of iio.git - initially pushed out
as testing.

cc'd Rhyland though not heard from him for quite a while.

Rhyland, if you are still interested in this driver, give me a shout and I can
hold these patches back until you get a chance to look at them.

Sorry, forgot to cc you earlier in the discussion!

Jonathan

> ---
>  drivers/staging/iio/light/isl29018.c | 224 +++++++++++++++++++++++++++--------
>  1 file changed, 175 insertions(+), 49 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/isl29018.c b/drivers/staging/iio/light/isl29018.c
> index ffc3d1b..435d417 100644
> --- a/drivers/staging/iio/light/isl29018.c
> +++ b/drivers/staging/iio/light/isl29018.c
> @@ -66,6 +66,39 @@
>  #define ISL29035_BOUT_SHIFT		0x07
>  #define ISL29035_BOUT_MASK		(0x01 << ISL29035_BOUT_SHIFT)
>  
> +#define ISL29018_INT_TIME_AVAIL		"0.090000 0.005630 0.000351 0.000021"
> +#define ISL29023_INT_TIME_AVAIL		"0.090000 0.005600 0.000352 0.000022"
> +#define ISL29035_INT_TIME_AVAIL		"0.105000 0.006500 0.000410 0.000025"
> +
> +static const char * const int_time_avail[] = {
> +	ISL29018_INT_TIME_AVAIL,
> +	ISL29023_INT_TIME_AVAIL,
> +	ISL29035_INT_TIME_AVAIL,
> +};
> +
> +enum isl29018_int_time {
> +	ISL29018_INT_TIME_16,
> +	ISL29018_INT_TIME_12,
> +	ISL29018_INT_TIME_8,
> +	ISL29018_INT_TIME_4,
> +};
> +
> +static const unsigned int isl29018_int_utimes[3][4] = {
> +	{90000, 5630, 351, 21},
> +	{90000, 5600, 352, 22},
> +	{105000, 6500, 410, 25},
> +};
> +
> +static const struct isl29018_scale {
> +	unsigned int scale;
> +	unsigned int uscale;
> +} isl29018_scales[4][4] = {
> +	{ {0, 15258}, {0, 61035}, {0, 244140}, {0, 976562} },
> +	{ {0, 244140}, {0, 976562}, {3, 906250}, {15, 625000} },
> +	{ {3, 906250}, {15, 625000}, {62, 500000}, {250, 0} },
> +	{ {62, 500000}, {250, 0}, {1000, 0}, {4000, 0} }
> +};
> +
>  struct isl29018_chip {
>  	struct device		*dev;
>  	struct regmap		*regmap;
> @@ -73,51 +106,75 @@ struct isl29018_chip {
>  	int			type;
>  	unsigned int		calibscale;
>  	unsigned int		ucalibscale;
> -	unsigned int		range;
> -	unsigned int		adc_bit;
> +	unsigned int		int_time;
> +	struct isl29018_scale	scale;
>  	int			prox_scheme;
>  	bool			suspended;
>  };
>  
> -static int isl29018_set_range(struct isl29018_chip *chip, unsigned long range,
> -		unsigned int *new_range)
> +static int isl29018_set_integration_time(struct isl29018_chip *chip,
> +					 unsigned int utime)
>  {
> -	static const unsigned long supp_ranges[] = {1000, 4000, 16000, 64000};
> -	int i;
> -
> -	for (i = 0; i < ARRAY_SIZE(supp_ranges); ++i) {
> -		if (range <= supp_ranges[i]) {
> -			*new_range = (unsigned int)supp_ranges[i];
> +	int i, ret;
> +	unsigned int int_time, new_int_time;
> +	struct isl29018_scale new_scale;
> +
> +	for (i = 0; i < ARRAY_SIZE(isl29018_int_utimes[chip->type]); ++i) {
> +		if (utime == isl29018_int_utimes[chip->type][i]) {
> +			new_int_time = i;
> +			new_scale = isl29018_scales[new_int_time][0];
>  			break;
>  		}
>  	}
>  
> -	if (i >= ARRAY_SIZE(supp_ranges))
> +	if (i >= ARRAY_SIZE(isl29018_int_utimes[chip->type]))
>  		return -EINVAL;
>  
> -	return regmap_update_bits(chip->regmap, ISL29018_REG_ADD_COMMANDII,
> -			COMMANDII_RANGE_MASK, i << COMMANDII_RANGE_SHIFT);
> +	ret = regmap_update_bits(chip->regmap, ISL29018_REG_ADD_COMMANDII,
> +				 COMMANDII_RESOLUTION_MASK,
> +				 i << COMMANDII_RESOLUTION_SHIFT);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* keep the same range when integration time changes */
> +	int_time = chip->int_time;
> +	for (i = 0; i < ARRAY_SIZE(isl29018_scales[int_time]); ++i) {
> +		if (chip->scale.scale == isl29018_scales[int_time][i].scale &&
> +		    chip->scale.uscale == isl29018_scales[int_time][i].uscale) {
> +			chip->scale = isl29018_scales[new_int_time][i];
> +			break;
> +		}
> +	}
> +	chip->int_time = new_int_time;
> +
> +	return 0;
>  }
>  
> -static int isl29018_set_resolution(struct isl29018_chip *chip,
> -			unsigned long adcbit, unsigned int *conf_adc_bit)
> +static int isl29018_set_scale(struct isl29018_chip *chip, int scale, int uscale)
>  {
> -	static const unsigned long supp_adcbit[] = {16, 12, 8, 4};
> -	int i;
> +	int i, ret;
> +	struct isl29018_scale new_scale;
>  
> -	for (i = 0; i < ARRAY_SIZE(supp_adcbit); ++i) {
> -		if (adcbit >= supp_adcbit[i]) {
> -			*conf_adc_bit = (unsigned int)supp_adcbit[i];
> +	for (i = 0; i < ARRAY_SIZE(isl29018_scales[chip->int_time]); ++i) {
> +		if (scale == isl29018_scales[chip->int_time][i].scale &&
> +		    uscale == isl29018_scales[chip->int_time][i].uscale) {
> +			new_scale = isl29018_scales[chip->int_time][i];
>  			break;
>  		}
>  	}
>  
> -	if (i >= ARRAY_SIZE(supp_adcbit))
> +	if (i >= ARRAY_SIZE(isl29018_scales[chip->int_time]))
>  		return -EINVAL;
>  
> -	return regmap_update_bits(chip->regmap, ISL29018_REG_ADD_COMMANDII,
> -			COMMANDII_RESOLUTION_MASK,
> -			i << COMMANDII_RESOLUTION_SHIFT);
> +	ret = regmap_update_bits(chip->regmap, ISL29018_REG_ADD_COMMANDII,
> +				 COMMANDII_RANGE_MASK,
> +				 i << COMMANDII_RANGE_SHIFT);
> +	if (ret < 0)
> +		return ret;
> +
> +	chip->scale = new_scale;
> +
> +	return 0;
>  }
>  
>  static int isl29018_read_sensor_input(struct isl29018_chip *chip, int mode)
> @@ -156,22 +213,17 @@ static int isl29018_read_sensor_input(struct isl29018_chip *chip, int mode)
>  static int isl29018_read_lux(struct isl29018_chip *chip, int *lux)
>  {
>  	int lux_data;
> -	unsigned int data_x_range, lux_unshifted;
> +	unsigned int data_x_range;
>  
>  	lux_data = isl29018_read_sensor_input(chip, COMMMAND1_OPMODE_ALS_ONCE);
>  
>  	if (lux_data < 0)
>  		return lux_data;
>  
> -	/* To support fractional scaling, separate the unshifted lux
> -	 * into two calculations: int scaling and micro-scaling.
> -	 * ucalibscale ranges from 0-999999, so about 20 bits.  Split
> -	 * the /1,000,000 in two to reduce the risk of over/underflow.
> -	 */
> -	data_x_range = lux_data * chip->range;
> -	lux_unshifted = data_x_range * chip->calibscale;
> -	lux_unshifted += data_x_range / 1000 * chip->ucalibscale / 1000;
> -	*lux = lux_unshifted >> chip->adc_bit;
> +	data_x_range = lux_data * chip->scale.scale +
> +		       lux_data * chip->scale.uscale / 1000000;
> +	*lux = data_x_range * chip->calibscale +
> +	       data_x_range * chip->ucalibscale / 1000000;
>  
>  	return 0;
>  }
> @@ -229,7 +281,39 @@ static int isl29018_read_proximity_ir(struct isl29018_chip *chip, int scheme,
>  	return 0;
>  }
>  
> -/* Sysfs interface */
> +static ssize_t show_scale_available(struct device *dev,
> +			struct device_attribute *attr, char *buf)
> +{
> +	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> +	struct isl29018_chip *chip = iio_priv(indio_dev);
> +	int i, len = 0;
> +
> +	for (i = 0; i < ARRAY_SIZE(isl29018_scales[chip->int_time]); ++i)
> +		len += sprintf(buf + len, "%d.%06d ",
> +			       isl29018_scales[chip->int_time][i].scale,
> +			       isl29018_scales[chip->int_time][i].uscale);
> +
> +	buf[len - 1] = '\n';
> +
> +	return len;
> +}
> +
> +static ssize_t show_int_time_available(struct device *dev,
> +			struct device_attribute *attr, char *buf)
> +{
> +	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> +	struct isl29018_chip *chip = iio_priv(indio_dev);
> +	int i, len = 0;
> +
> +	for (i = 0; i < ARRAY_SIZE(isl29018_int_utimes[chip->type]); ++i)
> +		len += sprintf(buf + len, "0.%06d ",
> +			       isl29018_int_utimes[chip->type][i]);
> +
> +	buf[len - 1] = '\n';
> +
> +	return len;
> +}
> +
>  /* proximity scheme */
>  static ssize_t show_prox_infrared_suppression(struct device *dev,
>  			struct device_attribute *attr, char *buf)
> @@ -276,11 +360,26 @@ static int isl29018_write_raw(struct iio_dev *indio_dev,
>  	int ret = -EINVAL;
>  
>  	mutex_lock(&chip->lock);
> -	if (mask == IIO_CHAN_INFO_CALIBSCALE && chan->type == IIO_LIGHT) {
> -		chip->calibscale = val;
> -		/* With no write_raw_get_fmt(), val2 is a MICRO fraction. */
> -		chip->ucalibscale = val2;
> -		ret = 0;
> +	switch (mask) {
> +	case IIO_CHAN_INFO_CALIBSCALE:
> +		if (chan->type == IIO_LIGHT) {
> +			chip->calibscale = val;
> +			chip->ucalibscale = val2;
> +			ret = 0;
> +		}
> +		break;
> +	case IIO_CHAN_INFO_INT_TIME:
> +		if (chan->type == IIO_LIGHT)
> +			if (val != 0)
> +				return -EINVAL;
> +			ret = isl29018_set_integration_time(chip, val2);
> +		break;
> +	case IIO_CHAN_INFO_SCALE:
> +		if (chan->type == IIO_LIGHT)
> +			ret = isl29018_set_scale(chip, val, val2);
> +		break;
> +	default:
> +		break;
>  	}
>  	mutex_unlock(&chip->lock);
>  
> @@ -321,6 +420,20 @@ static int isl29018_read_raw(struct iio_dev *indio_dev,
>  		if (!ret)
>  			ret = IIO_VAL_INT;
>  		break;
> +	case IIO_CHAN_INFO_INT_TIME:
> +		if (chan->type == IIO_LIGHT) {
> +			*val = 0;
> +			*val2 = isl29018_int_utimes[chip->type][chip->int_time];
> +			ret = IIO_VAL_INT_PLUS_MICRO;
> +		}
> +		break;
> +	case IIO_CHAN_INFO_SCALE:
> +		if (chan->type == IIO_LIGHT) {
> +			*val = chip->scale.scale;
> +			*val2 = chip->scale.uscale;
> +			ret = IIO_VAL_INT_PLUS_MICRO;
> +		}
> +		break;
>  	case IIO_CHAN_INFO_CALIBSCALE:
>  		if (chan->type == IIO_LIGHT) {
>  			*val = chip->calibscale;
> @@ -340,7 +453,9 @@ static int isl29018_read_raw(struct iio_dev *indio_dev,
>  	.indexed = 1,							\
>  	.channel = 0,							\
>  	.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |		\
> -	BIT(IIO_CHAN_INFO_CALIBSCALE),					\
> +	BIT(IIO_CHAN_INFO_CALIBSCALE) |					\
> +	BIT(IIO_CHAN_INFO_SCALE) |					\
> +	BIT(IIO_CHAN_INFO_INT_TIME),					\
>  }
>  
>  #define ISL29018_IR_CHANNEL {						\
> @@ -366,19 +481,27 @@ static const struct iio_chan_spec isl29023_channels[] = {
>  	ISL29018_IR_CHANNEL,
>  };
>  
> +static IIO_DEVICE_ATTR(in_illuminance_integration_time_available, S_IRUGO,
> +		       show_int_time_available, NULL, 0);
> +static IIO_DEVICE_ATTR(in_illuminance_scale_available, S_IRUGO,
> +		      show_scale_available, NULL, 0);
>  static IIO_DEVICE_ATTR(proximity_on_chip_ambient_infrared_suppression,
>  					S_IRUGO | S_IWUSR,
>  					show_prox_infrared_suppression,
>  					store_prox_infrared_suppression, 0);
>  
>  #define ISL29018_DEV_ATTR(name) (&iio_dev_attr_##name.dev_attr.attr)
> -#define ISL29018_CONST_ATTR(name) (&iio_const_attr_##name.dev_attr.attr)
> +
>  static struct attribute *isl29018_attributes[] = {
> +	ISL29018_DEV_ATTR(in_illuminance_scale_available),
> +	ISL29018_DEV_ATTR(in_illuminance_integration_time_available),
>  	ISL29018_DEV_ATTR(proximity_on_chip_ambient_infrared_suppression),
>  	NULL
>  };
>  
>  static struct attribute *isl29023_attributes[] = {
> +	ISL29018_DEV_ATTR(in_illuminance_scale_available),
> +	ISL29018_DEV_ATTR(in_illuminance_integration_time_available),
>  	NULL
>  };
>  
> @@ -422,8 +545,6 @@ enum {
>  static int isl29018_chip_init(struct isl29018_chip *chip)
>  {
>  	int status;
> -	unsigned int new_adc_bit;
> -	unsigned int new_range;
>  
>  	if (chip->type == isl29035) {
>  		status = isl29035_detect(chip);
> @@ -472,14 +593,19 @@ static int isl29018_chip_init(struct isl29018_chip *chip)
>  	usleep_range(1000, 2000);	/* per data sheet, page 10 */
>  
>  	/* set defaults */
> -	status = isl29018_set_range(chip, chip->range, &new_range);
> +	status = isl29018_set_scale(chip, chip->scale.scale,
> +				    chip->scale.uscale);
>  	if (status < 0) {
>  		dev_err(chip->dev, "Init of isl29018 fails\n");
>  		return status;
>  	}
>  
> -	status = isl29018_set_resolution(chip, chip->adc_bit,
> -						&new_adc_bit);
> +	status = isl29018_set_integration_time(chip,
> +			isl29018_int_utimes[chip->type][chip->int_time]);
> +	if (status < 0) {
> +		dev_err(chip->dev, "Init of isl29018 fails\n");
> +		return status;
> +	}
>  
>  	return 0;
>  }
> @@ -609,8 +735,8 @@ static int isl29018_probe(struct i2c_client *client,
>  	chip->type = dev_id;
>  	chip->calibscale = 1;
>  	chip->ucalibscale = 0;
> -	chip->range = 1000;
> -	chip->adc_bit = 16;
> +	chip->int_time = ISL29018_INT_TIME_16;
> +	chip->scale = isl29018_scales[chip->int_time][0];
>  	chip->suspended = false;
>  
>  	chip->regmap = devm_regmap_init_i2c(client,
> 


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

* Re: [PATCH v2 3/3] staging: iio: light: isl29018: Use standard sysfs attributes for scale and integration time
  2015-04-26 19:51   ` Jonathan Cameron
@ 2015-04-27 16:03     ` Rhyland Klein
  2015-05-02 11:02       ` Jonathan Cameron
  0 siblings, 1 reply; 12+ messages in thread
From: Rhyland Klein @ 2015-04-27 16:03 UTC (permalink / raw)
  To: Jonathan Cameron, Roberta Dobrescu, linux-iio
  Cc: knaack.h, lars, pmeerw, daniel.baluta

On 4/26/2015 3:51 PM, Jonathan Cameron wrote:
> On 16/04/15 20:20, Roberta Dobrescu wrote:
>> This patch refactors the isl29018 driver code in order to use standard
>> sysfs attributes for scale and integration time.
>>
>> ISL29018 light sensor uses four ranges and four ADC's resolutions
>> which influence the calculated lux. Adc resolution is strongly
>> connected to integration time and range should be controlled by scale.
>>
>> This patch introduces the usage of integration time and scale instead
>> of adc resolution and range.
>>
>> Signed-off-by: Roberta Dobrescu <roberta.dobrescu@gmail.com>
> Looks good to me,  Applied to the togreg branch of iio.git - initially pushed out
> as testing.
> 
> cc'd Rhyland though not heard from him for quite a while.
> 
> Rhyland, if you are still interested in this driver, give me a shout and I can
> hold these patches back until you get a chance to look at them.
> 
> Sorry, forgot to cc you earlier in the discussion!
> 
> Jonathan
> 

Its fine, don't hold up patches on my account. I haven't looked at this
in a while, so don't gate on me :)

-rhyland


-- 
nvpublic

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

* Re: [PATCH v2 3/3] staging: iio: light: isl29018: Use standard sysfs attributes for scale and integration time
  2015-04-27 16:03     ` Rhyland Klein
@ 2015-05-02 11:02       ` Jonathan Cameron
  2015-05-02 15:55         ` Roberta Dobrescu
  0 siblings, 1 reply; 12+ messages in thread
From: Jonathan Cameron @ 2015-05-02 11:02 UTC (permalink / raw)
  To: Rhyland Klein, Roberta Dobrescu, linux-iio
  Cc: knaack.h, lars, pmeerw, daniel.baluta

On 27/04/15 17:03, Rhyland Klein wrote:
> On 4/26/2015 3:51 PM, Jonathan Cameron wrote:
>> On 16/04/15 20:20, Roberta Dobrescu wrote:
>>> This patch refactors the isl29018 driver code in order to use standard
>>> sysfs attributes for scale and integration time.
>>>
>>> ISL29018 light sensor uses four ranges and four ADC's resolutions
>>> which influence the calculated lux. Adc resolution is strongly
>>> connected to integration time and range should be controlled by scale.
>>>
>>> This patch introduces the usage of integration time and scale instead
>>> of adc resolution and range.
>>>
>>> Signed-off-by: Roberta Dobrescu <roberta.dobrescu@gmail.com>
>> Looks good to me,  Applied to the togreg branch of iio.git - initially pushed out
>> as testing.
>>
>> cc'd Rhyland though not heard from him for quite a while.
>>
>> Rhyland, if you are still interested in this driver, give me a shout and I can
>> hold these patches back until you get a chance to look at them.
>>
>> Sorry, forgot to cc you earlier in the discussion!
>>
>> Jonathan
>>
> 
> Its fine, don't hold up patches on my account. I haven't looked at this
> in a while, so don't gate on me :)
> 
> -rhyland

I missed a small issue with some missing brackets in one of the switch statements.
The autobuilder caught it as a formatting issue:
drivers/staging/iio/light/isl29018.c:373:3-4: code aligned with following code on line 377.
I'll send out  a patch in a mo, but as it's trivial am happy to apply it without an ack.

note this isn't actually a bug due to the check being strictly unnecessary anyway, but
it is clearly not as intended!

Jonathan
> 
> 


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

* Re: [PATCH v2 3/3] staging: iio: light: isl29018: Use standard sysfs attributes for scale and integration time
  2015-05-02 11:02       ` Jonathan Cameron
@ 2015-05-02 15:55         ` Roberta Dobrescu
  0 siblings, 0 replies; 12+ messages in thread
From: Roberta Dobrescu @ 2015-05-02 15:55 UTC (permalink / raw)
  To: Jonathan Cameron, Rhyland Klein, linux-iio
  Cc: knaack.h, lars, pmeerw, daniel.baluta

On 02.05.2015 14:02, Jonathan Cameron wrote:
> I missed a small issue with some missing brackets in one of the switch statements.
> The autobuilder caught it as a formatting issue:
> drivers/staging/iio/light/isl29018.c:373:3-4: code aligned with following code on line 377.
> I'll send out  a patch in a mo, but as it's trivial am happy to apply it without an ack.
>
> note this isn't actually a bug due to the check being strictly unnecessary anyway, but
> it is clearly not as intended!

Oops. Thanks for fixing this, Jonathan!

Roberta

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

* Re: [PATCH v2 3/3] staging: iio: light: isl29018: Use standard sysfs attributes for scale and integration time
  2015-04-16 19:20 ` [PATCH v2 3/3] staging: iio: light: isl29018: Use standard sysfs attributes for scale and integration time Roberta Dobrescu
  2015-04-26 19:51   ` Jonathan Cameron
@ 2015-06-27  9:34   ` Hartmut Knaack
  2015-06-29  7:24     ` Daniel Baluta
  1 sibling, 1 reply; 12+ messages in thread
From: Hartmut Knaack @ 2015-06-27  9:34 UTC (permalink / raw)
  To: Roberta Dobrescu, linux-iio; +Cc: jic23, lars, pmeerw, daniel.baluta

Roberta Dobrescu schrieb am 16.04.2015 um 21:20:
> This patch refactors the isl29018 driver code in order to use standard
> sysfs attributes for scale and integration time.
> 
> ISL29018 light sensor uses four ranges and four ADC's resolutions
> which influence the calculated lux. Adc resolution is strongly
> connected to integration time and range should be controlled by scale.
> 
> This patch introduces the usage of integration time and scale instead
> of adc resolution and range.
> 
> Signed-off-by: Roberta Dobrescu <roberta.dobrescu@gmail.com>
> ---
>  drivers/staging/iio/light/isl29018.c | 224 +++++++++++++++++++++++++++--------
>  1 file changed, 175 insertions(+), 49 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/isl29018.c b/drivers/staging/iio/light/isl29018.c
> index ffc3d1b..435d417 100644
> --- a/drivers/staging/iio/light/isl29018.c
> +++ b/drivers/staging/iio/light/isl29018.c
> @@ -66,6 +66,39 @@
>  #define ISL29035_BOUT_SHIFT		0x07
>  #define ISL29035_BOUT_MASK		(0x01 << ISL29035_BOUT_SHIFT)
>  
> +#define ISL29018_INT_TIME_AVAIL		"0.090000 0.005630 0.000351 0.000021"
> +#define ISL29023_INT_TIME_AVAIL		"0.090000 0.005600 0.000352 0.000022"
> +#define ISL29035_INT_TIME_AVAIL		"0.105000 0.006500 0.000410 0.000025"
> +
> +static const char * const int_time_avail[] = {
> +	ISL29018_INT_TIME_AVAIL,
> +	ISL29023_INT_TIME_AVAIL,
> +	ISL29035_INT_TIME_AVAIL,
> +};
> +

Hi Roberta,
are you still working to get this one out of staging? You introduced here the
int_time_avail array (missing prefix), although you don't make use of it down
in show_int_time_available() (also missing prefix), where I would expect it to
be used. So, what's the status? Do you intend to use this array later on, or
can it be dropped?
Just for completeness: show_scale_available also misses a prefix.
Thanks,

Hartmut

> +enum isl29018_int_time {
> +	ISL29018_INT_TIME_16,
> +	ISL29018_INT_TIME_12,
> +	ISL29018_INT_TIME_8,
> +	ISL29018_INT_TIME_4,
> +};
> +
> +static const unsigned int isl29018_int_utimes[3][4] = {
> +	{90000, 5630, 351, 21},
> +	{90000, 5600, 352, 22},
> +	{105000, 6500, 410, 25},
> +};
> +
> +static const struct isl29018_scale {
> +	unsigned int scale;
> +	unsigned int uscale;
> +} isl29018_scales[4][4] = {
> +	{ {0, 15258}, {0, 61035}, {0, 244140}, {0, 976562} },
> +	{ {0, 244140}, {0, 976562}, {3, 906250}, {15, 625000} },
> +	{ {3, 906250}, {15, 625000}, {62, 500000}, {250, 0} },
> +	{ {62, 500000}, {250, 0}, {1000, 0}, {4000, 0} }
> +};
> +
>  struct isl29018_chip {
>  	struct device		*dev;
>  	struct regmap		*regmap;
> @@ -73,51 +106,75 @@ struct isl29018_chip {
>  	int			type;
>  	unsigned int		calibscale;
>  	unsigned int		ucalibscale;
> -	unsigned int		range;
> -	unsigned int		adc_bit;
> +	unsigned int		int_time;
> +	struct isl29018_scale	scale;
>  	int			prox_scheme;
>  	bool			suspended;
>  };
>  
> -static int isl29018_set_range(struct isl29018_chip *chip, unsigned long range,
> -		unsigned int *new_range)
> +static int isl29018_set_integration_time(struct isl29018_chip *chip,
> +					 unsigned int utime)
>  {
> -	static const unsigned long supp_ranges[] = {1000, 4000, 16000, 64000};
> -	int i;
> -
> -	for (i = 0; i < ARRAY_SIZE(supp_ranges); ++i) {
> -		if (range <= supp_ranges[i]) {
> -			*new_range = (unsigned int)supp_ranges[i];
> +	int i, ret;
> +	unsigned int int_time, new_int_time;
> +	struct isl29018_scale new_scale;
> +
> +	for (i = 0; i < ARRAY_SIZE(isl29018_int_utimes[chip->type]); ++i) {
> +		if (utime == isl29018_int_utimes[chip->type][i]) {
> +			new_int_time = i;
> +			new_scale = isl29018_scales[new_int_time][0];
>  			break;
>  		}
>  	}
>  
> -	if (i >= ARRAY_SIZE(supp_ranges))
> +	if (i >= ARRAY_SIZE(isl29018_int_utimes[chip->type]))
>  		return -EINVAL;
>  
> -	return regmap_update_bits(chip->regmap, ISL29018_REG_ADD_COMMANDII,
> -			COMMANDII_RANGE_MASK, i << COMMANDII_RANGE_SHIFT);
> +	ret = regmap_update_bits(chip->regmap, ISL29018_REG_ADD_COMMANDII,
> +				 COMMANDII_RESOLUTION_MASK,
> +				 i << COMMANDII_RESOLUTION_SHIFT);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* keep the same range when integration time changes */
> +	int_time = chip->int_time;
> +	for (i = 0; i < ARRAY_SIZE(isl29018_scales[int_time]); ++i) {
> +		if (chip->scale.scale == isl29018_scales[int_time][i].scale &&
> +		    chip->scale.uscale == isl29018_scales[int_time][i].uscale) {
> +			chip->scale = isl29018_scales[new_int_time][i];
> +			break;
> +		}
> +	}
> +	chip->int_time = new_int_time;
> +
> +	return 0;
>  }
>  
> -static int isl29018_set_resolution(struct isl29018_chip *chip,
> -			unsigned long adcbit, unsigned int *conf_adc_bit)
> +static int isl29018_set_scale(struct isl29018_chip *chip, int scale, int uscale)
>  {
> -	static const unsigned long supp_adcbit[] = {16, 12, 8, 4};
> -	int i;
> +	int i, ret;
> +	struct isl29018_scale new_scale;
>  
> -	for (i = 0; i < ARRAY_SIZE(supp_adcbit); ++i) {
> -		if (adcbit >= supp_adcbit[i]) {
> -			*conf_adc_bit = (unsigned int)supp_adcbit[i];
> +	for (i = 0; i < ARRAY_SIZE(isl29018_scales[chip->int_time]); ++i) {
> +		if (scale == isl29018_scales[chip->int_time][i].scale &&
> +		    uscale == isl29018_scales[chip->int_time][i].uscale) {
> +			new_scale = isl29018_scales[chip->int_time][i];
>  			break;
>  		}
>  	}
>  
> -	if (i >= ARRAY_SIZE(supp_adcbit))
> +	if (i >= ARRAY_SIZE(isl29018_scales[chip->int_time]))
>  		return -EINVAL;
>  
> -	return regmap_update_bits(chip->regmap, ISL29018_REG_ADD_COMMANDII,
> -			COMMANDII_RESOLUTION_MASK,
> -			i << COMMANDII_RESOLUTION_SHIFT);
> +	ret = regmap_update_bits(chip->regmap, ISL29018_REG_ADD_COMMANDII,
> +				 COMMANDII_RANGE_MASK,
> +				 i << COMMANDII_RANGE_SHIFT);
> +	if (ret < 0)
> +		return ret;
> +
> +	chip->scale = new_scale;
> +
> +	return 0;
>  }
>  
>  static int isl29018_read_sensor_input(struct isl29018_chip *chip, int mode)
> @@ -156,22 +213,17 @@ static int isl29018_read_sensor_input(struct isl29018_chip *chip, int mode)
>  static int isl29018_read_lux(struct isl29018_chip *chip, int *lux)
>  {
>  	int lux_data;
> -	unsigned int data_x_range, lux_unshifted;
> +	unsigned int data_x_range;
>  
>  	lux_data = isl29018_read_sensor_input(chip, COMMMAND1_OPMODE_ALS_ONCE);
>  
>  	if (lux_data < 0)
>  		return lux_data;
>  
> -	/* To support fractional scaling, separate the unshifted lux
> -	 * into two calculations: int scaling and micro-scaling.
> -	 * ucalibscale ranges from 0-999999, so about 20 bits.  Split
> -	 * the /1,000,000 in two to reduce the risk of over/underflow.
> -	 */
> -	data_x_range = lux_data * chip->range;
> -	lux_unshifted = data_x_range * chip->calibscale;
> -	lux_unshifted += data_x_range / 1000 * chip->ucalibscale / 1000;
> -	*lux = lux_unshifted >> chip->adc_bit;
> +	data_x_range = lux_data * chip->scale.scale +
> +		       lux_data * chip->scale.uscale / 1000000;
> +	*lux = data_x_range * chip->calibscale +
> +	       data_x_range * chip->ucalibscale / 1000000;
>  
>  	return 0;
>  }
> @@ -229,7 +281,39 @@ static int isl29018_read_proximity_ir(struct isl29018_chip *chip, int scheme,
>  	return 0;
>  }
>  
> -/* Sysfs interface */
> +static ssize_t show_scale_available(struct device *dev,
> +			struct device_attribute *attr, char *buf)
> +{
> +	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> +	struct isl29018_chip *chip = iio_priv(indio_dev);
> +	int i, len = 0;
> +
> +	for (i = 0; i < ARRAY_SIZE(isl29018_scales[chip->int_time]); ++i)
> +		len += sprintf(buf + len, "%d.%06d ",
> +			       isl29018_scales[chip->int_time][i].scale,
> +			       isl29018_scales[chip->int_time][i].uscale);
> +
> +	buf[len - 1] = '\n';
> +
> +	return len;
> +}
> +
> +static ssize_t show_int_time_available(struct device *dev,
> +			struct device_attribute *attr, char *buf)
> +{
> +	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> +	struct isl29018_chip *chip = iio_priv(indio_dev);
> +	int i, len = 0;
> +
> +	for (i = 0; i < ARRAY_SIZE(isl29018_int_utimes[chip->type]); ++i)
> +		len += sprintf(buf + len, "0.%06d ",
> +			       isl29018_int_utimes[chip->type][i]);
> +
> +	buf[len - 1] = '\n';
> +
> +	return len;
> +}
> +
>  /* proximity scheme */
>  static ssize_t show_prox_infrared_suppression(struct device *dev,
>  			struct device_attribute *attr, char *buf)
> @@ -276,11 +360,26 @@ static int isl29018_write_raw(struct iio_dev *indio_dev,
>  	int ret = -EINVAL;
>  
>  	mutex_lock(&chip->lock);
> -	if (mask == IIO_CHAN_INFO_CALIBSCALE && chan->type == IIO_LIGHT) {
> -		chip->calibscale = val;
> -		/* With no write_raw_get_fmt(), val2 is a MICRO fraction. */
> -		chip->ucalibscale = val2;
> -		ret = 0;
> +	switch (mask) {
> +	case IIO_CHAN_INFO_CALIBSCALE:
> +		if (chan->type == IIO_LIGHT) {
> +			chip->calibscale = val;
> +			chip->ucalibscale = val2;
> +			ret = 0;
> +		}
> +		break;
> +	case IIO_CHAN_INFO_INT_TIME:
> +		if (chan->type == IIO_LIGHT)
> +			if (val != 0)
> +				return -EINVAL;
> +			ret = isl29018_set_integration_time(chip, val2);
> +		break;
> +	case IIO_CHAN_INFO_SCALE:
> +		if (chan->type == IIO_LIGHT)
> +			ret = isl29018_set_scale(chip, val, val2);
> +		break;
> +	default:
> +		break;
>  	}
>  	mutex_unlock(&chip->lock);
>  
> @@ -321,6 +420,20 @@ static int isl29018_read_raw(struct iio_dev *indio_dev,
>  		if (!ret)
>  			ret = IIO_VAL_INT;
>  		break;
> +	case IIO_CHAN_INFO_INT_TIME:
> +		if (chan->type == IIO_LIGHT) {
> +			*val = 0;
> +			*val2 = isl29018_int_utimes[chip->type][chip->int_time];
> +			ret = IIO_VAL_INT_PLUS_MICRO;
> +		}
> +		break;
> +	case IIO_CHAN_INFO_SCALE:
> +		if (chan->type == IIO_LIGHT) {
> +			*val = chip->scale.scale;
> +			*val2 = chip->scale.uscale;
> +			ret = IIO_VAL_INT_PLUS_MICRO;
> +		}
> +		break;
>  	case IIO_CHAN_INFO_CALIBSCALE:
>  		if (chan->type == IIO_LIGHT) {
>  			*val = chip->calibscale;
> @@ -340,7 +453,9 @@ static int isl29018_read_raw(struct iio_dev *indio_dev,
>  	.indexed = 1,							\
>  	.channel = 0,							\
>  	.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |		\
> -	BIT(IIO_CHAN_INFO_CALIBSCALE),					\
> +	BIT(IIO_CHAN_INFO_CALIBSCALE) |					\
> +	BIT(IIO_CHAN_INFO_SCALE) |					\
> +	BIT(IIO_CHAN_INFO_INT_TIME),					\
>  }
>  
>  #define ISL29018_IR_CHANNEL {						\
> @@ -366,19 +481,27 @@ static const struct iio_chan_spec isl29023_channels[] = {
>  	ISL29018_IR_CHANNEL,
>  };
>  
> +static IIO_DEVICE_ATTR(in_illuminance_integration_time_available, S_IRUGO,
> +		       show_int_time_available, NULL, 0);
> +static IIO_DEVICE_ATTR(in_illuminance_scale_available, S_IRUGO,
> +		      show_scale_available, NULL, 0);
>  static IIO_DEVICE_ATTR(proximity_on_chip_ambient_infrared_suppression,
>  					S_IRUGO | S_IWUSR,
>  					show_prox_infrared_suppression,
>  					store_prox_infrared_suppression, 0);
>  
>  #define ISL29018_DEV_ATTR(name) (&iio_dev_attr_##name.dev_attr.attr)
> -#define ISL29018_CONST_ATTR(name) (&iio_const_attr_##name.dev_attr.attr)
> +
>  static struct attribute *isl29018_attributes[] = {
> +	ISL29018_DEV_ATTR(in_illuminance_scale_available),
> +	ISL29018_DEV_ATTR(in_illuminance_integration_time_available),
>  	ISL29018_DEV_ATTR(proximity_on_chip_ambient_infrared_suppression),
>  	NULL
>  };
>  
>  static struct attribute *isl29023_attributes[] = {
> +	ISL29018_DEV_ATTR(in_illuminance_scale_available),
> +	ISL29018_DEV_ATTR(in_illuminance_integration_time_available),
>  	NULL
>  };
>  
> @@ -422,8 +545,6 @@ enum {
>  static int isl29018_chip_init(struct isl29018_chip *chip)
>  {
>  	int status;
> -	unsigned int new_adc_bit;
> -	unsigned int new_range;
>  
>  	if (chip->type == isl29035) {
>  		status = isl29035_detect(chip);
> @@ -472,14 +593,19 @@ static int isl29018_chip_init(struct isl29018_chip *chip)
>  	usleep_range(1000, 2000);	/* per data sheet, page 10 */
>  
>  	/* set defaults */
> -	status = isl29018_set_range(chip, chip->range, &new_range);
> +	status = isl29018_set_scale(chip, chip->scale.scale,
> +				    chip->scale.uscale);
>  	if (status < 0) {
>  		dev_err(chip->dev, "Init of isl29018 fails\n");
>  		return status;
>  	}
>  
> -	status = isl29018_set_resolution(chip, chip->adc_bit,
> -						&new_adc_bit);
> +	status = isl29018_set_integration_time(chip,
> +			isl29018_int_utimes[chip->type][chip->int_time]);
> +	if (status < 0) {
> +		dev_err(chip->dev, "Init of isl29018 fails\n");
> +		return status;
> +	}
>  
>  	return 0;
>  }
> @@ -609,8 +735,8 @@ static int isl29018_probe(struct i2c_client *client,
>  	chip->type = dev_id;
>  	chip->calibscale = 1;
>  	chip->ucalibscale = 0;
> -	chip->range = 1000;
> -	chip->adc_bit = 16;
> +	chip->int_time = ISL29018_INT_TIME_16;
> +	chip->scale = isl29018_scales[chip->int_time][0];
>  	chip->suspended = false;
>  
>  	chip->regmap = devm_regmap_init_i2c(client,
> 

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

* Re: [PATCH v2 3/3] staging: iio: light: isl29018: Use standard sysfs attributes for scale and integration time
  2015-06-27  9:34   ` Hartmut Knaack
@ 2015-06-29  7:24     ` Daniel Baluta
  0 siblings, 0 replies; 12+ messages in thread
From: Daniel Baluta @ 2015-06-29  7:24 UTC (permalink / raw)
  To: Hartmut Knaack
  Cc: Roberta Dobrescu, linux-iio, Jonathan Cameron,
	Lars-Peter Clausen, Peter Meerwald, Daniel Baluta

On Sat, Jun 27, 2015 at 12:34 PM, Hartmut Knaack <knaack.h@gmx.de> wrote:
> Roberta Dobrescu schrieb am 16.04.2015 um 21:20:
>> This patch refactors the isl29018 driver code in order to use standard
>> sysfs attributes for scale and integration time.
>>
>> ISL29018 light sensor uses four ranges and four ADC's resolutions
>> which influence the calculated lux. Adc resolution is strongly
>> connected to integration time and range should be controlled by scale.
>>
>> This patch introduces the usage of integration time and scale instead
>> of adc resolution and range.
>>
>> Signed-off-by: Roberta Dobrescu <roberta.dobrescu@gmail.com>
>> ---
>>  drivers/staging/iio/light/isl29018.c | 224 +++++++++++++++++++++++++++--------
>>  1 file changed, 175 insertions(+), 49 deletions(-)
>>
>> diff --git a/drivers/staging/iio/light/isl29018.c b/drivers/staging/iio/light/isl29018.c
>> index ffc3d1b..435d417 100644
>> --- a/drivers/staging/iio/light/isl29018.c
>> +++ b/drivers/staging/iio/light/isl29018.c
>> @@ -66,6 +66,39 @@
>>  #define ISL29035_BOUT_SHIFT          0x07
>>  #define ISL29035_BOUT_MASK           (0x01 << ISL29035_BOUT_SHIFT)
>>
>> +#define ISL29018_INT_TIME_AVAIL              "0.090000 0.005630 0.000351 0.000021"
>> +#define ISL29023_INT_TIME_AVAIL              "0.090000 0.005600 0.000352 0.000022"
>> +#define ISL29035_INT_TIME_AVAIL              "0.105000 0.006500 0.000410 0.000025"
>> +
>> +static const char * const int_time_avail[] = {
>> +     ISL29018_INT_TIME_AVAIL,
>> +     ISL29023_INT_TIME_AVAIL,
>> +     ISL29035_INT_TIME_AVAIL,
>> +};
>> +
>
> Hi Roberta,
> are you still working to get this one out of staging? You introduced here the
> int_time_avail array (missing prefix), although you don't make use of it down
> in show_int_time_available() (also missing prefix), where I would expect it to
> be used. So, what's the status? Do you intend to use this array later on, or
> can it be dropped?
> Just for completeness: show_scale_available also misses a prefix.

Hi Hartmut,

Yes are still working to get this out of staging, but it wasn't a top priority.
I hope to have it finished by the end of July.

thanks,
Daniel.

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

end of thread, other threads:[~2015-06-29  7:24 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-16 19:20 [PATCH v2 0/3] staging: iio: light: isl29018: Remove non-standard sysfs attributes Roberta Dobrescu
2015-04-16 19:20 ` [PATCH v2 1/3] " Roberta Dobrescu
2015-04-26 19:46   ` Jonathan Cameron
2015-04-16 19:20 ` [PATCH v2 2/3] staging: iio: light: isl29018: Rename lux_scale to calibscale Roberta Dobrescu
2015-04-26 19:46   ` Jonathan Cameron
2015-04-16 19:20 ` [PATCH v2 3/3] staging: iio: light: isl29018: Use standard sysfs attributes for scale and integration time Roberta Dobrescu
2015-04-26 19:51   ` Jonathan Cameron
2015-04-27 16:03     ` Rhyland Klein
2015-05-02 11:02       ` Jonathan Cameron
2015-05-02 15:55         ` Roberta Dobrescu
2015-06-27  9:34   ` Hartmut Knaack
2015-06-29  7:24     ` 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.