[PATCH 0/3] iio: adc: stm32: Add support for differential channels

[PATCH 0/3] iio: adc: stm32: Add support for differential channels

[Fabrice Gasnier]

Add support for differential channels that can be found on STM32H7 ADC:
- Introduce new DT property 'st,adc-diff-channels'.
- Slightly rework single-ended channels definition in stm32-adc driver.
- Add support for differential channels to stm32-adc driver.

Fabrice Gasnier (3):
  dt-bindings: iio: adc: stm32: add support for diff channels
  iio: adc: stm32: remove const channel names definition
  iio: adc: stm32: add support for differential channels

 .../devicetree/bindings/iio/adc/st,stm32-adc.txt   |   6 +
 drivers/iio/adc/stm32-adc.c                        | 181 +++++++++++++--------
 2 files changed, 119 insertions(+), 68 deletions(-)

-- 
1.9.1

[PATCH 0/3] iio: adc: stm32: Add support for differential channels

[Fabrice Gasnier]

Add support for differential channels that can be found on STM32H7 ADC:
- Introduce new DT property 'st,adc-diff-channels'.
- Slightly rework single-ended channels definition in stm32-adc driver.
- Add support for differential channels to stm32-adc driver.

Fabrice Gasnier (3):
  dt-bindings: iio: adc: stm32: add support for diff channels
  iio: adc: stm32: remove const channel names definition
  iio: adc: stm32: add support for differential channels

 .../devicetree/bindings/iio/adc/st,stm32-adc.txt   |   6 +
 drivers/iio/adc/stm32-adc.c                        | 181 +++++++++++++--------
 2 files changed, 119 insertions(+), 68 deletions(-)

-- 
1.9.1

[PATCH 0/3] iio: adc: stm32: Add support for differential channels

[Fabrice Gasnier]

Add support for differential channels that can be found on STM32H7 ADC:
- Introduce new DT property 'st,adc-diff-channels'.
- Slightly rework single-ended channels definition in stm32-adc driver.
- Add support for differential channels to stm32-adc driver.

Fabrice Gasnier (3):
  dt-bindings: iio: adc: stm32: add support for diff channels
  iio: adc: stm32: remove const channel names definition
  iio: adc: stm32: add support for differential channels

 .../devicetree/bindings/iio/adc/st,stm32-adc.txt   |   6 +
 drivers/iio/adc/stm32-adc.c                        | 181 +++++++++++++--------
 2 files changed, 119 insertions(+), 68 deletions(-)

-- 
1.9.1

[PATCH 1/3] dt-bindings: iio: adc: stm32: add support for diff channels

[Fabrice Gasnier]

STM32H7 ADC channels may be configured either as single-ended or
differential.
Add 'st,adc-diff-channels' property to support differential channels.
Differential channels are defined as a pair of positive and negative
inputs: vinp & vinn.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
---
 Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
index 48bfcaa3..c3c8de1 100644
--- a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
+++ b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
@@ -62,6 +62,12 @@ Required properties:
 - st,adc-channels: List of single-ended channels muxed for this ADC.
   It can have up to 16 channels on stm32f4 or 20 channels on stm32h7, numbered
   from 0 to 15 or 19 (resp. for in0..in15 or in0..in19).
+- st,adc-diff-channels: List of differential channels muxed for this ADC.
+  Depending on part used, some channels can be configured as differential
+  instead of single-ended (e.g. stm32h7). List here positive and negative
+  inputs pairs as <vinp vinn>, <vinp vinn>,... vinp and vinn are numbered
+  from 0 to 19 on stm32h7)
+  Note: At least one of "st,adc-channels" or "st,adc-diff-channels" is required.
 - #io-channel-cells = <1>: See the IIO bindings section "IIO consumers" in
   Documentation/devicetree/bindings/iio/iio-bindings.txt
 
-- 
1.9.1

[PATCH 1/3] dt-bindings: iio: adc: stm32: add support for diff channels

[Fabrice Gasnier]

STM32H7 ADC channels may be configured either as single-ended or
differential.
Add 'st,adc-diff-channels' property to support differential channels.
Differential channels are defined as a pair of positive and negative
inputs: vinp & vinn.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
---
 Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
index 48bfcaa3..c3c8de1 100644
--- a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
+++ b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
@@ -62,6 +62,12 @@ Required properties:
 - st,adc-channels: List of single-ended channels muxed for this ADC.
   It can have up to 16 channels on stm32f4 or 20 channels on stm32h7, numbered
   from 0 to 15 or 19 (resp. for in0..in15 or in0..in19).
+- st,adc-diff-channels: List of differential channels muxed for this ADC.
+  Depending on part used, some channels can be configured as differential
+  instead of single-ended (e.g. stm32h7). List here positive and negative
+  inputs pairs as <vinp vinn>, <vinp vinn>,... vinp and vinn are numbered
+  from 0 to 19 on stm32h7)
+  Note: At least one of "st,adc-channels" or "st,adc-diff-channels" is required.
 - #io-channel-cells = <1>: See the IIO bindings section "IIO consumers" in
   Documentation/devicetree/bindings/iio/iio-bindings.txt
 
-- 
1.9.1

[PATCH 1/3] dt-bindings: iio: adc: stm32: add support for diff channels

[Fabrice Gasnier]

STM32H7 ADC channels may be configured either as single-ended or
differential.
Add 'st,adc-diff-channels' property to support differential channels.
Differential channels are defined as a pair of positive and negative
inputs: vinp & vinn.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
---
 Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
index 48bfcaa3..c3c8de1 100644
--- a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
+++ b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
@@ -62,6 +62,12 @@ Required properties:
 - st,adc-channels: List of single-ended channels muxed for this ADC.
   It can have up to 16 channels on stm32f4 or 20 channels on stm32h7, numbered
   from 0 to 15 or 19 (resp. for in0..in15 or in0..in19).
+- st,adc-diff-channels: List of differential channels muxed for this ADC.
+  Depending on part used, some channels can be configured as differential
+  instead of single-ended (e.g. stm32h7). List here positive and negative
+  inputs pairs as <vinp vinn>, <vinp vinn>,... vinp and vinn are numbered
+  from 0 to 19 on stm32h7)
+  Note: At least one of "st,adc-channels" or "st,adc-diff-channels" is required.
 - #io-channel-cells = <1>: See the IIO bindings section "IIO consumers" in
   Documentation/devicetree/bindings/iio/iio-bindings.txt
 
-- 
1.9.1

[PATCH 2/3] iio: adc: stm32: remove const channel names definition

[Fabrice Gasnier]

Remove const array that defines channels. Build channels definition
at probe time, when initializing channels (only for requested ones).
This will ease adding differential channels support.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
---
 drivers/iio/adc/stm32-adc.c | 66 ++++++++++-----------------------------------
 1 file changed, 14 insertions(+), 52 deletions(-)

diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
index 4df32cf..c95e6f7 100644
--- a/drivers/iio/adc/stm32-adc.c
+++ b/drivers/iio/adc/stm32-adc.c
@@ -153,6 +153,8 @@ enum stm32h7_adc_dmngt {
 /* BOOST bit must be set on STM32H7 when ADC clock is above 20MHz */
 #define STM32H7_BOOST_CLKRATE		20000000UL
 
+#define STM32_ADC_CH_MAX		20	/* max number of channels */
+#define STM32_ADC_CH_SZ			5	/* max channel name size */
 #define STM32_ADC_MAX_SQ		16	/* SQ1..SQ16 */
 #define STM32_ADC_MAX_SMP		7	/* SMPx range is [0..7] */
 #define STM32_ADC_TIMEOUT_US		100000
@@ -300,6 +302,7 @@ struct stm32_adc_cfg {
  * @pcsel		bitmask to preselect channels on some devices
  * @smpr_val:		sampling time settings (e.g. smpr1 / smpr2)
  * @cal:		optional calibration data on some devices
+ * @chan_name:		channel name array
  */
 struct stm32_adc {
 	struct stm32_adc_common	*common;
@@ -321,69 +324,28 @@ struct stm32_adc {
 	u32			pcsel;
 	u32			smpr_val[2];
 	struct stm32_adc_calib	cal;
-};
-
-/**
- * struct stm32_adc_chan_spec - specification of stm32 adc channel
- * @type:	IIO channel type
- * @channel:	channel number (single ended)
- * @name:	channel name (single ended)
- */
-struct stm32_adc_chan_spec {
-	enum iio_chan_type	type;
-	int			channel;
-	const char		*name;
+	char			chan_name[STM32_ADC_CH_MAX][STM32_ADC_CH_SZ];
 };
 
 /**
  * struct stm32_adc_info - stm32 ADC, per instance config data
- * @channels:		Reference to stm32 channels spec
  * @max_channels:	Number of channels
  * @resolutions:	available resolutions
  * @num_res:		number of available resolutions
  */
 struct stm32_adc_info {
-	const struct stm32_adc_chan_spec *channels;
 	int max_channels;
 	const unsigned int *resolutions;
 	const unsigned int num_res;
 };
 
-/*
- * Input definitions common for all instances:
- * stm32f4 can have up to 16 channels
- * stm32h7 can have up to 20 channels
- */
-static const struct stm32_adc_chan_spec stm32_adc_channels[] = {
-	{ IIO_VOLTAGE, 0, "in0" },
-	{ IIO_VOLTAGE, 1, "in1" },
-	{ IIO_VOLTAGE, 2, "in2" },
-	{ IIO_VOLTAGE, 3, "in3" },
-	{ IIO_VOLTAGE, 4, "in4" },
-	{ IIO_VOLTAGE, 5, "in5" },
-	{ IIO_VOLTAGE, 6, "in6" },
-	{ IIO_VOLTAGE, 7, "in7" },
-	{ IIO_VOLTAGE, 8, "in8" },
-	{ IIO_VOLTAGE, 9, "in9" },
-	{ IIO_VOLTAGE, 10, "in10" },
-	{ IIO_VOLTAGE, 11, "in11" },
-	{ IIO_VOLTAGE, 12, "in12" },
-	{ IIO_VOLTAGE, 13, "in13" },
-	{ IIO_VOLTAGE, 14, "in14" },
-	{ IIO_VOLTAGE, 15, "in15" },
-	{ IIO_VOLTAGE, 16, "in16" },
-	{ IIO_VOLTAGE, 17, "in17" },
-	{ IIO_VOLTAGE, 18, "in18" },
-	{ IIO_VOLTAGE, 19, "in19" },
-};
-
 static const unsigned int stm32f4_adc_resolutions[] = {
 	/* sorted values so the index matches RES[1:0] in STM32F4_ADC_CR1 */
 	12, 10, 8, 6,
 };
 
+/* stm32f4 can have up to 16 channels */
 static const struct stm32_adc_info stm32f4_adc_info = {
-	.channels = stm32_adc_channels,
 	.max_channels = 16,
 	.resolutions = stm32f4_adc_resolutions,
 	.num_res = ARRAY_SIZE(stm32f4_adc_resolutions),
@@ -394,9 +356,9 @@ struct stm32_adc_info {
 	16, 14, 12, 10, 8,
 };
 
+/* stm32h7 can have up to 20 channels */
 static const struct stm32_adc_info stm32h7_adc_info = {
-	.channels = stm32_adc_channels,
-	.max_channels = 20,
+	.max_channels = STM32_ADC_CH_MAX,
 	.resolutions = stm32h7_adc_resolutions,
 	.num_res = ARRAY_SIZE(stm32h7_adc_resolutions),
 };
@@ -1628,15 +1590,16 @@ static void stm32_adc_smpr_init(struct stm32_adc *adc, int channel, u32 smp_ns)
 }
 
 static void stm32_adc_chan_init_one(struct iio_dev *indio_dev,
-				    struct iio_chan_spec *chan,
-				    const struct stm32_adc_chan_spec *channel,
+				    struct iio_chan_spec *chan, u32 val,
 				    int scan_index, u32 smp)
 {
 	struct stm32_adc *adc = iio_priv(indio_dev);
+	char *name = adc->chan_name[val];
 
-	chan->type = channel->type;
-	chan->channel = channel->channel;
-	chan->datasheet_name = channel->name;
+	chan->type = IIO_VOLTAGE;
+	chan->channel = val;
+	snprintf(name, STM32_ADC_CH_SZ, "in%d", val);
+	chan->datasheet_name = name;
 	chan->scan_index = scan_index;
 	chan->indexed = 1;
 	chan->info_mask_separate = BIT(IIO_CHAN_INFO_RAW);
@@ -1699,8 +1662,7 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
 					   scan_index, &smp);
 
 		stm32_adc_chan_init_one(indio_dev, &channels[scan_index],
-					&adc_info->channels[val],
-					scan_index, smp);
+					val, scan_index, smp);
 		scan_index++;
 	}
 
-- 
1.9.1

[PATCH 2/3] iio: adc: stm32: remove const channel names definition

[Fabrice Gasnier]

Remove const array that defines channels. Build channels definition
at probe time, when initializing channels (only for requested ones).
This will ease adding differential channels support.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
---
 drivers/iio/adc/stm32-adc.c | 66 ++++++++++-----------------------------------
 1 file changed, 14 insertions(+), 52 deletions(-)

diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
index 4df32cf..c95e6f7 100644
--- a/drivers/iio/adc/stm32-adc.c
+++ b/drivers/iio/adc/stm32-adc.c
@@ -153,6 +153,8 @@ enum stm32h7_adc_dmngt {
 /* BOOST bit must be set on STM32H7 when ADC clock is above 20MHz */
 #define STM32H7_BOOST_CLKRATE		20000000UL
 
+#define STM32_ADC_CH_MAX		20	/* max number of channels */
+#define STM32_ADC_CH_SZ			5	/* max channel name size */
 #define STM32_ADC_MAX_SQ		16	/* SQ1..SQ16 */
 #define STM32_ADC_MAX_SMP		7	/* SMPx range is [0..7] */
 #define STM32_ADC_TIMEOUT_US		100000
@@ -300,6 +302,7 @@ struct stm32_adc_cfg {
  * @pcsel		bitmask to preselect channels on some devices
  * @smpr_val:		sampling time settings (e.g. smpr1 / smpr2)
  * @cal:		optional calibration data on some devices
+ * @chan_name:		channel name array
  */
 struct stm32_adc {
 	struct stm32_adc_common	*common;
@@ -321,69 +324,28 @@ struct stm32_adc {
 	u32			pcsel;
 	u32			smpr_val[2];
 	struct stm32_adc_calib	cal;
-};
-
-/**
- * struct stm32_adc_chan_spec - specification of stm32 adc channel
- * @type:	IIO channel type
- * @channel:	channel number (single ended)
- * @name:	channel name (single ended)
- */
-struct stm32_adc_chan_spec {
-	enum iio_chan_type	type;
-	int			channel;
-	const char		*name;
+	char			chan_name[STM32_ADC_CH_MAX][STM32_ADC_CH_SZ];
 };
 
 /**
  * struct stm32_adc_info - stm32 ADC, per instance config data
- * @channels:		Reference to stm32 channels spec
  * @max_channels:	Number of channels
  * @resolutions:	available resolutions
  * @num_res:		number of available resolutions
  */
 struct stm32_adc_info {
-	const struct stm32_adc_chan_spec *channels;
 	int max_channels;
 	const unsigned int *resolutions;
 	const unsigned int num_res;
 };
 
-/*
- * Input definitions common for all instances:
- * stm32f4 can have up to 16 channels
- * stm32h7 can have up to 20 channels
- */
-static const struct stm32_adc_chan_spec stm32_adc_channels[] = {
-	{ IIO_VOLTAGE, 0, "in0" },
-	{ IIO_VOLTAGE, 1, "in1" },
-	{ IIO_VOLTAGE, 2, "in2" },
-	{ IIO_VOLTAGE, 3, "in3" },
-	{ IIO_VOLTAGE, 4, "in4" },
-	{ IIO_VOLTAGE, 5, "in5" },
-	{ IIO_VOLTAGE, 6, "in6" },
-	{ IIO_VOLTAGE, 7, "in7" },
-	{ IIO_VOLTAGE, 8, "in8" },
-	{ IIO_VOLTAGE, 9, "in9" },
-	{ IIO_VOLTAGE, 10, "in10" },
-	{ IIO_VOLTAGE, 11, "in11" },
-	{ IIO_VOLTAGE, 12, "in12" },
-	{ IIO_VOLTAGE, 13, "in13" },
-	{ IIO_VOLTAGE, 14, "in14" },
-	{ IIO_VOLTAGE, 15, "in15" },
-	{ IIO_VOLTAGE, 16, "in16" },
-	{ IIO_VOLTAGE, 17, "in17" },
-	{ IIO_VOLTAGE, 18, "in18" },
-	{ IIO_VOLTAGE, 19, "in19" },
-};
-
 static const unsigned int stm32f4_adc_resolutions[] = {
 	/* sorted values so the index matches RES[1:0] in STM32F4_ADC_CR1 */
 	12, 10, 8, 6,
 };
 
+/* stm32f4 can have up to 16 channels */
 static const struct stm32_adc_info stm32f4_adc_info = {
-	.channels = stm32_adc_channels,
 	.max_channels = 16,
 	.resolutions = stm32f4_adc_resolutions,
 	.num_res = ARRAY_SIZE(stm32f4_adc_resolutions),
@@ -394,9 +356,9 @@ struct stm32_adc_info {
 	16, 14, 12, 10, 8,
 };
 
+/* stm32h7 can have up to 20 channels */
 static const struct stm32_adc_info stm32h7_adc_info = {
-	.channels = stm32_adc_channels,
-	.max_channels = 20,
+	.max_channels = STM32_ADC_CH_MAX,
 	.resolutions = stm32h7_adc_resolutions,
 	.num_res = ARRAY_SIZE(stm32h7_adc_resolutions),
 };
@@ -1628,15 +1590,16 @@ static void stm32_adc_smpr_init(struct stm32_adc *adc, int channel, u32 smp_ns)
 }
 
 static void stm32_adc_chan_init_one(struct iio_dev *indio_dev,
-				    struct iio_chan_spec *chan,
-				    const struct stm32_adc_chan_spec *channel,
+				    struct iio_chan_spec *chan, u32 val,
 				    int scan_index, u32 smp)
 {
 	struct stm32_adc *adc = iio_priv(indio_dev);
+	char *name = adc->chan_name[val];
 
-	chan->type = channel->type;
-	chan->channel = channel->channel;
-	chan->datasheet_name = channel->name;
+	chan->type = IIO_VOLTAGE;
+	chan->channel = val;
+	snprintf(name, STM32_ADC_CH_SZ, "in%d", val);
+	chan->datasheet_name = name;
 	chan->scan_index = scan_index;
 	chan->indexed = 1;
 	chan->info_mask_separate = BIT(IIO_CHAN_INFO_RAW);
@@ -1699,8 +1662,7 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
 					   scan_index, &smp);
 
 		stm32_adc_chan_init_one(indio_dev, &channels[scan_index],
-					&adc_info->channels[val],
-					scan_index, smp);
+					val, scan_index, smp);
 		scan_index++;
 	}
 
-- 
1.9.1

[PATCH 2/3] iio: adc: stm32: remove const channel names definition

[Fabrice Gasnier]

Remove const array that defines channels. Build channels definition
at probe time, when initializing channels (only for requested ones).
This will ease adding differential channels support.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
---
 drivers/iio/adc/stm32-adc.c | 66 ++++++++++-----------------------------------
 1 file changed, 14 insertions(+), 52 deletions(-)

diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
index 4df32cf..c95e6f7 100644
--- a/drivers/iio/adc/stm32-adc.c
+++ b/drivers/iio/adc/stm32-adc.c
@@ -153,6 +153,8 @@ enum stm32h7_adc_dmngt {
 /* BOOST bit must be set on STM32H7 when ADC clock is above 20MHz */
 #define STM32H7_BOOST_CLKRATE		20000000UL
 
+#define STM32_ADC_CH_MAX		20	/* max number of channels */
+#define STM32_ADC_CH_SZ			5	/* max channel name size */
 #define STM32_ADC_MAX_SQ		16	/* SQ1..SQ16 */
 #define STM32_ADC_MAX_SMP		7	/* SMPx range is [0..7] */
 #define STM32_ADC_TIMEOUT_US		100000
@@ -300,6 +302,7 @@ struct stm32_adc_cfg {
  * @pcsel		bitmask to preselect channels on some devices
  * @smpr_val:		sampling time settings (e.g. smpr1 / smpr2)
  * @cal:		optional calibration data on some devices
+ * @chan_name:		channel name array
  */
 struct stm32_adc {
 	struct stm32_adc_common	*common;
@@ -321,69 +324,28 @@ struct stm32_adc {
 	u32			pcsel;
 	u32			smpr_val[2];
 	struct stm32_adc_calib	cal;
-};
-
-/**
- * struct stm32_adc_chan_spec - specification of stm32 adc channel
- * @type:	IIO channel type
- * @channel:	channel number (single ended)
- * @name:	channel name (single ended)
- */
-struct stm32_adc_chan_spec {
-	enum iio_chan_type	type;
-	int			channel;
-	const char		*name;
+	char			chan_name[STM32_ADC_CH_MAX][STM32_ADC_CH_SZ];
 };
 
 /**
  * struct stm32_adc_info - stm32 ADC, per instance config data
- * @channels:		Reference to stm32 channels spec
  * @max_channels:	Number of channels
  * @resolutions:	available resolutions
  * @num_res:		number of available resolutions
  */
 struct stm32_adc_info {
-	const struct stm32_adc_chan_spec *channels;
 	int max_channels;
 	const unsigned int *resolutions;
 	const unsigned int num_res;
 };
 
-/*
- * Input definitions common for all instances:
- * stm32f4 can have up to 16 channels
- * stm32h7 can have up to 20 channels
- */
-static const struct stm32_adc_chan_spec stm32_adc_channels[] = {
-	{ IIO_VOLTAGE, 0, "in0" },
-	{ IIO_VOLTAGE, 1, "in1" },
-	{ IIO_VOLTAGE, 2, "in2" },
-	{ IIO_VOLTAGE, 3, "in3" },
-	{ IIO_VOLTAGE, 4, "in4" },
-	{ IIO_VOLTAGE, 5, "in5" },
-	{ IIO_VOLTAGE, 6, "in6" },
-	{ IIO_VOLTAGE, 7, "in7" },
-	{ IIO_VOLTAGE, 8, "in8" },
-	{ IIO_VOLTAGE, 9, "in9" },
-	{ IIO_VOLTAGE, 10, "in10" },
-	{ IIO_VOLTAGE, 11, "in11" },
-	{ IIO_VOLTAGE, 12, "in12" },
-	{ IIO_VOLTAGE, 13, "in13" },
-	{ IIO_VOLTAGE, 14, "in14" },
-	{ IIO_VOLTAGE, 15, "in15" },
-	{ IIO_VOLTAGE, 16, "in16" },
-	{ IIO_VOLTAGE, 17, "in17" },
-	{ IIO_VOLTAGE, 18, "in18" },
-	{ IIO_VOLTAGE, 19, "in19" },
-};
-
 static const unsigned int stm32f4_adc_resolutions[] = {
 	/* sorted values so the index matches RES[1:0] in STM32F4_ADC_CR1 */
 	12, 10, 8, 6,
 };
 
+/* stm32f4 can have up to 16 channels */
 static const struct stm32_adc_info stm32f4_adc_info = {
-	.channels = stm32_adc_channels,
 	.max_channels = 16,
 	.resolutions = stm32f4_adc_resolutions,
 	.num_res = ARRAY_SIZE(stm32f4_adc_resolutions),
@@ -394,9 +356,9 @@ struct stm32_adc_info {
 	16, 14, 12, 10, 8,
 };
 
+/* stm32h7 can have up to 20 channels */
 static const struct stm32_adc_info stm32h7_adc_info = {
-	.channels = stm32_adc_channels,
-	.max_channels = 20,
+	.max_channels = STM32_ADC_CH_MAX,
 	.resolutions = stm32h7_adc_resolutions,
 	.num_res = ARRAY_SIZE(stm32h7_adc_resolutions),
 };
@@ -1628,15 +1590,16 @@ static void stm32_adc_smpr_init(struct stm32_adc *adc, int channel, u32 smp_ns)
 }
 
 static void stm32_adc_chan_init_one(struct iio_dev *indio_dev,
-				    struct iio_chan_spec *chan,
-				    const struct stm32_adc_chan_spec *channel,
+				    struct iio_chan_spec *chan, u32 val,
 				    int scan_index, u32 smp)
 {
 	struct stm32_adc *adc = iio_priv(indio_dev);
+	char *name = adc->chan_name[val];
 
-	chan->type = channel->type;
-	chan->channel = channel->channel;
-	chan->datasheet_name = channel->name;
+	chan->type = IIO_VOLTAGE;
+	chan->channel = val;
+	snprintf(name, STM32_ADC_CH_SZ, "in%d", val);
+	chan->datasheet_name = name;
 	chan->scan_index = scan_index;
 	chan->indexed = 1;
 	chan->info_mask_separate = BIT(IIO_CHAN_INFO_RAW);
@@ -1699,8 +1662,7 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
 					   scan_index, &smp);
 
 		stm32_adc_chan_init_one(indio_dev, &channels[scan_index],
-					&adc_info->channels[val],
-					scan_index, smp);
+					val, scan_index, smp);
 		scan_index++;
 	}
 
-- 
1.9.1

[PATCH 3/3] iio: adc: stm32: add support for differential channels

[Fabrice Gasnier]

STM32H7 ADC channels can be configured either as single ended or
differential with 'st,adc-channels' or 'st,adc-diff-channels'
(positive and negative input pair: <vinp vinn>, ...).

Differential channels have different offset and scale, from spec:
raw value = (full_scale / 2) * (1 + (vinp - vinn) / vref).
Add offset attribute.

Differential channels are selected by DIFSEL register. Negative
inputs must be added to pre-selected channels as well (PCSEL).

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
---
 drivers/iio/adc/stm32-adc.c | 123 +++++++++++++++++++++++++++++++++++++-------
 1 file changed, 103 insertions(+), 20 deletions(-)

diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
index c95e6f7..cc7ca50 100644
--- a/drivers/iio/adc/stm32-adc.c
+++ b/drivers/iio/adc/stm32-adc.c
@@ -92,6 +92,7 @@
 #define STM32H7_ADC_SQR3		0x38
 #define STM32H7_ADC_SQR4		0x3C
 #define STM32H7_ADC_DR			0x40
+#define STM32H7_ADC_DIFSEL		0xC0
 #define STM32H7_ADC_CALFACT		0xC4
 #define STM32H7_ADC_CALFACT2		0xC8
 
@@ -154,7 +155,7 @@ enum stm32h7_adc_dmngt {
 #define STM32H7_BOOST_CLKRATE		20000000UL
 
 #define STM32_ADC_CH_MAX		20	/* max number of channels */
-#define STM32_ADC_CH_SZ			5	/* max channel name size */
+#define STM32_ADC_CH_SZ			10	/* max channel name size */
 #define STM32_ADC_MAX_SQ		16	/* SQ1..SQ16 */
 #define STM32_ADC_MAX_SMP		7	/* SMPx range is [0..7] */
 #define STM32_ADC_TIMEOUT_US		100000
@@ -299,6 +300,7 @@ struct stm32_adc_cfg {
  * @rx_buf:		dma rx buffer cpu address
  * @rx_dma_buf:		dma rx buffer bus address
  * @rx_buf_sz:		dma rx buffer size
+ * @difsel		bitmask to set single-ended/differential channel
  * @pcsel		bitmask to preselect channels on some devices
  * @smpr_val:		sampling time settings (e.g. smpr1 / smpr2)
  * @cal:		optional calibration data on some devices
@@ -321,12 +323,18 @@ struct stm32_adc {
 	u8			*rx_buf;
 	dma_addr_t		rx_dma_buf;
 	unsigned int		rx_buf_sz;
+	u32			difsel;
 	u32			pcsel;
 	u32			smpr_val[2];
 	struct stm32_adc_calib	cal;
 	char			chan_name[STM32_ADC_CH_MAX][STM32_ADC_CH_SZ];
 };
 
+struct stm32_adc_diff_channel {
+	u32 vinp;
+	u32 vinn;
+};
+
 /**
  * struct stm32_adc_info - stm32 ADC, per instance config data
  * @max_channels:	Number of channels
@@ -944,15 +952,19 @@ static int stm32h7_adc_selfcalib(struct stm32_adc *adc)
  * stm32h7_adc_prepare() - Leave power down mode to enable ADC.
  * @adc: stm32 adc instance
  * Leave power down mode.
+ * Configure channels as single ended or differential before enabling ADC.
  * Enable ADC.
  * Restore calibration data.
- * Pre-select channels that may be used in PCSEL (required by input MUX / IO).
+ * Pre-select channels that may be used in PCSEL (required by input MUX / IO):
+ * - Only one input is selected for single ended (e.g. 'vinp')
+ * - Two inputs are selected for differential channels (e.g. 'vinp' & 'vinn')
  */
 static int stm32h7_adc_prepare(struct stm32_adc *adc)
 {
 	int ret;
 
 	stm32h7_adc_exit_pwr_down(adc);
+	stm32_adc_writel(adc, STM32H7_ADC_DIFSEL, adc->difsel);
 
 	ret = stm32h7_adc_enable(adc);
 	if (ret)
@@ -1224,10 +1236,23 @@ static int stm32_adc_read_raw(struct iio_dev *indio_dev,
 		return ret;
 
 	case IIO_CHAN_INFO_SCALE:
-		*val = adc->common->vref_mv;
-		*val2 = chan->scan_type.realbits;
+		if (chan->differential) {
+			*val = adc->common->vref_mv * 2;
+			*val2 = chan->scan_type.realbits;
+		} else {
+			*val = adc->common->vref_mv;
+			*val2 = chan->scan_type.realbits;
+		}
 		return IIO_VAL_FRACTIONAL_LOG2;
 
+	case IIO_CHAN_INFO_OFFSET:
+		if (chan->differential)
+			/* ADC_full_scale / 2 */
+			*val = -((1 << chan->scan_type.realbits) / 2);
+		else
+			*val = 0;
+		return IIO_VAL_INT;
+
 	default:
 		return -EINVAL;
 	}
@@ -1591,29 +1616,39 @@ static void stm32_adc_smpr_init(struct stm32_adc *adc, int channel, u32 smp_ns)
 
 static void stm32_adc_chan_init_one(struct iio_dev *indio_dev,
 				    struct iio_chan_spec *chan, u32 val,
-				    int scan_index, u32 smp)
+				    u32 val2, int scan_index, bool differential)
 {
 	struct stm32_adc *adc = iio_priv(indio_dev);
 	char *name = adc->chan_name[val];
 
 	chan->type = IIO_VOLTAGE;
 	chan->channel = val;
-	snprintf(name, STM32_ADC_CH_SZ, "in%d", val);
+	if (differential) {
+		chan->differential = 1;
+		chan->channel2 = val2;
+		snprintf(name, STM32_ADC_CH_SZ, "in%d-in%d", val, val2);
+	} else {
+		snprintf(name, STM32_ADC_CH_SZ, "in%d", val);
+	}
 	chan->datasheet_name = name;
 	chan->scan_index = scan_index;
 	chan->indexed = 1;
 	chan->info_mask_separate = BIT(IIO_CHAN_INFO_RAW);
-	chan->info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE);
+	chan->info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |
+					 BIT(IIO_CHAN_INFO_OFFSET);
 	chan->scan_type.sign = 'u';
 	chan->scan_type.realbits = adc->cfg->adc_info->resolutions[adc->res];
 	chan->scan_type.storagebits = 16;
 	chan->ext_info = stm32_adc_ext_info;
 
-	/* Prepare sampling time settings */
-	stm32_adc_smpr_init(adc, chan->channel, smp);
-
 	/* pre-build selected channels mask */
 	adc->pcsel |= BIT(chan->channel);
+	if (differential) {
+		/* pre-build diff channels mask */
+		adc->difsel |= BIT(chan->channel);
+		/* Also add negative input to pre-selected channels */
+		adc->pcsel |= BIT(chan->channel2);
+	}
 }
 
 static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
@@ -1621,17 +1656,40 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
 	struct device_node *node = indio_dev->dev.of_node;
 	struct stm32_adc *adc = iio_priv(indio_dev);
 	const struct stm32_adc_info *adc_info = adc->cfg->adc_info;
+	struct stm32_adc_diff_channel diff[STM32_ADC_CH_MAX];
 	struct property *prop;
 	const __be32 *cur;
 	struct iio_chan_spec *channels;
-	int scan_index = 0, num_channels, ret;
+	int scan_index = 0, num_channels = 0, num_diff = 0, ret, i;
 	u32 val, smp = 0;
 
-	num_channels = of_property_count_u32_elems(node, "st,adc-channels");
-	if (num_channels < 0 ||
-	    num_channels > adc_info->max_channels) {
+	ret = of_property_count_u32_elems(node, "st,adc-channels");
+	if (ret > adc_info->max_channels) {
 		dev_err(&indio_dev->dev, "Bad st,adc-channels?\n");
-		return num_channels < 0 ? num_channels : -EINVAL;
+		return -EINVAL;
+	} else if (ret > 0) {
+		num_channels += ret;
+	}
+
+	ret = of_property_count_elems_of_size(node, "st,adc-diff-channels",
+					      sizeof(*diff));
+	if (ret > adc_info->max_channels) {
+		dev_err(&indio_dev->dev, "Bad st,adc-diff-channels?\n");
+		return -EINVAL;
+	} else if (ret > 0) {
+		int size = ret * sizeof(*diff) / sizeof(u32);
+
+		num_diff = ret;
+		num_channels += ret;
+		ret = of_property_read_u32_array(node, "st,adc-diff-channels",
+						 (u32 *)diff, size);
+		if (ret)
+			return ret;
+	}
+
+	if (!num_channels) {
+		dev_err(&indio_dev->dev, "No channels configured\n");
+		return -ENODATA;
 	}
 
 	/* Optional sample time is provided either for each, or all channels */
@@ -1652,6 +1710,33 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
 			return -EINVAL;
 		}
 
+		/* Channel can't be configured both as single-ended & diff */
+		for (i = 0; i < num_diff; i++) {
+			if (val == diff[i].vinp) {
+				dev_err(&indio_dev->dev,
+					"channel %d miss-configured\n",	val);
+				return -EINVAL;
+			}
+		}
+		stm32_adc_chan_init_one(indio_dev, &channels[scan_index], val,
+					0, scan_index, false);
+		scan_index++;
+	}
+
+	for (i = 0; i < num_diff; i++) {
+		if (diff[i].vinp >= adc_info->max_channels ||
+		    diff[i].vinn >= adc_info->max_channels) {
+			dev_err(&indio_dev->dev, "Invalid channel in%d-in%d\n",
+				diff[i].vinp, diff[i].vinn);
+			return -EINVAL;
+		}
+		stm32_adc_chan_init_one(indio_dev, &channels[scan_index],
+					diff[i].vinp, diff[i].vinn, scan_index,
+					true);
+		scan_index++;
+	}
+
+	for (i = 0; i < scan_index; i++) {
 		/*
 		 * Using of_property_read_u32_index(), smp value will only be
 		 * modified if valid u32 value can be decoded. This allows to
@@ -1659,11 +1744,9 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
 		 * value per channel.
 		 */
 		of_property_read_u32_index(node, "st,min-sample-time-nsecs",
-					   scan_index, &smp);
-
-		stm32_adc_chan_init_one(indio_dev, &channels[scan_index],
-					val, scan_index, smp);
-		scan_index++;
+					   i, &smp);
+		/* Prepare sampling time settings */
+		stm32_adc_smpr_init(adc, channels[i].channel, smp);
 	}
 
 	indio_dev->num_channels = scan_index;
-- 
1.9.1

[PATCH 3/3] iio: adc: stm32: add support for differential channels

[Fabrice Gasnier]

STM32H7 ADC channels can be configured either as single ended or
differential with 'st,adc-channels' or 'st,adc-diff-channels'
(positive and negative input pair: <vinp vinn>, ...).

Differential channels have different offset and scale, from spec:
raw value = (full_scale / 2) * (1 + (vinp - vinn) / vref).
Add offset attribute.

Differential channels are selected by DIFSEL register. Negative
inputs must be added to pre-selected channels as well (PCSEL).

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
---
 drivers/iio/adc/stm32-adc.c | 123 +++++++++++++++++++++++++++++++++++++-------
 1 file changed, 103 insertions(+), 20 deletions(-)

diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
index c95e6f7..cc7ca50 100644
--- a/drivers/iio/adc/stm32-adc.c
+++ b/drivers/iio/adc/stm32-adc.c
@@ -92,6 +92,7 @@
 #define STM32H7_ADC_SQR3		0x38
 #define STM32H7_ADC_SQR4		0x3C
 #define STM32H7_ADC_DR			0x40
+#define STM32H7_ADC_DIFSEL		0xC0
 #define STM32H7_ADC_CALFACT		0xC4
 #define STM32H7_ADC_CALFACT2		0xC8
 
@@ -154,7 +155,7 @@ enum stm32h7_adc_dmngt {
 #define STM32H7_BOOST_CLKRATE		20000000UL
 
 #define STM32_ADC_CH_MAX		20	/* max number of channels */
-#define STM32_ADC_CH_SZ			5	/* max channel name size */
+#define STM32_ADC_CH_SZ			10	/* max channel name size */
 #define STM32_ADC_MAX_SQ		16	/* SQ1..SQ16 */
 #define STM32_ADC_MAX_SMP		7	/* SMPx range is [0..7] */
 #define STM32_ADC_TIMEOUT_US		100000
@@ -299,6 +300,7 @@ struct stm32_adc_cfg {
  * @rx_buf:		dma rx buffer cpu address
  * @rx_dma_buf:		dma rx buffer bus address
  * @rx_buf_sz:		dma rx buffer size
+ * @difsel		bitmask to set single-ended/differential channel
  * @pcsel		bitmask to preselect channels on some devices
  * @smpr_val:		sampling time settings (e.g. smpr1 / smpr2)
  * @cal:		optional calibration data on some devices
@@ -321,12 +323,18 @@ struct stm32_adc {
 	u8			*rx_buf;
 	dma_addr_t		rx_dma_buf;
 	unsigned int		rx_buf_sz;
+	u32			difsel;
 	u32			pcsel;
 	u32			smpr_val[2];
 	struct stm32_adc_calib	cal;
 	char			chan_name[STM32_ADC_CH_MAX][STM32_ADC_CH_SZ];
 };
 
+struct stm32_adc_diff_channel {
+	u32 vinp;
+	u32 vinn;
+};
+
 /**
  * struct stm32_adc_info - stm32 ADC, per instance config data
  * @max_channels:	Number of channels
@@ -944,15 +952,19 @@ static int stm32h7_adc_selfcalib(struct stm32_adc *adc)
  * stm32h7_adc_prepare() - Leave power down mode to enable ADC.
  * @adc: stm32 adc instance
  * Leave power down mode.
+ * Configure channels as single ended or differential before enabling ADC.
  * Enable ADC.
  * Restore calibration data.
- * Pre-select channels that may be used in PCSEL (required by input MUX / IO).
+ * Pre-select channels that may be used in PCSEL (required by input MUX / IO):
+ * - Only one input is selected for single ended (e.g. 'vinp')
+ * - Two inputs are selected for differential channels (e.g. 'vinp' & 'vinn')
  */
 static int stm32h7_adc_prepare(struct stm32_adc *adc)
 {
 	int ret;
 
 	stm32h7_adc_exit_pwr_down(adc);
+	stm32_adc_writel(adc, STM32H7_ADC_DIFSEL, adc->difsel);
 
 	ret = stm32h7_adc_enable(adc);
 	if (ret)
@@ -1224,10 +1236,23 @@ static int stm32_adc_read_raw(struct iio_dev *indio_dev,
 		return ret;
 
 	case IIO_CHAN_INFO_SCALE:
-		*val = adc->common->vref_mv;
-		*val2 = chan->scan_type.realbits;
+		if (chan->differential) {
+			*val = adc->common->vref_mv * 2;
+			*val2 = chan->scan_type.realbits;
+		} else {
+			*val = adc->common->vref_mv;
+			*val2 = chan->scan_type.realbits;
+		}
 		return IIO_VAL_FRACTIONAL_LOG2;
 
+	case IIO_CHAN_INFO_OFFSET:
+		if (chan->differential)
+			/* ADC_full_scale / 2 */
+			*val = -((1 << chan->scan_type.realbits) / 2);
+		else
+			*val = 0;
+		return IIO_VAL_INT;
+
 	default:
 		return -EINVAL;
 	}
@@ -1591,29 +1616,39 @@ static void stm32_adc_smpr_init(struct stm32_adc *adc, int channel, u32 smp_ns)
 
 static void stm32_adc_chan_init_one(struct iio_dev *indio_dev,
 				    struct iio_chan_spec *chan, u32 val,
-				    int scan_index, u32 smp)
+				    u32 val2, int scan_index, bool differential)
 {
 	struct stm32_adc *adc = iio_priv(indio_dev);
 	char *name = adc->chan_name[val];
 
 	chan->type = IIO_VOLTAGE;
 	chan->channel = val;
-	snprintf(name, STM32_ADC_CH_SZ, "in%d", val);
+	if (differential) {
+		chan->differential = 1;
+		chan->channel2 = val2;
+		snprintf(name, STM32_ADC_CH_SZ, "in%d-in%d", val, val2);
+	} else {
+		snprintf(name, STM32_ADC_CH_SZ, "in%d", val);
+	}
 	chan->datasheet_name = name;
 	chan->scan_index = scan_index;
 	chan->indexed = 1;
 	chan->info_mask_separate = BIT(IIO_CHAN_INFO_RAW);
-	chan->info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE);
+	chan->info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |
+					 BIT(IIO_CHAN_INFO_OFFSET);
 	chan->scan_type.sign = 'u';
 	chan->scan_type.realbits = adc->cfg->adc_info->resolutions[adc->res];
 	chan->scan_type.storagebits = 16;
 	chan->ext_info = stm32_adc_ext_info;
 
-	/* Prepare sampling time settings */
-	stm32_adc_smpr_init(adc, chan->channel, smp);
-
 	/* pre-build selected channels mask */
 	adc->pcsel |= BIT(chan->channel);
+	if (differential) {
+		/* pre-build diff channels mask */
+		adc->difsel |= BIT(chan->channel);
+		/* Also add negative input to pre-selected channels */
+		adc->pcsel |= BIT(chan->channel2);
+	}
 }
 
 static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
@@ -1621,17 +1656,40 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
 	struct device_node *node = indio_dev->dev.of_node;
 	struct stm32_adc *adc = iio_priv(indio_dev);
 	const struct stm32_adc_info *adc_info = adc->cfg->adc_info;
+	struct stm32_adc_diff_channel diff[STM32_ADC_CH_MAX];
 	struct property *prop;
 	const __be32 *cur;
 	struct iio_chan_spec *channels;
-	int scan_index = 0, num_channels, ret;
+	int scan_index = 0, num_channels = 0, num_diff = 0, ret, i;
 	u32 val, smp = 0;
 
-	num_channels = of_property_count_u32_elems(node, "st,adc-channels");
-	if (num_channels < 0 ||
-	    num_channels > adc_info->max_channels) {
+	ret = of_property_count_u32_elems(node, "st,adc-channels");
+	if (ret > adc_info->max_channels) {
 		dev_err(&indio_dev->dev, "Bad st,adc-channels?\n");
-		return num_channels < 0 ? num_channels : -EINVAL;
+		return -EINVAL;
+	} else if (ret > 0) {
+		num_channels += ret;
+	}
+
+	ret = of_property_count_elems_of_size(node, "st,adc-diff-channels",
+					      sizeof(*diff));
+	if (ret > adc_info->max_channels) {
+		dev_err(&indio_dev->dev, "Bad st,adc-diff-channels?\n");
+		return -EINVAL;
+	} else if (ret > 0) {
+		int size = ret * sizeof(*diff) / sizeof(u32);
+
+		num_diff = ret;
+		num_channels += ret;
+		ret = of_property_read_u32_array(node, "st,adc-diff-channels",
+						 (u32 *)diff, size);
+		if (ret)
+			return ret;
+	}
+
+	if (!num_channels) {
+		dev_err(&indio_dev->dev, "No channels configured\n");
+		return -ENODATA;
 	}
 
 	/* Optional sample time is provided either for each, or all channels */
@@ -1652,6 +1710,33 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
 			return -EINVAL;
 		}
 
+		/* Channel can't be configured both as single-ended & diff */
+		for (i = 0; i < num_diff; i++) {
+			if (val == diff[i].vinp) {
+				dev_err(&indio_dev->dev,
+					"channel %d miss-configured\n",	val);
+				return -EINVAL;
+			}
+		}
+		stm32_adc_chan_init_one(indio_dev, &channels[scan_index], val,
+					0, scan_index, false);
+		scan_index++;
+	}
+
+	for (i = 0; i < num_diff; i++) {
+		if (diff[i].vinp >= adc_info->max_channels ||
+		    diff[i].vinn >= adc_info->max_channels) {
+			dev_err(&indio_dev->dev, "Invalid channel in%d-in%d\n",
+				diff[i].vinp, diff[i].vinn);
+			return -EINVAL;
+		}
+		stm32_adc_chan_init_one(indio_dev, &channels[scan_index],
+					diff[i].vinp, diff[i].vinn, scan_index,
+					true);
+		scan_index++;
+	}
+
+	for (i = 0; i < scan_index; i++) {
 		/*
 		 * Using of_property_read_u32_index(), smp value will only be
 		 * modified if valid u32 value can be decoded. This allows to
@@ -1659,11 +1744,9 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
 		 * value per channel.
 		 */
 		of_property_read_u32_index(node, "st,min-sample-time-nsecs",
-					   scan_index, &smp);
-
-		stm32_adc_chan_init_one(indio_dev, &channels[scan_index],
-					val, scan_index, smp);
-		scan_index++;
+					   i, &smp);
+		/* Prepare sampling time settings */
+		stm32_adc_smpr_init(adc, channels[i].channel, smp);
 	}
 
 	indio_dev->num_channels = scan_index;
-- 
1.9.1

[PATCH 3/3] iio: adc: stm32: add support for differential channels

[Fabrice Gasnier]

STM32H7 ADC channels can be configured either as single ended or
differential with 'st,adc-channels' or 'st,adc-diff-channels'
(positive and negative input pair: <vinp vinn>, ...).

Differential channels have different offset and scale, from spec:
raw value = (full_scale / 2) * (1 + (vinp - vinn) / vref).
Add offset attribute.

Differential channels are selected by DIFSEL register. Negative
inputs must be added to pre-selected channels as well (PCSEL).

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
---
 drivers/iio/adc/stm32-adc.c | 123 +++++++++++++++++++++++++++++++++++++-------
 1 file changed, 103 insertions(+), 20 deletions(-)

diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
index c95e6f7..cc7ca50 100644
--- a/drivers/iio/adc/stm32-adc.c
+++ b/drivers/iio/adc/stm32-adc.c
@@ -92,6 +92,7 @@
 #define STM32H7_ADC_SQR3		0x38
 #define STM32H7_ADC_SQR4		0x3C
 #define STM32H7_ADC_DR			0x40
+#define STM32H7_ADC_DIFSEL		0xC0
 #define STM32H7_ADC_CALFACT		0xC4
 #define STM32H7_ADC_CALFACT2		0xC8
 
@@ -154,7 +155,7 @@ enum stm32h7_adc_dmngt {
 #define STM32H7_BOOST_CLKRATE		20000000UL
 
 #define STM32_ADC_CH_MAX		20	/* max number of channels */
-#define STM32_ADC_CH_SZ			5	/* max channel name size */
+#define STM32_ADC_CH_SZ			10	/* max channel name size */
 #define STM32_ADC_MAX_SQ		16	/* SQ1..SQ16 */
 #define STM32_ADC_MAX_SMP		7	/* SMPx range is [0..7] */
 #define STM32_ADC_TIMEOUT_US		100000
@@ -299,6 +300,7 @@ struct stm32_adc_cfg {
  * @rx_buf:		dma rx buffer cpu address
  * @rx_dma_buf:		dma rx buffer bus address
  * @rx_buf_sz:		dma rx buffer size
+ * @difsel		bitmask to set single-ended/differential channel
  * @pcsel		bitmask to preselect channels on some devices
  * @smpr_val:		sampling time settings (e.g. smpr1 / smpr2)
  * @cal:		optional calibration data on some devices
@@ -321,12 +323,18 @@ struct stm32_adc {
 	u8			*rx_buf;
 	dma_addr_t		rx_dma_buf;
 	unsigned int		rx_buf_sz;
+	u32			difsel;
 	u32			pcsel;
 	u32			smpr_val[2];
 	struct stm32_adc_calib	cal;
 	char			chan_name[STM32_ADC_CH_MAX][STM32_ADC_CH_SZ];
 };
 
+struct stm32_adc_diff_channel {
+	u32 vinp;
+	u32 vinn;
+};
+
 /**
  * struct stm32_adc_info - stm32 ADC, per instance config data
  * @max_channels:	Number of channels
@@ -944,15 +952,19 @@ static int stm32h7_adc_selfcalib(struct stm32_adc *adc)
  * stm32h7_adc_prepare() - Leave power down mode to enable ADC.
  * @adc: stm32 adc instance
  * Leave power down mode.
+ * Configure channels as single ended or differential before enabling ADC.
  * Enable ADC.
  * Restore calibration data.
- * Pre-select channels that may be used in PCSEL (required by input MUX / IO).
+ * Pre-select channels that may be used in PCSEL (required by input MUX / IO):
+ * - Only one input is selected for single ended (e.g. 'vinp')
+ * - Two inputs are selected for differential channels (e.g. 'vinp' & 'vinn')
  */
 static int stm32h7_adc_prepare(struct stm32_adc *adc)
 {
 	int ret;
 
 	stm32h7_adc_exit_pwr_down(adc);
+	stm32_adc_writel(adc, STM32H7_ADC_DIFSEL, adc->difsel);
 
 	ret = stm32h7_adc_enable(adc);
 	if (ret)
@@ -1224,10 +1236,23 @@ static int stm32_adc_read_raw(struct iio_dev *indio_dev,
 		return ret;
 
 	case IIO_CHAN_INFO_SCALE:
-		*val = adc->common->vref_mv;
-		*val2 = chan->scan_type.realbits;
+		if (chan->differential) {
+			*val = adc->common->vref_mv * 2;
+			*val2 = chan->scan_type.realbits;
+		} else {
+			*val = adc->common->vref_mv;
+			*val2 = chan->scan_type.realbits;
+		}
 		return IIO_VAL_FRACTIONAL_LOG2;
 
+	case IIO_CHAN_INFO_OFFSET:
+		if (chan->differential)
+			/* ADC_full_scale / 2 */
+			*val = -((1 << chan->scan_type.realbits) / 2);
+		else
+			*val = 0;
+		return IIO_VAL_INT;
+
 	default:
 		return -EINVAL;
 	}
@@ -1591,29 +1616,39 @@ static void stm32_adc_smpr_init(struct stm32_adc *adc, int channel, u32 smp_ns)
 
 static void stm32_adc_chan_init_one(struct iio_dev *indio_dev,
 				    struct iio_chan_spec *chan, u32 val,
-				    int scan_index, u32 smp)
+				    u32 val2, int scan_index, bool differential)
 {
 	struct stm32_adc *adc = iio_priv(indio_dev);
 	char *name = adc->chan_name[val];
 
 	chan->type = IIO_VOLTAGE;
 	chan->channel = val;
-	snprintf(name, STM32_ADC_CH_SZ, "in%d", val);
+	if (differential) {
+		chan->differential = 1;
+		chan->channel2 = val2;
+		snprintf(name, STM32_ADC_CH_SZ, "in%d-in%d", val, val2);
+	} else {
+		snprintf(name, STM32_ADC_CH_SZ, "in%d", val);
+	}
 	chan->datasheet_name = name;
 	chan->scan_index = scan_index;
 	chan->indexed = 1;
 	chan->info_mask_separate = BIT(IIO_CHAN_INFO_RAW);
-	chan->info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE);
+	chan->info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |
+					 BIT(IIO_CHAN_INFO_OFFSET);
 	chan->scan_type.sign = 'u';
 	chan->scan_type.realbits = adc->cfg->adc_info->resolutions[adc->res];
 	chan->scan_type.storagebits = 16;
 	chan->ext_info = stm32_adc_ext_info;
 
-	/* Prepare sampling time settings */
-	stm32_adc_smpr_init(adc, chan->channel, smp);
-
 	/* pre-build selected channels mask */
 	adc->pcsel |= BIT(chan->channel);
+	if (differential) {
+		/* pre-build diff channels mask */
+		adc->difsel |= BIT(chan->channel);
+		/* Also add negative input to pre-selected channels */
+		adc->pcsel |= BIT(chan->channel2);
+	}
 }
 
 static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
@@ -1621,17 +1656,40 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
 	struct device_node *node = indio_dev->dev.of_node;
 	struct stm32_adc *adc = iio_priv(indio_dev);
 	const struct stm32_adc_info *adc_info = adc->cfg->adc_info;
+	struct stm32_adc_diff_channel diff[STM32_ADC_CH_MAX];
 	struct property *prop;
 	const __be32 *cur;
 	struct iio_chan_spec *channels;
-	int scan_index = 0, num_channels, ret;
+	int scan_index = 0, num_channels = 0, num_diff = 0, ret, i;
 	u32 val, smp = 0;
 
-	num_channels = of_property_count_u32_elems(node, "st,adc-channels");
-	if (num_channels < 0 ||
-	    num_channels > adc_info->max_channels) {
+	ret = of_property_count_u32_elems(node, "st,adc-channels");
+	if (ret > adc_info->max_channels) {
 		dev_err(&indio_dev->dev, "Bad st,adc-channels?\n");
-		return num_channels < 0 ? num_channels : -EINVAL;
+		return -EINVAL;
+	} else if (ret > 0) {
+		num_channels += ret;
+	}
+
+	ret = of_property_count_elems_of_size(node, "st,adc-diff-channels",
+					      sizeof(*diff));
+	if (ret > adc_info->max_channels) {
+		dev_err(&indio_dev->dev, "Bad st,adc-diff-channels?\n");
+		return -EINVAL;
+	} else if (ret > 0) {
+		int size = ret * sizeof(*diff) / sizeof(u32);
+
+		num_diff = ret;
+		num_channels += ret;
+		ret = of_property_read_u32_array(node, "st,adc-diff-channels",
+						 (u32 *)diff, size);
+		if (ret)
+			return ret;
+	}
+
+	if (!num_channels) {
+		dev_err(&indio_dev->dev, "No channels configured\n");
+		return -ENODATA;
 	}
 
 	/* Optional sample time is provided either for each, or all channels */
@@ -1652,6 +1710,33 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
 			return -EINVAL;
 		}
 
+		/* Channel can't be configured both as single-ended & diff */
+		for (i = 0; i < num_diff; i++) {
+			if (val == diff[i].vinp) {
+				dev_err(&indio_dev->dev,
+					"channel %d miss-configured\n",	val);
+				return -EINVAL;
+			}
+		}
+		stm32_adc_chan_init_one(indio_dev, &channels[scan_index], val,
+					0, scan_index, false);
+		scan_index++;
+	}
+
+	for (i = 0; i < num_diff; i++) {
+		if (diff[i].vinp >= adc_info->max_channels ||
+		    diff[i].vinn >= adc_info->max_channels) {
+			dev_err(&indio_dev->dev, "Invalid channel in%d-in%d\n",
+				diff[i].vinp, diff[i].vinn);
+			return -EINVAL;
+		}
+		stm32_adc_chan_init_one(indio_dev, &channels[scan_index],
+					diff[i].vinp, diff[i].vinn, scan_index,
+					true);
+		scan_index++;
+	}
+
+	for (i = 0; i < scan_index; i++) {
 		/*
 		 * Using of_property_read_u32_index(), smp value will only be
 		 * modified if valid u32 value can be decoded. This allows to
@@ -1659,11 +1744,9 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
 		 * value per channel.
 		 */
 		of_property_read_u32_index(node, "st,min-sample-time-nsecs",
-					   scan_index, &smp);
-
-		stm32_adc_chan_init_one(indio_dev, &channels[scan_index],
-					val, scan_index, smp);
-		scan_index++;
+					   i, &smp);
+		/* Prepare sampling time settings */
+		stm32_adc_smpr_init(adc, channels[i].channel, smp);
 	}
 
 	indio_dev->num_channels = scan_index;
