[PATCH V3 0/2] iio: adc: spmi-vadc: Changes to support different scaling

[PATCH V3 0/2] iio: adc: spmi-vadc: Changes to support different scaling

[Rama Krishna Phani A]

VADC is a 15 bit ADC that measures signals through the Main analog
multiplexer (AMUX). The driver arbitrates the request to issue ADC
read requests. These patches adds different scaling functionality
to convert adc code to physical units.

[V3]
	1. Update the commit text to give more information on the
	   functionality implemented in the patches.
	2. Added Dt maintainers.

[V2]
	1. Replaced the do_div with normal division.
	2. Split the functionality implemented in v1 patch.
	3. Abstracted out common logic and updated change to report
	   Raw adc code properly.
	4. Change to support different scaling functionality.
	5. Added support for introducing new bindings in driver.

[V1]
	https://lkml.org/lkml/2016/10/25/26

Rama Krishna Phani A (2):
  iio: adc: spmi-vadc: Update changes to support reporting of Raw adc
    code.
  iio: adc: spmi-vadc: Changes to support different scaling

 .../devicetree/bindings/iio/adc/qcom,spmi-vadc.txt |  14 +
 drivers/iio/adc/qcom-spmi-vadc.c                   | 311 ++++++++++++++++-----
 2 files changed, 261 insertions(+), 64 deletions(-)

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH V3 0/2] iio: adc: spmi-vadc: Changes to support different scaling

[Rama Krishna Phani A]

VADC is a 15 bit ADC that measures signals through the Main analog
multiplexer (AMUX). The driver arbitrates the request to issue ADC
read requests. These patches adds different scaling functionality
to convert adc code to physical units.

[V3]
	1. Update the commit text to give more information on the
	   functionality implemented in the patches.
	2. Added Dt maintainers.

[V2]
	1. Replaced the do_div with normal division.
	2. Split the functionality implemented in v1 patch.
	3. Abstracted out common logic and updated change to report
	   Raw adc code properly.
	4. Change to support different scaling functionality.
	5. Added support for introducing new bindings in driver.

[V1]
	https://lkml.org/lkml/2016/10/25/26

Rama Krishna Phani A (2):
  iio: adc: spmi-vadc: Update changes to support reporting of Raw adc
    code.
  iio: adc: spmi-vadc: Changes to support different scaling

 .../devicetree/bindings/iio/adc/qcom,spmi-vadc.txt |  14 +
 drivers/iio/adc/qcom-spmi-vadc.c                   | 311 ++++++++++++++++-----
 2 files changed, 261 insertions(+), 64 deletions(-)

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH V3 1/2] iio: adc: spmi-vadc: Update changes to support reporting of Raw adc code.

[Rama Krishna Phani A]

Logic to convert adc code to voltage is generic across all channels.
Different scaling can be done on the obtained voltage to report in physical
units. Implement separate function for generic conversion logic.
Scaling functionality can be changed per channel. Update changes to support
reporting of Raw adc code.

Signed-off-by: Rama Krishna Phani A <rphani-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
---
 drivers/iio/adc/qcom-spmi-vadc.c | 54 +++++++++++++++++++++-------------------
 1 file changed, 28 insertions(+), 26 deletions(-)

diff --git a/drivers/iio/adc/qcom-spmi-vadc.c b/drivers/iio/adc/qcom-spmi-vadc.c
index c2babe5..ff4d549 100644
--- a/drivers/iio/adc/qcom-spmi-vadc.c
+++ b/drivers/iio/adc/qcom-spmi-vadc.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012-2014, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2012-2016, The Linux Foundation. All rights reserved.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2 and
@@ -84,7 +84,7 @@
 #define VADC_MAX_ADC_CODE			0xa800
 
 #define VADC_ABSOLUTE_RANGE_UV			625000
-#define VADC_RATIOMETRIC_RANGE_UV		1800000
+#define VADC_RATIOMETRIC_RANGE			1800
 
 #define VADC_DEF_PRESCALING			0 /* 1:1 */
 #define VADC_DEF_DECIMATION			0 /* 512 */
@@ -418,7 +418,7 @@ static int vadc_measure_ref_points(struct vadc_priv *vadc)
 	u16 read_1, read_2;
 	int ret;
 
-	vadc->graph[VADC_CALIB_RATIOMETRIC].dx = VADC_RATIOMETRIC_RANGE_UV;
+	vadc->graph[VADC_CALIB_RATIOMETRIC].dx = VADC_RATIOMETRIC_RANGE;
 	vadc->graph[VADC_CALIB_ABSOLUTE].dx = VADC_ABSOLUTE_RANGE_UV;
 
 	prop = vadc_get_channel(vadc, VADC_REF_1250MV);
@@ -468,21 +468,30 @@ static int vadc_measure_ref_points(struct vadc_priv *vadc)
 	return ret;
 }
 
-static s32 vadc_calibrate(struct vadc_priv *vadc,
-			  const struct vadc_channel_prop *prop, u16 adc_code)
+static void vadc_scale_calib(struct vadc_priv *vadc, u16 adc_code,
+			     const struct vadc_channel_prop *prop,
+			     s64 *scale_voltage)
 {
-	const struct vadc_prescale_ratio *prescale;
-	s64 voltage;
+	*scale_voltage = (adc_code -
+		vadc->graph[prop->calibration].gnd);
+	*scale_voltage *= vadc->graph[prop->calibration].dx;
+	*scale_voltage = div64_s64(*scale_voltage,
+		vadc->graph[prop->calibration].dy);
+	if (prop->calibration == VADC_CALIB_ABSOLUTE)
+		*scale_voltage +=
+		vadc->graph[prop->calibration].dx;
 
-	voltage = adc_code - vadc->graph[prop->calibration].gnd;
-	voltage *= vadc->graph[prop->calibration].dx;
-	voltage = div64_s64(voltage, vadc->graph[prop->calibration].dy);
+	if (*scale_voltage < 0)
+		*scale_voltage = 0;
+}
 
