[PATCH 1/7] staging:iio:ad7291: Use IIO_VAL_FRACTIONAL_LOG2

[PATCH 1/7] staging:iio:ad7291: Use IIO_VAL_FRACTIONAL_LOG2

[Lars-Peter Clausen]

This is a bit more straight forward and also allows better precession when using
the an in-kernel consumer.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/staging/iio/adc/ad7291.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/iio/adc/ad7291.c b/drivers/staging/iio/adc/ad7291.c
index d088c66..a3a3a2f 100644
--- a/drivers/staging/iio/adc/ad7291.c
+++ b/drivers/staging/iio/adc/ad7291.c
@@ -456,7 +456,6 @@ static int ad7291_read_raw(struct iio_dev *indio_dev,
 {
 	int ret;
 	struct ad7291_chip_info *chip = iio_priv(indio_dev);
-	unsigned int scale_uv;
 	u16 regval;
 	s16 signval;
 
@@ -513,10 +512,9 @@ static int ad7291_read_raw(struct iio_dev *indio_dev,
 	case IIO_CHAN_INFO_SCALE:
 		switch (chan->type) {
 		case IIO_VOLTAGE:
-			scale_uv = (chip->int_vref_mv * 1000) >> AD7291_BITS;
-			*val =  scale_uv / 1000;
-			*val2 = (scale_uv % 1000) * 1000;
-			return IIO_VAL_INT_PLUS_MICRO;
+			*val = chip->int_vref_mv;
+			*val2 = AD7291_BITS;
+			return IIO_VAL_FRACTIONAL_LOG2;
 		case IIO_TEMP:
 			/*
 			 * One LSB of the ADC corresponds to 0.25 deg C.
-- 
1.8.0

[PATCH 2/7] staging:iio:ad7291: Remove userspace reset

[Lars-Peter Clausen]

There is no reason why userspace should want to trigger a manual reset of the
device, so remove this functionality.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/staging/iio/adc/ad7291.c | 24 ------------------------
 1 file changed, 24 deletions(-)

diff --git a/drivers/staging/iio/adc/ad7291.c b/drivers/staging/iio/adc/ad7291.c
index a3a3a2f..782e100 100644
--- a/drivers/staging/iio/adc/ad7291.c
+++ b/drivers/staging/iio/adc/ad7291.c
@@ -127,29 +127,6 @@ static int ad7291_i2c_write(struct ad7291_chip_info *chip, u8 reg, u16 data)
 	return i2c_smbus_write_word_data(chip->client, reg, swab16(data));
 }
 
-static ssize_t ad7291_store_reset(struct device *dev,
-		struct device_attribute *attr,
-		const char *buf,
-		size_t len)
-{
-	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-	struct ad7291_chip_info *chip = iio_priv(indio_dev);
-
-	return ad7291_i2c_write(chip, AD7291_COMMAND,
-				chip->command | AD7291_RESET);
-}
-
-static IIO_DEVICE_ATTR(reset, S_IWUSR, NULL, ad7291_store_reset, 0);
-
-static struct attribute *ad7291_attributes[] = {
-	&iio_dev_attr_reset.dev_attr.attr,
-	NULL,
-};
-
-static const struct attribute_group ad7291_attribute_group = {
-	.attrs = ad7291_attributes,
-};
-
 static irqreturn_t ad7291_event_handler(int irq, void *private)
 {
 	struct iio_dev *indio_dev = private;
@@ -569,7 +546,6 @@ static struct attribute_group ad7291_event_attribute_group = {
 };
 
 static const struct iio_info ad7291_info = {
-	.attrs = &ad7291_attribute_group,
 	.read_raw = &ad7291_read_raw,
 	.read_event_config = &ad7291_read_event_config,
 	.write_event_config = &ad7291_write_event_config,
-- 
1.8.0

Re: [PATCH 2/7] staging:iio:ad7291: Remove userspace reset

[Jonathan Cameron]

On 06/14/2013 04:58 PM, Lars-Peter Clausen wrote:
> There is no reason why userspace should want to trigger a manual reset of the
> device, so remove this functionality.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Applied to the togreg branch of iio.git
> ---
>  drivers/staging/iio/adc/ad7291.c | 24 ------------------------
>  1 file changed, 24 deletions(-)
> 
> diff --git a/drivers/staging/iio/adc/ad7291.c b/drivers/staging/iio/adc/ad7291.c
> index a3a3a2f..782e100 100644
> --- a/drivers/staging/iio/adc/ad7291.c
> +++ b/drivers/staging/iio/adc/ad7291.c
> @@ -127,29 +127,6 @@ static int ad7291_i2c_write(struct ad7291_chip_info *chip, u8 reg, u16 data)
>  	return i2c_smbus_write_word_data(chip->client, reg, swab16(data));
>  }
>  
> -static ssize_t ad7291_store_reset(struct device *dev,
> -		struct device_attribute *attr,
> -		const char *buf,
> -		size_t len)
> -{
> -	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> -	struct ad7291_chip_info *chip = iio_priv(indio_dev);
> -
> -	return ad7291_i2c_write(chip, AD7291_COMMAND,
> -				chip->command | AD7291_RESET);
> -}
> -
> -static IIO_DEVICE_ATTR(reset, S_IWUSR, NULL, ad7291_store_reset, 0);
> -
> -static struct attribute *ad7291_attributes[] = {
> -	&iio_dev_attr_reset.dev_attr.attr,
> -	NULL,
> -};
> -
> -static const struct attribute_group ad7291_attribute_group = {
> -	.attrs = ad7291_attributes,
> -};
> -
>  static irqreturn_t ad7291_event_handler(int irq, void *private)
>  {
>  	struct iio_dev *indio_dev = private;
> @@ -569,7 +546,6 @@ static struct attribute_group ad7291_event_attribute_group = {
>  };
>  
>  static const struct iio_info ad7291_info = {
> -	.attrs = &ad7291_attribute_group,
>  	.read_raw = &ad7291_read_raw,
>  	.read_event_config = &ad7291_read_event_config,
>  	.write_event_config = &ad7291_write_event_config,
> 

[PATCH 3/7] staging:iio:ad7291: Remove unnecessary dev_info() from probe()

[Lars-Peter Clausen]

No need spam the log during probe if nothing went wrong.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/staging/iio/adc/ad7291.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/staging/iio/adc/ad7291.c b/drivers/staging/iio/adc/ad7291.c
index 782e100..5bb1963 100644
--- a/drivers/staging/iio/adc/ad7291.c
+++ b/drivers/staging/iio/adc/ad7291.c
@@ -628,9 +628,6 @@ static int ad7291_probe(struct i2c_client *client,
 	if (ret)
 		goto error_unreg_irq;
 
-	dev_info(&client->dev, "%s ADC registered.\n",
-			 id->name);
-
 	return 0;
 
 error_unreg_irq:
-- 
1.8.0

[PATCH 4/7] staging:iio:ad7291: Simplify threshold register lookup

[Lars-Peter Clausen]

The AD7291 register map is nicely uniform and we can easily compute the
threshold register address for a certain channel using simple math instead of
using a look-up table.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/staging/iio/adc/ad7291.c | 121 ++++++++++++++-------------------------
 1 file changed, 42 insertions(+), 79 deletions(-)

diff --git a/drivers/staging/iio/adc/ad7291.c b/drivers/staging/iio/adc/ad7291.c
index 5bb1963..bf8d689 100644
--- a/drivers/staging/iio/adc/ad7291.c
+++ b/drivers/staging/iio/adc/ad7291.c
@@ -39,33 +39,9 @@
 #define AD7291_VOLTAGE			0x01
 #define AD7291_T_SENSE			0x02
 #define AD7291_T_AVERAGE		0x03
-#define AD7291_CH0_DATA_HIGH		0x04
-#define AD7291_CH0_DATA_LOW		0x05
-#define AD7291_CH0_HYST			0x06
-#define AD7291_CH1_DATA_HIGH		0x07
-#define AD7291_CH1_DATA_LOW		0x08
-#define AD7291_CH1_HYST			0x09
-#define AD7291_CH2_DATA_HIGH		0x0A
-#define AD7291_CH2_DATA_LOW		0x0B
-#define AD7291_CH2_HYST			0x0C
-#define AD7291_CH3_DATA_HIGH		0x0D
-#define AD7291_CH3_DATA_LOW		0x0E
-#define AD7291_CH3_HYST			0x0F
-#define AD7291_CH4_DATA_HIGH		0x10
-#define AD7291_CH4_DATA_LOW		0x11
-#define AD7291_CH4_HYST			0x12
-#define AD7291_CH5_DATA_HIGH		0x13
-#define AD7291_CH5_DATA_LOW		0x14
-#define AD7291_CH5_HYST			0x15
-#define AD7291_CH6_DATA_HIGH		0x16
-#define AD7291_CH6_DATA_LOW		0x17
-#define AD7291_CH6_HYST			0x18
-#define AD7291_CH7_DATA_HIGH		0x19
-#define AD7291_CH7_DATA_LOW		0x1A
-#define AD7291_CH7_HYST			0x2B
-#define AD7291_T_SENSE_HIGH		0x1C
-#define AD7291_T_SENSE_LOW		0x1D
-#define AD7291_T_SENSE_HYST		0x1E
+#define AD7291_DATA_HIGH(x)		((x) * 3 + 0x4)
+#define AD7291_DATA_LOW(x)		((x) * 3 + 0x5)
+#define AD7291_HYST(x)			((x) * 3 + 0x6)
 #define AD7291_VOLTAGE_ALERT_STATUS	0x1F
 #define AD7291_T_ALERT_STATUS		0x20
 
@@ -232,31 +208,31 @@ static inline ssize_t ad7291_set_hyst(struct device *dev,
 static IIO_DEVICE_ATTR(in_temp0_thresh_both_hyst_raw,
 		       S_IRUGO | S_IWUSR,
 		       ad7291_show_hyst, ad7291_set_hyst,
-		       AD7291_T_SENSE_HYST);
+		       AD7291_HYST(8));
 static IIO_DEVICE_ATTR(in_voltage0_thresh_both_hyst_raw,
 		       S_IRUGO | S_IWUSR,
-		       ad7291_show_hyst, ad7291_set_hyst, AD7291_CH0_HYST);
+		       ad7291_show_hyst, ad7291_set_hyst, AD7291_HYST(0));
 static IIO_DEVICE_ATTR(in_voltage1_thresh_both_hyst_raw,
 		       S_IRUGO | S_IWUSR,
-		       ad7291_show_hyst, ad7291_set_hyst, AD7291_CH1_HYST);
+		       ad7291_show_hyst, ad7291_set_hyst, AD7291_HYST(1));
 static IIO_DEVICE_ATTR(in_voltage2_thresh_both_hyst_raw,
 		       S_IRUGO | S_IWUSR,
-		       ad7291_show_hyst, ad7291_set_hyst, AD7291_CH2_HYST);
+		       ad7291_show_hyst, ad7291_set_hyst, AD7291_HYST(2));
 static IIO_DEVICE_ATTR(in_voltage3_thresh_both_hyst_raw,
 		       S_IRUGO | S_IWUSR,
-		       ad7291_show_hyst, ad7291_set_hyst, AD7291_CH3_HYST);
+		       ad7291_show_hyst, ad7291_set_hyst, AD7291_HYST(3));
 static IIO_DEVICE_ATTR(in_voltage4_thresh_both_hyst_raw,
 		       S_IRUGO | S_IWUSR,
-		       ad7291_show_hyst, ad7291_set_hyst, AD7291_CH4_HYST);
+		       ad7291_show_hyst, ad7291_set_hyst, AD7291_HYST(4));
 static IIO_DEVICE_ATTR(in_voltage5_thresh_both_hyst_raw,
 		       S_IRUGO | S_IWUSR,
-		       ad7291_show_hyst, ad7291_set_hyst, AD7291_CH5_HYST);
+		       ad7291_show_hyst, ad7291_set_hyst, AD7291_HYST(5));
 static IIO_DEVICE_ATTR(in_voltage6_thresh_both_hyst_raw,
 		       S_IRUGO | S_IWUSR,
-		       ad7291_show_hyst, ad7291_set_hyst, AD7291_CH6_HYST);
+		       ad7291_show_hyst, ad7291_set_hyst, AD7291_HYST(6));
 static IIO_DEVICE_ATTR(in_voltage7_thresh_both_hyst_raw,
 		       S_IRUGO | S_IWUSR,
-		       ad7291_show_hyst, ad7291_set_hyst, AD7291_CH7_HYST);
+		       ad7291_show_hyst, ad7291_set_hyst, AD7291_HYST(7));
 
 static struct attribute *ad7291_event_attributes[] = {
 	&iio_dev_attr_in_temp0_thresh_both_hyst_raw.dev_attr.attr,
@@ -271,52 +247,46 @@ static struct attribute *ad7291_event_attributes[] = {
 	NULL,
 };
 
-/* high / low */
-static u8 ad7291_limit_regs[9][2] = {
-	{ AD7291_CH0_DATA_HIGH, AD7291_CH0_DATA_LOW },
-	{ AD7291_CH1_DATA_HIGH, AD7291_CH1_DATA_LOW },
-	{ AD7291_CH2_DATA_HIGH, AD7291_CH2_DATA_LOW },
-	{ AD7291_CH3_DATA_HIGH, AD7291_CH3_DATA_LOW }, /* FIXME: ? */
-	{ AD7291_CH4_DATA_HIGH, AD7291_CH4_DATA_LOW },
-	{ AD7291_CH5_DATA_HIGH, AD7291_CH5_DATA_LOW },
-	{ AD7291_CH6_DATA_HIGH, AD7291_CH6_DATA_LOW },
-	{ AD7291_CH7_DATA_HIGH, AD7291_CH7_DATA_LOW },
-	/* temp */
-	{ AD7291_T_SENSE_HIGH, AD7291_T_SENSE_LOW },
-};
+static unsigned int ad7291_threshold_reg(u64 event_code)
+{
+	unsigned int offset;
+
+	switch (IIO_EVENT_CODE_EXTRACT_CHAN_TYPE(event_code)) {
+	case IIO_VOLTAGE:
+		offset = IIO_EVENT_CODE_EXTRACT_CHAN(event_code);
+		break;
+	case IIO_TEMP:
+		offset = 8;
+		break;
+	default:
+	    return 0;
+	}
+
+	if (IIO_EVENT_CODE_EXTRACT_DIR(event_code) == IIO_EV_DIR_FALLING)
+		return AD7291_DATA_LOW(offset);
+	else
+		return AD7291_DATA_HIGH(offset);
+}
 
 static int ad7291_read_event_value(struct iio_dev *indio_dev,
 				   u64 event_code,
 				   int *val)
 {
 	struct ad7291_chip_info *chip = iio_priv(indio_dev);
-
 	int ret;
-	u8 reg;
 	u16 uval;
 	s16 signval;
 
+	ret = ad7291_i2c_read(chip, ad7291_threshold_reg(event_code), &uval);
+	if (ret < 0)
+		return ret;
+
 	switch (IIO_EVENT_CODE_EXTRACT_CHAN_TYPE(event_code)) {
 	case IIO_VOLTAGE:
-		reg = ad7291_limit_regs[IIO_EVENT_CODE_EXTRACT_CHAN(event_code)]
-			[!(IIO_EVENT_CODE_EXTRACT_DIR(event_code) ==
-			   IIO_EV_DIR_RISING)];
-
-		ret = ad7291_i2c_read(chip, reg, &uval);
-		if (ret < 0)
-			return ret;
 		*val = uval & AD7291_VALUE_MASK;
 		return 0;
-
 	case IIO_TEMP:
-		reg = ad7291_limit_regs[8]
-			[!(IIO_EVENT_CODE_EXTRACT_DIR(event_code) ==
-			   IIO_EV_DIR_RISING)];
-
-		ret = ad7291_i2c_read(chip, reg, &signval);
-		if (ret < 0)
-			return ret;
-		signval = (s16)((signval & AD7291_VALUE_MASK) << 4) >> 4;
+		signval = (s16)((uval & AD7291_VALUE_MASK) << 4) >> 4;
 		*val = signval;
 		return 0;
 	default:
@@ -329,28 +299,21 @@ static int ad7291_write_event_value(struct iio_dev *indio_dev,
 				    int val)
 {
 	struct ad7291_chip_info *chip = iio_priv(indio_dev);
-	u8 reg;
-	s16 signval;
 
 	switch (IIO_EVENT_CODE_EXTRACT_CHAN_TYPE(event_code)) {
 	case IIO_VOLTAGE:
 		if (val > AD7291_VALUE_MASK || val < 0)
 			return -EINVAL;
-		reg = ad7291_limit_regs[IIO_EVENT_CODE_EXTRACT_CHAN(event_code)]
-			[!(IIO_EVENT_CODE_EXTRACT_DIR(event_code) ==
-			   IIO_EV_DIR_RISING)];
-		return ad7291_i2c_write(chip, reg, val);
+		break;
 	case IIO_TEMP:
 		if (val > 2047 || val < -2048)
 			return -EINVAL;
-		reg = ad7291_limit_regs[8]
-			[!(IIO_EVENT_CODE_EXTRACT_DIR(event_code) ==
-			   IIO_EV_DIR_RISING)];
-		signval = val;
-		return ad7291_i2c_write(chip, reg, *(u16 *)&signval);
+		break;
 	default:
 		return -EINVAL;
-	};
+	}
+
+	return ad7291_i2c_write(chip, ad7291_threshold_reg(event_code), val);
 }
 
 static int ad7291_read_event_config(struct iio_dev *indio_dev,
-- 
1.8.0

[PATCH 5/7] staging:iio:ad7291: Use i2c_smbus_{read,write}_word_swapped instead of open-coding it

[Lars-Peter Clausen]

No need to swap the upper and lower byte by hand if there is a helper function
which already does this.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/staging/iio/adc/ad7291.c | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/iio/adc/ad7291.c b/drivers/staging/iio/adc/ad7291.c
index bf8d689..2a7e6a7 100644
--- a/drivers/staging/iio/adc/ad7291.c
+++ b/drivers/staging/iio/adc/ad7291.c
@@ -87,20 +87,20 @@ static int ad7291_i2c_read(struct ad7291_chip_info *chip, u8 reg, u16 *data)
 	struct i2c_client *client = chip->client;
 	int ret = 0;
 
-	ret = i2c_smbus_read_word_data(client, reg);
+	ret = i2c_smbus_read_word_swapped(client, reg);
 	if (ret < 0) {
 		dev_err(&client->dev, "I2C read error\n");
 		return ret;
 	}
 
-	*data = swab16((u16)ret);
+	*data = ret;
 
 	return 0;
 }
 
 static int ad7291_i2c_write(struct ad7291_chip_info *chip, u8 reg, u16 data)
 {
-	return i2c_smbus_write_word_data(chip->client, reg, swab16(data));
+	return i2c_smbus_write_word_swapped(chip->client, reg, data);
 }
 
 static irqreturn_t ad7291_event_handler(int irq, void *private)
@@ -418,35 +418,33 @@ static int ad7291_read_raw(struct iio_dev *indio_dev,
 				return ret;
 			}
 			/* Read voltage */
-			ret = i2c_smbus_read_word_data(chip->client,
+			ret = i2c_smbus_read_word_swapped(chip->client,
 						       AD7291_VOLTAGE);
 			if (ret < 0) {
 				mutex_unlock(&chip->state_lock);
 				return ret;
 			}
-			*val = swab16((u16)ret) & AD7291_VALUE_MASK;
+			*val = ret & AD7291_VALUE_MASK;
 			mutex_unlock(&chip->state_lock);
 			return IIO_VAL_INT;
 		case IIO_TEMP:
 			/* Assumes tsense bit of command register always set */
-			ret = i2c_smbus_read_word_data(chip->client,
+			ret = i2c_smbus_read_word_swapped(chip->client,
 						       AD7291_T_SENSE);
 			if (ret < 0)
 				return ret;
-			signval = (s16)((swab16((u16)ret) &
-				AD7291_VALUE_MASK) << 4) >> 4;
+			signval = (s16)((ret & AD7291_VALUE_MASK) << 4) >> 4;
 			*val = signval;
 			return IIO_VAL_INT;
 		default:
 			return -EINVAL;
 		}
 	case IIO_CHAN_INFO_AVERAGE_RAW:
-		ret = i2c_smbus_read_word_data(chip->client,
+		ret = i2c_smbus_read_word_swapped(chip->client,
 					       AD7291_T_AVERAGE);
 			if (ret < 0)
 				return ret;
-			signval = (s16)((swab16((u16)ret) &
-				AD7291_VALUE_MASK) << 4) >> 4;
+			signval = (s16)((ret & AD7291_VALUE_MASK) << 4) >> 4;
 			*val = signval;
 			return IIO_VAL_INT;
 	case IIO_CHAN_INFO_SCALE:
-- 
1.8.0

[PATCH 6/7] staging:iio:ad7291: Use sign_extend32 instead of open-coding it

[Lars-Peter Clausen]

This makes it a bit more obvious what is going on than open-coding it.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/staging/iio/adc/ad7291.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/iio/adc/ad7291.c b/drivers/staging/iio/adc/ad7291.c
index 2a7e6a7..c2709d6 100644
--- a/drivers/staging/iio/adc/ad7291.c
+++ b/drivers/staging/iio/adc/ad7291.c
@@ -275,7 +275,6 @@ static int ad7291_read_event_value(struct iio_dev *indio_dev,
 	struct ad7291_chip_info *chip = iio_priv(indio_dev);
 	int ret;
 	u16 uval;
-	s16 signval;
 
 	ret = ad7291_i2c_read(chip, ad7291_threshold_reg(event_code), &uval);
 	if (ret < 0)
@@ -286,8 +285,7 @@ static int ad7291_read_event_value(struct iio_dev *indio_dev,
 		*val = uval & AD7291_VALUE_MASK;
 		return 0;
 	case IIO_TEMP:
-		signval = (s16)((uval & AD7291_VALUE_MASK) << 4) >> 4;
-		*val = signval;
+		*val = sign_extend32(uval, 11);
 		return 0;
 	default:
 		return -EINVAL;
@@ -397,7 +395,6 @@ static int ad7291_read_raw(struct iio_dev *indio_dev,
 	int ret;
 	struct ad7291_chip_info *chip = iio_priv(indio_dev);
 	u16 regval;
-	s16 signval;
 
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
@@ -433,8 +430,7 @@ static int ad7291_read_raw(struct iio_dev *indio_dev,
 						       AD7291_T_SENSE);
 			if (ret < 0)
 				return ret;
-			signval = (s16)((ret & AD7291_VALUE_MASK) << 4) >> 4;
-			*val = signval;
+			*val = sign_extend32(ret, 11);
 			return IIO_VAL_INT;
 		default:
 			return -EINVAL;
@@ -444,8 +440,7 @@ static int ad7291_read_raw(struct iio_dev *indio_dev,
 					       AD7291_T_AVERAGE);
 			if (ret < 0)
 				return ret;
-			signval = (s16)((ret & AD7291_VALUE_MASK) << 4) >> 4;
-			*val = signval;
+			*val = sign_extend32(ret, 11);
 			return IIO_VAL_INT;
 	case IIO_CHAN_INFO_SCALE:
 		switch (chan->type) {
-- 
1.8.0

Re: [PATCH 6/7] staging:iio:ad7291: Use sign_extend32 instead of open-coding it

[Jonathan Cameron]

On 06/14/2013 04:58 PM, Lars-Peter Clausen wrote:
> This makes it a bit more obvious what is going on than open-coding it.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
applied to the togreg branch of iio.git

> ---
>  drivers/staging/iio/adc/ad7291.c | 11 +++--------
>  1 file changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/staging/iio/adc/ad7291.c b/drivers/staging/iio/adc/ad7291.c
> index 2a7e6a7..c2709d6 100644
> --- a/drivers/staging/iio/adc/ad7291.c
> +++ b/drivers/staging/iio/adc/ad7291.c
> @@ -275,7 +275,6 @@ static int ad7291_read_event_value(struct iio_dev *indio_dev,
>  	struct ad7291_chip_info *chip = iio_priv(indio_dev);
>  	int ret;
>  	u16 uval;
> -	s16 signval;
>  
>  	ret = ad7291_i2c_read(chip, ad7291_threshold_reg(event_code), &uval);
>  	if (ret < 0)
> @@ -286,8 +285,7 @@ static int ad7291_read_event_value(struct iio_dev *indio_dev,
>  		*val = uval & AD7291_VALUE_MASK;
>  		return 0;
>  	case IIO_TEMP:
> -		signval = (s16)((uval & AD7291_VALUE_MASK) << 4) >> 4;
> -		*val = signval;
> +		*val = sign_extend32(uval, 11);
>  		return 0;
>  	default:
>  		return -EINVAL;
> @@ -397,7 +395,6 @@ static int ad7291_read_raw(struct iio_dev *indio_dev,
>  	int ret;
>  	struct ad7291_chip_info *chip = iio_priv(indio_dev);
>  	u16 regval;
> -	s16 signval;
>  
>  	switch (mask) {
>  	case IIO_CHAN_INFO_RAW:
> @@ -433,8 +430,7 @@ static int ad7291_read_raw(struct iio_dev *indio_dev,
>  						       AD7291_T_SENSE);
>  			if (ret < 0)
>  				return ret;
> -			signval = (s16)((ret & AD7291_VALUE_MASK) << 4) >> 4;
> -			*val = signval;
> +			*val = sign_extend32(ret, 11);
>  			return IIO_VAL_INT;
>  		default:
>  			return -EINVAL;
> @@ -444,8 +440,7 @@ static int ad7291_read_raw(struct iio_dev *indio_dev,
>  					       AD7291_T_AVERAGE);
>  			if (ret < 0)
>  				return ret;
> -			signval = (s16)((ret & AD7291_VALUE_MASK) << 4) >> 4;
> -			*val = signval;
> +			*val = sign_extend32(ret, 11);
>  			return IIO_VAL_INT;
>  	case IIO_CHAN_INFO_SCALE:
>  		switch (chan->type) {
> 

[PATCH 7/7] staging:iio:ad7291: Rework regulator handling

[Lars-Peter Clausen]

Add platform_data for the driver which allows to specify whether an external
reference voltage is used or not. It is not possible to use the return value of
regulator_get() for this since the regulator framework is able to return a dummy
regulator in case no regulator has been specified. In this case the driver will
always get a valid regulator, no matter if it has been specified or not.  Also
make the regulator non-optional if platform_data states that an external
reference voltage should be used.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/staging/iio/adc/ad7291.c | 38 +++++++++++++++++++++++---------------
 drivers/staging/iio/adc/ad7291.h | 12 ++++++++++++
 2 files changed, 35 insertions(+), 15 deletions(-)
 create mode 100644 drivers/staging/iio/adc/ad7291.h

diff --git a/drivers/staging/iio/adc/ad7291.c b/drivers/staging/iio/adc/ad7291.c
index c2709d6..3fc79e5 100644
--- a/drivers/staging/iio/adc/ad7291.c
+++ b/drivers/staging/iio/adc/ad7291.c
@@ -21,6 +21,8 @@
 #include <linux/iio/sysfs.h>
 #include <linux/iio/events.h>
 
+#include "ad7291.h"
+
 /*
  * Simplified handling
  *
@@ -76,7 +78,6 @@
 struct ad7291_chip_info {
 	struct i2c_client	*client;
 	struct regulator	*reg;
-	u16			int_vref_mv;
 	u16			command;
 	u16			c_mask;	/* Active voltage channels for events */
 	struct mutex		state_lock;
@@ -445,7 +446,15 @@ static int ad7291_read_raw(struct iio_dev *indio_dev,
 	case IIO_CHAN_INFO_SCALE:
 		switch (chan->type) {
 		case IIO_VOLTAGE:
-			*val = chip->int_vref_mv;
+			if (chip->reg) {
+				int vref;
+				vref = regulator_get_voltage(chip->reg);
+				if (vref < 0)
+					return vref;
+				*val = vref / 1000;
+			} else {
+				*val = 2500;
+			}
 			*val2 = AD7291_BITS;
 			return IIO_VAL_FRACTIONAL_LOG2;
 		case IIO_TEMP:
@@ -513,9 +522,10 @@ static const struct iio_info ad7291_info = {
 static int ad7291_probe(struct i2c_client *client,
 		const struct i2c_device_id *id)
 {
+	struct ad7291_platform_data *pdata = client->dev.platform_data;
 	struct ad7291_chip_info *chip;
 	struct iio_dev *indio_dev;
-	int ret = 0, voltage_uv = 0;
+	int ret = 0;
 
 	indio_dev = iio_device_alloc(sizeof(*chip));
 	if (indio_dev == NULL) {
@@ -524,12 +534,14 @@ static int ad7291_probe(struct i2c_client *client,
 	}
 	chip = iio_priv(indio_dev);
 
-	chip->reg = regulator_get(&client->dev, "vcc");
-	if (!IS_ERR(chip->reg)) {
+	if (pdata && pdata->use_external_ref) {
+		chip->reg = regulator_get(&client->dev, "vref");
+		if (IS_ERR(chip->reg))
+			goto error_free;
+
 		ret = regulator_enable(chip->reg);
 		if (ret)
 			goto error_put_reg;
-		voltage_uv = regulator_get_voltage(chip->reg);
 	}
 
 	mutex_init(&chip->state_lock);
@@ -542,12 +554,8 @@ static int ad7291_probe(struct i2c_client *client,
 			AD7291_T_SENSE_MASK | /* Tsense always enabled */
 			AD7291_ALERT_POLARITY; /* set irq polarity low level */
 
-	if (voltage_uv) {
-		chip->int_vref_mv = voltage_uv / 1000;
+	if (pdata && pdata->use_external_ref)
 		chip->command |= AD7291_EXT_REF;
-	} else {
-		chip->int_vref_mv = 2500; /* Build-in ref */
-	}
 
 	indio_dev->name = id->name;
 	indio_dev->channels = ad7291_channels;
@@ -590,12 +598,12 @@ error_unreg_irq:
 	if (client->irq)
 		free_irq(client->irq, indio_dev);
 error_disable_reg:
-	if (!IS_ERR(chip->reg))
+	if (chip->reg)
 		regulator_disable(chip->reg);
 error_put_reg:
-	if (!IS_ERR(chip->reg))
+	if (chip->reg)
 		regulator_put(chip->reg);
-
+error_free:
 	iio_device_free(indio_dev);
 error_ret:
 	return ret;
@@ -611,7 +619,7 @@ static int ad7291_remove(struct i2c_client *client)
 	if (client->irq)
 		free_irq(client->irq, indio_dev);
 
-	if (!IS_ERR(chip->reg)) {
+	if (chip->reg) {
 		regulator_disable(chip->reg);
 		regulator_put(chip->reg);
 	}
diff --git a/drivers/staging/iio/adc/ad7291.h b/drivers/staging/iio/adc/ad7291.h
new file mode 100644
index 0000000..bbd89fa
--- /dev/null
+++ b/drivers/staging/iio/adc/ad7291.h
@@ -0,0 +1,12 @@
+#ifndef __IIO_AD7291_H__
+#define __IIO_AD7291_H__
+
+/**
+ * struct ad7291_platform_data - AD7291 platform data
+ * @use_external_ref: Whether to use an external or internal reference voltage
+ */
+struct ad7291_platform_data {
+	bool use_external_ref;
+};
+
+#endif
-- 
1.8.0

Re: [PATCH 7/7] staging:iio:ad7291: Rework regulator handling

[Jonathan Cameron]

On 06/14/2013 04:58 PM, Lars-Peter Clausen wrote:
> Add platform_data for the driver which allows to specify whether an external
> reference voltage is used or not. It is not possible to use the return value of
> regulator_get() for this since the regulator framework is able to return a dummy
> regulator in case no regulator has been specified. In this case the driver will
> always get a valid regulator, no matter if it has been specified or not.  Also
> make the regulator non-optional if platform_data states that an external
> reference voltage should be used.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Applied to the togreg branch of iio.git
> ---
>  drivers/staging/iio/adc/ad7291.c | 38 +++++++++++++++++++++++---------------
>  drivers/staging/iio/adc/ad7291.h | 12 ++++++++++++
>  2 files changed, 35 insertions(+), 15 deletions(-)
>  create mode 100644 drivers/staging/iio/adc/ad7291.h
> 
> diff --git a/drivers/staging/iio/adc/ad7291.c b/drivers/staging/iio/adc/ad7291.c
> index c2709d6..3fc79e5 100644
> --- a/drivers/staging/iio/adc/ad7291.c
> +++ b/drivers/staging/iio/adc/ad7291.c
> @@ -21,6 +21,8 @@
>  #include <linux/iio/sysfs.h>
>  #include <linux/iio/events.h>
>  
> +#include "ad7291.h"
> +
>  /*
>   * Simplified handling
>   *
> @@ -76,7 +78,6 @@
>  struct ad7291_chip_info {
>  	struct i2c_client	*client;
>  	struct regulator	*reg;
> -	u16			int_vref_mv;
>  	u16			command;
>  	u16			c_mask;	/* Active voltage channels for events */
>  	struct mutex		state_lock;
> @@ -445,7 +446,15 @@ static int ad7291_read_raw(struct iio_dev *indio_dev,
>  	case IIO_CHAN_INFO_SCALE:
>  		switch (chan->type) {
>  		case IIO_VOLTAGE:
> -			*val = chip->int_vref_mv;
> +			if (chip->reg) {
> +				int vref;
> +				vref = regulator_get_voltage(chip->reg);
> +				if (vref < 0)
> +					return vref;
> +				*val = vref / 1000;
> +			} else {
> +				*val = 2500;
> +			}
>  			*val2 = AD7291_BITS;
>  			return IIO_VAL_FRACTIONAL_LOG2;
>  		case IIO_TEMP:
> @@ -513,9 +522,10 @@ static const struct iio_info ad7291_info = {
>  static int ad7291_probe(struct i2c_client *client,
>  		const struct i2c_device_id *id)
>  {
> +	struct ad7291_platform_data *pdata = client->dev.platform_data;
>  	struct ad7291_chip_info *chip;
>  	struct iio_dev *indio_dev;
> -	int ret = 0, voltage_uv = 0;
> +	int ret = 0;
>  
>  	indio_dev = iio_device_alloc(sizeof(*chip));
>  	if (indio_dev == NULL) {
> @@ -524,12 +534,14 @@ static int ad7291_probe(struct i2c_client *client,
>  	}
>  	chip = iio_priv(indio_dev);
>  
> -	chip->reg = regulator_get(&client->dev, "vcc");
> -	if (!IS_ERR(chip->reg)) {
> +	if (pdata && pdata->use_external_ref) {
> +		chip->reg = regulator_get(&client->dev, "vref");
> +		if (IS_ERR(chip->reg))
> +			goto error_free;
> +
>  		ret = regulator_enable(chip->reg);
>  		if (ret)
>  			goto error_put_reg;
> -		voltage_uv = regulator_get_voltage(chip->reg);
>  	}
>  
>  	mutex_init(&chip->state_lock);
> @@ -542,12 +554,8 @@ static int ad7291_probe(struct i2c_client *client,
>  			AD7291_T_SENSE_MASK | /* Tsense always enabled */
>  			AD7291_ALERT_POLARITY; /* set irq polarity low level */
>  
> -	if (voltage_uv) {
> -		chip->int_vref_mv = voltage_uv / 1000;
> +	if (pdata && pdata->use_external_ref)
>  		chip->command |= AD7291_EXT_REF;
> -	} else {
> -		chip->int_vref_mv = 2500; /* Build-in ref */
> -	}
>  
>  	indio_dev->name = id->name;
>  	indio_dev->channels = ad7291_channels;
> @@ -590,12 +598,12 @@ error_unreg_irq:
>  	if (client->irq)
>  		free_irq(client->irq, indio_dev);
>  error_disable_reg:
> -	if (!IS_ERR(chip->reg))
> +	if (chip->reg)
>  		regulator_disable(chip->reg);
>  error_put_reg:
> -	if (!IS_ERR(chip->reg))
> +	if (chip->reg)
>  		regulator_put(chip->reg);
> -
> +error_free:
>  	iio_device_free(indio_dev);
>  error_ret:
>  	return ret;
> @@ -611,7 +619,7 @@ static int ad7291_remove(struct i2c_client *client)
>  	if (client->irq)
>  		free_irq(client->irq, indio_dev);
>  
> -	if (!IS_ERR(chip->reg)) {
> +	if (chip->reg) {
>  		regulator_disable(chip->reg);
>  		regulator_put(chip->reg);
>  	}
> diff --git a/drivers/staging/iio/adc/ad7291.h b/drivers/staging/iio/adc/ad7291.h
> new file mode 100644
> index 0000000..bbd89fa
> --- /dev/null
> +++ b/drivers/staging/iio/adc/ad7291.h
> @@ -0,0 +1,12 @@
> +#ifndef __IIO_AD7291_H__
> +#define __IIO_AD7291_H__
> +
> +/**
> + * struct ad7291_platform_data - AD7291 platform data
> + * @use_external_ref: Whether to use an external or internal reference voltage
> + */
> +struct ad7291_platform_data {
> +	bool use_external_ref;
> +};
> +
> +#endif
> 

Re: [PATCH 1/7] staging:iio:ad7291: Use IIO_VAL_FRACTIONAL_LOG2

[Jonathan Cameron]

On 06/14/2013 04:58 PM, Lars-Peter Clausen wrote:
> This is a bit more straight forward and also allows better precession when using
> the an in-kernel consumer.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Applied to the togreg branch of iio.git
> ---
>  drivers/staging/iio/adc/ad7291.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/staging/iio/adc/ad7291.c b/drivers/staging/iio/adc/ad7291.c
> index d088c66..a3a3a2f 100644
> --- a/drivers/staging/iio/adc/ad7291.c
> +++ b/drivers/staging/iio/adc/ad7291.c
> @@ -456,7 +456,6 @@ static int ad7291_read_raw(struct iio_dev *indio_dev,
>  {
>  	int ret;
>  	struct ad7291_chip_info *chip = iio_priv(indio_dev);
> -	unsigned int scale_uv;
>  	u16 regval;
>  	s16 signval;
>  
> @@ -513,10 +512,9 @@ static int ad7291_read_raw(struct iio_dev *indio_dev,
>  	case IIO_CHAN_INFO_SCALE:
>  		switch (chan->type) {
>  		case IIO_VOLTAGE:
> -			scale_uv = (chip->int_vref_mv * 1000) >> AD7291_BITS;
> -			*val =  scale_uv / 1000;
> -			*val2 = (scale_uv % 1000) * 1000;
> -			return IIO_VAL_INT_PLUS_MICRO;
> +			*val = chip->int_vref_mv;
> +			*val2 = AD7291_BITS;
> +			return IIO_VAL_FRACTIONAL_LOG2;
>  		case IIO_TEMP:
>  			/*
>  			 * One LSB of the ADC corresponds to 0.25 deg C.
>