-- 
1.9.1

Re: [PATCH 1/3] dt-bindings: iio: adc: stm32: add support for diff channels

[Jonathan Cameron]

On Tue, 17 Oct 2017 15:15:43 +0200
Fabrice Gasnier <fabrice.gasnier@st.com> wrote:

> STM32H7 ADC channels may be configured either as single-ended or
> differential.
> Add 'st,adc-diff-channels' property to support differential channels.
> Differential channels are defined as a pair of positive and negative
> inputs: vinp & vinn.
> 
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
Hmm. Fair enough.  Sometimes we support both types of channels
and leave it to userspace, but in many cases that makes little sense
- particularly if like I think is going on here, we aren't combining channels
that can be separately read but rather the negative pin is simply unused
when we are in single channel mode... (did I understand that right?)

Jonathan

> ---
>  Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
> index 48bfcaa3..c3c8de1 100644
> --- a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
> +++ b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
> @@ -62,6 +62,12 @@ Required properties:
>  - st,adc-channels: List of single-ended channels muxed for this ADC.
>    It can have up to 16 channels on stm32f4 or 20 channels on stm32h7, numbered
>    from 0 to 15 or 19 (resp. for in0..in15 or in0..in19).
> +- st,adc-diff-channels: List of differential channels muxed for this ADC.
> +  Depending on part used, some channels can be configured as differential
> +  instead of single-ended (e.g. stm32h7). List here positive and negative
> +  inputs pairs as <vinp vinn>, <vinp vinn>,... vinp and vinn are numbered
> +  from 0 to 19 on stm32h7)
> +  Note: At least one of "st,adc-channels" or "st,adc-diff-channels" is required.
>  - #io-channel-cells = <1>: See the IIO bindings section "IIO consumers" in
>    Documentation/devicetree/bindings/iio/iio-bindings.txt
>  

Re: [PATCH 1/3] dt-bindings: iio: adc: stm32: add support for diff channels

[Jonathan Cameron]

On Tue, 17 Oct 2017 15:15:43 +0200
Fabrice Gasnier <fabrice.gasnier@st.com> wrote:

> STM32H7 ADC channels may be configured either as single-ended or
> differential.
> Add 'st,adc-diff-channels' property to support differential channels.
> Differential channels are defined as a pair of positive and negative
> inputs: vinp & vinn.
> 
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
Hmm. Fair enough.  Sometimes we support both types of channels
and leave it to userspace, but in many cases that makes little sense
- particularly if like I think is going on here, we aren't combining channels
that can be separately read but rather the negative pin is simply unused
when we are in single channel mode... (did I understand that right?)

Jonathan

> ---
>  Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
> index 48bfcaa3..c3c8de1 100644
> --- a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
> +++ b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
> @@ -62,6 +62,12 @@ Required properties:
>  - st,adc-channels: List of single-ended channels muxed for this ADC.
>    It can have up to 16 channels on stm32f4 or 20 channels on stm32h7, numbered
>    from 0 to 15 or 19 (resp. for in0..in15 or in0..in19).
> +- st,adc-diff-channels: List of differential channels muxed for this ADC.
> +  Depending on part used, some channels can be configured as differential
> +  instead of single-ended (e.g. stm32h7). List here positive and negative
> +  inputs pairs as <vinp vinn>, <vinp vinn>,... vinp and vinn are numbered
> +  from 0 to 19 on stm32h7)
> +  Note: At least one of "st,adc-channels" or "st,adc-diff-channels" is required.
>  - #io-channel-cells = <1>: See the IIO bindings section "IIO consumers" in
>    Documentation/devicetree/bindings/iio/iio-bindings.txt
>  

[PATCH 1/3] dt-bindings: iio: adc: stm32: add support for diff channels

[Jonathan Cameron]

On Tue, 17 Oct 2017 15:15:43 +0200
Fabrice Gasnier <fabrice.gasnier@st.com> wrote:

> STM32H7 ADC channels may be configured either as single-ended or
> differential.
> Add 'st,adc-diff-channels' property to support differential channels.
> Differential channels are defined as a pair of positive and negative
> inputs: vinp & vinn.
> 
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
Hmm. Fair enough.  Sometimes we support both types of channels
and leave it to userspace, but in many cases that makes little sense
- particularly if like I think is going on here, we aren't combining channels
that can be separately read but rather the negative pin is simply unused
when we are in single channel mode... (did I understand that right?)

Jonathan

> ---
>  Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
> index 48bfcaa3..c3c8de1 100644
> --- a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
> +++ b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
> @@ -62,6 +62,12 @@ Required properties:
>  - st,adc-channels: List of single-ended channels muxed for this ADC.
>    It can have up to 16 channels on stm32f4 or 20 channels on stm32h7, numbered
>    from 0 to 15 or 19 (resp. for in0..in15 or in0..in19).
> +- st,adc-diff-channels: List of differential channels muxed for this ADC.
> +  Depending on part used, some channels can be configured as differential
> +  instead of single-ended (e.g. stm32h7). List here positive and negative
> +  inputs pairs as <vinp vinn>, <vinp vinn>,... vinp and vinn are numbered
> +  from 0 to 19 on stm32h7)
> +  Note: At least one of "st,adc-channels" or "st,adc-diff-channels" is required.
>  - #io-channel-cells = <1>: See the IIO bindings section "IIO consumers" in
>    Documentation/devicetree/bindings/iio/iio-bindings.txt
>  

Re: [PATCH 1/3] dt-bindings: iio: adc: stm32: add support for diff channels

[Jonathan Cameron]

On Sat, 21 Oct 2017 18:54:01 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> On Tue, 17 Oct 2017 15:15:43 +0200
> Fabrice Gasnier <fabrice.gasnier@st.com> wrote:
> 
> > STM32H7 ADC channels may be configured either as single-ended or
> > differential.
> > Add 'st,adc-diff-channels' property to support differential channels.
> > Differential channels are defined as a pair of positive and negative
> > inputs: vinp & vinn.
> > 
> > Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> Hmm. Fair enough.  Sometimes we support both types of channels
> and leave it to userspace, but in many cases that makes little sense
> - particularly if like I think is going on here, we aren't combining channels
> that can be separately read but rather the negative pin is simply unused
> when we are in single channel mode... (did I understand that right?)
> 
Forgot to say - I would ideally like a devicetree maintainer review on this
one as it's a bit unusual!

> Jonathan
> 
> > ---
> >  Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
> > index 48bfcaa3..c3c8de1 100644
> > --- a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
> > +++ b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
> > @@ -62,6 +62,12 @@ Required properties:
> >  - st,adc-channels: List of single-ended channels muxed for this ADC.
> >    It can have up to 16 channels on stm32f4 or 20 channels on stm32h7, numbered
> >    from 0 to 15 or 19 (resp. for in0..in15 or in0..in19).
> > +- st,adc-diff-channels: List of differential channels muxed for this ADC.
> > +  Depending on part used, some channels can be configured as differential
> > +  instead of single-ended (e.g. stm32h7). List here positive and negative
> > +  inputs pairs as <vinp vinn>, <vinp vinn>,... vinp and vinn are numbered
> > +  from 0 to 19 on stm32h7)
> > +  Note: At least one of "st,adc-channels" or "st,adc-diff-channels" is required.
> >  - #io-channel-cells = <1>: See the IIO bindings section "IIO consumers" in
> >    Documentation/devicetree/bindings/iio/iio-bindings.txt
> >  
> 

Re: [PATCH 1/3] dt-bindings: iio: adc: stm32: add support for diff channels

[Jonathan Cameron]

On Sat, 21 Oct 2017 18:54:01 +0100
Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:

> On Tue, 17 Oct 2017 15:15:43 +0200
> Fabrice Gasnier <fabrice.gasnier-qxv4g6HH51o@public.gmane.org> wrote:
> 
> > STM32H7 ADC channels may be configured either as single-ended or
> > differential.
> > Add 'st,adc-diff-channels' property to support differential channels.
> > Differential channels are defined as a pair of positive and negative
> > inputs: vinp & vinn.
> > 
> > Signed-off-by: Fabrice Gasnier <fabrice.gasnier-qxv4g6HH51o@public.gmane.org>
> Hmm. Fair enough.  Sometimes we support both types of channels
> and leave it to userspace, but in many cases that makes little sense
> - particularly if like I think is going on here, we aren't combining channels
> that can be separately read but rather the negative pin is simply unused
> when we are in single channel mode... (did I understand that right?)
> 
Forgot to say - I would ideally like a devicetree maintainer review on this
one as it's a bit unusual!

> Jonathan
> 
> > ---
> >  Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
> > index 48bfcaa3..c3c8de1 100644
> > --- a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
> > +++ b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
> > @@ -62,6 +62,12 @@ Required properties:
> >  - st,adc-channels: List of single-ended channels muxed for this ADC.
> >    It can have up to 16 channels on stm32f4 or 20 channels on stm32h7, numbered
> >    from 0 to 15 or 19 (resp. for in0..in15 or in0..in19).
> > +- st,adc-diff-channels: List of differential channels muxed for this ADC.
> > +  Depending on part used, some channels can be configured as differential
> > +  instead of single-ended (e.g. stm32h7). List here positive and negative
> > +  inputs pairs as <vinp vinn>, <vinp vinn>,... vinp and vinn are numbered
> > +  from 0 to 19 on stm32h7)
> > +  Note: At least one of "st,adc-channels" or "st,adc-diff-channels" is required.
> >  - #io-channel-cells = <1>: See the IIO bindings section "IIO consumers" in
> >    Documentation/devicetree/bindings/iio/iio-bindings.txt
> >  
> 

--
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 1/3] dt-bindings: iio: adc: stm32: add support for diff channels

[Jonathan Cameron]

On Sat, 21 Oct 2017 18:54:01 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> On Tue, 17 Oct 2017 15:15:43 +0200
> Fabrice Gasnier <fabrice.gasnier@st.com> wrote:
> 
> > STM32H7 ADC channels may be configured either as single-ended or
> > differential.
> > Add 'st,adc-diff-channels' property to support differential channels.
> > Differential channels are defined as a pair of positive and negative
> > inputs: vinp & vinn.
> > 
> > Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> Hmm. Fair enough.  Sometimes we support both types of channels
> and leave it to userspace, but in many cases that makes little sense
> - particularly if like I think is going on here, we aren't combining channels
> that can be separately read but rather the negative pin is simply unused
> when we are in single channel mode... (did I understand that right?)
> 
Forgot to say - I would ideally like a devicetree maintainer review on this
one as it's a bit unusual!

> Jonathan
> 
> > ---
> >  Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
> > index 48bfcaa3..c3c8de1 100644
> > --- a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
> > +++ b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
> > @@ -62,6 +62,12 @@ Required properties:
> >  - st,adc-channels: List of single-ended channels muxed for this ADC.
> >    It can have up to 16 channels on stm32f4 or 20 channels on stm32h7, numbered
> >    from 0 to 15 or 19 (resp. for in0..in15 or in0..in19).
> > +- st,adc-diff-channels: List of differential channels muxed for this ADC.
> > +  Depending on part used, some channels can be configured as differential
> > +  instead of single-ended (e.g. stm32h7). List here positive and negative
> > +  inputs pairs as <vinp vinn>, <vinp vinn>,... vinp and vinn are numbered
> > +  from 0 to 19 on stm32h7)
> > +  Note: At least one of "st,adc-channels" or "st,adc-diff-channels" is required.
> >  - #io-channel-cells = <1>: See the IIO bindings section "IIO consumers" in
> >    Documentation/devicetree/bindings/iio/iio-bindings.txt
> >  
> 

Re: [PATCH 2/3] iio: adc: stm32: remove const channel names definition

[Jonathan Cameron]

On Tue, 17 Oct 2017 15:15:44 +0200
Fabrice Gasnier <fabrice.gasnier@st.com> wrote:

> Remove const array that defines channels. Build channels definition
> at probe time, when initializing channels (only for requested ones).
> This will ease adding differential channels support.
> 
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
Looks good to me.
> ---
>  drivers/iio/adc/stm32-adc.c | 66 ++++++++++-----------------------------------
>  1 file changed, 14 insertions(+), 52 deletions(-)
> 
> diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
> index 4df32cf..c95e6f7 100644
> --- a/drivers/iio/adc/stm32-adc.c
> +++ b/drivers/iio/adc/stm32-adc.c
> @@ -153,6 +153,8 @@ enum stm32h7_adc_dmngt {
>  /* BOOST bit must be set on STM32H7 when ADC clock is above 20MHz */
>  #define STM32H7_BOOST_CLKRATE		20000000UL
>  
> +#define STM32_ADC_CH_MAX		20	/* max number of channels */
> +#define STM32_ADC_CH_SZ			5	/* max channel name size */
>  #define STM32_ADC_MAX_SQ		16	/* SQ1..SQ16 */
>  #define STM32_ADC_MAX_SMP		7	/* SMPx range is [0..7] */
>  #define STM32_ADC_TIMEOUT_US		100000
> @@ -300,6 +302,7 @@ struct stm32_adc_cfg {
>   * @pcsel		bitmask to preselect channels on some devices
>   * @smpr_val:		sampling time settings (e.g. smpr1 / smpr2)
>   * @cal:		optional calibration data on some devices
> + * @chan_name:		channel name array
>   */
>  struct stm32_adc {
>  	struct stm32_adc_common	*common;
> @@ -321,69 +324,28 @@ struct stm32_adc {
>  	u32			pcsel;
>  	u32			smpr_val[2];
>  	struct stm32_adc_calib	cal;
> -};
> -
> -/**
> - * struct stm32_adc_chan_spec - specification of stm32 adc channel
> - * @type:	IIO channel type
> - * @channel:	channel number (single ended)
> - * @name:	channel name (single ended)
> - */
> -struct stm32_adc_chan_spec {
> -	enum iio_chan_type	type;
> -	int			channel;
> -	const char		*name;
> +	char			chan_name[STM32_ADC_CH_MAX][STM32_ADC_CH_SZ];
>  };
>  
>  /**
>   * struct stm32_adc_info - stm32 ADC, per instance config data
> - * @channels:		Reference to stm32 channels spec
>   * @max_channels:	Number of channels
>   * @resolutions:	available resolutions
>   * @num_res:		number of available resolutions
>   */
>  struct stm32_adc_info {
> -	const struct stm32_adc_chan_spec *channels;
>  	int max_channels;
>  	const unsigned int *resolutions;
>  	const unsigned int num_res;
>  };
>  
> -/*
> - * Input definitions common for all instances:
> - * stm32f4 can have up to 16 channels
> - * stm32h7 can have up to 20 channels
> - */
> -static const struct stm32_adc_chan_spec stm32_adc_channels[] = {
> -	{ IIO_VOLTAGE, 0, "in0" },
> -	{ IIO_VOLTAGE, 1, "in1" },
> -	{ IIO_VOLTAGE, 2, "in2" },
> -	{ IIO_VOLTAGE, 3, "in3" },
> -	{ IIO_VOLTAGE, 4, "in4" },
> -	{ IIO_VOLTAGE, 5, "in5" },
> -	{ IIO_VOLTAGE, 6, "in6" },
> -	{ IIO_VOLTAGE, 7, "in7" },
> -	{ IIO_VOLTAGE, 8, "in8" },
> -	{ IIO_VOLTAGE, 9, "in9" },
> -	{ IIO_VOLTAGE, 10, "in10" },
> -	{ IIO_VOLTAGE, 11, "in11" },
> -	{ IIO_VOLTAGE, 12, "in12" },
> -	{ IIO_VOLTAGE, 13, "in13" },
> -	{ IIO_VOLTAGE, 14, "in14" },
> -	{ IIO_VOLTAGE, 15, "in15" },
> -	{ IIO_VOLTAGE, 16, "in16" },
> -	{ IIO_VOLTAGE, 17, "in17" },
> -	{ IIO_VOLTAGE, 18, "in18" },
> -	{ IIO_VOLTAGE, 19, "in19" },
> -};
> -
>  static const unsigned int stm32f4_adc_resolutions[] = {
>  	/* sorted values so the index matches RES[1:0] in STM32F4_ADC_CR1 */
>  	12, 10, 8, 6,
>  };
>  
> +/* stm32f4 can have up to 16 channels */
>  static const struct stm32_adc_info stm32f4_adc_info = {
> -	.channels = stm32_adc_channels,
>  	.max_channels = 16,
>  	.resolutions = stm32f4_adc_resolutions,
>  	.num_res = ARRAY_SIZE(stm32f4_adc_resolutions),
> @@ -394,9 +356,9 @@ struct stm32_adc_info {
>  	16, 14, 12, 10, 8,
>  };
>  
> +/* stm32h7 can have up to 20 channels */
>  static const struct stm32_adc_info stm32h7_adc_info = {
> -	.channels = stm32_adc_channels,
> -	.max_channels = 20,
> +	.max_channels = STM32_ADC_CH_MAX,
>  	.resolutions = stm32h7_adc_resolutions,
>  	.num_res = ARRAY_SIZE(stm32h7_adc_resolutions),
>  };
> @@ -1628,15 +1590,16 @@ static void stm32_adc_smpr_init(struct stm32_adc *adc, int channel, u32 smp_ns)
>  }
>  
>  static void stm32_adc_chan_init_one(struct iio_dev *indio_dev,
> -				    struct iio_chan_spec *chan,
> -				    const struct stm32_adc_chan_spec *channel,
> +				    struct iio_chan_spec *chan, u32 val,
>  				    int scan_index, u32 smp)
>  {
>  	struct stm32_adc *adc = iio_priv(indio_dev);
> +	char *name = adc->chan_name[val];
>  
> -	chan->type = channel->type;
> -	chan->channel = channel->channel;
> -	chan->datasheet_name = channel->name;
> +	chan->type = IIO_VOLTAGE;
> +	chan->channel = val;
> +	snprintf(name, STM32_ADC_CH_SZ, "in%d", val);
> +	chan->datasheet_name = name;
>  	chan->scan_index = scan_index;
>  	chan->indexed = 1;
>  	chan->info_mask_separate = BIT(IIO_CHAN_INFO_RAW);
> @@ -1699,8 +1662,7 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
>  					   scan_index, &smp);
>  
>  		stm32_adc_chan_init_one(indio_dev, &channels[scan_index],
> -					&adc_info->channels[val],
> -					scan_index, smp);
> +					val, scan_index, smp);
>  		scan_index++;
>  	}
>  

Re: [PATCH 2/3] iio: adc: stm32: remove const channel names definition

[Jonathan Cameron]

On Tue, 17 Oct 2017 15:15:44 +0200
Fabrice Gasnier <fabrice.gasnier-qxv4g6HH51o@public.gmane.org> wrote:

> Remove const array that defines channels. Build channels definition
> at probe time, when initializing channels (only for requested ones).
> This will ease adding differential channels support.
> 
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier-qxv4g6HH51o@public.gmane.org>
Looks good to me.
> ---
>  drivers/iio/adc/stm32-adc.c | 66 ++++++++++-----------------------------------
>  1 file changed, 14 insertions(+), 52 deletions(-)
> 
> diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
> index 4df32cf..c95e6f7 100644
> --- a/drivers/iio/adc/stm32-adc.c
> +++ b/drivers/iio/adc/stm32-adc.c
> @@ -153,6 +153,8 @@ enum stm32h7_adc_dmngt {
>  /* BOOST bit must be set on STM32H7 when ADC clock is above 20MHz */
>  #define STM32H7_BOOST_CLKRATE		20000000UL
>  
> +#define STM32_ADC_CH_MAX		20	/* max number of channels */
> +#define STM32_ADC_CH_SZ			5	/* max channel name size */
>  #define STM32_ADC_MAX_SQ		16	/* SQ1..SQ16 */
>  #define STM32_ADC_MAX_SMP		7	/* SMPx range is [0..7] */
>  #define STM32_ADC_TIMEOUT_US		100000
> @@ -300,6 +302,7 @@ struct stm32_adc_cfg {
>   * @pcsel		bitmask to preselect channels on some devices
>   * @smpr_val:		sampling time settings (e.g. smpr1 / smpr2)
>   * @cal:		optional calibration data on some devices
> + * @chan_name:		channel name array
>   */
>  struct stm32_adc {
>  	struct stm32_adc_common	*common;
> @@ -321,69 +324,28 @@ struct stm32_adc {
>  	u32			pcsel;
>  	u32			smpr_val[2];
>  	struct stm32_adc_calib	cal;
> -};
> -
> -/**
> - * struct stm32_adc_chan_spec - specification of stm32 adc channel
> - * @type:	IIO channel type
> - * @channel:	channel number (single ended)
> - * @name:	channel name (single ended)
> - */
> -struct stm32_adc_chan_spec {
> -	enum iio_chan_type	type;
> -	int			channel;
> -	const char		*name;
> +	char			chan_name[STM32_ADC_CH_MAX][STM32_ADC_CH_SZ];
>  };
>  
>  /**
>   * struct stm32_adc_info - stm32 ADC, per instance config data
> - * @channels:		Reference to stm32 channels spec
>   * @max_channels:	Number of channels
>   * @resolutions:	available resolutions
>   * @num_res:		number of available resolutions
>   */
>  struct stm32_adc_info {
> -	const struct stm32_adc_chan_spec *channels;
>  	int max_channels;
>  	const unsigned int *resolutions;
>  	const unsigned int num_res;
>  };
>  
> -/*
> - * Input definitions common for all instances:
> - * stm32f4 can have up to 16 channels
> - * stm32h7 can have up to 20 channels
> - */
> -static const struct stm32_adc_chan_spec stm32_adc_channels[] = {
> -	{ IIO_VOLTAGE, 0, "in0" },
> -	{ IIO_VOLTAGE, 1, "in1" },
> -	{ IIO_VOLTAGE, 2, "in2" },
> -	{ IIO_VOLTAGE, 3, "in3" },
> -	{ IIO_VOLTAGE, 4, "in4" },
> -	{ IIO_VOLTAGE, 5, "in5" },
> -	{ IIO_VOLTAGE, 6, "in6" },
> -	{ IIO_VOLTAGE, 7, "in7" },
> -	{ IIO_VOLTAGE, 8, "in8" },
> -	{ IIO_VOLTAGE, 9, "in9" },
> -	{ IIO_VOLTAGE, 10, "in10" },
> -	{ IIO_VOLTAGE, 11, "in11" },
> -	{ IIO_VOLTAGE, 12, "in12" },
> -	{ IIO_VOLTAGE, 13, "in13" },
> -	{ IIO_VOLTAGE, 14, "in14" },
> -	{ IIO_VOLTAGE, 15, "in15" },
> -	{ IIO_VOLTAGE, 16, "in16" },
> -	{ IIO_VOLTAGE, 17, "in17" },
> -	{ IIO_VOLTAGE, 18, "in18" },
> -	{ IIO_VOLTAGE, 19, "in19" },
> -};
> -
>  static const unsigned int stm32f4_adc_resolutions[] = {
>  	/* sorted values so the index matches RES[1:0] in STM32F4_ADC_CR1 */
>  	12, 10, 8, 6,
>  };
>  
> +/* stm32f4 can have up to 16 channels */
>  static const struct stm32_adc_info stm32f4_adc_info = {
> -	.channels = stm32_adc_channels,
>  	.max_channels = 16,
>  	.resolutions = stm32f4_adc_resolutions,
>  	.num_res = ARRAY_SIZE(stm32f4_adc_resolutions),
> @@ -394,9 +356,9 @@ struct stm32_adc_info {
>  	16, 14, 12, 10, 8,
>  };
>  
> +/* stm32h7 can have up to 20 channels */
>  static const struct stm32_adc_info stm32h7_adc_info = {
> -	.channels = stm32_adc_channels,
> -	.max_channels = 20,
> +	.max_channels = STM32_ADC_CH_MAX,
>  	.resolutions = stm32h7_adc_resolutions,
>  	.num_res = ARRAY_SIZE(stm32h7_adc_resolutions),
>  };
> @@ -1628,15 +1590,16 @@ static void stm32_adc_smpr_init(struct stm32_adc *adc, int channel, u32 smp_ns)
>  }
>  
>  static void stm32_adc_chan_init_one(struct iio_dev *indio_dev,
> -				    struct iio_chan_spec *chan,
> -				    const struct stm32_adc_chan_spec *channel,
> +				    struct iio_chan_spec *chan, u32 val,
>  				    int scan_index, u32 smp)
>  {
>  	struct stm32_adc *adc = iio_priv(indio_dev);
> +	char *name = adc->chan_name[val];
>  
> -	chan->type = channel->type;
> -	chan->channel = channel->channel;
> -	chan->datasheet_name = channel->name;
> +	chan->type = IIO_VOLTAGE;
> +	chan->channel = val;
> +	snprintf(name, STM32_ADC_CH_SZ, "in%d", val);
> +	chan->datasheet_name = name;
>  	chan->scan_index = scan_index;
>  	chan->indexed = 1;
>  	chan->info_mask_separate = BIT(IIO_CHAN_INFO_RAW);
> @@ -1699,8 +1662,7 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
>  					   scan_index, &smp);
>  
>  		stm32_adc_chan_init_one(indio_dev, &channels[scan_index],
> -					&adc_info->channels[val],
> -					scan_index, smp);
> +					val, scan_index, smp);
>  		scan_index++;
>  	}
>  

--
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 2/3] iio: adc: stm32: remove const channel names definition

[Jonathan Cameron]

On Tue, 17 Oct 2017 15:15:44 +0200
Fabrice Gasnier <fabrice.gasnier@st.com> wrote:

> Remove const array that defines channels. Build channels definition
> at probe time, when initializing channels (only for requested ones).
> This will ease adding differential channels support.
> 
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
Looks good to me.
> ---
>  drivers/iio/adc/stm32-adc.c | 66 ++++++++++-----------------------------------
>  1 file changed, 14 insertions(+), 52 deletions(-)
> 
> diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
> index 4df32cf..c95e6f7 100644
> --- a/drivers/iio/adc/stm32-adc.c
> +++ b/drivers/iio/adc/stm32-adc.c
> @@ -153,6 +153,8 @@ enum stm32h7_adc_dmngt {
>  /* BOOST bit must be set on STM32H7 when ADC clock is above 20MHz */
>  #define STM32H7_BOOST_CLKRATE		20000000UL
>  
> +#define STM32_ADC_CH_MAX		20	/* max number of channels */
> +#define STM32_ADC_CH_SZ			5	/* max channel name size */
>  #define STM32_ADC_MAX_SQ		16	/* SQ1..SQ16 */
>  #define STM32_ADC_MAX_SMP		7	/* SMPx range is [0..7] */
>  #define STM32_ADC_TIMEOUT_US		100000
> @@ -300,6 +302,7 @@ struct stm32_adc_cfg {
>   * @pcsel		bitmask to preselect channels on some devices
>   * @smpr_val:		sampling time settings (e.g. smpr1 / smpr2)
>   * @cal:		optional calibration data on some devices
> + * @chan_name:		channel name array
>   */
>  struct stm32_adc {
>  	struct stm32_adc_common	*common;
> @@ -321,69 +324,28 @@ struct stm32_adc {
>  	u32			pcsel;
>  	u32			smpr_val[2];
>  	struct stm32_adc_calib	cal;
> -};
> -
> -/**
> - * struct stm32_adc_chan_spec - specification of stm32 adc channel
> - * @type:	IIO channel type
> - * @channel:	channel number (single ended)
> - * @name:	channel name (single ended)
> - */
> -struct stm32_adc_chan_spec {
> -	enum iio_chan_type	type;
> -	int			channel;
> -	const char		*name;
> +	char			chan_name[STM32_ADC_CH_MAX][STM32_ADC_CH_SZ];
>  };
>  
>  /**
>   * struct stm32_adc_info - stm32 ADC, per instance config data
> - * @channels:		Reference to stm32 channels spec
>   * @max_channels:	Number of channels
>   * @resolutions:	available resolutions
>   * @num_res:		number of available resolutions
>   */
>  struct stm32_adc_info {
> -	const struct stm32_adc_chan_spec *channels;
>  	int max_channels;
>  	const unsigned int *resolutions;
>  	const unsigned int num_res;
>  };
>  
> -/*
> - * Input definitions common for all instances:
> - * stm32f4 can have up to 16 channels
> - * stm32h7 can have up to 20 channels
> - */
> -static const struct stm32_adc_chan_spec stm32_adc_channels[] = {
> -	{ IIO_VOLTAGE, 0, "in0" },
> -	{ IIO_VOLTAGE, 1, "in1" },
> -	{ IIO_VOLTAGE, 2, "in2" },
> -	{ IIO_VOLTAGE, 3, "in3" },
> -	{ IIO_VOLTAGE, 4, "in4" },
> -	{ IIO_VOLTAGE, 5, "in5" },
> -	{ IIO_VOLTAGE, 6, "in6" },
> -	{ IIO_VOLTAGE, 7, "in7" },
> -	{ IIO_VOLTAGE, 8, "in8" },
> -	{ IIO_VOLTAGE, 9, "in9" },
> -	{ IIO_VOLTAGE, 10, "in10" },
> -	{ IIO_VOLTAGE, 11, "in11" },
> -	{ IIO_VOLTAGE, 12, "in12" },
> -	{ IIO_VOLTAGE, 13, "in13" },
> -	{ IIO_VOLTAGE, 14, "in14" },
> -	{ IIO_VOLTAGE, 15, "in15" },
> -	{ IIO_VOLTAGE, 16, "in16" },
> -	{ IIO_VOLTAGE, 17, "in17" },
> -	{ IIO_VOLTAGE, 18, "in18" },
> -	{ IIO_VOLTAGE, 19, "in19" },
> -};
> -
>  static const unsigned int stm32f4_adc_resolutions[] = {
>  	/* sorted values so the index matches RES[1:0] in STM32F4_ADC_CR1 */
>  	12, 10, 8, 6,
>  };
>  
> +/* stm32f4 can have up to 16 channels */
>  static const struct stm32_adc_info stm32f4_adc_info = {
> -	.channels = stm32_adc_channels,
>  	.max_channels = 16,
>  	.resolutions = stm32f4_adc_resolutions,
>  	.num_res = ARRAY_SIZE(stm32f4_adc_resolutions),
> @@ -394,9 +356,9 @@ struct stm32_adc_info {
>  	16, 14, 12, 10, 8,
>  };
>  
> +/* stm32h7 can have up to 20 channels */
>  static const struct stm32_adc_info stm32h7_adc_info = {
> -	.channels = stm32_adc_channels,
> -	.max_channels = 20,
> +	.max_channels = STM32_ADC_CH_MAX,
>  	.resolutions = stm32h7_adc_resolutions,
>  	.num_res = ARRAY_SIZE(stm32h7_adc_resolutions),
>  };
> @@ -1628,15 +1590,16 @@ static void stm32_adc_smpr_init(struct stm32_adc *adc, int channel, u32 smp_ns)
>  }
>  
>  static void stm32_adc_chan_init_one(struct iio_dev *indio_dev,
> -				    struct iio_chan_spec *chan,
> -				    const struct stm32_adc_chan_spec *channel,
> +				    struct iio_chan_spec *chan, u32 val,
>  				    int scan_index, u32 smp)
>  {
>  	struct stm32_adc *adc = iio_priv(indio_dev);
> +	char *name = adc->chan_name[val];
>  
> -	chan->type = channel->type;
> -	chan->channel = channel->channel;
> -	chan->datasheet_name = channel->name;
> +	chan->type = IIO_VOLTAGE;
> +	chan->channel = val;
> +	snprintf(name, STM32_ADC_CH_SZ, "in%d", val);
> +	chan->datasheet_name = name;
>  	chan->scan_index = scan_index;
>  	chan->indexed = 1;
>  	chan->info_mask_separate = BIT(IIO_CHAN_INFO_RAW);
> @@ -1699,8 +1662,7 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
>  					   scan_index, &smp);
>  
>  		stm32_adc_chan_init_one(indio_dev, &channels[scan_index],
> -					&adc_info->channels[val],
> -					scan_index, smp);
> +					val, scan_index, smp);
>  		scan_index++;
>  	}
>  

Re: [PATCH 1/3] dt-bindings: iio: adc: stm32: add support for diff channels

[Jonathan Cameron]

On Sat, 21 Oct 2017 18:54:01 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> On Tue, 17 Oct 2017 15:15:43 +0200
> Fabrice Gasnier <fabrice.gasnier@st.com> wrote:
> 
> > STM32H7 ADC channels may be configured either as single-ended or
> > differential.
> > Add 'st,adc-diff-channels' property to support differential channels.
> > Differential channels are defined as a pair of positive and negative
> > inputs: vinp & vinn.
> > 
> > Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>  
> Hmm. Fair enough.  Sometimes we support both types of channels
> and leave it to userspace, but in many cases that makes little sense
> - particularly if like I think is going on here, we aren't combining channels
> that can be separately read but rather the negative pin is simply unused
> when we are in single channel mode... (did I understand that right?)
I clearly didn't understand.  This is same as other devices
in that we are picking a pair from the channels that can otherwise be
used as single end channels. (I think). 

For this binding I think I'd like to see an example added showing how
this works...

Thanks,

Jonathan
> 
> Jonathan
> 
> > ---
> >  Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
> > index 48bfcaa3..c3c8de1 100644
> > --- a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
> > +++ b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
> > @@ -62,6 +62,12 @@ Required properties:
> >  - st,adc-channels: List of single-ended channels muxed for this ADC.
> >    It can have up to 16 channels on stm32f4 or 20 channels on stm32h7, numbered
> >    from 0 to 15 or 19 (resp. for in0..in15 or in0..in19).
> > +- st,adc-diff-channels: List of differential channels muxed for this ADC.
> > +  Depending on part used, some channels can be configured as differential
> > +  instead of single-ended (e.g. stm32h7). List here positive and negative
> > +  inputs pairs as <vinp vinn>, <vinp vinn>,... vinp and vinn are numbered
> > +  from 0 to 19 on stm32h7)
> > +  Note: At least one of "st,adc-channels" or "st,adc-diff-channels" is required.
> >  - #io-channel-cells = <1>: See the IIO bindings section "IIO consumers" in
> >    Documentation/devicetree/bindings/iio/iio-bindings.txt
> >    
> 
> --
> 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 1/3] dt-bindings: iio: adc: stm32: add support for diff channels

[Jonathan Cameron]

On Sat, 21 Oct 2017 18:54:01 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> On Tue, 17 Oct 2017 15:15:43 +0200
> Fabrice Gasnier <fabrice.gasnier@st.com> wrote:
> 
> > STM32H7 ADC channels may be configured either as single-ended or
> > differential.
> > Add 'st,adc-diff-channels' property to support differential channels.
> > Differential channels are defined as a pair of positive and negative
> > inputs: vinp & vinn.
> > 
> > Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>  
> Hmm. Fair enough.  Sometimes we support both types of channels
> and leave it to userspace, but in many cases that makes little sense
> - particularly if like I think is going on here, we aren't combining channels
> that can be separately read but rather the negative pin is simply unused
> when we are in single channel mode... (did I understand that right?)
I clearly didn't understand.  This is same as other devices
in that we are picking a pair from the channels that can otherwise be
used as single end channels. (I think). 

For this binding I think I'd like to see an example added showing how
this works...

Thanks,

Jonathan
> 
> Jonathan
> 
> > ---
> >  Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
> > index 48bfcaa3..c3c8de1 100644
> > --- a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
> > +++ b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
> > @@ -62,6 +62,12 @@ Required properties:
> >  - st,adc-channels: List of single-ended channels muxed for this ADC.
> >    It can have up to 16 channels on stm32f4 or 20 channels on stm32h7, numbered
> >    from 0 to 15 or 19 (resp. for in0..in15 or in0..in19).
> > +- st,adc-diff-channels: List of differential channels muxed for this ADC.
> > +  Depending on part used, some channels can be configured as differential
> > +  instead of single-ended (e.g. stm32h7). List here positive and negative
> > +  inputs pairs as <vinp vinn>, <vinp vinn>,... vinp and vinn are numbered
> > +  from 0 to 19 on stm32h7)
> > +  Note: At least one of "st,adc-channels" or "st,adc-diff-channels" is required.
> >  - #io-channel-cells = <1>: See the IIO bindings section "IIO consumers" in
> >    Documentation/devicetree/bindings/iio/iio-bindings.txt
> >    
> 
> --
> 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

[PATCH 1/3] dt-bindings: iio: adc: stm32: add support for diff channels

[Jonathan Cameron]

On Sat, 21 Oct 2017 18:54:01 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> On Tue, 17 Oct 2017 15:15:43 +0200
> Fabrice Gasnier <fabrice.gasnier@st.com> wrote:
> 
> > STM32H7 ADC channels may be configured either as single-ended or
> > differential.
> > Add 'st,adc-diff-channels' property to support differential channels.
> > Differential channels are defined as a pair of positive and negative
> > inputs: vinp & vinn.
> > 
> > Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>  
> Hmm. Fair enough.  Sometimes we support both types of channels
> and leave it to userspace, but in many cases that makes little sense
> - particularly if like I think is going on here, we aren't combining channels
> that can be separately read but rather the negative pin is simply unused
> when we are in single channel mode... (did I understand that right?)
I clearly didn't understand.  This is same as other devices
in that we are picking a pair from the channels that can otherwise be
used as single end channels. (I think). 

For this binding I think I'd like to see an example added showing how
this works...

Thanks,

Jonathan
> 
> Jonathan
> 
> > ---
> >  Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
> > index 48bfcaa3..c3c8de1 100644
> > --- a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
> > +++ b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
> > @@ -62,6 +62,12 @@ Required properties:
> >  - st,adc-channels: List of single-ended channels muxed for this ADC.
> >    It can have up to 16 channels on stm32f4 or 20 channels on stm32h7, numbered
> >    from 0 to 15 or 19 (resp. for in0..in15 or in0..in19).
> > +- st,adc-diff-channels: List of differential channels muxed for this ADC.
> > +  Depending on part used, some channels can be configured as differential
> > +  instead of single-ended (e.g. stm32h7). List here positive and negative
> > +  inputs pairs as <vinp vinn>, <vinp vinn>,... vinp and vinn are numbered
> > +  from 0 to 19 on stm32h7)
> > +  Note: At least one of "st,adc-channels" or "st,adc-diff-channels" is required.
> >  - #io-channel-cells = <1>: See the IIO bindings section "IIO consumers" in
> >    Documentation/devicetree/bindings/iio/iio-bindings.txt
> >    
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] iio: adc: stm32: add support for differential channels

[Jonathan Cameron]

On Tue, 17 Oct 2017 15:15:45 +0200
Fabrice Gasnier <fabrice.gasnier@st.com> wrote:

> STM32H7 ADC channels can be configured either as single ended or
> differential with 'st,adc-channels' or 'st,adc-diff-channels'
> (positive and negative input pair: <vinp vinn>, ...).
> 
> Differential channels have different offset and scale, from spec:
> raw value = (full_scale / 2) * (1 + (vinp - vinn) / vref).
> Add offset attribute.
> 
> Differential channels are selected by DIFSEL register. Negative
> inputs must be added to pre-selected channels as well (PCSEL).
> 
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
Looks fine to me. Will wait to see what others think on the binding in
particular.  I suppose that given we allow control over which single ended
channels are registered, it makes sense to do it for differential channels
as well.

Thanks,

Jonathan
> ---
>  drivers/iio/adc/stm32-adc.c | 123 +++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 103 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
> index c95e6f7..cc7ca50 100644
> --- a/drivers/iio/adc/stm32-adc.c
> +++ b/drivers/iio/adc/stm32-adc.c
> @@ -92,6 +92,7 @@
>  #define STM32H7_ADC_SQR3		0x38
>  #define STM32H7_ADC_SQR4		0x3C
>  #define STM32H7_ADC_DR			0x40
> +#define STM32H7_ADC_DIFSEL		0xC0
>  #define STM32H7_ADC_CALFACT		0xC4
>  #define STM32H7_ADC_CALFACT2		0xC8
>  
> @@ -154,7 +155,7 @@ enum stm32h7_adc_dmngt {
>  #define STM32H7_BOOST_CLKRATE		20000000UL
>  
>  #define STM32_ADC_CH_MAX		20	/* max number of channels */
> -#define STM32_ADC_CH_SZ			5	/* max channel name size */
> +#define STM32_ADC_CH_SZ			10	/* max channel name size */
>  #define STM32_ADC_MAX_SQ		16	/* SQ1..SQ16 */
>  #define STM32_ADC_MAX_SMP		7	/* SMPx range is [0..7] */
>  #define STM32_ADC_TIMEOUT_US		100000
> @@ -299,6 +300,7 @@ struct stm32_adc_cfg {
>   * @rx_buf:		dma rx buffer cpu address
>   * @rx_dma_buf:		dma rx buffer bus address
>   * @rx_buf_sz:		dma rx buffer size
> + * @difsel		bitmask to set single-ended/differential channel
>   * @pcsel		bitmask to preselect channels on some devices
>   * @smpr_val:		sampling time settings (e.g. smpr1 / smpr2)
>   * @cal:		optional calibration data on some devices
> @@ -321,12 +323,18 @@ struct stm32_adc {
>  	u8			*rx_buf;
>  	dma_addr_t		rx_dma_buf;
>  	unsigned int		rx_buf_sz;
> +	u32			difsel;
>  	u32			pcsel;
>  	u32			smpr_val[2];
>  	struct stm32_adc_calib	cal;
>  	char			chan_name[STM32_ADC_CH_MAX][STM32_ADC_CH_SZ];
>  };
>  
> +struct stm32_adc_diff_channel {
> +	u32 vinp;
> +	u32 vinn;
> +};
> +
>  /**
>   * struct stm32_adc_info - stm32 ADC, per instance config data
>   * @max_channels:	Number of channels
> @@ -944,15 +952,19 @@ static int stm32h7_adc_selfcalib(struct stm32_adc *adc)
>   * stm32h7_adc_prepare() - Leave power down mode to enable ADC.
>   * @adc: stm32 adc instance
>   * Leave power down mode.
> + * Configure channels as single ended or differential before enabling ADC.
>   * Enable ADC.
>   * Restore calibration data.
> - * Pre-select channels that may be used in PCSEL (required by input MUX / IO).
> + * Pre-select channels that may be used in PCSEL (required by input MUX / IO):
> + * - Only one input is selected for single ended (e.g. 'vinp')
> + * - Two inputs are selected for differential channels (e.g. 'vinp' & 'vinn')
>   */
>  static int stm32h7_adc_prepare(struct stm32_adc *adc)
>  {
>  	int ret;
>  
>  	stm32h7_adc_exit_pwr_down(adc);
> +	stm32_adc_writel(adc, STM32H7_ADC_DIFSEL, adc->difsel);
>  
>  	ret = stm32h7_adc_enable(adc);
>  	if (ret)
> @@ -1224,10 +1236,23 @@ static int stm32_adc_read_raw(struct iio_dev *indio_dev,
>  		return ret;
>  
>  	case IIO_CHAN_INFO_SCALE:
> -		*val = adc->common->vref_mv;
> -		*val2 = chan->scan_type.realbits;
> +		if (chan->differential) {
> +			*val = adc->common->vref_mv * 2;
> +			*val2 = chan->scan_type.realbits;
> +		} else {
> +			*val = adc->common->vref_mv;
> +			*val2 = chan->scan_type.realbits;
> +		}
>  		return IIO_VAL_FRACTIONAL_LOG2;
>  
> +	case IIO_CHAN_INFO_OFFSET:
> +		if (chan->differential)
> +			/* ADC_full_scale / 2 */
> +			*val = -((1 << chan->scan_type.realbits) / 2);
> +		else
> +			*val = 0;
> +		return IIO_VAL_INT;
> +
>  	default:
>  		return -EINVAL;
>  	}
> @@ -1591,29 +1616,39 @@ static void stm32_adc_smpr_init(struct stm32_adc *adc, int channel, u32 smp_ns)
>  
>  static void stm32_adc_chan_init_one(struct iio_dev *indio_dev,
>  				    struct iio_chan_spec *chan, u32 val,
> -				    int scan_index, u32 smp)
> +				    u32 val2, int scan_index, bool differential)
val and val2 are slightly odd names in here.
>  {
>  	struct stm32_adc *adc = iio_priv(indio_dev);
>  	char *name = adc->chan_name[val];
>  
>  	chan->type = IIO_VOLTAGE;
>  	chan->channel = val;
> -	snprintf(name, STM32_ADC_CH_SZ, "in%d", val);
> +	if (differential) {
> +		chan->differential = 1;
> +		chan->channel2 = val2;
> +		snprintf(name, STM32_ADC_CH_SZ, "in%d-in%d", val, val2);
> +	} else {
> +		snprintf(name, STM32_ADC_CH_SZ, "in%d", val);
> +	}
>  	chan->datasheet_name = name;
>  	chan->scan_index = scan_index;
>  	chan->indexed = 1;
>  	chan->info_mask_separate = BIT(IIO_CHAN_INFO_RAW);
> -	chan->info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE);
> +	chan->info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |
> +					 BIT(IIO_CHAN_INFO_OFFSET);
>  	chan->scan_type.sign = 'u';
>  	chan->scan_type.realbits = adc->cfg->adc_info->resolutions[adc->res];
>  	chan->scan_type.storagebits = 16;
>  	chan->ext_info = stm32_adc_ext_info;
>  
> -	/* Prepare sampling time settings */
> -	stm32_adc_smpr_init(adc, chan->channel, smp);
> -
>  	/* pre-build selected channels mask */
>  	adc->pcsel |= BIT(chan->channel);
> +	if (differential) {
> +		/* pre-build diff channels mask */
> +		adc->difsel |= BIT(chan->channel);
> +		/* Also add negative input to pre-selected channels */
> +		adc->pcsel |= BIT(chan->channel2);
> +	}
>  }
>  
>  static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
> @@ -1621,17 +1656,40 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
>  	struct device_node *node = indio_dev->dev.of_node;
>  	struct stm32_adc *adc = iio_priv(indio_dev);
>  	const struct stm32_adc_info *adc_info = adc->cfg->adc_info;
> +	struct stm32_adc_diff_channel diff[STM32_ADC_CH_MAX];
>  	struct property *prop;
>  	const __be32 *cur;
>  	struct iio_chan_spec *channels;
> -	int scan_index = 0, num_channels, ret;
> +	int scan_index = 0, num_channels = 0, num_diff = 0, ret, i;
>  	u32 val, smp = 0;
>  
> -	num_channels = of_property_count_u32_elems(node, "st,adc-channels");
> -	if (num_channels < 0 ||
> -	    num_channels > adc_info->max_channels) {
> +	ret = of_property_count_u32_elems(node, "st,adc-channels");
> +	if (ret > adc_info->max_channels) {
>  		dev_err(&indio_dev->dev, "Bad st,adc-channels?\n");
> -		return num_channels < 0 ? num_channels : -EINVAL;
> +		return -EINVAL;
> +	} else if (ret > 0) {
> +		num_channels += ret;
> +	}
> +
> +	ret = of_property_count_elems_of_size(node, "st,adc-diff-channels",
> +					      sizeof(*diff));
> +	if (ret > adc_info->max_channels) {
> +		dev_err(&indio_dev->dev, "Bad st,adc-diff-channels?\n");
> +		return -EINVAL;
> +	} else if (ret > 0) {
> +		int size = ret * sizeof(*diff) / sizeof(u32);
> +
> +		num_diff = ret;
> +		num_channels += ret;
> +		ret = of_property_read_u32_array(node, "st,adc-diff-channels",
> +						 (u32 *)diff, size);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	if (!num_channels) {
> +		dev_err(&indio_dev->dev, "No channels configured\n");
> +		return -ENODATA;
>  	}
>  
>  	/* Optional sample time is provided either for each, or all channels */
> @@ -1652,6 +1710,33 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
>  			return -EINVAL;
>  		}
>  
> +		/* Channel can't be configured both as single-ended & diff */
> +		for (i = 0; i < num_diff; i++) {
> +			if (val == diff[i].vinp) {
> +				dev_err(&indio_dev->dev,
> +					"channel %d miss-configured\n",	val);
> +				return -EINVAL;
> +			}
> +		}
> +		stm32_adc_chan_init_one(indio_dev, &channels[scan_index], val,
> +					0, scan_index, false);
> +		scan_index++;
> +	}
> +
> +	for (i = 0; i < num_diff; i++) {
> +		if (diff[i].vinp >= adc_info->max_channels ||
> +		    diff[i].vinn >= adc_info->max_channels) {
> +			dev_err(&indio_dev->dev, "Invalid channel in%d-in%d\n",
> +				diff[i].vinp, diff[i].vinn);
> +			return -EINVAL;
> +		}
> +		stm32_adc_chan_init_one(indio_dev, &channels[scan_index],
> +					diff[i].vinp, diff[i].vinn, scan_index,
> +					true);
> +		scan_index++;
> +	}
> +
> +	for (i = 0; i < scan_index; i++) {
>  		/*
>  		 * Using of_property_read_u32_index(), smp value will only be
>  		 * modified if valid u32 value can be decoded. This allows to
> @@ -1659,11 +1744,9 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
>  		 * value per channel.
>  		 */
>  		of_property_read_u32_index(node, "st,min-sample-time-nsecs",
> -					   scan_index, &smp);
> -
> -		stm32_adc_chan_init_one(indio_dev, &channels[scan_index],
> -					val, scan_index, smp);
> -		scan_index++;
> +					   i, &smp);
> +		/* Prepare sampling time settings */
> +		stm32_adc_smpr_init(adc, channels[i].channel, smp);
>  	}
>  
>  	indio_dev->num_channels = scan_index;