-	if (prop->calibration == VADC_CALIB_ABSOLUTE)
-		voltage += vadc->graph[prop->calibration].dx;
+static s64 vadc_scale_fn(struct vadc_priv *vadc,
+			 const struct vadc_channel_prop *prop, u16 adc_code)
+{
+	const struct vadc_prescale_ratio *prescale;
+	s64 voltage = 0;
 
-	if (voltage < 0)
-		voltage = 0;
+	vadc_scale_calib(vadc, adc_code, prop, &voltage);
 
 	prescale = &vadc_prescale_ratios[prop->prescale];
 
@@ -552,11 +561,8 @@ static int vadc_read_raw(struct iio_dev *indio_dev,
 		if (ret)
 			break;
 
-		*val = vadc_calibrate(vadc, prop, adc_code);
+		*val = vadc_scale_fn(vadc, prop, adc_code);
 
-		/* 2mV/K, return milli Celsius */
-		*val /= 2;
-		*val -= KELVINMIL_CELSIUSMIL;
 		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_RAW:
 		prop = &vadc->chan_props[chan->address];
@@ -564,12 +570,8 @@ static int vadc_read_raw(struct iio_dev *indio_dev,
 		if (ret)
 			break;
 
-		*val = vadc_calibrate(vadc, prop, adc_code);
+		*val = (int)adc_code;
 		return IIO_VAL_INT;
-	case IIO_CHAN_INFO_SCALE:
-		*val = 0;
-		*val2 = 1000;
-		return IIO_VAL_INT_PLUS_MICRO;
 	default:
 		ret = -EINVAL;
 		break;
@@ -616,8 +618,8 @@ struct vadc_channels {
 	VADC_CHAN(_dname, IIO_TEMP, BIT(IIO_CHAN_INFO_PROCESSED), _pre)	\
 
 #define VADC_CHAN_VOLT(_dname, _pre)					\
-	VADC_CHAN(_dname, IIO_VOLTAGE,					\
-		  BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),	\
+	VADC_CHAN(_dname, IIO_VOLTAGE,				\
+		  BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_PROCESSED),\
 		  _pre)							\
 
 /*
@@ -850,9 +852,9 @@ static int vadc_get_dt_data(struct vadc_priv *vadc, struct device_node *node)
 
 		iio_chan->channel = prop.channel;
 		iio_chan->datasheet_name = vadc_chan->datasheet_name;
+		iio_chan->extend_name = child->name;
 		iio_chan->info_mask_separate = vadc_chan->info_mask;
 		iio_chan->type = vadc_chan->type;
-		iio_chan->indexed = 1;
 		iio_chan->address = index++;
 
 		iio_chan++;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V3 1/2] iio: adc: spmi-vadc: Update changes to support reporting of Raw adc code.

[Rama Krishna Phani A]

Logic to convert adc code to voltage is generic across all channels.
Different scaling can be done on the obtained voltage to report in physical
units. Implement separate function for generic conversion logic.
Scaling functionality can be changed per channel. Update changes to support
reporting of Raw adc code.

Signed-off-by: Rama Krishna Phani A <rphani@codeaurora.org>
---
 drivers/iio/adc/qcom-spmi-vadc.c | 54 +++++++++++++++++++++-------------------
 1 file changed, 28 insertions(+), 26 deletions(-)

diff --git a/drivers/iio/adc/qcom-spmi-vadc.c b/drivers/iio/adc/qcom-spmi-vadc.c
index c2babe5..ff4d549 100644
--- a/drivers/iio/adc/qcom-spmi-vadc.c
+++ b/drivers/iio/adc/qcom-spmi-vadc.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012-2014, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2012-2016, The Linux Foundation. All rights reserved.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2 and
@@ -84,7 +84,7 @@
 #define VADC_MAX_ADC_CODE			0xa800
 
 #define VADC_ABSOLUTE_RANGE_UV			625000
-#define VADC_RATIOMETRIC_RANGE_UV		1800000
+#define VADC_RATIOMETRIC_RANGE			1800
 
 #define VADC_DEF_PRESCALING			0 /* 1:1 */
 #define VADC_DEF_DECIMATION			0 /* 512 */
@@ -418,7 +418,7 @@ static int vadc_measure_ref_points(struct vadc_priv *vadc)
 	u16 read_1, read_2;
 	int ret;
 
-	vadc->graph[VADC_CALIB_RATIOMETRIC].dx = VADC_RATIOMETRIC_RANGE_UV;
+	vadc->graph[VADC_CALIB_RATIOMETRIC].dx = VADC_RATIOMETRIC_RANGE;
 	vadc->graph[VADC_CALIB_ABSOLUTE].dx = VADC_ABSOLUTE_RANGE_UV;
 
 	prop = vadc_get_channel(vadc, VADC_REF_1250MV);
@@ -468,21 +468,30 @@ static int vadc_measure_ref_points(struct vadc_priv *vadc)
 	return ret;
 }
 
-static s32 vadc_calibrate(struct vadc_priv *vadc,
-			  const struct vadc_channel_prop *prop, u16 adc_code)
+static void vadc_scale_calib(struct vadc_priv *vadc, u16 adc_code,
+			     const struct vadc_channel_prop *prop,
+			     s64 *scale_voltage)
 {
-	const struct vadc_prescale_ratio *prescale;
-	s64 voltage;
+	*scale_voltage = (adc_code -
+		vadc->graph[prop->calibration].gnd);
+	*scale_voltage *= vadc->graph[prop->calibration].dx;
+	*scale_voltage = div64_s64(*scale_voltage,
+		vadc->graph[prop->calibration].dy);
+	if (prop->calibration == VADC_CALIB_ABSOLUTE)
+		*scale_voltage +=
+		vadc->graph[prop->calibration].dx;
 
-	voltage = adc_code - vadc->graph[prop->calibration].gnd;
-	voltage *= vadc->graph[prop->calibration].dx;
-	voltage = div64_s64(voltage, vadc->graph[prop->calibration].dy);
+	if (*scale_voltage < 0)
+		*scale_voltage = 0;
+}
 
-	if (prop->calibration == VADC_CALIB_ABSOLUTE)
-		voltage += vadc->graph[prop->calibration].dx;
+static s64 vadc_scale_fn(struct vadc_priv *vadc,
+			 const struct vadc_channel_prop *prop, u16 adc_code)
+{
+	const struct vadc_prescale_ratio *prescale;
+	s64 voltage = 0;
 
-	if (voltage < 0)
-		voltage = 0;
+	vadc_scale_calib(vadc, adc_code, prop, &voltage);
 
 	prescale = &vadc_prescale_ratios[prop->prescale];
 
@@ -552,11 +561,8 @@ static int vadc_read_raw(struct iio_dev *indio_dev,
 		if (ret)
 			break;
 
-		*val = vadc_calibrate(vadc, prop, adc_code);
+		*val = vadc_scale_fn(vadc, prop, adc_code);
 
-		/* 2mV/K, return milli Celsius */
-		*val /= 2;
-		*val -= KELVINMIL_CELSIUSMIL;
 		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_RAW:
 		prop = &vadc->chan_props[chan->address];
@@ -564,12 +570,8 @@ static int vadc_read_raw(struct iio_dev *indio_dev,
 		if (ret)
 			break;
 
-		*val = vadc_calibrate(vadc, prop, adc_code);
+		*val = (int)adc_code;
 		return IIO_VAL_INT;
-	case IIO_CHAN_INFO_SCALE:
-		*val = 0;
-		*val2 = 1000;
-		return IIO_VAL_INT_PLUS_MICRO;
 	default:
 		ret = -EINVAL;
 		break;
@@ -616,8 +618,8 @@ struct vadc_channels {
 	VADC_CHAN(_dname, IIO_TEMP, BIT(IIO_CHAN_INFO_PROCESSED), _pre)	\
 
 #define VADC_CHAN_VOLT(_dname, _pre)					\
-	VADC_CHAN(_dname, IIO_VOLTAGE,					\
-		  BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),	\
+	VADC_CHAN(_dname, IIO_VOLTAGE,				\
+		  BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_PROCESSED),\
 		  _pre)							\
 
 /*
@@ -850,9 +852,9 @@ static int vadc_get_dt_data(struct vadc_priv *vadc, struct device_node *node)
 
 		iio_chan->channel = prop.channel;
 		iio_chan->datasheet_name = vadc_chan->datasheet_name;
+		iio_chan->extend_name = child->name;
 		iio_chan->info_mask_separate = vadc_chan->info_mask;
 		iio_chan->type = vadc_chan->type;
-		iio_chan->indexed = 1;
 		iio_chan->address = index++;
 
 		iio_chan++;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH V3 1/2] iio: adc: spmi-vadc: Update changes to support reporting of Raw adc code.

[Jonathan Cameron]

On 26/10/16 15:41, Rama Krishna Phani A wrote:
> Logic to convert adc code to voltage is generic across all channels.
> Different scaling can be done on the obtained voltage to report in physical
> units. Implement separate function for generic conversion logic.
> Scaling functionality can be changed per channel. Update changes to support
> reporting of Raw adc code.
Pleas rewrite this description.  Perhaps give examples of the changes
it makes to what is read from the various attributes? 

I haven't immediately followed what this change is actually doing.

I 'think' the point here is to not apply the calibration to
the raw adc counts when a true raw read is requested?

There are several unconnected looking changes in here...
> 
> Signed-off-by: Rama Krishna Phani A <rphani@codeaurora.org>
> ---
>  drivers/iio/adc/qcom-spmi-vadc.c | 54 +++++++++++++++++++++-------------------
>  1 file changed, 28 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/iio/adc/qcom-spmi-vadc.c b/drivers/iio/adc/qcom-spmi-vadc.c
> index c2babe5..ff4d549 100644
> --- a/drivers/iio/adc/qcom-spmi-vadc.c
> +++ b/drivers/iio/adc/qcom-spmi-vadc.c
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (c) 2012-2014, The Linux Foundation. All rights reserved.
> + * Copyright (c) 2012-2016, The Linux Foundation. All rights reserved.
>   *
>   * This program is free software; you can redistribute it and/or modify
>   * it under the terms of the GNU General Public License version 2 and
> @@ -84,7 +84,7 @@
>  #define VADC_MAX_ADC_CODE			0xa800
>  
>  #define VADC_ABSOLUTE_RANGE_UV			625000
> -#define VADC_RATIOMETRIC_RANGE_UV		1800000
> +#define VADC_RATIOMETRIC_RANGE			1800
>  
>  #define VADC_DEF_PRESCALING			0 /* 1:1 */
>  #define VADC_DEF_DECIMATION			0 /* 512 */
> @@ -418,7 +418,7 @@ static int vadc_measure_ref_points(struct vadc_priv *vadc)
>  	u16 read_1, read_2;
>  	int ret;
>  
> -	vadc->graph[VADC_CALIB_RATIOMETRIC].dx = VADC_RATIOMETRIC_RANGE_UV;
> +	vadc->graph[VADC_CALIB_RATIOMETRIC].dx = VADC_RATIOMETRIC_RANGE;
>  	vadc->graph[VADC_CALIB_ABSOLUTE].dx = VADC_ABSOLUTE_RANGE_UV;
>  
>  	prop = vadc_get_channel(vadc, VADC_REF_1250MV);
> @@ -468,21 +468,30 @@ static int vadc_measure_ref_points(struct vadc_priv *vadc)
>  	return ret;
>  }
>  
> -static s32 vadc_calibrate(struct vadc_priv *vadc,
> -			  const struct vadc_channel_prop *prop, u16 adc_code)
> +static void vadc_scale_calib(struct vadc_priv *vadc, u16 adc_code,
> +			     const struct vadc_channel_prop *prop,
> +			     s64 *scale_voltage)
>  {
> -	const struct vadc_prescale_ratio *prescale;
> -	s64 voltage;
> +	*scale_voltage = (adc_code -
> +		vadc->graph[prop->calibration].gnd);
> +	*scale_voltage *= vadc->graph[prop->calibration].dx;
> +	*scale_voltage = div64_s64(*scale_voltage,
> +		vadc->graph[prop->calibration].dy);
> +	if (prop->calibration == VADC_CALIB_ABSOLUTE)
> +		*scale_voltage +=
> +		vadc->graph[prop->calibration].dx;
>  
> -	voltage = adc_code - vadc->graph[prop->calibration].gnd;
> -	voltage *= vadc->graph[prop->calibration].dx;
> -	voltage = div64_s64(voltage, vadc->graph[prop->calibration].dy);
> +	if (*scale_voltage < 0)
> +		*scale_voltage = 0;
> +}
>  
> -	if (prop->calibration == VADC_CALIB_ABSOLUTE)
> -		voltage += vadc->graph[prop->calibration].dx;
> +static s64 vadc_scale_fn(struct vadc_priv *vadc,
> +			 const struct vadc_channel_prop *prop, u16 adc_code)
> +{
> +	const struct vadc_prescale_ratio *prescale;
> +	s64 voltage = 0;
>  
> -	if (voltage < 0)
> -		voltage = 0;
> +	vadc_scale_calib(vadc, adc_code, prop, &voltage);
>  
>  	prescale = &vadc_prescale_ratios[prop->prescale];
>  
> @@ -552,11 +561,8 @@ static int vadc_read_raw(struct iio_dev *indio_dev,
>  		if (ret)
>  			break;
>  
> -		*val = vadc_calibrate(vadc, prop, adc_code);
> +		*val = vadc_scale_fn(vadc, prop, adc_code);
>  
> -		/* 2mV/K, return milli Celsius */
> -		*val /= 2;
> -		*val -= KELVINMIL_CELSIUSMIL;
>  		return IIO_VAL_INT;
>  	case IIO_CHAN_INFO_RAW:
>  		prop = &vadc->chan_props[chan->address];
> @@ -564,12 +570,8 @@ static int vadc_read_raw(struct iio_dev *indio_dev,
>  		if (ret)
>  			break;
>  
> -		*val = vadc_calibrate(vadc, prop, adc_code);
> +		*val = (int)adc_code;
>  		return IIO_VAL_INT;
So this is 'more raw'.
> -	case IIO_CHAN_INFO_SCALE:
> -		*val = 0;
> -		*val2 = 1000;
> -		return IIO_VAL_INT_PLUS_MICRO;
>  	default:
>  		ret = -EINVAL;
>  		break;
> @@ -616,8 +618,8 @@ struct vadc_channels {
>  	VADC_CHAN(_dname, IIO_TEMP, BIT(IIO_CHAN_INFO_PROCESSED), _pre)	\
>  
>  #define VADC_CHAN_VOLT(_dname, _pre)					\
> -	VADC_CHAN(_dname, IIO_VOLTAGE,					\
> -		  BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),	\
> +	VADC_CHAN(_dname, IIO_VOLTAGE,				\
> +		  BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_PROCESSED),\
>  		  _pre)							\
It very unusual to report both raw and processed values.  Please explain
why that is needed here?  It may be valid to maintain backwards compatibility
of ABI. Which would be fine. However if I read the above correctly you are
changing what comes out of reading the raw value so the ABI just changed...


>  
>  /*
> @@ -850,9 +852,9 @@ static int vadc_get_dt_data(struct vadc_priv *vadc, struct device_node *node)
>  
>  		iio_chan->channel = prop.channel;
>  		iio_chan->datasheet_name = vadc_chan->datasheet_name;
> +		iio_chan->extend_name = child->name;
What's this change?
>  		iio_chan->info_mask_separate = vadc_chan->info_mask;
>  		iio_chan->type = vadc_chan->type;
> -		iio_chan->indexed = 1;
Or for that matter this one...
>  		iio_chan->address = index++;
>  
>  		iio_chan++;
> 

Re: [PATCH V3 1/2] iio: adc: spmi-vadc: Update changes to support reporting of Raw adc code.

[Rama Krishna Phani A]

Hi Jonathan,

On 30-Oct-16 11:05 PM, Jonathan Cameron wrote:
> On 26/10/16 15:41, Rama Krishna Phani A wrote:
>> Logic to convert adc code to voltage is generic across all channels.
>> Different scaling can be done on the obtained voltage to report in physical
>> units. Implement separate function for generic conversion logic.
>> Scaling functionality can be changed per channel. Update changes to support
>> reporting of Raw adc code.
> Pleas rewrite this description.  Perhaps give examples of the changes
> it makes to what is read from the various attributes?
There are several channels in the ADC of PMIC which can be used to 
measure voltage, temperature, current etc., Hardware provides readings 
for all channels in adc code. That adc code needs to be converted to 
voltage. The logic for conversion of adc code to voltage is common for 
all ADC channels(voltage, temperature and current .,etc). Once voltage 
is obtained ., scaling is done on that voltage.

For Ex., Thermal SW wants to know the temperature of thermistor on PMIC 
and it expects the temperature to be reported in millidegC. ADC channel 
is used to read the adc code and convert it to voltage. Once the voltage 
is available based on the thermistor spec that voltage is mapped to a 
temperature and then that value is reported to Thermal SW.

Mapping of voltage to temperature is called scaling for that channel and 
scaling function can be different per channel based on how the voltage 
is reported.

>
> I haven't immediately followed what this change is actually doing.
>
> I 'think' the point here is to not apply the calibration to
> the raw adc counts when a true raw read is requested?
>
When a true raw read is requested .,Scaling is not applied.
> There are several unconnected looking changes in here...
>>
>> Signed-off-by: Rama Krishna Phani A <rphani-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>> ---
>>  drivers/iio/adc/qcom-spmi-vadc.c | 54 +++++++++++++++++++++-------------------
>>  1 file changed, 28 insertions(+), 26 deletions(-)
>>
>> diff --git a/drivers/iio/adc/qcom-spmi-vadc.c b/drivers/iio/adc/qcom-spmi-vadc.c
>> index c2babe5..ff4d549 100644
>> --- a/drivers/iio/adc/qcom-spmi-vadc.c
>> +++ b/drivers/iio/adc/qcom-spmi-vadc.c
>> @@ -1,5 +1,5 @@
>>  /*
>> - * Copyright (c) 2012-2014, The Linux Foundation. All rights reserved.
>> + * Copyright (c) 2012-2016, The Linux Foundation. All rights reserved.
>>   *
>>   * This program is free software; you can redistribute it and/or modify
>>   * it under the terms of the GNU General Public License version 2 and
>> @@ -84,7 +84,7 @@
>>  #define VADC_MAX_ADC_CODE			0xa800
>>
>>  #define VADC_ABSOLUTE_RANGE_UV			625000
>> -#define VADC_RATIOMETRIC_RANGE_UV		1800000
>> +#define VADC_RATIOMETRIC_RANGE			1800
>>
>>  #define VADC_DEF_PRESCALING			0 /* 1:1 */
>>  #define VADC_DEF_DECIMATION			0 /* 512 */
>> @@ -418,7 +418,7 @@ static int vadc_measure_ref_points(struct vadc_priv *vadc)
>>  	u16 read_1, read_2;
>>  	int ret;
>>
>> -	vadc->graph[VADC_CALIB_RATIOMETRIC].dx = VADC_RATIOMETRIC_RANGE_UV;
>> +	vadc->graph[VADC_CALIB_RATIOMETRIC].dx = VADC_RATIOMETRIC_RANGE;
>>  	vadc->graph[VADC_CALIB_ABSOLUTE].dx = VADC_ABSOLUTE_RANGE_UV;
>>
>>  	prop = vadc_get_channel(vadc, VADC_REF_1250MV);
>> @@ -468,21 +468,30 @@ static int vadc_measure_ref_points(struct vadc_priv *vadc)
>>  	return ret;
>>  }
>>
>> -static s32 vadc_calibrate(struct vadc_priv *vadc,
>> -			  const struct vadc_channel_prop *prop, u16 adc_code)
>> +static void vadc_scale_calib(struct vadc_priv *vadc, u16 adc_code,
>> +			     const struct vadc_channel_prop *prop,
>> +			     s64 *scale_voltage)
>>  {
>> -	const struct vadc_prescale_ratio *prescale;
>> -	s64 voltage;
>> +	*scale_voltage = (adc_code -
>> +		vadc->graph[prop->calibration].gnd);
>> +	*scale_voltage *= vadc->graph[prop->calibration].dx;
>> +	*scale_voltage = div64_s64(*scale_voltage,
>> +		vadc->graph[prop->calibration].dy);
>> +	if (prop->calibration == VADC_CALIB_ABSOLUTE)
>> +		*scale_voltage +=
>> +		vadc->graph[prop->calibration].dx;
>>
>> -	voltage = adc_code - vadc->graph[prop->calibration].gnd;
>> -	voltage *= vadc->graph[prop->calibration].dx;
>> -	voltage = div64_s64(voltage, vadc->graph[prop->calibration].dy);
>> +	if (*scale_voltage < 0)
>> +		*scale_voltage = 0;
>> +}
>>
>> -	if (prop->calibration == VADC_CALIB_ABSOLUTE)
>> -		voltage += vadc->graph[prop->calibration].dx;
>> +static s64 vadc_scale_fn(struct vadc_priv *vadc,
>> +			 const struct vadc_channel_prop *prop, u16 adc_code)
>> +{
>> +	const struct vadc_prescale_ratio *prescale;
>> +	s64 voltage = 0;
>>
>> -	if (voltage < 0)
>> -		voltage = 0;
>> +	vadc_scale_calib(vadc, adc_code, prop, &voltage);
>>
>>  	prescale = &vadc_prescale_ratios[prop->prescale];
>>
>> @@ -552,11 +561,8 @@ static int vadc_read_raw(struct iio_dev *indio_dev,
>>  		if (ret)
>>  			break;
>>
>> -		*val = vadc_calibrate(vadc, prop, adc_code);
>> +		*val = vadc_scale_fn(vadc, prop, adc_code);
>>
>> -		/* 2mV/K, return milli Celsius */
>> -		*val /= 2;
>> -		*val -= KELVINMIL_CELSIUSMIL;
>>  		return IIO_VAL_INT;
>>  	case IIO_CHAN_INFO_RAW:
>>  		prop = &vadc->chan_props[chan->address];
>> @@ -564,12 +570,8 @@ static int vadc_read_raw(struct iio_dev *indio_dev,
>>  		if (ret)
>>  			break;
>>
>> -		*val = vadc_calibrate(vadc, prop, adc_code);
>> +		*val = (int)adc_code;
>>  		return IIO_VAL_INT;
> So this is 'more raw'.
Yes., its raw value.
>> -	case IIO_CHAN_INFO_SCALE:
>> -		*val = 0;
>> -		*val2 = 1000;
>> -		return IIO_VAL_INT_PLUS_MICRO;
>>  	default:
>>  		ret = -EINVAL;
>>  		break;
>> @@ -616,8 +618,8 @@ struct vadc_channels {
>>  	VADC_CHAN(_dname, IIO_TEMP, BIT(IIO_CHAN_INFO_PROCESSED), _pre)	\
>>
>>  #define VADC_CHAN_VOLT(_dname, _pre)					\
>> -	VADC_CHAN(_dname, IIO_VOLTAGE,					\
>> -		  BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),	\
>> +	VADC_CHAN(_dname, IIO_VOLTAGE,				\
>> +		  BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_PROCESSED),\
>>  		  _pre)							\
> It very unusual to report both raw and processed values.  Please explain
> why that is needed here?  It may be valid to maintain backwards compatibility
> of ABI. Which would be fine. However if I read the above correctly you are
> changing what comes out of reading the raw value so the ABI just changed...
>
With the help of IIO sysfs ., we can read the ADC channel readings 
either in RAW format or in processed format. There are two separate 
individual entries to read the ADC channel either in Raw format or in 
processed format. Most of the clients for ADC expect the readings in 
processed format.
>
>>
>>  /*
>> @@ -850,9 +852,9 @@ static int vadc_get_dt_data(struct vadc_priv *vadc, struct device_node *node)
>>
>>  		iio_chan->channel = prop.channel;
>>  		iio_chan->datasheet_name = vadc_chan->datasheet_name;
>> +		iio_chan->extend_name = child->name;
> What's this change?
We can choose how we want to display our adc channel entries in sysfs. 
Am using the child node name to be displayed as the sysfs entry rather 
than channel number for easy interpretation.

For ex: for vcoin(coin battery voltage channel.,) with this change it 
appears like below in iio adc sysfs

"in_voltage_vcoin_input"

>>  		iio_chan->info_mask_separate = vadc_chan->info_mask;
>>  		iio_chan->type = vadc_chan->type;
>> -		iio_chan->indexed = 1;
> Or for that matter this one...
reason explained above.
>>  		iio_chan->address = index++;
>>
>>  		iio_chan++;
>>
>

Thanks,
Ramakrishna

---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V3 1/2] iio: adc: spmi-vadc: Update changes to support reporting of Raw adc code.

[Rama Krishna Phani A]

Hi Jonathan,

On 30-Oct-16 11:05 PM, Jonathan Cameron wrote:
> On 26/10/16 15:41, Rama Krishna Phani A wrote:
>> Logic to convert adc code to voltage is generic across all channels.
>> Different scaling can be done on the obtained voltage to report in physical
>> units. Implement separate function for generic conversion logic.
>> Scaling functionality can be changed per channel. Update changes to support
>> reporting of Raw adc code.
> Pleas rewrite this description.  Perhaps give examples of the changes
> it makes to what is read from the various attributes?
There are several channels in the ADC of PMIC which can be used to 
measure voltage, temperature, current etc., Hardware provides readings 
for all channels in adc code. That adc code needs to be converted to 
voltage. The logic for conversion of adc code to voltage is common for 
all ADC channels(voltage, temperature and current .,etc). Once voltage 
is obtained ., scaling is done on that voltage.

For Ex., Thermal SW wants to know the temperature of thermistor on PMIC 
and it expects the temperature to be reported in millidegC. ADC channel 
is used to read the adc code and convert it to voltage. Once the voltage 
is available based on the thermistor spec that voltage is mapped to a 
temperature and then that value is reported to Thermal SW.

Mapping of voltage to temperature is called scaling for that channel and 
scaling function can be different per channel based on how the voltage 
is reported.

>
> I haven't immediately followed what this change is actually doing.
>
> I 'think' the point here is to not apply the calibration to
> the raw adc counts when a true raw read is requested?
>
When a true raw read is requested .,Scaling is not applied.
> There are several unconnected looking changes in here...
>>
>> Signed-off-by: Rama Krishna Phani A <rphani@codeaurora.org>
>> ---
>>  drivers/iio/adc/qcom-spmi-vadc.c | 54 +++++++++++++++++++++-------------------
>>  1 file changed, 28 insertions(+), 26 deletions(-)
>>
>> diff --git a/drivers/iio/adc/qcom-spmi-vadc.c b/drivers/iio/adc/qcom-spmi-vadc.c
>> index c2babe5..ff4d549 100644
>> --- a/drivers/iio/adc/qcom-spmi-vadc.c
>> +++ b/drivers/iio/adc/qcom-spmi-vadc.c
>> @@ -1,5 +1,5 @@
>>  /*
>> - * Copyright (c) 2012-2014, The Linux Foundation. All rights reserved.
>> + * Copyright (c) 2012-2016, The Linux Foundation. All rights reserved.
>>   *
>>   * This program is free software; you can redistribute it and/or modify
>>   * it under the terms of the GNU General Public License version 2 and
>> @@ -84,7 +84,7 @@
>>  #define VADC_MAX_ADC_CODE			0xa800
>>
>>  #define VADC_ABSOLUTE_RANGE_UV			625000
>> -#define VADC_RATIOMETRIC_RANGE_UV		1800000
>> +#define VADC_RATIOMETRIC_RANGE			1800
>>
>>  #define VADC_DEF_PRESCALING			0 /* 1:1 */
>>  #define VADC_DEF_DECIMATION			0 /* 512 */
>> @@ -418,7 +418,7 @@ static int vadc_measure_ref_points(struct vadc_priv *vadc)
>>  	u16 read_1, read_2;
>>  	int ret;
>>
>> -	vadc->graph[VADC_CALIB_RATIOMETRIC].dx = VADC_RATIOMETRIC_RANGE_UV;
>> +	vadc->graph[VADC_CALIB_RATIOMETRIC].dx = VADC_RATIOMETRIC_RANGE;
>>  	vadc->graph[VADC_CALIB_ABSOLUTE].dx = VADC_ABSOLUTE_RANGE_UV;
>>
>>  	prop = vadc_get_channel(vadc, VADC_REF_1250MV);
>> @@ -468,21 +468,30 @@ static int vadc_measure_ref_points(struct vadc_priv *vadc)
>>  	return ret;
>>  }
>>
>> -static s32 vadc_calibrate(struct vadc_priv *vadc,
>> -			  const struct vadc_channel_prop *prop, u16 adc_code)
>> +static void vadc_scale_calib(struct vadc_priv *vadc, u16 adc_code,
>> +			     const struct vadc_channel_prop *prop,
>> +			     s64 *scale_voltage)
>>  {
>> -	const struct vadc_prescale_ratio *prescale;
>> -	s64 voltage;
>> +	*scale_voltage = (adc_code -
>> +		vadc->graph[prop->calibration].gnd);
>> +	*scale_voltage *= vadc->graph[prop->calibration].dx;
>> +	*scale_voltage = div64_s64(*scale_voltage,
>> +		vadc->graph[prop->calibration].dy);
>> +	if (prop->calibration == VADC_CALIB_ABSOLUTE)
>> +		*scale_voltage +=
>> +		vadc->graph[prop->calibration].dx;
>>
>> -	voltage = adc_code - vadc->graph[prop->calibration].gnd;
>> -	voltage *= vadc->graph[prop->calibration].dx;
>> -	voltage = div64_s64(voltage, vadc->graph[prop->calibration].dy);
>> +	if (*scale_voltage < 0)
>> +		*scale_voltage = 0;
>> +}
>>
>> -	if (prop->calibration == VADC_CALIB_ABSOLUTE)
>> -		voltage += vadc->graph[prop->calibration].dx;
>> +static s64 vadc_scale_fn(struct vadc_priv *vadc,
>> +			 const struct vadc_channel_prop *prop, u16 adc_code)
>> +{
>> +	const struct vadc_prescale_ratio *prescale;
>> +	s64 voltage = 0;
>>
>> -	if (voltage < 0)
>> -		voltage = 0;
>> +	vadc_scale_calib(vadc, adc_code, prop, &voltage);
>>
>>  	prescale = &vadc_prescale_ratios[prop->prescale];
>>
>> @@ -552,11 +561,8 @@ static int vadc_read_raw(struct iio_dev *indio_dev,
>>  		if (ret)
>>  			break;
>>
>> -		*val = vadc_calibrate(vadc, prop, adc_code);
>> +		*val = vadc_scale_fn(vadc, prop, adc_code);
>>
>> -		/* 2mV/K, return milli Celsius */
>> -		*val /= 2;
>> -		*val -= KELVINMIL_CELSIUSMIL;
>>  		return IIO_VAL_INT;
>>  	case IIO_CHAN_INFO_RAW:
>>  		prop = &vadc->chan_props[chan->address];
>> @@ -564,12 +570,8 @@ static int vadc_read_raw(struct iio_dev *indio_dev,
>>  		if (ret)
>>  			break;
>>
>> -		*val = vadc_calibrate(vadc, prop, adc_code);
>> +		*val = (int)adc_code;
>>  		return IIO_VAL_INT;
> So this is 'more raw'.
Yes., its raw value.
>> -	case IIO_CHAN_INFO_SCALE:
>> -		*val = 0;
>> -		*val2 = 1000;
>> -		return IIO_VAL_INT_PLUS_MICRO;
>>  	default:
>>  		ret = -EINVAL;
>>  		break;
>> @@ -616,8 +618,8 @@ struct vadc_channels {
>>  	VADC_CHAN(_dname, IIO_TEMP, BIT(IIO_CHAN_INFO_PROCESSED), _pre)	\
>>
>>  #define VADC_CHAN_VOLT(_dname, _pre)					\
>> -	VADC_CHAN(_dname, IIO_VOLTAGE,					\
>> -		  BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),	\
>> +	VADC_CHAN(_dname, IIO_VOLTAGE,				\
>> +		  BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_PROCESSED),\
>>  		  _pre)							\
> It very unusual to report both raw and processed values.  Please explain
> why that is needed here?  It may be valid to maintain backwards compatibility
> of ABI. Which would be fine. However if I read the above correctly you are
> changing what comes out of reading the raw value so the ABI just changed...
>
With the help of IIO sysfs ., we can read the ADC channel readings 
either in RAW format or in processed format. There are two separate 
individual entries to read the ADC channel either in Raw format or in 
processed format. Most of the clients for ADC expect the readings in 
processed format.
>
>>
>>  /*
>> @@ -850,9 +852,9 @@ static int vadc_get_dt_data(struct vadc_priv *vadc, struct device_node *node)
>>
>>  		iio_chan->channel = prop.channel;
>>  		iio_chan->datasheet_name = vadc_chan->datasheet_name;
>> +		iio_chan->extend_name = child->name;
> What's this change?
We can choose how we want to display our adc channel entries in sysfs. 
Am using the child node name to be displayed as the sysfs entry rather 
than channel number for easy interpretation.

For ex: for vcoin(coin battery voltage channel.,) with this change it 
appears like below in iio adc sysfs

"in_voltage_vcoin_input"

>>  		iio_chan->info_mask_separate = vadc_chan->info_mask;
>>  		iio_chan->type = vadc_chan->type;
>> -		iio_chan->indexed = 1;
> Or for that matter this one...
reason explained above.
>>  		iio_chan->address = index++;
>>
>>  		iio_chan++;
>>
>

Thanks,
Ramakrishna

---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus

Re: [PATCH V3 1/2] iio: adc: spmi-vadc: Update changes to support reporting of Raw adc code.

[Jonathan Cameron]

On 31/10/16 08:48, Rama Krishna Phani A wrote:
> Hi Jonathan,
> 
> On 30-Oct-16 11:05 PM, Jonathan Cameron wrote:
>> On 26/10/16 15:41, Rama Krishna Phani A wrote:
>>> Logic to convert adc code to voltage is generic across all channels.
>>> Different scaling can be done on the obtained voltage to report in physical
>>> units. Implement separate function for generic conversion logic.
>>> Scaling functionality can be changed per channel. Update changes to support
>>> reporting of Raw adc code.
>> Pleas rewrite this description.  Perhaps give examples of the changes
>> it makes to what is read from the various attributes?
> There are several channels in the ADC of PMIC which can be used to
> measure voltage, temperature, current etc., Hardware provides
> readings for all channels in adc code. That adc code needs to be
> converted to voltage. The logic for conversion of adc code to voltage
> is common for all ADC channels(voltage, temperature and current
> .,etc). Once voltage is obtained ., scaling is done on that voltage.
> 
> For Ex., Thermal SW wants to know the temperature of thermistor on
> PMIC and it expects the temperature to be reported in millidegC. ADC
> channel is used to read the adc code and convert it to voltage. Once
> the voltage is available based on the thermistor spec that voltage is
> mapped to a temperature and then that value is reported to Thermal
> SW.
> 
> Mapping of voltage to temperature is called scaling for that channel
> and scaling function can be different per channel based on how the
> voltage is reported.
Is the thermistor always part of the device? (i.e. in the chip) in which
case this might be fine.  If it's external then it needs to be described
by a separate device which acts as a consumer of the IIO channel and
in turn provides the scaled output to thermal.

The thermistor should really be separately described.   This is already
done in drivers/hwmon/ntc_thermistor 

Are any of these scalings characteristics of the chip supported by
this driver, or are they the result of external hardware?
>>
>> I haven't immediately followed what this change is actually doing.
>>
>> I 'think' the point here is to not apply the calibration to
>> the raw adc counts when a true raw read is requested?
>>
> When a true raw read is requested .,Scaling is not applied.
>> There are several unconnected looking changes in here...
>>>
>>> Signed-off-by: Rama Krishna Phani A <rphani-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>>> ---
>>>  drivers/iio/adc/qcom-spmi-vadc.c | 54 +++++++++++++++++++++-------------------
>>>  1 file changed, 28 insertions(+), 26 deletions(-)
>>>
>>> diff --git a/drivers/iio/adc/qcom-spmi-vadc.c b/drivers/iio/adc/qcom-spmi-vadc.c
>>> index c2babe5..ff4d549 100644
>>> --- a/drivers/iio/adc/qcom-spmi-vadc.c
>>> +++ b/drivers/iio/adc/qcom-spmi-vadc.c
>>> @@ -1,5 +1,5 @@
>>>  /*
>>> - * Copyright (c) 2012-2014, The Linux Foundation. All rights reserved.
>>> + * Copyright (c) 2012-2016, The Linux Foundation. All rights reserved.
>>>   *
>>>   * This program is free software; you can redistribute it and/or modify
>>>   * it under the terms of the GNU General Public License version 2 and
>>> @@ -84,7 +84,7 @@
>>>  #define VADC_MAX_ADC_CODE            0xa800
>>>
>>>  #define VADC_ABSOLUTE_RANGE_UV            625000
>>> -#define VADC_RATIOMETRIC_RANGE_UV        1800000
>>> +#define VADC_RATIOMETRIC_RANGE            1800
>>>
>>>  #define VADC_DEF_PRESCALING            0 /* 1:1 */
>>>  #define VADC_DEF_DECIMATION            0 /* 512 */
>>> @@ -418,7 +418,7 @@ static int vadc_measure_ref_points(struct vadc_priv *vadc)
>>>      u16 read_1, read_2;
>>>      int ret;
>>>
>>> -    vadc->graph[VADC_CALIB_RATIOMETRIC].dx = VADC_RATIOMETRIC_RANGE_UV;
>>> +    vadc->graph[VADC_CALIB_RATIOMETRIC].dx = VADC_RATIOMETRIC_RANGE;
>>>      vadc->graph[VADC_CALIB_ABSOLUTE].dx = VADC_ABSOLUTE_RANGE_UV;
>>>
>>>      prop = vadc_get_channel(vadc, VADC_REF_1250MV);
>>> @@ -468,21 +468,30 @@ static int vadc_measure_ref_points(struct vadc_priv *vadc)
>>>      return ret;
>>>  }
>>>
>>> -static s32 vadc_calibrate(struct vadc_priv *vadc,
>>> -              const struct vadc_channel_prop *prop, u16 adc_code)
>>> +static void vadc_scale_calib(struct vadc_priv *vadc, u16 adc_code,
>>> +                 const struct vadc_channel_prop *prop,
>>> +                 s64 *scale_voltage)
>>>  {
>>> -    const struct vadc_prescale_ratio *prescale;
>>> -    s64 voltage;
>>> +    *scale_voltage = (adc_code -
>>> +        vadc->graph[prop->calibration].gnd);
>>> +    *scale_voltage *= vadc->graph[prop->calibration].dx;
>>> +    *scale_voltage = div64_s64(*scale_voltage,
>>> +        vadc->graph[prop->calibration].dy);
>>> +    if (prop->calibration == VADC_CALIB_ABSOLUTE)
>>> +        *scale_voltage +=
>>> +        vadc->graph[prop->calibration].dx;
>>>
>>> -    voltage = adc_code - vadc->graph[prop->calibration].gnd;
>>> -    voltage *= vadc->graph[prop->calibration].dx;
>>> -    voltage = div64_s64(voltage, vadc->graph[prop->calibration].dy);
>>> +    if (*scale_voltage < 0)
>>> +        *scale_voltage = 0;
>>> +}
>>>
>>> -    if (prop->calibration == VADC_CALIB_ABSOLUTE)
>>> -        voltage += vadc->graph[prop->calibration].dx;
>>> +static s64 vadc_scale_fn(struct vadc_priv *vadc,
>>> +             const struct vadc_channel_prop *prop, u16 adc_code)
>>> +{
>>> +    const struct vadc_prescale_ratio *prescale;
>>> +    s64 voltage = 0;
>>>
>>> -    if (voltage < 0)
>>> -        voltage = 0;
>>> +    vadc_scale_calib(vadc, adc_code, prop, &voltage);
>>>
>>>      prescale = &vadc_prescale_ratios[prop->prescale];
>>>
>>> @@ -552,11 +561,8 @@ static int vadc_read_raw(struct iio_dev *indio_dev,
>>>          if (ret)
>>>              break;
>>>
>>> -        *val = vadc_calibrate(vadc, prop, adc_code);
>>> +        *val = vadc_scale_fn(vadc, prop, adc_code);
>>>
>>> -        /* 2mV/K, return milli Celsius */
>>> -        *val /= 2;
>>> -        *val -= KELVINMIL_CELSIUSMIL;
>>>          return IIO_VAL_INT;
>>>      case IIO_CHAN_INFO_RAW:
>>>          prop = &vadc->chan_props[chan->address];
>>> @@ -564,12 +570,8 @@ static int vadc_read_raw(struct iio_dev *indio_dev,
>>>          if (ret)
>>>              break;
>>>
>>> -        *val = vadc_calibrate(vadc, prop, adc_code);
>>> +        *val = (int)adc_code;
>>>          return IIO_VAL_INT;
>> So this is 'more raw'.
> Yes., its raw value.
>>> -    case IIO_CHAN_INFO_SCALE:
>>> -        *val = 0;
>>> -        *val2 = 1000;
>>> -        return IIO_VAL_INT_PLUS_MICRO;
>>>      default:
>>>          ret = -EINVAL;
>>>          break;
>>> @@ -616,8 +618,8 @@ struct vadc_channels {
>>>      VADC_CHAN(_dname, IIO_TEMP, BIT(IIO_CHAN_INFO_PROCESSED), _pre)    \
>>>
>>>  #define VADC_CHAN_VOLT(_dname, _pre)                    \
>>> -    VADC_CHAN(_dname, IIO_VOLTAGE,                    \
>>> -          BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),    \
>>> +    VADC_CHAN(_dname, IIO_VOLTAGE,                \
>>> +          BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_PROCESSED),\
>>>            _pre)                            \
>> It very unusual to report both raw and processed values.  Please explain
>> why that is needed here?  It may be valid to maintain backwards compatibility
>> of ABI. Which would be fine. However if I read the above correctly you are
>> changing what comes out of reading the raw value so the ABI just changed...
>>
> With the help of IIO sysfs ., we can read the ADC channel readings
> either in RAW format or in processed format. There are two separate
> individual entries to read the ADC channel either in Raw format or in
> processed format. Most of the clients for ADC expect the readings in
> processed format.
If we are talking in kernel, that is worked out by the application of
scale.  The IIO in kernel interfaces will do this automatically.

I we are talking in userspace, then the userspace needs to be
extended to support raw and scale reading.
>> 

>>>  /*
>>> @@ -850,9 +852,9 @@ static int vadc_get_dt_data(struct vadc_priv *vadc, struct device_node *node)
>>>
>>>          iio_chan->channel = prop.channel;
>>>          iio_chan->datasheet_name = vadc_chan->datasheet_name;
>>> +        iio_chan->extend_name = child->name;
>> What's this change?
> We can choose how we want to display our adc channel entries in sysfs. Am using the child node name to be displayed as the sysfs entry rather than channel number for easy interpretation.
> 
> For ex: for vcoin(coin battery voltage channel.,) with this change it appears like below in iio adc sysfs
> 
> "in_voltage_vcoin_input"
No.  This introduces a mass of undocumented (and uncontrolled) ABI.
If there are reasons to add such a label then it should not be done
in the file name.
> 
>>>          iio_chan->info_mask_separate = vadc_chan->info_mask;
>>>          iio_chan->type = vadc_chan->type;
>>> -        iio_chan->indexed = 1;
>> Or for that matter this one...
> reason explained above.
>>>          iio_chan->address = index++;
>>>
>>>          iio_chan++;
>>>
>>
> 
> Thanks,
> Ramakrishna
> 
> ---
> This email has been checked for viruses by Avast antivirus software.
> https://www.avast.com/antivirus
> 
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V3 1/2] iio: adc: spmi-vadc: Update changes to support reporting of Raw adc code.

[Jonathan Cameron]

On 31/10/16 08:48, Rama Krishna Phani A wrote:
> Hi Jonathan,
> 
> On 30-Oct-16 11:05 PM, Jonathan Cameron wrote:
>> On 26/10/16 15:41, Rama Krishna Phani A wrote:
>>> Logic to convert adc code to voltage is generic across all channels.
>>> Different scaling can be done on the obtained voltage to report in physical
>>> units. Implement separate function for generic conversion logic.
>>> Scaling functionality can be changed per channel. Update changes to support
>>> reporting of Raw adc code.
>> Pleas rewrite this description.  Perhaps give examples of the changes
>> it makes to what is read from the various attributes?
> There are several channels in the ADC of PMIC which can be used to
> measure voltage, temperature, current etc., Hardware provides
> readings for all channels in adc code. That adc code needs to be
> converted to voltage. The logic for conversion of adc code to voltage
> is common for all ADC channels(voltage, temperature and current
> .,etc). Once voltage is obtained ., scaling is done on that voltage.
> 
> For Ex., Thermal SW wants to know the temperature of thermistor on
> PMIC and it expects the temperature to be reported in millidegC. ADC
> channel is used to read the adc code and convert it to voltage. Once
> the voltage is available based on the thermistor spec that voltage is
> mapped to a temperature and then that value is reported to Thermal
> SW.
> 
> Mapping of voltage to temperature is called scaling for that channel
> and scaling function can be different per channel based on how the
> voltage is reported.
Is the thermistor always part of the device? (i.e. in the chip) in which
case this might be fine.  If it's external then it needs to be described
by a separate device which acts as a consumer of the IIO channel and
in turn provides the scaled output to thermal.

The thermistor should really be separately described.   This is already
done in drivers/hwmon/ntc_thermistor 

Are any of these scalings characteristics of the chip supported by
this driver, or are they the result of external hardware?
>>
>> I haven't immediately followed what this change is actually doing.
>>
>> I 'think' the point here is to not apply the calibration to
>> the raw adc counts when a true raw read is requested?
>>
> When a true raw read is requested .,Scaling is not applied.
>> There are several unconnected looking changes in here...
>>>
>>> Signed-off-by: Rama Krishna Phani A <rphani@codeaurora.org>
>>> ---
>>>  drivers/iio/adc/qcom-spmi-vadc.c | 54 +++++++++++++++++++++-------------------
>>>  1 file changed, 28 insertions(+), 26 deletions(-)
>>>
>>> diff --git a/drivers/iio/adc/qcom-spmi-vadc.c b/drivers/iio/adc/qcom-spmi-vadc.c
>>> index c2babe5..ff4d549 100644
>>> --- a/drivers/iio/adc/qcom-spmi-vadc.c
>>> +++ b/drivers/iio/adc/qcom-spmi-vadc.c
>>> @@ -1,5 +1,5 @@
>>>  /*
>>> - * Copyright (c) 2012-2014, The Linux Foundation. All rights reserved.
>>> + * Copyright (c) 2012-2016, The Linux Foundation. All rights reserved.
>>>   *
>>>   * This program is free software; you can redistribute it and/or modify
>>>   * it under the terms of the GNU General Public License version 2 and
>>> @@ -84,7 +84,7 @@
>>>  #define VADC_MAX_ADC_CODE            0xa800
>>>
>>>  #define VADC_ABSOLUTE_RANGE_UV            625000
>>> -#define VADC_RATIOMETRIC_RANGE_UV        1800000
>>> +#define VADC_RATIOMETRIC_RANGE            1800
>>>
>>>  #define VADC_DEF_PRESCALING            0 /* 1:1 */
>>>  #define VADC_DEF_DECIMATION            0 /* 512 */
>>> @@ -418,7 +418,7 @@ static int vadc_measure_ref_points(struct vadc_priv *vadc)
>>>      u16 read_1, read_2;
>>>      int ret;
>>>
>>> -    vadc->graph[VADC_CALIB_RATIOMETRIC].dx = VADC_RATIOMETRIC_RANGE_UV;
>>> +    vadc->graph[VADC_CALIB_RATIOMETRIC].dx = VADC_RATIOMETRIC_RANGE;
>>>      vadc->graph[VADC_CALIB_ABSOLUTE].dx = VADC_ABSOLUTE_RANGE_UV;
>>>
>>>      prop = vadc_get_channel(vadc, VADC_REF_1250MV);
>>> @@ -468,21 +468,30 @@ static int vadc_measure_ref_points(struct vadc_priv *vadc)
>>>      return ret;
>>>  }
>>>
>>> -static s32 vadc_calibrate(struct vadc_priv *vadc,
>>> -              const struct vadc_channel_prop *prop, u16 adc_code)
>>> +static void vadc_scale_calib(struct vadc_priv *vadc, u16 adc_code,
>>> +                 const struct vadc_channel_prop *prop,
>>> +                 s64 *scale_voltage)
>>>  {
>>> -    const struct vadc_prescale_ratio *prescale;
>>> -    s64 voltage;
>>> +    *scale_voltage = (adc_code -
>>> +        vadc->graph[prop->calibration].gnd);
>>> +    *scale_voltage *= vadc->graph[prop->calibration].dx;
>>> +    *scale_voltage = div64_s64(*scale_voltage,
>>> +        vadc->graph[prop->calibration].dy);
>>> +    if (prop->calibration == VADC_CALIB_ABSOLUTE)
>>> +        *scale_voltage +=
>>> +        vadc->graph[prop->calibration].dx;
>>>
>>> -    voltage = adc_code - vadc->graph[prop->calibration].gnd;
>>> -    voltage *= vadc->graph[prop->calibration].dx;
>>> -    voltage = div64_s64(voltage, vadc->graph[prop->calibration].dy);
>>> +    if (*scale_voltage < 0)
>>> +        *scale_voltage = 0;
>>> +}
>>>
>>> -    if (prop->calibration == VADC_CALIB_ABSOLUTE)
>>> -        voltage += vadc->graph[prop->calibration].dx;
>>> +static s64 vadc_scale_fn(struct vadc_priv *vadc,
>>> +             const struct vadc_channel_prop *prop, u16 adc_code)
>>> +{
>>> +    const struct vadc_prescale_ratio *prescale;
>>> +    s64 voltage = 0;
>>>
>>> -    if (voltage < 0)
>>> -        voltage = 0;
>>> +    vadc_scale_calib(vadc, adc_code, prop, &voltage);
>>>
>>>      prescale = &vadc_prescale_ratios[prop->prescale];
>>>
>>> @@ -552,11 +561,8 @@ static int vadc_read_raw(struct iio_dev *indio_dev,
>>>          if (ret)
>>>              break;
>>>
>>> -        *val = vadc_calibrate(vadc, prop, adc_code);
>>> +        *val = vadc_scale_fn(vadc, prop, adc_code);
>>>
>>> -        /* 2mV/K, return milli Celsius */
>>> -        *val /= 2;
>>> -        *val -= KELVINMIL_CELSIUSMIL;
>>>          return IIO_VAL_INT;
>>>      case IIO_CHAN_INFO_RAW:
>>>          prop = &vadc->chan_props[chan->address];
>>> @@ -564,12 +570,8 @@ static int vadc_read_raw(struct iio_dev *indio_dev,
>>>          if (ret)
>>>              break;
>>>
>>> -        *val = vadc_calibrate(vadc, prop, adc_code);
>>> +        *val = (int)adc_code;
>>>          return IIO_VAL_INT;
>> So this is 'more raw'.
> Yes., its raw value.
>>> -    case IIO_CHAN_INFO_SCALE:
>>> -        *val = 0;
>>> -        *val2 = 1000;
>>> -        return IIO_VAL_INT_PLUS_MICRO;
>>>      default:
>>>          ret = -EINVAL;
>>>          break;
>>> @@ -616,8 +618,8 @@ struct vadc_channels {
>>>      VADC_CHAN(_dname, IIO_TEMP, BIT(IIO_CHAN_INFO_PROCESSED), _pre)    \
>>>
>>>  #define VADC_CHAN_VOLT(_dname, _pre)                    \
>>> -    VADC_CHAN(_dname, IIO_VOLTAGE,                    \
>>> -          BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),    \
>>> +    VADC_CHAN(_dname, IIO_VOLTAGE,                \
>>> +          BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_PROCESSED),\
>>>            _pre)                            \
>> It very unusual to report both raw and processed values.  Please explain
>> why that is needed here?  It may be valid to maintain backwards compatibility
>> of ABI. Which would be fine. However if I read the above correctly you are
>> changing what comes out of reading the raw value so the ABI just changed...
>>
> With the help of IIO sysfs ., we can read the ADC channel readings
> either in RAW format or in processed format. There are two separate
> individual entries to read the ADC channel either in Raw format or in
> processed format. Most of the clients for ADC expect the readings in
> processed format.
If we are talking in kernel, that is worked out by the application of
scale.  The IIO in kernel interfaces will do this automatically.

I we are talking in userspace, then the userspace needs to be
extended to support raw and scale reading.
>> 

>>>  /*
>>> @@ -850,9 +852,9 @@ static int vadc_get_dt_data(struct vadc_priv *vadc, struct device_node *node)
>>>
>>>          iio_chan->channel = prop.channel;
>>>          iio_chan->datasheet_name = vadc_chan->datasheet_name;
>>> +        iio_chan->extend_name = child->name;
>> What's this change?
> We can choose how we want to display our adc channel entries in sysfs. Am using the child node name to be displayed as the sysfs entry rather than channel number for easy interpretation.
> 
> For ex: for vcoin(coin battery voltage channel.,) with this change it appears like below in iio adc sysfs
> 
> "in_voltage_vcoin_input"
No.  This introduces a mass of undocumented (and uncontrolled) ABI.
If there are reasons to add such a label then it should not be done
in the file name.
> 
>>>          iio_chan->info_mask_separate = vadc_chan->info_mask;
>>>          iio_chan->type = vadc_chan->type;
>>> -        iio_chan->indexed = 1;
>> Or for that matter this one...
> reason explained above.
>>>          iio_chan->address = index++;
>>>
>>>          iio_chan++;
>>>
>>
> 
> Thanks,
> Ramakrishna
> 
> ---
> This email has been checked for viruses by Avast antivirus software.
> https://www.avast.com/antivirus
> 
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V3 1/2] iio: adc: spmi-vadc: Update changes to support reporting of Raw adc code.

[Phani A, Rama Krishna]

Hi Jonathan,

On 02-Nov-16 12:00 AM, Jonathan Cameron wrote:
> On 31/10/16 08:48, Rama Krishna Phani A wrote:
>> Hi Jonathan,
>>
>> On 30-Oct-16 11:05 PM, Jonathan Cameron wrote:
>>> On 26/10/16 15:41, Rama Krishna Phani A wrote:
>>>> Logic to convert adc code to voltage is generic across all channels.
>>>> Different scaling can be done on the obtained voltage to report in physical
>>>> units. Implement separate function for generic conversion logic.
>>>> Scaling functionality can be changed per channel. Update changes to support
>>>> reporting of Raw adc code.
>>> Pleas rewrite this description.  Perhaps give examples of the changes
>>> it makes to what is read from the various attributes?
>> There are several channels in the ADC of PMIC which can be used to
>> measure voltage, temperature, current etc., Hardware provides
>> readings for all channels in adc code. That adc code needs to be
>> converted to voltage. The logic for conversion of adc code to voltage
>> is common for all ADC channels(voltage, temperature and current
>> .,etc). Once voltage is obtained ., scaling is done on that voltage.
>>
>> For Ex., Thermal SW wants to know the temperature of thermistor on
>> PMIC and it expects the temperature to be reported in millidegC. ADC
>> channel is used to read the adc code and convert it to voltage. Once
>> the voltage is available based on the thermistor spec that voltage is
>> mapped to a temperature and then that value is reported to Thermal
>> SW.
>>
>> Mapping of voltage to temperature is called scaling for that channel
>> and scaling function can be different per channel based on how the
>> voltage is reported.
> Is the thermistor always part of the device? (i.e. in the chip) in which
> case this might be fine.  If it's external then it needs to be described
> by a separate device which acts as a consumer of the IIO channel and
> in turn provides the scaled output to thermal.
>
> The thermistor should really be separately described.   This is already
> done in drivers/hwmon/ntc_thermistor
>
> Are any of these scalings characteristics of the chip supported by
> this driver, or are they the result of external hardware?

All the VADC channels i.e., Voltage, temperature(thermistors and other 
channels) are part of PMIC chip. The scaling functionalities supported 
in this driver are for the adc channels which are part of PMIC chip.

>>>
>>> I haven't immediately followed what this change is actually doing.
>>>
>>> I 'think' the point here is to not apply the calibration to
>>> the raw adc counts when a true raw read is requested?
>>>
>> When a true raw read is requested .,Scaling is not applied.
>>> There are several unconnected looking changes in here...
>>>>
>>>> Signed-off-by: Rama Krishna Phani A <rphani-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>>>> ---
>>>>  drivers/iio/adc/qcom-spmi-vadc.c | 54 +++++++++++++++++++++-------------------
>>>>  1 file changed, 28 insertions(+), 26 deletions(-)
>>>>
>>>> diff --git a/drivers/iio/adc/qcom-spmi-vadc.c b/drivers/iio/adc/qcom-spmi-vadc.c
>>>> index c2babe5..ff4d549 100644
>>>> --- a/drivers/iio/adc/qcom-spmi-vadc.c
>>>> +++ b/drivers/iio/adc/qcom-spmi-vadc.c
>>>> @@ -1,5 +1,5 @@
>>>>  /*
>>>> - * Copyright (c) 2012-2014, The Linux Foundation. All rights reserved.
>>>> + * Copyright (c) 2012-2016, The Linux Foundation. All rights reserved.
>>>>   *
>>>>   * This program is free software; you can redistribute it and/or modify
>>>>   * it under the terms of the GNU General Public License version 2 and
>>>> @@ -84,7 +84,7 @@
>>>>  #define VADC_MAX_ADC_CODE            0xa800
>>>>
>>>>  #define VADC_ABSOLUTE_RANGE_UV            625000
>>>> -#define VADC_RATIOMETRIC_RANGE_UV        1800000
>>>> +#define VADC_RATIOMETRIC_RANGE            1800
>>>>
>>>>  #define VADC_DEF_PRESCALING            0 /* 1:1 */
>>>>  #define VADC_DEF_DECIMATION            0 /* 512 */
>>>> @@ -418,7 +418,7 @@ static int vadc_measure_ref_points(struct vadc_priv *vadc)
>>>>      u16 read_1, read_2;
>>>>      int ret;
>>>>
>>>> -    vadc->graph[VADC_CALIB_RATIOMETRIC].dx = VADC_RATIOMETRIC_RANGE_UV;
>>>> +    vadc->graph[VADC_CALIB_RATIOMETRIC].dx = VADC_RATIOMETRIC_RANGE;
>>>>      vadc->graph[VADC_CALIB_ABSOLUTE].dx = VADC_ABSOLUTE_RANGE_UV;
>>>>
>>>>      prop = vadc_get_channel(vadc, VADC_REF_1250MV);
>>>> @@ -468,21 +468,30 @@ static int vadc_measure_ref_points(struct vadc_priv *vadc)
>>>>      return ret;
>>>>  }
>>>>
>>>> -static s32 vadc_calibrate(struct vadc_priv *vadc,
>>>> -              const struct vadc_channel_prop *prop, u16 adc_code)
>>>> +static void vadc_scale_calib(struct vadc_priv *vadc, u16 adc_code,
>>>> +                 const struct vadc_channel_prop *prop,
>>>> +                 s64 *scale_voltage)
>>>>  {
>>>> -    const struct vadc_prescale_ratio *prescale;
>>>> -    s64 voltage;
>>>> +    *scale_voltage = (adc_code -
>>>> +        vadc->graph[prop->calibration].gnd);
>>>> +    *scale_voltage *= vadc->graph[prop->calibration].dx;
>>>> +    *scale_voltage = div64_s64(*scale_voltage,
>>>> +        vadc->graph[prop->calibration].dy);
>>>> +    if (prop->calibration == VADC_CALIB_ABSOLUTE)
>>>> +        *scale_voltage +=
>>>> +        vadc->graph[prop->calibration].dx;
>>>>
>>>> -    voltage = adc_code - vadc->graph[prop->calibration].gnd;
>>>> -    voltage *= vadc->graph[prop->calibration].dx;
>>>> -    voltage = div64_s64(voltage, vadc->graph[prop->calibration].dy);
>>>> +    if (*scale_voltage < 0)
>>>> +        *scale_voltage = 0;
>>>> +}
>>>>
>>>> -    if (prop->calibration == VADC_CALIB_ABSOLUTE)
>>>> -        voltage += vadc->graph[prop->calibration].dx;
>>>> +static s64 vadc_scale_fn(struct vadc_priv *vadc,
>>>> +             const struct vadc_channel_prop *prop, u16 adc_code)
>>>> +{
>>>> +    const struct vadc_prescale_ratio *prescale;
>>>> +    s64 voltage = 0;
>>>>
>>>> -    if (voltage < 0)
>>>> -        voltage = 0;
>>>> +    vadc_scale_calib(vadc, adc_code, prop, &voltage);
>>>>
>>>>      prescale = &vadc_prescale_ratios[prop->prescale];
>>>>
>>>> @@ -552,11 +561,8 @@ static int vadc_read_raw(struct iio_dev *indio_dev,
>>>>          if (ret)
>>>>              break;
>>>>
>>>> -        *val = vadc_calibrate(vadc, prop, adc_code);
>>>> +        *val = vadc_scale_fn(vadc, prop, adc_code);
>>>>
>>>> -        /* 2mV/K, return milli Celsius */
>>>> -        *val /= 2;
>>>> -        *val -= KELVINMIL_CELSIUSMIL;
>>>>          return IIO_VAL_INT;
>>>>      case IIO_CHAN_INFO_RAW:
>>>>          prop = &vadc->chan_props[chan->address];
>>>> @@ -564,12 +570,8 @@ static int vadc_read_raw(struct iio_dev *indio_dev,
>>>>          if (ret)
>>>>              break;
>>>>
>>>> -        *val = vadc_calibrate(vadc, prop, adc_code);
>>>> +        *val = (int)adc_code;
>>>>          return IIO_VAL_INT;
>>> So this is 'more raw'.
>> Yes., its raw value.
>>>> -    case IIO_CHAN_INFO_SCALE:
>>>> -        *val = 0;
>>>> -        *val2 = 1000;
>>>> -        return IIO_VAL_INT_PLUS_MICRO;
>>>>      default:
>>>>          ret = -EINVAL;
>>>>          break;
>>>> @@ -616,8 +618,8 @@ struct vadc_channels {
>>>>      VADC_CHAN(_dname, IIO_TEMP, BIT(IIO_CHAN_INFO_PROCESSED), _pre)    \
>>>>
>>>>  #define VADC_CHAN_VOLT(_dname, _pre)                    \
>>>> -    VADC_CHAN(_dname, IIO_VOLTAGE,                    \
>>>> -          BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),    \
>>>> +    VADC_CHAN(_dname, IIO_VOLTAGE,                \
>>>> +          BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_PROCESSED),\
>>>>            _pre)                            \
>>> It very unusual to report both raw and processed values.  Please explain
>>> why that is needed here?  It may be valid to maintain backwards compatibility
>>> of ABI. Which would be fine. However if I read the above correctly you are
>>> changing what comes out of reading the raw value so the ABI just changed...
>>>
>> With the help of IIO sysfs ., we can read the ADC channel readings
>> either in RAW format or in processed format. There are two separate
>> individual entries to read the ADC channel either in Raw format or in
>> processed format. Most of the clients for ADC expect the readings in
>> processed format.
> If we are talking in kernel, that is worked out by the application of
> scale.  The IIO in kernel interfaces will do this automatically.
>
> I we are talking in userspace, then the userspace needs to be
> extended to support raw and scale reading.

Every channel present in adc has an unique conversion formula for 
obtained voltage, suggested by Hardware designers.

Ex: For die_temp channel., Temp = 2mv/Kelvin
Above formula has to be applied on obtained voltage in order for the 
channel to report the temperature in milldegC.

Like wise every channel has unique way of conversion logic suggested by 
HW folks. That conversion logic is done in ADC driver.

>>>
>
>>>>  /*
>>>> @@ -850,9 +852,9 @@ static int vadc_get_dt_data(struct vadc_priv *vadc, struct device_node *node)
>>>>
>>>>          iio_chan->channel = prop.channel;
>>>>          iio_chan->datasheet_name = vadc_chan->datasheet_name;
>>>> +        iio_chan->extend_name = child->name;
>>> What's this change?
>> We can choose how we want to display our adc channel entries in sysfs. Am using the child node name to be displayed as the sysfs entry rather than channel number for easy interpretation.
>>
>> For ex: for vcoin(coin battery voltage channel.,) with this change it appears like below in iio adc sysfs
>>
>> "in_voltage_vcoin_input"
> No.  This introduces a mass of undocumented (and uncontrolled) ABI.
> If there are reasons to add such a label then it should not be done
> in the file name.

"extended_name" is an existing field in "iio_chan_spec" structure, 
present in iio.h(include\linux\iio) and has documentation regarding the 
functionality. Pasting it here for quick reference.

  * @extend_name:Allows labeling of channel attributes with an
  *		informative name. Note this has no effect codes etc,
  *		unlike modifiers.

Am trying to use the existing field here., initializing it with a value 
which is easy for interpretation of channel attributes.

>>
>>>>          iio_chan->info_mask_separate = vadc_chan->info_mask;
>>>>          iio_chan->type = vadc_chan->type;
>>>> -        iio_chan->indexed = 1;
>>> Or for that matter this one...
>> reason explained above.
>>>>          iio_chan->address = index++;
>>>>
>>>>          iio_chan++;
>>>>
>>>
>>
>> Thanks,
>> Ramakrishna
>>
>> ---
>> This email has been checked for viruses by Avast antivirus software.
>> https://www.avast.com/antivirus
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Thanks,
Ramakrishna
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V3 1/2] iio: adc: spmi-vadc: Update changes to support reporting of Raw adc code.

[Phani A, Rama Krishna]

Hi Jonathan,

On 02-Nov-16 12:00 AM, Jonathan Cameron wrote:
> On 31/10/16 08:48, Rama Krishna Phani A wrote:
>> Hi Jonathan,
>>
>> On 30-Oct-16 11:05 PM, Jonathan Cameron wrote:
>>> On 26/10/16 15:41, Rama Krishna Phani A wrote:
>>>> Logic to convert adc code to voltage is generic across all channels.
>>>> Different scaling can be done on the obtained voltage to report in physical
>>>> units. Implement separate function for generic conversion logic.
>>>> Scaling functionality can be changed per channel. Update changes to support
>>>> reporting of Raw adc code.
>>> Pleas rewrite this description.  Perhaps give examples of the changes
>>> it makes to what is read from the various attributes?
>> There are several channels in the ADC of PMIC which can be used to
>> measure voltage, temperature, current etc., Hardware provides
>> readings for all channels in adc code. That adc code needs to be
>> converted to voltage. The logic for conversion of adc code to voltage
>> is common for all ADC channels(voltage, temperature and current
>> .,etc). Once voltage is obtained ., scaling is done on that voltage.
>>
>> For Ex., Thermal SW wants to know the temperature of thermistor on
>> PMIC and it expects the temperature to be reported in millidegC. ADC
>> channel is used to read the adc code and convert it to voltage. Once
>> the voltage is available based on the thermistor spec that voltage is
>> mapped to a temperature and then that value is reported to Thermal
>> SW.
>>
>> Mapping of voltage to temperature is called scaling for that channel
>> and scaling function can be different per channel based on how the
>> voltage is reported.
> Is the thermistor always part of the device? (i.e. in the chip) in which
> case this might be fine.  If it's external then it needs to be described
> by a separate device which acts as a consumer of the IIO channel and
> in turn provides the scaled output to thermal.
>
> The thermistor should really be separately described.   This is already
> done in drivers/hwmon/ntc_thermistor
>
> Are any of these scalings characteristics of the chip supported by
> this driver, or are they the result of external hardware?

All the VADC channels i.e., Voltage, temperature(thermistors and other 
channels) are part of PMIC chip. The scaling functionalities supported 
in this driver are for the adc channels which are part of PMIC chip.

>>>
>>> I haven't immediately followed what this change is actually doing.
>>>
>>> I 'think' the point here is to not apply the calibration to
>>> the raw adc counts when a true raw read is requested?
>>>
>> When a true raw read is requested .,Scaling is not applied.
>>> There are several unconnected looking changes in here...
>>>>
>>>> Signed-off-by: Rama Krishna Phani A <rphani@codeaurora.org>
>>>> ---
>>>>  drivers/iio/adc/qcom-spmi-vadc.c | 54 +++++++++++++++++++++-------------------
>>>>  1 file changed, 28 insertions(+), 26 deletions(-)
>>>>
>>>> diff --git a/drivers/iio/adc/qcom-spmi-vadc.c b/drivers/iio/adc/qcom-spmi-vadc.c
>>>> index c2babe5..ff4d549 100644
>>>> --- a/drivers/iio/adc/qcom-spmi-vadc.c
>>>> +++ b/drivers/iio/adc/qcom-spmi-vadc.c
>>>> @@ -1,5 +1,5 @@
>>>>  /*
>>>> - * Copyright (c) 2012-2014, The Linux Foundation. All rights reserved.
>>>> + * Copyright (c) 2012-2016, The Linux Foundation. All rights reserved.
>>>>   *
>>>>   * This program is free software; you can redistribute it and/or modify
>>>>   * it under the terms of the GNU General Public License version 2 and
>>>> @@ -84,7 +84,7 @@
>>>>  #define VADC_MAX_ADC_CODE            0xa800
>>>>
>>>>  #define VADC_ABSOLUTE_RANGE_UV            625000
>>>> -#define VADC_RATIOMETRIC_RANGE_UV        1800000
>>>> +#define VADC_RATIOMETRIC_RANGE            1800
>>>>
>>>>  #define VADC_DEF_PRESCALING            0 /* 1:1 */
>>>>  #define VADC_DEF_DECIMATION            0 /* 512 */
>>>> @@ -418,7 +418,7 @@ static int vadc_measure_ref_points(struct vadc_priv *vadc)
>>>>      u16 read_1, read_2;
>>>>      int ret;
>>>>
>>>> -    vadc->graph[VADC_CALIB_RATIOMETRIC].dx = VADC_RATIOMETRIC_RANGE_UV;
>>>> +    vadc->graph[VADC_CALIB_RATIOMETRIC].dx = VADC_RATIOMETRIC_RANGE;
>>>>      vadc->graph[VADC_CALIB_ABSOLUTE].dx = VADC_ABSOLUTE_RANGE_UV;
>>>>
>>>>      prop = vadc_get_channel(vadc, VADC_REF_1250MV);
>>>> @@ -468,21 +468,30 @@ static int vadc_measure_ref_points(struct vadc_priv *vadc)
>>>>      return ret;
>>>>  }
>>>>
>>>> -static s32 vadc_calibrate(struct vadc_priv *vadc,
>>>> -              const struct vadc_channel_prop *prop, u16 adc_code)
>>>> +static void vadc_scale_calib(struct vadc_priv *vadc, u16 adc_code,
>>>> +                 const struct vadc_channel_prop *prop,
>>>> +                 s64 *scale_voltage)
>>>>  {
>>>> -    const struct vadc_prescale_ratio *prescale;
>>>> -    s64 voltage;
>>>> +    *scale_voltage = (adc_code -
>>>> +        vadc->graph[prop->calibration].gnd);
>>>> +    *scale_voltage *= vadc->graph[prop->calibration].dx;
>>>> +    *scale_voltage = div64_s64(*scale_voltage,
>>>> +        vadc->graph[prop->calibration].dy);
>>>> +    if (prop->calibration == VADC_CALIB_ABSOLUTE)
>>>> +        *scale_voltage +=
>>>> +        vadc->graph[prop->calibration].dx;
>>>>
>>>> -    voltage = adc_code - vadc->graph[prop->calibration].gnd;
>>>> -    voltage *= vadc->graph[prop->calibration].dx;
>>>> -    voltage = div64_s64(voltage, vadc->graph[prop->calibration].dy);
>>>> +    if (*scale_voltage < 0)
>>>> +        *scale_voltage = 0;
>>>> +}
>>>>
>>>> -    if (prop->calibration == VADC_CALIB_ABSOLUTE)
>>>> -        voltage += vadc->graph[prop->calibration].dx;
>>>> +static s64 vadc_scale_fn(struct vadc_priv *vadc,
>>>> +             const struct vadc_channel_prop *prop, u16 adc_code)
>>>> +{
>>>> +    const struct vadc_prescale_ratio *prescale;
>>>> +    s64 voltage = 0;
>>>>
>>>> -    if (voltage < 0)
>>>> -        voltage = 0;
>>>> +    vadc_scale_calib(vadc, adc_code, prop, &voltage);
>>>>
>>>>      prescale = &vadc_prescale_ratios[prop->prescale];
>>>>
>>>> @@ -552,11 +561,8 @@ static int vadc_read_raw(struct iio_dev *indio_dev,
>>>>          if (ret)
>>>>              break;
>>>>
>>>> -        *val = vadc_calibrate(vadc, prop, adc_code);
>>>> +        *val = vadc_scale_fn(vadc, prop, adc_code);
>>>>
>>>> -        /* 2mV/K, return milli Celsius */
>>>> -        *val /= 2;
>>>> -        *val -= KELVINMIL_CELSIUSMIL;
>>>>          return IIO_VAL_INT;
>>>>      case IIO_CHAN_INFO_RAW:
>>>>          prop = &vadc->chan_props[chan->address];
>>>> @@ -564,12 +570,8 @@ static int vadc_read_raw(struct iio_dev *indio_dev,
>>>>          if (ret)
>>>>              break;
>>>>
>>>> -        *val = vadc_calibrate(vadc, prop, adc_code);
>>>> +        *val = (int)adc_code;
>>>>          return IIO_VAL_INT;
>>> So this is 'more raw'.
>> Yes., its raw value.
>>>> -    case IIO_CHAN_INFO_SCALE:
>>>> -        *val = 0;
>>>> -        *val2 = 1000;
>>>> -        return IIO_VAL_INT_PLUS_MICRO;
>>>>      default:
>>>>          ret = -EINVAL;
>>>>          break;
>>>> @@ -616,8 +618,8 @@ struct vadc_channels {
>>>>      VADC_CHAN(_dname, IIO_TEMP, BIT(IIO_CHAN_INFO_PROCESSED), _pre)    \
>>>>
>>>>  #define VADC_CHAN_VOLT(_dname, _pre)                    \
>>>> -    VADC_CHAN(_dname, IIO_VOLTAGE,                    \
>>>> -          BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),    \
>>>> +    VADC_CHAN(_dname, IIO_VOLTAGE,                \
>>>> +          BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_PROCESSED),\
>>>>            _pre)                            \
>>> It very unusual to report both raw and processed values.  Please explain
>>> why that is needed here?  It may be valid to maintain backwards compatibility
>>> of ABI. Which would be fine. However if I read the above correctly you are
>>> changing what comes out of reading the raw value so the ABI just changed...
>>>
>> With the help of IIO sysfs ., we can read the ADC channel readings
>> either in RAW format or in processed format. There are two separate
>> individual entries to read the ADC channel either in Raw format or in
>> processed format. Most of the clients for ADC expect the readings in
>> processed format.
> If we are talking in kernel, that is worked out by the application of
> scale.  The IIO in kernel interfaces will do this automatically.
>
> I we are talking in userspace, then the userspace needs to be
> extended to support raw and scale reading.

Every channel present in adc has an unique conversion formula for 
obtained voltage, suggested by Hardware designers.

Ex: For die_temp channel., Temp = 2mv/Kelvin
Above formula has to be applied on obtained voltage in order for the 
channel to report the temperature in milldegC.

Like wise every channel has unique way of conversion logic suggested by 
HW folks. That conversion logic is done in ADC driver.

>>>
>
>>>>  /*
>>>> @@ -850,9 +852,9 @@ static int vadc_get_dt_data(struct vadc_priv *vadc, struct device_node *node)
>>>>
>>>>          iio_chan->channel = prop.channel;
>>>>          iio_chan->datasheet_name = vadc_chan->datasheet_name;
>>>> +        iio_chan->extend_name = child->name;
>>> What's this change?
>> We can choose how we want to display our adc channel entries in sysfs. Am using the child node name to be displayed as the sysfs entry rather than channel number for easy interpretation.
>>
>> For ex: for vcoin(coin battery voltage channel.,) with this change it appears like below in iio adc sysfs
>>
>> "in_voltage_vcoin_input"
> No.  This introduces a mass of undocumented (and uncontrolled) ABI.
> If there are reasons to add such a label then it should not be done
> in the file name.

"extended_name" is an existing field in "iio_chan_spec" structure, 
present in iio.h(include\linux\iio) and has documentation regarding the 
functionality. Pasting it here for quick reference.

  * @extend_name:Allows labeling of channel attributes with an
  *		informative name. Note this has no effect codes etc,
  *		unlike modifiers.

Am trying to use the existing field here., initializing it with a value 
which is easy for interpretation of channel attributes.

>>
>>>>          iio_chan->info_mask_separate = vadc_chan->info_mask;
>>>>          iio_chan->type = vadc_chan->type;
>>>> -        iio_chan->indexed = 1;
>>> Or for that matter this one...
>> reason explained above.
>>>>          iio_chan->address = index++;
>>>>
>>>>          iio_chan++;
>>>>
>>>
>>
>> Thanks,
>> Ramakrishna
>>
>> ---
>> This email has been checked for viruses by Avast antivirus software.
>> https://www.avast.com/antivirus
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Thanks,
Ramakrishna

Re: [PATCH V3 1/2] iio: adc: spmi-vadc: Update changes to support reporting of Raw adc code.

[Jonathan Cameron]

On 02/11/16 13:12, Phani A, Rama Krishna wrote:
> Hi Jonathan,
> 
> On 02-Nov-16 12:00 AM, Jonathan Cameron wrote:
>> On 31/10/16 08:48, Rama Krishna Phani A wrote:
>>> Hi Jonathan,
>>>
>>> On 30-Oct-16 11:05 PM, Jonathan Cameron wrote:
>>>> On 26/10/16 15:41, Rama Krishna Phani A wrote:
>>>>> Logic to convert adc code to voltage is generic across all channels.
>>>>> Different scaling can be done on the obtained voltage to report in physical
>>>>> units. Implement separate function for generic conversion logic.
>>>>> Scaling functionality can be changed per channel. Update changes to support
>>>>> reporting of Raw adc code.
>>>> Pleas rewrite this description.  Perhaps give examples of the changes
>>>> it makes to what is read from the various attributes?
>>> There are several channels in the ADC of PMIC which can be used to
>>> measure voltage, temperature, current etc., Hardware provides
>>> readings for all channels in adc code. That adc code needs to be
>>> converted to voltage. The logic for conversion of adc code to voltage
>>> is common for all ADC channels(voltage, temperature and current
>>> .,etc). Once voltage is obtained ., scaling is done on that voltage.
>>>
>>> For Ex., Thermal SW wants to know the temperature of thermistor on
>>> PMIC and it expects the temperature to be reported in millidegC. ADC
>>> channel is used to read the adc code and convert it to voltage. Once
>>> the voltage is available based on the thermistor spec that voltage is
>>> mapped to a temperature and then that value is reported to Thermal
>>> SW.
>>>
>>> Mapping of voltage to temperature is called scaling for that channel
>>> and scaling function can be different per channel based on how the
>>> voltage is reported.
>> Is the thermistor always part of the device? (i.e. in the chip) in which
>> case this might be fine.  If it's external then it needs to be described
>> by a separate device which acts as a consumer of the IIO channel and
>> in turn provides the scaled output to thermal.
>>
>> The thermistor should really be separately described.   This is already
>> done in drivers/hwmon/ntc_thermistor
>>
>> Are any of these scalings characteristics of the chip supported by
>> this driver, or are they the result of external hardware?
> 
> All the VADC channels i.e., Voltage, temperature(thermistors and
> other channels) are part of PMIC chip. The scaling functionalities
> supported in this driver are for the adc channels which are part of
> PMIC chip.
>>>>
>>>> I haven't immediately followed what this change is actually doing.
>>>>
>>>> I 'think' the point here is to not apply the calibration to
>>>> the raw adc counts when a true raw read is requested?
>>>>
>>> When a true raw read is requested .,Scaling is not applied.
>>>> There are several unconnected looking changes in here...
>>>>>
>>>>> Signed-off-by: Rama Krishna Phani A <rphani-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>>>>> ---
>>>>>  drivers/iio/adc/qcom-spmi-vadc.c | 54 +++++++++++++++++++++-------------------
>>>>>  1 file changed, 28 insertions(+), 26 deletions(-)
>>>>>
>>>>> diff --git a/drivers/iio/adc/qcom-spmi-vadc.c b/drivers/iio/adc/qcom-spmi-vadc.c
>>>>> index c2babe5..ff4d549 100644
>>>>> --- a/drivers/iio/adc/qcom-spmi-vadc.c
>>>>> +++ b/drivers/iio/adc/qcom-spmi-vadc.c
>>>>> @@ -1,5 +1,5 @@
>>>>>  /*
>>>>> - * Copyright (c) 2012-2014, The Linux Foundation. All rights reserved.
>>>>> + * Copyright (c) 2012-2016, The Linux Foundation. All rights reserved.
>>>>>   *
>>>>>   * This program is free software; you can redistribute it and/or modify
>>>>>   * it under the terms of the GNU General Public License version 2 and
>>>>> @@ -84,7 +84,7 @@
>>>>>  #define VADC_MAX_ADC_CODE            0xa800
>>>>>
>>>>>  #define VADC_ABSOLUTE_RANGE_UV            625000
>>>>> -#define VADC_RATIOMETRIC_RANGE_UV        1800000
>>>>> +#define VADC_RATIOMETRIC_RANGE            1800
>>>>>
>>>>>  #define VADC_DEF_PRESCALING            0 /* 1:1 */
>>>>>  #define VADC_DEF_DECIMATION            0 /* 512 */
>>>>> @@ -418,7 +418,7 @@ static int vadc_measure_ref_points(struct vadc_priv *vadc)
>>>>>      u16 read_1, read_2;
>>>>>      int ret;
>>>>>
>>>>> -    vadc->graph[VADC_CALIB_RATIOMETRIC].dx = VADC_RATIOMETRIC_RANGE_UV;
>>>>> +    vadc->graph[VADC_CALIB_RATIOMETRIC].dx = VADC_RATIOMETRIC_RANGE;
>>>>>      vadc->graph[VADC_CALIB_ABSOLUTE].dx = VADC_ABSOLUTE_RANGE_UV;
>>>>>
>>>>>      prop = vadc_get_channel(vadc, VADC_REF_1250MV);
>>>>> @@ -468,21 +468,30 @@ static int vadc_measure_ref_points(struct vadc_priv *vadc)
>>>>>      return ret;
>>>>>  }
>>>>>
>>>>> -static s32 vadc_calibrate(struct vadc_priv *vadc,
>>>>> -              const struct vadc_channel_prop *prop, u16 adc_code)
>>>>> +static void vadc_scale_calib(struct vadc_priv *vadc, u16 adc_code,
>>>>> +                 const struct vadc_channel_prop *prop,
>>>>> +                 s64 *scale_voltage)
>>>>>  {
>>>>> -    const struct vadc_prescale_ratio *prescale;
>>>>> -    s64 voltage;
>>>>> +    *scale_voltage = (adc_code -
>>>>> +        vadc->graph[prop->calibration].gnd);
>>>>> +    *scale_voltage *= vadc->graph[prop->calibration].dx;
>>>>> +    *scale_voltage = div64_s64(*scale_voltage,
>>>>> +        vadc->graph[prop->calibration].dy);
>>>>> +    if (prop->calibration == VADC_CALIB_ABSOLUTE)
>>>>> +        *scale_voltage +=
>>>>> +        vadc->graph[prop->calibration].dx;
>>>>>
>>>>> -    voltage = adc_code - vadc->graph[prop->calibration].gnd;
>>>>> -    voltage *= vadc->graph[prop->calibration].dx;
>>>>> -    voltage = div64_s64(voltage, vadc->graph[prop->calibration].dy);
>>>>> +    if (*scale_voltage < 0)
>>>>> +        *scale_voltage = 0;
>>>>> +}
>>>>>
>>>>> -    if (prop->calibration == VADC_CALIB_ABSOLUTE)
>>>>> -        voltage += vadc->graph[prop->calibration].dx;
>>>>> +static s64 vadc_scale_fn(struct vadc_priv *vadc,
>>>>> +             const struct vadc_channel_prop *prop, u16 adc_code)
>>>>> +{
>>>>> +    const struct vadc_prescale_ratio *prescale;
>>>>> +    s64 voltage = 0;
>>>>>
>>>>> -    if (voltage < 0)
>>>>> -        voltage = 0;
>>>>> +    vadc_scale_calib(vadc, adc_code, prop, &voltage);
>>>>>
>>>>>      prescale = &vadc_prescale_ratios[prop->prescale];
>>>>>
>>>>> @@ -552,11 +561,8 @@ static int vadc_read_raw(struct iio_dev *indio_dev,
>>>>>          if (ret)
>>>>>              break;
>>>>>
>>>>> -        *val = vadc_calibrate(vadc, prop, adc_code);
>>>>> +        *val = vadc_scale_fn(vadc, prop, adc_code);
>>>>>
>>>>> -        /* 2mV/K, return milli Celsius */
>>>>> -        *val /= 2;
>>>>> -        *val -= KELVINMIL_CELSIUSMIL;
>>>>>          return IIO_VAL_INT;
>>>>>      case IIO_CHAN_INFO_RAW:
>>>>>          prop = &vadc->chan_props[chan->address];
>>>>> @@ -564,12 +570,8 @@ static int vadc_read_raw(struct iio_dev *indio_dev,
>>>>>          if (ret)
>>>>>              break;
>>>>>
>>>>> -        *val = vadc_calibrate(vadc, prop, adc_code);
>>>>> +        *val = (int)adc_code;
>>>>>          return IIO_VAL_INT;
>>>> So this is 'more raw'.
>>> Yes., its raw value.
>>>>> -    case IIO_CHAN_INFO_SCALE:
>>>>> -        *val = 0;
>>>>> -        *val2 = 1000;
>>>>> -        return IIO_VAL_INT_PLUS_MICRO;
>>>>>      default:
>>>>>          ret = -EINVAL;
>>>>>          break;
>>>>> @@ -616,8 +618,8 @@ struct vadc_channels {
>>>>>      VADC_CHAN(_dname, IIO_TEMP, BIT(IIO_CHAN_INFO_PROCESSED), _pre)    \
>>>>>
>>>>>  #define VADC_CHAN_VOLT(_dname, _pre)                    \
>>>>> -    VADC_CHAN(_dname, IIO_VOLTAGE,                    \
>>>>> -          BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),    \
>>>>> +    VADC_CHAN(_dname, IIO_VOLTAGE,                \
>>>>> +          BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_PROCESSED),\
>>>>>            _pre)                            \
>>>> It very unusual to report both raw and processed values.  Please explain
>>>> why that is needed here?  It may be valid to maintain backwards compatibility
>>>> of ABI. Which would be fine. However if I read the above correctly you are
>>>> changing what comes out of reading the raw value so the ABI just changed...
>>>>
>>> With the help of IIO sysfs ., we can read the ADC channel readings
>>> either in RAW format or in processed format. There are two separate
>>> individual entries to read the ADC channel either in Raw format or in
>>> processed format. Most of the clients for ADC expect the readings in
>>> processed format.
>> If we are talking in kernel, that is worked out by the application of
>> scale.  The IIO in kernel interfaces will do this automatically.
>>
>> I we are talking in userspace, then the userspace needs to be
>> extended to support raw and scale reading.
> 
> Every channel present in adc has an unique conversion formula for
> obtained voltage, suggested by Hardware designers.
> 
> Ex: For die_temp channel., Temp = 2mv/Kelvin Above formula has to be
> applied on obtained voltage in order for the channel to report the
> temperature in milldegC.
> 
> Like wise every channel has unique way of conversion logic suggested
> by HW folks. That conversion logic is done in ADC driver.
If it is linear then ideally expose it as a raw channel and offset + scale.
If non linear then a processed channel with the conversion logic in kernel
as we have no means of describing it to userspace.

> 
>>>>
>>
>>>>>  /*
>>>>> @@ -850,9 +852,9 @@ static int vadc_get_dt_data(struct vadc_priv *vadc, struct device_node *node)
>>>>>
>>>>>          iio_chan->channel = prop.channel;
>>>>>          iio_chan->datasheet_name = vadc_chan->datasheet_name;
>>>>> +        iio_chan->extend_name = child->name;
>>>> What's this change?
>>> We can choose how we want to display our adc channel entries in sysfs. Am using the child node name to be displayed as the sysfs entry rather than channel number for easy interpretation.
>>>
>>> For ex: for vcoin(coin battery voltage channel.,) with this change it appears like below in iio adc sysfs
>>>
>>> "in_voltage_vcoin_input"
>> No.  This introduces a mass of undocumented (and uncontrolled) ABI.
>> If there are reasons to add such a label then it should not be done
>> in the file name.
> 
> "extended_name" is an existing field in "iio_chan_spec" structure,
> present in iio.h(include\linux\iio) and has documentation regarding
> the functionality. Pasting it here for quick reference.> 
>  * @extend_name:Allows labeling of channel attributes with an
>  *        informative name. Note this has no effect codes etc,
>  *        unlike modifiers.

> 
> Am trying to use the existing field here., initializing it with a
> value which is easy for interpretation of channel attributes.
OK. I'd misunderstood what was going on here.  If and only if these
channels are internally linked to a particular voltage / temperature
sensor etc it 'may' make sense.

If they are linked to a hardware monitoring channel then ideally we
would also be mapping them across to hwmon through the iio_hwmon
bridge.
>>>
>>>>>          iio_chan->info_mask_separate = vadc_chan->info_mask;
>>>>>          iio_chan->type = vadc_chan->type;
>>>>> -        iio_chan->indexed = 1;
>>>> Or for that matter this one...
>>> reason explained above.
No they still need to be indexed.  If we ever have events etc
on these channels or want to 'consume' them in other kernel drivers
they need to be indexed.  The strings are not available.  They are
just an convenience in naming of channels (and one I'm kind of
wishing we had never provided as it leads to uncontrolled ABI
explosion like here).

Anyhow, the key thing that wasn't clear in this patch description
and left me confused is that the scaling previously reported was
simply wrong and this was fixing it!

You do have several overlapping changes here which confused matters
further.  This extend_name stuff for example is a different
issue.  Your big problem is that it is an ABI change and hence
a non starter at this point.  Fixing wrong scaling is one thing
but changing the naming of sysfs files like this is not going to
be possible now the driver has been out there for a while.

Jonathan
>>>>>          iio_chan->address = index++;
>>>>>
>>>>>          iio_chan++;
>>>>>
>>>>
>>>
>>> Thanks,
>>> Ramakrishna
>>>
>>> ---
>>> This email has been checked for viruses by Avast antivirus software.
>>> https://www.avast.com/antivirus
>>>
>>> -- 
>>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
> 
> Thanks,
> Ramakrishna

Re: [PATCH V3 1/2] iio: adc: spmi-vadc: Update changes to support reporting of Raw adc code.

[Jonathan Cameron]

On 02/11/16 13:12, Phani A, Rama Krishna wrote:
> Hi Jonathan,
> 
> On 02-Nov-16 12:00 AM, Jonathan Cameron wrote:
>> On 31/10/16 08:48, Rama Krishna Phani A wrote:
>>> Hi Jonathan,
>>>
>>> On 30-Oct-16 11:05 PM, Jonathan Cameron wrote:
>>>> On 26/10/16 15:41, Rama Krishna Phani A wrote:
>>>>> Logic to convert adc code to voltage is generic across all channels.
>>>>> Different scaling can be done on the obtained voltage to report in physical
>>>>> units. Implement separate function for generic conversion logic.
>>>>> Scaling functionality can be changed per channel. Update changes to support
>>>>> reporting of Raw adc code.
>>>> Pleas rewrite this description.  Perhaps give examples of the changes
>>>> it makes to what is read from the various attributes?
>>> There are several channels in the ADC of PMIC which can be used to
>>> measure voltage, temperature, current etc., Hardware provides
>>> readings for all channels in adc code. That adc code needs to be
>>> converted to voltage. The logic for conversion of adc code to voltage
>>> is common for all ADC channels(voltage, temperature and current
>>> .,etc). Once voltage is obtained ., scaling is done on that voltage.
>>>
>>> For Ex., Thermal SW wants to know the temperature of thermistor on
>>> PMIC and it expects the temperature to be reported in millidegC. ADC
>>> channel is used to read the adc code and convert it to voltage. Once
>>> the voltage is available based on the thermistor spec that voltage is
>>> mapped to a temperature and then that value is reported to Thermal
>>> SW.
>>>
>>> Mapping of voltage to temperature is called scaling for that channel
>>> and scaling function can be different per channel based on how the
>>> voltage is reported.
>> Is the thermistor always part of the device? (i.e. in the chip) in which
>> case this might be fine.  If it's external then it needs to be described
>> by a separate device which acts as a consumer of the IIO channel and
>> in turn provides the scaled output to thermal.
>>
>> The thermistor should really be separately described.   This is already
>> done in drivers/hwmon/ntc_thermistor
>>
>> Are any of these scalings characteristics of the chip supported by
>> this driver, or are they the result of external hardware?
> 
> All the VADC channels i.e., Voltage, temperature(thermistors and
> other channels) are part of PMIC chip. The scaling functionalities
> supported in this driver are for the adc channels which are part of
> PMIC chip.
>>>>
>>>> I haven't immediately followed what this change is actually doing.
>>>>
>>>> I 'think' the point here is to not apply the calibration to
>>>> the raw adc counts when a true raw read is requested?
>>>>
>>> When a true raw read is requested .,Scaling is not applied.
>>>> There are several unconnected looking changes in here...
>>>>>
>>>>> Signed-off-by: Rama Krishna Phani A <rphani@codeaurora.org>
>>>>> ---
>>>>>  drivers/iio/adc/qcom-spmi-vadc.c | 54 +++++++++++++++++++++-------------------
>>>>>  1 file changed, 28 insertions(+), 26 deletions(-)
>>>>>
>>>>> diff --git a/drivers/iio/adc/qcom-spmi-vadc.c b/drivers/iio/adc/qcom-spmi-vadc.c
>>>>> index c2babe5..ff4d549 100644
>>>>> --- a/drivers/iio/adc/qcom-spmi-vadc.c
>>>>> +++ b/drivers/iio/adc/qcom-spmi-vadc.c
>>>>> @@ -1,5 +1,5 @@
>>>>>  /*
>>>>> - * Copyright (c) 2012-2014, The Linux Foundation. All rights reserved.
>>>>> + * Copyright (c) 2012-2016, The Linux Foundation. All rights reserved.
>>>>>   *
>>>>>   * This program is free software; you can redistribute it and/or modify
>>>>>   * it under the terms of the GNU General Public License version 2 and
>>>>> @@ -84,7 +84,7 @@
>>>>>  #define VADC_MAX_ADC_CODE            0xa800
>>>>>
>>>>>  #define VADC_ABSOLUTE_RANGE_UV            625000
>>>>> -#define VADC_RATIOMETRIC_RANGE_UV        1800000
>>>>> +#define VADC_RATIOMETRIC_RANGE            1800
>>>>>
>>>>>  #define VADC_DEF_PRESCALING            0 /* 1:1 */
>>>>>  #define VADC_DEF_DECIMATION            0 /* 512 */
>>>>> @@ -418,7 +418,7 @@ static int vadc_measure_ref_points(struct vadc_priv *vadc)
>>>>>      u16 read_1, read_2;
>>>>>      int ret;
>>>>>
>>>>> -    vadc->graph[VADC_CALIB_RATIOMETRIC].dx = VADC_RATIOMETRIC_RANGE_UV;
>>>>> +    vadc->graph[VADC_CALIB_RATIOMETRIC].dx = VADC_RATIOMETRIC_RANGE;
>>>>>      vadc->graph[VADC_CALIB_ABSOLUTE].dx = VADC_ABSOLUTE_RANGE_UV;
>>>>>
>>>>>      prop = vadc_get_channel(vadc, VADC_REF_1250MV);
>>>>> @@ -468,21 +468,30 @@ static int vadc_measure_ref_points(struct vadc_priv *vadc)
>>>>>      return ret;
>>>>>  }
>>>>>
>>>>> -static s32 vadc_calibrate(struct vadc_priv *vadc,
>>>>> -              const struct vadc_channel_prop *prop, u16 adc_code)
>>>>> +static void vadc_scale_calib(struct vadc_priv *vadc, u16 adc_code,
>>>>> +                 const struct vadc_channel_prop *prop,
>>>>> +                 s64 *scale_voltage)
>>>>>  {
>>>>> -    const struct vadc_prescale_ratio *prescale;
>>>>> -    s64 voltage;
>>>>> +    *scale_voltage = (adc_code -
>>>>> +        vadc->graph[prop->calibration].gnd);
>>>>> +    *scale_voltage *= vadc->graph[prop->calibration].dx;
>>>>> +    *scale_voltage = div64_s64(*scale_voltage,
>>>>> +        vadc->graph[prop->calibration].dy);
>>>>> +    if (prop->calibration == VADC_CALIB_ABSOLUTE)
>>>>> +        *scale_voltage +=
>>>>> +        vadc->graph[prop->calibration].dx;
>>>>>
>>>>> -    voltage = adc_code - vadc->graph[prop->calibration].gnd;
>>>>> -    voltage *= vadc->graph[prop->calibration].dx;
>>>>> -    voltage = div64_s64(voltage, vadc->graph[prop->calibration].dy);
>>>>> +    if (*scale_voltage < 0)
>>>>> +        *scale_voltage = 0;
>>>>> +}
>>>>>
>>>>> -    if (prop->calibration == VADC_CALIB_ABSOLUTE)
>>>>> -        voltage += vadc->graph[prop->calibration].dx;
>>>>> +static s64 vadc_scale_fn(struct vadc_priv *vadc,
>>>>> +             const struct vadc_channel_prop *prop, u16 adc_code)
>>>>> +{
>>>>> +    const struct vadc_prescale_ratio *prescale;
>>>>> +    s64 voltage = 0;
>>>>>
>>>>> -    if (voltage < 0)
>>>>> -        voltage = 0;
>>>>> +    vadc_scale_calib(vadc, adc_code, prop, &voltage);
>>>>>
>>>>>      prescale = &vadc_prescale_ratios[prop->prescale];
>>>>>
>>>>> @@ -552,11 +561,8 @@ static int vadc_read_raw(struct iio_dev *indio_dev,
>>>>>          if (ret)
>>>>>              break;
>>>>>
>>>>> -        *val = vadc_calibrate(vadc, prop, adc_code);
>>>>> +        *val = vadc_scale_fn(vadc, prop, adc_code);
>>>>>
>>>>> -        /* 2mV/K, return milli Celsius */
>>>>> -        *val /= 2;
>>>>> -        *val -= KELVINMIL_CELSIUSMIL;
>>>>>          return IIO_VAL_INT;
>>>>>      case IIO_CHAN_INFO_RAW:
>>>>>          prop = &vadc->chan_props[chan->address];
>>>>> @@ -564,12 +570,8 @@ static int vadc_read_raw(struct iio_dev *indio_dev,
>>>>>          if (ret)
>>>>>              break;
>>>>>
>>>>> -        *val = vadc_calibrate(vadc, prop, adc_code);
>>>>> +        *val = (int)adc_code;
>>>>>          return IIO_VAL_INT;
>>>> So this is 'more raw'.
>>> Yes., its raw value.
>>>>> -    case IIO_CHAN_INFO_SCALE:
>>>>> -        *val = 0;
>>>>> -        *val2 = 1000;
>>>>> -        return IIO_VAL_INT_PLUS_MICRO;
>>>>>      default:
>>>>>          ret = -EINVAL;
>>>>>          break;
>>>>> @@ -616,8 +618,8 @@ struct vadc_channels {
>>>>>      VADC_CHAN(_dname, IIO_TEMP, BIT(IIO_CHAN_INFO_PROCESSED), _pre)    \
>>>>>
>>>>>  #define VADC_CHAN_VOLT(_dname, _pre)                    \
>>>>> -    VADC_CHAN(_dname, IIO_VOLTAGE,                    \
>>>>> -          BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),    \
>>>>> +    VADC_CHAN(_dname, IIO_VOLTAGE,                \
>>>>> +          BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_PROCESSED),\
>>>>>            _pre)                            \
>>>> It very unusual to report both raw and processed values.  Please explain
>>>> why that is needed here?  It may be valid to maintain backwards compatibility
>>>> of ABI. Which would be fine. However if I read the above correctly you are
>>>> changing what comes out of reading the raw value so the ABI just changed...
>>>>
>>> With the help of IIO sysfs ., we can read the ADC channel readings
>>> either in RAW format or in processed format. There are two separate
>>> individual entries to read the ADC channel either in Raw format or in
>>> processed format. Most of the clients for ADC expect the readings in
>>> processed format.
>> If we are talking in kernel, that is worked out by the application of
>> scale.  The IIO in kernel interfaces will do this automatically.
>>
>> I we are talking in userspace, then the userspace needs to be
>> extended to support raw and scale reading.
> 
> Every channel present in adc has an unique conversion formula for
> obtained voltage, suggested by Hardware designers.
> 
> Ex: For die_temp channel., Temp = 2mv/Kelvin Above formula has to be
> applied on obtained voltage in order for the channel to report the
> temperature in milldegC.
> 
> Like wise every channel has unique way of conversion logic suggested
> by HW folks. That conversion logic is done in ADC driver.
If it is linear then ideally expose it as a raw channel and offset + scale.
If non linear then a processed channel with the conversion logic in kernel
as we have no means of describing it to userspace.

> 
>>>>
>>
>>>>>  /*
>>>>> @@ -850,9 +852,9 @@ static int vadc_get_dt_data(struct vadc_priv *vadc, struct device_node *node)
>>>>>
>>>>>          iio_chan->channel = prop.channel;
>>>>>          iio_chan->datasheet_name = vadc_chan->datasheet_name;
>>>>> +        iio_chan->extend_name = child->name;
>>>> What's this change?
>>> We can choose how we want to display our adc channel entries in sysfs. Am using the child node name to be displayed as the sysfs entry rather than channel number for easy interpretation.
>>>
>>> For ex: for vcoin(coin battery voltage channel.,) with this change it appears like below in iio adc sysfs
>>>
>>> "in_voltage_vcoin_input"
>> No.  This introduces a mass of undocumented (and uncontrolled) ABI.
>> If there are reasons to add such a label then it should not be done
>> in the file name.
> 
> "extended_name" is an existing field in "iio_chan_spec" structure,
> present in iio.h(include\linux\iio) and has documentation regarding
> the functionality. Pasting it here for quick reference.> 
>  * @extend_name:Allows labeling of channel attributes with an
>  *        informative name. Note this has no effect codes etc,
>  *        unlike modifiers.

> 
> Am trying to use the existing field here., initializing it with a
> value which is easy for interpretation of channel attributes.
OK. I'd misunderstood what was going on here.  If and only if these
channels are internally linked to a particular voltage / temperature
sensor etc it 'may' make sense.

If they are linked to a hardware monitoring channel then ideally we
would also be mapping them across to hwmon through the iio_hwmon
bridge.
>>>
>>>>>          iio_chan->info_mask_separate = vadc_chan->info_mask;
>>>>>          iio_chan->type = vadc_chan->type;
>>>>> -        iio_chan->indexed = 1;
>>>> Or for that matter this one...
>>> reason explained above.
No they still need to be indexed.  If we ever have events etc
on these channels or want to 'consume' them in other kernel drivers
they need to be indexed.  The strings are not available.  They are
just an convenience in naming of channels (and one I'm kind of
wishing we had never provided as it leads to uncontrolled ABI
explosion like here).

Anyhow, the key thing that wasn't clear in this patch description
and left me confused is that the scaling previously reported was
simply wrong and this was fixing it!

You do have several overlapping changes here which confused matters
further.  This extend_name stuff for example is a different
issue.  Your big problem is that it is an ABI change and hence
a non starter at this point.  Fixing wrong scaling is one thing
but changing the naming of sysfs files like this is not going to
be possible now the driver has been out there for a while.

Jonathan
>>>>>          iio_chan->address = index++;
>>>>>
>>>>>          iio_chan++;
>>>>>
>>>>
>>>
>>> Thanks,
>>> Ramakrishna
>>>
>>> ---
>>> This email has been checked for viruses by Avast antivirus software.
>>> https://www.avast.com/antivirus
>>>
>>> -- 
>>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
> 
> Thanks,
> Ramakrishna

Re: [PATCH V3 1/2] iio: adc: spmi-vadc: Update changes to support reporting of Raw adc code.

[Phani A, Rama Krishna]

Hi Jonathan,

On 05-Nov-16 9:29 PM, Jonathan Cameron wrote:
> On 02/11/16 13:12, Phani A, Rama Krishna wrote:
>> Hi Jonathan,
>>
>> On 02-Nov-16 12:00 AM, Jonathan Cameron wrote:
>>> On 31/10/16 08:48, Rama Krishna Phani A wrote:
>>>> Hi Jonathan,
>>>>
>>>> On 30-Oct-16 11:05 PM, Jonathan Cameron wrote:
>>>>> On 26/10/16 15:41, Rama Krishna Phani A wrote:
>>>>>> Logic to convert adc code to voltage is generic across all channels.
>>>>>> Different scaling can be done on the obtained voltage to report in physical
>>>>>> units. Implement separate function for generic conversion logic.
>>>>>> Scaling functionality can be changed per channel. Update changes to support
>>>>>> reporting of Raw adc code.
>>>>> Pleas rewrite this description.  Perhaps give examples of the changes
>>>>> it makes to what is read from the various attributes?
>>>> There are several channels in the ADC of PMIC which can be used to
>>>> measure voltage, temperature, current etc., Hardware provides
>>>> readings for all channels in adc code. That adc code needs to be
>>>> converted to voltage. The logic for conversion of adc code to voltage
>>>> is common for all ADC channels(voltage, temperature and current
>>>> .,etc). Once voltage is obtained ., scaling is done on that voltage.
>>>>
>>>> For Ex., Thermal SW wants to know the temperature of thermistor on
>>>> PMIC and it expects the temperature to be reported in millidegC. ADC
>>>> channel is used to read the adc code and convert it to voltage. Once
>>>> the voltage is available based on the thermistor spec that voltage is
>>>> mapped to a temperature and then that value is reported to Thermal
>>>> SW.
>>>>
>>>> Mapping of voltage to temperature is called scaling for that channel
>>>> and scaling function can be different per channel based on how the
>>>> voltage is reported.
>>> Is the thermistor always part of the device? (i.e. in the chip) in which
>>> case this might be fine.  If it's external then it needs to be described
>>> by a separate device which acts as a consumer of the IIO channel and
>>> in turn provides the scaled output to thermal.
>>>
>>> The thermistor should really be separately described.   This is already
>>> done in drivers/hwmon/ntc_thermistor
>>>
>>> Are any of these scalings characteristics of the chip supported by
>>> this driver, or are they the result of external hardware?
>>
>> All the VADC channels i.e., Voltage, temperature(thermistors and
>> other channels) are part of PMIC chip. The scaling functionalities
>> supported in this driver are for the adc channels which are part of
>> PMIC chip.
>>>>>
>>>>> I haven't immediately followed what this change is actually doing.
>>>>>
>>>>> I 'think' the point here is to not apply the calibration to
>>>>> the raw adc counts when a true raw read is requested?
>>>>>
>>>> When a true raw read is requested .,Scaling is not applied.
>>>>> There are several unconnected looking changes in here...
>>>>>>
>>>>>> Signed-off-by: Rama Krishna Phani A <rphani@codeaurora.org>
>>>>>> ---
>>>>>>  drivers/iio/adc/qcom-spmi-vadc.c | 54 +++++++++++++++++++++-------------------
>>>>>>  1 file changed, 28 insertions(+), 26 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/iio/adc/qcom-spmi-vadc.c b/drivers/iio/adc/qcom-spmi-vadc.c
>>>>>> index c2babe5..ff4d549 100644
>>>>>> --- a/drivers/iio/adc/qcom-spmi-vadc.c
>>>>>> +++ b/drivers/iio/adc/qcom-spmi-vadc.c
>>>>>> @@ -1,5 +1,5 @@
>>>>>>  /*
>>>>>> - * Copyright (c) 2012-2014, The Linux Foundation. All rights reserved.
>>>>>> + * Copyright (c) 2012-2016, The Linux Foundation. All rights reserved.
>>>>>>   *
>>>>>>   * This program is free software; you can redistribute it and/or modify
>>>>>>   * it under the terms of the GNU General Public License version 2 and
>>>>>> @@ -84,7 +84,7 @@
>>>>>>  #define VADC_MAX_ADC_CODE            0xa800
>>>>>>
>>>>>>  #define VADC_ABSOLUTE_RANGE_UV            625000
>>>>>> -#define VADC_RATIOMETRIC_RANGE_UV        1800000
>>>>>> +#define VADC_RATIOMETRIC_RANGE            1800
>>>>>>
>>>>>>  #define VADC_DEF_PRESCALING            0 /* 1:1 */
>>>>>>  #define VADC_DEF_DECIMATION            0 /* 512 */
>>>>>> @@ -418,7 +418,7 @@ static int vadc_measure_ref_points(struct vadc_priv *vadc)
>>>>>>      u16 read_1, read_2;
>>>>>>      int ret;
>>>>>>
>>>>>> -    vadc->graph[VADC_CALIB_RATIOMETRIC].dx = VADC_RATIOMETRIC_RANGE_UV;
>>>>>> +    vadc->graph[VADC_CALIB_RATIOMETRIC].dx = VADC_RATIOMETRIC_RANGE;
>>>>>>      vadc->graph[VADC_CALIB_ABSOLUTE].dx = VADC_ABSOLUTE_RANGE_UV;
>>>>>>
>>>>>>      prop = vadc_get_channel(vadc, VADC_REF_1250MV);
>>>>>> @@ -468,21 +468,30 @@ static int vadc_measure_ref_points(struct vadc_priv *vadc)
>>>>>>      return ret;
>>>>>>  }
>>>>>>
>>>>>> -static s32 vadc_calibrate(struct vadc_priv *vadc,
>>>>>> -              const struct vadc_channel_prop *prop, u16 adc_code)
>>>>>> +static void vadc_scale_calib(struct vadc_priv *vadc, u16 adc_code,
>>>>>> +                 const struct vadc_channel_prop *prop,
>>>>>> +                 s64 *scale_voltage)
>>>>>>  {
>>>>>> -    const struct vadc_prescale_ratio *prescale;
>>>>>> -    s64 voltage;
>>>>>> +    *scale_voltage = (adc_code -
>>>>>> +        vadc->graph[prop->calibration].gnd);
>>>>>> +    *scale_voltage *= vadc->graph[prop->calibration].dx;
>>>>>> +    *scale_voltage = div64_s64(*scale_voltage,
>>>>>> +        vadc->graph[prop->calibration].dy);
>>>>>> +    if (prop->calibration == VADC_CALIB_ABSOLUTE)
>>>>>> +        *scale_voltage +=
>>>>>> +        vadc->graph[prop->calibration].dx;
>>>>>>
>>>>>> -    voltage = adc_code - vadc->graph[prop->calibration].gnd;
>>>>>> -    voltage *= vadc->graph[prop->calibration].dx;
>>>>>> -    voltage = div64_s64(voltage, vadc->graph[prop->calibration].dy);
>>>>>> +    if (*scale_voltage < 0)
>>>>>> +        *scale_voltage = 0;
>>>>>> +}
>>>>>>
>>>>>> -    if (prop->calibration == VADC_CALIB_ABSOLUTE)
>>>>>> -        voltage += vadc->graph[prop->calibration].dx;
>>>>>> +static s64 vadc_scale_fn(struct vadc_priv *vadc,
>>>>>> +             const struct vadc_channel_prop *prop, u16 adc_code)
>>>>>> +{
>>>>>> +    const struct vadc_prescale_ratio *prescale;
>>>>>> +    s64 voltage = 0;
>>>>>>
>>>>>> -    if (voltage < 0)
>>>>>> -        voltage = 0;
>>>>>> +    vadc_scale_calib(vadc, adc_code, prop, &voltage);
>>>>>>
>>>>>>      prescale = &vadc_prescale_ratios[prop->prescale];
>>>>>>
>>>>>> @@ -552,11 +561,8 @@ static int vadc_read_raw(struct iio_dev *indio_dev,
>>>>>>          if (ret)
>>>>>>              break;
>>>>>>
>>>>>> -        *val = vadc_calibrate(vadc, prop, adc_code);
>>>>>> +        *val = vadc_scale_fn(vadc, prop, adc_code);
>>>>>>
>>>>>> -        /* 2mV/K, return milli Celsius */
>>>>>> -        *val /= 2;
>>>>>> -        *val -= KELVINMIL_CELSIUSMIL;
>>>>>>          return IIO_VAL_INT;
>>>>>>      case IIO_CHAN_INFO_RAW:
>>>>>>          prop = &vadc->chan_props[chan->address];
>>>>>> @@ -564,12 +570,8 @@ static int vadc_read_raw(struct iio_dev *indio_dev,
>>>>>>          if (ret)
>>>>>>              break;
>>>>>>
>>>>>> -        *val = vadc_calibrate(vadc, prop, adc_code);
>>>>>> +        *val = (int)adc_code;
>>>>>>          return IIO_VAL_INT;
>>>>> So this is 'more raw'.
>>>> Yes., its raw value.
>>>>>> -    case IIO_CHAN_INFO_SCALE:
>>>>>> -        *val = 0;
>>>>>> -        *val2 = 1000;
>>>>>> -        return IIO_VAL_INT_PLUS_MICRO;
>>>>>>      default:
>>>>>>          ret = -EINVAL;
>>>>>>          break;
>>>>>> @@ -616,8 +618,8 @@ struct vadc_channels {
>>>>>>      VADC_CHAN(_dname, IIO_TEMP, BIT(IIO_CHAN_INFO_PROCESSED), _pre)    \
>>>>>>
>>>>>>  #define VADC_CHAN_VOLT(_dname, _pre)                    \
>>>>>> -    VADC_CHAN(_dname, IIO_VOLTAGE,                    \
>>>>>> -          BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),    \
>>>>>> +    VADC_CHAN(_dname, IIO_VOLTAGE,                \
>>>>>> +          BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_PROCESSED),\
>>>>>>            _pre)                            \
>>>>> It very unusual to report both raw and processed values.  Please explain
>>>>> why that is needed here?  It may be valid to maintain backwards compatibility
>>>>> of ABI. Which would be fine. However if I read the above correctly you are
>>>>> changing what comes out of reading the raw value so the ABI just changed...
>>>>>
>>>> With the help of IIO sysfs ., we can read the ADC channel readings
>>>> either in RAW format or in processed format. There are two separate
>>>> individual entries to read the ADC channel either in Raw format or in
>>>> processed format. Most of the clients for ADC expect the readings in
>>>> processed format.
>>> If we are talking in kernel, that is worked out by the application of
>>> scale.  The IIO in kernel interfaces will do this automatically.
>>>
>>> I we are talking in userspace, then the userspace needs to be
>>> extended to support raw and scale reading.
>>
>> Every channel present in adc has an unique conversion formula for
>> obtained voltage, suggested by Hardware designers.
>>
>> Ex: For die_temp channel., Temp = 2mv/Kelvin Above formula has to be
>> applied on obtained voltage in order for the channel to report the
>> temperature in milldegC.
>>
>> Like wise every channel has unique way of conversion logic suggested
>> by HW folks. That conversion logic is done in ADC driver.
> If it is linear then ideally expose it as a raw channel and offset + scale.
> If non linear then a processed channel with the conversion logic in kernel
> as we have no means of describing it to userspace.
Yes ., these channels are non linear and hence they needs to be processed.
>
>>
>>>>>
>>>
>>>>>>  /*
>>>>>> @@ -850,9 +852,9 @@ static int vadc_get_dt_data(struct vadc_priv *vadc, struct device_node *node)
>>>>>>
>>>>>>          iio_chan->channel = prop.channel;
>>>>>>          iio_chan->datasheet_name = vadc_chan->datasheet_name;
>>>>>> +        iio_chan->extend_name = child->name;
>>>>> What's this change?
>>>> We can choose how we want to display our adc channel entries in sysfs. Am using the child node name to be displayed as the sysfs entry rather than channel number for easy interpretation.
>>>>
>>>> For ex: for vcoin(coin battery voltage channel.,) with this change it appears like below in iio adc sysfs
>>>>
>>>> "in_voltage_vcoin_input"
>>> No.  This introduces a mass of undocumented (and uncontrolled) ABI.
>>> If there are reasons to add such a label then it should not be done
>>> in the file name.
>>
>> "extended_name" is an existing field in "iio_chan_spec" structure,
>> present in iio.h(include\linux\iio) and has documentation regarding
>> the functionality. Pasting it here for quick reference.>
>>  * @extend_name:Allows labeling of channel attributes with an
>>  *        informative name. Note this has no effect codes etc,
>>  *        unlike modifiers.
>
>>
>> Am trying to use the existing field here., initializing it with a
>> value which is easy for interpretation of channel attributes.
> OK. I'd misunderstood what was going on here.  If and only if these
> channels are internally linked to a particular voltage / temperature
> sensor etc it 'may' make sense.
>
> If they are linked to a hardware monitoring channel then ideally we
> would also be mapping them across to hwmon through the iio_hwmon
> bridge.
>>>>
>>>>>>          iio_chan->info_mask_separate = vadc_chan->info_mask;
>>>>>>          iio_chan->type = vadc_chan->type;
>>>>>> -        iio_chan->indexed = 1;
>>>>> Or for that matter this one...
>>>> reason explained above.
> No they still need to be indexed.  If we ever have events etc
> on these channels or want to 'consume' them in other kernel drivers
> they need to be indexed.  The strings are not available.  They are
> just an convenience in naming of channels (and one I'm kind of
> wishing we had never provided as it leads to uncontrolled ABI
> explosion like here).
>
> Anyhow, the key thing that wasn't clear in this patch description
> and left me confused is that the scaling previously reported was
> simply wrong and this was fixing it!
>
> You do have several overlapping changes here which confused matters
> further.  This extend_name stuff for example is a different
> issue.  Your big problem is that it is an ABI change and hence
> a non starter at this point.  Fixing wrong scaling is one thing
> but changing the naming of sysfs files like this is not going to
> be possible now the driver has been out there for a while.
>
Ok ., will retain the indexed change in next patch.
> Jonathan
>>>>>>          iio_chan->address = index++;
>>>>>>
>>>>>>          iio_chan++;
>>>>>>
>>>>>
>>>>
>>>> Thanks,
>>>> Ramakrishna
>>>>
>>>> ---
>>>> This email has been checked for viruses by Avast antivirus software.
>>>> https://www.avast.com/antivirus
>>>>
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>>>> the body of a message to majordomo@vger.kernel.org
>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>
>>
>> Thanks,
>> Ramakrishna
>
Thanks,
Ramakrishna

[PATCH V3 2/2] iio: adc: spmi-vadc: Changes to support different scaling

[Rama Krishna Phani A]

Polling can also be used for End of conversion completion. Implement logic
to choose either polling or interrupt for End of conversion completion.
Scaling can be done on the voltage to report adc code in physical units.
Add changes to support different scale functions to convert adc code to
physical units.

Signed-off-by: Rama Krishna Phani A <rphani-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
---
 .../devicetree/bindings/iio/adc/qcom,spmi-vadc.txt |  14 ++
 drivers/iio/adc/qcom-spmi-vadc.c                   | 263 +++++++++++++++++----
 2 files changed, 236 insertions(+), 41 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt
index 0fb4613..39e31c0e 100644
--- a/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt
+++ b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt
@@ -37,6 +37,12 @@ VADC node:
     Value type: <prop-encoded-array>
     Definition: End of conversion interrupt.
 
+- qcom,vadc-poll-eoc:
+    Usage: optional
+    Value type: <bool>
+    Definition: Use polling instead of interrupts for End of Conversion
+		completion.
+
 Channel node properties:
 
 - reg:
@@ -92,6 +98,14 @@ Channel node properties:
             Valid values are: 1, 2, 4, 8, 16, 32, 64, 128, 256, 512
             If property is not found, 1 sample will be used.
 
+- qcom,scale-function:
+    Usage: optional
+    Value type: <u32>
+    Definition: Scaling function used to convert raw ADC code to
+	units specific to a given channel. Scaled units can be
+	microvolts, millidegC.Valid values are: 0, 1, 2, 3, 4.
+	If property is not found, 0 scaling will be used.
+
 NOTE:
 
 Following channels, also known as reference point channels, are used for
diff --git a/drivers/iio/adc/qcom-spmi-vadc.c b/drivers/iio/adc/qcom-spmi-vadc.c
index ff4d549..6e521a9 100644
--- a/drivers/iio/adc/qcom-spmi-vadc.c
+++ b/drivers/iio/adc/qcom-spmi-vadc.c
@@ -92,6 +92,8 @@
 #define VADC_DEF_AVG_SAMPLES			0 /* 1 sample */
 #define VADC_DEF_CALIB_TYPE			VADC_CALIB_ABSOLUTE
 
+#define VADC_DEF_SCALE_FN			SCALE_DEFAULT
+
 #define VADC_DECIMATION_MIN			512
 #define VADC_DECIMATION_MAX			4096
 
@@ -100,9 +102,43 @@
 
 #define KELVINMIL_CELSIUSMIL			273150
 
+#define PMI_CHG_SCALE_1				-138890
+#define PMI_CHG_SCALE_2				391750000000
+
 #define VADC_CHAN_MIN			VADC_USBIN
 #define VADC_CHAN_MAX			VADC_LR_MUX3_BUF_PU1_PU2_XO_THERM
 
+/**
+ * enum vadc_scale_fn_type - Scaling function to convert ADC code to
+ *				physical scaled units for the channel.
+ * %SCALE_DEFAULT: Default scaling to convert raw adc code to voltage (uV).
+ * %SCALE_THERM_100K_PULLUP: Returns temperature in millidegC.
+ *				 Uses a mapping table with 100K pullup.
+ * %SCALE_PMIC_THERM: Returns result in milli degree's Centigrade.
+ * %SCALE_XOTHERM: Returns XO thermistor voltage in millidegC.
+ * %SCALE_PMI_CHG_TEMP: Conversion for PMI CHG temp
+ * %SCALE_NONE: Do not use this scaling type.
+ */
+enum vadc_scale_fn_type {
+	SCALE_DEFAULT = 0,
+	SCALE_THERM_100K_PULLUP,
+	SCALE_PMIC_THERM,
+	SCALE_XOTHERM,
+	SCALE_PMI_CHG_TEMP,
+	SCALE_NONE,
+};
+
+/**
+ * struct vadc_map_pt - Map the graph representation for ADC channel
+ * @x: Represent the ADC digitized code.
+ * @y: Represent the physical data which can be temperature, voltage,
+ *     resistance.
+ */
+struct vadc_map_pt {
+	s32 x;
+	s32 y;
+};
+
 /*
  * VADC_CALIB_ABSOLUTE: uses the 625mV and 1.25V as reference channels.
  * VADC_CALIB_RATIOMETRIC: uses the reference voltage (1.8V) and GND for
@@ -148,6 +184,9 @@ struct vadc_prescale_ratio {
  *	start of conversion.
  * @avg_samples: ability to provide single result from the ADC
  *	that is an average of multiple measurements.
+ *@scale_function: Represents the scaling function to convert voltage
+ *	physical units desired by the client for the channel.
+ *	Referenced from enum vadc_scale_fn_type.
  */
 struct vadc_channel_prop {
 	unsigned int channel;
@@ -156,6 +195,7 @@ struct vadc_channel_prop {
 	unsigned int prescale;
 	unsigned int hw_settle_time;
 	unsigned int avg_samples;
+	unsigned int scale_function;
 };
 
 /**
@@ -197,6 +237,44 @@ struct vadc_priv {
 	{.num =  1, .den = 10}
 };
 
+/* Voltage to temperature */
+static const struct vadc_map_pt adcmap_100k_104ef_104fb[] = {
+	{1758,	-40},
+	{1742,	-35},
+	{1719,	-30},
+	{1691,	-25},
+	{1654,	-20},
+	{1608,	-15},
+	{1551,	-10},
+	{1483,	-5},
+	{1404,	0},
+	{1315,	5},
+	{1218,	10},
+	{1114,	15},
+	{1007,	20},
+	{900,	25},
+	{795,	30},
+	{696,	35},
+	{605,	40},
+	{522,	45},
+	{448,	50},
+	{383,	55},
+	{327,	60},
+	{278,	65},
+	{237,	70},
+	{202,	75},
+	{172,	80},
+	{146,	85},
+	{125,	90},
+	{107,	95},
+	{92,	100},
+	{79,	105},
+	{68,	110},
+	{59,	115},
+	{51,	120},
+	{44,	125}
+};
+
 static int vadc_read(struct vadc_priv *vadc, u16 offset, u8 *data)
 {
 	return regmap_bulk_read(vadc->regmap, vadc->base + offset, data, 1);
@@ -468,6 +546,51 @@ static int vadc_measure_ref_points(struct vadc_priv *vadc)
 	return ret;
 }
 
+static int vadc_map_voltage_temp(const struct vadc_map_pt *pts,
+				 u32 tablesize, s32 input, s64 *output)
+{
+	bool descending = 1;
+	u32 i = 0;
+
+	if (!pts)
+		return -EINVAL;
+
+	/* Check if table is descending or ascending */
+	if (tablesize > 1) {
+		if (pts[0].x < pts[1].x)
+			descending = 0;
+	}
+
+	while (i < tablesize) {
+		if ((descending) && (pts[i].x < input)) {
+			/* table entry is less than measured*/
+			 /* value and table is descending, stop */
+			break;
+		} else if ((!descending) &&
+				(pts[i].x > input)) {
+			/* table entry is greater than measured*/
+			/*value and table is ascending, stop */
+			break;
+		}
+		i++;
+	}
+
+	if (i == 0) {
+		*output = pts[0].y;
+	} else if (i == tablesize) {
+		*output = pts[tablesize - 1].y;
+	} else {
+		/* result is between search_index and search_index-1 */
+		/* interpolate linearly */
+		*output = (((s32)((pts[i].y - pts[i - 1].y) *
+			(input - pts[i - 1].x)) /
+			(pts[i].x - pts[i - 1].x)) +
+			pts[i - 1].y);
+	}
+
+	return 0;
+}
+
 static void vadc_scale_calib(struct vadc_priv *vadc, u16 adc_code,
 			     const struct vadc_channel_prop *prop,
 			     s64 *scale_voltage)
@@ -489,15 +612,61 @@ static s64 vadc_scale_fn(struct vadc_priv *vadc,
 			 const struct vadc_channel_prop *prop, u16 adc_code)
 {
 	const struct vadc_prescale_ratio *prescale;
-	s64 voltage = 0;
+	s64 voltage = 0, result = 0;
+	int ret;
+
+	switch (prop->scale_function) {
+	case SCALE_DEFAULT:
+		vadc_scale_calib(vadc, adc_code, prop, &voltage);
+
+		prescale = &vadc_prescale_ratios[prop->prescale];
+		voltage = voltage * prescale->den;
+		return div64_s64(voltage, prescale->num);
+
+	case SCALE_THERM_100K_PULLUP:
+	case SCALE_XOTHERM:
+		vadc_scale_calib(vadc, adc_code, prop, &voltage);
+
+		if (prop->calibration == VADC_CALIB_ABSOLUTE)
+			voltage /= 1000;
+
+		vadc_map_voltage_temp(adcmap_100k_104ef_104fb,
+				      ARRAY_SIZE(adcmap_100k_104ef_104fb),
+				      voltage, &result);
+		result *= 1000;
+		return result;
+
+	case SCALE_PMIC_THERM:
+		vadc_scale_calib(vadc, adc_code, prop, &voltage);
+
+		if (voltage > 0) {
+			prescale = &vadc_prescale_ratios[prop->prescale];
+			voltage = voltage * prescale->den;
+			voltage /= (prescale->num * 2);
+		} else {
+			voltage = 0;
+		}
+
+		voltage -= KELVINMIL_CELSIUSMIL;
 
-	vadc_scale_calib(vadc, adc_code, prop, &voltage);
+		return voltage;
 
-	prescale = &vadc_prescale_ratios[prop->prescale];
+	case SCALE_PMI_CHG_TEMP:
+		vadc_scale_calib(vadc, adc_code, prop, &voltage);
+		prescale = &vadc_prescale_ratios[prop->prescale];
+		voltage = voltage * prescale->den;
 
-	voltage = voltage * prescale->den;
+		voltage = div64_s64(voltage, prescale->num);
+		voltage = ((PMI_CHG_SCALE_1) * (voltage * 2));
+		voltage = (voltage + PMI_CHG_SCALE_2);
+		return div64_s64(voltage, 1000000);
 
-	return div64_s64(voltage, prescale->num);
+	default:
+		ret = -EINVAL;
+		break;
+	}
+
+	return ret;
 }
 
 static int vadc_decimation_from_dt(u32 value)
@@ -615,7 +784,9 @@ struct vadc_channels {
 	},								\
 
 #define VADC_CHAN_TEMP(_dname, _pre)					\
-	VADC_CHAN(_dname, IIO_TEMP, BIT(IIO_CHAN_INFO_PROCESSED), _pre)	\
+	VADC_CHAN(_dname, IIO_TEMP,	\
+		BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_PROCESSED), \
+		_pre)	\
 
 #define VADC_CHAN_VOLT(_dname, _pre)					\
 	VADC_CHAN(_dname, IIO_VOLTAGE,				\
@@ -639,7 +810,7 @@ struct vadc_channels {
 	VADC_CHAN_TEMP(DIE_TEMP, 0)
 	VADC_CHAN_VOLT(REF_625MV, 0)
 	VADC_CHAN_VOLT(REF_1250MV, 0)
-	VADC_CHAN_VOLT(CHG_TEMP, 0)
+	VADC_CHAN_TEMP(CHG_TEMP, 0)
 	VADC_CHAN_VOLT(SPARE1, 0)
 	VADC_CHAN_VOLT(SPARE2, 0)
 	VADC_CHAN_VOLT(GND_REF, 0)
@@ -693,41 +864,41 @@ struct vadc_channels {
 	VADC_CHAN_VOLT(AMUX_PU2, 0)
 	VADC_CHAN_VOLT(LR_MUX3_BUF_XO_THERM, 0)
 
-	VADC_CHAN_VOLT(LR_MUX1_PU1_BAT_THERM, 0)
+	VADC_CHAN_TEMP(LR_MUX1_PU1_BAT_THERM, 0)
 	VADC_CHAN_VOLT(LR_MUX2_PU1_BAT_ID, 0)
-	VADC_CHAN_VOLT(LR_MUX3_PU1_XO_THERM, 0)
-	VADC_CHAN_VOLT(LR_MUX4_PU1_AMUX_THM1, 0)
-	VADC_CHAN_VOLT(LR_MUX5_PU1_AMUX_THM2, 0)
-	VADC_CHAN_VOLT(LR_MUX6_PU1_AMUX_THM3, 0)
+	VADC_CHAN_TEMP(LR_MUX3_PU1_XO_THERM, 0)
+	VADC_CHAN_TEMP(LR_MUX4_PU1_AMUX_THM1, 0)
+	VADC_CHAN_TEMP(LR_MUX5_PU1_AMUX_THM2, 0)
+	VADC_CHAN_TEMP(LR_MUX6_PU1_AMUX_THM3, 0)
 	VADC_CHAN_VOLT(LR_MUX7_PU1_AMUX_HW_ID, 0)
-	VADC_CHAN_VOLT(LR_MUX8_PU1_AMUX_THM4, 0)
-	VADC_CHAN_VOLT(LR_MUX9_PU1_AMUX_THM5, 0)
+	VADC_CHAN_TEMP(LR_MUX8_PU1_AMUX_THM4, 0)
+	VADC_CHAN_TEMP(LR_MUX9_PU1_AMUX_THM5, 0)
 	VADC_CHAN_VOLT(LR_MUX10_PU1_AMUX_USB_ID, 0)
-	VADC_CHAN_VOLT(LR_MUX3_BUF_PU1_XO_THERM, 0)
+	VADC_CHAN_TEMP(LR_MUX3_BUF_PU1_XO_THERM, 0)
 
-	VADC_CHAN_VOLT(LR_MUX1_PU2_BAT_THERM, 0)
+	VADC_CHAN_TEMP(LR_MUX1_PU2_BAT_THERM, 0)
 	VADC_CHAN_VOLT(LR_MUX2_PU2_BAT_ID, 0)
-	VADC_CHAN_VOLT(LR_MUX3_PU2_XO_THERM, 0)
-	VADC_CHAN_VOLT(LR_MUX4_PU2_AMUX_THM1, 0)
-	VADC_CHAN_VOLT(LR_MUX5_PU2_AMUX_THM2, 0)
-	VADC_CHAN_VOLT(LR_MUX6_PU2_AMUX_THM3, 0)
+	VADC_CHAN_TEMP(LR_MUX3_PU2_XO_THERM, 0)
+	VADC_CHAN_TEMP(LR_MUX4_PU2_AMUX_THM1, 0)
+	VADC_CHAN_TEMP(LR_MUX5_PU2_AMUX_THM2, 0)
+	VADC_CHAN_TEMP(LR_MUX6_PU2_AMUX_THM3, 0)
 	VADC_CHAN_VOLT(LR_MUX7_PU2_AMUX_HW_ID, 0)
-	VADC_CHAN_VOLT(LR_MUX8_PU2_AMUX_THM4, 0)
-	VADC_CHAN_VOLT(LR_MUX9_PU2_AMUX_THM5, 0)
+	VADC_CHAN_TEMP(LR_MUX8_PU2_AMUX_THM4, 0)
+	VADC_CHAN_TEMP(LR_MUX9_PU2_AMUX_THM5, 0)
 	VADC_CHAN_VOLT(LR_MUX10_PU2_AMUX_USB_ID, 0)
-	VADC_CHAN_VOLT(LR_MUX3_BUF_PU2_XO_THERM, 0)
+	VADC_CHAN_TEMP(LR_MUX3_BUF_PU2_XO_THERM, 0)
 
-	VADC_CHAN_VOLT(LR_MUX1_PU1_PU2_BAT_THERM, 0)
+	VADC_CHAN_TEMP(LR_MUX1_PU1_PU2_BAT_THERM, 0)
 	VADC_CHAN_VOLT(LR_MUX2_PU1_PU2_BAT_ID, 0)
-	VADC_CHAN_VOLT(LR_MUX3_PU1_PU2_XO_THERM, 0)
-	VADC_CHAN_VOLT(LR_MUX4_PU1_PU2_AMUX_THM1, 0)
-	VADC_CHAN_VOLT(LR_MUX5_PU1_PU2_AMUX_THM2, 0)
-	VADC_CHAN_VOLT(LR_MUX6_PU1_PU2_AMUX_THM3, 0)
+	VADC_CHAN_TEMP(LR_MUX3_PU1_PU2_XO_THERM, 0)
+	VADC_CHAN_TEMP(LR_MUX4_PU1_PU2_AMUX_THM1, 0)
+	VADC_CHAN_TEMP(LR_MUX5_PU1_PU2_AMUX_THM2, 0)
+	VADC_CHAN_TEMP(LR_MUX6_PU1_PU2_AMUX_THM3, 0)
 	VADC_CHAN_VOLT(LR_MUX7_PU1_PU2_AMUX_HW_ID, 0)
-	VADC_CHAN_VOLT(LR_MUX8_PU1_PU2_AMUX_THM4, 0)
-	VADC_CHAN_VOLT(LR_MUX9_PU1_PU2_AMUX_THM5, 0)
+	VADC_CHAN_TEMP(LR_MUX8_PU1_PU2_AMUX_THM4, 0)
+	VADC_CHAN_TEMP(LR_MUX9_PU1_PU2_AMUX_THM5, 0)
 	VADC_CHAN_VOLT(LR_MUX10_PU1_PU2_AMUX_USB_ID, 0)
-	VADC_CHAN_VOLT(LR_MUX3_BUF_PU1_PU2_XO_THERM, 0)
+	VADC_CHAN_TEMP(LR_MUX3_BUF_PU1_PU2_XO_THERM, 0)
 };
 
 static int vadc_get_dt_channel_data(struct device *dev,
@@ -804,6 +975,11 @@ static int vadc_get_dt_channel_data(struct device *dev,
 		prop->avg_samples = VADC_DEF_AVG_SAMPLES;
 	}
 
+	ret = of_property_read_u32(node, "qcom,scale-function",
+				   &prop->scale_function);
+	if (ret)
+		prop->scale_function = SCALE_DEFAULT;
+
 	if (of_property_read_bool(node, "qcom,ratiometric"))
 		prop->calibration = VADC_CALIB_RATIOMETRIC;
 	else
@@ -966,16 +1142,21 @@ static int vadc_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	irq_eoc = platform_get_irq(pdev, 0);
-	if (irq_eoc < 0) {
-		if (irq_eoc == -EPROBE_DEFER || irq_eoc == -EINVAL)
-			return irq_eoc;
-		vadc->poll_eoc = true;
-	} else {
-		ret = devm_request_irq(dev, irq_eoc, vadc_isr, 0,
-				       "spmi-vadc", vadc);
-		if (ret)
-			return ret;
+	vadc->poll_eoc = of_property_read_bool(node,
+						"qcom,vadc-poll-eoc");
+
+	if (!vadc->poll_eoc) {
+		irq_eoc = platform_get_irq(pdev, 0);
+		if (irq_eoc < 0) {
+			if (irq_eoc == -EPROBE_DEFER || irq_eoc == -EINVAL)
+				return irq_eoc;
+			vadc->poll_eoc = true;
+		} else {
+			ret = devm_request_irq(dev, irq_eoc, vadc_isr, 0,
+					       "spmi-vadc", vadc);
+			if (ret)
+				return ret;
+		}
 	}
 
 	ret = vadc_reset(vadc);
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH V3 2/2] iio: adc: spmi-vadc: Changes to support different scaling

[Rama Krishna Phani A]

Polling can also be used for End of conversion completion. Implement logic
to choose either polling or interrupt for End of conversion completion.
Scaling can be done on the voltage to report adc code in physical units.
Add changes to support different scale functions to convert adc code to
physical units.

Signed-off-by: Rama Krishna Phani A <rphani@codeaurora.org>
---
 .../devicetree/bindings/iio/adc/qcom,spmi-vadc.txt |  14 ++
 drivers/iio/adc/qcom-spmi-vadc.c                   | 263 +++++++++++++++++----
 2 files changed, 236 insertions(+), 41 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt
index 0fb4613..39e31c0e 100644
--- a/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt
+++ b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt
@@ -37,6 +37,12 @@ VADC node:
     Value type: <prop-encoded-array>
     Definition: End of conversion interrupt.
 
+- qcom,vadc-poll-eoc:
+    Usage: optional
+    Value type: <bool>
+    Definition: Use polling instead of interrupts for End of Conversion
+		completion.
+
 Channel node properties:
 
 - reg:
@@ -92,6 +98,14 @@ Channel node properties:
             Valid values are: 1, 2, 4, 8, 16, 32, 64, 128, 256, 512
             If property is not found, 1 sample will be used.
 
+- qcom,scale-function:
+    Usage: optional
+    Value type: <u32>
+    Definition: Scaling function used to convert raw ADC code to
+	units specific to a given channel. Scaled units can be
+	microvolts, millidegC.Valid values are: 0, 1, 2, 3, 4.
+	If property is not found, 0 scaling will be used.
+
 NOTE:
 
 Following channels, also known as reference point channels, are used for
diff --git a/drivers/iio/adc/qcom-spmi-vadc.c b/drivers/iio/adc/qcom-spmi-vadc.c
index ff4d549..6e521a9 100644
--- a/drivers/iio/adc/qcom-spmi-vadc.c
+++ b/drivers/iio/adc/qcom-spmi-vadc.c
@@ -92,6 +92,8 @@
 #define VADC_DEF_AVG_SAMPLES			0 /* 1 sample */
 #define VADC_DEF_CALIB_TYPE			VADC_CALIB_ABSOLUTE
 
+#define VADC_DEF_SCALE_FN			SCALE_DEFAULT
+
 #define VADC_DECIMATION_MIN			512
 #define VADC_DECIMATION_MAX			4096
 
@@ -100,9 +102,43 @@
 
 #define KELVINMIL_CELSIUSMIL			273150
 
+#define PMI_CHG_SCALE_1				-138890
+#define PMI_CHG_SCALE_2				391750000000
+
 #define VADC_CHAN_MIN			VADC_USBIN
 #define VADC_CHAN_MAX			VADC_LR_MUX3_BUF_PU1_PU2_XO_THERM
 
+/**
+ * enum vadc_scale_fn_type - Scaling function to convert ADC code to
+ *				physical scaled units for the channel.
+ * %SCALE_DEFAULT: Default scaling to convert raw adc code to voltage (uV).
+ * %SCALE_THERM_100K_PULLUP: Returns temperature in millidegC.
+ *				 Uses a mapping table with 100K pullup.
+ * %SCALE_PMIC_THERM: Returns result in milli degree's Centigrade.
+ * %SCALE_XOTHERM: Returns XO thermistor voltage in millidegC.
+ * %SCALE_PMI_CHG_TEMP: Conversion for PMI CHG temp
+ * %SCALE_NONE: Do not use this scaling type.
+ */
+enum vadc_scale_fn_type {
+	SCALE_DEFAULT = 0,
+	SCALE_THERM_100K_PULLUP,
+	SCALE_PMIC_THERM,
+	SCALE_XOTHERM,
+	SCALE_PMI_CHG_TEMP,
+	SCALE_NONE,
+};
+
+/**
+ * struct vadc_map_pt - Map the graph representation for ADC channel
+ * @x: Represent the ADC digitized code.
+ * @y: Represent the physical data which can be temperature, voltage,
+ *     resistance.
+ */
+struct vadc_map_pt {
+	s32 x;
+	s32 y;
+};
+
 /*
  * VADC_CALIB_ABSOLUTE: uses the 625mV and 1.25V as reference channels.
  * VADC_CALIB_RATIOMETRIC: uses the reference voltage (1.8V) and GND for
@@ -148,6 +184,9 @@ struct vadc_prescale_ratio {
  *	start of conversion.
  * @avg_samples: ability to provide single result from the ADC
  *	that is an average of multiple measurements.
+ *@scale_function: Represents the scaling function to convert voltage
+ *	physical units desired by the client for the channel.
+ *	Referenced from enum vadc_scale_fn_type.
  */
 struct vadc_channel_prop {
 	unsigned int channel;
@@ -156,6 +195,7 @@ struct vadc_channel_prop {
 	unsigned int prescale;
 	unsigned int hw_settle_time;
 	unsigned int avg_samples;
+	unsigned int scale_function;
 };
 
 /**
@@ -197,6 +237,44 @@ struct vadc_priv {
 	{.num =  1, .den = 10}
 };
 
+/* Voltage to temperature */
+static const struct vadc_map_pt adcmap_100k_104ef_104fb[] = {
+	{1758,	-40},
+	{1742,	-35},
+	{1719,	-30},
+	{1691,	-25},
+	{1654,	-20},
+	{1608,	-15},
+	{1551,	-10},
+	{1483,	-5},
+	{1404,	0},
+	{1315,	5},
+	{1218,	10},
+	{1114,	15},
+	{1007,	20},
+	{900,	25},
+	{795,	30},
+	{696,	35},
+	{605,	40},
+	{522,	45},
+	{448,	50},
+	{383,	55},
+	{327,	60},
+	{278,	65},
+	{237,	70},
+	{202,	75},
+	{172,	80},
+	{146,	85},
+	{125,	90},
+	{107,	95},
+	{92,	100},
+	{79,	105},
+	{68,	110},
+	{59,	115},
+	{51,	120},
+	{44,	125}
+};
+
 static int vadc_read(struct vadc_priv *vadc, u16 offset, u8 *data)
 {
 	return regmap_bulk_read(vadc->regmap, vadc->base + offset, data, 1);
@@ -468,6 +546,51 @@ static int vadc_measure_ref_points(struct vadc_priv *vadc)
 	return ret;
 }
 
+static int vadc_map_voltage_temp(const struct vadc_map_pt *pts,
+				 u32 tablesize, s32 input, s64 *output)
+{
+	bool descending = 1;
+	u32 i = 0;
+
+	if (!pts)
+		return -EINVAL;
+
+	/* Check if table is descending or ascending */
+	if (tablesize > 1) {
+		if (pts[0].x < pts[1].x)
+			descending = 0;
+	}
+
+	while (i < tablesize) {
+		if ((descending) && (pts[i].x < input)) {
+			/* table entry is less than measured*/
+			 /* value and table is descending, stop */
+			break;
+		} else if ((!descending) &&
+				(pts[i].x > input)) {
+			/* table entry is greater than measured*/
+			/*value and table is ascending, stop */
+			break;
+		}
+		i++;
+	}
+
+	if (i == 0) {
+		*output = pts[0].y;
+	} else if (i == tablesize) {
+		*output = pts[tablesize - 1].y;
+	} else {
+		/* result is between search_index and search_index-1 */
+		/* interpolate linearly */
+		*output = (((s32)((pts[i].y - pts[i - 1].y) *
+			(input - pts[i - 1].x)) /
+			(pts[i].x - pts[i - 1].x)) +
+			pts[i - 1].y);
+	}
+
+	return 0;
+}
+
 static void vadc_scale_calib(struct vadc_priv *vadc, u16 adc_code,
 			     const struct vadc_channel_prop *prop,
 			     s64 *scale_voltage)
@@ -489,15 +612,61 @@ static s64 vadc_scale_fn(struct vadc_priv *vadc,
 			 const struct vadc_channel_prop *prop, u16 adc_code)
 {
 	const struct vadc_prescale_ratio *prescale;
-	s64 voltage = 0;
+	s64 voltage = 0, result = 0;
+	int ret;
+
+	switch (prop->scale_function) {
+	case SCALE_DEFAULT:
+		vadc_scale_calib(vadc, adc_code, prop, &voltage);
+
+		prescale = &vadc_prescale_ratios[prop->prescale];
+		voltage = voltage * prescale->den;
+		return div64_s64(voltage, prescale->num);
+
+	case SCALE_THERM_100K_PULLUP:
+	case SCALE_XOTHERM:
+		vadc_scale_calib(vadc, adc_code, prop, &voltage);
+
+		if (prop->calibration == VADC_CALIB_ABSOLUTE)
+			voltage /= 1000;
+
+		vadc_map_voltage_temp(adcmap_100k_104ef_104fb,
+				      ARRAY_SIZE(adcmap_100k_104ef_104fb),
+				      voltage, &result);
+		result *= 1000;
+		return result;
+
+	case SCALE_PMIC_THERM:
+		vadc_scale_calib(vadc, adc_code, prop, &voltage);
+
+		if (voltage > 0) {
+			prescale = &vadc_prescale_ratios[prop->prescale];
+			voltage = voltage * prescale->den;
+			voltage /= (prescale->num * 2);
+		} else {
+			voltage = 0;
+		}
+
+		voltage -= KELVINMIL_CELSIUSMIL;
 
-	vadc_scale_calib(vadc, adc_code, prop, &voltage);
+		return voltage;
 
-	prescale = &vadc_prescale_ratios[prop->prescale];
+	case SCALE_PMI_CHG_TEMP:
+		vadc_scale_calib(vadc, adc_code, prop, &voltage);
+		prescale = &vadc_prescale_ratios[prop->prescale];
+		voltage = voltage * prescale->den;
 
-	voltage = voltage * prescale->den;
+		voltage = div64_s64(voltage, prescale->num);
+		voltage = ((PMI_CHG_SCALE_1) * (voltage * 2));
+		voltage = (voltage + PMI_CHG_SCALE_2);
+		return div64_s64(voltage, 1000000);
 
-	return div64_s64(voltage, prescale->num);
+	default:
+		ret = -EINVAL;
+		break;
+	}
+
+	return ret;
 }
 
 static int vadc_decimation_from_dt(u32 value)
@@ -615,7 +784,9 @@ struct vadc_channels {
 	},								\
 
 #define VADC_CHAN_TEMP(_dname, _pre)					\
-	VADC_CHAN(_dname, IIO_TEMP, BIT(IIO_CHAN_INFO_PROCESSED), _pre)	\
+	VADC_CHAN(_dname, IIO_TEMP,	\
+		BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_PROCESSED), \
+		_pre)	\
 
 #define VADC_CHAN_VOLT(_dname, _pre)					\
 	VADC_CHAN(_dname, IIO_VOLTAGE,				\
@@ -639,7 +810,7 @@ struct vadc_channels {
 	VADC_CHAN_TEMP(DIE_TEMP, 0)
 	VADC_CHAN_VOLT(REF_625MV, 0)
 	VADC_CHAN_VOLT(REF_1250MV, 0)
-	VADC_CHAN_VOLT(CHG_TEMP, 0)
+	VADC_CHAN_TEMP(CHG_TEMP, 0)
 	VADC_CHAN_VOLT(SPARE1, 0)
 	VADC_CHAN_VOLT(SPARE2, 0)
 	VADC_CHAN_VOLT(GND_REF, 0)
@@ -693,41 +864,41 @@ struct vadc_channels {
 	VADC_CHAN_VOLT(AMUX_PU2, 0)
 	VADC_CHAN_VOLT(LR_MUX3_BUF_XO_THERM, 0)
 
-	VADC_CHAN_VOLT(LR_MUX1_PU1_BAT_THERM, 0)
+	VADC_CHAN_TEMP(LR_MUX1_PU1_BAT_THERM, 0)
 	VADC_CHAN_VOLT(LR_MUX2_PU1_BAT_ID, 0)
-	VADC_CHAN_VOLT(LR_MUX3_PU1_XO_THERM, 0)
-	VADC_CHAN_VOLT(LR_MUX4_PU1_AMUX_THM1, 0)
-	VADC_CHAN_VOLT(LR_MUX5_PU1_AMUX_THM2, 0)
-	VADC_CHAN_VOLT(LR_MUX6_PU1_AMUX_THM3, 0)
+	VADC_CHAN_TEMP(LR_MUX3_PU1_XO_THERM, 0)
+	VADC_CHAN_TEMP(LR_MUX4_PU1_AMUX_THM1, 0)
+	VADC_CHAN_TEMP(LR_MUX5_PU1_AMUX_THM2, 0)
+	VADC_CHAN_TEMP(LR_MUX6_PU1_AMUX_THM3, 0)
 	VADC_CHAN_VOLT(LR_MUX7_PU1_AMUX_HW_ID, 0)
-	VADC_CHAN_VOLT(LR_MUX8_PU1_AMUX_THM4, 0)
-	VADC_CHAN_VOLT(LR_MUX9_PU1_AMUX_THM5, 0)
+	VADC_CHAN_TEMP(LR_MUX8_PU1_AMUX_THM4, 0)
+	VADC_CHAN_TEMP(LR_MUX9_PU1_AMUX_THM5, 0)
 	VADC_CHAN_VOLT(LR_MUX10_PU1_AMUX_USB_ID, 0)
-	VADC_CHAN_VOLT(LR_MUX3_BUF_PU1_XO_THERM, 0)
+	VADC_CHAN_TEMP(LR_MUX3_BUF_PU1_XO_THERM, 0)
 
-	VADC_CHAN_VOLT(LR_MUX1_PU2_BAT_THERM, 0)
+	VADC_CHAN_TEMP(LR_MUX1_PU2_BAT_THERM, 0)
 	VADC_CHAN_VOLT(LR_MUX2_PU2_BAT_ID, 0)
-	VADC_CHAN_VOLT(LR_MUX3_PU2_XO_THERM, 0)
-	VADC_CHAN_VOLT(LR_MUX4_PU2_AMUX_THM1, 0)
-	VADC_CHAN_VOLT(LR_MUX5_PU2_AMUX_THM2, 0)
-	VADC_CHAN_VOLT(LR_MUX6_PU2_AMUX_THM3, 0)
+	VADC_CHAN_TEMP(LR_MUX3_PU2_XO_THERM, 0)
+	VADC_CHAN_TEMP(LR_MUX4_PU2_AMUX_THM1, 0)
+	VADC_CHAN_TEMP(LR_MUX5_PU2_AMUX_THM2, 0)
+	VADC_CHAN_TEMP(LR_MUX6_PU2_AMUX_THM3, 0)
 	VADC_CHAN_VOLT(LR_MUX7_PU2_AMUX_HW_ID, 0)
-	VADC_CHAN_VOLT(LR_MUX8_PU2_AMUX_THM4, 0)
-	VADC_CHAN_VOLT(LR_MUX9_PU2_AMUX_THM5, 0)
+	VADC_CHAN_TEMP(LR_MUX8_PU2_AMUX_THM4, 0)
+	VADC_CHAN_TEMP(LR_MUX9_PU2_AMUX_THM5, 0)
 	VADC_CHAN_VOLT(LR_MUX10_PU2_AMUX_USB_ID, 0)
-	VADC_CHAN_VOLT(LR_MUX3_BUF_PU2_XO_THERM, 0)
+	VADC_CHAN_TEMP(LR_MUX3_BUF_PU2_XO_THERM, 0)
 
-	VADC_CHAN_VOLT(LR_MUX1_PU1_PU2_BAT_THERM, 0)
+	VADC_CHAN_TEMP(LR_MUX1_PU1_PU2_BAT_THERM, 0)
 	VADC_CHAN_VOLT(LR_MUX2_PU1_PU2_BAT_ID, 0)
-	VADC_CHAN_VOLT(LR_MUX3_PU1_PU2_XO_THERM, 0)
-	VADC_CHAN_VOLT(LR_MUX4_PU1_PU2_AMUX_THM1, 0)
-	VADC_CHAN_VOLT(LR_MUX5_PU1_PU2_AMUX_THM2, 0)
-	VADC_CHAN_VOLT(LR_MUX6_PU1_PU2_AMUX_THM3, 0)
+	VADC_CHAN_TEMP(LR_MUX3_PU1_PU2_XO_THERM, 0)
+	VADC_CHAN_TEMP(LR_MUX4_PU1_PU2_AMUX_THM1, 0)
+	VADC_CHAN_TEMP(LR_MUX5_PU1_PU2_AMUX_THM2, 0)
+	VADC_CHAN_TEMP(LR_MUX6_PU1_PU2_AMUX_THM3, 0)
 	VADC_CHAN_VOLT(LR_MUX7_PU1_PU2_AMUX_HW_ID, 0)
-	VADC_CHAN_VOLT(LR_MUX8_PU1_PU2_AMUX_THM4, 0)
-	VADC_CHAN_VOLT(LR_MUX9_PU1_PU2_AMUX_THM5, 0)
+	VADC_CHAN_TEMP(LR_MUX8_PU1_PU2_AMUX_THM4, 0)
+	VADC_CHAN_TEMP(LR_MUX9_PU1_PU2_AMUX_THM5, 0)
 	VADC_CHAN_VOLT(LR_MUX10_PU1_PU2_AMUX_USB_ID, 0)
-	VADC_CHAN_VOLT(LR_MUX3_BUF_PU1_PU2_XO_THERM, 0)
+	VADC_CHAN_TEMP(LR_MUX3_BUF_PU1_PU2_XO_THERM, 0)
 };
 
 static int vadc_get_dt_channel_data(struct device *dev,
@@ -804,6 +975,11 @@ static int vadc_get_dt_channel_data(struct device *dev,
 		prop->avg_samples = VADC_DEF_AVG_SAMPLES;
 	}
 
+	ret = of_property_read_u32(node, "qcom,scale-function",
+				   &prop->scale_function);
+	if (ret)
+		prop->scale_function = SCALE_DEFAULT;
+
 	if (of_property_read_bool(node, "qcom,ratiometric"))
 		prop->calibration = VADC_CALIB_RATIOMETRIC;
 	else
@@ -966,16 +1142,21 @@ static int vadc_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	irq_eoc = platform_get_irq(pdev, 0);
-	if (irq_eoc < 0) {
-		if (irq_eoc == -EPROBE_DEFER || irq_eoc == -EINVAL)
-			return irq_eoc;
-		vadc->poll_eoc = true;
-	} else {
-		ret = devm_request_irq(dev, irq_eoc, vadc_isr, 0,
-				       "spmi-vadc", vadc);
-		if (ret)
-			return ret;
+	vadc->poll_eoc = of_property_read_bool(node,
+						"qcom,vadc-poll-eoc");
+
+	if (!vadc->poll_eoc) {
+		irq_eoc = platform_get_irq(pdev, 0);
+		if (irq_eoc < 0) {
+			if (irq_eoc == -EPROBE_DEFER || irq_eoc == -EINVAL)
+				return irq_eoc;
+			vadc->poll_eoc = true;
+		} else {
+			ret = devm_request_irq(dev, irq_eoc, vadc_isr, 0,
+					       "spmi-vadc", vadc);
+			if (ret)
+				return ret;
+		}
 	}
 
 	ret = vadc_reset(vadc);
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH V3 2/2] iio: adc: spmi-vadc: Changes to support different scaling

[Stanimir Varbanov]

Hi Rama,

On 10/26/2016 05:41 PM, Rama Krishna Phani A wrote:
> Polling can also be used for End of conversion completion. Implement logic
> to choose either polling or interrupt for End of conversion completion.
> Scaling can be done on the voltage to report adc code in physical units.
> Add changes to support different scale functions to convert adc code to
> physical units.
> 
> Signed-off-by: Rama Krishna Phani A <rphani-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> ---
>  .../devicetree/bindings/iio/adc/qcom,spmi-vadc.txt |  14 ++
>  drivers/iio/adc/qcom-spmi-vadc.c                   | 263 +++++++++++++++++----
>  2 files changed, 236 insertions(+), 41 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt
> index 0fb4613..39e31c0e 100644
> --- a/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt
> +++ b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt
> @@ -37,6 +37,12 @@ VADC node:
>      Value type: <prop-encoded-array>
>      Definition: End of conversion interrupt.
>  
> +- qcom,vadc-poll-eoc:
> +    Usage: optional
> +    Value type: <bool>
> +    Definition: Use polling instead of interrupts for End of Conversion
> +		completion.

Why you need to add such a flag in DT?

The DT should describe hardware details not how the driver will choose
pooling vs interrupt.

On which use-case you would prefer pooling?

> +
>  Channel node properties:
>  
>  - reg:
> @@ -92,6 +98,14 @@ Channel node properties:
>              Valid values are: 1, 2, 4, 8, 16, 32, 64, 128, 256, 512
>              If property is not found, 1 sample will be used.
>  
> +- qcom,scale-function:
> +    Usage: optional
> +    Value type: <u32>
> +    Definition: Scaling function used to convert raw ADC code to
> +	units specific to a given channel. Scaled units can be
> +	microvolts, millidegC.Valid values are: 0, 1, 2, 3, 4.
> +	If property is not found, 0 scaling will be used.

This shouldn't be in DT binding. Just select the scale function for each
channel in the driver based on compatible property.


-- 
regards,
Stan

Re: [PATCH V3 2/2] iio: adc: spmi-vadc: Changes to support different scaling

[Stanimir Varbanov]

Hi Rama,

On 10/26/2016 05:41 PM, Rama Krishna Phani A wrote:
> Polling can also be used for End of conversion completion. Implement logic
> to choose either polling or interrupt for End of conversion completion.
> Scaling can be done on the voltage to report adc code in physical units.
> Add changes to support different scale functions to convert adc code to
> physical units.
> 
> Signed-off-by: Rama Krishna Phani A <rphani@codeaurora.org>
> ---
>  .../devicetree/bindings/iio/adc/qcom,spmi-vadc.txt |  14 ++
>  drivers/iio/adc/qcom-spmi-vadc.c                   | 263 +++++++++++++++++----
>  2 files changed, 236 insertions(+), 41 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt
> index 0fb4613..39e31c0e 100644
> --- a/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt
> +++ b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt
> @@ -37,6 +37,12 @@ VADC node:
>      Value type: <prop-encoded-array>
>      Definition: End of conversion interrupt.
>  
> +- qcom,vadc-poll-eoc:
> +    Usage: optional
> +    Value type: <bool>
> +    Definition: Use polling instead of interrupts for End of Conversion
> +		completion.

Why you need to add such a flag in DT?

The DT should describe hardware details not how the driver will choose
pooling vs interrupt.

On which use-case you would prefer pooling?

> +
>  Channel node properties:
>  
>  - reg:
> @@ -92,6 +98,14 @@ Channel node properties:
>              Valid values are: 1, 2, 4, 8, 16, 32, 64, 128, 256, 512
>              If property is not found, 1 sample will be used.
>  
> +- qcom,scale-function:
> +    Usage: optional
> +    Value type: <u32>
> +    Definition: Scaling function used to convert raw ADC code to
> +	units specific to a given channel. Scaled units can be
> +	microvolts, millidegC.Valid values are: 0, 1, 2, 3, 4.
> +	If property is not found, 0 scaling will be used.

This shouldn't be in DT binding. Just select the scale function for each
channel in the driver based on compatible property.


-- 
regards,
Stan

Re: [PATCH V3 2/2] iio: adc: spmi-vadc: Changes to support different scaling

[Phani A, Rama Krishna]

Hi Stan,

On 27-Oct-16 4:48 PM, Stanimir Varbanov wrote:
> Hi Rama,
>
> On 10/26/2016 05:41 PM, Rama Krishna Phani A wrote:
>> Polling can also be used for End of conversion completion. Implement logic
>> to choose either polling or interrupt for End of conversion completion.
>> Scaling can be done on the voltage to report adc code in physical units.
>> Add changes to support different scale functions to convert adc code to
>> physical units.
>>
>> Signed-off-by: Rama Krishna Phani A <rphani-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>> ---
>>  .../devicetree/bindings/iio/adc/qcom,spmi-vadc.txt |  14 ++
>>  drivers/iio/adc/qcom-spmi-vadc.c                   | 263 +++++++++++++++++----
>>  2 files changed, 236 insertions(+), 41 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt
>> index 0fb4613..39e31c0e 100644
>> --- a/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt
>> +++ b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt
>> @@ -37,6 +37,12 @@ VADC node:
>>      Value type: <prop-encoded-array>
>>      Definition: End of conversion interrupt.
>>
>> +- qcom,vadc-poll-eoc:
>> +    Usage: optional
>> +    Value type: <bool>
>> +    Definition: Use polling instead of interrupts for End of Conversion
>> +		completion.
>
> Why you need to add such a flag in DT?
>
> The DT should describe hardware details not how the driver will choose
> pooling vs interrupt.
>
> On which use-case you would prefer pooling?
>

Few PMIC's support interrupt functionality for ADC where as few PMIC's 
dont support. Based on the functionality that is supported in hardware 
we choose whether to go for polling or for interrupt.

>> +
>>  Channel node properties:
>>
>>  - reg:
>> @@ -92,6 +98,14 @@ Channel node properties:
>>              Valid values are: 1, 2, 4, 8, 16, 32, 64, 128, 256, 512
>>              If property is not found, 1 sample will be used.
>>
>> +- qcom,scale-function:
>> +    Usage: optional
>> +    Value type: <u32>
>> +    Definition: Scaling function used to convert raw ADC code to
>> +	units specific to a given channel. Scaled units can be
>> +	microvolts, millidegC.Valid values are: 0, 1, 2, 3, 4.
>> +	If property is not found, 0 scaling will be used.
>
> This shouldn't be in DT binding. Just select the scale function for each
> channel in the driver based on compatible property.
>
>
Ok ., Will remove this binding from DT, implement logic in driver and 
will post next patch.

Thanks,
Ramakrishna

Re: [PATCH V3 2/2] iio: adc: spmi-vadc: Changes to support different scaling

[Phani A, Rama Krishna]

Hi Stan,

On 27-Oct-16 4:48 PM, Stanimir Varbanov wrote:
> Hi Rama,
>
> On 10/26/2016 05:41 PM, Rama Krishna Phani A wrote:
>> Polling can also be used for End of conversion completion. Implement logic
>> to choose either polling or interrupt for End of conversion completion.
>> Scaling can be done on the voltage to report adc code in physical units.
>> Add changes to support different scale functions to convert adc code to
>> physical units.
>>
>> Signed-off-by: Rama Krishna Phani A <rphani@codeaurora.org>
>> ---
>>  .../devicetree/bindings/iio/adc/qcom,spmi-vadc.txt |  14 ++
>>  drivers/iio/adc/qcom-spmi-vadc.c                   | 263 +++++++++++++++++----
>>  2 files changed, 236 insertions(+), 41 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt
>> index 0fb4613..39e31c0e 100644
>> --- a/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt
>> +++ b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt
>> @@ -37,6 +37,12 @@ VADC node:
>>      Value type: <prop-encoded-array>
>>      Definition: End of conversion interrupt.
>>
>> +- qcom,vadc-poll-eoc:
>> +    Usage: optional
>> +    Value type: <bool>
>> +    Definition: Use polling instead of interrupts for End of Conversion
>> +		completion.
>
> Why you need to add such a flag in DT?
>
> The DT should describe hardware details not how the driver will choose
> pooling vs interrupt.
>
> On which use-case you would prefer pooling?
>

Few PMIC's support interrupt functionality for ADC where as few PMIC's 
dont support. Based on the functionality that is supported in hardware 
we choose whether to go for polling or for interrupt.

>> +
>>  Channel node properties:
>>
>>  - reg:
>> @@ -92,6 +98,14 @@ Channel node properties:
>>              Valid values are: 1, 2, 4, 8, 16, 32, 64, 128, 256, 512
>>              If property is not found, 1 sample will be used.
>>
>> +- qcom,scale-function:
>> +    Usage: optional
>> +    Value type: <u32>
>> +    Definition: Scaling function used to convert raw ADC code to
>> +	units specific to a given channel. Scaled units can be
>> +	microvolts, millidegC.Valid values are: 0, 1, 2, 3, 4.
>> +	If property is not found, 0 scaling will be used.
>
> This shouldn't be in DT binding. Just select the scale function for each
> channel in the driver based on compatible property.
>
>
Ok ., Will remove this binding from DT, implement logic in driver and 
will post next patch.

Thanks,
Ramakrishna

Re: [PATCH V3 2/2] iio: adc: spmi-vadc: Changes to support different scaling

[Jonathan Cameron]

On 27/10/16 18:37, Phani A, Rama Krishna wrote:
> Hi Stan,
> 
> On 27-Oct-16 4:48 PM, Stanimir Varbanov wrote:
>> Hi Rama,
>>
>> On 10/26/2016 05:41 PM, Rama Krishna Phani A wrote:
>>> Polling can also be used for End of conversion completion. Implement logic
>>> to choose either polling or interrupt for End of conversion completion.
>>> Scaling can be done on the voltage to report adc code in physical units.
>>> Add changes to support different scale functions to convert adc code to
>>> physical units.
>>>
>>> Signed-off-by: Rama Krishna Phani A <rphani@codeaurora.org>
>>> ---
>>>  .../devicetree/bindings/iio/adc/qcom,spmi-vadc.txt |  14 ++
>>>  drivers/iio/adc/qcom-spmi-vadc.c                   | 263 +++++++++++++++++----
>>>  2 files changed, 236 insertions(+), 41 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt
>>> index 0fb4613..39e31c0e 100644
>>> --- a/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt
>>> +++ b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt
>>> @@ -37,6 +37,12 @@ VADC node:
>>>      Value type: <prop-encoded-array>
>>>      Definition: End of conversion interrupt.
>>>
>>> +- qcom,vadc-poll-eoc:
>>> +    Usage: optional
>>> +    Value type: <bool>
>>> +    Definition: Use polling instead of interrupts for End of Conversion
>>> +        completion.
>>
>> Why you need to add such a flag in DT?
>>
>> The DT should describe hardware details not how the driver will choose
>> pooling vs interrupt.
>>
>> On which use-case you would prefer pooling?
>>
> 
> Few PMIC's support interrupt functionality for ADC where as few
> PMIC's dont support. Based on the functionality that is supported in
> hardware we choose whether to go for polling or for interrupt.
Can't use the usual trick of an optional interrupt in DT?
If it's there we try to use it, if not then fall back to polling?


>>> +
>>>  Channel node properties:
>>>
>>>  - reg:
>>> @@ -92,6 +98,14 @@ Channel node properties:
>>>              Valid values are: 1, 2, 4, 8, 16, 32, 64, 128, 256, 512
>>>              If property is not found, 1 sample will be used.
>>>
>>> +- qcom,scale-function:
>>> +    Usage: optional
>>> +    Value type: <u32>
>>> +    Definition: Scaling function used to convert raw ADC code to
>>> +    units specific to a given channel. Scaled units can be
>>> +    microvolts, millidegC.Valid values are: 0, 1, 2, 3, 4.
>>> +    If property is not found, 0 scaling will be used.
>>
>> This shouldn't be in DT binding. Just select the scale function for each
>> channel in the driver based on compatible property.
>>
>>
> Ok ., Will remove this binding from DT, implement logic in driver and will post next patch.
> 
> Thanks,
> Ramakrishna
> 
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V3 2/2] iio: adc: spmi-vadc: Changes to support different scaling

[Rama Krishna Phani A]

Hi Jonathan,

On 30-Oct-16 10:43 PM, Jonathan Cameron wrote:
> On 27/10/16 18:37, Phani A, Rama Krishna wrote:
>> Hi Stan,
>>
>> On 27-Oct-16 4:48 PM, Stanimir Varbanov wrote:
>>> Hi Rama,
>>>
>>> On 10/26/2016 05:41 PM, Rama Krishna Phani A wrote:
>>>> Polling can also be used for End of conversion completion. Implement logic
>>>> to choose either polling or interrupt for End of conversion completion.
>>>> Scaling can be done on the voltage to report adc code in physical units.
>>>> Add changes to support different scale functions to convert adc code to
>>>> physical units.
>>>>
>>>> Signed-off-by: Rama Krishna Phani A <rphani-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>>>> ---
>>>>  .../devicetree/bindings/iio/adc/qcom,spmi-vadc.txt |  14 ++
>>>>  drivers/iio/adc/qcom-spmi-vadc.c                   | 263 +++++++++++++++++----
>>>>  2 files changed, 236 insertions(+), 41 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt
>>>> index 0fb4613..39e31c0e 100644
>>>> --- a/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt
>>>> +++ b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt
>>>> @@ -37,6 +37,12 @@ VADC node:
>>>>      Value type: <prop-encoded-array>
>>>>      Definition: End of conversion interrupt.
>>>>
>>>> +- qcom,vadc-poll-eoc:
>>>> +    Usage: optional
>>>> +    Value type: <bool>
>>>> +    Definition: Use polling instead of interrupts for End of Conversion
>>>> +        completion.
>>>
>>> Why you need to add such a flag in DT?
>>>
>>> The DT should describe hardware details not how the driver will choose
>>> pooling vs interrupt.
>>>
>>> On which use-case you would prefer pooling?
>>>
>>
>> Few PMIC's support interrupt functionality for ADC where as few
>> PMIC's dont support. Based on the functionality that is supported in
>> hardware we choose whether to go for polling or for interrupt.
> Can't use the usual trick of an optional interrupt in DT?
> If it's there we try to use it, if not then fall back to polling?
>
Ok., Will check this logic for implementation and will post next patch.

>
>>>> +
>>>>  Channel node properties:
>>>>
>>>>  - reg:
>>>> @@ -92,6 +98,14 @@ Channel node properties:
>>>>              Valid values are: 1, 2, 4, 8, 16, 32, 64, 128, 256, 512
>>>>              If property is not found, 1 sample will be used.
>>>>
>>>> +- qcom,scale-function:
>>>> +    Usage: optional
>>>> +    Value type: <u32>
>>>> +    Definition: Scaling function used to convert raw ADC code to
>>>> +    units specific to a given channel. Scaled units can be
>>>> +    microvolts, millidegC.Valid values are: 0, 1, 2, 3, 4.
>>>> +    If property is not found, 0 scaling will be used.
>>>
>>> This shouldn't be in DT binding. Just select the scale function for each
>>> channel in the driver based on compatible property.
>>>
>>>
>> Ok ., Will remove this binding from DT, implement logic in driver and will post next patch.
>>
>> Thanks,
>> Ramakrishna
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus

Re: [PATCH V3 2/2] iio: adc: spmi-vadc: Changes to support different scaling

[Rama Krishna Phani A]

Hi Jonathan,

On 30-Oct-16 10:43 PM, Jonathan Cameron wrote:
> On 27/10/16 18:37, Phani A, Rama Krishna wrote:
>> Hi Stan,
>>
>> On 27-Oct-16 4:48 PM, Stanimir Varbanov wrote:
>>> Hi Rama,
>>>
>>> On 10/26/2016 05:41 PM, Rama Krishna Phani A wrote:
>>>> Polling can also be used for End of conversion completion. Implement logic
>>>> to choose either polling or interrupt for End of conversion completion.
>>>> Scaling can be done on the voltage to report adc code in physical units.
>>>> Add changes to support different scale functions to convert adc code to
>>>> physical units.
>>>>
>>>> Signed-off-by: Rama Krishna Phani A <rphani@codeaurora.org>
>>>> ---
>>>>  .../devicetree/bindings/iio/adc/qcom,spmi-vadc.txt |  14 ++
>>>>  drivers/iio/adc/qcom-spmi-vadc.c                   | 263 +++++++++++++++++----
>>>>  2 files changed, 236 insertions(+), 41 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt
>>>> index 0fb4613..39e31c0e 100644
>>>> --- a/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt
>>>> +++ b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt
>>>> @@ -37,6 +37,12 @@ VADC node:
>>>>      Value type: <prop-encoded-array>
>>>>      Definition: End of conversion interrupt.
>>>>
>>>> +- qcom,vadc-poll-eoc:
>>>> +    Usage: optional
>>>> +    Value type: <bool>
>>>> +    Definition: Use polling instead of interrupts for End of Conversion
>>>> +        completion.
>>>
>>> Why you need to add such a flag in DT?
>>>
>>> The DT should describe hardware details not how the driver will choose
>>> pooling vs interrupt.
>>>
>>> On which use-case you would prefer pooling?
>>>
>>
>> Few PMIC's support interrupt functionality for ADC where as few
>> PMIC's dont support. Based on the functionality that is supported in
>> hardware we choose whether to go for polling or for interrupt.
> Can't use the usual trick of an optional interrupt in DT?
> If it's there we try to use it, if not then fall back to polling?
>
Ok., Will check this logic for implementation and will post next patch.

>
>>>> +
>>>>  Channel node properties:
>>>>
>>>>  - reg:
>>>> @@ -92,6 +98,14 @@ Channel node properties:
>>>>              Valid values are: 1, 2, 4, 8, 16, 32, 64, 128, 256, 512
>>>>              If property is not found, 1 sample will be used.
>>>>
>>>> +- qcom,scale-function:
>>>> +    Usage: optional
>>>> +    Value type: <u32>
>>>> +    Definition: Scaling function used to convert raw ADC code to
>>>> +    units specific to a given channel. Scaled units can be
>>>> +    microvolts, millidegC.Valid values are: 0, 1, 2, 3, 4.
>>>> +    If property is not found, 0 scaling will be used.
>>>
>>> This shouldn't be in DT binding. Just select the scale function for each
>>> channel in the driver based on compatible property.
>>>
>>>
>> Ok ., Will remove this binding from DT, implement logic in driver and will post next patch.
>>
>> Thanks,
>> Ramakrishna
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus

Re: [PATCH V3 2/2] iio: adc: spmi-vadc: Changes to support different scaling

[Stanimir Varbanov]

Hi Rama,

On 10/31/2016 09:12 AM, Rama Krishna Phani A wrote:
> Hi Jonathan,
> 
> On 30-Oct-16 10:43 PM, Jonathan Cameron wrote:
>> On 27/10/16 18:37, Phani A, Rama Krishna wrote:
>>> Hi Stan,
>>>
>>> On 27-Oct-16 4:48 PM, Stanimir Varbanov wrote:
>>>> Hi Rama,
>>>>
>>>> On 10/26/2016 05:41 PM, Rama Krishna Phani A wrote:
>>>>> Polling can also be used for End of conversion completion.
>>>>> Implement logic
>>>>> to choose either polling or interrupt for End of conversion
>>>>> completion.
>>>>> Scaling can be done on the voltage to report adc code in physical
>>>>> units.
>>>>> Add changes to support different scale functions to convert adc
>>>>> code to
>>>>> physical units.
>>>>>
>>>>> Signed-off-by: Rama Krishna Phani A <rphani@codeaurora.org>
>>>>> ---
>>>>>  .../devicetree/bindings/iio/adc/qcom,spmi-vadc.txt |  14 ++
>>>>>  drivers/iio/adc/qcom-spmi-vadc.c                   | 263
>>>>> +++++++++++++++++----
>>>>>  2 files changed, 236 insertions(+), 41 deletions(-)
>>>>>
>>>>> diff --git
>>>>> a/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt
>>>>> b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt
>>>>> index 0fb4613..39e31c0e 100644
>>>>> --- a/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt
>>>>> +++ b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt
>>>>> @@ -37,6 +37,12 @@ VADC node:
>>>>>      Value type: <prop-encoded-array>
>>>>>      Definition: End of conversion interrupt.
>>>>>
>>>>> +- qcom,vadc-poll-eoc:
>>>>> +    Usage: optional
>>>>> +    Value type: <bool>
>>>>> +    Definition: Use polling instead of interrupts for End of
>>>>> Conversion
>>>>> +        completion.
>>>>
>>>> Why you need to add such a flag in DT?
>>>>
>>>> The DT should describe hardware details not how the driver will choose
>>>> pooling vs interrupt.
>>>>
>>>> On which use-case you would prefer pooling?
>>>>
>>>
>>> Few PMIC's support interrupt functionality for ADC where as few
>>> PMIC's dont support. Based on the functionality that is supported in
>>> hardware we choose whether to go for polling or for interrupt.
>> Can't use the usual trick of an optional interrupt in DT?
>> If it's there we try to use it, if not then fall back to polling?
>>
> Ok., Will check this logic for implementation and will post next patch.

The interrupts DT property in binding doc is marked as optional already,
so I can't really understand what you are trying to achieve with this
new qcom,vadc-poll-eoc boolean property?

-- 
regards,
Stan

Re: [PATCH V3 2/2] iio: adc: spmi-vadc: Changes to support different scaling

[Phani A, Rama Krishna]

Hi Stan,

On 31-Oct-16 2:56 PM, Stanimir Varbanov wrote:
> Hi Rama,
>
> On 10/31/2016 09:12 AM, Rama Krishna Phani A wrote:
>> Hi Jonathan,
>>
>> On 30-Oct-16 10:43 PM, Jonathan Cameron wrote:
>>> On 27/10/16 18:37, Phani A, Rama Krishna wrote:
>>>> Hi Stan,
>>>>
>>>> On 27-Oct-16 4:48 PM, Stanimir Varbanov wrote:
>>>>> Hi Rama,
>>>>>
>>>>> On 10/26/2016 05:41 PM, Rama Krishna Phani A wrote:
>>>>>> Polling can also be used for End of conversion completion.
>>>>>> Implement logic
>>>>>> to choose either polling or interrupt for End of conversion
>>>>>> completion.
>>>>>> Scaling can be done on the voltage to report adc code in physical
>>>>>> units.
>>>>>> Add changes to support different scale functions to convert adc
>>>>>> code to
>>>>>> physical units.
>>>>>>
>>>>>> Signed-off-by: Rama Krishna Phani A <rphani@codeaurora.org>
>>>>>> ---
>>>>>>  .../devicetree/bindings/iio/adc/qcom,spmi-vadc.txt |  14 ++
>>>>>>  drivers/iio/adc/qcom-spmi-vadc.c                   | 263
>>>>>> +++++++++++++++++----
>>>>>>  2 files changed, 236 insertions(+), 41 deletions(-)
>>>>>>
>>>>>> diff --git
>>>>>> a/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt
>>>>>> b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt
>>>>>> index 0fb4613..39e31c0e 100644
>>>>>> --- a/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt
>>>>>> +++ b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt
>>>>>> @@ -37,6 +37,12 @@ VADC node:
>>>>>>      Value type: <prop-encoded-array>
>>>>>>      Definition: End of conversion interrupt.
>>>>>>
>>>>>> +- qcom,vadc-poll-eoc:
>>>>>> +    Usage: optional
>>>>>> +    Value type: <bool>
>>>>>> +    Definition: Use polling instead of interrupts for End of
>>>>>> Conversion
>>>>>> +        completion.
>>>>>
>>>>> Why you need to add such a flag in DT?
>>>>>
>>>>> The DT should describe hardware details not how the driver will choose
>>>>> pooling vs interrupt.
>>>>>
>>>>> On which use-case you would prefer pooling?
>>>>>
>>>>
>>>> Few PMIC's support interrupt functionality for ADC where as few
>>>> PMIC's dont support. Based on the functionality that is supported in
>>>> hardware we choose whether to go for polling or for interrupt.
>>> Can't use the usual trick of an optional interrupt in DT?
>>> If it's there we try to use it, if not then fall back to polling?
>>>
>> Ok., Will check this logic for implementation and will post next patch.
>
> The interrupts DT property in binding doc is marked as optional already,
> so I can't really understand what you are trying to achieve with this
> new qcom,vadc-poll-eoc boolean property?
>
Agree., interrupts property is sufficient for this functionality. Will 
remove this DT binding in next patch.

Thanks,
Ramakrishna

Re: [PATCH V3 2/2] iio: adc: spmi-vadc: Changes to support different scaling

[Jonathan Cameron]

On 26/10/16 15:41, Rama Krishna Phani A wrote:
> Polling can also be used for End of conversion completion. Implement logic
> to choose either polling or interrupt for End of conversion completion.
Why is this change in a patch with the title above?  Should be a separate patch.

> Scaling can be done on the voltage to report adc code in physical units.
> Add changes to support different scale functions to convert adc code to
> physical units.
> 
> Signed-off-by: Rama Krishna Phani A <rphani@codeaurora.org>
> ---
>  .../devicetree/bindings/iio/adc/qcom,spmi-vadc.txt |  14 ++
>  drivers/iio/adc/qcom-spmi-vadc.c                   | 263 +++++++++++++++++----
>  2 files changed, 236 insertions(+), 41 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt
> index 0fb4613..39e31c0e 100644
> --- a/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt
> +++ b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt
> @@ -37,6 +37,12 @@ VADC node:
>      Value type: <prop-encoded-array>
>      Definition: End of conversion interrupt.
>  
> +- qcom,vadc-poll-eoc:
> +    Usage: optional
> +    Value type: <bool>
> +    Definition: Use polling instead of interrupts for End of Conversion
> +		completion.
> +
>  Channel node properties:
>  
>  - reg:
> @@ -92,6 +98,14 @@ Channel node properties:
>              Valid values are: 1, 2, 4, 8, 16, 32, 64, 128, 256, 512
>              If property is not found, 1 sample will be used.
>  
> +- qcom,scale-function:
> +    Usage: optional
> +    Value type: <u32>
> +    Definition: Scaling function used to convert raw ADC code to
> +	units specific to a given channel. Scaled units can be
> +	microvolts, millidegC.Valid values are: 0, 1, 2, 3, 4.
> +	If property is not found, 0 scaling will be used.
> +
>  NOTE:
>  
>  Following channels, also known as reference point channels, are used for
> diff --git a/drivers/iio/adc/qcom-spmi-vadc.c b/drivers/iio/adc/qcom-spmi-vadc.c
> index ff4d549..6e521a9 100644
> --- a/drivers/iio/adc/qcom-spmi-vadc.c
> +++ b/drivers/iio/adc/qcom-spmi-vadc.c
> @@ -92,6 +92,8 @@
>  #define VADC_DEF_AVG_SAMPLES			0 /* 1 sample */
>  #define VADC_DEF_CALIB_TYPE			VADC_CALIB_ABSOLUTE
>  
> +#define VADC_DEF_SCALE_FN			SCALE_DEFAULT
> +
>  #define VADC_DECIMATION_MIN			512
>  #define VADC_DECIMATION_MAX			4096
>  
> @@ -100,9 +102,43 @@
>  
>  #define KELVINMIL_CELSIUSMIL			273150
>  
> +#define PMI_CHG_SCALE_1				-138890
> +#define PMI_CHG_SCALE_2				391750000000
> +
>  #define VADC_CHAN_MIN			VADC_USBIN
>  #define VADC_CHAN_MAX			VADC_LR_MUX3_BUF_PU1_PU2_XO_THERM
>  
> +/**
> + * enum vadc_scale_fn_type - Scaling function to convert ADC code to
> + *				physical scaled units for the channel.
> + * %SCALE_DEFAULT: Default scaling to convert raw adc code to voltage (uV).
> + * %SCALE_THERM_100K_PULLUP: Returns temperature in millidegC.
> + *				 Uses a mapping table with 100K pullup.
> + * %SCALE_PMIC_THERM: Returns result in milli degree's Centigrade.
> + * %SCALE_XOTHERM: Returns XO thermistor voltage in millidegC.
> + * %SCALE_PMI_CHG_TEMP: Conversion for PMI CHG temp
> + * %SCALE_NONE: Do not use this scaling type.
> + */
> +enum vadc_scale_fn_type {
> +	SCALE_DEFAULT = 0,
> +	SCALE_THERM_100K_PULLUP,
> +	SCALE_PMIC_THERM,
> +	SCALE_XOTHERM,
> +	SCALE_PMI_CHG_TEMP,
> +	SCALE_NONE,
> +};
> +
> +/**
> + * struct vadc_map_pt - Map the graph representation for ADC channel
> + * @x: Represent the ADC digitized code.
> + * @y: Represent the physical data which can be temperature, voltage,
> + *     resistance.
> + */
> +struct vadc_map_pt {
> +	s32 x;
> +	s32 y;
> +};
> +
>  /*
>   * VADC_CALIB_ABSOLUTE: uses the 625mV and 1.25V as reference channels.
>   * VADC_CALIB_RATIOMETRIC: uses the reference voltage (1.8V) and GND for
> @@ -148,6 +184,9 @@ struct vadc_prescale_ratio {
>   *	start of conversion.
>   * @avg_samples: ability to provide single result from the ADC
>   *	that is an average of multiple measurements.
Make sure your indentation matches the rest of the comment..
> + *@scale_function: Represents the scaling function to convert voltage
> + *	physical units desired by the client for the channel.
> + *	Referenced from enum vadc_scale_fn_type.
>   */
>  struct vadc_channel_prop {
>  	unsigned int channel;
> @@ -156,6 +195,7 @@ struct vadc_channel_prop {
>  	unsigned int prescale;
>  	unsigned int hw_settle_time;
>  	unsigned int avg_samples;
> +	unsigned int scale_function;
>  };
>  
>  /**
> @@ -197,6 +237,44 @@ struct vadc_priv {
>  	{.num =  1, .den = 10}
>  };
>  
> +/* Voltage to temperature */
> +static const struct vadc_map_pt adcmap_100k_104ef_104fb[] = {
> +	{1758,	-40},
> +	{1742,	-35},
> +	{1719,	-30},
> +	{1691,	-25},
> +	{1654,	-20},
> +	{1608,	-15},
> +	{1551,	-10},
> +	{1483,	-5},
> +	{1404,	0},
> +	{1315,	5},
> +	{1218,	10},
> +	{1114,	15},
> +	{1007,	20},
> +	{900,	25},
> +	{795,	30},
> +	{696,	35},
> +	{605,	40},
> +	{522,	45},
> +	{448,	50},
> +	{383,	55},
> +	{327,	60},
> +	{278,	65},
> +	{237,	70},
> +	{202,	75},
> +	{172,	80},
> +	{146,	85},
> +	{125,	90},
> +	{107,	95},
> +	{92,	100},
> +	{79,	105},
> +	{68,	110},
> +	{59,	115},
> +	{51,	120},
> +	{44,	125}
> +};
> +
>  static int vadc_read(struct vadc_priv *vadc, u16 offset, u8 *data)
>  {
>  	return regmap_bulk_read(vadc->regmap, vadc->base + offset, data, 1);
> @@ -468,6 +546,51 @@ static int vadc_measure_ref_points(struct vadc_priv *vadc)
>  	return ret;
>  }
>  
> +static int vadc_map_voltage_temp(const struct vadc_map_pt *pts,
> +				 u32 tablesize, s32 input, s64 *output)
> +{
> +	bool descending = 1;
> +	u32 i = 0;
> +
> +	if (!pts)
> +		return -EINVAL;
> +
> +	/* Check if table is descending or ascending */
> +	if (tablesize > 1) {
> +		if (pts[0].x < pts[1].x)
> +			descending = 0;
> +	}
> +
> +	while (i < tablesize) {
> +		if ((descending) && (pts[i].x < input)) {
> +			/* table entry is less than measured*/
> +			 /* value and table is descending, stop */
> +			break;
> +		} else if ((!descending) &&
> +				(pts[i].x > input)) {
> +			/* table entry is greater than measured*/
> +			/*value and table is ascending, stop */
> +			break;
> +		}
> +		i++;
> +	}
> +
> +	if (i == 0) {
> +		*output = pts[0].y;
> +	} else if (i == tablesize) {
> +		*output = pts[tablesize - 1].y;
> +	} else {
> +		/* result is between search_index and search_index-1 */
> +		/* interpolate linearly */
> +		*output = (((s32)((pts[i].y - pts[i - 1].y) *
> +			(input - pts[i - 1].x)) /
> +			(pts[i].x - pts[i - 1].x)) +
> +			pts[i - 1].y);
> +	}
> +
> +	return 0;
> +}
> +
>  static void vadc_scale_calib(struct vadc_priv *vadc, u16 adc_code,
>  			     const struct vadc_channel_prop *prop,
>  			     s64 *scale_voltage)
> @@ -489,15 +612,61 @@ static s64 vadc_scale_fn(struct vadc_priv *vadc,
>  			 const struct vadc_channel_prop *prop, u16 adc_code)
>  {
>  	const struct vadc_prescale_ratio *prescale;
> -	s64 voltage = 0;
> +	s64 voltage = 0, result = 0;
> +	int ret;
> +
> +	switch (prop->scale_function) {
> +	case SCALE_DEFAULT:
> +		vadc_scale_calib(vadc, adc_code, prop, &voltage);
> +
> +		prescale = &vadc_prescale_ratios[prop->prescale];
> +		voltage = voltage * prescale->den;
> +		return div64_s64(voltage, prescale->num);
> +
> +	case SCALE_THERM_100K_PULLUP:
> +	case SCALE_XOTHERM:
> +		vadc_scale_calib(vadc, adc_code, prop, &voltage);
> +
> +		if (prop->calibration == VADC_CALIB_ABSOLUTE)
> +			voltage /= 1000;
> +
> +		vadc_map_voltage_temp(adcmap_100k_104ef_104fb,
> +				      ARRAY_SIZE(adcmap_100k_104ef_104fb),
> +				      voltage, &result);
> +		result *= 1000;
> +		return result;
> +
> +	case SCALE_PMIC_THERM:
> +		vadc_scale_calib(vadc, adc_code, prop, &voltage);
> +
> +		if (voltage > 0) {
> +			prescale = &vadc_prescale_ratios[prop->prescale];
> +			voltage = voltage * prescale->den;
> +			voltage /= (prescale->num * 2);
> +		} else {
> +			voltage = 0;
> +		}
> +
> +		voltage -= KELVINMIL_CELSIUSMIL;
>  
> -	vadc_scale_calib(vadc, adc_code, prop, &voltage);
> +		return voltage;
>  
> -	prescale = &vadc_prescale_ratios[prop->prescale];
> +	case SCALE_PMI_CHG_TEMP:
> +		vadc_scale_calib(vadc, adc_code, prop, &voltage);
> +		prescale = &vadc_prescale_ratios[prop->prescale];
> +		voltage = voltage * prescale->den;
>  
> -	voltage = voltage * prescale->den;
> +		voltage = div64_s64(voltage, prescale->num);
> +		voltage = ((PMI_CHG_SCALE_1) * (voltage * 2));
> +		voltage = (voltage + PMI_CHG_SCALE_2);
> +		return div64_s64(voltage, 1000000);
>  
> -	return div64_s64(voltage, prescale->num);
> +	default:
> +		ret = -EINVAL;
> +		break;
> +	}
> +
> +	return ret;
>  }
>  
>  static int vadc_decimation_from_dt(u32 value)
> @@ -615,7 +784,9 @@ struct vadc_channels {
>  	},								\
>  
>  #define VADC_CHAN_TEMP(_dname, _pre)					\
> -	VADC_CHAN(_dname, IIO_TEMP, BIT(IIO_CHAN_INFO_PROCESSED), _pre)	\
> +	VADC_CHAN(_dname, IIO_TEMP,	\
> +		BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_PROCESSED), \
> +		_pre)	\
>  
>  #define VADC_CHAN_VOLT(_dname, _pre)					\
>  	VADC_CHAN(_dname, IIO_VOLTAGE,				\
> @@ -639,7 +810,7 @@ struct vadc_channels {
>  	VADC_CHAN_TEMP(DIE_TEMP, 0)
>  	VADC_CHAN_VOLT(REF_625MV, 0)
>  	VADC_CHAN_VOLT(REF_1250MV, 0)
> -	VADC_CHAN_VOLT(CHG_TEMP, 0)
> +	VADC_CHAN_TEMP(CHG_TEMP, 0)
>  	VADC_CHAN_VOLT(SPARE1, 0)
>  	VADC_CHAN_VOLT(SPARE2, 0)
>  	VADC_CHAN_VOLT(GND_REF, 0)
> @@ -693,41 +864,41 @@ struct vadc_channels {
>  	VADC_CHAN_VOLT(AMUX_PU2, 0)
>  	VADC_CHAN_VOLT(LR_MUX3_BUF_XO_THERM, 0)
>  
> -	VADC_CHAN_VOLT(LR_MUX1_PU1_BAT_THERM, 0)
> +	VADC_CHAN_TEMP(LR_MUX1_PU1_BAT_THERM, 0)
>  	VADC_CHAN_VOLT(LR_MUX2_PU1_BAT_ID, 0)
> -	VADC_CHAN_VOLT(LR_MUX3_PU1_XO_THERM, 0)
> -	VADC_CHAN_VOLT(LR_MUX4_PU1_AMUX_THM1, 0)
> -	VADC_CHAN_VOLT(LR_MUX5_PU1_AMUX_THM2, 0)
> -	VADC_CHAN_VOLT(LR_MUX6_PU1_AMUX_THM3, 0)
> +	VADC_CHAN_TEMP(LR_MUX3_PU1_XO_THERM, 0)
> +	VADC_CHAN_TEMP(LR_MUX4_PU1_AMUX_THM1, 0)
> +	VADC_CHAN_TEMP(LR_MUX5_PU1_AMUX_THM2, 0)
> +	VADC_CHAN_TEMP(LR_MUX6_PU1_AMUX_THM3, 0)
>  	VADC_CHAN_VOLT(LR_MUX7_PU1_AMUX_HW_ID, 0)
> -	VADC_CHAN_VOLT(LR_MUX8_PU1_AMUX_THM4, 0)
> -	VADC_CHAN_VOLT(LR_MUX9_PU1_AMUX_THM5, 0)
> +	VADC_CHAN_TEMP(LR_MUX8_PU1_AMUX_THM4, 0)
> +	VADC_CHAN_TEMP(LR_MUX9_PU1_AMUX_THM5, 0)
>  	VADC_CHAN_VOLT(LR_MUX10_PU1_AMUX_USB_ID, 0)
> -	VADC_CHAN_VOLT(LR_MUX3_BUF_PU1_XO_THERM, 0)
> +	VADC_CHAN_TEMP(LR_MUX3_BUF_PU1_XO_THERM, 0)
>  
> -	VADC_CHAN_VOLT(LR_MUX1_PU2_BAT_THERM, 0)
> +	VADC_CHAN_TEMP(LR_MUX1_PU2_BAT_THERM, 0)
>  	VADC_CHAN_VOLT(LR_MUX2_PU2_BAT_ID, 0)
> -	VADC_CHAN_VOLT(LR_MUX3_PU2_XO_THERM, 0)
> -	VADC_CHAN_VOLT(LR_MUX4_PU2_AMUX_THM1, 0)
> -	VADC_CHAN_VOLT(LR_MUX5_PU2_AMUX_THM2, 0)
> -	VADC_CHAN_VOLT(LR_MUX6_PU2_AMUX_THM3, 0)
> +	VADC_CHAN_TEMP(LR_MUX3_PU2_XO_THERM, 0)
> +	VADC_CHAN_TEMP(LR_MUX4_PU2_AMUX_THM1, 0)
> +	VADC_CHAN_TEMP(LR_MUX5_PU2_AMUX_THM2, 0)
> +	VADC_CHAN_TEMP(LR_MUX6_PU2_AMUX_THM3, 0)
>  	VADC_CHAN_VOLT(LR_MUX7_PU2_AMUX_HW_ID, 0)
> -	VADC_CHAN_VOLT(LR_MUX8_PU2_AMUX_THM4, 0)
> -	VADC_CHAN_VOLT(LR_MUX9_PU2_AMUX_THM5, 0)
> +	VADC_CHAN_TEMP(LR_MUX8_PU2_AMUX_THM4, 0)
> +	VADC_CHAN_TEMP(LR_MUX9_PU2_AMUX_THM5, 0)
>  	VADC_CHAN_VOLT(LR_MUX10_PU2_AMUX_USB_ID, 0)
> -	VADC_CHAN_VOLT(LR_MUX3_BUF_PU2_XO_THERM, 0)
> +	VADC_CHAN_TEMP(LR_MUX3_BUF_PU2_XO_THERM, 0)
>  
> -	VADC_CHAN_VOLT(LR_MUX1_PU1_PU2_BAT_THERM, 0)
> +	VADC_CHAN_TEMP(LR_MUX1_PU1_PU2_BAT_THERM, 0)
>  	VADC_CHAN_VOLT(LR_MUX2_PU1_PU2_BAT_ID, 0)
> -	VADC_CHAN_VOLT(LR_MUX3_PU1_PU2_XO_THERM, 0)
> -	VADC_CHAN_VOLT(LR_MUX4_PU1_PU2_AMUX_THM1, 0)
> -	VADC_CHAN_VOLT(LR_MUX5_PU1_PU2_AMUX_THM2, 0)
> -	VADC_CHAN_VOLT(LR_MUX6_PU1_PU2_AMUX_THM3, 0)
> +	VADC_CHAN_TEMP(LR_MUX3_PU1_PU2_XO_THERM, 0)
> +	VADC_CHAN_TEMP(LR_MUX4_PU1_PU2_AMUX_THM1, 0)
> +	VADC_CHAN_TEMP(LR_MUX5_PU1_PU2_AMUX_THM2, 0)
> +	VADC_CHAN_TEMP(LR_MUX6_PU1_PU2_AMUX_THM3, 0)
>  	VADC_CHAN_VOLT(LR_MUX7_PU1_PU2_AMUX_HW_ID, 0)
> -	VADC_CHAN_VOLT(LR_MUX8_PU1_PU2_AMUX_THM4, 0)
> -	VADC_CHAN_VOLT(LR_MUX9_PU1_PU2_AMUX_THM5, 0)
> +	VADC_CHAN_TEMP(LR_MUX8_PU1_PU2_AMUX_THM4, 0)
> +	VADC_CHAN_TEMP(LR_MUX9_PU1_PU2_AMUX_THM5, 0)
>  	VADC_CHAN_VOLT(LR_MUX10_PU1_PU2_AMUX_USB_ID, 0)
> -	VADC_CHAN_VOLT(LR_MUX3_BUF_PU1_PU2_XO_THERM, 0)
> +	VADC_CHAN_TEMP(LR_MUX3_BUF_PU1_PU2_XO_THERM, 0)
>  };
>  
>  static int vadc_get_dt_channel_data(struct device *dev,
> @@ -804,6 +975,11 @@ static int vadc_get_dt_channel_data(struct device *dev,
>  		prop->avg_samples = VADC_DEF_AVG_SAMPLES;
>  	}
>  
> +	ret = of_property_read_u32(node, "qcom,scale-function",
> +				   &prop->scale_function);
> +	if (ret)
> +		prop->scale_function = SCALE_DEFAULT;
> +
>  	if (of_property_read_bool(node, "qcom,ratiometric"))
>  		prop->calibration = VADC_CALIB_RATIOMETRIC;
>  	else
> @@ -966,16 +1142,21 @@ static int vadc_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>  
> -	irq_eoc = platform_get_irq(pdev, 0);
> -	if (irq_eoc < 0) {
> -		if (irq_eoc == -EPROBE_DEFER || irq_eoc == -EINVAL)
> -			return irq_eoc;
> -		vadc->poll_eoc = true;
> -	} else {
> -		ret = devm_request_irq(dev, irq_eoc, vadc_isr, 0,
> -				       "spmi-vadc", vadc);
> -		if (ret)
> -			return ret;
> +	vadc->poll_eoc = of_property_read_bool(node,
> +						"qcom,vadc-poll-eoc");
Should definitely be done on availability of the IRQ rather than a separate
device tree element.
> +
> +	if (!vadc->poll_eoc) {
> +		irq_eoc = platform_get_irq(pdev, 0);
> +		if (irq_eoc < 0) {
> +			if (irq_eoc == -EPROBE_DEFER || irq_eoc == -EINVAL)
> +				return irq_eoc;
> +			vadc->poll_eoc = true;
> +		} else {
> +			ret = devm_request_irq(dev, irq_eoc, vadc_isr, 0,
> +					       "spmi-vadc", vadc);
> +			if (ret)
> +				return ret;
> +		}
>  	}
>  
>  	ret = vadc_reset(vadc);
> 

Re: [PATCH V3 2/2] iio: adc: spmi-vadc: Changes to support different scaling

[Rama Krishna Phani A]

Hi Jonathan,

On 30-Oct-16 11:09 PM, Jonathan Cameron wrote:
> On 26/10/16 15:41, Rama Krishna Phani A wrote:
>> Polling can also be used for End of conversion completion. Implement logic
>> to choose either polling or interrupt for End of conversion completion.
> Why is this change in a patch with the title above?  Should be a separate patch.
>
Ok., Will split the patch.

>> Scaling can be done on the voltage to report adc code in physical units.
>> Add changes to support different scale functions to convert adc code to
>> physical units.
>>
>> Signed-off-by: Rama Krishna Phani A <rphani@codeaurora.org>
>> ---
>>  .../devicetree/bindings/iio/adc/qcom,spmi-vadc.txt |  14 ++
>>  drivers/iio/adc/qcom-spmi-vadc.c                   | 263 +++++++++++++++++----
>>  2 files changed, 236 insertions(+), 41 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt
>> index 0fb4613..39e31c0e 100644
>> --- a/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt
>> +++ b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt
>> @@ -37,6 +37,12 @@ VADC node:
>>      Value type: <prop-encoded-array>
>>      Definition: End of conversion interrupt.
>>
>> +- qcom,vadc-poll-eoc:
>> +    Usage: optional
>> +    Value type: <bool>
>> +    Definition: Use polling instead of interrupts for End of Conversion
>> +		completion.
>> +
>>  Channel node properties:
>>
>>  - reg:
>> @@ -92,6 +98,14 @@ Channel node properties:
>>              Valid values are: 1, 2, 4, 8, 16, 32, 64, 128, 256, 512
>>              If property is not found, 1 sample will be used.
>>
>> +- qcom,scale-function:
>> +    Usage: optional
>> +    Value type: <u32>
>> +    Definition: Scaling function used to convert raw ADC code to
>> +	units specific to a given channel. Scaled units can be
>> +	microvolts, millidegC.Valid values are: 0, 1, 2, 3, 4.
>> +	If property is not found, 0 scaling will be used.
>> +
>>  NOTE:
>>
>>  Following channels, also known as reference point channels, are used for
>> diff --git a/drivers/iio/adc/qcom-spmi-vadc.c b/drivers/iio/adc/qcom-spmi-vadc.c
>> index ff4d549..6e521a9 100644
>> --- a/drivers/iio/adc/qcom-spmi-vadc.c
>> +++ b/drivers/iio/adc/qcom-spmi-vadc.c
>> @@ -92,6 +92,8 @@
>>  #define VADC_DEF_AVG_SAMPLES			0 /* 1 sample */
>>  #define VADC_DEF_CALIB_TYPE			VADC_CALIB_ABSOLUTE
>>
>> +#define VADC_DEF_SCALE_FN			SCALE_DEFAULT
>> +
>>  #define VADC_DECIMATION_MIN			512
>>  #define VADC_DECIMATION_MAX			4096
>>
>> @@ -100,9 +102,43 @@
>>
>>  #define KELVINMIL_CELSIUSMIL			273150
>>
>> +#define PMI_CHG_SCALE_1				-138890
>> +#define PMI_CHG_SCALE_2				391750000000
>> +
>>  #define VADC_CHAN_MIN			VADC_USBIN
>>  #define VADC_CHAN_MAX			VADC_LR_MUX3_BUF_PU1_PU2_XO_THERM
>>
>> +/**
>> + * enum vadc_scale_fn_type - Scaling function to convert ADC code to
>> + *				physical scaled units for the channel.
>> + * %SCALE_DEFAULT: Default scaling to convert raw adc code to voltage (uV).
>> + * %SCALE_THERM_100K_PULLUP: Returns temperature in millidegC.
>> + *				 Uses a mapping table with 100K pullup.
>> + * %SCALE_PMIC_THERM: Returns result in milli degree's Centigrade.
>> + * %SCALE_XOTHERM: Returns XO thermistor voltage in millidegC.
>> + * %SCALE_PMI_CHG_TEMP: Conversion for PMI CHG temp
>> + * %SCALE_NONE: Do not use this scaling type.
>> + */
>> +enum vadc_scale_fn_type {
>> +	SCALE_DEFAULT = 0,
>> +	SCALE_THERM_100K_PULLUP,
>> +	SCALE_PMIC_THERM,
>> +	SCALE_XOTHERM,
>> +	SCALE_PMI_CHG_TEMP,
>> +	SCALE_NONE,
>> +};
>> +
>> +/**
>> + * struct vadc_map_pt - Map the graph representation for ADC channel
>> + * @x: Represent the ADC digitized code.
>> + * @y: Represent the physical data which can be temperature, voltage,
>> + *     resistance.
>> + */
>> +struct vadc_map_pt {
>> +	s32 x;
>> +	s32 y;
>> +};
>> +
>>  /*
>>   * VADC_CALIB_ABSOLUTE: uses the 625mV and 1.25V as reference channels.
>>   * VADC_CALIB_RATIOMETRIC: uses the reference voltage (1.8V) and GND for
>> @@ -148,6 +184,9 @@ struct vadc_prescale_ratio {
>>   *	start of conversion.
>>   * @avg_samples: ability to provide single result from the ADC
>>   *	that is an average of multiple measurements.
> Make sure your indentation matches the rest of the comment..
Sure ., Will check for indentation.

>> + *@scale_function: Represents the scaling function to convert voltage
>> + *	physical units desired by the client for the channel.
>> + *	Referenced from enum vadc_scale_fn_type.
>>   */
>>  struct vadc_channel_prop {
>>  	unsigned int channel;
>> @@ -156,6 +195,7 @@ struct vadc_channel_prop {
>>  	unsigned int prescale;
>>  	unsigned int hw_settle_time;
>>  	unsigned int avg_samples;
>> +	unsigned int scale_function;
>>  };
>>
>>  /**
>> @@ -197,6 +237,44 @@ struct vadc_priv {
>>  	{.num =  1, .den = 10}
>>  };
>>
>> +/* Voltage to temperature */
>> +static const struct vadc_map_pt adcmap_100k_104ef_104fb[] = {
>> +	{1758,	-40},
>> +	{1742,	-35},
>> +	{1719,	-30},
>> +	{1691,	-25},
>> +	{1654,	-20},
>> +	{1608,	-15},
>> +	{1551,	-10},
>> +	{1483,	-5},
>> +	{1404,	0},
>> +	{1315,	5},
>> +	{1218,	10},
>> +	{1114,	15},
>> +	{1007,	20},
>> +	{900,	25},
>> +	{795,	30},
>> +	{696,	35},
>> +	{605,	40},
>> +	{522,	45},
>> +	{448,	50},
>> +	{383,	55},
>> +	{327,	60},
>> +	{278,	65},
>> +	{237,	70},
>> +	{202,	75},
>> +	{172,	80},
>> +	{146,	85},
>> +	{125,	90},
>> +	{107,	95},
>> +	{92,	100},
>> +	{79,	105},
>> +	{68,	110},
>> +	{59,	115},
>> +	{51,	120},
>> +	{44,	125}
>> +};
>> +
>>  static int vadc_read(struct vadc_priv *vadc, u16 offset, u8 *data)
>>  {
>>  	return regmap_bulk_read(vadc->regmap, vadc->base + offset, data, 1);
>> @@ -468,6 +546,51 @@ static int vadc_measure_ref_points(struct vadc_priv *vadc)
>>  	return ret;
>>  }
>>
>> +static int vadc_map_voltage_temp(const struct vadc_map_pt *pts,
>> +				 u32 tablesize, s32 input, s64 *output)
>> +{
>> +	bool descending = 1;
>> +	u32 i = 0;
>> +
>> +	if (!pts)
>> +		return -EINVAL;
>> +
>> +	/* Check if table is descending or ascending */
>> +	if (tablesize > 1) {
>> +		if (pts[0].x < pts[1].x)
>> +			descending = 0;
>> +	}
>> +
>> +	while (i < tablesize) {
>> +		if ((descending) && (pts[i].x < input)) {
>> +			/* table entry is less than measured*/
>> +			 /* value and table is descending, stop */
>> +			break;
>> +		} else if ((!descending) &&
>> +				(pts[i].x > input)) {
>> +			/* table entry is greater than measured*/
>> +			/*value and table is ascending, stop */
>> +			break;
>> +		}
>> +		i++;
>> +	}
>> +
>> +	if (i == 0) {
>> +		*output = pts[0].y;
>> +	} else if (i == tablesize) {
>> +		*output = pts[tablesize - 1].y;
>> +	} else {
>> +		/* result is between search_index and search_index-1 */
>> +		/* interpolate linearly */
>> +		*output = (((s32)((pts[i].y - pts[i - 1].y) *
>> +			(input - pts[i - 1].x)) /
>> +			(pts[i].x - pts[i - 1].x)) +
>> +			pts[i - 1].y);
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>>  static void vadc_scale_calib(struct vadc_priv *vadc, u16 adc_code,
>>  			     const struct vadc_channel_prop *prop,
>>  			     s64 *scale_voltage)
>> @@ -489,15 +612,61 @@ static s64 vadc_scale_fn(struct vadc_priv *vadc,
>>  			 const struct vadc_channel_prop *prop, u16 adc_code)
>>  {
>>  	const struct vadc_prescale_ratio *prescale;
>> -	s64 voltage = 0;
>> +	s64 voltage = 0, result = 0;
>> +	int ret;
>> +
>> +	switch (prop->scale_function) {
>> +	case SCALE_DEFAULT:
>> +		vadc_scale_calib(vadc, adc_code, prop, &voltage);
>> +
>> +		prescale = &vadc_prescale_ratios[prop->prescale];
>> +		voltage = voltage * prescale->den;
>> +		return div64_s64(voltage, prescale->num);
>> +
>> +	case SCALE_THERM_100K_PULLUP:
>> +	case SCALE_XOTHERM:
>> +		vadc_scale_calib(vadc, adc_code, prop, &voltage);
>> +
>> +		if (prop->calibration == VADC_CALIB_ABSOLUTE)
>> +			voltage /= 1000;
>> +
>> +		vadc_map_voltage_temp(adcmap_100k_104ef_104fb,
>> +				      ARRAY_SIZE(adcmap_100k_104ef_104fb),
>> +				      voltage, &result);
>> +		result *= 1000;
>> +		return result;
>> +
>> +	case SCALE_PMIC_THERM:
>> +		vadc_scale_calib(vadc, adc_code, prop, &voltage);
>> +
>> +		if (voltage > 0) {
>> +			prescale = &vadc_prescale_ratios[prop->prescale];
>> +			voltage = voltage * prescale->den;
>> +			voltage /= (prescale->num * 2);
>> +		} else {
>> +			voltage = 0;
>> +		}
>> +
>> +		voltage -= KELVINMIL_CELSIUSMIL;
>>
>> -	vadc_scale_calib(vadc, adc_code, prop, &voltage);
>> +		return voltage;
>>
>> -	prescale = &vadc_prescale_ratios[prop->prescale];
>> +	case SCALE_PMI_CHG_TEMP:
>> +		vadc_scale_calib(vadc, adc_code, prop, &voltage);
>> +		prescale = &vadc_prescale_ratios[prop->prescale];
>> +		voltage = voltage * prescale->den;
>>
>> -	voltage = voltage * prescale->den;
>> +		voltage = div64_s64(voltage, prescale->num);
>> +		voltage = ((PMI_CHG_SCALE_1) * (voltage * 2));
>> +		voltage = (voltage + PMI_CHG_SCALE_2);
>> +		return div64_s64(voltage, 1000000);
>>
>> -	return div64_s64(voltage, prescale->num);
>> +	default:
>> +		ret = -EINVAL;
>> +		break;
>> +	}
>> +
>> +	return ret;
>>  }
>>
>>  static int vadc_decimation_from_dt(u32 value)
>> @@ -615,7 +784,9 @@ struct vadc_channels {
>>  	},								\
>>
>>  #define VADC_CHAN_TEMP(_dname, _pre)					\
>> -	VADC_CHAN(_dname, IIO_TEMP, BIT(IIO_CHAN_INFO_PROCESSED), _pre)	\
>> +	VADC_CHAN(_dname, IIO_TEMP,	\
>> +		BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_PROCESSED), \
>> +		_pre)	\
>>
>>  #define VADC_CHAN_VOLT(_dname, _pre)					\
>>  	VADC_CHAN(_dname, IIO_VOLTAGE,				\
>> @@ -639,7 +810,7 @@ struct vadc_channels {
>>  	VADC_CHAN_TEMP(DIE_TEMP, 0)
>>  	VADC_CHAN_VOLT(REF_625MV, 0)
>>  	VADC_CHAN_VOLT(REF_1250MV, 0)
>> -	VADC_CHAN_VOLT(CHG_TEMP, 0)
>> +	VADC_CHAN_TEMP(CHG_TEMP, 0)
>>  	VADC_CHAN_VOLT(SPARE1, 0)
>>  	VADC_CHAN_VOLT(SPARE2, 0)
>>  	VADC_CHAN_VOLT(GND_REF, 0)
>> @@ -693,41 +864,41 @@ struct vadc_channels {
>>  	VADC_CHAN_VOLT(AMUX_PU2, 0)
>>  	VADC_CHAN_VOLT(LR_MUX3_BUF_XO_THERM, 0)
>>
>> -	VADC_CHAN_VOLT(LR_MUX1_PU1_BAT_THERM, 0)
>> +	VADC_CHAN_TEMP(LR_MUX1_PU1_BAT_THERM, 0)
>>  	VADC_CHAN_VOLT(LR_MUX2_PU1_BAT_ID, 0)
>> -	VADC_CHAN_VOLT(LR_MUX3_PU1_XO_THERM, 0)
>> -	VADC_CHAN_VOLT(LR_MUX4_PU1_AMUX_THM1, 0)
>> -	VADC_CHAN_VOLT(LR_MUX5_PU1_AMUX_THM2, 0)
>> -	VADC_CHAN_VOLT(LR_MUX6_PU1_AMUX_THM3, 0)
>> +	VADC_CHAN_TEMP(LR_MUX3_PU1_XO_THERM, 0)
>> +	VADC_CHAN_TEMP(LR_MUX4_PU1_AMUX_THM1, 0)
>> +	VADC_CHAN_TEMP(LR_MUX5_PU1_AMUX_THM2, 0)
>> +	VADC_CHAN_TEMP(LR_MUX6_PU1_AMUX_THM3, 0)
>>  	VADC_CHAN_VOLT(LR_MUX7_PU1_AMUX_HW_ID, 0)
>> -	VADC_CHAN_VOLT(LR_MUX8_PU1_AMUX_THM4, 0)
>> -	VADC_CHAN_VOLT(LR_MUX9_PU1_AMUX_THM5, 0)
>> +	VADC_CHAN_TEMP(LR_MUX8_PU1_AMUX_THM4, 0)
>> +	VADC_CHAN_TEMP(LR_MUX9_PU1_AMUX_THM5, 0)
>>  	VADC_CHAN_VOLT(LR_MUX10_PU1_AMUX_USB_ID, 0)
>> -	VADC_CHAN_VOLT(LR_MUX3_BUF_PU1_XO_THERM, 0)
>> +	VADC_CHAN_TEMP(LR_MUX3_BUF_PU1_XO_THERM, 0)
>>
>> -	VADC_CHAN_VOLT(LR_MUX1_PU2_BAT_THERM, 0)
>> +	VADC_CHAN_TEMP(LR_MUX1_PU2_BAT_THERM, 0)
>>  	VADC_CHAN_VOLT(LR_MUX2_PU2_BAT_ID, 0)
>> -	VADC_CHAN_VOLT(LR_MUX3_PU2_XO_THERM, 0)
>> -	VADC_CHAN_VOLT(LR_MUX4_PU2_AMUX_THM1, 0)
>> -	VADC_CHAN_VOLT(LR_MUX5_PU2_AMUX_THM2, 0)
>> -	VADC_CHAN_VOLT(LR_MUX6_PU2_AMUX_THM3, 0)
>> +	VADC_CHAN_TEMP(LR_MUX3_PU2_XO_THERM, 0)
>> +	VADC_CHAN_TEMP(LR_MUX4_PU2_AMUX_THM1, 0)
>> +	VADC_CHAN_TEMP(LR_MUX5_PU2_AMUX_THM2, 0)
>> +	VADC_CHAN_TEMP(LR_MUX6_PU2_AMUX_THM3, 0)
>>  	VADC_CHAN_VOLT(LR_MUX7_PU2_AMUX_HW_ID, 0)
>> -	VADC_CHAN_VOLT(LR_MUX8_PU2_AMUX_THM4, 0)
>> -	VADC_CHAN_VOLT(LR_MUX9_PU2_AMUX_THM5, 0)
>> +	VADC_CHAN_TEMP(LR_MUX8_PU2_AMUX_THM4, 0)
>> +	VADC_CHAN_TEMP(LR_MUX9_PU2_AMUX_THM5, 0)
>>  	VADC_CHAN_VOLT(LR_MUX10_PU2_AMUX_USB_ID, 0)
>> -	VADC_CHAN_VOLT(LR_MUX3_BUF_PU2_XO_THERM, 0)
>> +	VADC_CHAN_TEMP(LR_MUX3_BUF_PU2_XO_THERM, 0)
>>
>> -	VADC_CHAN_VOLT(LR_MUX1_PU1_PU2_BAT_THERM, 0)
>> +	VADC_CHAN_TEMP(LR_MUX1_PU1_PU2_BAT_THERM, 0)
>>  	VADC_CHAN_VOLT(LR_MUX2_PU1_PU2_BAT_ID, 0)
>> -	VADC_CHAN_VOLT(LR_MUX3_PU1_PU2_XO_THERM, 0)
>> -	VADC_CHAN_VOLT(LR_MUX4_PU1_PU2_AMUX_THM1, 0)
>> -	VADC_CHAN_VOLT(LR_MUX5_PU1_PU2_AMUX_THM2, 0)
>> -	VADC_CHAN_VOLT(LR_MUX6_PU1_PU2_AMUX_THM3, 0)
>> +	VADC_CHAN_TEMP(LR_MUX3_PU1_PU2_XO_THERM, 0)
>> +	VADC_CHAN_TEMP(LR_MUX4_PU1_PU2_AMUX_THM1, 0)
>> +	VADC_CHAN_TEMP(LR_MUX5_PU1_PU2_AMUX_THM2, 0)
>> +	VADC_CHAN_TEMP(LR_MUX6_PU1_PU2_AMUX_THM3, 0)
>>  	VADC_CHAN_VOLT(LR_MUX7_PU1_PU2_AMUX_HW_ID, 0)
>> -	VADC_CHAN_VOLT(LR_MUX8_PU1_PU2_AMUX_THM4, 0)
>> -	VADC_CHAN_VOLT(LR_MUX9_PU1_PU2_AMUX_THM5, 0)
>> +	VADC_CHAN_TEMP(LR_MUX8_PU1_PU2_AMUX_THM4, 0)
>> +	VADC_CHAN_TEMP(LR_MUX9_PU1_PU2_AMUX_THM5, 0)
>>  	VADC_CHAN_VOLT(LR_MUX10_PU1_PU2_AMUX_USB_ID, 0)
>> -	VADC_CHAN_VOLT(LR_MUX3_BUF_PU1_PU2_XO_THERM, 0)
>> +	VADC_CHAN_TEMP(LR_MUX3_BUF_PU1_PU2_XO_THERM, 0)
>>  };
>>
>>  static int vadc_get_dt_channel_data(struct device *dev,
>> @@ -804,6 +975,11 @@ static int vadc_get_dt_channel_data(struct device *dev,
>>  		prop->avg_samples = VADC_DEF_AVG_SAMPLES;
>>  	}
>>
>> +	ret = of_property_read_u32(node, "qcom,scale-function",
>> +				   &prop->scale_function);
>> +	if (ret)
>> +		prop->scale_function = SCALE_DEFAULT;
>> +
>>  	if (of_property_read_bool(node, "qcom,ratiometric"))
>>  		prop->calibration = VADC_CALIB_RATIOMETRIC;
>>  	else
>> @@ -966,16 +1142,21 @@ static int vadc_probe(struct platform_device *pdev)
>>  	if (ret)
>>  		return ret;
>>
>> -	irq_eoc = platform_get_irq(pdev, 0);
>> -	if (irq_eoc < 0) {
>> -		if (irq_eoc == -EPROBE_DEFER || irq_eoc == -EINVAL)
>> -			return irq_eoc;
>> -		vadc->poll_eoc = true;
>> -	} else {
>> -		ret = devm_request_irq(dev, irq_eoc, vadc_isr, 0,
>> -				       "spmi-vadc", vadc);
>> -		if (ret)
>> -			return ret;
>> +	vadc->poll_eoc = of_property_read_bool(node,
>> +						"qcom,vadc-poll-eoc");
> Should definitely be done on availability of the IRQ rather than a separate
> device tree element.
Will check the logic for implementation and will post next patch.

>> +
>> +	if (!vadc->poll_eoc) {
>> +		irq_eoc = platform_get_irq(pdev, 0);
>> +		if (irq_eoc < 0) {
>> +			if (irq_eoc == -EPROBE_DEFER || irq_eoc == -EINVAL)
>> +				return irq_eoc;
>> +			vadc->poll_eoc = true;
>> +		} else {
>> +			ret = devm_request_irq(dev, irq_eoc, vadc_isr, 0,
>> +					       "spmi-vadc", vadc);
>> +			if (ret)
>> +				return ret;
>> +		}
>>  	}
>>
>>  	ret = vadc_reset(vadc);
>>
>

Thanks,
Ramakrishna

---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus