linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] iio: adc: sc27xx: Add raw data support
@ 2018-08-24  9:53 Baolin Wang
  2018-08-24  9:53 ` [PATCH 2/2] iio: adc: sc27xx: Add ADC scale calibration Baolin Wang
  2018-08-25  8:38 ` [PATCH 1/2] iio: adc: sc27xx: Add raw data support Jonathan Cameron
  0 siblings, 2 replies; 6+ messages in thread
From: Baolin Wang @ 2018-08-24  9:53 UTC (permalink / raw)
  To: jic23, robh+dt, mark.rutland
  Cc: knaack.h, lars, pmeerw, freeman.liu, broonie, baolin.wang,
	devicetree, linux-iio, linux-kernel

The headset device will use channel 20 of ADC controller to detect events,
but it needs the raw ADC data to do conversion according to its own formula.

Thus we should configure the channel mask separately and configure channel
20 as IIO_CHAN_INFO_RAW, as well as adding raw data read support.

Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
 drivers/iio/adc/sc27xx_adc.c |   80 ++++++++++++++++++++++++------------------
 1 file changed, 45 insertions(+), 35 deletions(-)

diff --git a/drivers/iio/adc/sc27xx_adc.c b/drivers/iio/adc/sc27xx_adc.c
index 2b60efe..153c311 100644
--- a/drivers/iio/adc/sc27xx_adc.c
+++ b/drivers/iio/adc/sc27xx_adc.c
@@ -273,6 +273,17 @@ static int sc27xx_adc_read_raw(struct iio_dev *indio_dev,
 	int ret, tmp;
 
 	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		mutex_lock(&indio_dev->mlock);
+		ret = sc27xx_adc_read(data, chan->channel, scale, &tmp);
+		mutex_unlock(&indio_dev->mlock);
+
+		if (ret)
+			return ret;
+
+		*val = tmp;
+		return IIO_VAL_INT;
+
 	case IIO_CHAN_INFO_PROCESSED:
 		mutex_lock(&indio_dev->mlock);
 		ret = sc27xx_adc_read_processed(data, chan->channel, scale,
@@ -315,48 +326,47 @@ static int sc27xx_adc_write_raw(struct iio_dev *indio_dev,
 	.write_raw = &sc27xx_adc_write_raw,
 };
 
-#define SC27XX_ADC_CHANNEL(index) {				\
+#define SC27XX_ADC_CHANNEL(index, mask) {			\
 	.type = IIO_VOLTAGE,					\
 	.channel = index,					\
-	.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |	\
-			      BIT(IIO_CHAN_INFO_SCALE),		\
+	.info_mask_separate = mask | BIT(IIO_CHAN_INFO_SCALE),	\
 	.datasheet_name = "CH##index",				\
 	.indexed = 1,						\
 }
 
 static const struct iio_chan_spec sc27xx_channels[] = {
-	SC27XX_ADC_CHANNEL(0),
-	SC27XX_ADC_CHANNEL(1),
-	SC27XX_ADC_CHANNEL(2),
-	SC27XX_ADC_CHANNEL(3),
-	SC27XX_ADC_CHANNEL(4),
-	SC27XX_ADC_CHANNEL(5),
-	SC27XX_ADC_CHANNEL(6),
-	SC27XX_ADC_CHANNEL(7),
-	SC27XX_ADC_CHANNEL(8),
-	SC27XX_ADC_CHANNEL(9),
-	SC27XX_ADC_CHANNEL(10),
-	SC27XX_ADC_CHANNEL(11),
-	SC27XX_ADC_CHANNEL(12),
-	SC27XX_ADC_CHANNEL(13),
-	SC27XX_ADC_CHANNEL(14),
-	SC27XX_ADC_CHANNEL(15),
-	SC27XX_ADC_CHANNEL(16),
-	SC27XX_ADC_CHANNEL(17),
-	SC27XX_ADC_CHANNEL(18),
-	SC27XX_ADC_CHANNEL(19),
-	SC27XX_ADC_CHANNEL(20),
-	SC27XX_ADC_CHANNEL(21),
-	SC27XX_ADC_CHANNEL(22),
-	SC27XX_ADC_CHANNEL(23),
-	SC27XX_ADC_CHANNEL(24),
-	SC27XX_ADC_CHANNEL(25),
-	SC27XX_ADC_CHANNEL(26),
-	SC27XX_ADC_CHANNEL(27),
-	SC27XX_ADC_CHANNEL(28),
-	SC27XX_ADC_CHANNEL(29),
-	SC27XX_ADC_CHANNEL(30),
-	SC27XX_ADC_CHANNEL(31),
+	SC27XX_ADC_CHANNEL(0, BIT(IIO_CHAN_INFO_PROCESSED)),
+	SC27XX_ADC_CHANNEL(1, BIT(IIO_CHAN_INFO_PROCESSED)),
+	SC27XX_ADC_CHANNEL(2, BIT(IIO_CHAN_INFO_PROCESSED)),
+	SC27XX_ADC_CHANNEL(3, BIT(IIO_CHAN_INFO_PROCESSED)),
+	SC27XX_ADC_CHANNEL(4, BIT(IIO_CHAN_INFO_PROCESSED)),
+	SC27XX_ADC_CHANNEL(5, BIT(IIO_CHAN_INFO_PROCESSED)),
+	SC27XX_ADC_CHANNEL(6, BIT(IIO_CHAN_INFO_PROCESSED)),
+	SC27XX_ADC_CHANNEL(7, BIT(IIO_CHAN_INFO_PROCESSED)),
+	SC27XX_ADC_CHANNEL(8, BIT(IIO_CHAN_INFO_PROCESSED)),
+	SC27XX_ADC_CHANNEL(9, BIT(IIO_CHAN_INFO_PROCESSED)),
+	SC27XX_ADC_CHANNEL(10, BIT(IIO_CHAN_INFO_PROCESSED)),
+	SC27XX_ADC_CHANNEL(11, BIT(IIO_CHAN_INFO_PROCESSED)),
+	SC27XX_ADC_CHANNEL(12, BIT(IIO_CHAN_INFO_PROCESSED)),
+	SC27XX_ADC_CHANNEL(13, BIT(IIO_CHAN_INFO_PROCESSED)),
+	SC27XX_ADC_CHANNEL(14, BIT(IIO_CHAN_INFO_PROCESSED)),
+	SC27XX_ADC_CHANNEL(15, BIT(IIO_CHAN_INFO_PROCESSED)),
+	SC27XX_ADC_CHANNEL(16, BIT(IIO_CHAN_INFO_PROCESSED)),
+	SC27XX_ADC_CHANNEL(17, BIT(IIO_CHAN_INFO_PROCESSED)),
+	SC27XX_ADC_CHANNEL(18, BIT(IIO_CHAN_INFO_PROCESSED)),
+	SC27XX_ADC_CHANNEL(19, BIT(IIO_CHAN_INFO_PROCESSED)),
+	SC27XX_ADC_CHANNEL(20, BIT(IIO_CHAN_INFO_RAW)),
+	SC27XX_ADC_CHANNEL(21, BIT(IIO_CHAN_INFO_PROCESSED)),
+	SC27XX_ADC_CHANNEL(22, BIT(IIO_CHAN_INFO_PROCESSED)),
+	SC27XX_ADC_CHANNEL(23, BIT(IIO_CHAN_INFO_PROCESSED)),
+	SC27XX_ADC_CHANNEL(24, BIT(IIO_CHAN_INFO_PROCESSED)),
+	SC27XX_ADC_CHANNEL(25, BIT(IIO_CHAN_INFO_PROCESSED)),
+	SC27XX_ADC_CHANNEL(26, BIT(IIO_CHAN_INFO_PROCESSED)),
+	SC27XX_ADC_CHANNEL(27, BIT(IIO_CHAN_INFO_PROCESSED)),
+	SC27XX_ADC_CHANNEL(28, BIT(IIO_CHAN_INFO_PROCESSED)),
+	SC27XX_ADC_CHANNEL(29, BIT(IIO_CHAN_INFO_PROCESSED)),
+	SC27XX_ADC_CHANNEL(30, BIT(IIO_CHAN_INFO_PROCESSED)),
+	SC27XX_ADC_CHANNEL(31, BIT(IIO_CHAN_INFO_PROCESSED)),
 };
 
 static int sc27xx_adc_enable(struct sc27xx_adc_data *data)
-- 
1.7.9.5


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

* [PATCH 2/2] iio: adc: sc27xx: Add ADC scale calibration
  2018-08-24  9:53 [PATCH 1/2] iio: adc: sc27xx: Add raw data support Baolin Wang
@ 2018-08-24  9:53 ` Baolin Wang
  2018-08-25  8:43   ` Jonathan Cameron
  2018-08-25  8:38 ` [PATCH 1/2] iio: adc: sc27xx: Add raw data support Jonathan Cameron
  1 sibling, 1 reply; 6+ messages in thread
From: Baolin Wang @ 2018-08-24  9:53 UTC (permalink / raw)
  To: jic23, robh+dt, mark.rutland
  Cc: knaack.h, lars, pmeerw, freeman.liu, broonie, baolin.wang,
	devicetree, linux-iio, linux-kernel

This patch adds support to read calibration values from the eFuse
controller to calibrate the ADC channel scales, which can make ADC
sample data more accurate.

Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
 .../bindings/iio/adc/sprd,sc27xx-adc.txt           |    4 ++
 drivers/iio/adc/sc27xx_adc.c                       |   52 ++++++++++++++++++--
 2 files changed, 53 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/adc/sprd,sc27xx-adc.txt b/Documentation/devicetree/bindings/iio/adc/sprd,sc27xx-adc.txt
index 8aad960..b4daa15 100644
--- a/Documentation/devicetree/bindings/iio/adc/sprd,sc27xx-adc.txt
+++ b/Documentation/devicetree/bindings/iio/adc/sprd,sc27xx-adc.txt
@@ -12,6 +12,8 @@ Required properties:
 - interrupts: The interrupt number for the ADC device.
 - #io-channel-cells: Number of cells in an IIO specifier.
 - hwlocks: Reference to a phandle of a hwlock provider node.
+- nvmem-cells: A phandle to the calibration cells provided by eFuse device.
+- nvmem-cell-names: Should be "big_scale_calib", "small_scale_calib".
 
 Example:
 
@@ -32,5 +34,7 @@ Example:
 			interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
 			#io-channel-cells = <1>;
 			hwlocks = <&hwlock 4>;
+			nvmem-cells = <&adc_big_scale>, <&adc_small_scale>;
+			nvmem-cell-names = "big_scale_calib", "small_scale_calib";
 		};
 	};
diff --git a/drivers/iio/adc/sc27xx_adc.c b/drivers/iio/adc/sc27xx_adc.c
index 153c311..7ac78eda 100644
--- a/drivers/iio/adc/sc27xx_adc.c
+++ b/drivers/iio/adc/sc27xx_adc.c
@@ -5,6 +5,7 @@
 #include <linux/iio/iio.h>
 #include <linux/interrupt.h>
 #include <linux/module.h>
+#include <linux/nvmem-consumer.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
 #include <linux/platform_device.h>
@@ -87,16 +88,48 @@ struct sc27xx_adc_linear_graph {
  * should use the small-scale graph, and if more than 1.2v, we should use the
  * big-scale graph.
  */
-static const struct sc27xx_adc_linear_graph big_scale_graph = {
+static struct sc27xx_adc_linear_graph big_scale_graph = {
 	4200, 3310,
 	3600, 2832,
 };
 
-static const struct sc27xx_adc_linear_graph small_scale_graph = {
+static struct sc27xx_adc_linear_graph small_scale_graph = {
 	1000, 3413,
 	100, 341,
 };
 
+static const struct sc27xx_adc_linear_graph big_scale_graph_calib = {
+	4200, 856,
+	3600, 733,
+};
+
+static const struct sc27xx_adc_linear_graph small_scale_graph_calib = {
+	1000, 833,
+	100, 80,
+};
+
+static int sc27xx_adc_get_calib_data(u32 calib_data, int calib_adc)
+{
+	return ((calib_data & 0xff) + calib_adc - 128) * 4;
+}
+
+static void
+sc27xx_adc_scale_calibration(const struct sc27xx_adc_linear_graph *calib_graph,
+			     u32 calib_data, bool big_scale)
+{
+	struct sc27xx_adc_linear_graph *graph;
+
+	if (big_scale)
+		graph = &big_scale_graph;
+	else
+		graph = &small_scale_graph;
+
+	/* Only need to calibrate the adc values in the linear graph. */
+	graph->adc0 = sc27xx_adc_get_calib_data(calib_data, calib_graph->adc0);
+	graph->adc1 = sc27xx_adc_get_calib_data(calib_data >> 8,
+						calib_graph->adc1);
+}
+
 static int sc27xx_adc_get_ratio(int channel, int scale)
 {
 	switch (channel) {
@@ -209,7 +242,7 @@ static void sc27xx_adc_volt_ratio(struct sc27xx_adc_data *data,
 	*div_denominator = ratio & SC27XX_RATIO_DENOMINATOR_MASK;
 }
 
-static int sc27xx_adc_to_volt(const struct sc27xx_adc_linear_graph *graph,
+static int sc27xx_adc_to_volt(struct sc27xx_adc_linear_graph *graph,
 			      int raw_adc)
 {
 	int tmp;
@@ -371,6 +404,7 @@ static int sc27xx_adc_write_raw(struct iio_dev *indio_dev,
 
 static int sc27xx_adc_enable(struct sc27xx_adc_data *data)
 {
+	u32 val;
 	int ret;
 
 	ret = regmap_update_bits(data->regmap, SC27XX_MODULE_EN,
@@ -390,6 +424,18 @@ static int sc27xx_adc_enable(struct sc27xx_adc_data *data)
 	if (ret)
 		goto disable_clk;
 
+	/* ADC channel scales' calibration from nvmem device */
+	ret = nvmem_cell_read_u32(data->dev, "big_scale_calib", &val);
+	if (ret)
+		goto disable_clk;
+
+	sc27xx_adc_scale_calibration(&big_scale_graph_calib, val, true);
+
+	ret = nvmem_cell_read_u32(data->dev, "small_scale_calib", &val);
+	if (ret)
+		goto disable_clk;
+
+	sc27xx_adc_scale_calibration(&small_scale_graph_calib, val, false);
 	return 0;
 
 disable_clk:
-- 
1.7.9.5


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

* Re: [PATCH 1/2] iio: adc: sc27xx: Add raw data support
  2018-08-24  9:53 [PATCH 1/2] iio: adc: sc27xx: Add raw data support Baolin Wang
  2018-08-24  9:53 ` [PATCH 2/2] iio: adc: sc27xx: Add ADC scale calibration Baolin Wang
@ 2018-08-25  8:38 ` Jonathan Cameron
  2018-08-28  3:14   ` Baolin Wang
  1 sibling, 1 reply; 6+ messages in thread
From: Jonathan Cameron @ 2018-08-25  8:38 UTC (permalink / raw)
  To: Baolin Wang
  Cc: robh+dt, mark.rutland, knaack.h, lars, pmeerw, freeman.liu,
	broonie, devicetree, linux-iio, linux-kernel

On Fri, 24 Aug 2018 17:53:15 +0800
Baolin Wang <baolin.wang@linaro.org> wrote:

> The headset device will use channel 20 of ADC controller to detect events,
> but it needs the raw ADC data to do conversion according to its own formula.
> 
> Thus we should configure the channel mask separately and configure channel
> 20 as IIO_CHAN_INFO_RAW, as well as adding raw data read support.

So is this a general thing, i.e. that channel is 'meant' to be used for the
headset, or just a one off for a particular board?

If it is a general thing, than I'm fine with this (unlikely we'll break any
other users), but if not we need to find a nicer way to do it.
I am a little unclear on how a channel would provide the voltage on it's pin
but that could be wrong when used for a different purpose?
If it's just a matter of unusual loading characteristics then perhaps that
is valid, but I'd like to understand this a little.

> 
> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
> ---
>  drivers/iio/adc/sc27xx_adc.c |   80 ++++++++++++++++++++++++------------------
>  1 file changed, 45 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/iio/adc/sc27xx_adc.c b/drivers/iio/adc/sc27xx_adc.c
> index 2b60efe..153c311 100644
> --- a/drivers/iio/adc/sc27xx_adc.c
> +++ b/drivers/iio/adc/sc27xx_adc.c
> @@ -273,6 +273,17 @@ static int sc27xx_adc_read_raw(struct iio_dev *indio_dev,
>  	int ret, tmp;
>  
>  	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		mutex_lock(&indio_dev->mlock);
> +		ret = sc27xx_adc_read(data, chan->channel, scale, &tmp);
> +		mutex_unlock(&indio_dev->mlock);
> +
> +		if (ret)
> +			return ret;
> +
> +		*val = tmp;
> +		return IIO_VAL_INT;
> +
>  	case IIO_CHAN_INFO_PROCESSED:
>  		mutex_lock(&indio_dev->mlock);
>  		ret = sc27xx_adc_read_processed(data, chan->channel, scale,
> @@ -315,48 +326,47 @@ static int sc27xx_adc_write_raw(struct iio_dev *indio_dev,
>  	.write_raw = &sc27xx_adc_write_raw,
>  };
>  
> -#define SC27XX_ADC_CHANNEL(index) {				\
> +#define SC27XX_ADC_CHANNEL(index, mask) {			\
>  	.type = IIO_VOLTAGE,					\
>  	.channel = index,					\
> -	.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |	\
> -			      BIT(IIO_CHAN_INFO_SCALE),		\
> +	.info_mask_separate = mask | BIT(IIO_CHAN_INFO_SCALE),	\
>  	.datasheet_name = "CH##index",				\
>  	.indexed = 1,						\
>  }
>  
>  static const struct iio_chan_spec sc27xx_channels[] = {
> -	SC27XX_ADC_CHANNEL(0),
> -	SC27XX_ADC_CHANNEL(1),
> -	SC27XX_ADC_CHANNEL(2),
> -	SC27XX_ADC_CHANNEL(3),
> -	SC27XX_ADC_CHANNEL(4),
> -	SC27XX_ADC_CHANNEL(5),
> -	SC27XX_ADC_CHANNEL(6),
> -	SC27XX_ADC_CHANNEL(7),
> -	SC27XX_ADC_CHANNEL(8),
> -	SC27XX_ADC_CHANNEL(9),
> -	SC27XX_ADC_CHANNEL(10),
> -	SC27XX_ADC_CHANNEL(11),
> -	SC27XX_ADC_CHANNEL(12),
> -	SC27XX_ADC_CHANNEL(13),
> -	SC27XX_ADC_CHANNEL(14),
> -	SC27XX_ADC_CHANNEL(15),
> -	SC27XX_ADC_CHANNEL(16),
> -	SC27XX_ADC_CHANNEL(17),
> -	SC27XX_ADC_CHANNEL(18),
> -	SC27XX_ADC_CHANNEL(19),
> -	SC27XX_ADC_CHANNEL(20),
> -	SC27XX_ADC_CHANNEL(21),
> -	SC27XX_ADC_CHANNEL(22),
> -	SC27XX_ADC_CHANNEL(23),
> -	SC27XX_ADC_CHANNEL(24),
> -	SC27XX_ADC_CHANNEL(25),
> -	SC27XX_ADC_CHANNEL(26),
> -	SC27XX_ADC_CHANNEL(27),
> -	SC27XX_ADC_CHANNEL(28),
> -	SC27XX_ADC_CHANNEL(29),
> -	SC27XX_ADC_CHANNEL(30),
> -	SC27XX_ADC_CHANNEL(31),
> +	SC27XX_ADC_CHANNEL(0, BIT(IIO_CHAN_INFO_PROCESSED)),
> +	SC27XX_ADC_CHANNEL(1, BIT(IIO_CHAN_INFO_PROCESSED)),
> +	SC27XX_ADC_CHANNEL(2, BIT(IIO_CHAN_INFO_PROCESSED)),
> +	SC27XX_ADC_CHANNEL(3, BIT(IIO_CHAN_INFO_PROCESSED)),
> +	SC27XX_ADC_CHANNEL(4, BIT(IIO_CHAN_INFO_PROCESSED)),
> +	SC27XX_ADC_CHANNEL(5, BIT(IIO_CHAN_INFO_PROCESSED)),
> +	SC27XX_ADC_CHANNEL(6, BIT(IIO_CHAN_INFO_PROCESSED)),
> +	SC27XX_ADC_CHANNEL(7, BIT(IIO_CHAN_INFO_PROCESSED)),
> +	SC27XX_ADC_CHANNEL(8, BIT(IIO_CHAN_INFO_PROCESSED)),
> +	SC27XX_ADC_CHANNEL(9, BIT(IIO_CHAN_INFO_PROCESSED)),
> +	SC27XX_ADC_CHANNEL(10, BIT(IIO_CHAN_INFO_PROCESSED)),
> +	SC27XX_ADC_CHANNEL(11, BIT(IIO_CHAN_INFO_PROCESSED)),
> +	SC27XX_ADC_CHANNEL(12, BIT(IIO_CHAN_INFO_PROCESSED)),
> +	SC27XX_ADC_CHANNEL(13, BIT(IIO_CHAN_INFO_PROCESSED)),
> +	SC27XX_ADC_CHANNEL(14, BIT(IIO_CHAN_INFO_PROCESSED)),
> +	SC27XX_ADC_CHANNEL(15, BIT(IIO_CHAN_INFO_PROCESSED)),
> +	SC27XX_ADC_CHANNEL(16, BIT(IIO_CHAN_INFO_PROCESSED)),
> +	SC27XX_ADC_CHANNEL(17, BIT(IIO_CHAN_INFO_PROCESSED)),
> +	SC27XX_ADC_CHANNEL(18, BIT(IIO_CHAN_INFO_PROCESSED)),
> +	SC27XX_ADC_CHANNEL(19, BIT(IIO_CHAN_INFO_PROCESSED)),
> +	SC27XX_ADC_CHANNEL(20, BIT(IIO_CHAN_INFO_RAW)),
> +	SC27XX_ADC_CHANNEL(21, BIT(IIO_CHAN_INFO_PROCESSED)),
> +	SC27XX_ADC_CHANNEL(22, BIT(IIO_CHAN_INFO_PROCESSED)),
> +	SC27XX_ADC_CHANNEL(23, BIT(IIO_CHAN_INFO_PROCESSED)),
> +	SC27XX_ADC_CHANNEL(24, BIT(IIO_CHAN_INFO_PROCESSED)),
> +	SC27XX_ADC_CHANNEL(25, BIT(IIO_CHAN_INFO_PROCESSED)),
> +	SC27XX_ADC_CHANNEL(26, BIT(IIO_CHAN_INFO_PROCESSED)),
> +	SC27XX_ADC_CHANNEL(27, BIT(IIO_CHAN_INFO_PROCESSED)),
> +	SC27XX_ADC_CHANNEL(28, BIT(IIO_CHAN_INFO_PROCESSED)),
> +	SC27XX_ADC_CHANNEL(29, BIT(IIO_CHAN_INFO_PROCESSED)),
> +	SC27XX_ADC_CHANNEL(30, BIT(IIO_CHAN_INFO_PROCESSED)),
> +	SC27XX_ADC_CHANNEL(31, BIT(IIO_CHAN_INFO_PROCESSED)),
>  };
>  
>  static int sc27xx_adc_enable(struct sc27xx_adc_data *data)


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

* Re: [PATCH 2/2] iio: adc: sc27xx: Add ADC scale calibration
  2018-08-24  9:53 ` [PATCH 2/2] iio: adc: sc27xx: Add ADC scale calibration Baolin Wang
@ 2018-08-25  8:43   ` Jonathan Cameron
  2018-08-28  3:20     ` Baolin Wang
  0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Cameron @ 2018-08-25  8:43 UTC (permalink / raw)
  To: Baolin Wang
  Cc: robh+dt, mark.rutland, knaack.h, lars, pmeerw, freeman.liu,
	broonie, devicetree, linux-iio, linux-kernel

On Fri, 24 Aug 2018 17:53:16 +0800
Baolin Wang <baolin.wang@linaro.org> wrote:

> This patch adds support to read calibration values from the eFuse
> controller to calibrate the ADC channel scales, which can make ADC
> sample data more accurate.
> 
> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
This looks good to me.  I'll leave it for now though given open questions
on patch 1 and to allow others time to comment on this if they wish -
particularly as it includes a binding addition.

Jonathan

> ---
>  .../bindings/iio/adc/sprd,sc27xx-adc.txt           |    4 ++
>  drivers/iio/adc/sc27xx_adc.c                       |   52 ++++++++++++++++++--
>  2 files changed, 53 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/sprd,sc27xx-adc.txt b/Documentation/devicetree/bindings/iio/adc/sprd,sc27xx-adc.txt
> index 8aad960..b4daa15 100644
> --- a/Documentation/devicetree/bindings/iio/adc/sprd,sc27xx-adc.txt
> +++ b/Documentation/devicetree/bindings/iio/adc/sprd,sc27xx-adc.txt
> @@ -12,6 +12,8 @@ Required properties:
>  - interrupts: The interrupt number for the ADC device.
>  - #io-channel-cells: Number of cells in an IIO specifier.
>  - hwlocks: Reference to a phandle of a hwlock provider node.
> +- nvmem-cells: A phandle to the calibration cells provided by eFuse device.
> +- nvmem-cell-names: Should be "big_scale_calib", "small_scale_calib".
>  
>  Example:
>  
> @@ -32,5 +34,7 @@ Example:
>  			interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
>  			#io-channel-cells = <1>;
>  			hwlocks = <&hwlock 4>;
> +			nvmem-cells = <&adc_big_scale>, <&adc_small_scale>;
> +			nvmem-cell-names = "big_scale_calib", "small_scale_calib";
>  		};
>  	};
> diff --git a/drivers/iio/adc/sc27xx_adc.c b/drivers/iio/adc/sc27xx_adc.c
> index 153c311..7ac78eda 100644
> --- a/drivers/iio/adc/sc27xx_adc.c
> +++ b/drivers/iio/adc/sc27xx_adc.c
> @@ -5,6 +5,7 @@
>  #include <linux/iio/iio.h>
>  #include <linux/interrupt.h>
>  #include <linux/module.h>
> +#include <linux/nvmem-consumer.h>
>  #include <linux/of.h>
>  #include <linux/of_device.h>
>  #include <linux/platform_device.h>
> @@ -87,16 +88,48 @@ struct sc27xx_adc_linear_graph {
>   * should use the small-scale graph, and if more than 1.2v, we should use the
>   * big-scale graph.
>   */
> -static const struct sc27xx_adc_linear_graph big_scale_graph = {
> +static struct sc27xx_adc_linear_graph big_scale_graph = {
>  	4200, 3310,
>  	3600, 2832,
>  };
>  
> -static const struct sc27xx_adc_linear_graph small_scale_graph = {
> +static struct sc27xx_adc_linear_graph small_scale_graph = {
>  	1000, 3413,
>  	100, 341,
>  };
>  
> +static const struct sc27xx_adc_linear_graph big_scale_graph_calib = {
> +	4200, 856,
> +	3600, 733,
> +};
> +
> +static const struct sc27xx_adc_linear_graph small_scale_graph_calib = {
> +	1000, 833,
> +	100, 80,
> +};
> +
> +static int sc27xx_adc_get_calib_data(u32 calib_data, int calib_adc)
> +{
> +	return ((calib_data & 0xff) + calib_adc - 128) * 4;
> +}
> +
> +static void
> +sc27xx_adc_scale_calibration(const struct sc27xx_adc_linear_graph *calib_graph,
> +			     u32 calib_data, bool big_scale)
> +{
> +	struct sc27xx_adc_linear_graph *graph;
> +
> +	if (big_scale)
> +		graph = &big_scale_graph;
> +	else
> +		graph = &small_scale_graph;
> +
> +	/* Only need to calibrate the adc values in the linear graph. */
> +	graph->adc0 = sc27xx_adc_get_calib_data(calib_data, calib_graph->adc0);
> +	graph->adc1 = sc27xx_adc_get_calib_data(calib_data >> 8,
> +						calib_graph->adc1);
> +}
> +
>  static int sc27xx_adc_get_ratio(int channel, int scale)
>  {
>  	switch (channel) {
> @@ -209,7 +242,7 @@ static void sc27xx_adc_volt_ratio(struct sc27xx_adc_data *data,
>  	*div_denominator = ratio & SC27XX_RATIO_DENOMINATOR_MASK;
>  }
>  
> -static int sc27xx_adc_to_volt(const struct sc27xx_adc_linear_graph *graph,
> +static int sc27xx_adc_to_volt(struct sc27xx_adc_linear_graph *graph,
>  			      int raw_adc)
>  {
>  	int tmp;
> @@ -371,6 +404,7 @@ static int sc27xx_adc_write_raw(struct iio_dev *indio_dev,
>  
>  static int sc27xx_adc_enable(struct sc27xx_adc_data *data)
>  {
> +	u32 val;
>  	int ret;
>  
>  	ret = regmap_update_bits(data->regmap, SC27XX_MODULE_EN,
> @@ -390,6 +424,18 @@ static int sc27xx_adc_enable(struct sc27xx_adc_data *data)
>  	if (ret)
>  		goto disable_clk;
>  
> +	/* ADC channel scales' calibration from nvmem device */
> +	ret = nvmem_cell_read_u32(data->dev, "big_scale_calib", &val);
> +	if (ret)
> +		goto disable_clk;
> +
> +	sc27xx_adc_scale_calibration(&big_scale_graph_calib, val, true);
> +
> +	ret = nvmem_cell_read_u32(data->dev, "small_scale_calib", &val);
> +	if (ret)
> +		goto disable_clk;
> +
> +	sc27xx_adc_scale_calibration(&small_scale_graph_calib, val, false);
>  	return 0;
>  
>  disable_clk:


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

* Re: [PATCH 1/2] iio: adc: sc27xx: Add raw data support
  2018-08-25  8:38 ` [PATCH 1/2] iio: adc: sc27xx: Add raw data support Jonathan Cameron
@ 2018-08-28  3:14   ` Baolin Wang
  0 siblings, 0 replies; 6+ messages in thread
From: Baolin Wang @ 2018-08-28  3:14 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Rob Herring, Mark Rutland, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, freeman.liu, Mark Brown, DTML, linux-iio,
	LKML

Hi Jonathan,

On 25 August 2018 at 16:38, Jonathan Cameron <jic23@kernel.org> wrote:
> On Fri, 24 Aug 2018 17:53:15 +0800
> Baolin Wang <baolin.wang@linaro.org> wrote:
>
>> The headset device will use channel 20 of ADC controller to detect events,
>> but it needs the raw ADC data to do conversion according to its own formula.
>>
>> Thus we should configure the channel mask separately and configure channel
>> 20 as IIO_CHAN_INFO_RAW, as well as adding raw data read support.
>
> So is this a general thing, i.e. that channel is 'meant' to be used for the
> headset, or just a one off for a particular board?
> If it is a general thing, than I'm fine with this (unlikely we'll break any
> other users), but if not we need to find a nicer way to do it.

Yes, it is a general thing. Now channel 20 is always used for the headset.

> I am a little unclear on how a channel would provide the voltage on it's pin
> but that could be wrong when used for a different purpose?
> If it's just a matter of unusual loading characteristics then perhaps that
> is valid, but I'd like to understand this a little.

On our platform, the headset will use this voltage value reading from
ADC to check the headset events: headset buttons, headset plugin and
so on.

>>
>> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
>> ---
>>  drivers/iio/adc/sc27xx_adc.c |   80 ++++++++++++++++++++++++------------------
>>  1 file changed, 45 insertions(+), 35 deletions(-)
>>
>> diff --git a/drivers/iio/adc/sc27xx_adc.c b/drivers/iio/adc/sc27xx_adc.c
>> index 2b60efe..153c311 100644
>> --- a/drivers/iio/adc/sc27xx_adc.c
>> +++ b/drivers/iio/adc/sc27xx_adc.c
>> @@ -273,6 +273,17 @@ static int sc27xx_adc_read_raw(struct iio_dev *indio_dev,
>>       int ret, tmp;
>>
>>       switch (mask) {
>> +     case IIO_CHAN_INFO_RAW:
>> +             mutex_lock(&indio_dev->mlock);
>> +             ret = sc27xx_adc_read(data, chan->channel, scale, &tmp);
>> +             mutex_unlock(&indio_dev->mlock);
>> +
>> +             if (ret)
>> +                     return ret;
>> +
>> +             *val = tmp;
>> +             return IIO_VAL_INT;
>> +
>>       case IIO_CHAN_INFO_PROCESSED:
>>               mutex_lock(&indio_dev->mlock);
>>               ret = sc27xx_adc_read_processed(data, chan->channel, scale,
>> @@ -315,48 +326,47 @@ static int sc27xx_adc_write_raw(struct iio_dev *indio_dev,
>>       .write_raw = &sc27xx_adc_write_raw,
>>  };
>>
>> -#define SC27XX_ADC_CHANNEL(index) {                          \
>> +#define SC27XX_ADC_CHANNEL(index, mask) {                    \
>>       .type = IIO_VOLTAGE,                                    \
>>       .channel = index,                                       \
>> -     .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |    \
>> -                           BIT(IIO_CHAN_INFO_SCALE),         \
>> +     .info_mask_separate = mask | BIT(IIO_CHAN_INFO_SCALE),  \
>>       .datasheet_name = "CH##index",                          \
>>       .indexed = 1,                                           \
>>  }
>>
>>  static const struct iio_chan_spec sc27xx_channels[] = {
>> -     SC27XX_ADC_CHANNEL(0),
>> -     SC27XX_ADC_CHANNEL(1),
>> -     SC27XX_ADC_CHANNEL(2),
>> -     SC27XX_ADC_CHANNEL(3),
>> -     SC27XX_ADC_CHANNEL(4),
>> -     SC27XX_ADC_CHANNEL(5),
>> -     SC27XX_ADC_CHANNEL(6),
>> -     SC27XX_ADC_CHANNEL(7),
>> -     SC27XX_ADC_CHANNEL(8),
>> -     SC27XX_ADC_CHANNEL(9),
>> -     SC27XX_ADC_CHANNEL(10),
>> -     SC27XX_ADC_CHANNEL(11),
>> -     SC27XX_ADC_CHANNEL(12),
>> -     SC27XX_ADC_CHANNEL(13),
>> -     SC27XX_ADC_CHANNEL(14),
>> -     SC27XX_ADC_CHANNEL(15),
>> -     SC27XX_ADC_CHANNEL(16),
>> -     SC27XX_ADC_CHANNEL(17),
>> -     SC27XX_ADC_CHANNEL(18),
>> -     SC27XX_ADC_CHANNEL(19),
>> -     SC27XX_ADC_CHANNEL(20),
>> -     SC27XX_ADC_CHANNEL(21),
>> -     SC27XX_ADC_CHANNEL(22),
>> -     SC27XX_ADC_CHANNEL(23),
>> -     SC27XX_ADC_CHANNEL(24),
>> -     SC27XX_ADC_CHANNEL(25),
>> -     SC27XX_ADC_CHANNEL(26),
>> -     SC27XX_ADC_CHANNEL(27),
>> -     SC27XX_ADC_CHANNEL(28),
>> -     SC27XX_ADC_CHANNEL(29),
>> -     SC27XX_ADC_CHANNEL(30),
>> -     SC27XX_ADC_CHANNEL(31),
>> +     SC27XX_ADC_CHANNEL(0, BIT(IIO_CHAN_INFO_PROCESSED)),
>> +     SC27XX_ADC_CHANNEL(1, BIT(IIO_CHAN_INFO_PROCESSED)),
>> +     SC27XX_ADC_CHANNEL(2, BIT(IIO_CHAN_INFO_PROCESSED)),
>> +     SC27XX_ADC_CHANNEL(3, BIT(IIO_CHAN_INFO_PROCESSED)),
>> +     SC27XX_ADC_CHANNEL(4, BIT(IIO_CHAN_INFO_PROCESSED)),
>> +     SC27XX_ADC_CHANNEL(5, BIT(IIO_CHAN_INFO_PROCESSED)),
>> +     SC27XX_ADC_CHANNEL(6, BIT(IIO_CHAN_INFO_PROCESSED)),
>> +     SC27XX_ADC_CHANNEL(7, BIT(IIO_CHAN_INFO_PROCESSED)),
>> +     SC27XX_ADC_CHANNEL(8, BIT(IIO_CHAN_INFO_PROCESSED)),
>> +     SC27XX_ADC_CHANNEL(9, BIT(IIO_CHAN_INFO_PROCESSED)),
>> +     SC27XX_ADC_CHANNEL(10, BIT(IIO_CHAN_INFO_PROCESSED)),
>> +     SC27XX_ADC_CHANNEL(11, BIT(IIO_CHAN_INFO_PROCESSED)),
>> +     SC27XX_ADC_CHANNEL(12, BIT(IIO_CHAN_INFO_PROCESSED)),
>> +     SC27XX_ADC_CHANNEL(13, BIT(IIO_CHAN_INFO_PROCESSED)),
>> +     SC27XX_ADC_CHANNEL(14, BIT(IIO_CHAN_INFO_PROCESSED)),
>> +     SC27XX_ADC_CHANNEL(15, BIT(IIO_CHAN_INFO_PROCESSED)),
>> +     SC27XX_ADC_CHANNEL(16, BIT(IIO_CHAN_INFO_PROCESSED)),
>> +     SC27XX_ADC_CHANNEL(17, BIT(IIO_CHAN_INFO_PROCESSED)),
>> +     SC27XX_ADC_CHANNEL(18, BIT(IIO_CHAN_INFO_PROCESSED)),
>> +     SC27XX_ADC_CHANNEL(19, BIT(IIO_CHAN_INFO_PROCESSED)),
>> +     SC27XX_ADC_CHANNEL(20, BIT(IIO_CHAN_INFO_RAW)),
>> +     SC27XX_ADC_CHANNEL(21, BIT(IIO_CHAN_INFO_PROCESSED)),
>> +     SC27XX_ADC_CHANNEL(22, BIT(IIO_CHAN_INFO_PROCESSED)),
>> +     SC27XX_ADC_CHANNEL(23, BIT(IIO_CHAN_INFO_PROCESSED)),
>> +     SC27XX_ADC_CHANNEL(24, BIT(IIO_CHAN_INFO_PROCESSED)),
>> +     SC27XX_ADC_CHANNEL(25, BIT(IIO_CHAN_INFO_PROCESSED)),
>> +     SC27XX_ADC_CHANNEL(26, BIT(IIO_CHAN_INFO_PROCESSED)),
>> +     SC27XX_ADC_CHANNEL(27, BIT(IIO_CHAN_INFO_PROCESSED)),
>> +     SC27XX_ADC_CHANNEL(28, BIT(IIO_CHAN_INFO_PROCESSED)),
>> +     SC27XX_ADC_CHANNEL(29, BIT(IIO_CHAN_INFO_PROCESSED)),
>> +     SC27XX_ADC_CHANNEL(30, BIT(IIO_CHAN_INFO_PROCESSED)),
>> +     SC27XX_ADC_CHANNEL(31, BIT(IIO_CHAN_INFO_PROCESSED)),
>>  };
>>
>>  static int sc27xx_adc_enable(struct sc27xx_adc_data *data)
>



-- 
Baolin Wang
Best Regards

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

* Re: [PATCH 2/2] iio: adc: sc27xx: Add ADC scale calibration
  2018-08-25  8:43   ` Jonathan Cameron
@ 2018-08-28  3:20     ` Baolin Wang
  0 siblings, 0 replies; 6+ messages in thread
From: Baolin Wang @ 2018-08-28  3:20 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Rob Herring, Mark Rutland, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, freeman.liu, Mark Brown, DTML, linux-iio,
	LKML

Hi Jonathan,

On 25 August 2018 at 16:43, Jonathan Cameron <jic23@kernel.org> wrote:
> On Fri, 24 Aug 2018 17:53:16 +0800
> Baolin Wang <baolin.wang@linaro.org> wrote:
>
>> This patch adds support to read calibration values from the eFuse
>> controller to calibrate the ADC channel scales, which can make ADC
>> sample data more accurate.
>>
>> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
> This looks good to me.  I'll leave it for now though given open questions
> on patch 1 and to allow others time to comment on this if they wish -
> particularly as it includes a binding addition.

Thanks for your reviewing. But after more testing, we found one issue
when reading calibration data from efuse, which is not always 4 bytes
(sizeof(u32)). So I will change to use nvmem_cell_read() instead of
nvmem_cell_read_u32() in next version.

-- 
Baolin Wang
Best Regards

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

end of thread, other threads:[~2018-08-28  3:20 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-24  9:53 [PATCH 1/2] iio: adc: sc27xx: Add raw data support Baolin Wang
2018-08-24  9:53 ` [PATCH 2/2] iio: adc: sc27xx: Add ADC scale calibration Baolin Wang
2018-08-25  8:43   ` Jonathan Cameron
2018-08-28  3:20     ` Baolin Wang
2018-08-25  8:38 ` [PATCH 1/2] iio: adc: sc27xx: Add raw data support Jonathan Cameron
2018-08-28  3:14   ` Baolin Wang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).