Re: [PATCH 3/3] iio: adc: stm32: add support for differential channels

[Jonathan Cameron]

On Tue, 17 Oct 2017 15:15:45 +0200
Fabrice Gasnier <fabrice.gasnier-qxv4g6HH51o@public.gmane.org> wrote:

> STM32H7 ADC channels can be configured either as single ended or
> differential with 'st,adc-channels' or 'st,adc-diff-channels'
> (positive and negative input pair: <vinp vinn>, ...).
> 
> Differential channels have different offset and scale, from spec:
> raw value = (full_scale / 2) * (1 + (vinp - vinn) / vref).
> Add offset attribute.
> 
> Differential channels are selected by DIFSEL register. Negative
> inputs must be added to pre-selected channels as well (PCSEL).
> 
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier-qxv4g6HH51o@public.gmane.org>
Looks fine to me. Will wait to see what others think on the binding in
particular.  I suppose that given we allow control over which single ended
channels are registered, it makes sense to do it for differential channels
as well.

Thanks,

Jonathan
> ---
>  drivers/iio/adc/stm32-adc.c | 123 +++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 103 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
> index c95e6f7..cc7ca50 100644
> --- a/drivers/iio/adc/stm32-adc.c
> +++ b/drivers/iio/adc/stm32-adc.c
> @@ -92,6 +92,7 @@
>  #define STM32H7_ADC_SQR3		0x38
>  #define STM32H7_ADC_SQR4		0x3C
>  #define STM32H7_ADC_DR			0x40
> +#define STM32H7_ADC_DIFSEL		0xC0
>  #define STM32H7_ADC_CALFACT		0xC4
>  #define STM32H7_ADC_CALFACT2		0xC8
>  
> @@ -154,7 +155,7 @@ enum stm32h7_adc_dmngt {
>  #define STM32H7_BOOST_CLKRATE		20000000UL
>  
>  #define STM32_ADC_CH_MAX		20	/* max number of channels */
> -#define STM32_ADC_CH_SZ			5	/* max channel name size */
> +#define STM32_ADC_CH_SZ			10	/* max channel name size */
>  #define STM32_ADC_MAX_SQ		16	/* SQ1..SQ16 */
>  #define STM32_ADC_MAX_SMP		7	/* SMPx range is [0..7] */
>  #define STM32_ADC_TIMEOUT_US		100000
> @@ -299,6 +300,7 @@ struct stm32_adc_cfg {
>   * @rx_buf:		dma rx buffer cpu address
>   * @rx_dma_buf:		dma rx buffer bus address
>   * @rx_buf_sz:		dma rx buffer size
> + * @difsel		bitmask to set single-ended/differential channel
>   * @pcsel		bitmask to preselect channels on some devices
>   * @smpr_val:		sampling time settings (e.g. smpr1 / smpr2)
>   * @cal:		optional calibration data on some devices
> @@ -321,12 +323,18 @@ struct stm32_adc {
>  	u8			*rx_buf;
>  	dma_addr_t		rx_dma_buf;
>  	unsigned int		rx_buf_sz;
> +	u32			difsel;
>  	u32			pcsel;
>  	u32			smpr_val[2];
>  	struct stm32_adc_calib	cal;
>  	char			chan_name[STM32_ADC_CH_MAX][STM32_ADC_CH_SZ];
>  };
>  
> +struct stm32_adc_diff_channel {
> +	u32 vinp;
> +	u32 vinn;
> +};
> +
>  /**
>   * struct stm32_adc_info - stm32 ADC, per instance config data
>   * @max_channels:	Number of channels
> @@ -944,15 +952,19 @@ static int stm32h7_adc_selfcalib(struct stm32_adc *adc)
>   * stm32h7_adc_prepare() - Leave power down mode to enable ADC.
>   * @adc: stm32 adc instance
>   * Leave power down mode.
> + * Configure channels as single ended or differential before enabling ADC.
>   * Enable ADC.
>   * Restore calibration data.
> - * Pre-select channels that may be used in PCSEL (required by input MUX / IO).
> + * Pre-select channels that may be used in PCSEL (required by input MUX / IO):
> + * - Only one input is selected for single ended (e.g. 'vinp')
> + * - Two inputs are selected for differential channels (e.g. 'vinp' & 'vinn')
>   */
>  static int stm32h7_adc_prepare(struct stm32_adc *adc)
>  {
>  	int ret;
>  
>  	stm32h7_adc_exit_pwr_down(adc);
> +	stm32_adc_writel(adc, STM32H7_ADC_DIFSEL, adc->difsel);
>  
>  	ret = stm32h7_adc_enable(adc);
>  	if (ret)
> @@ -1224,10 +1236,23 @@ static int stm32_adc_read_raw(struct iio_dev *indio_dev,
>  		return ret;
>  
>  	case IIO_CHAN_INFO_SCALE:
> -		*val = adc->common->vref_mv;
> -		*val2 = chan->scan_type.realbits;
> +		if (chan->differential) {
> +			*val = adc->common->vref_mv * 2;
> +			*val2 = chan->scan_type.realbits;
> +		} else {
> +			*val = adc->common->vref_mv;
> +			*val2 = chan->scan_type.realbits;
> +		}
>  		return IIO_VAL_FRACTIONAL_LOG2;
>  
> +	case IIO_CHAN_INFO_OFFSET:
> +		if (chan->differential)
> +			/* ADC_full_scale / 2 */
> +			*val = -((1 << chan->scan_type.realbits) / 2);
> +		else
> +			*val = 0;
> +		return IIO_VAL_INT;
> +
>  	default:
>  		return -EINVAL;
>  	}
> @@ -1591,29 +1616,39 @@ static void stm32_adc_smpr_init(struct stm32_adc *adc, int channel, u32 smp_ns)
>  
>  static void stm32_adc_chan_init_one(struct iio_dev *indio_dev,
>  				    struct iio_chan_spec *chan, u32 val,
> -				    int scan_index, u32 smp)
> +				    u32 val2, int scan_index, bool differential)
val and val2 are slightly odd names in here.
>  {
>  	struct stm32_adc *adc = iio_priv(indio_dev);
>  	char *name = adc->chan_name[val];
>  
>  	chan->type = IIO_VOLTAGE;
>  	chan->channel = val;
> -	snprintf(name, STM32_ADC_CH_SZ, "in%d", val);
> +	if (differential) {
> +		chan->differential = 1;
> +		chan->channel2 = val2;
> +		snprintf(name, STM32_ADC_CH_SZ, "in%d-in%d", val, val2);
> +	} else {
> +		snprintf(name, STM32_ADC_CH_SZ, "in%d", val);
> +	}
>  	chan->datasheet_name = name;
>  	chan->scan_index = scan_index;
>  	chan->indexed = 1;
>  	chan->info_mask_separate = BIT(IIO_CHAN_INFO_RAW);
> -	chan->info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE);
> +	chan->info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |
> +					 BIT(IIO_CHAN_INFO_OFFSET);
>  	chan->scan_type.sign = 'u';
>  	chan->scan_type.realbits = adc->cfg->adc_info->resolutions[adc->res];
>  	chan->scan_type.storagebits = 16;
>  	chan->ext_info = stm32_adc_ext_info;
>  
> -	/* Prepare sampling time settings */
> -	stm32_adc_smpr_init(adc, chan->channel, smp);
> -
>  	/* pre-build selected channels mask */
>  	adc->pcsel |= BIT(chan->channel);
> +	if (differential) {
> +		/* pre-build diff channels mask */
> +		adc->difsel |= BIT(chan->channel);
> +		/* Also add negative input to pre-selected channels */
> +		adc->pcsel |= BIT(chan->channel2);
> +	}
>  }
>  
>  static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
> @@ -1621,17 +1656,40 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
>  	struct device_node *node = indio_dev->dev.of_node;
>  	struct stm32_adc *adc = iio_priv(indio_dev);
>  	const struct stm32_adc_info *adc_info = adc->cfg->adc_info;
> +	struct stm32_adc_diff_channel diff[STM32_ADC_CH_MAX];
>  	struct property *prop;
>  	const __be32 *cur;
>  	struct iio_chan_spec *channels;
> -	int scan_index = 0, num_channels, ret;
> +	int scan_index = 0, num_channels = 0, num_diff = 0, ret, i;
>  	u32 val, smp = 0;
>  
> -	num_channels = of_property_count_u32_elems(node, "st,adc-channels");
> -	if (num_channels < 0 ||
> -	    num_channels > adc_info->max_channels) {
> +	ret = of_property_count_u32_elems(node, "st,adc-channels");
> +	if (ret > adc_info->max_channels) {
>  		dev_err(&indio_dev->dev, "Bad st,adc-channels?\n");
> -		return num_channels < 0 ? num_channels : -EINVAL;
> +		return -EINVAL;
> +	} else if (ret > 0) {
> +		num_channels += ret;
> +	}
> +
> +	ret = of_property_count_elems_of_size(node, "st,adc-diff-channels",
> +					      sizeof(*diff));
> +	if (ret > adc_info->max_channels) {
> +		dev_err(&indio_dev->dev, "Bad st,adc-diff-channels?\n");
> +		return -EINVAL;
> +	} else if (ret > 0) {
> +		int size = ret * sizeof(*diff) / sizeof(u32);
> +
> +		num_diff = ret;
> +		num_channels += ret;
> +		ret = of_property_read_u32_array(node, "st,adc-diff-channels",
> +						 (u32 *)diff, size);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	if (!num_channels) {
> +		dev_err(&indio_dev->dev, "No channels configured\n");
> +		return -ENODATA;
>  	}
>  
>  	/* Optional sample time is provided either for each, or all channels */
> @@ -1652,6 +1710,33 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
>  			return -EINVAL;
>  		}
>  
> +		/* Channel can't be configured both as single-ended & diff */
> +		for (i = 0; i < num_diff; i++) {
> +			if (val == diff[i].vinp) {
> +				dev_err(&indio_dev->dev,
> +					"channel %d miss-configured\n",	val);
> +				return -EINVAL;
> +			}
> +		}
> +		stm32_adc_chan_init_one(indio_dev, &channels[scan_index], val,
> +					0, scan_index, false);
> +		scan_index++;
> +	}
> +
> +	for (i = 0; i < num_diff; i++) {
> +		if (diff[i].vinp >= adc_info->max_channels ||
> +		    diff[i].vinn >= adc_info->max_channels) {
> +			dev_err(&indio_dev->dev, "Invalid channel in%d-in%d\n",
> +				diff[i].vinp, diff[i].vinn);
> +			return -EINVAL;
> +		}
> +		stm32_adc_chan_init_one(indio_dev, &channels[scan_index],
> +					diff[i].vinp, diff[i].vinn, scan_index,
> +					true);
> +		scan_index++;
> +	}
> +
> +	for (i = 0; i < scan_index; i++) {
>  		/*
>  		 * Using of_property_read_u32_index(), smp value will only be
>  		 * modified if valid u32 value can be decoded. This allows to
> @@ -1659,11 +1744,9 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
>  		 * value per channel.
>  		 */
>  		of_property_read_u32_index(node, "st,min-sample-time-nsecs",
> -					   scan_index, &smp);
> -
> -		stm32_adc_chan_init_one(indio_dev, &channels[scan_index],
> -					val, scan_index, smp);
> -		scan_index++;
> +					   i, &smp);
> +		/* Prepare sampling time settings */
> +		stm32_adc_smpr_init(adc, channels[i].channel, smp);
>  	}
>  
>  	indio_dev->num_channels = scan_index;

[PATCH 3/3] iio: adc: stm32: add support for differential channels

[Jonathan Cameron]

On Tue, 17 Oct 2017 15:15:45 +0200
Fabrice Gasnier <fabrice.gasnier@st.com> wrote:

> STM32H7 ADC channels can be configured either as single ended or
> differential with 'st,adc-channels' or 'st,adc-diff-channels'
> (positive and negative input pair: <vinp vinn>, ...).
> 
> Differential channels have different offset and scale, from spec:
> raw value = (full_scale / 2) * (1 + (vinp - vinn) / vref).
> Add offset attribute.
> 
> Differential channels are selected by DIFSEL register. Negative
> inputs must be added to pre-selected channels as well (PCSEL).
> 
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
Looks fine to me. Will wait to see what others think on the binding in
particular.  I suppose that given we allow control over which single ended
channels are registered, it makes sense to do it for differential channels
as well.

Thanks,

Jonathan
> ---
>  drivers/iio/adc/stm32-adc.c | 123 +++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 103 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
> index c95e6f7..cc7ca50 100644
> --- a/drivers/iio/adc/stm32-adc.c
> +++ b/drivers/iio/adc/stm32-adc.c
> @@ -92,6 +92,7 @@
>  #define STM32H7_ADC_SQR3		0x38
>  #define STM32H7_ADC_SQR4		0x3C
>  #define STM32H7_ADC_DR			0x40
> +#define STM32H7_ADC_DIFSEL		0xC0
>  #define STM32H7_ADC_CALFACT		0xC4
>  #define STM32H7_ADC_CALFACT2		0xC8
>  
> @@ -154,7 +155,7 @@ enum stm32h7_adc_dmngt {
>  #define STM32H7_BOOST_CLKRATE		20000000UL
>  
>  #define STM32_ADC_CH_MAX		20	/* max number of channels */
> -#define STM32_ADC_CH_SZ			5	/* max channel name size */
> +#define STM32_ADC_CH_SZ			10	/* max channel name size */
>  #define STM32_ADC_MAX_SQ		16	/* SQ1..SQ16 */
>  #define STM32_ADC_MAX_SMP		7	/* SMPx range is [0..7] */
>  #define STM32_ADC_TIMEOUT_US		100000
> @@ -299,6 +300,7 @@ struct stm32_adc_cfg {
>   * @rx_buf:		dma rx buffer cpu address
>   * @rx_dma_buf:		dma rx buffer bus address
>   * @rx_buf_sz:		dma rx buffer size
> + * @difsel		bitmask to set single-ended/differential channel
>   * @pcsel		bitmask to preselect channels on some devices
>   * @smpr_val:		sampling time settings (e.g. smpr1 / smpr2)
>   * @cal:		optional calibration data on some devices
> @@ -321,12 +323,18 @@ struct stm32_adc {
>  	u8			*rx_buf;
>  	dma_addr_t		rx_dma_buf;
>  	unsigned int		rx_buf_sz;
> +	u32			difsel;
>  	u32			pcsel;
>  	u32			smpr_val[2];
>  	struct stm32_adc_calib	cal;
>  	char			chan_name[STM32_ADC_CH_MAX][STM32_ADC_CH_SZ];
>  };
>  
> +struct stm32_adc_diff_channel {
> +	u32 vinp;
> +	u32 vinn;
> +};
> +
>  /**
>   * struct stm32_adc_info - stm32 ADC, per instance config data
>   * @max_channels:	Number of channels
> @@ -944,15 +952,19 @@ static int stm32h7_adc_selfcalib(struct stm32_adc *adc)
>   * stm32h7_adc_prepare() - Leave power down mode to enable ADC.
>   * @adc: stm32 adc instance
>   * Leave power down mode.
> + * Configure channels as single ended or differential before enabling ADC.
>   * Enable ADC.
>   * Restore calibration data.
> - * Pre-select channels that may be used in PCSEL (required by input MUX / IO).
> + * Pre-select channels that may be used in PCSEL (required by input MUX / IO):
> + * - Only one input is selected for single ended (e.g. 'vinp')
> + * - Two inputs are selected for differential channels (e.g. 'vinp' & 'vinn')
>   */
>  static int stm32h7_adc_prepare(struct stm32_adc *adc)
>  {
>  	int ret;
>  
>  	stm32h7_adc_exit_pwr_down(adc);
> +	stm32_adc_writel(adc, STM32H7_ADC_DIFSEL, adc->difsel);
>  
>  	ret = stm32h7_adc_enable(adc);
>  	if (ret)
> @@ -1224,10 +1236,23 @@ static int stm32_adc_read_raw(struct iio_dev *indio_dev,
>  		return ret;
>  
>  	case IIO_CHAN_INFO_SCALE:
> -		*val = adc->common->vref_mv;
> -		*val2 = chan->scan_type.realbits;
> +		if (chan->differential) {
> +			*val = adc->common->vref_mv * 2;
> +			*val2 = chan->scan_type.realbits;
> +		} else {
> +			*val = adc->common->vref_mv;
> +			*val2 = chan->scan_type.realbits;
> +		}
>  		return IIO_VAL_FRACTIONAL_LOG2;
>  
> +	case IIO_CHAN_INFO_OFFSET:
> +		if (chan->differential)
> +			/* ADC_full_scale / 2 */
> +			*val = -((1 << chan->scan_type.realbits) / 2);
> +		else
> +			*val = 0;
> +		return IIO_VAL_INT;
> +
>  	default:
>  		return -EINVAL;
>  	}
> @@ -1591,29 +1616,39 @@ static void stm32_adc_smpr_init(struct stm32_adc *adc, int channel, u32 smp_ns)
>  
>  static void stm32_adc_chan_init_one(struct iio_dev *indio_dev,
>  				    struct iio_chan_spec *chan, u32 val,
> -				    int scan_index, u32 smp)
> +				    u32 val2, int scan_index, bool differential)
val and val2 are slightly odd names in here.
>  {
>  	struct stm32_adc *adc = iio_priv(indio_dev);
>  	char *name = adc->chan_name[val];
>  
>  	chan->type = IIO_VOLTAGE;
>  	chan->channel = val;
> -	snprintf(name, STM32_ADC_CH_SZ, "in%d", val);
> +	if (differential) {
> +		chan->differential = 1;
> +		chan->channel2 = val2;
> +		snprintf(name, STM32_ADC_CH_SZ, "in%d-in%d", val, val2);
> +	} else {
> +		snprintf(name, STM32_ADC_CH_SZ, "in%d", val);
> +	}
>  	chan->datasheet_name = name;
>  	chan->scan_index = scan_index;
>  	chan->indexed = 1;
>  	chan->info_mask_separate = BIT(IIO_CHAN_INFO_RAW);
> -	chan->info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE);
> +	chan->info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |
> +					 BIT(IIO_CHAN_INFO_OFFSET);
>  	chan->scan_type.sign = 'u';
>  	chan->scan_type.realbits = adc->cfg->adc_info->resolutions[adc->res];
>  	chan->scan_type.storagebits = 16;
>  	chan->ext_info = stm32_adc_ext_info;
>  
> -	/* Prepare sampling time settings */
> -	stm32_adc_smpr_init(adc, chan->channel, smp);
> -
>  	/* pre-build selected channels mask */
>  	adc->pcsel |= BIT(chan->channel);
> +	if (differential) {
> +		/* pre-build diff channels mask */
> +		adc->difsel |= BIT(chan->channel);
> +		/* Also add negative input to pre-selected channels */
> +		adc->pcsel |= BIT(chan->channel2);
> +	}
>  }
>  
>  static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
> @@ -1621,17 +1656,40 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
>  	struct device_node *node = indio_dev->dev.of_node;
>  	struct stm32_adc *adc = iio_priv(indio_dev);
>  	const struct stm32_adc_info *adc_info = adc->cfg->adc_info;
> +	struct stm32_adc_diff_channel diff[STM32_ADC_CH_MAX];
>  	struct property *prop;
>  	const __be32 *cur;
>  	struct iio_chan_spec *channels;
> -	int scan_index = 0, num_channels, ret;
> +	int scan_index = 0, num_channels = 0, num_diff = 0, ret, i;
>  	u32 val, smp = 0;
>  
> -	num_channels = of_property_count_u32_elems(node, "st,adc-channels");
> -	if (num_channels < 0 ||
> -	    num_channels > adc_info->max_channels) {
> +	ret = of_property_count_u32_elems(node, "st,adc-channels");
> +	if (ret > adc_info->max_channels) {
>  		dev_err(&indio_dev->dev, "Bad st,adc-channels?\n");
> -		return num_channels < 0 ? num_channels : -EINVAL;
> +		return -EINVAL;
> +	} else if (ret > 0) {
> +		num_channels += ret;
> +	}
> +
> +	ret = of_property_count_elems_of_size(node, "st,adc-diff-channels",
> +					      sizeof(*diff));
> +	if (ret > adc_info->max_channels) {
> +		dev_err(&indio_dev->dev, "Bad st,adc-diff-channels?\n");
> +		return -EINVAL;
> +	} else if (ret > 0) {
> +		int size = ret * sizeof(*diff) / sizeof(u32);
> +
> +		num_diff = ret;
> +		num_channels += ret;
> +		ret = of_property_read_u32_array(node, "st,adc-diff-channels",
> +						 (u32 *)diff, size);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	if (!num_channels) {
> +		dev_err(&indio_dev->dev, "No channels configured\n");
> +		return -ENODATA;
>  	}
>  
>  	/* Optional sample time is provided either for each, or all channels */
> @@ -1652,6 +1710,33 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
>  			return -EINVAL;
>  		}
>  
> +		/* Channel can't be configured both as single-ended & diff */
> +		for (i = 0; i < num_diff; i++) {
> +			if (val == diff[i].vinp) {
> +				dev_err(&indio_dev->dev,
> +					"channel %d miss-configured\n",	val);
> +				return -EINVAL;
> +			}
> +		}
> +		stm32_adc_chan_init_one(indio_dev, &channels[scan_index], val,
> +					0, scan_index, false);
> +		scan_index++;
> +	}
> +
> +	for (i = 0; i < num_diff; i++) {
> +		if (diff[i].vinp >= adc_info->max_channels ||
> +		    diff[i].vinn >= adc_info->max_channels) {
> +			dev_err(&indio_dev->dev, "Invalid channel in%d-in%d\n",
> +				diff[i].vinp, diff[i].vinn);
> +			return -EINVAL;
> +		}
> +		stm32_adc_chan_init_one(indio_dev, &channels[scan_index],
> +					diff[i].vinp, diff[i].vinn, scan_index,
> +					true);
> +		scan_index++;
> +	}
> +
> +	for (i = 0; i < scan_index; i++) {
>  		/*
>  		 * Using of_property_read_u32_index(), smp value will only be
>  		 * modified if valid u32 value can be decoded. This allows to
> @@ -1659,11 +1744,9 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
>  		 * value per channel.
>  		 */
>  		of_property_read_u32_index(node, "st,min-sample-time-nsecs",
> -					   scan_index, &smp);
> -
> -		stm32_adc_chan_init_one(indio_dev, &channels[scan_index],
> -					val, scan_index, smp);
> -		scan_index++;
> +					   i, &smp);
> +		/* Prepare sampling time settings */
> +		stm32_adc_smpr_init(adc, channels[i].channel, smp);
>  	}
>  
>  	indio_dev->num_channels = scan_index;

Re: [PATCH 1/3] dt-bindings: iio: adc: stm32: add support for diff channels

[Fabrice Gasnier]

On 10/21/2017 09:23 PM, Jonathan Cameron wrote:
> On Sat, 21 Oct 2017 18:54:01 +0100
> Jonathan Cameron <jic23@kernel.org> wrote:
> 
>> On Tue, 17 Oct 2017 15:15:43 +0200
>> Fabrice Gasnier <fabrice.gasnier@st.com> wrote:
>>
>>> STM32H7 ADC channels may be configured either as single-ended or
>>> differential.
>>> Add 'st,adc-diff-channels' property to support differential channels.
>>> Differential channels are defined as a pair of positive and negative
>>> inputs: vinp & vinn.
>>>
>>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>  
>> Hmm. Fair enough.  Sometimes we support both types of channels
>> and leave it to userspace, but in many cases that makes little sense
>> - particularly if like I think is going on here, we aren't combining channels
>> that can be separately read but rather the negative pin is simply unused
>> when we are in single channel mode... (did I understand that right?)
> I clearly didn't understand.  This is same as other devices
> in that we are picking a pair from the channels that can otherwise be
> used as single end channels. (I think). 

Hi Jonathan,

Indeed, we're picking a pair from the channels that can be used as
single channels otherwise.
Also, it doesn't makes sense to leave the choice to userspace: when
these channels are used in differential mode, both input should be
biased. This is quite dependent on hardware (board), not really a
runtime choice.
Hope this clarifies ?

> 
> For this binding I think I'd like to see an example added showing how
> this works...

You're right, I should have added that. Please let me know if I can add
bellow example in v2:

Example to setup differential channels 1, 2 & 3 (with resp. 0, 6 & 7
negative inputs):
	adc: adc@40022000 {
		compatible = "st,stm32h7-adc-core";
		...
		adc1: adc@0 {
			compatible = "st,stm32h7-adc";
			...
			st,adc-diff-channels = <1 0>, <2 6>, <3 7>;
		};
	};

Thanks for reviewing,
Best Regards,
Fabrice

> 
> Thanks,
> 
> Jonathan
>>
>> Jonathan
>>
>>> ---
>>>  Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt | 6 ++++++
>>>  1 file changed, 6 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
>>> index 48bfcaa3..c3c8de1 100644
>>> --- a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
>>> +++ b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
>>> @@ -62,6 +62,12 @@ Required properties:
>>>  - st,adc-channels: List of single-ended channels muxed for this ADC.
>>>    It can have up to 16 channels on stm32f4 or 20 channels on stm32h7, numbered
>>>    from 0 to 15 or 19 (resp. for in0..in15 or in0..in19).
>>> +- st,adc-diff-channels: List of differential channels muxed for this ADC.
>>> +  Depending on part used, some channels can be configured as differential
>>> +  instead of single-ended (e.g. stm32h7). List here positive and negative
>>> +  inputs pairs as <vinp vinn>, <vinp vinn>,... vinp and vinn are numbered
>>> +  from 0 to 19 on stm32h7)
>>> +  Note: At least one of "st,adc-channels" or "st,adc-diff-channels" is required.
>>>  - #io-channel-cells = <1>: See the IIO bindings section "IIO consumers" in
>>>    Documentation/devicetree/bindings/iio/iio-bindings.txt
>>>    
>>
>> --
>> 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 1/3] dt-bindings: iio: adc: stm32: add support for diff channels

[Fabrice Gasnier]

On 10/21/2017 09:23 PM, Jonathan Cameron wrote:
> On Sat, 21 Oct 2017 18:54:01 +0100
> Jonathan Cameron <jic23@kernel.org> wrote:
> 
>> On Tue, 17 Oct 2017 15:15:43 +0200
>> Fabrice Gasnier <fabrice.gasnier@st.com> wrote:
>>
>>> STM32H7 ADC channels may be configured either as single-ended or
>>> differential.
>>> Add 'st,adc-diff-channels' property to support differential channels.
>>> Differential channels are defined as a pair of positive and negative
>>> inputs: vinp & vinn.
>>>
>>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>  
>> Hmm. Fair enough.  Sometimes we support both types of channels
>> and leave it to userspace, but in many cases that makes little sense
>> - particularly if like I think is going on here, we aren't combining channels
>> that can be separately read but rather the negative pin is simply unused
>> when we are in single channel mode... (did I understand that right?)
> I clearly didn't understand.  This is same as other devices
> in that we are picking a pair from the channels that can otherwise be
> used as single end channels. (I think). 

Hi Jonathan,

Indeed, we're picking a pair from the channels that can be used as
single channels otherwise.
Also, it doesn't makes sense to leave the choice to userspace: when
these channels are used in differential mode, both input should be
biased. This is quite dependent on hardware (board), not really a
runtime choice.
Hope this clarifies ?

> 
> For this binding I think I'd like to see an example added showing how
> this works...

You're right, I should have added that. Please let me know if I can add
bellow example in v2:

Example to setup differential channels 1, 2 & 3 (with resp. 0, 6 & 7
negative inputs):
	adc: adc@40022000 {
		compatible = "st,stm32h7-adc-core";
		...
		adc1: adc@0 {
			compatible = "st,stm32h7-adc";
			...
			st,adc-diff-channels = <1 0>, <2 6>, <3 7>;
		};
	};

Thanks for reviewing,
Best Regards,
Fabrice

> 
> Thanks,
> 
> Jonathan
>>
>> Jonathan
>>
>>> ---
>>>  Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt | 6 ++++++
>>>  1 file changed, 6 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
>>> index 48bfcaa3..c3c8de1 100644
>>> --- a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
>>> +++ b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
>>> @@ -62,6 +62,12 @@ Required properties:
>>>  - st,adc-channels: List of single-ended channels muxed for this ADC.
>>>    It can have up to 16 channels on stm32f4 or 20 channels on stm32h7, numbered
>>>    from 0 to 15 or 19 (resp. for in0..in15 or in0..in19).
>>> +- st,adc-diff-channels: List of differential channels muxed for this ADC.
>>> +  Depending on part used, some channels can be configured as differential
>>> +  instead of single-ended (e.g. stm32h7). List here positive and negative
>>> +  inputs pairs as <vinp vinn>, <vinp vinn>,... vinp and vinn are numbered
>>> +  from 0 to 19 on stm32h7)
>>> +  Note: At least one of "st,adc-channels" or "st,adc-diff-channels" is required.
>>>  - #io-channel-cells = <1>: See the IIO bindings section "IIO consumers" in
>>>    Documentation/devicetree/bindings/iio/iio-bindings.txt
>>>    
>>
>> --
>> 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
> 

[PATCH 1/3] dt-bindings: iio: adc: stm32: add support for diff channels

[Fabrice Gasnier]

On 10/21/2017 09:23 PM, Jonathan Cameron wrote:
> On Sat, 21 Oct 2017 18:54:01 +0100
> Jonathan Cameron <jic23@kernel.org> wrote:
> 
>> On Tue, 17 Oct 2017 15:15:43 +0200
>> Fabrice Gasnier <fabrice.gasnier@st.com> wrote:
>>
>>> STM32H7 ADC channels may be configured either as single-ended or
>>> differential.
>>> Add 'st,adc-diff-channels' property to support differential channels.
>>> Differential channels are defined as a pair of positive and negative
>>> inputs: vinp & vinn.
>>>
>>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>  
>> Hmm. Fair enough.  Sometimes we support both types of channels
>> and leave it to userspace, but in many cases that makes little sense
>> - particularly if like I think is going on here, we aren't combining channels
>> that can be separately read but rather the negative pin is simply unused
>> when we are in single channel mode... (did I understand that right?)
> I clearly didn't understand.  This is same as other devices
> in that we are picking a pair from the channels that can otherwise be
> used as single end channels. (I think). 

Hi Jonathan,

Indeed, we're picking a pair from the channels that can be used as
single channels otherwise.
Also, it doesn't makes sense to leave the choice to userspace: when
these channels are used in differential mode, both input should be
biased. This is quite dependent on hardware (board), not really a
runtime choice.
Hope this clarifies ?

> 
> For this binding I think I'd like to see an example added showing how
> this works...

You're right, I should have added that. Please let me know if I can add
bellow example in v2:

Example to setup differential channels 1, 2 & 3 (with resp. 0, 6 & 7
negative inputs):
	adc: adc at 40022000 {
		compatible = "st,stm32h7-adc-core";
		...
		adc1: adc at 0 {
			compatible = "st,stm32h7-adc";
			...
			st,adc-diff-channels = <1 0>, <2 6>, <3 7>;
		};
	};

Thanks for reviewing,
Best Regards,
Fabrice

> 
> Thanks,
> 
> Jonathan
>>
>> Jonathan
>>
>>> ---
>>>  Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt | 6 ++++++
>>>  1 file changed, 6 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
>>> index 48bfcaa3..c3c8de1 100644
>>> --- a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
>>> +++ b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
>>> @@ -62,6 +62,12 @@ Required properties:
>>>  - st,adc-channels: List of single-ended channels muxed for this ADC.
>>>    It can have up to 16 channels on stm32f4 or 20 channels on stm32h7, numbered
>>>    from 0 to 15 or 19 (resp. for in0..in15 or in0..in19).
>>> +- st,adc-diff-channels: List of differential channels muxed for this ADC.
>>> +  Depending on part used, some channels can be configured as differential
>>> +  instead of single-ended (e.g. stm32h7). List here positive and negative
>>> +  inputs pairs as <vinp vinn>, <vinp vinn>,... vinp and vinn are numbered
>>> +  from 0 to 19 on stm32h7)
>>> +  Note: At least one of "st,adc-channels" or "st,adc-diff-channels" is required.
>>>  - #io-channel-cells = <1>: See the IIO bindings section "IIO consumers" in
>>>    Documentation/devicetree/bindings/iio/iio-bindings.txt
>>>    
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>> the body of a message to majordomo at vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 3/3] iio: adc: stm32: add support for differential channels

[Fabrice Gasnier]

On 10/21/2017 09:25 PM, Jonathan Cameron wrote:
> On Tue, 17 Oct 2017 15:15:45 +0200
> Fabrice Gasnier <fabrice.gasnier@st.com> wrote:
> 
>> STM32H7 ADC channels can be configured either as single ended or
>> differential with 'st,adc-channels' or 'st,adc-diff-channels'
>> (positive and negative input pair: <vinp vinn>, ...).
>>
>> Differential channels have different offset and scale, from spec:
>> raw value = (full_scale / 2) * (1 + (vinp - vinn) / vref).
>> Add offset attribute.
>>
>> Differential channels are selected by DIFSEL register. Negative
>> inputs must be added to pre-selected channels as well (PCSEL).
>>
>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> Looks fine to me. Will wait to see what others think on the binding in
> particular.  I suppose that given we allow control over which single ended
> channels are registered, it makes sense to do it for differential channels
> as well.
> 
> Thanks,
> 
> Jonathan
>> ---
>>  drivers/iio/adc/stm32-adc.c | 123 +++++++++++++++++++++++++++++++++++++-------
>>  1 file changed, 103 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
>> index c95e6f7..cc7ca50 100644
>> --- a/drivers/iio/adc/stm32-adc.c
>> +++ b/drivers/iio/adc/stm32-adc.c
>> @@ -92,6 +92,7 @@
>>  #define STM32H7_ADC_SQR3		0x38
>>  #define STM32H7_ADC_SQR4		0x3C
>>  #define STM32H7_ADC_DR			0x40
>> +#define STM32H7_ADC_DIFSEL		0xC0
>>  #define STM32H7_ADC_CALFACT		0xC4
>>  #define STM32H7_ADC_CALFACT2		0xC8
>>  
>> @@ -154,7 +155,7 @@ enum stm32h7_adc_dmngt {
>>  #define STM32H7_BOOST_CLKRATE		20000000UL
>>  
>>  #define STM32_ADC_CH_MAX		20	/* max number of channels */
>> -#define STM32_ADC_CH_SZ			5	/* max channel name size */
>> +#define STM32_ADC_CH_SZ			10	/* max channel name size */
>>  #define STM32_ADC_MAX_SQ		16	/* SQ1..SQ16 */
>>  #define STM32_ADC_MAX_SMP		7	/* SMPx range is [0..7] */
>>  #define STM32_ADC_TIMEOUT_US		100000
>> @@ -299,6 +300,7 @@ struct stm32_adc_cfg {
>>   * @rx_buf:		dma rx buffer cpu address
>>   * @rx_dma_buf:		dma rx buffer bus address
>>   * @rx_buf_sz:		dma rx buffer size
>> + * @difsel		bitmask to set single-ended/differential channel
>>   * @pcsel		bitmask to preselect channels on some devices
>>   * @smpr_val:		sampling time settings (e.g. smpr1 / smpr2)
>>   * @cal:		optional calibration data on some devices
>> @@ -321,12 +323,18 @@ struct stm32_adc {
>>  	u8			*rx_buf;
>>  	dma_addr_t		rx_dma_buf;
>>  	unsigned int		rx_buf_sz;
>> +	u32			difsel;
>>  	u32			pcsel;
>>  	u32			smpr_val[2];
>>  	struct stm32_adc_calib	cal;
>>  	char			chan_name[STM32_ADC_CH_MAX][STM32_ADC_CH_SZ];
>>  };
>>  
>> +struct stm32_adc_diff_channel {
>> +	u32 vinp;
>> +	u32 vinn;
>> +};
>> +
>>  /**
>>   * struct stm32_adc_info - stm32 ADC, per instance config data
>>   * @max_channels:	Number of channels
>> @@ -944,15 +952,19 @@ static int stm32h7_adc_selfcalib(struct stm32_adc *adc)
>>   * stm32h7_adc_prepare() - Leave power down mode to enable ADC.
>>   * @adc: stm32 adc instance
>>   * Leave power down mode.
>> + * Configure channels as single ended or differential before enabling ADC.
>>   * Enable ADC.
>>   * Restore calibration data.
>> - * Pre-select channels that may be used in PCSEL (required by input MUX / IO).
>> + * Pre-select channels that may be used in PCSEL (required by input MUX / IO):
>> + * - Only one input is selected for single ended (e.g. 'vinp')
>> + * - Two inputs are selected for differential channels (e.g. 'vinp' & 'vinn')
>>   */
>>  static int stm32h7_adc_prepare(struct stm32_adc *adc)
>>  {
>>  	int ret;
>>  
>>  	stm32h7_adc_exit_pwr_down(adc);
>> +	stm32_adc_writel(adc, STM32H7_ADC_DIFSEL, adc->difsel);
>>  
>>  	ret = stm32h7_adc_enable(adc);
>>  	if (ret)
>> @@ -1224,10 +1236,23 @@ static int stm32_adc_read_raw(struct iio_dev *indio_dev,
>>  		return ret;
>>  
>>  	case IIO_CHAN_INFO_SCALE:
>> -		*val = adc->common->vref_mv;
>> -		*val2 = chan->scan_type.realbits;
>> +		if (chan->differential) {
>> +			*val = adc->common->vref_mv * 2;
>> +			*val2 = chan->scan_type.realbits;
>> +		} else {
>> +			*val = adc->common->vref_mv;
>> +			*val2 = chan->scan_type.realbits;
>> +		}
>>  		return IIO_VAL_FRACTIONAL_LOG2;
>>  
>> +	case IIO_CHAN_INFO_OFFSET:
>> +		if (chan->differential)
>> +			/* ADC_full_scale / 2 */
>> +			*val = -((1 << chan->scan_type.realbits) / 2);
>> +		else
>> +			*val = 0;
>> +		return IIO_VAL_INT;
>> +
>>  	default:
>>  		return -EINVAL;
>>  	}
>> @@ -1591,29 +1616,39 @@ static void stm32_adc_smpr_init(struct stm32_adc *adc, int channel, u32 smp_ns)
>>  
>>  static void stm32_adc_chan_init_one(struct iio_dev *indio_dev,
>>  				    struct iio_chan_spec *chan, u32 val,
>> -				    int scan_index, u32 smp)
>> +				    u32 val2, int scan_index, bool differential)
> val and val2 are slightly odd names in here.

Hi Jonathan,

You're right. I suppose vinp and vinn would be better names. This would
also better match with reference manual.

I'll update this in v2.

Thanks!
Best Regards,
Fabrice

>>  {
>>  	struct stm32_adc *adc = iio_priv(indio_dev);
>>  	char *name = adc->chan_name[val];
>>  
>>  	chan->type = IIO_VOLTAGE;
>>  	chan->channel = val;
>> -	snprintf(name, STM32_ADC_CH_SZ, "in%d", val);
>> +	if (differential) {
>> +		chan->differential = 1;
>> +		chan->channel2 = val2;
>> +		snprintf(name, STM32_ADC_CH_SZ, "in%d-in%d", val, val2);
>> +	} else {
>> +		snprintf(name, STM32_ADC_CH_SZ, "in%d", val);
>> +	}
>>  	chan->datasheet_name = name;
>>  	chan->scan_index = scan_index;
>>  	chan->indexed = 1;
>>  	chan->info_mask_separate = BIT(IIO_CHAN_INFO_RAW);
>> -	chan->info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE);
>> +	chan->info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |
>> +					 BIT(IIO_CHAN_INFO_OFFSET);
>>  	chan->scan_type.sign = 'u';
>>  	chan->scan_type.realbits = adc->cfg->adc_info->resolutions[adc->res];
>>  	chan->scan_type.storagebits = 16;
>>  	chan->ext_info = stm32_adc_ext_info;
>>  
>> -	/* Prepare sampling time settings */
>> -	stm32_adc_smpr_init(adc, chan->channel, smp);
>> -
>>  	/* pre-build selected channels mask */
>>  	adc->pcsel |= BIT(chan->channel);
>> +	if (differential) {
>> +		/* pre-build diff channels mask */
>> +		adc->difsel |= BIT(chan->channel);
>> +		/* Also add negative input to pre-selected channels */
>> +		adc->pcsel |= BIT(chan->channel2);
>> +	}
>>  }
>>  
>>  static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
>> @@ -1621,17 +1656,40 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
>>  	struct device_node *node = indio_dev->dev.of_node;
>>  	struct stm32_adc *adc = iio_priv(indio_dev);
>>  	const struct stm32_adc_info *adc_info = adc->cfg->adc_info;
>> +	struct stm32_adc_diff_channel diff[STM32_ADC_CH_MAX];
>>  	struct property *prop;
>>  	const __be32 *cur;
>>  	struct iio_chan_spec *channels;
>> -	int scan_index = 0, num_channels, ret;
>> +	int scan_index = 0, num_channels = 0, num_diff = 0, ret, i;
>>  	u32 val, smp = 0;
>>  
>> -	num_channels = of_property_count_u32_elems(node, "st,adc-channels");
>> -	if (num_channels < 0 ||
>> -	    num_channels > adc_info->max_channels) {
>> +	ret = of_property_count_u32_elems(node, "st,adc-channels");
>> +	if (ret > adc_info->max_channels) {
>>  		dev_err(&indio_dev->dev, "Bad st,adc-channels?\n");
>> -		return num_channels < 0 ? num_channels : -EINVAL;
>> +		return -EINVAL;
>> +	} else if (ret > 0) {
>> +		num_channels += ret;
>> +	}
>> +
>> +	ret = of_property_count_elems_of_size(node, "st,adc-diff-channels",
>> +					      sizeof(*diff));
>> +	if (ret > adc_info->max_channels) {
>> +		dev_err(&indio_dev->dev, "Bad st,adc-diff-channels?\n");
>> +		return -EINVAL;
>> +	} else if (ret > 0) {
>> +		int size = ret * sizeof(*diff) / sizeof(u32);
>> +
>> +		num_diff = ret;
>> +		num_channels += ret;
>> +		ret = of_property_read_u32_array(node, "st,adc-diff-channels",
>> +						 (u32 *)diff, size);
>> +		if (ret)
>> +			return ret;
>> +	}
>> +
>> +	if (!num_channels) {
>> +		dev_err(&indio_dev->dev, "No channels configured\n");
>> +		return -ENODATA;
>>  	}
>>  
>>  	/* Optional sample time is provided either for each, or all channels */
>> @@ -1652,6 +1710,33 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
>>  			return -EINVAL;
>>  		}
>>  
>> +		/* Channel can't be configured both as single-ended & diff */
>> +		for (i = 0; i < num_diff; i++) {
>> +			if (val == diff[i].vinp) {
>> +				dev_err(&indio_dev->dev,
>> +					"channel %d miss-configured\n",	val);
>> +				return -EINVAL;
>> +			}
>> +		}
>> +		stm32_adc_chan_init_one(indio_dev, &channels[scan_index], val,
>> +					0, scan_index, false);
>> +		scan_index++;
>> +	}
>> +
>> +	for (i = 0; i < num_diff; i++) {
>> +		if (diff[i].vinp >= adc_info->max_channels ||
>> +		    diff[i].vinn >= adc_info->max_channels) {
>> +			dev_err(&indio_dev->dev, "Invalid channel in%d-in%d\n",
>> +				diff[i].vinp, diff[i].vinn);
>> +			return -EINVAL;
>> +		}
>> +		stm32_adc_chan_init_one(indio_dev, &channels[scan_index],
>> +					diff[i].vinp, diff[i].vinn, scan_index,
>> +					true);
>> +		scan_index++;
>> +	}
>> +
>> +	for (i = 0; i < scan_index; i++) {
>>  		/*
>>  		 * Using of_property_read_u32_index(), smp value will only be
>>  		 * modified if valid u32 value can be decoded. This allows to
>> @@ -1659,11 +1744,9 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
>>  		 * value per channel.
>>  		 */
>>  		of_property_read_u32_index(node, "st,min-sample-time-nsecs",
>> -					   scan_index, &smp);
>> -
>> -		stm32_adc_chan_init_one(indio_dev, &channels[scan_index],
>> -					val, scan_index, smp);
>> -		scan_index++;
>> +					   i, &smp);
>> +		/* Prepare sampling time settings */
>> +		stm32_adc_smpr_init(adc, channels[i].channel, smp);
>>  	}
>>  
>>  	indio_dev->num_channels = scan_index;
> 

Re: [PATCH 3/3] iio: adc: stm32: add support for differential channels

[Fabrice Gasnier]

