* [PATCH 3/3] iio: adc: stm32: add optional min-sample-time
@ 2017-07-18 12:35 ` Fabrice Gasnier
0 siblings, 0 replies; 39+ messages in thread
From: Fabrice Gasnier @ 2017-07-18 12:35 UTC (permalink / raw)
To: jic23, robh+dt
Cc: mark.rutland, devicetree, benjamin.gaignard, lars,
alexandre.torgue, linux-iio, pmeerw, linux, linux-kernel,
mcoquelin.stm32, knaack.h, fabrice.gasnier, linux-arm-kernel,
benjamin.gaignard
STM32 ADC allows each channel to be sampled with a different sampling time,
by setting SMPR registers. Basically, value depends on local electrical
properties. Selecting correct value for sampling time highly depends on
analog source impedance. There is a manual that may help in this process:
'How to get the best ADC accuracy in STM32...'
This patch allows to configure min-sample-time via device tree, either for:
- all channels at once:
min-sample-time = <10000>; /* nanosecs */
- independently for each channel (must match "st,adc-channels" list):
st,adc-channels = <0 1>;
min-sample-time = <5000 10000>; /* nanosecs */
Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
---
drivers/iio/adc/stm32-adc.c | 134 ++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 130 insertions(+), 4 deletions(-)
diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
index 5bfcc1f..e6f6b5e 100644
--- a/drivers/iio/adc/stm32-adc.c
+++ b/drivers/iio/adc/stm32-adc.c
@@ -83,6 +83,8 @@
#define STM32H7_ADC_IER 0x04
#define STM32H7_ADC_CR 0x08
#define STM32H7_ADC_CFGR 0x0C
+#define STM32H7_ADC_SMPR1 0x14
+#define STM32H7_ADC_SMPR2 0x18
#define STM32H7_ADC_PCSEL 0x1C
#define STM32H7_ADC_SQR1 0x30
#define STM32H7_ADC_SQR2 0x34
@@ -151,6 +153,7 @@ enum stm32h7_adc_dmngt {
#define STM32H7_BOOST_CLKRATE 20000000UL
#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
#define STM32_ADC_TIMEOUT (msecs_to_jiffies(STM32_ADC_TIMEOUT_US / 1000))
@@ -227,6 +230,8 @@ struct stm32_adc_regs {
* @exten: trigger control register & bitfield
* @extsel: trigger selection register & bitfield
* @res: resolution selection register & bitfield
+ * @smpr: smpr1 & smpr2 registers offset array
+ * @smp_bits: smpr1 & smpr2 index and bitfields
*/
struct stm32_adc_regspec {
const u32 dr;
@@ -236,6 +241,8 @@ struct stm32_adc_regspec {
const struct stm32_adc_regs exten;
const struct stm32_adc_regs extsel;
const struct stm32_adc_regs res;
+ const u32 smpr[2];
+ const struct stm32_adc_regs *smp_bits;
};
struct stm32_adc;
@@ -251,6 +258,7 @@ struct stm32_adc_regspec {
* @start_conv: routine to start conversions
* @stop_conv: routine to stop conversions
* @unprepare: optional unprepare routine (disable, power-down)
+ * @smp_cycles: programmable sampling time (ADC clock cycles)
*/
struct stm32_adc_cfg {
const struct stm32_adc_regspec *regs;
@@ -262,6 +270,7 @@ struct stm32_adc_cfg {
void (*start_conv)(struct stm32_adc *, bool dma);
void (*stop_conv)(struct stm32_adc *);
void (*unprepare)(struct stm32_adc *);
+ const unsigned int *smp_cycles;
};
/**
@@ -283,6 +292,7 @@ struct stm32_adc_cfg {
* @rx_dma_buf: dma rx buffer bus address
* @rx_buf_sz: dma rx buffer size
* @pcsel bitmask to preselect channels on some devices
+ * @smpr_val: sampling time settings (e.g. smpr1 / smpr2)
* @cal: optional calibration data on some devices
*/
struct stm32_adc {
@@ -303,6 +313,7 @@ struct stm32_adc {
dma_addr_t rx_dma_buf;
unsigned int rx_buf_sz;
u32 pcsel;
+ u32 smpr_val[2];
struct stm32_adc_calib cal;
};
@@ -431,6 +442,39 @@ struct stm32_adc_info {
{}, /* sentinel */
};
+/**
+ * stm32f4_smp_bits[] - describe sampling time register index & bit fields
+ * Sorted so it can be indexed by channel number.
+ */
+static const struct stm32_adc_regs stm32f4_smp_bits[] = {
+ /* STM32F4_ADC_SMPR2: smpr[] index, mask, shift for SMP0 to SMP9 */
+ { 1, GENMASK(2, 0), 0 },
+ { 1, GENMASK(5, 3), 3 },
+ { 1, GENMASK(8, 6), 6 },
+ { 1, GENMASK(11, 9), 9 },
+ { 1, GENMASK(14, 12), 12 },
+ { 1, GENMASK(17, 15), 15 },
+ { 1, GENMASK(20, 18), 18 },
+ { 1, GENMASK(23, 21), 21 },
+ { 1, GENMASK(26, 24), 24 },
+ { 1, GENMASK(29, 27), 27 },
+ /* STM32F4_ADC_SMPR1, smpr[] index, mask, shift for SMP10 to SMP18 */
+ { 0, GENMASK(2, 0), 0 },
+ { 0, GENMASK(5, 3), 3 },
+ { 0, GENMASK(8, 6), 6 },
+ { 0, GENMASK(11, 9), 9 },
+ { 0, GENMASK(14, 12), 12 },
+ { 0, GENMASK(17, 15), 15 },
+ { 0, GENMASK(20, 18), 18 },
+ { 0, GENMASK(23, 21), 21 },
+ { 0, GENMASK(26, 24), 24 },
+};
+
+/* STM32F4 programmable sampling time (ADC clock cycles) */
+static const unsigned int stm32f4_adc_smp_cycles[STM32_ADC_MAX_SMP + 1] = {
+ 3, 15, 28, 56, 84, 112, 144, 480,
+};
+
static const struct stm32_adc_regspec stm32f4_adc_regspec = {
.dr = STM32F4_ADC_DR,
.ier_eoc = { STM32F4_ADC_CR1, STM32F4_EOCIE },
@@ -440,6 +484,8 @@ struct stm32_adc_info {
.extsel = { STM32F4_ADC_CR2, STM32F4_EXTSEL_MASK,
STM32F4_EXTSEL_SHIFT },
.res = { STM32F4_ADC_CR1, STM32F4_RES_MASK, STM32F4_RES_SHIFT },
+ .smpr = { STM32F4_ADC_SMPR1, STM32F4_ADC_SMPR2 },
+ .smp_bits = stm32f4_smp_bits,
};
static const struct stm32_adc_regs stm32h7_sq[STM32_ADC_MAX_SQ + 1] = {
@@ -483,6 +529,40 @@ struct stm32_adc_info {
{},
};
+/**
+ * stm32h7_smp_bits - describe sampling time register index & bit fields
+ * Sorted so it can be indexed by channel number.
+ */
+static const struct stm32_adc_regs stm32h7_smp_bits[] = {
+ /* STM32H7_ADC_SMPR1, smpr[] index, mask, shift for SMP0 to SMP9 */
+ { 0, GENMASK(2, 0), 0 },
+ { 0, GENMASK(5, 3), 3 },
+ { 0, GENMASK(8, 6), 6 },
+ { 0, GENMASK(11, 9), 9 },
+ { 0, GENMASK(14, 12), 12 },
+ { 0, GENMASK(17, 15), 15 },
+ { 0, GENMASK(20, 18), 18 },
+ { 0, GENMASK(23, 21), 21 },
+ { 0, GENMASK(26, 24), 24 },
+ { 0, GENMASK(29, 27), 27 },
+ /* STM32H7_ADC_SMPR2, smpr[] index, mask, shift for SMP10 to SMP19 */
+ { 1, GENMASK(2, 0), 0 },
+ { 1, GENMASK(5, 3), 3 },
+ { 1, GENMASK(8, 6), 6 },
+ { 1, GENMASK(11, 9), 9 },
+ { 1, GENMASK(14, 12), 12 },
+ { 1, GENMASK(17, 15), 15 },
+ { 1, GENMASK(20, 18), 18 },
+ { 1, GENMASK(23, 21), 21 },
+ { 1, GENMASK(26, 24), 24 },
+ { 1, GENMASK(29, 27), 27 },
+};
+
+/* STM32H7 programmable sampling time (ADC clock cycles, rounded down) */
+static const unsigned int stm32h7_adc_smp_cycles[STM32_ADC_MAX_SMP + 1] = {
+ 1, 2, 8, 16, 32, 64, 387, 810,
+};
+
static const struct stm32_adc_regspec stm32h7_adc_regspec = {
.dr = STM32H7_ADC_DR,
.ier_eoc = { STM32H7_ADC_IER, STM32H7_EOCIE },
@@ -492,6 +572,8 @@ struct stm32_adc_info {
.extsel = { STM32H7_ADC_CFGR, STM32H7_EXTSEL_MASK,
STM32H7_EXTSEL_SHIFT },
.res = { STM32H7_ADC_CFGR, STM32H7_RES_MASK, STM32H7_RES_SHIFT },
+ .smpr = { STM32H7_ADC_SMPR1, STM32H7_ADC_SMPR2 },
+ .smp_bits = stm32h7_smp_bits,
};
/**
@@ -933,6 +1015,7 @@ static void stm32h7_adc_unprepare(struct stm32_adc *adc)
* @scan_mask: channels to be converted
*
* Conversion sequence :
+ * Apply sampling time settings for all channels.
* Configure ADC scan sequence based on selected channels in scan_mask.
* Add channels to SQR registers, from scan_mask LSB to MSB, then
* program sequence len.
@@ -946,6 +1029,10 @@ static int stm32_adc_conf_scan_seq(struct iio_dev *indio_dev,
u32 val, bit;
int i = 0;
+ /* Apply sampling time settings */
+ stm32_adc_writel(adc, adc->cfg->regs->smpr[0], adc->smpr_val[0]);
+ stm32_adc_writel(adc, adc->cfg->regs->smpr[1], adc->smpr_val[1]);
+
for_each_set_bit(bit, scan_mask, indio_dev->masklength) {
chan = indio_dev->channels + bit;
/*
@@ -1079,6 +1166,7 @@ static int stm32_adc_get_trig_pol(struct iio_dev *indio_dev,
* @res: conversion result
*
* The function performs a single conversion on a given channel:
+ * - Apply sampling time settings
* - Program sequencer with one channel (e.g. in SQ1 with len = 1)
* - Use SW trigger
* - Start conversion, then wait for interrupt completion.
@@ -1103,6 +1191,10 @@ static int stm32_adc_single_conv(struct iio_dev *indio_dev,
return ret;
}
+ /* Apply sampling time settings */
+ stm32_adc_writel(adc, regs->smpr[0], adc->smpr_val[0]);
+ stm32_adc_writel(adc, regs->smpr[1], adc->smpr_val[1]);
+
/* Program chan number in regular sequence (SQ1) */
val = stm32_adc_readl(adc, regs->sqr[1].reg);
val &= ~regs->sqr[1].mask;
@@ -1507,10 +1599,28 @@ static int stm32_adc_of_get_resolution(struct iio_dev *indio_dev)
return 0;
}
+static void stm32_adc_smpr_init(struct stm32_adc *adc, int channel, u32 smp_ns)
+{
+ const struct stm32_adc_regs *smpr = &adc->cfg->regs->smp_bits[channel];
+ u32 period_ns, shift = smpr->shift, mask = smpr->mask;
+ unsigned int smp, r = smpr->reg;
+
+ /* Determine sampling time (ADC clock cycles) */
+ period_ns = NSEC_PER_SEC / adc->common->rate;
+ for (smp = 0; smp <= STM32_ADC_MAX_SMP; smp++)
+ if ((period_ns * adc->cfg->smp_cycles[smp]) >= smp_ns)
+ break;
+ if (smp > STM32_ADC_MAX_SMP)
+ smp = STM32_ADC_MAX_SMP;
+
+ /* pre-build sampling time registers (e.g. smpr1, smpr2) */
+ adc->smpr_val[r] = (adc->smpr_val[r] & ~mask) | (smp << shift);
+}
+
static void stm32_adc_chan_init_one(struct iio_dev *indio_dev,
struct iio_chan_spec *chan,
const struct stm32_adc_chan_spec *channel,
- int scan_index)
+ int scan_index, u32 smp)
{
struct stm32_adc *adc = iio_priv(indio_dev);
@@ -1526,6 +1636,9 @@ static void stm32_adc_chan_init_one(struct iio_dev *indio_dev,
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);
}
@@ -1538,8 +1651,8 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
struct property *prop;
const __be32 *cur;
struct iio_chan_spec *channels;
- int scan_index = 0, num_channels;
- u32 val;
+ int scan_index = 0, num_channels, ret;
+ u32 val, smp = 0;
num_channels = of_property_count_u32_elems(node, "st,adc-channels");
if (num_channels < 0 ||
@@ -1548,6 +1661,13 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
return num_channels < 0 ? num_channels : -EINVAL;
}
+ /* Optional sample time is provided either for each, or all channels */
+ ret = of_property_count_u32_elems(node, "min-sample-time");
+ if (ret > 1 && ret != num_channels) {
+ dev_err(&indio_dev->dev, "Invalid min-sample-time\n");
+ return -EINVAL;
+ }
+
channels = devm_kcalloc(&indio_dev->dev, num_channels,
sizeof(struct iio_chan_spec), GFP_KERNEL);
if (!channels)
@@ -1558,9 +1678,13 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
dev_err(&indio_dev->dev, "Invalid channel %d\n", val);
return -EINVAL;
}
+
+ of_property_read_u32_index(node, "min-sample-time", scan_index,
+ &smp);
+
stm32_adc_chan_init_one(indio_dev, &channels[scan_index],
&adc_info->channels[val],
- scan_index);
+ scan_index, smp);
scan_index++;
}
@@ -1755,6 +1879,7 @@ static int stm32_adc_remove(struct platform_device *pdev)
.clk_required = true,
.start_conv = stm32f4_adc_start_conv,
.stop_conv = stm32f4_adc_stop_conv,
+ .smp_cycles = stm32f4_adc_smp_cycles,
};
static const struct stm32_adc_cfg stm32h7_adc_cfg = {
@@ -1766,6 +1891,7 @@ static int stm32_adc_remove(struct platform_device *pdev)
.stop_conv = stm32h7_adc_stop_conv,
.prepare = stm32h7_adc_prepare,
.unprepare = stm32h7_adc_unprepare,
+ .smp_cycles = stm32h7_adc_smp_cycles,
};
static const struct of_device_id stm32_adc_of_match[] = {
--
1.9.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH 3/3] iio: adc: stm32: add optional min-sample-time
@ 2017-07-18 12:35 ` Fabrice Gasnier
0 siblings, 0 replies; 39+ messages in thread
From: Fabrice Gasnier @ 2017-07-18 12:35 UTC (permalink / raw)
To: linux-arm-kernel
STM32 ADC allows each channel to be sampled with a different sampling time,
by setting SMPR registers. Basically, value depends on local electrical
properties. Selecting correct value for sampling time highly depends on
analog source impedance. There is a manual that may help in this process:
'How to get the best ADC accuracy in STM32...'
This patch allows to configure min-sample-time via device tree, either for:
- all channels at once:
min-sample-time = <10000>; /* nanosecs */
- independently for each channel (must match "st,adc-channels" list):
st,adc-channels = <0 1>;
min-sample-time = <5000 10000>; /* nanosecs */
Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
---
drivers/iio/adc/stm32-adc.c | 134 ++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 130 insertions(+), 4 deletions(-)
diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
index 5bfcc1f..e6f6b5e 100644
--- a/drivers/iio/adc/stm32-adc.c
+++ b/drivers/iio/adc/stm32-adc.c
@@ -83,6 +83,8 @@
#define STM32H7_ADC_IER 0x04
#define STM32H7_ADC_CR 0x08
#define STM32H7_ADC_CFGR 0x0C
+#define STM32H7_ADC_SMPR1 0x14
+#define STM32H7_ADC_SMPR2 0x18
#define STM32H7_ADC_PCSEL 0x1C
#define STM32H7_ADC_SQR1 0x30
#define STM32H7_ADC_SQR2 0x34
@@ -151,6 +153,7 @@ enum stm32h7_adc_dmngt {
#define STM32H7_BOOST_CLKRATE 20000000UL
#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
#define STM32_ADC_TIMEOUT (msecs_to_jiffies(STM32_ADC_TIMEOUT_US / 1000))
@@ -227,6 +230,8 @@ struct stm32_adc_regs {
* @exten: trigger control register & bitfield
* @extsel: trigger selection register & bitfield
* @res: resolution selection register & bitfield
+ * @smpr: smpr1 & smpr2 registers offset array
+ * @smp_bits: smpr1 & smpr2 index and bitfields
*/
struct stm32_adc_regspec {
const u32 dr;
@@ -236,6 +241,8 @@ struct stm32_adc_regspec {
const struct stm32_adc_regs exten;
const struct stm32_adc_regs extsel;
const struct stm32_adc_regs res;
+ const u32 smpr[2];
+ const struct stm32_adc_regs *smp_bits;
};
struct stm32_adc;
@@ -251,6 +258,7 @@ struct stm32_adc_regspec {
* @start_conv: routine to start conversions
* @stop_conv: routine to stop conversions
* @unprepare: optional unprepare routine (disable, power-down)
+ * @smp_cycles: programmable sampling time (ADC clock cycles)
*/
struct stm32_adc_cfg {
const struct stm32_adc_regspec *regs;
@@ -262,6 +270,7 @@ struct stm32_adc_cfg {
void (*start_conv)(struct stm32_adc *, bool dma);
void (*stop_conv)(struct stm32_adc *);
void (*unprepare)(struct stm32_adc *);
+ const unsigned int *smp_cycles;
};
/**
@@ -283,6 +292,7 @@ struct stm32_adc_cfg {
* @rx_dma_buf: dma rx buffer bus address
* @rx_buf_sz: dma rx buffer size
* @pcsel bitmask to preselect channels on some devices
+ * @smpr_val: sampling time settings (e.g. smpr1 / smpr2)
* @cal: optional calibration data on some devices
*/
struct stm32_adc {
@@ -303,6 +313,7 @@ struct stm32_adc {
dma_addr_t rx_dma_buf;
unsigned int rx_buf_sz;
u32 pcsel;
+ u32 smpr_val[2];
struct stm32_adc_calib cal;
};
@@ -431,6 +442,39 @@ struct stm32_adc_info {
{}, /* sentinel */
};
+/**
+ * stm32f4_smp_bits[] - describe sampling time register index & bit fields
+ * Sorted so it can be indexed by channel number.
+ */
+static const struct stm32_adc_regs stm32f4_smp_bits[] = {
+ /* STM32F4_ADC_SMPR2: smpr[] index, mask, shift for SMP0 to SMP9 */
+ { 1, GENMASK(2, 0), 0 },
+ { 1, GENMASK(5, 3), 3 },
+ { 1, GENMASK(8, 6), 6 },
+ { 1, GENMASK(11, 9), 9 },
+ { 1, GENMASK(14, 12), 12 },
+ { 1, GENMASK(17, 15), 15 },
+ { 1, GENMASK(20, 18), 18 },
+ { 1, GENMASK(23, 21), 21 },
+ { 1, GENMASK(26, 24), 24 },
+ { 1, GENMASK(29, 27), 27 },
+ /* STM32F4_ADC_SMPR1, smpr[] index, mask, shift for SMP10 to SMP18 */
+ { 0, GENMASK(2, 0), 0 },
+ { 0, GENMASK(5, 3), 3 },
+ { 0, GENMASK(8, 6), 6 },
+ { 0, GENMASK(11, 9), 9 },
+ { 0, GENMASK(14, 12), 12 },
+ { 0, GENMASK(17, 15), 15 },
+ { 0, GENMASK(20, 18), 18 },
+ { 0, GENMASK(23, 21), 21 },
+ { 0, GENMASK(26, 24), 24 },
+};
+
+/* STM32F4 programmable sampling time (ADC clock cycles) */
+static const unsigned int stm32f4_adc_smp_cycles[STM32_ADC_MAX_SMP + 1] = {
+ 3, 15, 28, 56, 84, 112, 144, 480,
+};
+
static const struct stm32_adc_regspec stm32f4_adc_regspec = {
.dr = STM32F4_ADC_DR,
.ier_eoc = { STM32F4_ADC_CR1, STM32F4_EOCIE },
@@ -440,6 +484,8 @@ struct stm32_adc_info {
.extsel = { STM32F4_ADC_CR2, STM32F4_EXTSEL_MASK,
STM32F4_EXTSEL_SHIFT },
.res = { STM32F4_ADC_CR1, STM32F4_RES_MASK, STM32F4_RES_SHIFT },
+ .smpr = { STM32F4_ADC_SMPR1, STM32F4_ADC_SMPR2 },
+ .smp_bits = stm32f4_smp_bits,
};
static const struct stm32_adc_regs stm32h7_sq[STM32_ADC_MAX_SQ + 1] = {
@@ -483,6 +529,40 @@ struct stm32_adc_info {
{},
};
+/**
+ * stm32h7_smp_bits - describe sampling time register index & bit fields
+ * Sorted so it can be indexed by channel number.
+ */
+static const struct stm32_adc_regs stm32h7_smp_bits[] = {
+ /* STM32H7_ADC_SMPR1, smpr[] index, mask, shift for SMP0 to SMP9 */
+ { 0, GENMASK(2, 0), 0 },
+ { 0, GENMASK(5, 3), 3 },
+ { 0, GENMASK(8, 6), 6 },
+ { 0, GENMASK(11, 9), 9 },
+ { 0, GENMASK(14, 12), 12 },
+ { 0, GENMASK(17, 15), 15 },
+ { 0, GENMASK(20, 18), 18 },
+ { 0, GENMASK(23, 21), 21 },
+ { 0, GENMASK(26, 24), 24 },
+ { 0, GENMASK(29, 27), 27 },
+ /* STM32H7_ADC_SMPR2, smpr[] index, mask, shift for SMP10 to SMP19 */
+ { 1, GENMASK(2, 0), 0 },
+ { 1, GENMASK(5, 3), 3 },
+ { 1, GENMASK(8, 6), 6 },
+ { 1, GENMASK(11, 9), 9 },
+ { 1, GENMASK(14, 12), 12 },
+ { 1, GENMASK(17, 15), 15 },
+ { 1, GENMASK(20, 18), 18 },
+ { 1, GENMASK(23, 21), 21 },
+ { 1, GENMASK(26, 24), 24 },
+ { 1, GENMASK(29, 27), 27 },
+};
+
+/* STM32H7 programmable sampling time (ADC clock cycles, rounded down) */
+static const unsigned int stm32h7_adc_smp_cycles[STM32_ADC_MAX_SMP + 1] = {
+ 1, 2, 8, 16, 32, 64, 387, 810,
+};
+
static const struct stm32_adc_regspec stm32h7_adc_regspec = {
.dr = STM32H7_ADC_DR,
.ier_eoc = { STM32H7_ADC_IER, STM32H7_EOCIE },
@@ -492,6 +572,8 @@ struct stm32_adc_info {
.extsel = { STM32H7_ADC_CFGR, STM32H7_EXTSEL_MASK,
STM32H7_EXTSEL_SHIFT },
.res = { STM32H7_ADC_CFGR, STM32H7_RES_MASK, STM32H7_RES_SHIFT },
+ .smpr = { STM32H7_ADC_SMPR1, STM32H7_ADC_SMPR2 },
+ .smp_bits = stm32h7_smp_bits,
};
/**
@@ -933,6 +1015,7 @@ static void stm32h7_adc_unprepare(struct stm32_adc *adc)
* @scan_mask: channels to be converted
*
* Conversion sequence :
+ * Apply sampling time settings for all channels.
* Configure ADC scan sequence based on selected channels in scan_mask.
* Add channels to SQR registers, from scan_mask LSB to MSB, then
* program sequence len.
@@ -946,6 +1029,10 @@ static int stm32_adc_conf_scan_seq(struct iio_dev *indio_dev,
u32 val, bit;
int i = 0;
+ /* Apply sampling time settings */
+ stm32_adc_writel(adc, adc->cfg->regs->smpr[0], adc->smpr_val[0]);
+ stm32_adc_writel(adc, adc->cfg->regs->smpr[1], adc->smpr_val[1]);
+
for_each_set_bit(bit, scan_mask, indio_dev->masklength) {
chan = indio_dev->channels + bit;
/*
@@ -1079,6 +1166,7 @@ static int stm32_adc_get_trig_pol(struct iio_dev *indio_dev,
* @res: conversion result
*
* The function performs a single conversion on a given channel:
+ * - Apply sampling time settings
* - Program sequencer with one channel (e.g. in SQ1 with len = 1)
* - Use SW trigger
* - Start conversion, then wait for interrupt completion.
@@ -1103,6 +1191,10 @@ static int stm32_adc_single_conv(struct iio_dev *indio_dev,
return ret;
}
+ /* Apply sampling time settings */
+ stm32_adc_writel(adc, regs->smpr[0], adc->smpr_val[0]);
+ stm32_adc_writel(adc, regs->smpr[1], adc->smpr_val[1]);
+
/* Program chan number in regular sequence (SQ1) */
val = stm32_adc_readl(adc, regs->sqr[1].reg);
val &= ~regs->sqr[1].mask;
@@ -1507,10 +1599,28 @@ static int stm32_adc_of_get_resolution(struct iio_dev *indio_dev)
return 0;
}
+static void stm32_adc_smpr_init(struct stm32_adc *adc, int channel, u32 smp_ns)
+{
+ const struct stm32_adc_regs *smpr = &adc->cfg->regs->smp_bits[channel];
+ u32 period_ns, shift = smpr->shift, mask = smpr->mask;
+ unsigned int smp, r = smpr->reg;
+
+ /* Determine sampling time (ADC clock cycles) */
+ period_ns = NSEC_PER_SEC / adc->common->rate;
+ for (smp = 0; smp <= STM32_ADC_MAX_SMP; smp++)
+ if ((period_ns * adc->cfg->smp_cycles[smp]) >= smp_ns)
+ break;
+ if (smp > STM32_ADC_MAX_SMP)
+ smp = STM32_ADC_MAX_SMP;
+
+ /* pre-build sampling time registers (e.g. smpr1, smpr2) */
+ adc->smpr_val[r] = (adc->smpr_val[r] & ~mask) | (smp << shift);
+}
+
static void stm32_adc_chan_init_one(struct iio_dev *indio_dev,
struct iio_chan_spec *chan,
const struct stm32_adc_chan_spec *channel,
- int scan_index)
+ int scan_index, u32 smp)
{
struct stm32_adc *adc = iio_priv(indio_dev);
@@ -1526,6 +1636,9 @@ static void stm32_adc_chan_init_one(struct iio_dev *indio_dev,
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);
}
@@ -1538,8 +1651,8 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
struct property *prop;
const __be32 *cur;
struct iio_chan_spec *channels;
- int scan_index = 0, num_channels;
- u32 val;
+ int scan_index = 0, num_channels, ret;
+ u32 val, smp = 0;
num_channels = of_property_count_u32_elems(node, "st,adc-channels");
if (num_channels < 0 ||
@@ -1548,6 +1661,13 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
return num_channels < 0 ? num_channels : -EINVAL;
}
+ /* Optional sample time is provided either for each, or all channels */
+ ret = of_property_count_u32_elems(node, "min-sample-time");
+ if (ret > 1 && ret != num_channels) {
+ dev_err(&indio_dev->dev, "Invalid min-sample-time\n");
+ return -EINVAL;
+ }
+
channels = devm_kcalloc(&indio_dev->dev, num_channels,
sizeof(struct iio_chan_spec), GFP_KERNEL);
if (!channels)
@@ -1558,9 +1678,13 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
dev_err(&indio_dev->dev, "Invalid channel %d\n", val);
return -EINVAL;
}
+
+ of_property_read_u32_index(node, "min-sample-time", scan_index,
+ &smp);
+
stm32_adc_chan_init_one(indio_dev, &channels[scan_index],
&adc_info->channels[val],
- scan_index);
+ scan_index, smp);
scan_index++;
}
@@ -1755,6 +1879,7 @@ static int stm32_adc_remove(struct platform_device *pdev)
.clk_required = true,
.start_conv = stm32f4_adc_start_conv,
.stop_conv = stm32f4_adc_stop_conv,
+ .smp_cycles = stm32f4_adc_smp_cycles,
};
static const struct stm32_adc_cfg stm32h7_adc_cfg = {
@@ -1766,6 +1891,7 @@ static int stm32_adc_remove(struct platform_device *pdev)
.stop_conv = stm32h7_adc_stop_conv,
.prepare = stm32h7_adc_prepare,
.unprepare = stm32h7_adc_unprepare,
+ .smp_cycles = stm32h7_adc_smp_cycles,
};
static const struct of_device_id stm32_adc_of_match[] = {
--
1.9.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [PATCH 3/3] iio: adc: stm32: add optional min-sample-time
2017-07-18 12:35 ` Fabrice Gasnier
(?)
@ 2017-07-23 11:00 ` Jonathan Cameron
-1 siblings, 0 replies; 39+ messages in thread
From: Jonathan Cameron @ 2017-07-23 11:00 UTC (permalink / raw)
To: Fabrice Gasnier
Cc: robh+dt, linux, mark.rutland, mcoquelin.stm32, alexandre.torgue,
lars, knaack.h, pmeerw, benjamin.gaignard, benjamin.gaignard,
linux-iio, devicetree, linux-arm-kernel, linux-kernel
On Tue, 18 Jul 2017 14:35:32 +0200
Fabrice Gasnier <fabrice.gasnier@st.com> wrote:
> STM32 ADC allows each channel to be sampled with a different sampling time,
> by setting SMPR registers. Basically, value depends on local electrical
> properties. Selecting correct value for sampling time highly depends on
> analog source impedance. There is a manual that may help in this process:
> 'How to get the best ADC accuracy in STM32...'
>
> This patch allows to configure min-sample-time via device tree, either for:
> - all channels at once:
> min-sample-time = <10000>; /* nanosecs */
>
> - independently for each channel (must match "st,adc-channels" list):
> st,adc-channels = <0 1>;
> min-sample-time = <5000 10000>; /* nanosecs */
>
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
On question inline which may well just be down to me missing what
happens when you query index element 2 from a 1 element device tree
array.
> ---
> drivers/iio/adc/stm32-adc.c | 134 ++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 130 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
> index 5bfcc1f..e6f6b5e 100644
> --- a/drivers/iio/adc/stm32-adc.c
> +++ b/drivers/iio/adc/stm32-adc.c
> @@ -83,6 +83,8 @@
> #define STM32H7_ADC_IER 0x04
> #define STM32H7_ADC_CR 0x08
> #define STM32H7_ADC_CFGR 0x0C
> +#define STM32H7_ADC_SMPR1 0x14
> +#define STM32H7_ADC_SMPR2 0x18
> #define STM32H7_ADC_PCSEL 0x1C
> #define STM32H7_ADC_SQR1 0x30
> #define STM32H7_ADC_SQR2 0x34
> @@ -151,6 +153,7 @@ enum stm32h7_adc_dmngt {
> #define STM32H7_BOOST_CLKRATE 20000000UL
>
> #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
> #define STM32_ADC_TIMEOUT (msecs_to_jiffies(STM32_ADC_TIMEOUT_US / 1000))
>
> @@ -227,6 +230,8 @@ struct stm32_adc_regs {
> * @exten: trigger control register & bitfield
> * @extsel: trigger selection register & bitfield
> * @res: resolution selection register & bitfield
> + * @smpr: smpr1 & smpr2 registers offset array
> + * @smp_bits: smpr1 & smpr2 index and bitfields
> */
> struct stm32_adc_regspec {
> const u32 dr;
> @@ -236,6 +241,8 @@ struct stm32_adc_regspec {
> const struct stm32_adc_regs exten;
> const struct stm32_adc_regs extsel;
> const struct stm32_adc_regs res;
> + const u32 smpr[2];
> + const struct stm32_adc_regs *smp_bits;
> };
>
> struct stm32_adc;
> @@ -251,6 +258,7 @@ struct stm32_adc_regspec {
> * @start_conv: routine to start conversions
> * @stop_conv: routine to stop conversions
> * @unprepare: optional unprepare routine (disable, power-down)
> + * @smp_cycles: programmable sampling time (ADC clock cycles)
> */
> struct stm32_adc_cfg {
> const struct stm32_adc_regspec *regs;
> @@ -262,6 +270,7 @@ struct stm32_adc_cfg {
> void (*start_conv)(struct stm32_adc *, bool dma);
> void (*stop_conv)(struct stm32_adc *);
> void (*unprepare)(struct stm32_adc *);
> + const unsigned int *smp_cycles;
> };
>
> /**
> @@ -283,6 +292,7 @@ struct stm32_adc_cfg {
> * @rx_dma_buf: dma rx buffer bus address
> * @rx_buf_sz: dma rx buffer size
> * @pcsel bitmask to preselect channels on some devices
> + * @smpr_val: sampling time settings (e.g. smpr1 / smpr2)
> * @cal: optional calibration data on some devices
> */
> struct stm32_adc {
> @@ -303,6 +313,7 @@ struct stm32_adc {
> dma_addr_t rx_dma_buf;
> unsigned int rx_buf_sz;
> u32 pcsel;
> + u32 smpr_val[2];
> struct stm32_adc_calib cal;
> };
>
> @@ -431,6 +442,39 @@ struct stm32_adc_info {
> {}, /* sentinel */
> };
>
> +/**
> + * stm32f4_smp_bits[] - describe sampling time register index & bit fields
> + * Sorted so it can be indexed by channel number.
> + */
> +static const struct stm32_adc_regs stm32f4_smp_bits[] = {
> + /* STM32F4_ADC_SMPR2: smpr[] index, mask, shift for SMP0 to SMP9 */
> + { 1, GENMASK(2, 0), 0 },
> + { 1, GENMASK(5, 3), 3 },
> + { 1, GENMASK(8, 6), 6 },
> + { 1, GENMASK(11, 9), 9 },
> + { 1, GENMASK(14, 12), 12 },
> + { 1, GENMASK(17, 15), 15 },
> + { 1, GENMASK(20, 18), 18 },
> + { 1, GENMASK(23, 21), 21 },
> + { 1, GENMASK(26, 24), 24 },
> + { 1, GENMASK(29, 27), 27 },
> + /* STM32F4_ADC_SMPR1, smpr[] index, mask, shift for SMP10 to SMP18 */
> + { 0, GENMASK(2, 0), 0 },
> + { 0, GENMASK(5, 3), 3 },
> + { 0, GENMASK(8, 6), 6 },
> + { 0, GENMASK(11, 9), 9 },
> + { 0, GENMASK(14, 12), 12 },
> + { 0, GENMASK(17, 15), 15 },
> + { 0, GENMASK(20, 18), 18 },
> + { 0, GENMASK(23, 21), 21 },
> + { 0, GENMASK(26, 24), 24 },
> +};
> +
> +/* STM32F4 programmable sampling time (ADC clock cycles) */
> +static const unsigned int stm32f4_adc_smp_cycles[STM32_ADC_MAX_SMP + 1] = {
> + 3, 15, 28, 56, 84, 112, 144, 480,
> +};
> +
> static const struct stm32_adc_regspec stm32f4_adc_regspec = {
> .dr = STM32F4_ADC_DR,
> .ier_eoc = { STM32F4_ADC_CR1, STM32F4_EOCIE },
> @@ -440,6 +484,8 @@ struct stm32_adc_info {
> .extsel = { STM32F4_ADC_CR2, STM32F4_EXTSEL_MASK,
> STM32F4_EXTSEL_SHIFT },
> .res = { STM32F4_ADC_CR1, STM32F4_RES_MASK, STM32F4_RES_SHIFT },
> + .smpr = { STM32F4_ADC_SMPR1, STM32F4_ADC_SMPR2 },
> + .smp_bits = stm32f4_smp_bits,
> };
>
> static const struct stm32_adc_regs stm32h7_sq[STM32_ADC_MAX_SQ + 1] = {
> @@ -483,6 +529,40 @@ struct stm32_adc_info {
> {},
> };
>
> +/**
> + * stm32h7_smp_bits - describe sampling time register index & bit fields
> + * Sorted so it can be indexed by channel number.
> + */
> +static const struct stm32_adc_regs stm32h7_smp_bits[] = {
> + /* STM32H7_ADC_SMPR1, smpr[] index, mask, shift for SMP0 to SMP9 */
> + { 0, GENMASK(2, 0), 0 },
> + { 0, GENMASK(5, 3), 3 },
> + { 0, GENMASK(8, 6), 6 },
> + { 0, GENMASK(11, 9), 9 },
> + { 0, GENMASK(14, 12), 12 },
> + { 0, GENMASK(17, 15), 15 },
> + { 0, GENMASK(20, 18), 18 },
> + { 0, GENMASK(23, 21), 21 },
> + { 0, GENMASK(26, 24), 24 },
> + { 0, GENMASK(29, 27), 27 },
> + /* STM32H7_ADC_SMPR2, smpr[] index, mask, shift for SMP10 to SMP19 */
> + { 1, GENMASK(2, 0), 0 },
> + { 1, GENMASK(5, 3), 3 },
> + { 1, GENMASK(8, 6), 6 },
> + { 1, GENMASK(11, 9), 9 },
> + { 1, GENMASK(14, 12), 12 },
> + { 1, GENMASK(17, 15), 15 },
> + { 1, GENMASK(20, 18), 18 },
> + { 1, GENMASK(23, 21), 21 },
> + { 1, GENMASK(26, 24), 24 },
> + { 1, GENMASK(29, 27), 27 },
> +};
> +
> +/* STM32H7 programmable sampling time (ADC clock cycles, rounded down) */
> +static const unsigned int stm32h7_adc_smp_cycles[STM32_ADC_MAX_SMP + 1] = {
> + 1, 2, 8, 16, 32, 64, 387, 810,
> +};
> +
> static const struct stm32_adc_regspec stm32h7_adc_regspec = {
> .dr = STM32H7_ADC_DR,
> .ier_eoc = { STM32H7_ADC_IER, STM32H7_EOCIE },
> @@ -492,6 +572,8 @@ struct stm32_adc_info {
> .extsel = { STM32H7_ADC_CFGR, STM32H7_EXTSEL_MASK,
> STM32H7_EXTSEL_SHIFT },
> .res = { STM32H7_ADC_CFGR, STM32H7_RES_MASK, STM32H7_RES_SHIFT },
> + .smpr = { STM32H7_ADC_SMPR1, STM32H7_ADC_SMPR2 },
> + .smp_bits = stm32h7_smp_bits,
> };
>
> /**
> @@ -933,6 +1015,7 @@ static void stm32h7_adc_unprepare(struct stm32_adc *adc)
> * @scan_mask: channels to be converted
> *
> * Conversion sequence :
> + * Apply sampling time settings for all channels.
> * Configure ADC scan sequence based on selected channels in scan_mask.
> * Add channels to SQR registers, from scan_mask LSB to MSB, then
> * program sequence len.
> @@ -946,6 +1029,10 @@ static int stm32_adc_conf_scan_seq(struct iio_dev *indio_dev,
> u32 val, bit;
> int i = 0;
>
> + /* Apply sampling time settings */
> + stm32_adc_writel(adc, adc->cfg->regs->smpr[0], adc->smpr_val[0]);
> + stm32_adc_writel(adc, adc->cfg->regs->smpr[1], adc->smpr_val[1]);
> +
> for_each_set_bit(bit, scan_mask, indio_dev->masklength) {
> chan = indio_dev->channels + bit;
> /*
> @@ -1079,6 +1166,7 @@ static int stm32_adc_get_trig_pol(struct iio_dev *indio_dev,
> * @res: conversion result
> *
> * The function performs a single conversion on a given channel:
> + * - Apply sampling time settings
> * - Program sequencer with one channel (e.g. in SQ1 with len = 1)
> * - Use SW trigger
> * - Start conversion, then wait for interrupt completion.
> @@ -1103,6 +1191,10 @@ static int stm32_adc_single_conv(struct iio_dev *indio_dev,
> return ret;
> }
>
> + /* Apply sampling time settings */
> + stm32_adc_writel(adc, regs->smpr[0], adc->smpr_val[0]);
> + stm32_adc_writel(adc, regs->smpr[1], adc->smpr_val[1]);
> +
> /* Program chan number in regular sequence (SQ1) */
> val = stm32_adc_readl(adc, regs->sqr[1].reg);
> val &= ~regs->sqr[1].mask;
> @@ -1507,10 +1599,28 @@ static int stm32_adc_of_get_resolution(struct iio_dev *indio_dev)
> return 0;
> }
>
> +static void stm32_adc_smpr_init(struct stm32_adc *adc, int channel, u32 smp_ns)
> +{
> + const struct stm32_adc_regs *smpr = &adc->cfg->regs->smp_bits[channel];
> + u32 period_ns, shift = smpr->shift, mask = smpr->mask;
> + unsigned int smp, r = smpr->reg;
> +
> + /* Determine sampling time (ADC clock cycles) */
> + period_ns = NSEC_PER_SEC / adc->common->rate;
> + for (smp = 0; smp <= STM32_ADC_MAX_SMP; smp++)
> + if ((period_ns * adc->cfg->smp_cycles[smp]) >= smp_ns)
> + break;
> + if (smp > STM32_ADC_MAX_SMP)
> + smp = STM32_ADC_MAX_SMP;
> +
> + /* pre-build sampling time registers (e.g. smpr1, smpr2) */
> + adc->smpr_val[r] = (adc->smpr_val[r] & ~mask) | (smp << shift);
> +}
> +
> static void stm32_adc_chan_init_one(struct iio_dev *indio_dev,
> struct iio_chan_spec *chan,
> const struct stm32_adc_chan_spec *channel,
> - int scan_index)
> + int scan_index, u32 smp)
> {
> struct stm32_adc *adc = iio_priv(indio_dev);
>
> @@ -1526,6 +1636,9 @@ static void stm32_adc_chan_init_one(struct iio_dev *indio_dev,
> 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);
> }
> @@ -1538,8 +1651,8 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
> struct property *prop;
> const __be32 *cur;
> struct iio_chan_spec *channels;
> - int scan_index = 0, num_channels;
> - u32 val;
> + int scan_index = 0, num_channels, ret;
> + u32 val, smp = 0;
>
> num_channels = of_property_count_u32_elems(node, "st,adc-channels");
> if (num_channels < 0 ||
> @@ -1548,6 +1661,13 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
> return num_channels < 0 ? num_channels : -EINVAL;
> }
>
> + /* Optional sample time is provided either for each, or all channels */
> + ret = of_property_count_u32_elems(node, "min-sample-time");
> + if (ret > 1 && ret != num_channels) {
> + dev_err(&indio_dev->dev, "Invalid min-sample-time\n");
> + return -EINVAL;
> + }
> +
> channels = devm_kcalloc(&indio_dev->dev, num_channels,
> sizeof(struct iio_chan_spec), GFP_KERNEL);
> if (!channels)
> @@ -1558,9 +1678,13 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
> dev_err(&indio_dev->dev, "Invalid channel %d\n", val);
> return -EINVAL;
> }
> +
> + of_property_read_u32_index(node, "min-sample-time", scan_index,
> + &smp);
> +
I might be missing something, but doesn't this fail for the single shared
value case?
> stm32_adc_chan_init_one(indio_dev, &channels[scan_index],
> &adc_info->channels[val],
> - scan_index);
> + scan_index, smp);
> scan_index++;
> }
>
> @@ -1755,6 +1879,7 @@ static int stm32_adc_remove(struct platform_device *pdev)
> .clk_required = true,
> .start_conv = stm32f4_adc_start_conv,
> .stop_conv = stm32f4_adc_stop_conv,
> + .smp_cycles = stm32f4_adc_smp_cycles,
> };
>
> static const struct stm32_adc_cfg stm32h7_adc_cfg = {
> @@ -1766,6 +1891,7 @@ static int stm32_adc_remove(struct platform_device *pdev)
> .stop_conv = stm32h7_adc_stop_conv,
> .prepare = stm32h7_adc_prepare,
> .unprepare = stm32h7_adc_unprepare,
> + .smp_cycles = stm32h7_adc_smp_cycles,
> };
>
> static const struct of_device_id stm32_adc_of_match[] = {
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [PATCH 3/3] iio: adc: stm32: add optional min-sample-time
@ 2017-07-23 11:00 ` Jonathan Cameron
0 siblings, 0 replies; 39+ messages in thread
From: Jonathan Cameron @ 2017-07-23 11:00 UTC (permalink / raw)
To: Fabrice Gasnier
Cc: robh+dt, linux, mark.rutland, mcoquelin.stm32, alexandre.torgue,
lars, knaack.h, pmeerw, benjamin.gaignard, benjamin.gaignard,
linux-iio, devicetree, linux-arm-kernel, linux-kernel
On Tue, 18 Jul 2017 14:35:32 +0200
Fabrice Gasnier <fabrice.gasnier@st.com> wrote:
> STM32 ADC allows each channel to be sampled with a different sampling time,
> by setting SMPR registers. Basically, value depends on local electrical
> properties. Selecting correct value for sampling time highly depends on
> analog source impedance. There is a manual that may help in this process:
> 'How to get the best ADC accuracy in STM32...'
>
> This patch allows to configure min-sample-time via device tree, either for:
> - all channels at once:
> min-sample-time = <10000>; /* nanosecs */
>
> - independently for each channel (must match "st,adc-channels" list):
> st,adc-channels = <0 1>;
> min-sample-time = <5000 10000>; /* nanosecs */
>
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
On question inline which may well just be down to me missing what
happens when you query index element 2 from a 1 element device tree
array.
> ---
> drivers/iio/adc/stm32-adc.c | 134 ++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 130 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
> index 5bfcc1f..e6f6b5e 100644
> --- a/drivers/iio/adc/stm32-adc.c
> +++ b/drivers/iio/adc/stm32-adc.c
> @@ -83,6 +83,8 @@
> #define STM32H7_ADC_IER 0x04
> #define STM32H7_ADC_CR 0x08
> #define STM32H7_ADC_CFGR 0x0C
> +#define STM32H7_ADC_SMPR1 0x14
> +#define STM32H7_ADC_SMPR2 0x18
> #define STM32H7_ADC_PCSEL 0x1C
> #define STM32H7_ADC_SQR1 0x30
> #define STM32H7_ADC_SQR2 0x34
> @@ -151,6 +153,7 @@ enum stm32h7_adc_dmngt {
> #define STM32H7_BOOST_CLKRATE 20000000UL
>
> #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
> #define STM32_ADC_TIMEOUT (msecs_to_jiffies(STM32_ADC_TIMEOUT_US / 1000))
>
> @@ -227,6 +230,8 @@ struct stm32_adc_regs {
> * @exten: trigger control register & bitfield
> * @extsel: trigger selection register & bitfield
> * @res: resolution selection register & bitfield
> + * @smpr: smpr1 & smpr2 registers offset array
> + * @smp_bits: smpr1 & smpr2 index and bitfields
> */
> struct stm32_adc_regspec {
> const u32 dr;
> @@ -236,6 +241,8 @@ struct stm32_adc_regspec {
> const struct stm32_adc_regs exten;
> const struct stm32_adc_regs extsel;
> const struct stm32_adc_regs res;
> + const u32 smpr[2];
> + const struct stm32_adc_regs *smp_bits;
> };
>
> struct stm32_adc;
> @@ -251,6 +258,7 @@ struct stm32_adc_regspec {
> * @start_conv: routine to start conversions
> * @stop_conv: routine to stop conversions
> * @unprepare: optional unprepare routine (disable, power-down)
> + * @smp_cycles: programmable sampling time (ADC clock cycles)
> */
> struct stm32_adc_cfg {
> const struct stm32_adc_regspec *regs;
> @@ -262,6 +270,7 @@ struct stm32_adc_cfg {
> void (*start_conv)(struct stm32_adc *, bool dma);
> void (*stop_conv)(struct stm32_adc *);
> void (*unprepare)(struct stm32_adc *);
> + const unsigned int *smp_cycles;
> };
>
> /**
> @@ -283,6 +292,7 @@ struct stm32_adc_cfg {
> * @rx_dma_buf: dma rx buffer bus address
> * @rx_buf_sz: dma rx buffer size
> * @pcsel bitmask to preselect channels on some devices
> + * @smpr_val: sampling time settings (e.g. smpr1 / smpr2)
> * @cal: optional calibration data on some devices
> */
> struct stm32_adc {
> @@ -303,6 +313,7 @@ struct stm32_adc {
> dma_addr_t rx_dma_buf;
> unsigned int rx_buf_sz;
> u32 pcsel;
> + u32 smpr_val[2];
> struct stm32_adc_calib cal;
> };
>
> @@ -431,6 +442,39 @@ struct stm32_adc_info {
> {}, /* sentinel */
> };
>
> +/**
> + * stm32f4_smp_bits[] - describe sampling time register index & bit fields
> + * Sorted so it can be indexed by channel number.
> + */
> +static const struct stm32_adc_regs stm32f4_smp_bits[] = {
> + /* STM32F4_ADC_SMPR2: smpr[] index, mask, shift for SMP0 to SMP9 */
> + { 1, GENMASK(2, 0), 0 },
> + { 1, GENMASK(5, 3), 3 },
> + { 1, GENMASK(8, 6), 6 },
> + { 1, GENMASK(11, 9), 9 },
> + { 1, GENMASK(14, 12), 12 },
> + { 1, GENMASK(17, 15), 15 },
> + { 1, GENMASK(20, 18), 18 },
> + { 1, GENMASK(23, 21), 21 },
> + { 1, GENMASK(26, 24), 24 },
> + { 1, GENMASK(29, 27), 27 },
> + /* STM32F4_ADC_SMPR1, smpr[] index, mask, shift for SMP10 to SMP18 */
> + { 0, GENMASK(2, 0), 0 },
> + { 0, GENMASK(5, 3), 3 },
> + { 0, GENMASK(8, 6), 6 },
> + { 0, GENMASK(11, 9), 9 },
> + { 0, GENMASK(14, 12), 12 },
> + { 0, GENMASK(17, 15), 15 },
> + { 0, GENMASK(20, 18), 18 },
> + { 0, GENMASK(23, 21), 21 },
> + { 0, GENMASK(26, 24), 24 },
> +};
> +
> +/* STM32F4 programmable sampling time (ADC clock cycles) */
> +static const unsigned int stm32f4_adc_smp_cycles[STM32_ADC_MAX_SMP + 1] = {
> + 3, 15, 28, 56, 84, 112, 144, 480,
> +};
> +
> static const struct stm32_adc_regspec stm32f4_adc_regspec = {
> .dr = STM32F4_ADC_DR,
> .ier_eoc = { STM32F4_ADC_CR1, STM32F4_EOCIE },
> @@ -440,6 +484,8 @@ struct stm32_adc_info {
> .extsel = { STM32F4_ADC_CR2, STM32F4_EXTSEL_MASK,
> STM32F4_EXTSEL_SHIFT },
> .res = { STM32F4_ADC_CR1, STM32F4_RES_MASK, STM32F4_RES_SHIFT },
> + .smpr = { STM32F4_ADC_SMPR1, STM32F4_ADC_SMPR2 },
> + .smp_bits = stm32f4_smp_bits,
> };
>
> static const struct stm32_adc_regs stm32h7_sq[STM32_ADC_MAX_SQ + 1] = {
> @@ -483,6 +529,40 @@ struct stm32_adc_info {
> {},
> };
>
> +/**
> + * stm32h7_smp_bits - describe sampling time register index & bit fields
> + * Sorted so it can be indexed by channel number.
> + */
> +static const struct stm32_adc_regs stm32h7_smp_bits[] = {
> + /* STM32H7_ADC_SMPR1, smpr[] index, mask, shift for SMP0 to SMP9 */
> + { 0, GENMASK(2, 0), 0 },
> + { 0, GENMASK(5, 3), 3 },
> + { 0, GENMASK(8, 6), 6 },
> + { 0, GENMASK(11, 9), 9 },
> + { 0, GENMASK(14, 12), 12 },
> + { 0, GENMASK(17, 15), 15 },
> + { 0, GENMASK(20, 18), 18 },
> + { 0, GENMASK(23, 21), 21 },
> + { 0, GENMASK(26, 24), 24 },
> + { 0, GENMASK(29, 27), 27 },
> + /* STM32H7_ADC_SMPR2, smpr[] index, mask, shift for SMP10 to SMP19 */
> + { 1, GENMASK(2, 0), 0 },
> + { 1, GENMASK(5, 3), 3 },
> + { 1, GENMASK(8, 6), 6 },
> + { 1, GENMASK(11, 9), 9 },
> + { 1, GENMASK(14, 12), 12 },
> + { 1, GENMASK(17, 15), 15 },
> + { 1, GENMASK(20, 18), 18 },
> + { 1, GENMASK(23, 21), 21 },
> + { 1, GENMASK(26, 24), 24 },
> + { 1, GENMASK(29, 27), 27 },
> +};
> +
> +/* STM32H7 programmable sampling time (ADC clock cycles, rounded down) */
> +static const unsigned int stm32h7_adc_smp_cycles[STM32_ADC_MAX_SMP + 1] = {
> + 1, 2, 8, 16, 32, 64, 387, 810,
> +};
> +
> static const struct stm32_adc_regspec stm32h7_adc_regspec = {
> .dr = STM32H7_ADC_DR,
> .ier_eoc = { STM32H7_ADC_IER, STM32H7_EOCIE },
> @@ -492,6 +572,8 @@ struct stm32_adc_info {
> .extsel = { STM32H7_ADC_CFGR, STM32H7_EXTSEL_MASK,
> STM32H7_EXTSEL_SHIFT },
> .res = { STM32H7_ADC_CFGR, STM32H7_RES_MASK, STM32H7_RES_SHIFT },
> + .smpr = { STM32H7_ADC_SMPR1, STM32H7_ADC_SMPR2 },
> + .smp_bits = stm32h7_smp_bits,
> };
>
> /**
> @@ -933,6 +1015,7 @@ static void stm32h7_adc_unprepare(struct stm32_adc *adc)
> * @scan_mask: channels to be converted
> *
> * Conversion sequence :
> + * Apply sampling time settings for all channels.
> * Configure ADC scan sequence based on selected channels in scan_mask.
> * Add channels to SQR registers, from scan_mask LSB to MSB, then
> * program sequence len.
> @@ -946,6 +1029,10 @@ static int stm32_adc_conf_scan_seq(struct iio_dev *indio_dev,
> u32 val, bit;
> int i = 0;
>
> + /* Apply sampling time settings */
> + stm32_adc_writel(adc, adc->cfg->regs->smpr[0], adc->smpr_val[0]);
> + stm32_adc_writel(adc, adc->cfg->regs->smpr[1], adc->smpr_val[1]);
> +
> for_each_set_bit(bit, scan_mask, indio_dev->masklength) {
> chan = indio_dev->channels + bit;
> /*
> @@ -1079,6 +1166,7 @@ static int stm32_adc_get_trig_pol(struct iio_dev *indio_dev,
> * @res: conversion result
> *
> * The function performs a single conversion on a given channel:
> + * - Apply sampling time settings
> * - Program sequencer with one channel (e.g. in SQ1 with len = 1)
> * - Use SW trigger
> * - Start conversion, then wait for interrupt completion.
> @@ -1103,6 +1191,10 @@ static int stm32_adc_single_conv(struct iio_dev *indio_dev,
> return ret;
> }
>
> + /* Apply sampling time settings */
> + stm32_adc_writel(adc, regs->smpr[0], adc->smpr_val[0]);
> + stm32_adc_writel(adc, regs->smpr[1], adc->smpr_val[1]);
> +
> /* Program chan number in regular sequence (SQ1) */
> val = stm32_adc_readl(adc, regs->sqr[1].reg);
> val &= ~regs->sqr[1].mask;
> @@ -1507,10 +1599,28 @@ static int stm32_adc_of_get_resolution(struct iio_dev *indio_dev)
> return 0;
> }
>
> +static void stm32_adc_smpr_init(struct stm32_adc *adc, int channel, u32 smp_ns)
> +{
> + const struct stm32_adc_regs *smpr = &adc->cfg->regs->smp_bits[channel];
> + u32 period_ns, shift = smpr->shift, mask = smpr->mask;
> + unsigned int smp, r = smpr->reg;
> +
> + /* Determine sampling time (ADC clock cycles) */
> + period_ns = NSEC_PER_SEC / adc->common->rate;
> + for (smp = 0; smp <= STM32_ADC_MAX_SMP; smp++)
> + if ((period_ns * adc->cfg->smp_cycles[smp]) >= smp_ns)
> + break;
> + if (smp > STM32_ADC_MAX_SMP)
> + smp = STM32_ADC_MAX_SMP;
> +
> + /* pre-build sampling time registers (e.g. smpr1, smpr2) */
> + adc->smpr_val[r] = (adc->smpr_val[r] & ~mask) | (smp << shift);
> +}
> +
> static void stm32_adc_chan_init_one(struct iio_dev *indio_dev,
> struct iio_chan_spec *chan,
> const struct stm32_adc_chan_spec *channel,
> - int scan_index)
> + int scan_index, u32 smp)
> {
> struct stm32_adc *adc = iio_priv(indio_dev);
>
> @@ -1526,6 +1636,9 @@ static void stm32_adc_chan_init_one(struct iio_dev *indio_dev,
> 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);
> }
> @@ -1538,8 +1651,8 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
> struct property *prop;
> const __be32 *cur;
> struct iio_chan_spec *channels;
> - int scan_index = 0, num_channels;
> - u32 val;
> + int scan_index = 0, num_channels, ret;
> + u32 val, smp = 0;
>
> num_channels = of_property_count_u32_elems(node, "st,adc-channels");
> if (num_channels < 0 ||
> @@ -1548,6 +1661,13 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
> return num_channels < 0 ? num_channels : -EINVAL;
> }
>
> + /* Optional sample time is provided either for each, or all channels */
> + ret = of_property_count_u32_elems(node, "min-sample-time");
> + if (ret > 1 && ret != num_channels) {
> + dev_err(&indio_dev->dev, "Invalid min-sample-time\n");
> + return -EINVAL;
> + }
> +
> channels = devm_kcalloc(&indio_dev->dev, num_channels,
> sizeof(struct iio_chan_spec), GFP_KERNEL);
> if (!channels)
> @@ -1558,9 +1678,13 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
> dev_err(&indio_dev->dev, "Invalid channel %d\n", val);
> return -EINVAL;
> }
> +
> + of_property_read_u32_index(node, "min-sample-time", scan_index,
> + &smp);
> +
I might be missing something, but doesn't this fail for the single shared
value case?
> stm32_adc_chan_init_one(indio_dev, &channels[scan_index],
> &adc_info->channels[val],
> - scan_index);
> + scan_index, smp);
> scan_index++;
> }
>
> @@ -1755,6 +1879,7 @@ static int stm32_adc_remove(struct platform_device *pdev)
> .clk_required = true,
> .start_conv = stm32f4_adc_start_conv,
> .stop_conv = stm32f4_adc_stop_conv,
> + .smp_cycles = stm32f4_adc_smp_cycles,
> };
>
> static const struct stm32_adc_cfg stm32h7_adc_cfg = {
> @@ -1766,6 +1891,7 @@ static int stm32_adc_remove(struct platform_device *pdev)
> .stop_conv = stm32h7_adc_stop_conv,
> .prepare = stm32h7_adc_prepare,
> .unprepare = stm32h7_adc_unprepare,
> + .smp_cycles = stm32h7_adc_smp_cycles,
> };
>
> static const struct of_device_id stm32_adc_of_match[] = {
^ permalink raw reply [flat|nested] 39+ messages in thread* [PATCH 3/3] iio: adc: stm32: add optional min-sample-time
@ 2017-07-23 11:00 ` Jonathan Cameron
0 siblings, 0 replies; 39+ messages in thread
From: Jonathan Cameron @ 2017-07-23 11:00 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, 18 Jul 2017 14:35:32 +0200
Fabrice Gasnier <fabrice.gasnier@st.com> wrote:
> STM32 ADC allows each channel to be sampled with a different sampling time,
> by setting SMPR registers. Basically, value depends on local electrical
> properties. Selecting correct value for sampling time highly depends on
> analog source impedance. There is a manual that may help in this process:
> 'How to get the best ADC accuracy in STM32...'
>
> This patch allows to configure min-sample-time via device tree, either for:
> - all channels at once:
> min-sample-time = <10000>; /* nanosecs */
>
> - independently for each channel (must match "st,adc-channels" list):
> st,adc-channels = <0 1>;
> min-sample-time = <5000 10000>; /* nanosecs */
>
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
On question inline which may well just be down to me missing what
happens when you query index element 2 from a 1 element device tree
array.
> ---
> drivers/iio/adc/stm32-adc.c | 134 ++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 130 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
> index 5bfcc1f..e6f6b5e 100644
> --- a/drivers/iio/adc/stm32-adc.c
> +++ b/drivers/iio/adc/stm32-adc.c
> @@ -83,6 +83,8 @@
> #define STM32H7_ADC_IER 0x04
> #define STM32H7_ADC_CR 0x08
> #define STM32H7_ADC_CFGR 0x0C
> +#define STM32H7_ADC_SMPR1 0x14
> +#define STM32H7_ADC_SMPR2 0x18
> #define STM32H7_ADC_PCSEL 0x1C
> #define STM32H7_ADC_SQR1 0x30
> #define STM32H7_ADC_SQR2 0x34
> @@ -151,6 +153,7 @@ enum stm32h7_adc_dmngt {
> #define STM32H7_BOOST_CLKRATE 20000000UL
>
> #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
> #define STM32_ADC_TIMEOUT (msecs_to_jiffies(STM32_ADC_TIMEOUT_US / 1000))
>
> @@ -227,6 +230,8 @@ struct stm32_adc_regs {
> * @exten: trigger control register & bitfield
> * @extsel: trigger selection register & bitfield
> * @res: resolution selection register & bitfield
> + * @smpr: smpr1 & smpr2 registers offset array
> + * @smp_bits: smpr1 & smpr2 index and bitfields
> */
> struct stm32_adc_regspec {
> const u32 dr;
> @@ -236,6 +241,8 @@ struct stm32_adc_regspec {
> const struct stm32_adc_regs exten;
> const struct stm32_adc_regs extsel;
> const struct stm32_adc_regs res;
> + const u32 smpr[2];
> + const struct stm32_adc_regs *smp_bits;
> };
>
> struct stm32_adc;
> @@ -251,6 +258,7 @@ struct stm32_adc_regspec {
> * @start_conv: routine to start conversions
> * @stop_conv: routine to stop conversions
> * @unprepare: optional unprepare routine (disable, power-down)
> + * @smp_cycles: programmable sampling time (ADC clock cycles)
> */
> struct stm32_adc_cfg {
> const struct stm32_adc_regspec *regs;
> @@ -262,6 +270,7 @@ struct stm32_adc_cfg {
> void (*start_conv)(struct stm32_adc *, bool dma);
> void (*stop_conv)(struct stm32_adc *);
> void (*unprepare)(struct stm32_adc *);
> + const unsigned int *smp_cycles;
> };
>
> /**
> @@ -283,6 +292,7 @@ struct stm32_adc_cfg {
> * @rx_dma_buf: dma rx buffer bus address
> * @rx_buf_sz: dma rx buffer size
> * @pcsel bitmask to preselect channels on some devices
> + * @smpr_val: sampling time settings (e.g. smpr1 / smpr2)
> * @cal: optional calibration data on some devices
> */
> struct stm32_adc {
> @@ -303,6 +313,7 @@ struct stm32_adc {
> dma_addr_t rx_dma_buf;
> unsigned int rx_buf_sz;
> u32 pcsel;
> + u32 smpr_val[2];
> struct stm32_adc_calib cal;
> };
>
> @@ -431,6 +442,39 @@ struct stm32_adc_info {
> {}, /* sentinel */
> };
>
> +/**
> + * stm32f4_smp_bits[] - describe sampling time register index & bit fields
> + * Sorted so it can be indexed by channel number.
> + */
> +static const struct stm32_adc_regs stm32f4_smp_bits[] = {
> + /* STM32F4_ADC_SMPR2: smpr[] index, mask, shift for SMP0 to SMP9 */
> + { 1, GENMASK(2, 0), 0 },
> + { 1, GENMASK(5, 3), 3 },
> + { 1, GENMASK(8, 6), 6 },
> + { 1, GENMASK(11, 9), 9 },
> + { 1, GENMASK(14, 12), 12 },
> + { 1, GENMASK(17, 15), 15 },
> + { 1, GENMASK(20, 18), 18 },
> + { 1, GENMASK(23, 21), 21 },
> + { 1, GENMASK(26, 24), 24 },
> + { 1, GENMASK(29, 27), 27 },
> + /* STM32F4_ADC_SMPR1, smpr[] index, mask, shift for SMP10 to SMP18 */
> + { 0, GENMASK(2, 0), 0 },
> + { 0, GENMASK(5, 3), 3 },
> + { 0, GENMASK(8, 6), 6 },
> + { 0, GENMASK(11, 9), 9 },
> + { 0, GENMASK(14, 12), 12 },
> + { 0, GENMASK(17, 15), 15 },
> + { 0, GENMASK(20, 18), 18 },
> + { 0, GENMASK(23, 21), 21 },
> + { 0, GENMASK(26, 24), 24 },
> +};
> +
> +/* STM32F4 programmable sampling time (ADC clock cycles) */
> +static const unsigned int stm32f4_adc_smp_cycles[STM32_ADC_MAX_SMP + 1] = {
> + 3, 15, 28, 56, 84, 112, 144, 480,
> +};
> +
> static const struct stm32_adc_regspec stm32f4_adc_regspec = {
> .dr = STM32F4_ADC_DR,
> .ier_eoc = { STM32F4_ADC_CR1, STM32F4_EOCIE },
> @@ -440,6 +484,8 @@ struct stm32_adc_info {
> .extsel = { STM32F4_ADC_CR2, STM32F4_EXTSEL_MASK,
> STM32F4_EXTSEL_SHIFT },
> .res = { STM32F4_ADC_CR1, STM32F4_RES_MASK, STM32F4_RES_SHIFT },
> + .smpr = { STM32F4_ADC_SMPR1, STM32F4_ADC_SMPR2 },
> + .smp_bits = stm32f4_smp_bits,
> };
>
> static const struct stm32_adc_regs stm32h7_sq[STM32_ADC_MAX_SQ + 1] = {
> @@ -483,6 +529,40 @@ struct stm32_adc_info {
> {},
> };
>
> +/**
> + * stm32h7_smp_bits - describe sampling time register index & bit fields
> + * Sorted so it can be indexed by channel number.
> + */
> +static const struct stm32_adc_regs stm32h7_smp_bits[] = {
> + /* STM32H7_ADC_SMPR1, smpr[] index, mask, shift for SMP0 to SMP9 */
> + { 0, GENMASK(2, 0), 0 },
> + { 0, GENMASK(5, 3), 3 },
> + { 0, GENMASK(8, 6), 6 },
> + { 0, GENMASK(11, 9), 9 },
> + { 0, GENMASK(14, 12), 12 },
> + { 0, GENMASK(17, 15), 15 },
> + { 0, GENMASK(20, 18), 18 },
> + { 0, GENMASK(23, 21), 21 },
> + { 0, GENMASK(26, 24), 24 },
> + { 0, GENMASK(29, 27), 27 },
> + /* STM32H7_ADC_SMPR2, smpr[] index, mask, shift for SMP10 to SMP19 */
> + { 1, GENMASK(2, 0), 0 },
> + { 1, GENMASK(5, 3), 3 },
> + { 1, GENMASK(8, 6), 6 },
> + { 1, GENMASK(11, 9), 9 },
> + { 1, GENMASK(14, 12), 12 },
> + { 1, GENMASK(17, 15), 15 },
> + { 1, GENMASK(20, 18), 18 },
> + { 1, GENMASK(23, 21), 21 },
> + { 1, GENMASK(26, 24), 24 },
> + { 1, GENMASK(29, 27), 27 },
> +};
> +
> +/* STM32H7 programmable sampling time (ADC clock cycles, rounded down) */
> +static const unsigned int stm32h7_adc_smp_cycles[STM32_ADC_MAX_SMP + 1] = {
> + 1, 2, 8, 16, 32, 64, 387, 810,
> +};
> +
> static const struct stm32_adc_regspec stm32h7_adc_regspec = {
> .dr = STM32H7_ADC_DR,
> .ier_eoc = { STM32H7_ADC_IER, STM32H7_EOCIE },
> @@ -492,6 +572,8 @@ struct stm32_adc_info {
> .extsel = { STM32H7_ADC_CFGR, STM32H7_EXTSEL_MASK,
> STM32H7_EXTSEL_SHIFT },
> .res = { STM32H7_ADC_CFGR, STM32H7_RES_MASK, STM32H7_RES_SHIFT },
> + .smpr = { STM32H7_ADC_SMPR1, STM32H7_ADC_SMPR2 },
> + .smp_bits = stm32h7_smp_bits,
> };
>
> /**
> @@ -933,6 +1015,7 @@ static void stm32h7_adc_unprepare(struct stm32_adc *adc)
> * @scan_mask: channels to be converted
> *
> * Conversion sequence :
> + * Apply sampling time settings for all channels.
> * Configure ADC scan sequence based on selected channels in scan_mask.
> * Add channels to SQR registers, from scan_mask LSB to MSB, then
> * program sequence len.
> @@ -946,6 +1029,10 @@ static int stm32_adc_conf_scan_seq(struct iio_dev *indio_dev,
> u32 val, bit;
> int i = 0;
>
> + /* Apply sampling time settings */
> + stm32_adc_writel(adc, adc->cfg->regs->smpr[0], adc->smpr_val[0]);
> + stm32_adc_writel(adc, adc->cfg->regs->smpr[1], adc->smpr_val[1]);
> +
> for_each_set_bit(bit, scan_mask, indio_dev->masklength) {
> chan = indio_dev->channels + bit;
> /*
> @@ -1079,6 +1166,7 @@ static int stm32_adc_get_trig_pol(struct iio_dev *indio_dev,
> * @res: conversion result
> *
> * The function performs a single conversion on a given channel:
> + * - Apply sampling time settings
> * - Program sequencer with one channel (e.g. in SQ1 with len = 1)
> * - Use SW trigger
> * - Start conversion, then wait for interrupt completion.
> @@ -1103,6 +1191,10 @@ static int stm32_adc_single_conv(struct iio_dev *indio_dev,
> return ret;
> }
>
> + /* Apply sampling time settings */
> + stm32_adc_writel(adc, regs->smpr[0], adc->smpr_val[0]);
> + stm32_adc_writel(adc, regs->smpr[1], adc->smpr_val[1]);
> +
> /* Program chan number in regular sequence (SQ1) */
> val = stm32_adc_readl(adc, regs->sqr[1].reg);
> val &= ~regs->sqr[1].mask;
> @@ -1507,10 +1599,28 @@ static int stm32_adc_of_get_resolution(struct iio_dev *indio_dev)
> return 0;
> }
>
> +static void stm32_adc_smpr_init(struct stm32_adc *adc, int channel, u32 smp_ns)
> +{
> + const struct stm32_adc_regs *smpr = &adc->cfg->regs->smp_bits[channel];
> + u32 period_ns, shift = smpr->shift, mask = smpr->mask;
> + unsigned int smp, r = smpr->reg;
> +
> + /* Determine sampling time (ADC clock cycles) */
> + period_ns = NSEC_PER_SEC / adc->common->rate;
> + for (smp = 0; smp <= STM32_ADC_MAX_SMP; smp++)
> + if ((period_ns * adc->cfg->smp_cycles[smp]) >= smp_ns)
> + break;
> + if (smp > STM32_ADC_MAX_SMP)
> + smp = STM32_ADC_MAX_SMP;
> +
> + /* pre-build sampling time registers (e.g. smpr1, smpr2) */
> + adc->smpr_val[r] = (adc->smpr_val[r] & ~mask) | (smp << shift);
> +}
> +
> static void stm32_adc_chan_init_one(struct iio_dev *indio_dev,
> struct iio_chan_spec *chan,
> const struct stm32_adc_chan_spec *channel,
> - int scan_index)
> + int scan_index, u32 smp)
> {
> struct stm32_adc *adc = iio_priv(indio_dev);
>
> @@ -1526,6 +1636,9 @@ static void stm32_adc_chan_init_one(struct iio_dev *indio_dev,
> 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);
> }
> @@ -1538,8 +1651,8 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
> struct property *prop;
> const __be32 *cur;
> struct iio_chan_spec *channels;
> - int scan_index = 0, num_channels;
> - u32 val;
> + int scan_index = 0, num_channels, ret;
> + u32 val, smp = 0;
>
> num_channels = of_property_count_u32_elems(node, "st,adc-channels");
> if (num_channels < 0 ||
> @@ -1548,6 +1661,13 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
> return num_channels < 0 ? num_channels : -EINVAL;
> }
>
> + /* Optional sample time is provided either for each, or all channels */
> + ret = of_property_count_u32_elems(node, "min-sample-time");
> + if (ret > 1 && ret != num_channels) {
> + dev_err(&indio_dev->dev, "Invalid min-sample-time\n");
> + return -EINVAL;
> + }
> +
> channels = devm_kcalloc(&indio_dev->dev, num_channels,
> sizeof(struct iio_chan_spec), GFP_KERNEL);
> if (!channels)
> @@ -1558,9 +1678,13 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
> dev_err(&indio_dev->dev, "Invalid channel %d\n", val);
> return -EINVAL;
> }
> +
> + of_property_read_u32_index(node, "min-sample-time", scan_index,
> + &smp);
> +
I might be missing something, but doesn't this fail for the single shared
value case?
> stm32_adc_chan_init_one(indio_dev, &channels[scan_index],
> &adc_info->channels[val],
> - scan_index);
> + scan_index, smp);
> scan_index++;
> }
>
> @@ -1755,6 +1879,7 @@ static int stm32_adc_remove(struct platform_device *pdev)
> .clk_required = true,
> .start_conv = stm32f4_adc_start_conv,
> .stop_conv = stm32f4_adc_stop_conv,
> + .smp_cycles = stm32f4_adc_smp_cycles,
> };
>
> static const struct stm32_adc_cfg stm32h7_adc_cfg = {
> @@ -1766,6 +1891,7 @@ static int stm32_adc_remove(struct platform_device *pdev)
> .stop_conv = stm32h7_adc_stop_conv,
> .prepare = stm32h7_adc_prepare,
> .unprepare = stm32h7_adc_unprepare,
> + .smp_cycles = stm32h7_adc_smp_cycles,
> };
>
> static const struct of_device_id stm32_adc_of_match[] = {
^ permalink raw reply [flat|nested] 39+ messages in thread* [PATCH 3/3] iio: adc: stm32: add optional min-sample-time
2017-07-23 11:00 ` Jonathan Cameron
(?)
@ 2017-07-24 8:16 ` Fabrice Gasnier
-1 siblings, 0 replies; 39+ messages in thread
From: Fabrice Gasnier @ 2017-07-24 8:16 UTC (permalink / raw)
To: linux-arm-kernel
On 07/23/2017 01:00 PM, Jonathan Cameron wrote:
> On Tue, 18 Jul 2017 14:35:32 +0200
> Fabrice Gasnier <fabrice.gasnier@st.com> wrote:
>
>> STM32 ADC allows each channel to be sampled with a different sampling time,
>> by setting SMPR registers. Basically, value depends on local electrical
>> properties. Selecting correct value for sampling time highly depends on
>> analog source impedance. There is a manual that may help in this process:
>> 'How to get the best ADC accuracy in STM32...'
>>
>> This patch allows to configure min-sample-time via device tree, either for:
>> - all channels at once:
>> min-sample-time = <10000>; /* nanosecs */
>>
>> - independently for each channel (must match "st,adc-channels" list):
>> st,adc-channels = <0 1>;
>> min-sample-time = <5000 10000>; /* nanosecs */
>>
>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
>
> On question inline which may well just be down to me missing what
> happens when you query index element 2 from a 1 element device tree
> array.
Hi Jonathan,
I should probably comment on it, please see inline.
>> ---
>> drivers/iio/adc/stm32-adc.c | 134 ++++++++++++++++++++++++++++++++++++++++++--
>> 1 file changed, 130 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
>> index 5bfcc1f..e6f6b5e 100644
>> --- a/drivers/iio/adc/stm32-adc.c
>> +++ b/drivers/iio/adc/stm32-adc.c
>> @@ -83,6 +83,8 @@
>> #define STM32H7_ADC_IER 0x04
>> #define STM32H7_ADC_CR 0x08
>> #define STM32H7_ADC_CFGR 0x0C
>> +#define STM32H7_ADC_SMPR1 0x14
>> +#define STM32H7_ADC_SMPR2 0x18
>> #define STM32H7_ADC_PCSEL 0x1C
>> #define STM32H7_ADC_SQR1 0x30
>> #define STM32H7_ADC_SQR2 0x34
>> @@ -151,6 +153,7 @@ enum stm32h7_adc_dmngt {
>> #define STM32H7_BOOST_CLKRATE 20000000UL
>>
>> #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
>> #define STM32_ADC_TIMEOUT (msecs_to_jiffies(STM32_ADC_TIMEOUT_US / 1000))
>>
>> @@ -227,6 +230,8 @@ struct stm32_adc_regs {
>> * @exten: trigger control register & bitfield
>> * @extsel: trigger selection register & bitfield
>> * @res: resolution selection register & bitfield
>> + * @smpr: smpr1 & smpr2 registers offset array
>> + * @smp_bits: smpr1 & smpr2 index and bitfields
>> */
>> struct stm32_adc_regspec {
>> const u32 dr;
>> @@ -236,6 +241,8 @@ struct stm32_adc_regspec {
>> const struct stm32_adc_regs exten;
>> const struct stm32_adc_regs extsel;
>> const struct stm32_adc_regs res;
>> + const u32 smpr[2];
>> + const struct stm32_adc_regs *smp_bits;
>> };
>>
>> struct stm32_adc;
>> @@ -251,6 +258,7 @@ struct stm32_adc_regspec {
>> * @start_conv: routine to start conversions
>> * @stop_conv: routine to stop conversions
>> * @unprepare: optional unprepare routine (disable, power-down)
>> + * @smp_cycles: programmable sampling time (ADC clock cycles)
>> */
>> struct stm32_adc_cfg {
>> const struct stm32_adc_regspec *regs;
>> @@ -262,6 +270,7 @@ struct stm32_adc_cfg {
>> void (*start_conv)(struct stm32_adc *, bool dma);
>> void (*stop_conv)(struct stm32_adc *);
>> void (*unprepare)(struct stm32_adc *);
>> + const unsigned int *smp_cycles;
>> };
>>
>> /**
>> @@ -283,6 +292,7 @@ struct stm32_adc_cfg {
>> * @rx_dma_buf: dma rx buffer bus address
>> * @rx_buf_sz: dma rx buffer size
>> * @pcsel bitmask to preselect channels on some devices
>> + * @smpr_val: sampling time settings (e.g. smpr1 / smpr2)
>> * @cal: optional calibration data on some devices
>> */
>> struct stm32_adc {
>> @@ -303,6 +313,7 @@ struct stm32_adc {
>> dma_addr_t rx_dma_buf;
>> unsigned int rx_buf_sz;
>> u32 pcsel;
>> + u32 smpr_val[2];
>> struct stm32_adc_calib cal;
>> };
>>
>> @@ -431,6 +442,39 @@ struct stm32_adc_info {
>> {}, /* sentinel */
>> };
>>
>> +/**
>> + * stm32f4_smp_bits[] - describe sampling time register index & bit fields
>> + * Sorted so it can be indexed by channel number.
>> + */
>> +static const struct stm32_adc_regs stm32f4_smp_bits[] = {
>> + /* STM32F4_ADC_SMPR2: smpr[] index, mask, shift for SMP0 to SMP9 */
>> + { 1, GENMASK(2, 0), 0 },
>> + { 1, GENMASK(5, 3), 3 },
>> + { 1, GENMASK(8, 6), 6 },
>> + { 1, GENMASK(11, 9), 9 },
>> + { 1, GENMASK(14, 12), 12 },
>> + { 1, GENMASK(17, 15), 15 },
>> + { 1, GENMASK(20, 18), 18 },
>> + { 1, GENMASK(23, 21), 21 },
>> + { 1, GENMASK(26, 24), 24 },
>> + { 1, GENMASK(29, 27), 27 },
>> + /* STM32F4_ADC_SMPR1, smpr[] index, mask, shift for SMP10 to SMP18 */
>> + { 0, GENMASK(2, 0), 0 },
>> + { 0, GENMASK(5, 3), 3 },
>> + { 0, GENMASK(8, 6), 6 },
>> + { 0, GENMASK(11, 9), 9 },
>> + { 0, GENMASK(14, 12), 12 },
>> + { 0, GENMASK(17, 15), 15 },
>> + { 0, GENMASK(20, 18), 18 },
>> + { 0, GENMASK(23, 21), 21 },
>> + { 0, GENMASK(26, 24), 24 },
>> +};
>> +
>> +/* STM32F4 programmable sampling time (ADC clock cycles) */
>> +static const unsigned int stm32f4_adc_smp_cycles[STM32_ADC_MAX_SMP + 1] = {
>> + 3, 15, 28, 56, 84, 112, 144, 480,
>> +};
>> +
>> static const struct stm32_adc_regspec stm32f4_adc_regspec = {
>> .dr = STM32F4_ADC_DR,
>> .ier_eoc = { STM32F4_ADC_CR1, STM32F4_EOCIE },
>> @@ -440,6 +484,8 @@ struct stm32_adc_info {
>> .extsel = { STM32F4_ADC_CR2, STM32F4_EXTSEL_MASK,
>> STM32F4_EXTSEL_SHIFT },
>> .res = { STM32F4_ADC_CR1, STM32F4_RES_MASK, STM32F4_RES_SHIFT },
>> + .smpr = { STM32F4_ADC_SMPR1, STM32F4_ADC_SMPR2 },
>> + .smp_bits = stm32f4_smp_bits,
>> };
>>
>> static const struct stm32_adc_regs stm32h7_sq[STM32_ADC_MAX_SQ + 1] = {
>> @@ -483,6 +529,40 @@ struct stm32_adc_info {
>> {},
>> };
>>
>> +/**
>> + * stm32h7_smp_bits - describe sampling time register index & bit fields
>> + * Sorted so it can be indexed by channel number.
>> + */
>> +static const struct stm32_adc_regs stm32h7_smp_bits[] = {
>> + /* STM32H7_ADC_SMPR1, smpr[] index, mask, shift for SMP0 to SMP9 */
>> + { 0, GENMASK(2, 0), 0 },
>> + { 0, GENMASK(5, 3), 3 },
>> + { 0, GENMASK(8, 6), 6 },
>> + { 0, GENMASK(11, 9), 9 },
>> + { 0, GENMASK(14, 12), 12 },
>> + { 0, GENMASK(17, 15), 15 },
>> + { 0, GENMASK(20, 18), 18 },
>> + { 0, GENMASK(23, 21), 21 },
>> + { 0, GENMASK(26, 24), 24 },
>> + { 0, GENMASK(29, 27), 27 },
>> + /* STM32H7_ADC_SMPR2, smpr[] index, mask, shift for SMP10 to SMP19 */
>> + { 1, GENMASK(2, 0), 0 },
>> + { 1, GENMASK(5, 3), 3 },
>> + { 1, GENMASK(8, 6), 6 },
>> + { 1, GENMASK(11, 9), 9 },
>> + { 1, GENMASK(14, 12), 12 },
>> + { 1, GENMASK(17, 15), 15 },
>> + { 1, GENMASK(20, 18), 18 },
>> + { 1, GENMASK(23, 21), 21 },
>> + { 1, GENMASK(26, 24), 24 },
>> + { 1, GENMASK(29, 27), 27 },
>> +};
>> +
>> +/* STM32H7 programmable sampling time (ADC clock cycles, rounded down) */
>> +static const unsigned int stm32h7_adc_smp_cycles[STM32_ADC_MAX_SMP + 1] = {
>> + 1, 2, 8, 16, 32, 64, 387, 810,
>> +};
>> +
>> static const struct stm32_adc_regspec stm32h7_adc_regspec = {
>> .dr = STM32H7_ADC_DR,
>> .ier_eoc = { STM32H7_ADC_IER, STM32H7_EOCIE },
>> @@ -492,6 +572,8 @@ struct stm32_adc_info {
>> .extsel = { STM32H7_ADC_CFGR, STM32H7_EXTSEL_MASK,
>> STM32H7_EXTSEL_SHIFT },
>> .res = { STM32H7_ADC_CFGR, STM32H7_RES_MASK, STM32H7_RES_SHIFT },
>> + .smpr = { STM32H7_ADC_SMPR1, STM32H7_ADC_SMPR2 },
>> + .smp_bits = stm32h7_smp_bits,
>> };
>>
>> /**
>> @@ -933,6 +1015,7 @@ static void stm32h7_adc_unprepare(struct stm32_adc *adc)
>> * @scan_mask: channels to be converted
>> *
>> * Conversion sequence :
>> + * Apply sampling time settings for all channels.
>> * Configure ADC scan sequence based on selected channels in scan_mask.
>> * Add channels to SQR registers, from scan_mask LSB to MSB, then
>> * program sequence len.
>> @@ -946,6 +1029,10 @@ static int stm32_adc_conf_scan_seq(struct iio_dev *indio_dev,
>> u32 val, bit;
>> int i = 0;
>>
>> + /* Apply sampling time settings */
>> + stm32_adc_writel(adc, adc->cfg->regs->smpr[0], adc->smpr_val[0]);
>> + stm32_adc_writel(adc, adc->cfg->regs->smpr[1], adc->smpr_val[1]);
>> +
>> for_each_set_bit(bit, scan_mask, indio_dev->masklength) {
>> chan = indio_dev->channels + bit;
>> /*
>> @@ -1079,6 +1166,7 @@ static int stm32_adc_get_trig_pol(struct iio_dev *indio_dev,
>> * @res: conversion result
>> *
>> * The function performs a single conversion on a given channel:
>> + * - Apply sampling time settings
>> * - Program sequencer with one channel (e.g. in SQ1 with len = 1)
>> * - Use SW trigger
>> * - Start conversion, then wait for interrupt completion.
>> @@ -1103,6 +1191,10 @@ static int stm32_adc_single_conv(struct iio_dev *indio_dev,
>> return ret;
>> }
>>
>> + /* Apply sampling time settings */
>> + stm32_adc_writel(adc, regs->smpr[0], adc->smpr_val[0]);
>> + stm32_adc_writel(adc, regs->smpr[1], adc->smpr_val[1]);
>> +
>> /* Program chan number in regular sequence (SQ1) */
>> val = stm32_adc_readl(adc, regs->sqr[1].reg);
>> val &= ~regs->sqr[1].mask;
>> @@ -1507,10 +1599,28 @@ static int stm32_adc_of_get_resolution(struct iio_dev *indio_dev)
>> return 0;
>> }
>>
>> +static void stm32_adc_smpr_init(struct stm32_adc *adc, int channel, u32 smp_ns)
>> +{
>> + const struct stm32_adc_regs *smpr = &adc->cfg->regs->smp_bits[channel];
>> + u32 period_ns, shift = smpr->shift, mask = smpr->mask;
>> + unsigned int smp, r = smpr->reg;
>> +
>> + /* Determine sampling time (ADC clock cycles) */
>> + period_ns = NSEC_PER_SEC / adc->common->rate;
>> + for (smp = 0; smp <= STM32_ADC_MAX_SMP; smp++)
>> + if ((period_ns * adc->cfg->smp_cycles[smp]) >= smp_ns)
>> + break;
>> + if (smp > STM32_ADC_MAX_SMP)
>> + smp = STM32_ADC_MAX_SMP;
>> +
>> + /* pre-build sampling time registers (e.g. smpr1, smpr2) */
>> + adc->smpr_val[r] = (adc->smpr_val[r] & ~mask) | (smp << shift);
>> +}
>> +
>> static void stm32_adc_chan_init_one(struct iio_dev *indio_dev,
>> struct iio_chan_spec *chan,
>> const struct stm32_adc_chan_spec *channel,
>> - int scan_index)
>> + int scan_index, u32 smp)
>> {
>> struct stm32_adc *adc = iio_priv(indio_dev);
>>
>> @@ -1526,6 +1636,9 @@ static void stm32_adc_chan_init_one(struct iio_dev *indio_dev,
>> 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);
>> }
>> @@ -1538,8 +1651,8 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
>> struct property *prop;
>> const __be32 *cur;
>> struct iio_chan_spec *channels;
>> - int scan_index = 0, num_channels;
>> - u32 val;
>> + int scan_index = 0, num_channels, ret;
>> + u32 val, smp = 0;
>>
>> num_channels = of_property_count_u32_elems(node, "st,adc-channels");
>> if (num_channels < 0 ||
>> @@ -1548,6 +1661,13 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
>> return num_channels < 0 ? num_channels : -EINVAL;
>> }
>>
>> + /* Optional sample time is provided either for each, or all channels */
>> + ret = of_property_count_u32_elems(node, "min-sample-time");
>> + if (ret > 1 && ret != num_channels) {
>> + dev_err(&indio_dev->dev, "Invalid min-sample-time\n");
>> + return -EINVAL;
>> + }
>> +
>> channels = devm_kcalloc(&indio_dev->dev, num_channels,
>> sizeof(struct iio_chan_spec), GFP_KERNEL);
>> if (!channels)
>> @@ -1558,9 +1678,13 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
>> dev_err(&indio_dev->dev, "Invalid channel %d\n", val);
>> return -EINVAL;
>> }
>> +
>> + of_property_read_u32_index(node, "min-sample-time", scan_index,
>> + &smp);
>> +
> I might be missing something, but doesn't this fail for the single shared
> value case?
Yes, this fails in the single shared value case, with index >= 1.
Rewinding... when index is 0, it picks up 1st (shared) value.
Then fails for other index values.
of_property_read_u32_index() documentation states out value remains
untouched in case of error:
/**
* of_property_read_u32_index...
[...]
* The out_value is modified only if a valid u32 value can be decoded.
This is what I use here (as far as I tested it, it worked like a charm).
Do you wish I add a comment to describe this ?
Please let me know,
Best Regards,
Fabrice
>> stm32_adc_chan_init_one(indio_dev, &channels[scan_index],
>> &adc_info->channels[val],
>> - scan_index);
>> + scan_index, smp);
>> scan_index++;
>> }
>>
>> @@ -1755,6 +1879,7 @@ static int stm32_adc_remove(struct platform_device *pdev)
>> .clk_required = true,
>> .start_conv = stm32f4_adc_start_conv,
>> .stop_conv = stm32f4_adc_stop_conv,
>> + .smp_cycles = stm32f4_adc_smp_cycles,
>> };
>>
>> static const struct stm32_adc_cfg stm32h7_adc_cfg = {
>> @@ -1766,6 +1891,7 @@ static int stm32_adc_remove(struct platform_device *pdev)
>> .stop_conv = stm32h7_adc_stop_conv,
>> .prepare = stm32h7_adc_prepare,
>> .unprepare = stm32h7_adc_unprepare,
>> + .smp_cycles = stm32h7_adc_smp_cycles,
>> };
>>
>> static const struct of_device_id stm32_adc_of_match[] = {
>
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [PATCH 3/3] iio: adc: stm32: add optional min-sample-time
@ 2017-07-24 8:16 ` Fabrice Gasnier
0 siblings, 0 replies; 39+ messages in thread
From: Fabrice Gasnier @ 2017-07-24 8:16 UTC (permalink / raw)
To: Jonathan Cameron
Cc: robh+dt, linux, mark.rutland, mcoquelin.stm32, alexandre.torgue,
lars, knaack.h, pmeerw, benjamin.gaignard, benjamin.gaignard,
linux-iio, devicetree, linux-arm-kernel, linux-kernel
On 07/23/2017 01:00 PM, Jonathan Cameron wrote:
> On Tue, 18 Jul 2017 14:35:32 +0200
> Fabrice Gasnier <fabrice.gasnier@st.com> wrote:
>
>> STM32 ADC allows each channel to be sampled with a different sampling time,
>> by setting SMPR registers. Basically, value depends on local electrical
>> properties. Selecting correct value for sampling time highly depends on
>> analog source impedance. There is a manual that may help in this process:
>> 'How to get the best ADC accuracy in STM32...'
>>
>> This patch allows to configure min-sample-time via device tree, either for:
>> - all channels at once:
>> min-sample-time = <10000>; /* nanosecs */
>>
>> - independently for each channel (must match "st,adc-channels" list):
>> st,adc-channels = <0 1>;
>> min-sample-time = <5000 10000>; /* nanosecs */
>>
>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
>
> On question inline which may well just be down to me missing what
> happens when you query index element 2 from a 1 element device tree
> array.
Hi Jonathan,
I should probably comment on it, please see inline.
>> ---
>> drivers/iio/adc/stm32-adc.c | 134 ++++++++++++++++++++++++++++++++++++++++++--
>> 1 file changed, 130 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
>> index 5bfcc1f..e6f6b5e 100644
>> --- a/drivers/iio/adc/stm32-adc.c
>> +++ b/drivers/iio/adc/stm32-adc.c
>> @@ -83,6 +83,8 @@
>> #define STM32H7_ADC_IER 0x04
>> #define STM32H7_ADC_CR 0x08
>> #define STM32H7_ADC_CFGR 0x0C
>> +#define STM32H7_ADC_SMPR1 0x14
>> +#define STM32H7_ADC_SMPR2 0x18
>> #define STM32H7_ADC_PCSEL 0x1C
>> #define STM32H7_ADC_SQR1 0x30
>> #define STM32H7_ADC_SQR2 0x34
>> @@ -151,6 +153,7 @@ enum stm32h7_adc_dmngt {
>> #define STM32H7_BOOST_CLKRATE 20000000UL
>>
>> #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
>> #define STM32_ADC_TIMEOUT (msecs_to_jiffies(STM32_ADC_TIMEOUT_US / 1000))
>>
>> @@ -227,6 +230,8 @@ struct stm32_adc_regs {
>> * @exten: trigger control register & bitfield
>> * @extsel: trigger selection register & bitfield
>> * @res: resolution selection register & bitfield
>> + * @smpr: smpr1 & smpr2 registers offset array
>> + * @smp_bits: smpr1 & smpr2 index and bitfields
>> */
>> struct stm32_adc_regspec {
>> const u32 dr;
>> @@ -236,6 +241,8 @@ struct stm32_adc_regspec {
>> const struct stm32_adc_regs exten;
>> const struct stm32_adc_regs extsel;
>> const struct stm32_adc_regs res;
>> + const u32 smpr[2];
>> + const struct stm32_adc_regs *smp_bits;
>> };
>>
>> struct stm32_adc;
>> @@ -251,6 +258,7 @@ struct stm32_adc_regspec {
>> * @start_conv: routine to start conversions
>> * @stop_conv: routine to stop conversions
>> * @unprepare: optional unprepare routine (disable, power-down)
>> + * @smp_cycles: programmable sampling time (ADC clock cycles)
>> */
>> struct stm32_adc_cfg {
>> const struct stm32_adc_regspec *regs;
>> @@ -262,6 +270,7 @@ struct stm32_adc_cfg {
>> void (*start_conv)(struct stm32_adc *, bool dma);
>> void (*stop_conv)(struct stm32_adc *);
>> void (*unprepare)(struct stm32_adc *);
>> + const unsigned int *smp_cycles;
>> };
>>
>> /**
>> @@ -283,6 +292,7 @@ struct stm32_adc_cfg {
>> * @rx_dma_buf: dma rx buffer bus address
>> * @rx_buf_sz: dma rx buffer size
>> * @pcsel bitmask to preselect channels on some devices
>> + * @smpr_val: sampling time settings (e.g. smpr1 / smpr2)
>> * @cal: optional calibration data on some devices
>> */
>> struct stm32_adc {
>> @@ -303,6 +313,7 @@ struct stm32_adc {
>> dma_addr_t rx_dma_buf;
>> unsigned int rx_buf_sz;
>> u32 pcsel;
>> + u32 smpr_val[2];
>> struct stm32_adc_calib cal;
>> };
>>
>> @@ -431,6 +442,39 @@ struct stm32_adc_info {
>> {}, /* sentinel */
>> };
>>
>> +/**
>> + * stm32f4_smp_bits[] - describe sampling time register index & bit fields
>> + * Sorted so it can be indexed by channel number.
>> + */
>> +static const struct stm32_adc_regs stm32f4_smp_bits[] = {
>> + /* STM32F4_ADC_SMPR2: smpr[] index, mask, shift for SMP0 to SMP9 */
>> + { 1, GENMASK(2, 0), 0 },
>> + { 1, GENMASK(5, 3), 3 },
>> + { 1, GENMASK(8, 6), 6 },
>> + { 1, GENMASK(11, 9), 9 },
>> + { 1, GENMASK(14, 12), 12 },
>> + { 1, GENMASK(17, 15), 15 },
>> + { 1, GENMASK(20, 18), 18 },
>> + { 1, GENMASK(23, 21), 21 },
>> + { 1, GENMASK(26, 24), 24 },
>> + { 1, GENMASK(29, 27), 27 },
>> + /* STM32F4_ADC_SMPR1, smpr[] index, mask, shift for SMP10 to SMP18 */
>> + { 0, GENMASK(2, 0), 0 },
>> + { 0, GENMASK(5, 3), 3 },
>> + { 0, GENMASK(8, 6), 6 },
>> + { 0, GENMASK(11, 9), 9 },
>> + { 0, GENMASK(14, 12), 12 },
>> + { 0, GENMASK(17, 15), 15 },
>> + { 0, GENMASK(20, 18), 18 },
>> + { 0, GENMASK(23, 21), 21 },
>> + { 0, GENMASK(26, 24), 24 },
>> +};
>> +
>> +/* STM32F4 programmable sampling time (ADC clock cycles) */
>> +static const unsigned int stm32f4_adc_smp_cycles[STM32_ADC_MAX_SMP + 1] = {
>> + 3, 15, 28, 56, 84, 112, 144, 480,
>> +};
>> +
>> static const struct stm32_adc_regspec stm32f4_adc_regspec = {
>> .dr = STM32F4_ADC_DR,
>> .ier_eoc = { STM32F4_ADC_CR1, STM32F4_EOCIE },
>> @@ -440,6 +484,8 @@ struct stm32_adc_info {
>> .extsel = { STM32F4_ADC_CR2, STM32F4_EXTSEL_MASK,
>> STM32F4_EXTSEL_SHIFT },
>> .res = { STM32F4_ADC_CR1, STM32F4_RES_MASK, STM32F4_RES_SHIFT },
>> + .smpr = { STM32F4_ADC_SMPR1, STM32F4_ADC_SMPR2 },
>> + .smp_bits = stm32f4_smp_bits,
>> };
>>
>> static const struct stm32_adc_regs stm32h7_sq[STM32_ADC_MAX_SQ + 1] = {
>> @@ -483,6 +529,40 @@ struct stm32_adc_info {
>> {},
>> };
>>
>> +/**
>> + * stm32h7_smp_bits - describe sampling time register index & bit fields
>> + * Sorted so it can be indexed by channel number.
>> + */
>> +static const struct stm32_adc_regs stm32h7_smp_bits[] = {
>> + /* STM32H7_ADC_SMPR1, smpr[] index, mask, shift for SMP0 to SMP9 */
>> + { 0, GENMASK(2, 0), 0 },
>> + { 0, GENMASK(5, 3), 3 },
>> + { 0, GENMASK(8, 6), 6 },
>> + { 0, GENMASK(11, 9), 9 },
>> + { 0, GENMASK(14, 12), 12 },
>> + { 0, GENMASK(17, 15), 15 },
>> + { 0, GENMASK(20, 18), 18 },
>> + { 0, GENMASK(23, 21), 21 },
>> + { 0, GENMASK(26, 24), 24 },
>> + { 0, GENMASK(29, 27), 27 },
>> + /* STM32H7_ADC_SMPR2, smpr[] index, mask, shift for SMP10 to SMP19 */
>> + { 1, GENMASK(2, 0), 0 },
>> + { 1, GENMASK(5, 3), 3 },
>> + { 1, GENMASK(8, 6), 6 },
>> + { 1, GENMASK(11, 9), 9 },
>> + { 1, GENMASK(14, 12), 12 },
>> + { 1, GENMASK(17, 15), 15 },
>> + { 1, GENMASK(20, 18), 18 },
>> + { 1, GENMASK(23, 21), 21 },
>> + { 1, GENMASK(26, 24), 24 },
>> + { 1, GENMASK(29, 27), 27 },
>> +};
>> +
>> +/* STM32H7 programmable sampling time (ADC clock cycles, rounded down) */
>> +static const unsigned int stm32h7_adc_smp_cycles[STM32_ADC_MAX_SMP + 1] = {
>> + 1, 2, 8, 16, 32, 64, 387, 810,
>> +};
>> +
>> static const struct stm32_adc_regspec stm32h7_adc_regspec = {
>> .dr = STM32H7_ADC_DR,
>> .ier_eoc = { STM32H7_ADC_IER, STM32H7_EOCIE },
>> @@ -492,6 +572,8 @@ struct stm32_adc_info {
>> .extsel = { STM32H7_ADC_CFGR, STM32H7_EXTSEL_MASK,
>> STM32H7_EXTSEL_SHIFT },
>> .res = { STM32H7_ADC_CFGR, STM32H7_RES_MASK, STM32H7_RES_SHIFT },
>> + .smpr = { STM32H7_ADC_SMPR1, STM32H7_ADC_SMPR2 },
>> + .smp_bits = stm32h7_smp_bits,
>> };
>>
>> /**
>> @@ -933,6 +1015,7 @@ static void stm32h7_adc_unprepare(struct stm32_adc *adc)
>> * @scan_mask: channels to be converted
>> *
>> * Conversion sequence :
>> + * Apply sampling time settings for all channels.
>> * Configure ADC scan sequence based on selected channels in scan_mask.
>> * Add channels to SQR registers, from scan_mask LSB to MSB, then
>> * program sequence len.
>> @@ -946,6 +1029,10 @@ static int stm32_adc_conf_scan_seq(struct iio_dev *indio_dev,
>> u32 val, bit;
>> int i = 0;
>>
>> + /* Apply sampling time settings */
>> + stm32_adc_writel(adc, adc->cfg->regs->smpr[0], adc->smpr_val[0]);
>> + stm32_adc_writel(adc, adc->cfg->regs->smpr[1], adc->smpr_val[1]);
>> +
>> for_each_set_bit(bit, scan_mask, indio_dev->masklength) {
>> chan = indio_dev->channels + bit;
>> /*
>> @@ -1079,6 +1166,7 @@ static int stm32_adc_get_trig_pol(struct iio_dev *indio_dev,
>> * @res: conversion result
>> *
>> * The function performs a single conversion on a given channel:
>> + * - Apply sampling time settings
>> * - Program sequencer with one channel (e.g. in SQ1 with len = 1)
>> * - Use SW trigger
>> * - Start conversion, then wait for interrupt completion.
>> @@ -1103,6 +1191,10 @@ static int stm32_adc_single_conv(struct iio_dev *indio_dev,
>> return ret;
>> }
>>
>> + /* Apply sampling time settings */
>> + stm32_adc_writel(adc, regs->smpr[0], adc->smpr_val[0]);
>> + stm32_adc_writel(adc, regs->smpr[1], adc->smpr_val[1]);
>> +
>> /* Program chan number in regular sequence (SQ1) */
>> val = stm32_adc_readl(adc, regs->sqr[1].reg);
>> val &= ~regs->sqr[1].mask;
>> @@ -1507,10 +1599,28 @@ static int stm32_adc_of_get_resolution(struct iio_dev *indio_dev)
>> return 0;
>> }
>>
>> +static void stm32_adc_smpr_init(struct stm32_adc *adc, int channel, u32 smp_ns)
>> +{
>> + const struct stm32_adc_regs *smpr = &adc->cfg->regs->smp_bits[channel];
>> + u32 period_ns, shift = smpr->shift, mask = smpr->mask;
>> + unsigned int smp, r = smpr->reg;
>> +
>> + /* Determine sampling time (ADC clock cycles) */
>> + period_ns = NSEC_PER_SEC / adc->common->rate;
>> + for (smp = 0; smp <= STM32_ADC_MAX_SMP; smp++)
>> + if ((period_ns * adc->cfg->smp_cycles[smp]) >= smp_ns)
>> + break;
>> + if (smp > STM32_ADC_MAX_SMP)
>> + smp = STM32_ADC_MAX_SMP;
>> +
>> + /* pre-build sampling time registers (e.g. smpr1, smpr2) */
>> + adc->smpr_val[r] = (adc->smpr_val[r] & ~mask) | (smp << shift);
>> +}
>> +
>> static void stm32_adc_chan_init_one(struct iio_dev *indio_dev,
>> struct iio_chan_spec *chan,
>> const struct stm32_adc_chan_spec *channel,
>> - int scan_index)
>> + int scan_index, u32 smp)
>> {
>> struct stm32_adc *adc = iio_priv(indio_dev);
>>
>> @@ -1526,6 +1636,9 @@ static void stm32_adc_chan_init_one(struct iio_dev *indio_dev,
>> 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);
>> }
>> @@ -1538,8 +1651,8 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
>> struct property *prop;
>> const __be32 *cur;
>> struct iio_chan_spec *channels;
>> - int scan_index = 0, num_channels;
>> - u32 val;
>> + int scan_index = 0, num_channels, ret;
>> + u32 val, smp = 0;
>>
>> num_channels = of_property_count_u32_elems(node, "st,adc-channels");
>> if (num_channels < 0 ||
>> @@ -1548,6 +1661,13 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
>> return num_channels < 0 ? num_channels : -EINVAL;
>> }
>>
>> + /* Optional sample time is provided either for each, or all channels */
>> + ret = of_property_count_u32_elems(node, "min-sample-time");
>> + if (ret > 1 && ret != num_channels) {
>> + dev_err(&indio_dev->dev, "Invalid min-sample-time\n");
>> + return -EINVAL;
>> + }
>> +
>> channels = devm_kcalloc(&indio_dev->dev, num_channels,
>> sizeof(struct iio_chan_spec), GFP_KERNEL);
>> if (!channels)
>> @@ -1558,9 +1678,13 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
>> dev_err(&indio_dev->dev, "Invalid channel %d\n", val);
>> return -EINVAL;
>> }
>> +
>> + of_property_read_u32_index(node, "min-sample-time", scan_index,
>> + &smp);
>> +
> I might be missing something, but doesn't this fail for the single shared
> value case?
Yes, this fails in the single shared value case, with index >= 1.
Rewinding... when index is 0, it picks up 1st (shared) value.
Then fails for other index values.
of_property_read_u32_index() documentation states out value remains
untouched in case of error:
/**
* of_property_read_u32_index...
[...]
* The out_value is modified only if a valid u32 value can be decoded.
This is what I use here (as far as I tested it, it worked like a charm).
Do you wish I add a comment to describe this ?
Please let me know,
Best Regards,
Fabrice
>> stm32_adc_chan_init_one(indio_dev, &channels[scan_index],
>> &adc_info->channels[val],
>> - scan_index);
>> + scan_index, smp);
>> scan_index++;
>> }
>>
>> @@ -1755,6 +1879,7 @@ static int stm32_adc_remove(struct platform_device *pdev)
>> .clk_required = true,
>> .start_conv = stm32f4_adc_start_conv,
>> .stop_conv = stm32f4_adc_stop_conv,
>> + .smp_cycles = stm32f4_adc_smp_cycles,
>> };
>>
>> static const struct stm32_adc_cfg stm32h7_adc_cfg = {
>> @@ -1766,6 +1891,7 @@ static int stm32_adc_remove(struct platform_device *pdev)
>> .stop_conv = stm32h7_adc_stop_conv,
>> .prepare = stm32h7_adc_prepare,
>> .unprepare = stm32h7_adc_unprepare,
>> + .smp_cycles = stm32h7_adc_smp_cycles,
>> };
>>
>> static const struct of_device_id stm32_adc_of_match[] = {
>
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [PATCH 3/3] iio: adc: stm32: add optional min-sample-time
@ 2017-07-24 8:16 ` Fabrice Gasnier
0 siblings, 0 replies; 39+ messages in thread
From: Fabrice Gasnier @ 2017-07-24 8:16 UTC (permalink / raw)
To: Jonathan Cameron
Cc: 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 07/23/2017 01:00 PM, Jonathan Cameron wrote:
> On Tue, 18 Jul 2017 14:35:32 +0200
> Fabrice Gasnier <fabrice.gasnier@st.com> wrote:
>
>> STM32 ADC allows each channel to be sampled with a different sampling time,
>> by setting SMPR registers. Basically, value depends on local electrical
>> properties. Selecting correct value for sampling time highly depends on
>> analog source impedance. There is a manual that may help in this process:
>> 'How to get the best ADC accuracy in STM32...'
>>
>> This patch allows to configure min-sample-time via device tree, either for:
>> - all channels at once:
>> min-sample-time = <10000>; /* nanosecs */
>>
>> - independently for each channel (must match "st,adc-channels" list):
>> st,adc-channels = <0 1>;
>> min-sample-time = <5000 10000>; /* nanosecs */
>>
>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
>
> On question inline which may well just be down to me missing what
> happens when you query index element 2 from a 1 element device tree
> array.
Hi Jonathan,
I should probably comment on it, please see inline.
>> ---
>> drivers/iio/adc/stm32-adc.c | 134 ++++++++++++++++++++++++++++++++++++++++++--
>> 1 file changed, 130 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
>> index 5bfcc1f..e6f6b5e 100644
>> --- a/drivers/iio/adc/stm32-adc.c
>> +++ b/drivers/iio/adc/stm32-adc.c
>> @@ -83,6 +83,8 @@
>> #define STM32H7_ADC_IER 0x04
>> #define STM32H7_ADC_CR 0x08
>> #define STM32H7_ADC_CFGR 0x0C
>> +#define STM32H7_ADC_SMPR1 0x14
>> +#define STM32H7_ADC_SMPR2 0x18
>> #define STM32H7_ADC_PCSEL 0x1C
>> #define STM32H7_ADC_SQR1 0x30
>> #define STM32H7_ADC_SQR2 0x34
>> @@ -151,6 +153,7 @@ enum stm32h7_adc_dmngt {
>> #define STM32H7_BOOST_CLKRATE 20000000UL
>>
>> #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
>> #define STM32_ADC_TIMEOUT (msecs_to_jiffies(STM32_ADC_TIMEOUT_US / 1000))
>>
>> @@ -227,6 +230,8 @@ struct stm32_adc_regs {
>> * @exten: trigger control register & bitfield
>> * @extsel: trigger selection register & bitfield
>> * @res: resolution selection register & bitfield
>> + * @smpr: smpr1 & smpr2 registers offset array
>> + * @smp_bits: smpr1 & smpr2 index and bitfields
>> */
>> struct stm32_adc_regspec {
>> const u32 dr;
>> @@ -236,6 +241,8 @@ struct stm32_adc_regspec {
>> const struct stm32_adc_regs exten;
>> const struct stm32_adc_regs extsel;
>> const struct stm32_adc_regs res;
>> + const u32 smpr[2];
>> + const struct stm32_adc_regs *smp_bits;
>> };
>>
>> struct stm32_adc;
>> @@ -251,6 +258,7 @@ struct stm32_adc_regspec {
>> * @start_conv: routine to start conversions
>> * @stop_conv: routine to stop conversions
>> * @unprepare: optional unprepare routine (disable, power-down)
>> + * @smp_cycles: programmable sampling time (ADC clock cycles)
>> */
>> struct stm32_adc_cfg {
>> const struct stm32_adc_regspec *regs;
>> @@ -262,6 +270,7 @@ struct stm32_adc_cfg {
>> void (*start_conv)(struct stm32_adc *, bool dma);
>> void (*stop_conv)(struct stm32_adc *);
>> void (*unprepare)(struct stm32_adc *);
>> + const unsigned int *smp_cycles;
>> };
>>
>> /**
>> @@ -283,6 +292,7 @@ struct stm32_adc_cfg {
>> * @rx_dma_buf: dma rx buffer bus address
>> * @rx_buf_sz: dma rx buffer size
>> * @pcsel bitmask to preselect channels on some devices
>> + * @smpr_val: sampling time settings (e.g. smpr1 / smpr2)
>> * @cal: optional calibration data on some devices
>> */
>> struct stm32_adc {
>> @@ -303,6 +313,7 @@ struct stm32_adc {
>> dma_addr_t rx_dma_buf;
>> unsigned int rx_buf_sz;
>> u32 pcsel;
>> + u32 smpr_val[2];
>> struct stm32_adc_calib cal;
>> };
>>
>> @@ -431,6 +442,39 @@ struct stm32_adc_info {
>> {}, /* sentinel */
>> };
>>
>> +/**
>> + * stm32f4_smp_bits[] - describe sampling time register index & bit fields
>> + * Sorted so it can be indexed by channel number.
>> + */
>> +static const struct stm32_adc_regs stm32f4_smp_bits[] = {
>> + /* STM32F4_ADC_SMPR2: smpr[] index, mask, shift for SMP0 to SMP9 */
>> + { 1, GENMASK(2, 0), 0 },
>> + { 1, GENMASK(5, 3), 3 },
>> + { 1, GENMASK(8, 6), 6 },
>> + { 1, GENMASK(11, 9), 9 },
>> + { 1, GENMASK(14, 12), 12 },
>> + { 1, GENMASK(17, 15), 15 },
>> + { 1, GENMASK(20, 18), 18 },
>> + { 1, GENMASK(23, 21), 21 },
>> + { 1, GENMASK(26, 24), 24 },
>> + { 1, GENMASK(29, 27), 27 },
>> + /* STM32F4_ADC_SMPR1, smpr[] index, mask, shift for SMP10 to SMP18 */
>> + { 0, GENMASK(2, 0), 0 },
>> + { 0, GENMASK(5, 3), 3 },
>> + { 0, GENMASK(8, 6), 6 },
>> + { 0, GENMASK(11, 9), 9 },
>> + { 0, GENMASK(14, 12), 12 },
>> + { 0, GENMASK(17, 15), 15 },
>> + { 0, GENMASK(20, 18), 18 },
>> + { 0, GENMASK(23, 21), 21 },
>> + { 0, GENMASK(26, 24), 24 },
>> +};
>> +
>> +/* STM32F4 programmable sampling time (ADC clock cycles) */
>> +static const unsigned int stm32f4_adc_smp_cycles[STM32_ADC_MAX_SMP + 1] = {
>> + 3, 15, 28, 56, 84, 112, 144, 480,
>> +};
>> +
>> static const struct stm32_adc_regspec stm32f4_adc_regspec = {
>> .dr = STM32F4_ADC_DR,
>> .ier_eoc = { STM32F4_ADC_CR1, STM32F4_EOCIE },
>> @@ -440,6 +484,8 @@ struct stm32_adc_info {
>> .extsel = { STM32F4_ADC_CR2, STM32F4_EXTSEL_MASK,
>> STM32F4_EXTSEL_SHIFT },
>> .res = { STM32F4_ADC_CR1, STM32F4_RES_MASK, STM32F4_RES_SHIFT },
>> + .smpr = { STM32F4_ADC_SMPR1, STM32F4_ADC_SMPR2 },
>> + .smp_bits = stm32f4_smp_bits,
>> };
>>
>> static const struct stm32_adc_regs stm32h7_sq[STM32_ADC_MAX_SQ + 1] = {
>> @@ -483,6 +529,40 @@ struct stm32_adc_info {
>> {},
>> };
>>
>> +/**
>> + * stm32h7_smp_bits - describe sampling time register index & bit fields
>> + * Sorted so it can be indexed by channel number.
>> + */
>> +static const struct stm32_adc_regs stm32h7_smp_bits[] = {
>> + /* STM32H7_ADC_SMPR1, smpr[] index, mask, shift for SMP0 to SMP9 */
>> + { 0, GENMASK(2, 0), 0 },
>> + { 0, GENMASK(5, 3), 3 },
>> + { 0, GENMASK(8, 6), 6 },
>> + { 0, GENMASK(11, 9), 9 },
>> + { 0, GENMASK(14, 12), 12 },
>> + { 0, GENMASK(17, 15), 15 },
>> + { 0, GENMASK(20, 18), 18 },
>> + { 0, GENMASK(23, 21), 21 },
>> + { 0, GENMASK(26, 24), 24 },
>> + { 0, GENMASK(29, 27), 27 },
>> + /* STM32H7_ADC_SMPR2, smpr[] index, mask, shift for SMP10 to SMP19 */
>> + { 1, GENMASK(2, 0), 0 },
>> + { 1, GENMASK(5, 3), 3 },
>> + { 1, GENMASK(8, 6), 6 },
>> + { 1, GENMASK(11, 9), 9 },
>> + { 1, GENMASK(14, 12), 12 },
>> + { 1, GENMASK(17, 15), 15 },
>> + { 1, GENMASK(20, 18), 18 },
>> + { 1, GENMASK(23, 21), 21 },
>> + { 1, GENMASK(26, 24), 24 },
>> + { 1, GENMASK(29, 27), 27 },
>> +};
>> +
>> +/* STM32H7 programmable sampling time (ADC clock cycles, rounded down) */
>> +static const unsigned int stm32h7_adc_smp_cycles[STM32_ADC_MAX_SMP + 1] = {
>> + 1, 2, 8, 16, 32, 64, 387, 810,
>> +};
>> +
>> static const struct stm32_adc_regspec stm32h7_adc_regspec = {
>> .dr = STM32H7_ADC_DR,
>> .ier_eoc = { STM32H7_ADC_IER, STM32H7_EOCIE },
>> @@ -492,6 +572,8 @@ struct stm32_adc_info {
>> .extsel = { STM32H7_ADC_CFGR, STM32H7_EXTSEL_MASK,
>> STM32H7_EXTSEL_SHIFT },
>> .res = { STM32H7_ADC_CFGR, STM32H7_RES_MASK, STM32H7_RES_SHIFT },
>> + .smpr = { STM32H7_ADC_SMPR1, STM32H7_ADC_SMPR2 },
>> + .smp_bits = stm32h7_smp_bits,
>> };
>>
>> /**
>> @@ -933,6 +1015,7 @@ static void stm32h7_adc_unprepare(struct stm32_adc *adc)
>> * @scan_mask: channels to be converted
>> *
>> * Conversion sequence :
>> + * Apply sampling time settings for all channels.
>> * Configure ADC scan sequence based on selected channels in scan_mask.
>> * Add channels to SQR registers, from scan_mask LSB to MSB, then
>> * program sequence len.
>> @@ -946,6 +1029,10 @@ static int stm32_adc_conf_scan_seq(struct iio_dev *indio_dev,
>> u32 val, bit;
>> int i = 0;
>>
>> + /* Apply sampling time settings */
>> + stm32_adc_writel(adc, adc->cfg->regs->smpr[0], adc->smpr_val[0]);
>> + stm32_adc_writel(adc, adc->cfg->regs->smpr[1], adc->smpr_val[1]);
>> +
>> for_each_set_bit(bit, scan_mask, indio_dev->masklength) {
>> chan = indio_dev->channels + bit;
>> /*
>> @@ -1079,6 +1166,7 @@ static int stm32_adc_get_trig_pol(struct iio_dev *indio_dev,
>> * @res: conversion result
>> *
>> * The function performs a single conversion on a given channel:
>> + * - Apply sampling time settings
>> * - Program sequencer with one channel (e.g. in SQ1 with len = 1)
>> * - Use SW trigger
>> * - Start conversion, then wait for interrupt completion.
>> @@ -1103,6 +1191,10 @@ static int stm32_adc_single_conv(struct iio_dev *indio_dev,
>> return ret;
>> }
>>
>> + /* Apply sampling time settings */
>> + stm32_adc_writel(adc, regs->smpr[0], adc->smpr_val[0]);
>> + stm32_adc_writel(adc, regs->smpr[1], adc->smpr_val[1]);
>> +
>> /* Program chan number in regular sequence (SQ1) */
>> val = stm32_adc_readl(adc, regs->sqr[1].reg);
>> val &= ~regs->sqr[1].mask;
>> @@ -1507,10 +1599,28 @@ static int stm32_adc_of_get_resolution(struct iio_dev *indio_dev)
>> return 0;
>> }
>>
>> +static void stm32_adc_smpr_init(struct stm32_adc *adc, int channel, u32 smp_ns)
>> +{
>> + const struct stm32_adc_regs *smpr = &adc->cfg->regs->smp_bits[channel];
>> + u32 period_ns, shift = smpr->shift, mask = smpr->mask;
>> + unsigned int smp, r = smpr->reg;
>> +
>> + /* Determine sampling time (ADC clock cycles) */
>> + period_ns = NSEC_PER_SEC / adc->common->rate;
>> + for (smp = 0; smp <= STM32_ADC_MAX_SMP; smp++)
>> + if ((period_ns * adc->cfg->smp_cycles[smp]) >= smp_ns)
>> + break;
>> + if (smp > STM32_ADC_MAX_SMP)
>> + smp = STM32_ADC_MAX_SMP;
>> +
>> + /* pre-build sampling time registers (e.g. smpr1, smpr2) */
>> + adc->smpr_val[r] = (adc->smpr_val[r] & ~mask) | (smp << shift);
>> +}
>> +
>> static void stm32_adc_chan_init_one(struct iio_dev *indio_dev,
>> struct iio_chan_spec *chan,
>> const struct stm32_adc_chan_spec *channel,
>> - int scan_index)
>> + int scan_index, u32 smp)
>> {
>> struct stm32_adc *adc = iio_priv(indio_dev);
>>
>> @@ -1526,6 +1636,9 @@ static void stm32_adc_chan_init_one(struct iio_dev *indio_dev,
>> 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);
>> }
>> @@ -1538,8 +1651,8 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
>> struct property *prop;
>> const __be32 *cur;
>> struct iio_chan_spec *channels;
>> - int scan_index = 0, num_channels;
>> - u32 val;
>> + int scan_index = 0, num_channels, ret;
>> + u32 val, smp = 0;
>>
>> num_channels = of_property_count_u32_elems(node, "st,adc-channels");
>> if (num_channels < 0 ||
>> @@ -1548,6 +1661,13 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
>> return num_channels < 0 ? num_channels : -EINVAL;
>> }
>>
>> + /* Optional sample time is provided either for each, or all channels */
>> + ret = of_property_count_u32_elems(node, "min-sample-time");
>> + if (ret > 1 && ret != num_channels) {
>> + dev_err(&indio_dev->dev, "Invalid min-sample-time\n");
>> + return -EINVAL;
>> + }
>> +
>> channels = devm_kcalloc(&indio_dev->dev, num_channels,
>> sizeof(struct iio_chan_spec), GFP_KERNEL);
>> if (!channels)
>> @@ -1558,9 +1678,13 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
>> dev_err(&indio_dev->dev, "Invalid channel %d\n", val);
>> return -EINVAL;
>> }
>> +
>> + of_property_read_u32_index(node, "min-sample-time", scan_index,
>> + &smp);
>> +
> I might be missing something, but doesn't this fail for the single shared
> value case?
Yes, this fails in the single shared value case, with index >= 1.
Rewinding... when index is 0, it picks up 1st (shared) value.
Then fails for other index values.
of_property_read_u32_index() documentation states out value remains
untouched in case of error:
/**
* of_property_read_u32_index...
[...]
* The out_value is modified only if a valid u32 value can be decoded.
This is what I use here (as far as I tested it, it worked like a charm).
Do you wish I add a comment to describe this ?
Please let me know,
Best Regards,
Fabrice
>> stm32_adc_chan_init_one(indio_dev, &channels[scan_index],
>> &adc_info->channels[val],
>> - scan_index);
>> + scan_index, smp);
>> scan_index++;
>> }
>>
>> @@ -1755,6 +1879,7 @@ static int stm32_adc_remove(struct platform_device *pdev)
>> .clk_required = true,
>> .start_conv = stm32f4_adc_start_conv,
>> .stop_conv = stm32f4_adc_stop_conv,
>> + .smp_cycles = stm32f4_adc_smp_cycles,
>> };
>>
>> static const struct stm32_adc_cfg stm32h7_adc_cfg = {
>> @@ -1766,6 +1891,7 @@ static int stm32_adc_remove(struct platform_device *pdev)
>> .stop_conv = stm32h7_adc_stop_conv,
>> .prepare = stm32h7_adc_prepare,
>> .unprepare = stm32h7_adc_unprepare,
>> + .smp_cycles = stm32h7_adc_smp_cycles,
>> };
>>
>> static const struct of_device_id stm32_adc_of_match[] = {
>
^ permalink raw reply [flat|nested] 39+ messages in thread* [PATCH 3/3] iio: adc: stm32: add optional min-sample-time
@ 2017-07-24 15:06 ` Jonathan Cameron
0 siblings, 0 replies; 39+ messages in thread
From: Jonathan Cameron @ 2017-07-24 15:06 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, 24 Jul 2017 10:16:00 +0200
Fabrice Gasnier <fabrice.gasnier@st.com> wrote:
> On 07/23/2017 01:00 PM, Jonathan Cameron wrote:
> > On Tue, 18 Jul 2017 14:35:32 +0200
> > Fabrice Gasnier <fabrice.gasnier@st.com> wrote:
> >
> >> STM32 ADC allows each channel to be sampled with a different sampling time,
> >> by setting SMPR registers. Basically, value depends on local electrical
> >> properties. Selecting correct value for sampling time highly depends on
> >> analog source impedance. There is a manual that may help in this process:
> >> 'How to get the best ADC accuracy in STM32...'
> >>
> >> This patch allows to configure min-sample-time via device tree, either for:
> >> - all channels at once:
> >> min-sample-time = <10000>; /* nanosecs */
> >>
> >> - independently for each channel (must match "st,adc-channels" list):
> >> st,adc-channels = <0 1>;
> >> min-sample-time = <5000 10000>; /* nanosecs */
> >>
> >> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> >
> > On question inline which may well just be down to me missing what
> > happens when you query index element 2 from a 1 element device tree
> > array.
>
> Hi Jonathan,
>
> I should probably comment on it, please see inline.
>
<snip>
> >> }
> >> @@ -1538,8 +1651,8 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
> >> struct property *prop;
> >> const __be32 *cur;
> >> struct iio_chan_spec *channels;
> >> - int scan_index = 0, num_channels;
> >> - u32 val;
> >> + int scan_index = 0, num_channels, ret;
> >> + u32 val, smp = 0;
> >>
> >> num_channels = of_property_count_u32_elems(node, "st,adc-channels");
> >> if (num_channels < 0 ||
> >> @@ -1548,6 +1661,13 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
> >> return num_channels < 0 ? num_channels : -EINVAL;
> >> }
> >>
> >> + /* Optional sample time is provided either for each, or all channels */
> >> + ret = of_property_count_u32_elems(node, "min-sample-time");
> >> + if (ret > 1 && ret != num_channels) {
> >> + dev_err(&indio_dev->dev, "Invalid min-sample-time\n");
> >> + return -EINVAL;
> >> + }
> >> +
> >> channels = devm_kcalloc(&indio_dev->dev, num_channels,
> >> sizeof(struct iio_chan_spec), GFP_KERNEL);
> >> if (!channels)
> >> @@ -1558,9 +1678,13 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
> >> dev_err(&indio_dev->dev, "Invalid channel %d\n", val);
> >> return -EINVAL;
> >> }
> >> +
> >> + of_property_read_u32_index(node, "min-sample-time", scan_index,
> >> + &smp);
> >> +
> > I might be missing something, but doesn't this fail for the single shared
> > value case?
>
> Yes, this fails in the single shared value case, with index >= 1.
> Rewinding... when index is 0, it picks up 1st (shared) value.
> Then fails for other index values.
>
> of_property_read_u32_index() documentation states out value remains
> untouched in case of error:
> /**
> * of_property_read_u32_index...
> [...]
> * The out_value is modified only if a valid u32 value can be decoded.
>
> This is what I use here (as far as I tested it, it worked like a charm).
> Do you wish I add a comment to describe this ?
Probably best or I'll forget sometime down the line and think it looks
crazy again ;)
Thanks for the explanation.
Jonathan
>
> Please let me know,
> Best Regards,
> Fabrice
>
> >> stm32_adc_chan_init_one(indio_dev, &channels[scan_index],
> >> &adc_info->channels[val],
> >> - scan_index);
> >> + scan_index, smp);
> >> scan_index++;
> >> }
> >>
> >> @@ -1755,6 +1879,7 @@ static int stm32_adc_remove(struct platform_device *pdev)
> >> .clk_required = true,
> >> .start_conv = stm32f4_adc_start_conv,
> >> .stop_conv = stm32f4_adc_stop_conv,
> >> + .smp_cycles = stm32f4_adc_smp_cycles,
> >> };
> >>
> >> static const struct stm32_adc_cfg stm32h7_adc_cfg = {
> >> @@ -1766,6 +1891,7 @@ static int stm32_adc_remove(struct platform_device *pdev)
> >> .stop_conv = stm32h7_adc_stop_conv,
> >> .prepare = stm32h7_adc_prepare,
> >> .unprepare = stm32h7_adc_unprepare,
> >> + .smp_cycles = stm32h7_adc_smp_cycles,
> >> };
> >>
> >> static const struct of_device_id stm32_adc_of_match[] = {
> >
> --
> 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
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [PATCH 3/3] iio: adc: stm32: add optional min-sample-time
@ 2017-07-24 15:06 ` Jonathan Cameron
0 siblings, 0 replies; 39+ messages in thread
From: Jonathan Cameron @ 2017-07-24 15:06 UTC (permalink / raw)
To: Fabrice Gasnier
Cc: Jonathan Cameron, robh+dt, linux, mark.rutland, mcoquelin.stm32,
alexandre.torgue, lars, knaack.h, pmeerw, benjamin.gaignard,
benjamin.gaignard, linux-iio, devicetree, linux-arm-kernel,
linux-kernel
On Mon, 24 Jul 2017 10:16:00 +0200
Fabrice Gasnier <fabrice.gasnier@st.com> wrote:
> On 07/23/2017 01:00 PM, Jonathan Cameron wrote:
> > On Tue, 18 Jul 2017 14:35:32 +0200
> > Fabrice Gasnier <fabrice.gasnier@st.com> wrote:
> >
> >> STM32 ADC allows each channel to be sampled with a different sampling time,
> >> by setting SMPR registers. Basically, value depends on local electrical
> >> properties. Selecting correct value for sampling time highly depends on
> >> analog source impedance. There is a manual that may help in this process:
> >> 'How to get the best ADC accuracy in STM32...'
> >>
> >> This patch allows to configure min-sample-time via device tree, either for:
> >> - all channels at once:
> >> min-sample-time = <10000>; /* nanosecs */
> >>
> >> - independently for each channel (must match "st,adc-channels" list):
> >> st,adc-channels = <0 1>;
> >> min-sample-time = <5000 10000>; /* nanosecs */
> >>
> >> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> >
> > On question inline which may well just be down to me missing what
> > happens when you query index element 2 from a 1 element device tree
> > array.
>
> Hi Jonathan,
>
> I should probably comment on it, please see inline.
>
<snip>
> >> }
> >> @@ -1538,8 +1651,8 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
> >> struct property *prop;
> >> const __be32 *cur;
> >> struct iio_chan_spec *channels;
> >> - int scan_index = 0, num_channels;
> >> - u32 val;
> >> + int scan_index = 0, num_channels, ret;
> >> + u32 val, smp = 0;
> >>
> >> num_channels = of_property_count_u32_elems(node, "st,adc-channels");
> >> if (num_channels < 0 ||
> >> @@ -1548,6 +1661,13 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
> >> return num_channels < 0 ? num_channels : -EINVAL;
> >> }
> >>
> >> + /* Optional sample time is provided either for each, or all channels */
> >> + ret = of_property_count_u32_elems(node, "min-sample-time");
> >> + if (ret > 1 && ret != num_channels) {
> >> + dev_err(&indio_dev->dev, "Invalid min-sample-time\n");
> >> + return -EINVAL;
> >> + }
> >> +
> >> channels = devm_kcalloc(&indio_dev->dev, num_channels,
> >> sizeof(struct iio_chan_spec), GFP_KERNEL);
> >> if (!channels)
> >> @@ -1558,9 +1678,13 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
> >> dev_err(&indio_dev->dev, "Invalid channel %d\n", val);
> >> return -EINVAL;
> >> }
> >> +
> >> + of_property_read_u32_index(node, "min-sample-time", scan_index,
> >> + &smp);
> >> +
> > I might be missing something, but doesn't this fail for the single shared
> > value case?
>
> Yes, this fails in the single shared value case, with index >= 1.
> Rewinding... when index is 0, it picks up 1st (shared) value.
> Then fails for other index values.
>
> of_property_read_u32_index() documentation states out value remains
> untouched in case of error:
> /**
> * of_property_read_u32_index...
> [...]
> * The out_value is modified only if a valid u32 value can be decoded.
>
> This is what I use here (as far as I tested it, it worked like a charm).
> Do you wish I add a comment to describe this ?
Probably best or I'll forget sometime down the line and think it looks
crazy again ;)
Thanks for the explanation.
Jonathan
>
> Please let me know,
> Best Regards,
> Fabrice
>
> >> stm32_adc_chan_init_one(indio_dev, &channels[scan_index],
> >> &adc_info->channels[val],
> >> - scan_index);
> >> + scan_index, smp);
> >> scan_index++;
> >> }
> >>
> >> @@ -1755,6 +1879,7 @@ static int stm32_adc_remove(struct platform_device *pdev)
> >> .clk_required = true,
> >> .start_conv = stm32f4_adc_start_conv,
> >> .stop_conv = stm32f4_adc_stop_conv,
> >> + .smp_cycles = stm32f4_adc_smp_cycles,
> >> };
> >>
> >> static const struct stm32_adc_cfg stm32h7_adc_cfg = {
> >> @@ -1766,6 +1891,7 @@ static int stm32_adc_remove(struct platform_device *pdev)
> >> .stop_conv = stm32h7_adc_stop_conv,
> >> .prepare = stm32h7_adc_prepare,
> >> .unprepare = stm32h7_adc_unprepare,
> >> + .smp_cycles = stm32h7_adc_smp_cycles,
> >> };
> >>
> >> static const struct of_device_id stm32_adc_of_match[] = {
> >
> --
> 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] 39+ messages in thread* Re: [PATCH 3/3] iio: adc: stm32: add optional min-sample-time
@ 2017-07-24 15:06 ` Jonathan Cameron
0 siblings, 0 replies; 39+ messages in thread
From: Jonathan Cameron @ 2017-07-24 15:06 UTC (permalink / raw)
To: Fabrice Gasnier
Cc: Jonathan Cameron, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
linux-I+IVW8TIWO2tmTQ+vhA3Yw, mark.rutland-5wv7dgnIgG8,
mcoquelin.stm32-Re5JQEeQqe8AvxtiuMwx3w,
alexandre.torgue-qxv4g6HH51o, lars-Qo5EllUWu/uELgA04lAiVw,
knaack.h-Mmb7MZpHnFY, pmeerw-jW+XmwGofnusTnJN9+BGXg,
benjamin.gaignard-QSEj5FYQhm4dnm+yROfE0A,
benjamin.gaignard-qxv4g6HH51o, linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
On Mon, 24 Jul 2017 10:16:00 +0200
Fabrice Gasnier <fabrice.gasnier-qxv4g6HH51o@public.gmane.org> wrote:
> On 07/23/2017 01:00 PM, Jonathan Cameron wrote:
> > On Tue, 18 Jul 2017 14:35:32 +0200
> > Fabrice Gasnier <fabrice.gasnier-qxv4g6HH51o@public.gmane.org> wrote:
> >
> >> STM32 ADC allows each channel to be sampled with a different sampling time,
> >> by setting SMPR registers. Basically, value depends on local electrical
> >> properties. Selecting correct value for sampling time highly depends on
> >> analog source impedance. There is a manual that may help in this process:
> >> 'How to get the best ADC accuracy in STM32...'
> >>
> >> This patch allows to configure min-sample-time via device tree, either for:
> >> - all channels at once:
> >> min-sample-time = <10000>; /* nanosecs */
> >>
> >> - independently for each channel (must match "st,adc-channels" list):
> >> st,adc-channels = <0 1>;
> >> min-sample-time = <5000 10000>; /* nanosecs */
> >>
> >> Signed-off-by: Fabrice Gasnier <fabrice.gasnier-qxv4g6HH51o@public.gmane.org>
> >
> > On question inline which may well just be down to me missing what
> > happens when you query index element 2 from a 1 element device tree
> > array.
>
> Hi Jonathan,
>
> I should probably comment on it, please see inline.
>
<snip>
> >> }
> >> @@ -1538,8 +1651,8 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
> >> struct property *prop;
> >> const __be32 *cur;
> >> struct iio_chan_spec *channels;
> >> - int scan_index = 0, num_channels;
> >> - u32 val;
> >> + int scan_index = 0, num_channels, ret;
> >> + u32 val, smp = 0;
> >>
> >> num_channels = of_property_count_u32_elems(node, "st,adc-channels");
> >> if (num_channels < 0 ||
> >> @@ -1548,6 +1661,13 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
> >> return num_channels < 0 ? num_channels : -EINVAL;
> >> }
> >>
> >> + /* Optional sample time is provided either for each, or all channels */
> >> + ret = of_property_count_u32_elems(node, "min-sample-time");
> >> + if (ret > 1 && ret != num_channels) {
> >> + dev_err(&indio_dev->dev, "Invalid min-sample-time\n");
> >> + return -EINVAL;
> >> + }
> >> +
> >> channels = devm_kcalloc(&indio_dev->dev, num_channels,
> >> sizeof(struct iio_chan_spec), GFP_KERNEL);
> >> if (!channels)
> >> @@ -1558,9 +1678,13 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
> >> dev_err(&indio_dev->dev, "Invalid channel %d\n", val);
> >> return -EINVAL;
> >> }
> >> +
> >> + of_property_read_u32_index(node, "min-sample-time", scan_index,
> >> + &smp);
> >> +
> > I might be missing something, but doesn't this fail for the single shared
> > value case?
>
> Yes, this fails in the single shared value case, with index >= 1.
> Rewinding... when index is 0, it picks up 1st (shared) value.
> Then fails for other index values.
>
> of_property_read_u32_index() documentation states out value remains
> untouched in case of error:
> /**
> * of_property_read_u32_index...
> [...]
> * The out_value is modified only if a valid u32 value can be decoded.
>
> This is what I use here (as far as I tested it, it worked like a charm).
> Do you wish I add a comment to describe this ?
Probably best or I'll forget sometime down the line and think it looks
crazy again ;)
Thanks for the explanation.
Jonathan
>
> Please let me know,
> Best Regards,
> Fabrice
>
> >> stm32_adc_chan_init_one(indio_dev, &channels[scan_index],
> >> &adc_info->channels[val],
> >> - scan_index);
> >> + scan_index, smp);
> >> scan_index++;
> >> }
> >>
> >> @@ -1755,6 +1879,7 @@ static int stm32_adc_remove(struct platform_device *pdev)
> >> .clk_required = true,
> >> .start_conv = stm32f4_adc_start_conv,
> >> .stop_conv = stm32f4_adc_stop_conv,
> >> + .smp_cycles = stm32f4_adc_smp_cycles,
> >> };
> >>
> >> static const struct stm32_adc_cfg stm32h7_adc_cfg = {
> >> @@ -1766,6 +1891,7 @@ static int stm32_adc_remove(struct platform_device *pdev)
> >> .stop_conv = stm32h7_adc_stop_conv,
> >> .prepare = stm32h7_adc_prepare,
> >> .unprepare = stm32h7_adc_unprepare,
> >> + .smp_cycles = stm32h7_adc_smp_cycles,
> >> };
> >>
> >> static const struct of_device_id stm32_adc_of_match[] = {
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 39+ messages in thread