Add scaling support to STM32 DFSDM.
Signed-off-by: Olivier Moysan <olivier.moysan@foss.st.com>
---
drivers/iio/adc/Kconfig | 1 +
drivers/iio/adc/stm32-dfsdm-adc.c | 94 ++++++++++++++++++++++++++++++-
2 files changed, 92 insertions(+), 3 deletions(-)
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index f3dfdaa80678..858ae8161fa4 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -1223,6 +1223,7 @@ config STM32_DFSDM_ADC
select IIO_BUFFER
select IIO_BUFFER_HW_CONSUMER
select IIO_TRIGGERED_BUFFER
+ select IIO_BACKEND
help
Select this option to support ADCSigma delta modulator for
STMicroelectronics STM32 digital filter for sigma delta converter.
diff --git a/drivers/iio/adc/stm32-dfsdm-adc.c b/drivers/iio/adc/stm32-dfsdm-adc.c
index 69b4764d7cba..93bf6035bd6d 100644
--- a/drivers/iio/adc/stm32-dfsdm-adc.c
+++ b/drivers/iio/adc/stm32-dfsdm-adc.c
@@ -9,6 +9,7 @@
#include <linux/dmaengine.h>
#include <linux/dma-mapping.h>
#include <linux/iio/adc/stm32-dfsdm-adc.h>
+#include <linux/iio/backend.h>
#include <linux/iio/buffer.h>
#include <linux/iio/hw-consumer.h>
#include <linux/iio/sysfs.h>
@@ -78,6 +79,7 @@ struct stm32_dfsdm_adc {
/* ADC specific */
unsigned int oversamp;
struct iio_hw_consumer *hwc;
+ struct iio_backend **backend;
struct completion completion;
u32 *buffer;
@@ -672,6 +674,8 @@ static int stm32_dfsdm_generic_channel_parse_of(struct stm32_dfsdm *dfsdm,
struct fwnode_handle *node)
{
struct stm32_dfsdm_channel *df_ch;
+ struct stm32_dfsdm_adc *adc = iio_priv(indio_dev);
+ struct iio_backend *backend;
const char *of_str;
int ret, val;
@@ -721,6 +725,14 @@ static int stm32_dfsdm_generic_channel_parse_of(struct stm32_dfsdm *dfsdm,
if (ret != -EINVAL)
df_ch->alt_si = 0;
+ if (adc->dev_data->type == DFSDM_IIO) {
+ backend = devm_iio_backend_subnode_get(&indio_dev->dev, NULL, node);
+ if (IS_ERR(backend))
+ return dev_err_probe(&indio_dev->dev, PTR_ERR(backend),
+ "Failed to get backend\n");
+ adc->backend[ch->scan_index] = backend;
+ }
+
return 0;
}
@@ -1060,7 +1072,7 @@ static int stm32_dfsdm_update_scan_mode(struct iio_dev *indio_dev,
static int stm32_dfsdm_postenable(struct iio_dev *indio_dev)
{
struct stm32_dfsdm_adc *adc = iio_priv(indio_dev);
- int ret;
+ int i = 0, ret;
/* Reset adc buffer index */
adc->bufi = 0;
@@ -1071,6 +1083,15 @@ static int stm32_dfsdm_postenable(struct iio_dev *indio_dev)
return ret;
}
+ if (adc->backend) {
+ while (adc->backend[i]) {
+ ret = iio_backend_enable(&indio_dev->dev, adc->backend[i]);
+ if (ret < 0)
+ return ret;
+ i++;
+ }
+ }
+
ret = stm32_dfsdm_start_dfsdm(adc->dfsdm);
if (ret < 0)
goto err_stop_hwc;
@@ -1103,6 +1124,7 @@ static int stm32_dfsdm_postenable(struct iio_dev *indio_dev)
static int stm32_dfsdm_predisable(struct iio_dev *indio_dev)
{
struct stm32_dfsdm_adc *adc = iio_priv(indio_dev);
+ int i = 0;
stm32_dfsdm_stop_conv(indio_dev);
@@ -1110,6 +1132,13 @@ static int stm32_dfsdm_predisable(struct iio_dev *indio_dev)
stm32_dfsdm_stop_dfsdm(adc->dfsdm);
+ if (adc->backend) {
+ while (adc->backend[i]) {
+ iio_backend_disable(&indio_dev->dev, adc->backend[i]);
+ i++;
+ }
+ }
+
if (adc->hwc)
iio_hw_consumer_disable(adc->hwc);
@@ -1282,7 +1311,14 @@ static int stm32_dfsdm_read_raw(struct iio_dev *indio_dev,
int *val2, long mask)
{
struct stm32_dfsdm_adc *adc = iio_priv(indio_dev);
- int ret;
+
+ struct stm32_dfsdm_filter *fl = &adc->dfsdm->fl_list[adc->fl_id];
+ struct stm32_dfsdm_filter_osr *flo = &fl->flo[fl->fast];
+ u32 max = flo->max << (flo->lshift - chan->scan_type.shift);
+ int ret, idx = chan->scan_index;
+
+ if (flo->lshift < chan->scan_type.shift)
+ max = flo->max >> (chan->scan_type.shift - flo->lshift);
switch (mask) {
case IIO_CHAN_INFO_RAW:
@@ -1291,6 +1327,8 @@ static int stm32_dfsdm_read_raw(struct iio_dev *indio_dev,
return ret;
if (adc->hwc)
ret = iio_hw_consumer_enable(adc->hwc);
+ if (adc->backend[idx])
+ ret = iio_backend_enable(&indio_dev->dev, adc->backend[idx]);
if (ret < 0) {
dev_err(&indio_dev->dev,
"%s: IIO enable failed (channel %d)\n",
@@ -1301,6 +1339,8 @@ static int stm32_dfsdm_read_raw(struct iio_dev *indio_dev,
ret = stm32_dfsdm_single_conv(indio_dev, chan, val);
if (adc->hwc)
iio_hw_consumer_disable(adc->hwc);
+ if (adc->backend[idx])
+ iio_backend_disable(&indio_dev->dev, adc->backend[idx]);
if (ret < 0) {
dev_err(&indio_dev->dev,
"%s: Conversion failed (channel %d)\n",
@@ -1320,6 +1360,45 @@ static int stm32_dfsdm_read_raw(struct iio_dev *indio_dev,
*val = adc->sample_freq;
return IIO_VAL_INT;
+
+ case IIO_CHAN_INFO_SCALE:
+ /*
+ * Scale is expressed in mV.
+ * When fast mode is disabled, actual resolution may be lower
+ * than 2^n, where n=realbits-1.
+ * This leads to underestimating input voltage. To
+ * compensate this deviation, the voltage reference can be
+ * corrected with a factor = realbits resolution / actual max
+ */
+ if (adc->backend[idx]) {
+ iio_backend_read_raw(adc->backend[idx], val, val2, mask);
+
+ *val = div_u64((u64)*val * (u64)BIT(DFSDM_DATA_RES - 1), max);
+ *val2 = chan->scan_type.realbits;
+ if (chan->differential)
+ *val *= 2;
+ }
+ return IIO_VAL_FRACTIONAL_LOG2;
+
+ case IIO_CHAN_INFO_OFFSET:
+ /*
+ * DFSDM output data are in the range [-2^n,2^n],
+ * with n=realbits-1.
+ * - Differential modulator:
+ * Offset correspond to SD modulator offset.
+ * - Single ended modulator:
+ * Input is in [0V,Vref] range, where 0V corresponds to -2^n, and Vref to 2^n.
+ * Add 2^n to offset. (i.e. middle of input range)
+ * offset = offset(sd) * vref / res(sd) * max / vref.
+ */
+ if (adc->backend[idx]) {
+ iio_backend_read_raw(adc->backend[idx], val, val2, mask);
+
+ *val = div_u64((u64)max * *val, BIT(*val2 - 1));
+ if (!chan->differential)
+ *val += max;
+ }
+ return IIO_VAL_INT;
}
return -EINVAL;
@@ -1449,7 +1528,15 @@ static int stm32_dfsdm_adc_chan_init_one(struct iio_dev *indio_dev, struct iio_c
* IIO_CHAN_INFO_RAW: used to compute regular conversion
* IIO_CHAN_INFO_OVERSAMPLING_RATIO: used to set oversampling
*/
- ch->info_mask_separate = BIT(IIO_CHAN_INFO_RAW);
+ if (child) {
+ ch->info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+ BIT(IIO_CHAN_INFO_SCALE) |
+ BIT(IIO_CHAN_INFO_OFFSET);
+ } else {
+ /* Legacy. Scaling not supported */
+ ch->info_mask_separate = BIT(IIO_CHAN_INFO_RAW);
+ }
+
ch->info_mask_shared_by_all = BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO) |
BIT(IIO_CHAN_INFO_SAMP_FREQ);
@@ -1816,3 +1903,4 @@ module_platform_driver(stm32_dfsdm_adc_driver);
MODULE_DESCRIPTION("STM32 sigma delta ADC");
MODULE_AUTHOR("Arnaud Pouliquen <arnaud.pouliquen@st.com>");
MODULE_LICENSE("GPL v2");
+MODULE_IMPORT_NS(IIO_BACKEND);
--
2.25.1On Tue, 18 Jun 2024 18:08:34 +0200
Olivier Moysan <olivier.moysan@foss.st.com> wrote:
> Add scaling support to STM32 DFSDM.
Perhaps a short description here of how this works? Where does the scale come
from, what assumptions are made etc.
>
> Signed-off-by: Olivier Moysan <olivier.moysan@foss.st.com>
Some minor stuff.
> diff --git a/drivers/iio/adc/stm32-dfsdm-adc.c b/drivers/iio/adc/stm32-dfsdm-adc.c
> index 69b4764d7cba..93bf6035bd6d 100644
> --- a/drivers/iio/adc/stm32-dfsdm-adc.c
> +++ b/drivers/iio/adc/stm32-dfsdm-adc.c
urn 0;
> }
>
> @@ -1060,7 +1072,7 @@ static int stm32_dfsdm_update_scan_mode(struct iio_dev *indio_dev,
> static int stm32_dfsdm_postenable(struct iio_dev *indio_dev)
> {
> struct stm32_dfsdm_adc *adc = iio_priv(indio_dev);
> - int ret;
> + int i = 0, ret;
Don't mix assigned and unassigned variable declarations. Just use a separate line
as this can mean subtle assignment or lack of assignment issues sneak in.
>
> /* Reset adc buffer index */
> adc->bufi = 0;
> @@ -1071,6 +1083,15 @@ static int stm32_dfsdm_postenable(struct iio_dev *indio_dev)
> return ret;
> }
>
> + if (adc->backend) {
> + while (adc->backend[i]) {
Could do similar to the suggestion below.
Mostly I don't like the index variable manipulation.
> + ret = iio_backend_enable(&indio_dev->dev, adc->backend[i]);
> + if (ret < 0)
> + return ret;
> + i++;
> + }
> + }
> +
> ret = stm32_dfsdm_start_dfsdm(adc->dfsdm);
> if (ret < 0)
> goto err_stop_hwc;
> @@ -1103,6 +1124,7 @@ static int stm32_dfsdm_postenable(struct iio_dev *indio_dev)
> static int stm32_dfsdm_predisable(struct iio_dev *indio_dev)
> {
> struct stm32_dfsdm_adc *adc = iio_priv(indio_dev);
> + int i = 0;
>
> stm32_dfsdm_stop_conv(indio_dev);
>
> @@ -1110,6 +1132,13 @@ static int stm32_dfsdm_predisable(struct iio_dev *indio_dev)
>
> stm32_dfsdm_stop_dfsdm(adc->dfsdm);
>
> + if (adc->backend) {
> + while (adc->backend[i]) {
> + iio_backend_disable(&indio_dev->dev, adc->backend[i]);
> + i++;
> + }
Something like
struct iio_backend **be = &adc->backend[0];
do {
iio_backend_disable(&indio-dev->dev, be);
} while (be++);
maybe. Up to you.
> + }
> @@ -1320,6 +1360,45 @@ static int stm32_dfsdm_read_raw(struct iio_dev *indio_dev,
> *val = adc->sample_freq;
>
> return IIO_VAL_INT;
> +
> + case IIO_CHAN_INFO_SCALE:
> + /*
> + * Scale is expressed in mV.
> + * When fast mode is disabled, actual resolution may be lower
> + * than 2^n, where n=realbits-1.
As below, use a few more spaces.
> + * This leads to underestimating input voltage. To
> + * compensate this deviation, the voltage reference can be
> + * corrected with a factor = realbits resolution / actual max
> + */
> + if (adc->backend[idx]) {
> + iio_backend_read_raw(adc->backend[idx], val, val2, mask);
> +
> + *val = div_u64((u64)*val * (u64)BIT(DFSDM_DATA_RES - 1), max);
> + *val2 = chan->scan_type.realbits;
> + if (chan->differential)
> + *val *= 2;
> + }
> + return IIO_VAL_FRACTIONAL_LOG2;
> +
> + case IIO_CHAN_INFO_OFFSET:
> + /*
> + * DFSDM output data are in the range [-2^n,2^n],
Use a few more spaces. [-2^2, 2^n]
> + * with n=realbits-1.
n = realbits - 1
Just to keep it closer to the C coding style.
> + * - Differential modulator:
> + * Offset correspond to SD modulator offset.
> + * - Single ended modulator:
> + * Input is in [0V,Vref] range, where 0V corresponds to -2^n, and Vref to 2^n.
Avoid that long line with a suitable line break.
> + * Add 2^n to offset. (i.e. middle of input range)
> + * offset = offset(sd) * vref / res(sd) * max / vref.
> + */
> + if (adc->backend[idx]) {
> + iio_backend_read_raw(adc->backend[idx], val, val2, mask);
> +
> + *val = div_u64((u64)max * *val, BIT(*val2 - 1));
> + if (!chan->differential)
> + *val += max;
> + }
> + return IIO_VAL_INT;
> }
>
> return -EINVAL;
> @@ -1449,7 +1528,15 @@ static int stm32_dfsdm_adc_chan_init_one(struct iio_dev *indio_dev, struct iio_c
> * IIO_CHAN_INFO_RAW: used to compute regular conversion
> * IIO_CHAN_INFO_OVERSAMPLING_RATIO: used to set oversampling
> */
> - ch->info_mask_separate = BIT(IIO_CHAN_INFO_RAW);
> + if (child) {
> + ch->info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> + BIT(IIO_CHAN_INFO_SCALE) |
> + BIT(IIO_CHAN_INFO_OFFSET);
Indent looks a little odd. Maybe one more space neede?
> + } else {
> + /* Legacy. Scaling not supported */
> + ch->info_mask_separate = BIT(IIO_CHAN_INFO_RAW);
> + }
> +
> ch->info_mask_shared_by_all = BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO) |
> BIT(IIO_CHAN_INFO_SAMP_FREQ);
>
> @@ -1816,3 +1903,4 @@ module_platform_driver(stm32_dfsdm_adc_driver);
> MODULE_DESCRIPTION("STM32 sigma delta ADC");
> MODULE_AUTHOR("Arnaud Pouliquen <arnaud.pouliquen@st.com>");
> MODULE_LICENSE("GPL v2");
> +MODULE_IMPORT_NS(IIO_BACKEND);
Hi Jonathan,
On 6/23/24 17:21, Jonathan Cameron wrote:
> On Tue, 18 Jun 2024 18:08:34 +0200
> Olivier Moysan <olivier.moysan@foss.st.com> wrote:
>
>> Add scaling support to STM32 DFSDM.
> Perhaps a short description here of how this works? Where does the scale come
> from, what assumptions are made etc.
>>
Sure. Requires more explanations. Done
>> Signed-off-by: Olivier Moysan <olivier.moysan@foss.st.com>
>
> Some minor stuff.
>
>> diff --git a/drivers/iio/adc/stm32-dfsdm-adc.c b/drivers/iio/adc/stm32-dfsdm-adc.c
>> index 69b4764d7cba..93bf6035bd6d 100644
>> --- a/drivers/iio/adc/stm32-dfsdm-adc.c
>> +++ b/drivers/iio/adc/stm32-dfsdm-adc.c
> urn 0;
>> }
>>
>> @@ -1060,7 +1072,7 @@ static int stm32_dfsdm_update_scan_mode(struct iio_dev *indio_dev,
>> static int stm32_dfsdm_postenable(struct iio_dev *indio_dev)
>> {
>> struct stm32_dfsdm_adc *adc = iio_priv(indio_dev);
>> - int ret;
>> + int i = 0, ret;
>
> Don't mix assigned and unassigned variable declarations. Just use a separate line
> as this can mean subtle assignment or lack of assignment issues sneak in.
>
Ack
>>
>> /* Reset adc buffer index */
>> adc->bufi = 0;
>> @@ -1071,6 +1083,15 @@ static int stm32_dfsdm_postenable(struct iio_dev *indio_dev)
>> return ret;
>> }
>>
>> + if (adc->backend) {
>> + while (adc->backend[i]) {
>
> Could do similar to the suggestion below.
> Mostly I don't like the index variable manipulation.
>
>> + ret = iio_backend_enable(&indio_dev->dev, adc->backend[i]);
>> + if (ret < 0)
>> + return ret;
>> + i++;
>> + }
>> + }
>> +
>> ret = stm32_dfsdm_start_dfsdm(adc->dfsdm);
>> if (ret < 0)
>> goto err_stop_hwc;
>> @@ -1103,6 +1124,7 @@ static int stm32_dfsdm_postenable(struct iio_dev *indio_dev)
>> static int stm32_dfsdm_predisable(struct iio_dev *indio_dev)
>> {
>> struct stm32_dfsdm_adc *adc = iio_priv(indio_dev);
>> + int i = 0;
>>
>> stm32_dfsdm_stop_conv(indio_dev);
>>
>> @@ -1110,6 +1132,13 @@ static int stm32_dfsdm_predisable(struct iio_dev *indio_dev)
>>
>> stm32_dfsdm_stop_dfsdm(adc->dfsdm);
>>
>> + if (adc->backend) {
>> + while (adc->backend[i]) {
>> + iio_backend_disable(&indio_dev->dev, adc->backend[i]);
>> + i++;
>> + }
> Something like
> struct iio_backend **be = &adc->backend[0];
> do {
> iio_backend_disable(&indio-dev->dev, be);
> } while (be++);
>
> maybe. Up to you.
>
If you don't mind, I will keep indexes. Pointers here looks more
difficult to read, and pre/post incrementation in the loop condition can
easily introduce bugs, imho.
>
>> + }
>
>> @@ -1320,6 +1360,45 @@ static int stm32_dfsdm_read_raw(struct iio_dev *indio_dev,
>> *val = adc->sample_freq;
>>
>> return IIO_VAL_INT;
>> +
>> + case IIO_CHAN_INFO_SCALE:
>> + /*
>> + * Scale is expressed in mV.
>> + * When fast mode is disabled, actual resolution may be lower
>> + * than 2^n, where n=realbits-1.
>
> As below, use a few more spaces.
>
>> + * This leads to underestimating input voltage. To
>> + * compensate this deviation, the voltage reference can be
>> + * corrected with a factor = realbits resolution / actual max
>> + */
>> + if (adc->backend[idx]) {
>> + iio_backend_read_raw(adc->backend[idx], val, val2, mask);
>> +
>> + *val = div_u64((u64)*val * (u64)BIT(DFSDM_DATA_RES - 1), max);
>> + *val2 = chan->scan_type.realbits;
>> + if (chan->differential)
>> + *val *= 2;
>> + }
>> + return IIO_VAL_FRACTIONAL_LOG2;
>> +
>> + case IIO_CHAN_INFO_OFFSET:
>> + /*
>> + * DFSDM output data are in the range [-2^n,2^n],
> Use a few more spaces. [-2^2, 2^n]
>> + * with n=realbits-1.
> n = realbits - 1
>
> Just to keep it closer to the C coding style.
>
Done
>> + * - Differential modulator:
>> + * Offset correspond to SD modulator offset.
>> + * - Single ended modulator:
>> + * Input is in [0V,Vref] range, where 0V corresponds to -2^n, and Vref to 2^n.
>
> Avoid that long line with a suitable line break.
>
Ack
>> + * Add 2^n to offset. (i.e. middle of input range)
>> + * offset = offset(sd) * vref / res(sd) * max / vref.
>> + */
>> + if (adc->backend[idx]) {
>> + iio_backend_read_raw(adc->backend[idx], val, val2, mask);
>> +
>> + *val = div_u64((u64)max * *val, BIT(*val2 - 1));
>> + if (!chan->differential)
>> + *val += max;
>> + }
>> + return IIO_VAL_INT;
>> }
>>
>> return -EINVAL;
>> @@ -1449,7 +1528,15 @@ static int stm32_dfsdm_adc_chan_init_one(struct iio_dev *indio_dev, struct iio_c
>> * IIO_CHAN_INFO_RAW: used to compute regular conversion
>> * IIO_CHAN_INFO_OVERSAMPLING_RATIO: used to set oversampling
>> */
>> - ch->info_mask_separate = BIT(IIO_CHAN_INFO_RAW);
>> + if (child) {
>> + ch->info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
>> + BIT(IIO_CHAN_INFO_SCALE) |
>> + BIT(IIO_CHAN_INFO_OFFSET);
>
> Indent looks a little odd. Maybe one more space neede?
>
Ack
>> + } else {
>> + /* Legacy. Scaling not supported */
>> + ch->info_mask_separate = BIT(IIO_CHAN_INFO_RAW);
>> + }
>> +
>> ch->info_mask_shared_by_all = BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO) |
>> BIT(IIO_CHAN_INFO_SAMP_FREQ);
>>
>> @@ -1816,3 +1903,4 @@ module_platform_driver(stm32_dfsdm_adc_driver);
>> MODULE_DESCRIPTION("STM32 sigma delta ADC");
>> MODULE_AUTHOR("Arnaud Pouliquen <arnaud.pouliquen@st.com>");
>> MODULE_LICENSE("GPL v2");
>> +MODULE_IMPORT_NS(IIO_BACKEND);
>
BRs
Olivier
On Tue, 2024-06-18 at 18:08 +0200, Olivier Moysan wrote:
> Add scaling support to STM32 DFSDM.
>
> Signed-off-by: Olivier Moysan <olivier.moysan@foss.st.com>
> ---
Just some general comments...
Acked-by: Nuno Sa <nuno.sa@analog.com>
> drivers/iio/adc/Kconfig | 1 +
> drivers/iio/adc/stm32-dfsdm-adc.c | 94 ++++++++++++++++++++++++++++++-
> 2 files changed, 92 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index f3dfdaa80678..858ae8161fa4 100644
>
...
> @@ -1301,6 +1339,8 @@ static int stm32_dfsdm_read_raw(struct iio_dev *indio_dev,
> ret = stm32_dfsdm_single_conv(indio_dev, chan, val);
> if (adc->hwc)
> iio_hw_consumer_disable(adc->hwc);
> + if (adc->backend[idx])
> + iio_backend_disable(&indio_dev->dev, adc->backend[idx]);
> if (ret < 0) {
> dev_err(&indio_dev->dev,
> "%s: Conversion failed (channel %d)\n",
> @@ -1320,6 +1360,45 @@ static int stm32_dfsdm_read_raw(struct iio_dev *indio_dev,
> *val = adc->sample_freq;
>
> return IIO_VAL_INT;
> +
> + case IIO_CHAN_INFO_SCALE:
> + /*
> + * Scale is expressed in mV.
> + * When fast mode is disabled, actual resolution may be lower
> + * than 2^n, where n=realbits-1.
> + * This leads to underestimating input voltage. To
> + * compensate this deviation, the voltage reference can be
> + * corrected with a factor = realbits resolution / actual max
> + */
> + if (adc->backend[idx]) {
> + iio_backend_read_raw(adc->backend[idx], val, val2, mask);
> +
This is something that I've been thinking about since the beginning of backends
support. Basically having the "catch all" read_raw()/write_raw() or more dedicated
interfaces. For example, in your usecase I think it would make more sense to have
dedicated API's like iio_backend_read_scale() and iio_backend_read_offset() as you're
extending that functionality. Calling iio_backend_read_raw() seems a bit weird to me.
OTOH, iio_backend_read_raw() could be useful in cases where frontends call
iio_backend_extend_chan_spec() and may get some functionality they are not aware of.
And so, in the default: statement this "catch all" API could be used.
Anyways, this are really minor (likely pedantic) details and you kind of came first
and made the decision for me. As I don't really have strong feelings about it, I'm
fine with it as-is.
> + *val = div_u64((u64)*val * (u64)BIT(DFSDM_DATA_RES - 1),
> max);
> + *val2 = chan->scan_type.realbits;
> + if (chan->differential)
> + *val *= 2;
> + }
> + return IIO_VAL_FRACTIONAL_LOG2;
> +
> + case IIO_CHAN_INFO_OFFSET:
> + /*
> + * DFSDM output data are in the range [-2^n,2^n],
> + * with n=realbits-1.
> + * - Differential modulator:
> + * Offset correspond to SD modulator offset.
> + * - Single ended modulator:
> + * Input is in [0V,Vref] range, where 0V corresponds to -2^n, and
> Vref to 2^n.
> + * Add 2^n to offset. (i.e. middle of input range)
> + * offset = offset(sd) * vref / res(sd) * max / vref.
> + */
> + if (adc->backend[idx]) {
> + iio_backend_read_raw(adc->backend[idx], val, val2, mask);
> +
> + *val = div_u64((u64)max * *val, BIT(*val2 - 1));
> + if (!chan->differential)
> + *val += max;
> + }
> + return IIO_VAL_INT;
> }
>
> return -EINVAL;
> @@ -1449,7 +1528,15 @@ static int stm32_dfsdm_adc_chan_init_one(struct iio_dev
> *indio_dev, struct iio_c
> * IIO_CHAN_INFO_RAW: used to compute regular conversion
> * IIO_CHAN_INFO_OVERSAMPLING_RATIO: used to set oversampling
> */
> - ch->info_mask_separate = BIT(IIO_CHAN_INFO_RAW);
> + if (child) {
> + ch->info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> + BIT(IIO_CHAN_INFO_SCALE) |
> + BIT(IIO_CHAN_INFO_OFFSET);
Not sure if the above SCALE and OFFSET are solely properties if the backend. If so,
you could maybe use iio_backend_extend_chan_spec().
- Nuno Sá
Hi Nuno,
On 6/19/24 07:47, Nuno Sá wrote:
> On Tue, 2024-06-18 at 18:08 +0200, Olivier Moysan wrote:
>> Add scaling support to STM32 DFSDM.
>>
>> Signed-off-by: Olivier Moysan <olivier.moysan@foss.st.com>
>> ---
>
> Just some general comments...
>
> Acked-by: Nuno Sa <nuno.sa@analog.com>
>
>> drivers/iio/adc/Kconfig | 1 +
>> drivers/iio/adc/stm32-dfsdm-adc.c | 94 ++++++++++++++++++++++++++++++-
>> 2 files changed, 92 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
>> index f3dfdaa80678..858ae8161fa4 100644
>>
>
> ...
>
>> @@ -1301,6 +1339,8 @@ static int stm32_dfsdm_read_raw(struct iio_dev *indio_dev,
>> ret = stm32_dfsdm_single_conv(indio_dev, chan, val);
>> if (adc->hwc)
>> iio_hw_consumer_disable(adc->hwc);
>> + if (adc->backend[idx])
>> + iio_backend_disable(&indio_dev->dev, adc->backend[idx]);
>> if (ret < 0) {
>> dev_err(&indio_dev->dev,
>> "%s: Conversion failed (channel %d)\n",
>> @@ -1320,6 +1360,45 @@ static int stm32_dfsdm_read_raw(struct iio_dev *indio_dev,
>> *val = adc->sample_freq;
>>
>> return IIO_VAL_INT;
>> +
>> + case IIO_CHAN_INFO_SCALE:
>> + /*
>> + * Scale is expressed in mV.
>> + * When fast mode is disabled, actual resolution may be lower
>> + * than 2^n, where n=realbits-1.
>> + * This leads to underestimating input voltage. To
>> + * compensate this deviation, the voltage reference can be
>> + * corrected with a factor = realbits resolution / actual max
>> + */
>> + if (adc->backend[idx]) {
>> + iio_backend_read_raw(adc->backend[idx], val, val2, mask);
>> +
>
> This is something that I've been thinking about since the beginning of backends
> support. Basically having the "catch all" read_raw()/write_raw() or more dedicated
> interfaces. For example, in your usecase I think it would make more sense to have
> dedicated API's like iio_backend_read_scale() and iio_backend_read_offset() as you're
> extending that functionality. Calling iio_backend_read_raw() seems a bit weird to me.
>
> OTOH, iio_backend_read_raw() could be useful in cases where frontends call
> iio_backend_extend_chan_spec() and may get some functionality they are not aware of.
> And so, in the default: statement this "catch all" API could be used.
>
> Anyways, this are really minor (likely pedantic) details and you kind of came first
> and made the decision for me. As I don't really have strong feelings about it, I'm
> fine with it as-is.
>
My first idea was to stick to IIO read_raw API ('struct iio_chan_spec
const *chan' should at least be added to prototype as you pointed it
out). But I agree that iio_backend_read_raw() is not explicit regarding
the requested info.
I consider using the existing API iio_backend_ext_info_get() for v2, as
you suggested.
BRs
Olivier
>> + *val = div_u64((u64)*val * (u64)BIT(DFSDM_DATA_RES - 1),
>> max);
>> + *val2 = chan->scan_type.realbits;
>> + if (chan->differential)
>> + *val *= 2;
>> + }
>> + return IIO_VAL_FRACTIONAL_LOG2;
>> +
>> + case IIO_CHAN_INFO_OFFSET:
>> + /*
>> + * DFSDM output data are in the range [-2^n,2^n],
>> + * with n=realbits-1.
>> + * - Differential modulator:
>> + * Offset correspond to SD modulator offset.
>> + * - Single ended modulator:
>> + * Input is in [0V,Vref] range, where 0V corresponds to -2^n, and
>> Vref to 2^n.
>> + * Add 2^n to offset. (i.e. middle of input range)
>> + * offset = offset(sd) * vref / res(sd) * max / vref.
>> + */
>> + if (adc->backend[idx]) {
>> + iio_backend_read_raw(adc->backend[idx], val, val2, mask);
>> +
>> + *val = div_u64((u64)max * *val, BIT(*val2 - 1));
>> + if (!chan->differential)
>> + *val += max;
>> + }
>> + return IIO_VAL_INT;
>> }
>>
>> return -EINVAL;
>> @@ -1449,7 +1528,15 @@ static int stm32_dfsdm_adc_chan_init_one(struct iio_dev
>> *indio_dev, struct iio_c
>> * IIO_CHAN_INFO_RAW: used to compute regular conversion
>> * IIO_CHAN_INFO_OVERSAMPLING_RATIO: used to set oversampling
>> */
>> - ch->info_mask_separate = BIT(IIO_CHAN_INFO_RAW);
>> + if (child) {
>> + ch->info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
>> + BIT(IIO_CHAN_INFO_SCALE) |
>> + BIT(IIO_CHAN_INFO_OFFSET);
>
> Not sure if the above SCALE and OFFSET are solely properties if the backend. If so,
> you could maybe use iio_backend_extend_chan_spec().
>
> - Nuno Sá
>
© 2016 - 2025 Red Hat, Inc.