On 10/21/2017 09:25 PM, Jonathan Cameron wrote:
> On Tue, 17 Oct 2017 15:15:45 +0200
> Fabrice Gasnier <fabrice.gasnier@st.com> wrote:
> 
>> STM32H7 ADC channels can be configured either as single ended or
>> differential with 'st,adc-channels' or 'st,adc-diff-channels'
>> (positive and negative input pair: <vinp vinn>, ...).
>>
>> Differential channels have different offset and scale, from spec:
>> raw value = (full_scale / 2) * (1 + (vinp - vinn) / vref).
>> Add offset attribute.
>>
>> Differential channels are selected by DIFSEL register. Negative
>> inputs must be added to pre-selected channels as well (PCSEL).
>>
>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> Looks fine to me. Will wait to see what others think on the binding in
> particular.  I suppose that given we allow control over which single ended
> channels are registered, it makes sense to do it for differential channels
> as well.
> 
> Thanks,
> 
> Jonathan
>> ---
>>  drivers/iio/adc/stm32-adc.c | 123 +++++++++++++++++++++++++++++++++++++-------
>>  1 file changed, 103 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
>> index c95e6f7..cc7ca50 100644
>> --- a/drivers/iio/adc/stm32-adc.c
>> +++ b/drivers/iio/adc/stm32-adc.c
>> @@ -92,6 +92,7 @@
>>  #define STM32H7_ADC_SQR3		0x38
>>  #define STM32H7_ADC_SQR4		0x3C
>>  #define STM32H7_ADC_DR			0x40
>> +#define STM32H7_ADC_DIFSEL		0xC0
>>  #define STM32H7_ADC_CALFACT		0xC4
>>  #define STM32H7_ADC_CALFACT2		0xC8
>>  
>> @@ -154,7 +155,7 @@ enum stm32h7_adc_dmngt {
>>  #define STM32H7_BOOST_CLKRATE		20000000UL
>>  
>>  #define STM32_ADC_CH_MAX		20	/* max number of channels */
>> -#define STM32_ADC_CH_SZ			5	/* max channel name size */
>> +#define STM32_ADC_CH_SZ			10	/* max channel name size */
>>  #define STM32_ADC_MAX_SQ		16	/* SQ1..SQ16 */
>>  #define STM32_ADC_MAX_SMP		7	/* SMPx range is [0..7] */
>>  #define STM32_ADC_TIMEOUT_US		100000
>> @@ -299,6 +300,7 @@ struct stm32_adc_cfg {
>>   * @rx_buf:		dma rx buffer cpu address
>>   * @rx_dma_buf:		dma rx buffer bus address
>>   * @rx_buf_sz:		dma rx buffer size
>> + * @difsel		bitmask to set single-ended/differential channel
>>   * @pcsel		bitmask to preselect channels on some devices
>>   * @smpr_val:		sampling time settings (e.g. smpr1 / smpr2)
>>   * @cal:		optional calibration data on some devices
>> @@ -321,12 +323,18 @@ struct stm32_adc {
>>  	u8			*rx_buf;
>>  	dma_addr_t		rx_dma_buf;
>>  	unsigned int		rx_buf_sz;
>> +	u32			difsel;
>>  	u32			pcsel;
>>  	u32			smpr_val[2];
>>  	struct stm32_adc_calib	cal;
>>  	char			chan_name[STM32_ADC_CH_MAX][STM32_ADC_CH_SZ];
>>  };
>>  
>> +struct stm32_adc_diff_channel {
>> +	u32 vinp;
>> +	u32 vinn;
>> +};
>> +
>>  /**
>>   * struct stm32_adc_info - stm32 ADC, per instance config data
>>   * @max_channels:	Number of channels
>> @@ -944,15 +952,19 @@ static int stm32h7_adc_selfcalib(struct stm32_adc *adc)
>>   * stm32h7_adc_prepare() - Leave power down mode to enable ADC.
>>   * @adc: stm32 adc instance
>>   * Leave power down mode.
>> + * Configure channels as single ended or differential before enabling ADC.
>>   * Enable ADC.
>>   * Restore calibration data.
>> - * Pre-select channels that may be used in PCSEL (required by input MUX / IO).
>> + * Pre-select channels that may be used in PCSEL (required by input MUX / IO):
>> + * - Only one input is selected for single ended (e.g. 'vinp')
>> + * - Two inputs are selected for differential channels (e.g. 'vinp' & 'vinn')
>>   */
>>  static int stm32h7_adc_prepare(struct stm32_adc *adc)
>>  {
>>  	int ret;
>>  
>>  	stm32h7_adc_exit_pwr_down(adc);
>> +	stm32_adc_writel(adc, STM32H7_ADC_DIFSEL, adc->difsel);
>>  
>>  	ret = stm32h7_adc_enable(adc);
>>  	if (ret)
>> @@ -1224,10 +1236,23 @@ static int stm32_adc_read_raw(struct iio_dev *indio_dev,
>>  		return ret;
>>  
>>  	case IIO_CHAN_INFO_SCALE:
>> -		*val = adc->common->vref_mv;
>> -		*val2 = chan->scan_type.realbits;
>> +		if (chan->differential) {
>> +			*val = adc->common->vref_mv * 2;
>> +			*val2 = chan->scan_type.realbits;
>> +		} else {
>> +			*val = adc->common->vref_mv;
>> +			*val2 = chan->scan_type.realbits;
>> +		}
>>  		return IIO_VAL_FRACTIONAL_LOG2;
>>  
>> +	case IIO_CHAN_INFO_OFFSET:
>> +		if (chan->differential)
>> +			/* ADC_full_scale / 2 */
>> +			*val = -((1 << chan->scan_type.realbits) / 2);
>> +		else
>> +			*val = 0;
>> +		return IIO_VAL_INT;
>> +
>>  	default:
>>  		return -EINVAL;
>>  	}
>> @@ -1591,29 +1616,39 @@ static void stm32_adc_smpr_init(struct stm32_adc *adc, int channel, u32 smp_ns)
>>  
>>  static void stm32_adc_chan_init_one(struct iio_dev *indio_dev,
>>  				    struct iio_chan_spec *chan, u32 val,
>> -				    int scan_index, u32 smp)
>> +				    u32 val2, int scan_index, bool differential)
> val and val2 are slightly odd names in here.

Hi Jonathan,

You're right. I suppose vinp and vinn would be better names. This would
also better match with reference manual.

I'll update this in v2.

Thanks!
Best Regards,
Fabrice

>>  {
>>  	struct stm32_adc *adc = iio_priv(indio_dev);
>>  	char *name = adc->chan_name[val];
>>  
>>  	chan->type = IIO_VOLTAGE;
>>  	chan->channel = val;
>> -	snprintf(name, STM32_ADC_CH_SZ, "in%d", val);
>> +	if (differential) {
>> +		chan->differential = 1;
>> +		chan->channel2 = val2;
>> +		snprintf(name, STM32_ADC_CH_SZ, "in%d-in%d", val, val2);
>> +	} else {
>> +		snprintf(name, STM32_ADC_CH_SZ, "in%d", val);
>> +	}
>>  	chan->datasheet_name = name;
>>  	chan->scan_index = scan_index;
>>  	chan->indexed = 1;
>>  	chan->info_mask_separate = BIT(IIO_CHAN_INFO_RAW);
>> -	chan->info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE);
>> +	chan->info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |
>> +					 BIT(IIO_CHAN_INFO_OFFSET);
>>  	chan->scan_type.sign = 'u';
>>  	chan->scan_type.realbits = adc->cfg->adc_info->resolutions[adc->res];
>>  	chan->scan_type.storagebits = 16;
>>  	chan->ext_info = stm32_adc_ext_info;
>>  
>> -	/* Prepare sampling time settings */
>> -	stm32_adc_smpr_init(adc, chan->channel, smp);
>> -
>>  	/* pre-build selected channels mask */
>>  	adc->pcsel |= BIT(chan->channel);
>> +	if (differential) {
>> +		/* pre-build diff channels mask */
>> +		adc->difsel |= BIT(chan->channel);
>> +		/* Also add negative input to pre-selected channels */
>> +		adc->pcsel |= BIT(chan->channel2);
>> +	}
>>  }
>>  
>>  static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
>> @@ -1621,17 +1656,40 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
>>  	struct device_node *node = indio_dev->dev.of_node;
>>  	struct stm32_adc *adc = iio_priv(indio_dev);
>>  	const struct stm32_adc_info *adc_info = adc->cfg->adc_info;
>> +	struct stm32_adc_diff_channel diff[STM32_ADC_CH_MAX];
>>  	struct property *prop;
>>  	const __be32 *cur;
>>  	struct iio_chan_spec *channels;
>> -	int scan_index = 0, num_channels, ret;
>> +	int scan_index = 0, num_channels = 0, num_diff = 0, ret, i;
>>  	u32 val, smp = 0;
>>  
>> -	num_channels = of_property_count_u32_elems(node, "st,adc-channels");
>> -	if (num_channels < 0 ||
>> -	    num_channels > adc_info->max_channels) {
>> +	ret = of_property_count_u32_elems(node, "st,adc-channels");
>> +	if (ret > adc_info->max_channels) {
>>  		dev_err(&indio_dev->dev, "Bad st,adc-channels?\n");
>> -		return num_channels < 0 ? num_channels : -EINVAL;
>> +		return -EINVAL;
>> +	} else if (ret > 0) {
>> +		num_channels += ret;
>> +	}
>> +
>> +	ret = of_property_count_elems_of_size(node, "st,adc-diff-channels",
>> +					      sizeof(*diff));
>> +	if (ret > adc_info->max_channels) {
>> +		dev_err(&indio_dev->dev, "Bad st,adc-diff-channels?\n");
>> +		return -EINVAL;
>> +	} else if (ret > 0) {
>> +		int size = ret * sizeof(*diff) / sizeof(u32);
>> +
>> +		num_diff = ret;
>> +		num_channels += ret;
>> +		ret = of_property_read_u32_array(node, "st,adc-diff-channels",
>> +						 (u32 *)diff, size);
>> +		if (ret)
>> +			return ret;
>> +	}
>> +
>> +	if (!num_channels) {
>> +		dev_err(&indio_dev->dev, "No channels configured\n");
>> +		return -ENODATA;
>>  	}
>>  
>>  	/* Optional sample time is provided either for each, or all channels */
>> @@ -1652,6 +1710,33 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
>>  			return -EINVAL;
>>  		}
>>  
>> +		/* Channel can't be configured both as single-ended & diff */
>> +		for (i = 0; i < num_diff; i++) {
>> +			if (val == diff[i].vinp) {
>> +				dev_err(&indio_dev->dev,
>> +					"channel %d miss-configured\n",	val);
>> +				return -EINVAL;
>> +			}
>> +		}
>> +		stm32_adc_chan_init_one(indio_dev, &channels[scan_index], val,
>> +					0, scan_index, false);
>> +		scan_index++;
>> +	}
>> +
>> +	for (i = 0; i < num_diff; i++) {
>> +		if (diff[i].vinp >= adc_info->max_channels ||
>> +		    diff[i].vinn >= adc_info->max_channels) {
>> +			dev_err(&indio_dev->dev, "Invalid channel in%d-in%d\n",
>> +				diff[i].vinp, diff[i].vinn);
>> +			return -EINVAL;
>> +		}
>> +		stm32_adc_chan_init_one(indio_dev, &channels[scan_index],
>> +					diff[i].vinp, diff[i].vinn, scan_index,
>> +					true);
>> +		scan_index++;
>> +	}
>> +
>> +	for (i = 0; i < scan_index; i++) {
>>  		/*
>>  		 * Using of_property_read_u32_index(), smp value will only be
>>  		 * modified if valid u32 value can be decoded. This allows to
>> @@ -1659,11 +1744,9 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
>>  		 * value per channel.
>>  		 */
>>  		of_property_read_u32_index(node, "st,min-sample-time-nsecs",
>> -					   scan_index, &smp);
>> -
>> -		stm32_adc_chan_init_one(indio_dev, &channels[scan_index],
>> -					val, scan_index, smp);
>> -		scan_index++;
>> +					   i, &smp);
>> +		/* Prepare sampling time settings */
>> +		stm32_adc_smpr_init(adc, channels[i].channel, smp);
>>  	}
>>  
>>  	indio_dev->num_channels = scan_index;
> 

[PATCH 3/3] iio: adc: stm32: add support for differential channels

[Fabrice Gasnier]

On 10/21/2017 09:25 PM, Jonathan Cameron wrote:
> On Tue, 17 Oct 2017 15:15:45 +0200
> Fabrice Gasnier <fabrice.gasnier@st.com> wrote:
> 
>> STM32H7 ADC channels can be configured either as single ended or
>> differential with 'st,adc-channels' or 'st,adc-diff-channels'
>> (positive and negative input pair: <vinp vinn>, ...).
>>
>> Differential channels have different offset and scale, from spec:
>> raw value = (full_scale / 2) * (1 + (vinp - vinn) / vref).
>> Add offset attribute.
>>
>> Differential channels are selected by DIFSEL register. Negative
>> inputs must be added to pre-selected channels as well (PCSEL).
>>
>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> Looks fine to me. Will wait to see what others think on the binding in
> particular.  I suppose that given we allow control over which single ended
> channels are registered, it makes sense to do it for differential channels
> as well.
> 
> Thanks,
> 
> Jonathan
>> ---
>>  drivers/iio/adc/stm32-adc.c | 123 +++++++++++++++++++++++++++++++++++++-------
>>  1 file changed, 103 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
>> index c95e6f7..cc7ca50 100644
>> --- a/drivers/iio/adc/stm32-adc.c
>> +++ b/drivers/iio/adc/stm32-adc.c
>> @@ -92,6 +92,7 @@
>>  #define STM32H7_ADC_SQR3		0x38
>>  #define STM32H7_ADC_SQR4		0x3C
>>  #define STM32H7_ADC_DR			0x40
>> +#define STM32H7_ADC_DIFSEL		0xC0
>>  #define STM32H7_ADC_CALFACT		0xC4
>>  #define STM32H7_ADC_CALFACT2		0xC8
>>  
>> @@ -154,7 +155,7 @@ enum stm32h7_adc_dmngt {
>>  #define STM32H7_BOOST_CLKRATE		20000000UL
>>  
>>  #define STM32_ADC_CH_MAX		20	/* max number of channels */
>> -#define STM32_ADC_CH_SZ			5	/* max channel name size */
>> +#define STM32_ADC_CH_SZ			10	/* max channel name size */
>>  #define STM32_ADC_MAX_SQ		16	/* SQ1..SQ16 */
>>  #define STM32_ADC_MAX_SMP		7	/* SMPx range is [0..7] */
>>  #define STM32_ADC_TIMEOUT_US		100000
>> @@ -299,6 +300,7 @@ struct stm32_adc_cfg {
>>   * @rx_buf:		dma rx buffer cpu address
>>   * @rx_dma_buf:		dma rx buffer bus address
>>   * @rx_buf_sz:		dma rx buffer size
>> + * @difsel		bitmask to set single-ended/differential channel
>>   * @pcsel		bitmask to preselect channels on some devices
>>   * @smpr_val:		sampling time settings (e.g. smpr1 / smpr2)
>>   * @cal:		optional calibration data on some devices
>> @@ -321,12 +323,18 @@ struct stm32_adc {
>>  	u8			*rx_buf;
>>  	dma_addr_t		rx_dma_buf;
>>  	unsigned int		rx_buf_sz;
>> +	u32			difsel;
>>  	u32			pcsel;
>>  	u32			smpr_val[2];
>>  	struct stm32_adc_calib	cal;
>>  	char			chan_name[STM32_ADC_CH_MAX][STM32_ADC_CH_SZ];
>>  };
>>  
>> +struct stm32_adc_diff_channel {
>> +	u32 vinp;
>> +	u32 vinn;
>> +};
>> +
>>  /**
>>   * struct stm32_adc_info - stm32 ADC, per instance config data
>>   * @max_channels:	Number of channels
>> @@ -944,15 +952,19 @@ static int stm32h7_adc_selfcalib(struct stm32_adc *adc)
>>   * stm32h7_adc_prepare() - Leave power down mode to enable ADC.
>>   * @adc: stm32 adc instance
>>   * Leave power down mode.
>> + * Configure channels as single ended or differential before enabling ADC.
>>   * Enable ADC.
>>   * Restore calibration data.
>> - * Pre-select channels that may be used in PCSEL (required by input MUX / IO).
>> + * Pre-select channels that may be used in PCSEL (required by input MUX / IO):
>> + * - Only one input is selected for single ended (e.g. 'vinp')
>> + * - Two inputs are selected for differential channels (e.g. 'vinp' & 'vinn')
>>   */
>>  static int stm32h7_adc_prepare(struct stm32_adc *adc)
>>  {
>>  	int ret;
>>  
>>  	stm32h7_adc_exit_pwr_down(adc);
>> +	stm32_adc_writel(adc, STM32H7_ADC_DIFSEL, adc->difsel);
>>  
>>  	ret = stm32h7_adc_enable(adc);
>>  	if (ret)
>> @@ -1224,10 +1236,23 @@ static int stm32_adc_read_raw(struct iio_dev *indio_dev,
>>  		return ret;
>>  
>>  	case IIO_CHAN_INFO_SCALE:
>> -		*val = adc->common->vref_mv;
>> -		*val2 = chan->scan_type.realbits;
>> +		if (chan->differential) {
>> +			*val = adc->common->vref_mv * 2;
>> +			*val2 = chan->scan_type.realbits;
>> +		} else {
>> +			*val = adc->common->vref_mv;
>> +			*val2 = chan->scan_type.realbits;
>> +		}
>>  		return IIO_VAL_FRACTIONAL_LOG2;
>>  
>> +	case IIO_CHAN_INFO_OFFSET:
>> +		if (chan->differential)
>> +			/* ADC_full_scale / 2 */
>> +			*val = -((1 << chan->scan_type.realbits) / 2);
>> +		else
>> +			*val = 0;
>> +		return IIO_VAL_INT;
>> +
>>  	default:
>>  		return -EINVAL;
>>  	}
>> @@ -1591,29 +1616,39 @@ static void stm32_adc_smpr_init(struct stm32_adc *adc, int channel, u32 smp_ns)
>>  
>>  static void stm32_adc_chan_init_one(struct iio_dev *indio_dev,
>>  				    struct iio_chan_spec *chan, u32 val,
>> -				    int scan_index, u32 smp)
>> +				    u32 val2, int scan_index, bool differential)
> val and val2 are slightly odd names in here.

Hi Jonathan,

You're right. I suppose vinp and vinn would be better names. This would
also better match with reference manual.

I'll update this in v2.

Thanks!
Best Regards,
Fabrice

>>  {
>>  	struct stm32_adc *adc = iio_priv(indio_dev);
>>  	char *name = adc->chan_name[val];
>>  
>>  	chan->type = IIO_VOLTAGE;
>>  	chan->channel = val;
>> -	snprintf(name, STM32_ADC_CH_SZ, "in%d", val);
>> +	if (differential) {
>> +		chan->differential = 1;
>> +		chan->channel2 = val2;
>> +		snprintf(name, STM32_ADC_CH_SZ, "in%d-in%d", val, val2);
>> +	} else {
>> +		snprintf(name, STM32_ADC_CH_SZ, "in%d", val);
>> +	}
>>  	chan->datasheet_name = name;
>>  	chan->scan_index = scan_index;
>>  	chan->indexed = 1;
>>  	chan->info_mask_separate = BIT(IIO_CHAN_INFO_RAW);
>> -	chan->info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE);
>> +	chan->info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |
>> +					 BIT(IIO_CHAN_INFO_OFFSET);
>>  	chan->scan_type.sign = 'u';
>>  	chan->scan_type.realbits = adc->cfg->adc_info->resolutions[adc->res];
>>  	chan->scan_type.storagebits = 16;
>>  	chan->ext_info = stm32_adc_ext_info;
>>  
>> -	/* Prepare sampling time settings */
>> -	stm32_adc_smpr_init(adc, chan->channel, smp);
>> -
>>  	/* pre-build selected channels mask */
>>  	adc->pcsel |= BIT(chan->channel);
>> +	if (differential) {
>> +		/* pre-build diff channels mask */
>> +		adc->difsel |= BIT(chan->channel);
>> +		/* Also add negative input to pre-selected channels */
>> +		adc->pcsel |= BIT(chan->channel2);
>> +	}
>>  }
>>  
>>  static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
>> @@ -1621,17 +1656,40 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
>>  	struct device_node *node = indio_dev->dev.of_node;
>>  	struct stm32_adc *adc = iio_priv(indio_dev);
>>  	const struct stm32_adc_info *adc_info = adc->cfg->adc_info;
>> +	struct stm32_adc_diff_channel diff[STM32_ADC_CH_MAX];
>>  	struct property *prop;
>>  	const __be32 *cur;
>>  	struct iio_chan_spec *channels;
>> -	int scan_index = 0, num_channels, ret;
>> +	int scan_index = 0, num_channels = 0, num_diff = 0, ret, i;
>>  	u32 val, smp = 0;
>>  
>> -	num_channels = of_property_count_u32_elems(node, "st,adc-channels");
>> -	if (num_channels < 0 ||
>> -	    num_channels > adc_info->max_channels) {
>> +	ret = of_property_count_u32_elems(node, "st,adc-channels");
>> +	if (ret > adc_info->max_channels) {
>>  		dev_err(&indio_dev->dev, "Bad st,adc-channels?\n");
>> -		return num_channels < 0 ? num_channels : -EINVAL;
>> +		return -EINVAL;
>> +	} else if (ret > 0) {
>> +		num_channels += ret;
>> +	}
>> +
>> +	ret = of_property_count_elems_of_size(node, "st,adc-diff-channels",
>> +					      sizeof(*diff));
>> +	if (ret > adc_info->max_channels) {
>> +		dev_err(&indio_dev->dev, "Bad st,adc-diff-channels?\n");
>> +		return -EINVAL;
>> +	} else if (ret > 0) {
>> +		int size = ret * sizeof(*diff) / sizeof(u32);
>> +
>> +		num_diff = ret;
>> +		num_channels += ret;
>> +		ret = of_property_read_u32_array(node, "st,adc-diff-channels",
>> +						 (u32 *)diff, size);
>> +		if (ret)
>> +			return ret;
>> +	}
>> +
>> +	if (!num_channels) {
>> +		dev_err(&indio_dev->dev, "No channels configured\n");
>> +		return -ENODATA;
>>  	}
>>  
>>  	/* Optional sample time is provided either for each, or all channels */
>> @@ -1652,6 +1710,33 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
>>  			return -EINVAL;
>>  		}
>>  
>> +		/* Channel can't be configured both as single-ended & diff */
>> +		for (i = 0; i < num_diff; i++) {
>> +			if (val == diff[i].vinp) {
>> +				dev_err(&indio_dev->dev,
>> +					"channel %d miss-configured\n",	val);
>> +				return -EINVAL;
>> +			}
>> +		}
>> +		stm32_adc_chan_init_one(indio_dev, &channels[scan_index], val,
>> +					0, scan_index, false);
>> +		scan_index++;
>> +	}
>> +
>> +	for (i = 0; i < num_diff; i++) {
>> +		if (diff[i].vinp >= adc_info->max_channels ||
>> +		    diff[i].vinn >= adc_info->max_channels) {
>> +			dev_err(&indio_dev->dev, "Invalid channel in%d-in%d\n",
>> +				diff[i].vinp, diff[i].vinn);
>> +			return -EINVAL;
>> +		}
>> +		stm32_adc_chan_init_one(indio_dev, &channels[scan_index],
>> +					diff[i].vinp, diff[i].vinn, scan_index,
>> +					true);
>> +		scan_index++;
>> +	}
>> +
>> +	for (i = 0; i < scan_index; i++) {
>>  		/*
>>  		 * Using of_property_read_u32_index(), smp value will only be
>>  		 * modified if valid u32 value can be decoded. This allows to
>> @@ -1659,11 +1744,9 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
>>  		 * value per channel.
>>  		 */
>>  		of_property_read_u32_index(node, "st,min-sample-time-nsecs",
>> -					   scan_index, &smp);
>> -
>> -		stm32_adc_chan_init_one(indio_dev, &channels[scan_index],
>> -					val, scan_index, smp);
>> -		scan_index++;
>> +					   i, &smp);
>> +		/* Prepare sampling time settings */
>> +		stm32_adc_smpr_init(adc, channels[i].channel, smp);
>>  	}
>>  
>>  	indio_dev->num_channels = scan_index;
> 

Re: [PATCH 1/3] dt-bindings: iio: adc: stm32: add support for diff channels

[Jonathan Cameron]

On Mon, 23 Oct 2017 10:06:27 +0200
Fabrice Gasnier <fabrice.gasnier@st.com> wrote:

> On 10/21/2017 09:23 PM, Jonathan Cameron wrote:
> > On Sat, 21 Oct 2017 18:54:01 +0100
> > Jonathan Cameron <jic23@kernel.org> wrote:
> >   
> >> On Tue, 17 Oct 2017 15:15:43 +0200
> >> Fabrice Gasnier <fabrice.gasnier@st.com> wrote:
> >>  
> >>> STM32H7 ADC channels may be configured either as single-ended or
> >>> differential.
> >>> Add 'st,adc-diff-channels' property to support differential channels.
> >>> Differential channels are defined as a pair of positive and negative
> >>> inputs: vinp & vinn.
> >>>
> >>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>    
> >> Hmm. Fair enough.  Sometimes we support both types of channels
> >> and leave it to userspace, but in many cases that makes little sense
> >> - particularly if like I think is going on here, we aren't combining channels
> >> that can be separately read but rather the negative pin is simply unused
> >> when we are in single channel mode... (did I understand that right?)  
> > I clearly didn't understand.  This is same as other devices
> > in that we are picking a pair from the channels that can otherwise be
> > used as single end channels. (I think).   
> 
> Hi Jonathan,
> 
> Indeed, we're picking a pair from the channels that can be used as
> single channels otherwise.
> Also, it doesn't makes sense to leave the choice to userspace: when
> these channels are used in differential mode, both input should be
> biased. This is quite dependent on hardware (board), not really a
> runtime choice.
> Hope this clarifies ?
> 
> > 
> > For this binding I think I'd like to see an example added showing how
> > this works...  
> 
> You're right, I should have added that. Please let me know if I can add
> bellow example in v2:
Looks good to me.

Jonathan
> 
> Example to setup differential channels 1, 2 & 3 (with resp. 0, 6 & 7
> negative inputs):
> 	adc: adc@40022000 {
> 		compatible = "st,stm32h7-adc-core";
> 		...
> 		adc1: adc@0 {
> 			compatible = "st,stm32h7-adc";
> 			...
> 			st,adc-diff-channels = <1 0>, <2 6>, <3 7>;
> 		};
> 	};
> 
> Thanks for reviewing,
> Best Regards,
> Fabrice
> 
> > 
> > Thanks,
> > 
> > Jonathan  
> >>
> >> Jonathan
> >>  
> >>> ---
> >>>  Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt | 6 ++++++
> >>>  1 file changed, 6 insertions(+)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
> >>> index 48bfcaa3..c3c8de1 100644
> >>> --- a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
> >>> +++ b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
> >>> @@ -62,6 +62,12 @@ Required properties:
> >>>  - st,adc-channels: List of single-ended channels muxed for this ADC.
> >>>    It can have up to 16 channels on stm32f4 or 20 channels on stm32h7, numbered
> >>>    from 0 to 15 or 19 (resp. for in0..in15 or in0..in19).
> >>> +- st,adc-diff-channels: List of differential channels muxed for this ADC.
> >>> +  Depending on part used, some channels can be configured as differential
> >>> +  instead of single-ended (e.g. stm32h7). List here positive and negative
> >>> +  inputs pairs as <vinp vinn>, <vinp vinn>,... vinp and vinn are numbered
> >>> +  from 0 to 19 on stm32h7)
> >>> +  Note: At least one of "st,adc-channels" or "st,adc-diff-channels" is required.
> >>>  - #io-channel-cells = <1>: See the IIO bindings section "IIO consumers" in
> >>>    Documentation/devicetree/bindings/iio/iio-bindings.txt
> >>>      
> >>
> >> --
> >> 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  
> >   
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/3] dt-bindings: iio: adc: stm32: add support for diff channels

[Jonathan Cameron]

On Mon, 23 Oct 2017 10:06:27 +0200
Fabrice Gasnier <fabrice.gasnier-qxv4g6HH51o@public.gmane.org> wrote:

