[PATCH RFC] iio: mxs-lradc: add scaling to enable divide_by_two operation

[PATCH RFC] iio: mxs-lradc: add scaling to enable divide_by_two operation

[Hector Palacios]

Most LRADC channels have an optional divisor by two which allows
a maximum input voltage of VDDIO - 50mV. This patch adds the scaling
info flag to these channels (all except 7 -VBATT-, 10 -VDDIO- and
15 -VDD5V-) to allow enabled divide_by_two read operation.

Reference: http://www.spinics.net/lists/linux-iio/msg08876.html
Signed-off-by: Hector Palacios <hector.palacios@digi.com>
---
 drivers/staging/iio/adc/mxs-lradc.c | 52 +++++++++++++++++++++++--------------
 1 file changed, 33 insertions(+), 19 deletions(-)

diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c
index 51f781f..4363bb6 100644
--- a/drivers/staging/iio/adc/mxs-lradc.c
+++ b/drivers/staging/iio/adc/mxs-lradc.c
@@ -197,6 +197,7 @@ struct mxs_lradc {
 #define	LRADC_CTRL1_LRADC_IRQ_OFFSET		0
 
 #define	LRADC_CTRL2				0x20
+#define LRADC_CTRL2_DIVIDE_BY_TWO_OFFSET	24
 #define	LRADC_CTRL2_TEMPSENSE_PWD		(1 << 15)
 
 #define	LRADC_STATUS				0x40
@@ -234,7 +235,7 @@ static int mxs_lradc_read_raw(struct iio_dev *iio_dev,
 	struct mxs_lradc *lradc = iio_priv(iio_dev);
 	int ret;
 
-	if (m != IIO_CHAN_INFO_RAW)
+	if (m != IIO_CHAN_INFO_RAW && m != IIO_CHAN_INFO_SCALE)
 		return -EINVAL;
 
 	/* Check for invalid channel */
@@ -262,6 +263,11 @@ static int mxs_lradc_read_raw(struct iio_dev *iio_dev,
 		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
 	writel(0xff, lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
 
+	/* Enable DIVIDE_BY_TWO on channel 0 if reading scaled value */
+	if (IIO_CHAN_INFO_SCALE == m)
+		writel(1 << LRADC_CTRL2_DIVIDE_BY_TWO_OFFSET,
+		       lradc->base + LRADC_CTRL2 + STMP_OFFSET_REG_SET);
+
 	/* Clean the slot's previous content, then set new one. */
 	writel(LRADC_CTRL4_LRADCSELECT_MASK(0),
 		lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_CLR);
@@ -286,6 +292,10 @@ static int mxs_lradc_read_raw(struct iio_dev *iio_dev,
 	ret = IIO_VAL_INT;
 
 err:
+	/* Disable DIVIDE_BY_TWO on channel 0 if reading scaled value */
+	if (IIO_CHAN_INFO_SCALE == m)
+		writel(1 << LRADC_CTRL2_DIVIDE_BY_TWO_OFFSET,
+		       lradc->base + LRADC_CTRL2 + STMP_OFFSET_REG_CLR);
 	writel(LRADC_CTRL1_LRADC_IRQ_EN(0),
 		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
 
@@ -806,11 +816,11 @@ static const struct iio_buffer_setup_ops mxs_lradc_buffer_ops = {
  * Driver initialization
  */
 
-#define MXS_ADC_CHAN(idx, chan_type) {				\
+#define MXS_ADC_CHAN(idx, chan_type, info) {			\
 	.type = (chan_type),					\
 	.indexed = 1,						\
 	.scan_index = (idx),					\
-	.info_mask = IIO_CHAN_INFO_RAW_SEPARATE_BIT,		\
+	.info_mask = (info),					\
 	.channel = (idx),					\
 	.scan_type = {						\
 		.sign = 'u',					\
@@ -819,23 +829,27 @@ static const struct iio_buffer_setup_ops mxs_lradc_buffer_ops = {
 	},							\
 }
 
+#define RAW		IIO_CHAN_INFO_RAW_SEPARATE_BIT
+#define RAW_SCALED	(IIO_CHAN_INFO_RAW_SEPARATE_BIT |	\
+			 IIO_CHAN_INFO_SCALE_SEPARATE_BIT)
+
 static const struct iio_chan_spec mxs_lradc_chan_spec[] = {
-	MXS_ADC_CHAN(0, IIO_VOLTAGE),
-	MXS_ADC_CHAN(1, IIO_VOLTAGE),
-	MXS_ADC_CHAN(2, IIO_VOLTAGE),
-	MXS_ADC_CHAN(3, IIO_VOLTAGE),
-	MXS_ADC_CHAN(4, IIO_VOLTAGE),
-	MXS_ADC_CHAN(5, IIO_VOLTAGE),
-	MXS_ADC_CHAN(6, IIO_VOLTAGE),
-	MXS_ADC_CHAN(7, IIO_VOLTAGE),	/* VBATT */
-	MXS_ADC_CHAN(8, IIO_TEMP),	/* Temp sense 0 */
-	MXS_ADC_CHAN(9, IIO_TEMP),	/* Temp sense 1 */
-	MXS_ADC_CHAN(10, IIO_VOLTAGE),	/* VDDIO */
-	MXS_ADC_CHAN(11, IIO_VOLTAGE),	/* VTH */
-	MXS_ADC_CHAN(12, IIO_VOLTAGE),	/* VDDA */
-	MXS_ADC_CHAN(13, IIO_VOLTAGE),	/* VDDD */
-	MXS_ADC_CHAN(14, IIO_VOLTAGE),	/* VBG */
-	MXS_ADC_CHAN(15, IIO_VOLTAGE),	/* VDD5V */
+	MXS_ADC_CHAN(0, IIO_VOLTAGE, RAW_SCALED),
+	MXS_ADC_CHAN(1, IIO_VOLTAGE, RAW_SCALED),
+	MXS_ADC_CHAN(2, IIO_VOLTAGE, RAW_SCALED),
+	MXS_ADC_CHAN(3, IIO_VOLTAGE, RAW_SCALED),
+	MXS_ADC_CHAN(4, IIO_VOLTAGE, RAW_SCALED),
+	MXS_ADC_CHAN(5, IIO_VOLTAGE, RAW_SCALED),
+	MXS_ADC_CHAN(6, IIO_VOLTAGE, RAW_SCALED),
+	MXS_ADC_CHAN(7, IIO_VOLTAGE, RAW),		/* VBATT */
+	MXS_ADC_CHAN(8, IIO_TEMP, RAW_SCALED),		/* Temp sense 0 */
+	MXS_ADC_CHAN(9, IIO_TEMP, RAW_SCALED),		/* Temp sense 1 */
+	MXS_ADC_CHAN(10, IIO_VOLTAGE, RAW),		/* VDDIO */
+	MXS_ADC_CHAN(11, IIO_VOLTAGE, RAW_SCALED),	/* VTH */
+	MXS_ADC_CHAN(12, IIO_VOLTAGE, RAW_SCALED),	/* VDDA */
+	MXS_ADC_CHAN(13, IIO_VOLTAGE, RAW_SCALED),	/* VDDD */
+	MXS_ADC_CHAN(14, IIO_VOLTAGE, RAW_SCALED),	/* VBG */
+	MXS_ADC_CHAN(15, IIO_VOLTAGE, RAW),		/* VDD5V */
 };
 
 static void mxs_lradc_hw_init(struct mxs_lradc *lradc)

Re: [PATCH RFC] iio: mxs-lradc: add scaling to enable divide_by_two operation

[Alexandre Belloni]

Dear Hector,

On 03/07/2013 12:06, Hector Palacios wrote:
> Most LRADC channels have an optional divisor by two which allows
> a maximum input voltage of VDDIO - 50mV. This patch adds the scaling
> info flag to these channels (all except 7 -VBATT-, 10 -VDDIO- and
> 15 -VDD5V-) to allow enabled divide_by_two read operation.
> 
> Reference: http://www.spinics.net/lists/linux-iio/msg08876.html
> Signed-off-by: Hector Palacios <hector.palacios@digi.com>
> ---
>  drivers/staging/iio/adc/mxs-lradc.c | 52 +++++++++++++++++++++++--------------
>  1 file changed, 33 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c
> index 51f781f..4363bb6 100644
> --- a/drivers/staging/iio/adc/mxs-lradc.c
> +++ b/drivers/staging/iio/adc/mxs-lradc.c
> @@ -197,6 +197,7 @@ struct mxs_lradc {
>  #define	LRADC_CTRL1_LRADC_IRQ_OFFSET		0
>  
>  #define	LRADC_CTRL2				0x20
> +#define LRADC_CTRL2_DIVIDE_BY_TWO_OFFSET	24
>  #define	LRADC_CTRL2_TEMPSENSE_PWD		(1 << 15)
>  
>  #define	LRADC_STATUS				0x40
> @@ -234,7 +235,7 @@ static int mxs_lradc_read_raw(struct iio_dev *iio_dev,
>  	struct mxs_lradc *lradc = iio_priv(iio_dev);
>  	int ret;
>  
> -	if (m != IIO_CHAN_INFO_RAW)
> +	if (m != IIO_CHAN_INFO_RAW && m != IIO_CHAN_INFO_SCALE)
>  		return -EINVAL;
>  
>  	/* Check for invalid channel */
> @@ -262,6 +263,11 @@ static int mxs_lradc_read_raw(struct iio_dev *iio_dev,
>  		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
>  	writel(0xff, lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
>  
> +	/* Enable DIVIDE_BY_TWO on channel 0 if reading scaled value */
> +	if (IIO_CHAN_INFO_SCALE == m)
> +		writel(1 << LRADC_CTRL2_DIVIDE_BY_TWO_OFFSET,
> +		       lradc->base + LRADC_CTRL2 + STMP_OFFSET_REG_SET);
> +
>  	/* Clean the slot's previous content, then set new one. */
>  	writel(LRADC_CTRL4_LRADCSELECT_MASK(0),
>  		lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_CLR);
> @@ -286,6 +292,10 @@ static int mxs_lradc_read_raw(struct iio_dev *iio_dev,
>  	ret = IIO_VAL_INT;
>  
>  err:
> +	/* Disable DIVIDE_BY_TWO on channel 0 if reading scaled value */
> +	if (IIO_CHAN_INFO_SCALE == m)
> +		writel(1 << LRADC_CTRL2_DIVIDE_BY_TWO_OFFSET,
> +		       lradc->base + LRADC_CTRL2 + STMP_OFFSET_REG_CLR);
>  	writel(LRADC_CTRL1_LRADC_IRQ_EN(0),
>  		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
>  
> @@ -806,11 +816,11 @@ static const struct iio_buffer_setup_ops mxs_lradc_buffer_ops = {
>   * Driver initialization
>   */
>  
> -#define MXS_ADC_CHAN(idx, chan_type) {				\
> +#define MXS_ADC_CHAN(idx, chan_type, info) {			\
>  	.type = (chan_type),					\
>  	.indexed = 1,						\
>  	.scan_index = (idx),					\
> -	.info_mask = IIO_CHAN_INFO_RAW_SEPARATE_BIT,		\
> +	.info_mask = (info),					\
>  	.channel = (idx),					\
>  	.scan_type = {						\
>  		.sign = 'u',					\
> @@ -819,23 +829,27 @@ static const struct iio_buffer_setup_ops mxs_lradc_buffer_ops = {
>  	},							\
>  }
>  
> +#define RAW		IIO_CHAN_INFO_RAW_SEPARATE_BIT
> +#define RAW_SCALED	(IIO_CHAN_INFO_RAW_SEPARATE_BIT |	\
> +			 IIO_CHAN_INFO_SCALE_SEPARATE_BIT)
> +
>  static const struct iio_chan_spec mxs_lradc_chan_spec[] = {
> -	MXS_ADC_CHAN(0, IIO_VOLTAGE),
> -	MXS_ADC_CHAN(1, IIO_VOLTAGE),
> -	MXS_ADC_CHAN(2, IIO_VOLTAGE),
> -	MXS_ADC_CHAN(3, IIO_VOLTAGE),
> -	MXS_ADC_CHAN(4, IIO_VOLTAGE),
> -	MXS_ADC_CHAN(5, IIO_VOLTAGE),
> -	MXS_ADC_CHAN(6, IIO_VOLTAGE),
> -	MXS_ADC_CHAN(7, IIO_VOLTAGE),	/* VBATT */
> -	MXS_ADC_CHAN(8, IIO_TEMP),	/* Temp sense 0 */
> -	MXS_ADC_CHAN(9, IIO_TEMP),	/* Temp sense 1 */
> -	MXS_ADC_CHAN(10, IIO_VOLTAGE),	/* VDDIO */
> -	MXS_ADC_CHAN(11, IIO_VOLTAGE),	/* VTH */
> -	MXS_ADC_CHAN(12, IIO_VOLTAGE),	/* VDDA */
> -	MXS_ADC_CHAN(13, IIO_VOLTAGE),	/* VDDD */
> -	MXS_ADC_CHAN(14, IIO_VOLTAGE),	/* VBG */
> -	MXS_ADC_CHAN(15, IIO_VOLTAGE),	/* VDD5V */
> +	MXS_ADC_CHAN(0, IIO_VOLTAGE, RAW_SCALED),
> +	MXS_ADC_CHAN(1, IIO_VOLTAGE, RAW_SCALED),
> +	MXS_ADC_CHAN(2, IIO_VOLTAGE, RAW_SCALED),
> +	MXS_ADC_CHAN(3, IIO_VOLTAGE, RAW_SCALED),
> +	MXS_ADC_CHAN(4, IIO_VOLTAGE, RAW_SCALED),
> +	MXS_ADC_CHAN(5, IIO_VOLTAGE, RAW_SCALED),
> +	MXS_ADC_CHAN(6, IIO_VOLTAGE, RAW_SCALED),
> +	MXS_ADC_CHAN(7, IIO_VOLTAGE, RAW),		/* VBATT */
> +	MXS_ADC_CHAN(8, IIO_TEMP, RAW_SCALED),		/* Temp sense 0 */
> +	MXS_ADC_CHAN(9, IIO_TEMP, RAW_SCALED),		/* Temp sense 1 */
> +	MXS_ADC_CHAN(10, IIO_VOLTAGE, RAW),		/* VDDIO */
> +	MXS_ADC_CHAN(11, IIO_VOLTAGE, RAW_SCALED),	/* VTH */
> +	MXS_ADC_CHAN(12, IIO_VOLTAGE, RAW_SCALED),	/* VDDA */
> +	MXS_ADC_CHAN(13, IIO_VOLTAGE, RAW_SCALED),	/* VDDD */
> +	MXS_ADC_CHAN(14, IIO_VOLTAGE, RAW_SCALED),	/* VBG */
> +	MXS_ADC_CHAN(15, IIO_VOLTAGE, RAW),		/* VDD5V */
>  };
>  
>  static void mxs_lradc_hw_init(struct mxs_lradc *lradc)


This is not how it is supposed to work. you should actually add
IIO_CHAN_INFO_SCALE_SEPARATE_BIT to all channels .info_mask_separate.

Then, you will get two files: in_voltageX_raw and in_voltageX_scale.
Userland should calculate in_voltageX_raw * in_voltageX_scale. So when
reading in_voltageX_scale, you actually want to read the current scale.
To set the scale, you'll have to implement a write_raw.

Also, it would probably be a good idea to expose a
in_voltageX_scale_available file.


-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH RFC] iio: mxs-lradc: add scaling to enable divide_by_two operation

[Hector Palacios]

Dear Alexandre,

On 07/03/2013 12:25 PM, Alexandre Belloni wrote:
> Dear Hector,
>
> On 03/07/2013 12:06, Hector Palacios wrote:
>> Most LRADC channels have an optional divisor by two which allows
>> a maximum input voltage of VDDIO - 50mV. This patch adds the scaling
>> info flag to these channels (all except 7 -VBATT-, 10 -VDDIO- and
>> 15 -VDD5V-) to allow enabled divide_by_two read operation.
>>
>> Reference: http://www.spinics.net/lists/linux-iio/msg08876.html
>> Signed-off-by: Hector Palacios <hector.palacios@digi.com>
>> ---
>>   drivers/staging/iio/adc/mxs-lradc.c | 52 +++++++++++++++++++++++--------------
>>   1 file changed, 33 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c
>> index 51f781f..4363bb6 100644
>> --- a/drivers/staging/iio/adc/mxs-lradc.c
>> +++ b/drivers/staging/iio/adc/mxs-lradc.c
>> @@ -197,6 +197,7 @@ struct mxs_lradc {
>>   #define	LRADC_CTRL1_LRADC_IRQ_OFFSET		0
>>
>>   #define	LRADC_CTRL2				0x20
>> +#define LRADC_CTRL2_DIVIDE_BY_TWO_OFFSET	24
>>   #define	LRADC_CTRL2_TEMPSENSE_PWD		(1 << 15)
>>
>>   #define	LRADC_STATUS				0x40
>> @@ -234,7 +235,7 @@ static int mxs_lradc_read_raw(struct iio_dev *iio_dev,
>>   	struct mxs_lradc *lradc = iio_priv(iio_dev);
>>   	int ret;
>>
>> -	if (m != IIO_CHAN_INFO_RAW)
>> +	if (m != IIO_CHAN_INFO_RAW && m != IIO_CHAN_INFO_SCALE)
>>   		return -EINVAL;
>>
>>   	/* Check for invalid channel */
>> @@ -262,6 +263,11 @@ static int mxs_lradc_read_raw(struct iio_dev *iio_dev,
>>   		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
>>   	writel(0xff, lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
>>
>> +	/* Enable DIVIDE_BY_TWO on channel 0 if reading scaled value */
>> +	if (IIO_CHAN_INFO_SCALE == m)
>> +		writel(1 << LRADC_CTRL2_DIVIDE_BY_TWO_OFFSET,
>> +		       lradc->base + LRADC_CTRL2 + STMP_OFFSET_REG_SET);
>> +
>>   	/* Clean the slot's previous content, then set new one. */
>>   	writel(LRADC_CTRL4_LRADCSELECT_MASK(0),
>>   		lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_CLR);
>> @@ -286,6 +292,10 @@ static int mxs_lradc_read_raw(struct iio_dev *iio_dev,
>>   	ret = IIO_VAL_INT;
>>
>>   err:
>> +	/* Disable DIVIDE_BY_TWO on channel 0 if reading scaled value */
>> +	if (IIO_CHAN_INFO_SCALE == m)
>> +		writel(1 << LRADC_CTRL2_DIVIDE_BY_TWO_OFFSET,
>> +		       lradc->base + LRADC_CTRL2 + STMP_OFFSET_REG_CLR);
>>   	writel(LRADC_CTRL1_LRADC_IRQ_EN(0),
>>   		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
>>
>> @@ -806,11 +816,11 @@ static const struct iio_buffer_setup_ops mxs_lradc_buffer_ops = {
>>    * Driver initialization
>>    */
>>
>> -#define MXS_ADC_CHAN(idx, chan_type) {				\
>> +#define MXS_ADC_CHAN(idx, chan_type, info) {			\
>>   	.type = (chan_type),					\
>>   	.indexed = 1,						\
>>   	.scan_index = (idx),					\
>> -	.info_mask = IIO_CHAN_INFO_RAW_SEPARATE_BIT,		\
>> +	.info_mask = (info),					\
>>   	.channel = (idx),					\
>>   	.scan_type = {						\
>>   		.sign = 'u',					\
>> @@ -819,23 +829,27 @@ static const struct iio_buffer_setup_ops mxs_lradc_buffer_ops = {
>>   	},							\
>>   }
>>
>> +#define RAW		IIO_CHAN_INFO_RAW_SEPARATE_BIT
>> +#define RAW_SCALED	(IIO_CHAN_INFO_RAW_SEPARATE_BIT |	\
>> +			 IIO_CHAN_INFO_SCALE_SEPARATE_BIT)
>> +
>>   static const struct iio_chan_spec mxs_lradc_chan_spec[] = {
>> -	MXS_ADC_CHAN(0, IIO_VOLTAGE),
>> -	MXS_ADC_CHAN(1, IIO_VOLTAGE),
>> -	MXS_ADC_CHAN(2, IIO_VOLTAGE),
>> -	MXS_ADC_CHAN(3, IIO_VOLTAGE),
>> -	MXS_ADC_CHAN(4, IIO_VOLTAGE),
>> -	MXS_ADC_CHAN(5, IIO_VOLTAGE),
>> -	MXS_ADC_CHAN(6, IIO_VOLTAGE),
>> -	MXS_ADC_CHAN(7, IIO_VOLTAGE),	/* VBATT */
>> -	MXS_ADC_CHAN(8, IIO_TEMP),	/* Temp sense 0 */
>> -	MXS_ADC_CHAN(9, IIO_TEMP),	/* Temp sense 1 */
>> -	MXS_ADC_CHAN(10, IIO_VOLTAGE),	/* VDDIO */
>> -	MXS_ADC_CHAN(11, IIO_VOLTAGE),	/* VTH */
>> -	MXS_ADC_CHAN(12, IIO_VOLTAGE),	/* VDDA */
>> -	MXS_ADC_CHAN(13, IIO_VOLTAGE),	/* VDDD */
>> -	MXS_ADC_CHAN(14, IIO_VOLTAGE),	/* VBG */
>> -	MXS_ADC_CHAN(15, IIO_VOLTAGE),	/* VDD5V */
>> +	MXS_ADC_CHAN(0, IIO_VOLTAGE, RAW_SCALED),
>> +	MXS_ADC_CHAN(1, IIO_VOLTAGE, RAW_SCALED),
>> +	MXS_ADC_CHAN(2, IIO_VOLTAGE, RAW_SCALED),
>> +	MXS_ADC_CHAN(3, IIO_VOLTAGE, RAW_SCALED),
>> +	MXS_ADC_CHAN(4, IIO_VOLTAGE, RAW_SCALED),
>> +	MXS_ADC_CHAN(5, IIO_VOLTAGE, RAW_SCALED),
>> +	MXS_ADC_CHAN(6, IIO_VOLTAGE, RAW_SCALED),
>> +	MXS_ADC_CHAN(7, IIO_VOLTAGE, RAW),		/* VBATT */
>> +	MXS_ADC_CHAN(8, IIO_TEMP, RAW_SCALED),		/* Temp sense 0 */
>> +	MXS_ADC_CHAN(9, IIO_TEMP, RAW_SCALED),		/* Temp sense 1 */
>> +	MXS_ADC_CHAN(10, IIO_VOLTAGE, RAW),		/* VDDIO */
>> +	MXS_ADC_CHAN(11, IIO_VOLTAGE, RAW_SCALED),	/* VTH */
>> +	MXS_ADC_CHAN(12, IIO_VOLTAGE, RAW_SCALED),	/* VDDA */
>> +	MXS_ADC_CHAN(13, IIO_VOLTAGE, RAW_SCALED),	/* VDDD */
>> +	MXS_ADC_CHAN(14, IIO_VOLTAGE, RAW_SCALED),	/* VBG */
>> +	MXS_ADC_CHAN(15, IIO_VOLTAGE, RAW),		/* VDD5V */
>>   };
>>
>>   static void mxs_lradc_hw_init(struct mxs_lradc *lradc)
>
>
> This is not how it is supposed to work. you should actually add
> IIO_CHAN_INFO_SCALE_SEPARATE_BIT to all channels .info_mask_separate.
>
> Then, you will get two files: in_voltageX_raw and in_voltageX_scale.
> Userland should calculate in_voltageX_raw * in_voltageX_scale. So when
> reading in_voltageX_scale, you actually want to read the current scale.
> To set the scale, you'll have to implement a write_raw.
>
> Also, it would probably be a good idea to expose a
> in_voltageX_scale_available file.

Thanks, I totally misunderstood it.
One question: to implement a divisor by two what should I write to the scale? A 
decimal number like 0.5? And I guess I should return -EINVAL for any other value, right?


Best regards,
--
Hector Palacios

Re: [PATCH RFC] iio: mxs-lradc: add scaling to enable divide_by_two operation

[Lars-Peter Clausen]

On 07/03/2013 12:06 PM, Hector Palacios wrote:
> Most LRADC channels have an optional divisor by two which allows
> a maximum input voltage of VDDIO - 50mV. This patch adds the scaling
> info flag to these channels (all except 7 -VBATT-, 10 -VDDIO- and
> 15 -VDD5V-) to allow enabled divide_by_two read operation.
> 
> Reference: http://www.spinics.net/lists/linux-iio/msg08876.html
> Signed-off-by: Hector Palacios <hector.palacios@digi.com>

The scale attribute reports the scale of the raw value. To gain the value of
the result in mV you have to multiply the raw attribute with the scale
attribute. The scale attribute does not return the result of a conversion with
a scale different applied.

I suggest you implement this in two steps. First implement the scale property
for all channels. Use a LUT for each channel's fullscale value (in mV) and
implement reading the scale like this.

case IIO_CHAN_INFO_SCALE:
*val = lut[chan->channel];
*val2 = chan->scan_type.realbits;
return IIO_VAL_FRACTIONAL_LOG2;

The next step is to make the scale modifiable for channels which support this.

- Lars

> ---
>  drivers/staging/iio/adc/mxs-lradc.c | 52 +++++++++++++++++++++++--------------
>  1 file changed, 33 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c
> index 51f781f..4363bb6 100644
> --- a/drivers/staging/iio/adc/mxs-lradc.c
> +++ b/drivers/staging/iio/adc/mxs-lradc.c
> @@ -197,6 +197,7 @@ struct mxs_lradc {
>  #define	LRADC_CTRL1_LRADC_IRQ_OFFSET		0
>  
>  #define	LRADC_CTRL2				0x20
> +#define LRADC_CTRL2_DIVIDE_BY_TWO_OFFSET	24
>  #define	LRADC_CTRL2_TEMPSENSE_PWD		(1 << 15)
>  
>  #define	LRADC_STATUS				0x40
> @@ -234,7 +235,7 @@ static int mxs_lradc_read_raw(struct iio_dev *iio_dev,
>  	struct mxs_lradc *lradc = iio_priv(iio_dev);
>  	int ret;
>  
> -	if (m != IIO_CHAN_INFO_RAW)
> +	if (m != IIO_CHAN_INFO_RAW && m != IIO_CHAN_INFO_SCALE)
>  		return -EINVAL;
>  
>  	/* Check for invalid channel */
> @@ -262,6 +263,11 @@ static int mxs_lradc_read_raw(struct iio_dev *iio_dev,
>  		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
>  	writel(0xff, lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
>  
> +	/* Enable DIVIDE_BY_TWO on channel 0 if reading scaled value */
> +	if (IIO_CHAN_INFO_SCALE == m)
> +		writel(1 << LRADC_CTRL2_DIVIDE_BY_TWO_OFFSET,
> +		       lradc->base + LRADC_CTRL2 + STMP_OFFSET_REG_SET);
> +
>  	/* Clean the slot's previous content, then set new one. */
>  	writel(LRADC_CTRL4_LRADCSELECT_MASK(0),
>  		lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_CLR);
> @@ -286,6 +292,10 @@ static int mxs_lradc_read_raw(struct iio_dev *iio_dev,
>  	ret = IIO_VAL_INT;
>  
>  err:
> +	/* Disable DIVIDE_BY_TWO on channel 0 if reading scaled value */
> +	if (IIO_CHAN_INFO_SCALE == m)
> +		writel(1 << LRADC_CTRL2_DIVIDE_BY_TWO_OFFSET,
> +		       lradc->base + LRADC_CTRL2 + STMP_OFFSET_REG_CLR);
>  	writel(LRADC_CTRL1_LRADC_IRQ_EN(0),
>  		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
>  
> @@ -806,11 +816,11 @@ static const struct iio_buffer_setup_ops mxs_lradc_buffer_ops = {
>   * Driver initialization
>   */
>  
> -#define MXS_ADC_CHAN(idx, chan_type) {				\
> +#define MXS_ADC_CHAN(idx, chan_type, info) {			\
>  	.type = (chan_type),					\
>  	.indexed = 1,						\
>  	.scan_index = (idx),					\
> -	.info_mask = IIO_CHAN_INFO_RAW_SEPARATE_BIT,		\
> +	.info_mask = (info),					\
>  	.channel = (idx),					\
>  	.scan_type = {						\
>  		.sign = 'u',					\
> @@ -819,23 +829,27 @@ static const struct iio_buffer_setup_ops mxs_lradc_buffer_ops = {
>  	},							\
>  }
>  
> +#define RAW		IIO_CHAN_INFO_RAW_SEPARATE_BIT
> +#define RAW_SCALED	(IIO_CHAN_INFO_RAW_SEPARATE_BIT |	\
> +			 IIO_CHAN_INFO_SCALE_SEPARATE_BIT)
> +
>  static const struct iio_chan_spec mxs_lradc_chan_spec[] = {
> -	MXS_ADC_CHAN(0, IIO_VOLTAGE),
> -	MXS_ADC_CHAN(1, IIO_VOLTAGE),
> -	MXS_ADC_CHAN(2, IIO_VOLTAGE),
> -	MXS_ADC_CHAN(3, IIO_VOLTAGE),
> -	MXS_ADC_CHAN(4, IIO_VOLTAGE),
> -	MXS_ADC_CHAN(5, IIO_VOLTAGE),
> -	MXS_ADC_CHAN(6, IIO_VOLTAGE),
> -	MXS_ADC_CHAN(7, IIO_VOLTAGE),	/* VBATT */
> -	MXS_ADC_CHAN(8, IIO_TEMP),	/* Temp sense 0 */
> -	MXS_ADC_CHAN(9, IIO_TEMP),	/* Temp sense 1 */
> -	MXS_ADC_CHAN(10, IIO_VOLTAGE),	/* VDDIO */
> -	MXS_ADC_CHAN(11, IIO_VOLTAGE),	/* VTH */
> -	MXS_ADC_CHAN(12, IIO_VOLTAGE),	/* VDDA */
> -	MXS_ADC_CHAN(13, IIO_VOLTAGE),	/* VDDD */
> -	MXS_ADC_CHAN(14, IIO_VOLTAGE),	/* VBG */
> -	MXS_ADC_CHAN(15, IIO_VOLTAGE),	/* VDD5V */
> +	MXS_ADC_CHAN(0, IIO_VOLTAGE, RAW_SCALED),
> +	MXS_ADC_CHAN(1, IIO_VOLTAGE, RAW_SCALED),
> +	MXS_ADC_CHAN(2, IIO_VOLTAGE, RAW_SCALED),
> +	MXS_ADC_CHAN(3, IIO_VOLTAGE, RAW_SCALED),
> +	MXS_ADC_CHAN(4, IIO_VOLTAGE, RAW_SCALED),
> +	MXS_ADC_CHAN(5, IIO_VOLTAGE, RAW_SCALED),
> +	MXS_ADC_CHAN(6, IIO_VOLTAGE, RAW_SCALED),
> +	MXS_ADC_CHAN(7, IIO_VOLTAGE, RAW),		/* VBATT */
> +	MXS_ADC_CHAN(8, IIO_TEMP, RAW_SCALED),		/* Temp sense 0 */
> +	MXS_ADC_CHAN(9, IIO_TEMP, RAW_SCALED),		/* Temp sense 1 */
> +	MXS_ADC_CHAN(10, IIO_VOLTAGE, RAW),		/* VDDIO */
> +	MXS_ADC_CHAN(11, IIO_VOLTAGE, RAW_SCALED),	/* VTH */
> +	MXS_ADC_CHAN(12, IIO_VOLTAGE, RAW_SCALED),	/* VDDA */
> +	MXS_ADC_CHAN(13, IIO_VOLTAGE, RAW_SCALED),	/* VDDD */
> +	MXS_ADC_CHAN(14, IIO_VOLTAGE, RAW_SCALED),	/* VBG */
> +	MXS_ADC_CHAN(15, IIO_VOLTAGE, RAW),		/* VDD5V */
>  };
>  
>  static void mxs_lradc_hw_init(struct mxs_lradc *lradc)

Re: [PATCH RFC] iio: mxs-lradc: add scaling to enable divide_by_two operation

[Marek Vasut]

Dear Hector Palacios,

> Most LRADC channels have an optional divisor by two which allows
> a maximum input voltage of VDDIO - 50mV. This patch adds the scaling
> info flag to these channels (all except 7 -VBATT-, 10 -VDDIO- and
> 15 -VDD5V-) to allow enabled divide_by_two read operation.
> 
> Reference: http://www.spinics.net/lists/linux-iio/msg08876.html
> Signed-off-by: Hector Palacios <hector.palacios@digi.com>
> ---
>  drivers/staging/iio/adc/mxs-lradc.c | 52
> +++++++++++++++++++++++-------------- 1 file changed, 33 insertions(+), 19
> deletions(-)
> 
> diff --git a/drivers/staging/iio/adc/mxs-lradc.c
> b/drivers/staging/iio/adc/mxs-lradc.c index 51f781f..4363bb6 100644
> --- a/drivers/staging/iio/adc/mxs-lradc.c
> +++ b/drivers/staging/iio/adc/mxs-lradc.c
> @@ -197,6 +197,7 @@ struct mxs_lradc {
>  #define	LRADC_CTRL1_LRADC_IRQ_OFFSET		0
> 
>  #define	LRADC_CTRL2				0x20
> +#define LRADC_CTRL2_DIVIDE_BY_TWO_OFFSET	24
>  #define	LRADC_CTRL2_TEMPSENSE_PWD		(1 << 15)
> 
>  #define	LRADC_STATUS				0x40
> @@ -234,7 +235,7 @@ static int mxs_lradc_read_raw(struct iio_dev *iio_dev,
>  	struct mxs_lradc *lradc = iio_priv(iio_dev);
>  	int ret;
> 
> -	if (m != IIO_CHAN_INFO_RAW)
> +	if (m != IIO_CHAN_INFO_RAW && m != IIO_CHAN_INFO_SCALE)
>  		return -EINVAL;
> 
>  	/* Check for invalid channel */
> @@ -262,6 +263,11 @@ static int mxs_lradc_read_raw(struct iio_dev *iio_dev,
>  		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
>  	writel(0xff, lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
> 
> +	/* Enable DIVIDE_BY_TWO on channel 0 if reading scaled value */
> +	if (IIO_CHAN_INFO_SCALE == m)
> +		writel(1 << LRADC_CTRL2_DIVIDE_BY_TWO_OFFSET,
> +		       lradc->base + LRADC_CTRL2 + STMP_OFFSET_REG_SET);
> +
>  	/* Clean the slot's previous content, then set new one. */
>  	writel(LRADC_CTRL4_LRADCSELECT_MASK(0),
>  		lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_CLR);
> @@ -286,6 +292,10 @@ static int mxs_lradc_read_raw(struct iio_dev *iio_dev,
>  	ret = IIO_VAL_INT;
> 
>  err:
> +	/* Disable DIVIDE_BY_TWO on channel 0 if reading scaled value */
> +	if (IIO_CHAN_INFO_SCALE == m)
> +		writel(1 << LRADC_CTRL2_DIVIDE_BY_TWO_OFFSET,
> +		       lradc->base + LRADC_CTRL2 + STMP_OFFSET_REG_CLR);
>  	writel(LRADC_CTRL1_LRADC_IRQ_EN(0),
>  		lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
> 
> @@ -806,11 +816,11 @@ static const struct iio_buffer_setup_ops
> mxs_lradc_buffer_ops = { * Driver initialization
>   */
> 
> -#define MXS_ADC_CHAN(idx, chan_type) {				\
> +#define MXS_ADC_CHAN(idx, chan_type, info) {			\
>  	.type = (chan_type),					\
>  	.indexed = 1,						\
>  	.scan_index = (idx),					\
> -	.info_mask = IIO_CHAN_INFO_RAW_SEPARATE_BIT,		\
> +	.info_mask = (info),					\
>  	.channel = (idx),					\
>  	.scan_type = {						\
>  		.sign = 'u',					\
> @@ -819,23 +829,27 @@ static const struct iio_buffer_setup_ops
> mxs_lradc_buffer_ops = { },							
\
>  }
> 
> +#define RAW		IIO_CHAN_INFO_RAW_SEPARATE_BIT
> +#define RAW_SCALED	(IIO_CHAN_INFO_RAW_SEPARATE_BIT |	\
> +			 IIO_CHAN_INFO_SCALE_SEPARATE_BIT)
> +
>  static const struct iio_chan_spec mxs_lradc_chan_spec[] = {
> -	MXS_ADC_CHAN(0, IIO_VOLTAGE),
> -	MXS_ADC_CHAN(1, IIO_VOLTAGE),
> -	MXS_ADC_CHAN(2, IIO_VOLTAGE),
> -	MXS_ADC_CHAN(3, IIO_VOLTAGE),
> -	MXS_ADC_CHAN(4, IIO_VOLTAGE),
> -	MXS_ADC_CHAN(5, IIO_VOLTAGE),
> -	MXS_ADC_CHAN(6, IIO_VOLTAGE),
> -	MXS_ADC_CHAN(7, IIO_VOLTAGE),	/* VBATT */
> -	MXS_ADC_CHAN(8, IIO_TEMP),	/* Temp sense 0 */
> -	MXS_ADC_CHAN(9, IIO_TEMP),	/* Temp sense 1 */
> -	MXS_ADC_CHAN(10, IIO_VOLTAGE),	/* VDDIO */
> -	MXS_ADC_CHAN(11, IIO_VOLTAGE),	/* VTH */
> -	MXS_ADC_CHAN(12, IIO_VOLTAGE),	/* VDDA */
> -	MXS_ADC_CHAN(13, IIO_VOLTAGE),	/* VDDD */
> -	MXS_ADC_CHAN(14, IIO_VOLTAGE),	/* VBG */
> -	MXS_ADC_CHAN(15, IIO_VOLTAGE),	/* VDD5V */
> +	MXS_ADC_CHAN(0, IIO_VOLTAGE, RAW_SCALED),
> +	MXS_ADC_CHAN(1, IIO_VOLTAGE, RAW_SCALED),
> +	MXS_ADC_CHAN(2, IIO_VOLTAGE, RAW_SCALED),
> +	MXS_ADC_CHAN(3, IIO_VOLTAGE, RAW_SCALED),
> +	MXS_ADC_CHAN(4, IIO_VOLTAGE, RAW_SCALED),
> +	MXS_ADC_CHAN(5, IIO_VOLTAGE, RAW_SCALED),
> +	MXS_ADC_CHAN(6, IIO_VOLTAGE, RAW_SCALED),
> +	MXS_ADC_CHAN(7, IIO_VOLTAGE, RAW),		/* VBATT */
> +	MXS_ADC_CHAN(8, IIO_TEMP, RAW_SCALED),		/* Temp sense 0 */
> +	MXS_ADC_CHAN(9, IIO_TEMP, RAW_SCALED),		/* Temp sense 1 */
> +	MXS_ADC_CHAN(10, IIO_VOLTAGE, RAW),		/* VDDIO */
> +	MXS_ADC_CHAN(11, IIO_VOLTAGE, RAW_SCALED),	/* VTH */
> +	MXS_ADC_CHAN(12, IIO_VOLTAGE, RAW_SCALED),	/* VDDA */
> +	MXS_ADC_CHAN(13, IIO_VOLTAGE, RAW_SCALED),	/* VDDD */
> +	MXS_ADC_CHAN(14, IIO_VOLTAGE, RAW_SCALED),	/* VBG */
> +	MXS_ADC_CHAN(15, IIO_VOLTAGE, RAW),		/* VDD5V */

Please check the MX23 datasheet, I think what is, isn't and how is scaled 
differs there. Also, can you make sure you also implement the fixed pre-dividers 
? I think some channel is pre-divided with 1:4 ratio etc (see bullets at the top 
of 38.2 in MX28 datasheet).

Best regards,
Marek Vasut

Re: [PATCH RFC] iio: mxs-lradc: add scaling to enable divide_by_two operation

[Hector Palacios]

Dear Lars,

On 07/03/2013 12:46 PM, Lars-Peter Clausen wrote:
> The scale attribute reports the scale of the raw value. To gain the value of
> the result in mV you have to multiply the raw attribute with the scale
> attribute. The scale attribute does not return the result of a conversion with
> a scale different applied.

Thank you for the guidance, I misunderstood the whole thing.

> I suggest you implement this in two steps. First implement the scale property
> for all channels. Use a LUT for each channel's fullscale value (in mV) and
> implement reading the scale like this.
>
> case IIO_CHAN_INFO_SCALE:
> *val = lut[chan->channel];
> *val2 = chan->scan_type.realbits;
> return IIO_VAL_FRACTIONAL_LOG2;
>
> The next step is to make the scale modifiable for channels which support this.

I'm sorry, it's the first time I work with an iio driver and I'm afraid I don't 
understand this.
Let's say the fullscale value for channel 0 is 1.85V by default (when divide_by_two is 
disabled) and 3.7V (when enabled).

The channel's realbits is currently set to 18, because (although the ADC resolution is 
12 bits) the device can read several samples and return an accumulated value in 18 bits.

Now, considering the default case where 1.85V is the max ref voltage and considering 
that the driver only reads one sample during the raw_read operation (12 bits), what 
value should a read of 'scale' return?

	scale = Vref_mv / ((1 << 12) - 1) = 1850 /4095 = 0.45177045 ?

That is, the value to multiply the raw value to get the voltage in mV?
So that if reading the 'raw' returns say 425 and reading scale returns 0.45177 the 
measured voltage is 425 * 0.45177 = 192mV?


Then let's say I have implemented a function to modify the scale (which should 
eventually allow me to enable/disable the by two divisor). What is the user supposed 
to write to the 'scale'? Is he supposed to supply a decimal number like 0.45177 * 2 = 
0.903? This would be pretty odd.
Should I create a table instead so that writing a 0 means divisor disabled and writing 
1 means divisor enabled, for example?

Does the iio provide any descriptor that automatically shows the value in mV (already 
converted)?

Thank you for your help.
Best regards,
--
Hector Palacios

Re: [PATCH RFC] iio: mxs-lradc: add scaling to enable divide_by_two operation

[Alexandre Belloni]

On 03/07/2013 18:39, Hector Palacios wrote:
> Dear Lars,
> 
> On 07/03/2013 12:46 PM, Lars-Peter Clausen wrote:
>> The scale attribute reports the scale of the raw value. To gain the
>> value of
>> the result in mV you have to multiply the raw attribute with the scale
>> attribute. The scale attribute does not return the result of a
>> conversion with
>> a scale different applied.
> 
> Thank you for the guidance, I misunderstood the whole thing.
> 
>> I suggest you implement this in two steps. First implement the scale
>> property
>> for all channels. Use a LUT for each channel's fullscale value (in mV)
>> and
>> implement reading the scale like this.
>>
>> case IIO_CHAN_INFO_SCALE:
>> *val = lut[chan->channel];
>> *val2 = chan->scan_type.realbits;
>> return IIO_VAL_FRACTIONAL_LOG2;
>>
>> The next step is to make the scale modifiable for channels which
>> support this.
> 
> I'm sorry, it's the first time I work with an iio driver and I'm afraid
> I don't understand this.
> Let's say the fullscale value for channel 0 is 1.85V by default (when
> divide_by_two is disabled) and 3.7V (when enabled).
> 
> The channel's realbits is currently set to 18, because (although the ADC
> resolution is 12 bits) the device can read several samples and return an
> accumulated value in 18 bits.
> 
> Now, considering the default case where 1.85V is the max ref voltage and
> considering that the driver only reads one sample during the raw_read
> operation (12 bits), what value should a read of 'scale' return?
> 
>     scale = Vref_mv / ((1 << 12) - 1) = 1850 /4095 = 0.45177045 ?
> 

That's right. Although, I would think that all the values are on 18bits,
so you probably want to use chan->scan_type.realbits there.

Using IIO_VAL_FRACTIONAL_LOG2 allows you to avoid doing that computation
in your driver (sometimes, it is actually difficult to get it right
because you have to use 64 bits numbers).

Also, it simplifies things a bit, you can then use something like:

case IIO_CHAN_INFO_SCALE:
*val = lut[chan->channel];
*val2 = chan->scan_type.realbits - is_divide_by_two;
return IIO_VAL_FRACTIONAL_LOG2;


> That is, the value to multiply the raw value to get the voltage in mV?
> So that if reading the 'raw' returns say 425 and reading scale returns
> 0.45177 the measured voltage is 425 * 0.45177 = 192mV?
> 

Right.

> 
> Then let's say I have implemented a function to modify the scale (which
> should eventually allow me to enable/disable the by two divisor). What
> is the user supposed to write to the 'scale'? Is he supposed to supply a
> decimal number like 0.45177 * 2 = 0.903? This would be pretty odd.
> Should I create a table instead so that writing a 0 means divisor
> disabled and writing 1 means divisor enabled, for example?
> 

That is why you should provide an in_voltageX_scale_available file,
providing a list of valid values.

> Does the iio provide any descriptor that automatically shows the value
> in mV (already converted)?
> 

You can use IIO_CHAN_INFO_PROCESSED to expose that information.




-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH RFC] iio: mxs-lradc: add scaling to enable divide_by_two operation

[Lars-Peter Clausen]

On 07/03/2013 06:54 PM, Alexandre Belloni wrote:
[...]
>> Does the iio provide any descriptor that automatically shows the value
>> in mV (already converted)?
>>
> 
> You can use IIO_CHAN_INFO_PROCESSED to expose that information.

But in this case you really shouldn't. IIO_CHAN_INFO_PROCESSED is for cases
where a non trivial formula is needed to calculate the real value from the raw
value.

- Lars

Re: [PATCH RFC] iio: mxs-lradc: add scaling to enable divide_by_two operation

[Hector Palacios]

Dear Alexandre,

On 07/03/2013 06:54 PM, Alexandre Belloni wrote:
> On 03/07/2013 18:39, Hector Palacios wrote:
>> Dear Lars,
>>
>> On 07/03/2013 12:46 PM, Lars-Peter Clausen wrote:
>>> The scale attribute reports the scale of the raw value. To gain the
>>> value of
>>> the result in mV you have to multiply the raw attribute with the scale
>>> attribute. The scale attribute does not return the result of a
>>> conversion with
>>> a scale different applied.
>>
>> Thank you for the guidance, I misunderstood the whole thing.
>>
>>> I suggest you implement this in two steps. First implement the scale
>>> property
>>> for all channels. Use a LUT for each channel's fullscale value (in mV)
>>> and
>>> implement reading the scale like this.
>>>
>>> case IIO_CHAN_INFO_SCALE:
>>> *val = lut[chan->channel];
>>> *val2 = chan->scan_type.realbits;
>>> return IIO_VAL_FRACTIONAL_LOG2;
>>>
>>> The next step is to make the scale modifiable for channels which
>>> support this.
>>
>> I'm sorry, it's the first time I work with an iio driver and I'm afraid
>> I don't understand this.
>> Let's say the fullscale value for channel 0 is 1.85V by default (when
>> divide_by_two is disabled) and 3.7V (when enabled).
>>
>> The channel's realbits is currently set to 18, because (although the ADC
>> resolution is 12 bits) the device can read several samples and return an
>> accumulated value in 18 bits.
>>
>> Now, considering the default case where 1.85V is the max ref voltage and
>> considering that the driver only reads one sample during the raw_read
>> operation (12 bits), what value should a read of 'scale' return?
>>
>>      scale = Vref_mv / ((1 << 12) - 1) = 1850 /4095 = 0.45177045 ?
>>
>
> That's right. Although, I would think that all the values are on 18bits,
> so you probably want to use chan->scan_type.realbits there.

The field of the register that stores the sample is 18 bits, but each sample is only 
12 bits. The read_raw of a given channel will only read one sample and thus returns a 
value between 0 and 4095.
If I use realbits here it will divide the Vref_mv by (1 << 18) and will give me a 
wrong scale value.
Either we change the 'realbits' to 12 which is the real resolution of the ADC, or we 
leave it as 18 and use a 12 in this operation.

> Using IIO_VAL_FRACTIONAL_LOG2 allows you to avoid doing that computation
> in your driver (sometimes, it is actually difficult to get it right
> because you have to use 64 bits numbers).
>
> Also, it simplifies things a bit, you can then use something like:
>
> case IIO_CHAN_INFO_SCALE:
> *val = lut[chan->channel];
> *val2 = chan->scan_type.realbits - is_divide_by_two;
> return IIO_VAL_FRACTIONAL_LOG2;

Yes, but not all channels have a by two divisor. Some have a by 4, so I'll need to 
work out a formula per channel divisor.
When using IIO_VAL_FRACTIONAL_LOG2, val must be given in mV?

>> That is, the value to multiply the raw value to get the voltage in mV?
>> So that if reading the 'raw' returns say 425 and reading scale returns
>> 0.45177 the measured voltage is 425 * 0.45177 = 192mV?
>>
>
> Right.
>
>>
>> Then let's say I have implemented a function to modify the scale (which
>> should eventually allow me to enable/disable the by two divisor). What
>> is the user supposed to write to the 'scale'? Is he supposed to supply a
>> decimal number like 0.45177 * 2 = 0.903? This would be pretty odd.
>> Should I create a table instead so that writing a 0 means divisor
>> disabled and writing 1 means divisor enabled, for example?
>>
>
> That is why you should provide an in_voltageX_scale_available file,
> providing a list of valid values.

I see. Thank you very much.

Best regards,
--
Hector Palacios

Re: [PATCH RFC] iio: mxs-lradc: add scaling to enable divide_by_two operation

[Lars-Peter Clausen]

On 07/04/2013 10:58 AM, Hector Palacios wrote:
> Dear Alexandre,
> 
> On 07/03/2013 06:54 PM, Alexandre Belloni wrote:
>> On 03/07/2013 18:39, Hector Palacios wrote:
>>> Dear Lars,
>>>
>>> On 07/03/2013 12:46 PM, Lars-Peter Clausen wrote:
>>>> The scale attribute reports the scale of the raw value. To gain the
>>>> value of
>>>> the result in mV you have to multiply the raw attribute with the scale
>>>> attribute. The scale attribute does not return the result of a
>>>> conversion with
>>>> a scale different applied.
>>>
>>> Thank you for the guidance, I misunderstood the whole thing.
>>>
>>>> I suggest you implement this in two steps. First implement the scale
>>>> property
>>>> for all channels. Use a LUT for each channel's fullscale value (in mV)
>>>> and
>>>> implement reading the scale like this.
>>>>
>>>> case IIO_CHAN_INFO_SCALE:
>>>> *val = lut[chan->channel];
>>>> *val2 = chan->scan_type.realbits;
>>>> return IIO_VAL_FRACTIONAL_LOG2;
>>>>
>>>> The next step is to make the scale modifiable for channels which
>>>> support this.
>>>
>>> I'm sorry, it's the first time I work with an iio driver and I'm afraid
>>> I don't understand this.
>>> Let's say the fullscale value for channel 0 is 1.85V by default (when
>>> divide_by_two is disabled) and 3.7V (when enabled).
>>>
>>> The channel's realbits is currently set to 18, because (although the ADC
>>> resolution is 12 bits) the device can read several samples and return an
>>> accumulated value in 18 bits.
>>>
>>> Now, considering the default case where 1.85V is the max ref voltage and
>>> considering that the driver only reads one sample during the raw_read
>>> operation (12 bits), what value should a read of 'scale' return?
>>>
>>>      scale = Vref_mv / ((1 << 12) - 1) = 1850 /4095 = 0.45177045 ?
>>>
>>
>> That's right. Although, I would think that all the values are on 18bits,
>> so you probably want to use chan->scan_type.realbits there.
> 
> The field of the register that stores the sample is 18 bits, but each sample
> is only 12 bits. The read_raw of a given channel will only read one sample
> and thus returns a value between 0 and 4095.
> If I use realbits here it will divide the Vref_mv by (1 << 18) and will give
> me a wrong scale value.
> Either we change the 'realbits' to 12 which is the real resolution of the
> ADC, or we leave it as 18 and use a 12 in this operation.

I'd change realbits to 12.

> 
>> Using IIO_VAL_FRACTIONAL_LOG2 allows you to avoid doing that computation
>> in your driver (sometimes, it is actually difficult to get it right
>> because you have to use 64 bits numbers).
>>
>> Also, it simplifies things a bit, you can then use something like:
>>
>> case IIO_CHAN_INFO_SCALE:
>> *val = lut[chan->channel];
>> *val2 = chan->scan_type.realbits - is_divide_by_two;
>> return IIO_VAL_FRACTIONAL_LOG2;
> 
> Yes, but not all channels have a by two divisor. Some have a by 4, so I'll
> need to work out a formula per channel divisor.

The by 4 divider is a fixed divider though. That's why you use a lookup
table for the default scale. E.g. for channels with a fixed by 4 divider the
lut would contain 1850 * 4, for channels with a fixed by 2 divider 1850 * 2
and so on.

> When using IIO_VAL_FRACTIONAL_LOG2, val must be given in mV?

Yes

> 
>>> That is, the value to multiply the raw value to get the voltage in mV?
>>> So that if reading the 'raw' returns say 425 and reading scale returns
>>> 0.45177 the measured voltage is 425 * 0.45177 = 192mV?
>>>
>>
>> Right.
>>
>>>
>>> Then let's say I have implemented a function to modify the scale (which
>>> should eventually allow me to enable/disable the by two divisor). What
>>> is the user supposed to write to the 'scale'? Is he supposed to supply a
>>> decimal number like 0.45177 * 2 = 0.903? This would be pretty odd.
>>> Should I create a table instead so that writing a 0 means divisor
>>> disabled and writing 1 means divisor enabled, for example?
>>>
>>
>> That is why you should provide an in_voltageX_scale_available file,
>> providing a list of valid values.
> 
> I see. Thank you very much.
> 
> Best regards,
> -- 
> Hector Palacios

Re: [PATCH RFC] iio: mxs-lradc: add scaling to enable divide_by_two operation

[Alexandre Belloni]

Hi,

On 04/07/2013 10:58, Hector Palacios wrote:
> 
> The field of the register that stores the sample is 18 bits, but each
> sample is only 12 bits. The read_raw of a given channel will only read
> one sample and thus returns a value between 0 and 4095.
> If I use realbits here it will divide the Vref_mv by (1 << 18) and will
> give me a wrong scale value.
> Either we change the 'realbits' to 12 which is the real resolution of
> the ADC, or we leave it as 18 and use a 12 in this operation.
>

As said I didn't have a deep look at it so you are probably right.

>> Using IIO_VAL_FRACTIONAL_LOG2 allows you to avoid doing that computation
>> in your driver (sometimes, it is actually difficult to get it right
>> because you have to use 64 bits numbers).
>>
>> Also, it simplifies things a bit, you can then use something like:
>>
>> case IIO_CHAN_INFO_SCALE:
>> *val = lut[chan->channel];
>> *val2 = chan->scan_type.realbits - is_divide_by_two;
>> return IIO_VAL_FRACTIONAL_LOG2;
> 
> Yes, but not all channels have a by two divisor. Some have a by 4, so
> I'll need to work out a formula per channel divisor.
> When using IIO_VAL_FRACTIONAL_LOG2, val must be given in mV?
>

val is in mV indeed.

My first go at it was something along the lines:

*val = 1850;
*val2 = 12 - divider[chan->channel] - is_divide_by_two;

But as Lars suggested you could also have something like:

*val = Vmax[chan->channel];
*val2 = 12 - is_divide_by_two;



-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH RFC] iio: mxs-lradc: add scaling to enable divide_by_two operation

[Hector Palacios]

Dear Alexandre,

On 07/03/2013 06:54 PM, Alexandre Belloni wrote:
>> Then let's say I have implemented a function to modify the scale (which
>> should eventually allow me to enable/disable the by two divisor). What
>> is the user supposed to write to the 'scale'? Is he supposed to supply a
>> decimal number like 0.45177 * 2 = 0.903? This would be pretty odd.
>> Should I create a table instead so that writing a 0 means divisor
>> disabled and writing 1 means divisor enabled, for example?
>>
>
> That is why you should provide an in_voltageX_scale_available file,
> providing a list of valid values.

Getting there...
So since all channels may have the optional divider by two I need to have two 
available scales per channel, for example:

	int	scale_avail[LRADC_MAX_TOTAL_CHANS][2][2];

where the first [2] is the number of available scales per channel (with divisor 
disabled/enabled) and the second [2] is for storing the integer and nanoV parts.

Then I must populate each channel with the two available scales:

	[0] = 0.451660156  	-> [0][0] = 0
				-> [0][1] = 451660156

	[1] = 0.903320312	-> [1][0] = 0
				-> [1][1] = 903320312

where the [0] value is calculated by dividing the Vref by the realbits and the [1] 
value is calculated by multiplying the [0] by two.

Then in the write_raw function I must check whether the integer and nano components 
(val, val2) match those of the array, and if they match the one at the component [1], 
then I can finally set the divisor flag on the register, or else clear it. Is this 
correct?

Best regards,
--
Hector Palacios