* [PATCH 0/3] iio: adc: stm32: Add support for differential channels
@ 2017-10-17 13:15 Fabrice Gasnier
2017-10-17 13:15 ` [PATCH 1/3] dt-bindings: iio: adc: stm32: add support for diff channels Fabrice Gasnier
` (2 more replies)
0 siblings, 3 replies; 15+ messages in thread
From: Fabrice Gasnier @ 2017-10-17 13:15 UTC (permalink / raw)
To: jic23, robh+dt
Cc: linux, mark.rutland, mcoquelin.stm32, alexandre.torgue, lars,
knaack.h, pmeerw, fabrice.gasnier, linux-iio, devicetree,
linux-arm-kernel, linux-kernel, benjamin.gaignard,
benjamin.gaignard
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/3] dt-bindings: iio: adc: stm32: add support for diff channels
2017-10-17 13:15 [PATCH 0/3] iio: adc: stm32: Add support for differential channels Fabrice Gasnier
@ 2017-10-17 13:15 ` Fabrice Gasnier
2017-10-21 17:54 ` Jonathan Cameron
2017-10-24 16:41 ` Rob Herring
2017-10-17 13:15 ` [PATCH 2/3] iio: adc: stm32: remove const channel names definition Fabrice Gasnier
2017-10-17 13:15 ` [PATCH 3/3] iio: adc: stm32: add support for differential channels Fabrice Gasnier
2 siblings, 2 replies; 15+ messages in thread
From: Fabrice Gasnier @ 2017-10-17 13:15 UTC (permalink / raw)
To: jic23, robh+dt
Cc: linux, mark.rutland, mcoquelin.stm32, alexandre.torgue, lars,
knaack.h, pmeerw, fabrice.gasnier, linux-iio, devicetree,
linux-arm-kernel, linux-kernel, benjamin.gaignard,
benjamin.gaignard
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/3] iio: adc: stm32: remove const channel names definition
2017-10-17 13:15 [PATCH 0/3] iio: adc: stm32: Add support for differential channels Fabrice Gasnier
2017-10-17 13:15 ` [PATCH 1/3] dt-bindings: iio: adc: stm32: add support for diff channels Fabrice Gasnier
@ 2017-10-17 13:15 ` Fabrice Gasnier
2017-10-21 17:59 ` Jonathan Cameron
2017-10-17 13:15 ` [PATCH 3/3] iio: adc: stm32: add support for differential channels Fabrice Gasnier
2 siblings, 1 reply; 15+ messages in thread
From: Fabrice Gasnier @ 2017-10-17 13:15 UTC (permalink / raw)
To: jic23, robh+dt
Cc: linux, mark.rutland, mcoquelin.stm32, alexandre.torgue, lars,
knaack.h, pmeerw, fabrice.gasnier, linux-iio, devicetree,
linux-arm-kernel, linux-kernel, benjamin.gaignard,
benjamin.gaignard
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 3/3] iio: adc: stm32: add support for differential channels
2017-10-17 13:15 [PATCH 0/3] iio: adc: stm32: Add support for differential channels Fabrice Gasnier
2017-10-17 13:15 ` [PATCH 1/3] dt-bindings: iio: adc: stm32: add support for diff channels Fabrice Gasnier
2017-10-17 13:15 ` [PATCH 2/3] iio: adc: stm32: remove const channel names definition Fabrice Gasnier
@ 2017-10-17 13:15 ` Fabrice Gasnier
2017-10-21 19:25 ` Jonathan Cameron
2 siblings, 1 reply; 15+ messages in thread
From: Fabrice Gasnier @ 2017-10-17 13:15 UTC (permalink / raw)
To: jic23, robh+dt
Cc: linux, mark.rutland, mcoquelin.stm32, alexandre.torgue, lars,
knaack.h, pmeerw, fabrice.gasnier, linux-iio, devicetree,
linux-arm-kernel, linux-kernel, benjamin.gaignard,
benjamin.gaignard
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 1/3] dt-bindings: iio: adc: stm32: add support for diff channels
2017-10-17 13:15 ` [PATCH 1/3] dt-bindings: iio: adc: stm32: add support for diff channels Fabrice Gasnier
@ 2017-10-21 17:54 ` Jonathan Cameron
2017-10-21 17:55 ` Jonathan Cameron
2017-10-21 19:23 ` Jonathan Cameron
2017-10-24 16:41 ` Rob Herring
1 sibling, 2 replies; 15+ messages in thread
From: Jonathan Cameron @ 2017-10-21 17:54 UTC (permalink / raw)
To: Fabrice Gasnier
Cc: robh+dt, linux, mark.rutland, mcoquelin.stm32, alexandre.torgue,
lars, knaack.h, pmeerw, linux-iio, devicetree, linux-arm-kernel,
linux-kernel, benjamin.gaignard, benjamin.gaignard
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
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/3] dt-bindings: iio: adc: stm32: add support for diff channels
2017-10-21 17:54 ` Jonathan Cameron
@ 2017-10-21 17:55 ` Jonathan Cameron
2017-10-21 19:23 ` Jonathan Cameron
1 sibling, 0 replies; 15+ messages in thread
From: Jonathan Cameron @ 2017-10-21 17:55 UTC (permalink / raw)
To: Fabrice Gasnier
Cc: robh+dt, linux, mark.rutland, mcoquelin.stm32, alexandre.torgue,
lars, knaack.h, pmeerw, linux-iio, devicetree, linux-arm-kernel,
linux-kernel, benjamin.gaignard, benjamin.gaignard, Rob Herring,
Mark Rutland
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
> >
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] iio: adc: stm32: remove const channel names definition
2017-10-17 13:15 ` [PATCH 2/3] iio: adc: stm32: remove const channel names definition Fabrice Gasnier
@ 2017-10-21 17:59 ` Jonathan Cameron
0 siblings, 0 replies; 15+ messages in thread
From: Jonathan Cameron @ 2017-10-21 17:59 UTC (permalink / raw)
To: Fabrice Gasnier
Cc: robh+dt, linux, mark.rutland, mcoquelin.stm32, alexandre.torgue,
lars, knaack.h, pmeerw, linux-iio, devicetree, linux-arm-kernel,
linux-kernel, benjamin.gaignard, benjamin.gaignard
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++;
> }
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/3] dt-bindings: iio: adc: stm32: add support for diff channels
2017-10-21 17:54 ` Jonathan Cameron
2017-10-21 17:55 ` Jonathan Cameron
@ 2017-10-21 19:23 ` Jonathan Cameron
2017-10-23 8:06 ` Fabrice Gasnier
1 sibling, 1 reply; 15+ messages in thread
From: Jonathan Cameron @ 2017-10-21 19:23 UTC (permalink / raw)
To: Fabrice Gasnier
Cc: robh+dt, linux, mark.rutland, mcoquelin.stm32, alexandre.torgue,
lars, knaack.h, pmeerw, linux-iio, devicetree, linux-arm-kernel,
linux-kernel, benjamin.gaignard, benjamin.gaignard
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] iio: adc: stm32: add support for differential channels
2017-10-17 13:15 ` [PATCH 3/3] iio: adc: stm32: add support for differential channels Fabrice Gasnier
@ 2017-10-21 19:25 ` Jonathan Cameron
2017-10-23 8:09 ` Fabrice Gasnier
0 siblings, 1 reply; 15+ messages in thread
From: Jonathan Cameron @ 2017-10-21 19:25 UTC (permalink / raw)
To: Fabrice Gasnier
Cc: robh+dt, linux, mark.rutland, mcoquelin.stm32, alexandre.torgue,
lars, knaack.h, pmeerw, linux-iio, devicetree, linux-arm-kernel,
linux-kernel, benjamin.gaignard, benjamin.gaignard
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;
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/3] dt-bindings: iio: adc: stm32: add support for diff channels
2017-10-21 19:23 ` Jonathan Cameron
@ 2017-10-23 8:06 ` Fabrice Gasnier
2017-10-23 13:09 ` Jonathan Cameron
0 siblings, 1 reply; 15+ messages in thread
From: Fabrice Gasnier @ 2017-10-23 8:06 UTC (permalink / raw)
To: Jonathan Cameron
Cc: robh+dt, linux, mark.rutland, mcoquelin.stm32, alexandre.torgue,
lars, knaack.h, pmeerw, linux-iio, devicetree, linux-arm-kernel,
linux-kernel, benjamin.gaignard, benjamin.gaignard
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
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] iio: adc: stm32: add support for differential channels
2017-10-21 19:25 ` Jonathan Cameron
@ 2017-10-23 8:09 ` Fabrice Gasnier
0 siblings, 0 replies; 15+ messages in thread
From: Fabrice Gasnier @ 2017-10-23 8:09 UTC (permalink / raw)
To: Jonathan Cameron
Cc: robh+dt, linux, mark.rutland, mcoquelin.stm32, alexandre.torgue,
lars, knaack.h, pmeerw, linux-iio, devicetree, linux-arm-kernel,
linux-kernel, benjamin.gaignard, benjamin.gaignard
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;
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/3] dt-bindings: iio: adc: stm32: add support for diff channels
2017-10-23 8:06 ` Fabrice Gasnier
@ 2017-10-23 13:09 ` Jonathan Cameron
0 siblings, 0 replies; 15+ messages in thread
From: Jonathan Cameron @ 2017-10-23 13:09 UTC (permalink / raw)
To: Fabrice Gasnier
Cc: Jonathan Cameron, mark.rutland, devicetree, benjamin.gaignard,
lars, alexandre.torgue, linux-iio, pmeerw, linux, linux-kernel,
robh+dt, mcoquelin.stm32, knaack.h, linux-arm-kernel,
benjamin.gaignard
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/3] dt-bindings: iio: adc: stm32: add support for diff channels
2017-10-17 13:15 ` [PATCH 1/3] dt-bindings: iio: adc: stm32: add support for diff channels Fabrice Gasnier
2017-10-21 17:54 ` Jonathan Cameron
@ 2017-10-24 16:41 ` Rob Herring
2017-10-24 18:42 ` Jonathan Cameron
1 sibling, 1 reply; 15+ messages in thread
From: Rob Herring @ 2017-10-24 16:41 UTC (permalink / raw)
To: Fabrice Gasnier
Cc: jic23, linux, mark.rutland, mcoquelin.stm32, alexandre.torgue,
lars, knaack.h, pmeerw, linux-iio, devicetree, linux-arm-kernel,
linux-kernel, benjamin.gaignard, benjamin.gaignard
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
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/3] dt-bindings: iio: adc: stm32: add support for diff channels
2017-10-24 16:41 ` Rob Herring
@ 2017-10-24 18:42 ` Jonathan Cameron
2017-10-25 8:05 ` Fabrice Gasnier
0 siblings, 1 reply; 15+ messages in thread
From: Jonathan Cameron @ 2017-10-24 18:42 UTC (permalink / raw)
To: Rob Herring, Fabrice Gasnier
Cc: jic23, linux, mark.rutland, mcoquelin.stm32, alexandre.torgue,
lars, knaack.h, pmeerw, linux-iio, devicetree, linux-arm-kernel,
linux-kernel, benjamin.gaignard, benjamin.gaignard
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.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/3] dt-bindings: iio: adc: stm32: add support for diff channels
2017-10-24 18:42 ` Jonathan Cameron
@ 2017-10-25 8:05 ` Fabrice Gasnier
0 siblings, 0 replies; 15+ messages in thread
From: Fabrice Gasnier @ 2017-10-25 8:05 UTC (permalink / raw)
To: Jonathan Cameron, Rob Herring
Cc: jic23, linux, mark.rutland, mcoquelin.stm32, alexandre.torgue,
lars, knaack.h, pmeerw, linux-iio, devicetree, linux-arm-kernel,
linux-kernel, benjamin.gaignard, benjamin.gaignard
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
>>>
>
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2017-10-25 8:06 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-17 13:15 [PATCH 0/3] iio: adc: stm32: Add support for differential channels Fabrice Gasnier
2017-10-17 13:15 ` [PATCH 1/3] dt-bindings: iio: adc: stm32: add support for diff channels Fabrice Gasnier
2017-10-21 17:54 ` Jonathan Cameron
2017-10-21 17:55 ` Jonathan Cameron
2017-10-21 19:23 ` Jonathan Cameron
2017-10-23 8:06 ` Fabrice Gasnier
2017-10-23 13:09 ` Jonathan Cameron
2017-10-24 16:41 ` Rob Herring
2017-10-24 18:42 ` Jonathan Cameron
2017-10-25 8:05 ` Fabrice Gasnier
2017-10-17 13:15 ` [PATCH 2/3] iio: adc: stm32: remove const channel names definition Fabrice Gasnier
2017-10-21 17:59 ` Jonathan Cameron
2017-10-17 13:15 ` [PATCH 3/3] iio: adc: stm32: add support for differential channels Fabrice Gasnier
2017-10-21 19:25 ` Jonathan Cameron
2017-10-23 8:09 ` Fabrice Gasnier
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).