> On 10/21/2017 09:23 PM, Jonathan Cameron wrote:
> > On Sat, 21 Oct 2017 18:54:01 +0100
> > Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> >   
> >> On Tue, 17 Oct 2017 15:15:43 +0200
> >> Fabrice Gasnier <fabrice.gasnier-qxv4g6HH51o@public.gmane.org> wrote:
> >>  
> >>> STM32H7 ADC channels may be configured either as single-ended or
> >>> differential.
> >>> Add 'st,adc-diff-channels' property to support differential channels.
> >>> Differential channels are defined as a pair of positive and negative
> >>> inputs: vinp & vinn.
> >>>
> >>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier-qxv4g6HH51o@public.gmane.org>    
> >> Hmm. Fair enough.  Sometimes we support both types of channels
> >> and leave it to userspace, but in many cases that makes little sense
> >> - particularly if like I think is going on here, we aren't combining channels
> >> that can be separately read but rather the negative pin is simply unused
> >> when we are in single channel mode... (did I understand that right?)  
> > I clearly didn't understand.  This is same as other devices
> > in that we are picking a pair from the channels that can otherwise be
> > used as single end channels. (I think).   
> 
> Hi Jonathan,
> 
> Indeed, we're picking a pair from the channels that can be used as
> single channels otherwise.
> Also, it doesn't makes sense to leave the choice to userspace: when
> these channels are used in differential mode, both input should be
> biased. This is quite dependent on hardware (board), not really a
> runtime choice.
> Hope this clarifies ?
> 
> > 
> > For this binding I think I'd like to see an example added showing how
> > this works...  
> 
> You're right, I should have added that. Please let me know if I can add
> bellow example in v2:
Looks good to me.

Jonathan
> 
> Example to setup differential channels 1, 2 & 3 (with resp. 0, 6 & 7
> negative inputs):
> 	adc: adc@40022000 {
> 		compatible = "st,stm32h7-adc-core";
> 		...
> 		adc1: adc@0 {
> 			compatible = "st,stm32h7-adc";
> 			...
> 			st,adc-diff-channels = <1 0>, <2 6>, <3 7>;
> 		};
> 	};
> 
> Thanks for reviewing,
> Best Regards,
> Fabrice
> 
> > 
> > Thanks,
> > 
> > Jonathan  
> >>
> >> Jonathan
> >>  
> >>> ---
> >>>  Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt | 6 ++++++
> >>>  1 file changed, 6 insertions(+)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
> >>> index 48bfcaa3..c3c8de1 100644
> >>> --- a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
> >>> +++ b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
> >>> @@ -62,6 +62,12 @@ Required properties:
> >>>  - st,adc-channels: List of single-ended channels muxed for this ADC.
> >>>    It can have up to 16 channels on stm32f4 or 20 channels on stm32h7, numbered
> >>>    from 0 to 15 or 19 (resp. for in0..in15 or in0..in19).
> >>> +- st,adc-diff-channels: List of differential channels muxed for this ADC.
> >>> +  Depending on part used, some channels can be configured as differential
> >>> +  instead of single-ended (e.g. stm32h7). List here positive and negative
> >>> +  inputs pairs as <vinp vinn>, <vinp vinn>,... vinp and vinn are numbered
> >>> +  from 0 to 19 on stm32h7)
> >>> +  Note: At least one of "st,adc-channels" or "st,adc-diff-channels" is required.
> >>>  - #io-channel-cells = <1>: See the IIO bindings section "IIO consumers" in
> >>>    Documentation/devicetree/bindings/iio/iio-bindings.txt
> >>>      
> >>
> >> --
> >> 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  
> >   
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/3] dt-bindings: iio: adc: stm32: add support for diff channels

[Jonathan Cameron]

On Mon, 23 Oct 2017 10:06:27 +0200
Fabrice Gasnier <fabrice.gasnier@st.com> wrote:

> On 10/21/2017 09:23 PM, Jonathan Cameron wrote:
> > On Sat, 21 Oct 2017 18:54:01 +0100
> > Jonathan Cameron <jic23@kernel.org> wrote:
> >   
> >> On Tue, 17 Oct 2017 15:15:43 +0200
> >> Fabrice Gasnier <fabrice.gasnier@st.com> wrote:
> >>  
> >>> STM32H7 ADC channels may be configured either as single-ended or
> >>> differential.
> >>> Add 'st,adc-diff-channels' property to support differential channels.
> >>> Differential channels are defined as a pair of positive and negative
> >>> inputs: vinp & vinn.
> >>>
> >>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>    
> >> Hmm. Fair enough.  Sometimes we support both types of channels
> >> and leave it to userspace, but in many cases that makes little sense
> >> - particularly if like I think is going on here, we aren't combining channels
> >> that can be separately read but rather the negative pin is simply unused
> >> when we are in single channel mode... (did I understand that right?)  
> > I clearly didn't understand.  This is same as other devices
> > in that we are picking a pair from the channels that can otherwise be
> > used as single end channels. (I think).   
> 
> Hi Jonathan,
> 
> Indeed, we're picking a pair from the channels that can be used as
> single channels otherwise.
> Also, it doesn't makes sense to leave the choice to userspace: when
> these channels are used in differential mode, both input should be
> biased. This is quite dependent on hardware (board), not really a
> runtime choice.
> Hope this clarifies ?
> 
> > 
> > For this binding I think I'd like to see an example added showing how
> > this works...  
> 
> You're right, I should have added that. Please let me know if I can add
> bellow example in v2:
Looks good to me.

Jonathan
> 
> Example to setup differential channels 1, 2 & 3 (with resp. 0, 6 & 7
> negative inputs):
> 	adc: adc at 40022000 {
> 		compatible = "st,stm32h7-adc-core";
> 		...
> 		adc1: adc at 0 {
> 			compatible = "st,stm32h7-adc";
> 			...
> 			st,adc-diff-channels = <1 0>, <2 6>, <3 7>;
> 		};
> 	};
> 
> Thanks for reviewing,
> Best Regards,
> Fabrice
> 
> > 
> > Thanks,
> > 
> > Jonathan  
> >>
> >> Jonathan
> >>  
> >>> ---
> >>>  Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt | 6 ++++++
> >>>  1 file changed, 6 insertions(+)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
> >>> index 48bfcaa3..c3c8de1 100644
> >>> --- a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
> >>> +++ b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
> >>> @@ -62,6 +62,12 @@ Required properties:
> >>>  - st,adc-channels: List of single-ended channels muxed for this ADC.
> >>>    It can have up to 16 channels on stm32f4 or 20 channels on stm32h7, numbered
> >>>    from 0 to 15 or 19 (resp. for in0..in15 or in0..in19).
> >>> +- st,adc-diff-channels: List of differential channels muxed for this ADC.
> >>> +  Depending on part used, some channels can be configured as differential
> >>> +  instead of single-ended (e.g. stm32h7). List here positive and negative
> >>> +  inputs pairs as <vinp vinn>, <vinp vinn>,... vinp and vinn are numbered
> >>> +  from 0 to 19 on stm32h7)
> >>> +  Note: At least one of "st,adc-channels" or "st,adc-diff-channels" is required.
> >>>  - #io-channel-cells = <1>: See the IIO bindings section "IIO consumers" in
> >>>    Documentation/devicetree/bindings/iio/iio-bindings.txt
> >>>      
> >>
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> >> the body of a message to majordomo at vger.kernel.org
> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html  
> >   
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/3] dt-bindings: iio: adc: stm32: add support for diff channels

[Rob Herring]

On Tue, Oct 17, 2017 at 03:15:43PM +0200, Fabrice Gasnier wrote:
> STM32H7 ADC channels may be configured either as single-ended or
> differential.
> Add 'st,adc-diff-channels' property to support differential channels.
> Differential channels are defined as a pair of positive and negative
> inputs: vinp & vinn.
> 
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> ---
>  Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
> index 48bfcaa3..c3c8de1 100644
> --- a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
> +++ b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
> @@ -62,6 +62,12 @@ Required properties:
>  - st,adc-channels: List of single-ended channels muxed for this ADC.
>    It can have up to 16 channels on stm32f4 or 20 channels on stm32h7, numbered
>    from 0 to 15 or 19 (resp. for in0..in15 or in0..in19).
> +- st,adc-diff-channels: List of differential channels muxed for this ADC.
> +  Depending on part used, some channels can be configured as differential
> +  instead of single-ended (e.g. stm32h7). List here positive and negative
> +  inputs pairs as <vinp vinn>, <vinp vinn>,... vinp and vinn are numbered
> +  from 0 to 19 on stm32h7)
> +  Note: At least one of "st,adc-channels" or "st,adc-diff-channels" is required.

Wouldn't both present be invalid?

>  - #io-channel-cells = <1>: See the IIO bindings section "IIO consumers" in
>    Documentation/devicetree/bindings/iio/iio-bindings.txt
>  
> -- 
> 1.9.1
> 

Re: [PATCH 1/3] dt-bindings: iio: adc: stm32: add support for diff channels

[Rob Herring]

On Tue, Oct 17, 2017 at 03:15:43PM +0200, Fabrice Gasnier wrote:
> STM32H7 ADC channels may be configured either as single-ended or
> differential.
> Add 'st,adc-diff-channels' property to support differential channels.
> Differential channels are defined as a pair of positive and negative
> inputs: vinp & vinn.
> 
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier-qxv4g6HH51o@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
> index 48bfcaa3..c3c8de1 100644
> --- a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
> +++ b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
> @@ -62,6 +62,12 @@ Required properties:
>  - st,adc-channels: List of single-ended channels muxed for this ADC.
>    It can have up to 16 channels on stm32f4 or 20 channels on stm32h7, numbered
>    from 0 to 15 or 19 (resp. for in0..in15 or in0..in19).
> +- st,adc-diff-channels: List of differential channels muxed for this ADC.
> +  Depending on part used, some channels can be configured as differential
> +  instead of single-ended (e.g. stm32h7). List here positive and negative
> +  inputs pairs as <vinp vinn>, <vinp vinn>,... vinp and vinn are numbered
> +  from 0 to 19 on stm32h7)
> +  Note: At least one of "st,adc-channels" or "st,adc-diff-channels" is required.

Wouldn't both present be invalid?

>  - #io-channel-cells = <1>: See the IIO bindings section "IIO consumers" in
>    Documentation/devicetree/bindings/iio/iio-bindings.txt
>  
> -- 
> 1.9.1
> 
--
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 1/3] dt-bindings: iio: adc: stm32: add support for diff channels

[Rob Herring]

On Tue, Oct 17, 2017 at 03:15:43PM +0200, Fabrice Gasnier wrote:
> STM32H7 ADC channels may be configured either as single-ended or
> differential.
> Add 'st,adc-diff-channels' property to support differential channels.
> Differential channels are defined as a pair of positive and negative
> inputs: vinp & vinn.
> 
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> ---
>  Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
> index 48bfcaa3..c3c8de1 100644
> --- a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
> +++ b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
> @@ -62,6 +62,12 @@ Required properties:
>  - st,adc-channels: List of single-ended channels muxed for this ADC.
>    It can have up to 16 channels on stm32f4 or 20 channels on stm32h7, numbered
>    from 0 to 15 or 19 (resp. for in0..in15 or in0..in19).
> +- st,adc-diff-channels: List of differential channels muxed for this ADC.
> +  Depending on part used, some channels can be configured as differential
> +  instead of single-ended (e.g. stm32h7). List here positive and negative
> +  inputs pairs as <vinp vinn>, <vinp vinn>,... vinp and vinn are numbered
> +  from 0 to 19 on stm32h7)
> +  Note: At least one of "st,adc-channels" or "st,adc-diff-channels" is required.

Wouldn't both present be invalid?

>  - #io-channel-cells = <1>: See the IIO bindings section "IIO consumers" in
>    Documentation/devicetree/bindings/iio/iio-bindings.txt
>  
> -- 
> 1.9.1
> 

Re: [PATCH 1/3] dt-bindings: iio: adc: stm32: add support for diff channels

[Jonathan Cameron]

On 24 October 2017 17:41:38 BST, Rob Herring <robh@kernel.org> wrote:
>On Tue, Oct 17, 2017 at 03:15:43PM +0200, Fabrice Gasnier wrote:
>> STM32H7 ADC channels may be configured either as single-ended or
>> differential.
>> Add 'st,adc-diff-channels' property to support differential channels.
>> Differential channels are defined as a pair of positive and negative
>> inputs: vinp & vinn.
>> 
>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
>> ---
>>  Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt | 6
>++++++
>>  1 file changed, 6 insertions(+)
>> 
>> diff --git
>a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
>b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
>> index 48bfcaa3..c3c8de1 100644
>> --- a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
>> +++ b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
>> @@ -62,6 +62,12 @@ Required properties:
>>  - st,adc-channels: List of single-ended channels muxed for this ADC.
>>    It can have up to 16 channels on stm32f4 or 20 channels on
>stm32h7, numbered
>>    from 0 to 15 or 19 (resp. for in0..in15 or in0..in19).
>> +- st,adc-diff-channels: List of differential channels muxed for this
>ADC.
>> +  Depending on part used, some channels can be configured as
>differential
>> +  instead of single-ended (e.g. stm32h7). List here positive and
>negative
>> +  inputs pairs as <vinp vinn>, <vinp vinn>,... vinp and vinn are
>numbered
>> +  from 0 to 19 on stm32h7)
>> +  Note: At least one of "st,adc-channels" or "st,adc-diff-channels"
>is required.
>
>Wouldn't both present be invalid?
>

Probably invalid to have a number in both but some channels in each would be fine.

>>  - #io-channel-cells = <1>: See the IIO bindings section "IIO
>consumers" in
>>    Documentation/devicetree/bindings/iio/iio-bindings.txt
>>  
>> -- 
>> 1.9.1
>> 

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: [PATCH 1/3] dt-bindings: iio: adc: stm32: add support for diff channels

[Jonathan Cameron]

On 24 October 2017 17:41:38 BST, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>On Tue, Oct 17, 2017 at 03:15:43PM +0200, Fabrice Gasnier wrote:
>> STM32H7 ADC channels may be configured either as single-ended or
>> differential.
>> Add 'st,adc-diff-channels' property to support differential channels.
>> Differential channels are defined as a pair of positive and negative
>> inputs: vinp & vinn.
>> 
>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier-qxv4g6HH51o@public.gmane.org>
>> ---
>>  Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt | 6
>++++++
>>  1 file changed, 6 insertions(+)
>> 
>> diff --git
>a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
>b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
>> index 48bfcaa3..c3c8de1 100644
>> --- a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
>> +++ b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
>> @@ -62,6 +62,12 @@ Required properties:
>>  - st,adc-channels: List of single-ended channels muxed for this ADC.
>>    It can have up to 16 channels on stm32f4 or 20 channels on
>stm32h7, numbered
>>    from 0 to 15 or 19 (resp. for in0..in15 or in0..in19).
>> +- st,adc-diff-channels: List of differential channels muxed for this
>ADC.
>> +  Depending on part used, some channels can be configured as
>differential
>> +  instead of single-ended (e.g. stm32h7). List here positive and
>negative
>> +  inputs pairs as <vinp vinn>, <vinp vinn>,... vinp and vinn are
>numbered
>> +  from 0 to 19 on stm32h7)
>> +  Note: At least one of "st,adc-channels" or "st,adc-diff-channels"
>is required.
>
>Wouldn't both present be invalid?
>

Probably invalid to have a number in both but some channels in each would be fine.

>>  - #io-channel-cells = <1>: See the IIO bindings section "IIO
>consumers" in
>>    Documentation/devicetree/bindings/iio/iio-bindings.txt
>>  
>> -- 
>> 1.9.1
>> 

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

[PATCH 1/3] dt-bindings: iio: adc: stm32: add support for diff channels

[Jonathan Cameron]

On 24 October 2017 17:41:38 BST, Rob Herring <robh@kernel.org> wrote:
>On Tue, Oct 17, 2017 at 03:15:43PM +0200, Fabrice Gasnier wrote:
>> STM32H7 ADC channels may be configured either as single-ended or
>> differential.
>> Add 'st,adc-diff-channels' property to support differential channels.
>> Differential channels are defined as a pair of positive and negative
>> inputs: vinp & vinn.
>> 
>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
>> ---
>>  Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt | 6
>++++++
>>  1 file changed, 6 insertions(+)
>> 
>> diff --git
>a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
>b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
>> index 48bfcaa3..c3c8de1 100644
>> --- a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
>> +++ b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
>> @@ -62,6 +62,12 @@ Required properties:
>>  - st,adc-channels: List of single-ended channels muxed for this ADC.
>>    It can have up to 16 channels on stm32f4 or 20 channels on
>stm32h7, numbered
>>    from 0 to 15 or 19 (resp. for in0..in15 or in0..in19).
>> +- st,adc-diff-channels: List of differential channels muxed for this
>ADC.
>> +  Depending on part used, some channels can be configured as
>differential
>> +  instead of single-ended (e.g. stm32h7). List here positive and
>negative
>> +  inputs pairs as <vinp vinn>, <vinp vinn>,... vinp and vinn are
>numbered
>> +  from 0 to 19 on stm32h7)
>> +  Note: At least one of "st,adc-channels" or "st,adc-diff-channels"
>is required.
>
>Wouldn't both present be invalid?
>

Probably invalid to have a number in both but some channels in each would be fine.

>>  - #io-channel-cells = <1>: See the IIO bindings section "IIO
>consumers" in
>>    Documentation/devicetree/bindings/iio/iio-bindings.txt
>>  
>> -- 
>> 1.9.1
>> 

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: [PATCH 1/3] dt-bindings: iio: adc: stm32: add support for diff channels

[Fabrice Gasnier]

On 10/24/2017 08:42 PM, Jonathan Cameron wrote:
> 
> 
> On 24 October 2017 17:41:38 BST, Rob Herring <robh@kernel.org> wrote:
>> On Tue, Oct 17, 2017 at 03:15:43PM +0200, Fabrice Gasnier wrote:
>>> STM32H7 ADC channels may be configured either as single-ended or
>>> differential.
>>> Add 'st,adc-diff-channels' property to support differential channels.
>>> Differential channels are defined as a pair of positive and negative
>>> inputs: vinp & vinn.
>>>
>>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
>>> ---
>>>  Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt | 6
>> ++++++
>>>  1 file changed, 6 insertions(+)
>>>
>>> diff --git
>> a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
>> b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
>>> index 48bfcaa3..c3c8de1 100644
>>> --- a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
>>> +++ b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
>>> @@ -62,6 +62,12 @@ Required properties:
>>>  - st,adc-channels: List of single-ended channels muxed for this ADC.
>>>    It can have up to 16 channels on stm32f4 or 20 channels on
>> stm32h7, numbered
>>>    from 0 to 15 or 19 (resp. for in0..in15 or in0..in19).
>>> +- st,adc-diff-channels: List of differential channels muxed for this
>> ADC.
>>> +  Depending on part used, some channels can be configured as
>> differential
>>> +  instead of single-ended (e.g. stm32h7). List here positive and
>> negative
>>> +  inputs pairs as <vinp vinn>, <vinp vinn>,... vinp and vinn are
>> numbered
>>> +  from 0 to 19 on stm32h7)
>>> +  Note: At least one of "st,adc-channels" or "st,adc-diff-channels"
>> is required.
>>
>> Wouldn't both present be invalid?
>>
> 
Hi Rob, Jonathan,

> Probably invalid to have a number in both but some channels in each would be fine.

Yes, both properties can be used together. Some channels can be used as
single-ended and some other ones as differential (mixed). But channels
can't be configured both as single-ended and differential (invalid).

I'll mention this in the note, and also update differential channels
example:

Example to setup:
- channel 1 as single-ended
- channels 2 & 3 as differential (with resp. 6 & 7 negative inputs)

	adc: adc@40022000 {
		compatible = "st,stm32h7-adc-core";
		...
		adc1: adc@0 {
			compatible = "st,stm32h7-adc";
			...
			st,adc-channels = <1>;
			st,adc-diff-channels = <2 6>, <3 7>;
		};
	};

I will send V2 with these updates.

Thanks,
Best Regards,
Fabrice
> 
>>>  - #io-channel-cells = <1>: See the IIO bindings section "IIO
>> consumers" in
>>>    Documentation/devicetree/bindings/iio/iio-bindings.txt
>>>  
>>> -- 
>>> 1.9.1
>>>
> 

Re: [PATCH 1/3] dt-bindings: iio: adc: stm32: add support for diff channels

[Fabrice Gasnier]

On 10/24/2017 08:42 PM, Jonathan Cameron wrote:
> 
> 
> On 24 October 2017 17:41:38 BST, Rob Herring <robh@kernel.org> wrote:
>> On Tue, Oct 17, 2017 at 03:15:43PM +0200, Fabrice Gasnier wrote:
>>> STM32H7 ADC channels may be configured either as single-ended or
>>> differential.
>>> Add 'st,adc-diff-channels' property to support differential channels.
>>> Differential channels are defined as a pair of positive and negative
>>> inputs: vinp & vinn.
>>>
>>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
>>> ---
>>>  Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt | 6
>> ++++++
>>>  1 file changed, 6 insertions(+)
>>>
>>> diff --git
>> a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
>> b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
>>> index 48bfcaa3..c3c8de1 100644
>>> --- a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
>>> +++ b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
>>> @@ -62,6 +62,12 @@ Required properties:
>>>  - st,adc-channels: List of single-ended channels muxed for this ADC.
>>>    It can have up to 16 channels on stm32f4 or 20 channels on
>> stm32h7, numbered
>>>    from 0 to 15 or 19 (resp. for in0..in15 or in0..in19).
>>> +- st,adc-diff-channels: List of differential channels muxed for this
>> ADC.
>>> +  Depending on part used, some channels can be configured as
>> differential
>>> +  instead of single-ended (e.g. stm32h7). List here positive and
>> negative
>>> +  inputs pairs as <vinp vinn>, <vinp vinn>,... vinp and vinn are
>> numbered
>>> +  from 0 to 19 on stm32h7)
>>> +  Note: At least one of "st,adc-channels" or "st,adc-diff-channels"
>> is required.
>>
>> Wouldn't both present be invalid?
>>
> 
Hi Rob, Jonathan,

> Probably invalid to have a number in both but some channels in each would be fine.

Yes, both properties can be used together. Some channels can be used as
single-ended and some other ones as differential (mixed). But channels
can't be configured both as single-ended and differential (invalid).

I'll mention this in the note, and also update differential channels
example:

Example to setup:
- channel 1 as single-ended
- channels 2 & 3 as differential (with resp. 6 & 7 negative inputs)

	adc: adc@40022000 {
		compatible = "st,stm32h7-adc-core";
		...
		adc1: adc@0 {
			compatible = "st,stm32h7-adc";
			...
			st,adc-channels = <1>;
			st,adc-diff-channels = <2 6>, <3 7>;
		};
	};

I will send V2 with these updates.

Thanks,
Best Regards,
Fabrice
> 
>>>  - #io-channel-cells = <1>: See the IIO bindings section "IIO
>> consumers" in
>>>    Documentation/devicetree/bindings/iio/iio-bindings.txt
>>>  
>>> -- 
>>> 1.9.1
>>>
> 

[PATCH 1/3] dt-bindings: iio: adc: stm32: add support for diff channels

[Fabrice Gasnier]

On 10/24/2017 08:42 PM, Jonathan Cameron wrote:
> 
> 
> On 24 October 2017 17:41:38 BST, Rob Herring <robh@kernel.org> wrote:
>> On Tue, Oct 17, 2017 at 03:15:43PM +0200, Fabrice Gasnier wrote:
>>> STM32H7 ADC channels may be configured either as single-ended or
>>> differential.
>>> Add 'st,adc-diff-channels' property to support differential channels.
>>> Differential channels are defined as a pair of positive and negative
>>> inputs: vinp & vinn.
>>>
>>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
>>> ---
>>>  Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt | 6
>> ++++++
>>>  1 file changed, 6 insertions(+)
>>>
>>> diff --git
>> a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
>> b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
>>> index 48bfcaa3..c3c8de1 100644
>>> --- a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
>>> +++ b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
>>> @@ -62,6 +62,12 @@ Required properties:
>>>  - st,adc-channels: List of single-ended channels muxed for this ADC.
>>>    It can have up to 16 channels on stm32f4 or 20 channels on
>> stm32h7, numbered
>>>    from 0 to 15 or 19 (resp. for in0..in15 or in0..in19).
>>> +- st,adc-diff-channels: List of differential channels muxed for this
>> ADC.
>>> +  Depending on part used, some channels can be configured as
>> differential
>>> +  instead of single-ended (e.g. stm32h7). List here positive and
>> negative
>>> +  inputs pairs as <vinp vinn>, <vinp vinn>,... vinp and vinn are
>> numbered
>>> +  from 0 to 19 on stm32h7)
>>> +  Note: At least one of "st,adc-channels" or "st,adc-diff-channels"
>> is required.
>>
>> Wouldn't both present be invalid?
>>
> 
Hi Rob, Jonathan,

> Probably invalid to have a number in both but some channels in each would be fine.

Yes, both properties can be used together. Some channels can be used as
single-ended and some other ones as differential (mixed). But channels
can't be configured both as single-ended and differential (invalid).

I'll mention this in the note, and also update differential channels
example:

Example to setup:
- channel 1 as single-ended
- channels 2 & 3 as differential (with resp. 6 & 7 negative inputs)

	adc: adc at 40022000 {
		compatible = "st,stm32h7-adc-core";
		...
		adc1: adc at 0 {
			compatible = "st,stm32h7-adc";
			...
			st,adc-channels = <1>;
			st,adc-diff-channels = <2 6>, <3 7>;
		};
	};

I will send V2 with these updates.

Thanks,
Best Regards,
Fabrice
> 
>>>  - #io-channel-cells = <1>: See the IIO bindings section "IIO
>> consumers" in
>>>    Documentation/devicetree/bindings/iio/iio-bindings.txt
>>>  
>>> -- 
>>> 1.9.1
>>>
>