2018-02-23 12:54:12

by Fabrice Gasnier

[permalink] [raw]
Subject: [PATCH 0/7] iio: adc: stm32-dfsdm: misc fixes and improvements

This series brings some fixes and improvements to STM32 DFSDM driver.

Fabrice Gasnier (7):
iio: adc: stm32-dfsdm: fix compatible data use
iio: adc: stm32-dfsdm: fix call to stop channel
iio: adc: stm32-dfsdm: fix clock source selection
iio: adc: stm32-dfsdm: fix multiple channel initialization
iio: adc: stm32-dfsdm: misc style improvements and fixes
iio: adc: stm32-dfsdm: add check on max filter id
iio: adc: stm32-dfsdm: add check on spi-max-frequency

drivers/iio/adc/stm32-dfsdm-adc.c | 52 ++++++++++++++++++++------------------
drivers/iio/adc/stm32-dfsdm-core.c | 17 +++++++++++--
2 files changed, 42 insertions(+), 27 deletions(-)

--
1.9.1



2018-02-23 12:53:03

by Fabrice Gasnier

[permalink] [raw]
Subject: [PATCH 7/7] iio: adc: stm32-dfsdm: add check on spi-max-frequency

spi-max-frequency is requested for SPI master mode (only), to tune output
clock. It may happen requested frequency isn't reachable.
Add explicit check, so probe fails with error in this case. Otherwise,
output clock may simply be silently turned off (conversions fail).

Signed-off-by: Fabrice Gasnier <[email protected]>
---
drivers/iio/adc/stm32-dfsdm-core.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/drivers/iio/adc/stm32-dfsdm-core.c b/drivers/iio/adc/stm32-dfsdm-core.c
index e50efdc..1d0d823 100644
--- a/drivers/iio/adc/stm32-dfsdm-core.c
+++ b/drivers/iio/adc/stm32-dfsdm-core.c
@@ -227,6 +227,11 @@ static int stm32_dfsdm_parse_of(struct platform_device *pdev,
}

priv->spi_clk_out_div = div_u64_rem(clk_freq, spi_freq, &rem) - 1;
+ if (!priv->spi_clk_out_div) {
+ /* spi_clk_out_div == 0 means ckout is OFF */
+ dev_err(&pdev->dev, "spi-max-frequency not achievable\n");
+ return -EINVAL;
+ }
priv->dfsdm.spi_master_freq = spi_freq;

if (rem) {
--
1.9.1


2018-02-23 12:53:19

by Fabrice Gasnier

[permalink] [raw]
Subject: [PATCH 6/7] iio: adc: stm32-dfsdm: add check on max filter id

reg property should be checked against number of available filters.
BTW, dfsdm->num_fls wasn't used. But it can be used for this purpose.
This prevents using data out of allocated dfsdm->fl_list array.

Signed-off-by: Fabrice Gasnier <[email protected]>
---
drivers/iio/adc/stm32-dfsdm-adc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/adc/stm32-dfsdm-adc.c b/drivers/iio/adc/stm32-dfsdm-adc.c
index 57bcb45..1b78bec 100644
--- a/drivers/iio/adc/stm32-dfsdm-adc.c
+++ b/drivers/iio/adc/stm32-dfsdm-adc.c
@@ -1111,8 +1111,8 @@ static int stm32_dfsdm_adc_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, adc);

ret = of_property_read_u32(dev->of_node, "reg", &adc->fl_id);
- if (ret != 0) {
- dev_err(dev, "Missing reg property\n");
+ if (ret != 0 || adc->fl_id >= adc->dfsdm->num_fls) {
+ dev_err(dev, "Missing or bad reg property\n");
return -EINVAL;
}

--
1.9.1


2018-02-23 12:53:39

by Fabrice Gasnier

[permalink] [raw]
Subject: [PATCH 2/7] iio: adc: stm32-dfsdm: fix call to stop channel

stm32_dfsdm_stop_channel must be called with channel id, not filter id.

Fixes: e2e6771c6462 ("IIO: ADC: add STM32 DFSDM sigma delta ADC support")

Signed-off-by: Fabrice Gasnier <[email protected]>
---
drivers/iio/adc/stm32-dfsdm-adc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/adc/stm32-dfsdm-adc.c b/drivers/iio/adc/stm32-dfsdm-adc.c
index daa026d..0eff811 100644
--- a/drivers/iio/adc/stm32-dfsdm-adc.c
+++ b/drivers/iio/adc/stm32-dfsdm-adc.c
@@ -464,7 +464,7 @@ static int stm32_dfsdm_start_conv(struct stm32_dfsdm_adc *adc, bool dma)

regmap_update_bits(regmap, DFSDM_CR1(adc->fl_id),
DFSDM_CR1_RCONT_MASK, 0);
- stm32_dfsdm_stop_channel(adc->dfsdm, adc->fl_id);
+ stm32_dfsdm_stop_channel(adc->dfsdm, adc->ch_id);

return ret;
}
--
1.9.1


2018-02-23 12:54:35

by Fabrice Gasnier

[permalink] [raw]
Subject: [PATCH 1/7] iio: adc: stm32-dfsdm: fix compatible data use

Fix use of compatible data: stm32h7 regmap configuration is statically
used. Rather use regmap_cfg from compatible data.

Fixes: bed73904e76f ("IIO: ADC: add stm32 DFSDM core support")

Signed-off-by: Fabrice Gasnier <[email protected]>
---
drivers/iio/adc/stm32-dfsdm-core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/adc/stm32-dfsdm-core.c b/drivers/iio/adc/stm32-dfsdm-core.c
index 6290332..0635f93 100644
--- a/drivers/iio/adc/stm32-dfsdm-core.c
+++ b/drivers/iio/adc/stm32-dfsdm-core.c
@@ -274,7 +274,7 @@ static int stm32_dfsdm_probe(struct platform_device *pdev)

dfsdm->regmap = devm_regmap_init_mmio_clk(&pdev->dev, "dfsdm",
dfsdm->base,
- &stm32h7_dfsdm_regmap_cfg);
+ dev_data->regmap_cfg);
if (IS_ERR(dfsdm->regmap)) {
ret = PTR_ERR(dfsdm->regmap);
dev_err(&pdev->dev, "%s: Failed to allocate regmap: %d\n",
--
1.9.1


2018-02-23 12:54:38

by Fabrice Gasnier

[permalink] [raw]
Subject: [PATCH 4/7] iio: adc: stm32-dfsdm: fix multiple channel initialization

When several channels are registered (e.g. via st,adc-channels property):
- channels array is wrongly filled in. Only 1st element in array is being
initialized with last registered channel.
Fix it by passing reference to relevant channel (e.g. array[index]).
- only last initialized channel can work properly (e.g. unique 'ch_id'
is used). Converting any other channel result in conversion timeout.
Fix it by getting rid of 'ch_id', use chan->channel instead.

Signed-off-by: Fabrice Gasnier <[email protected]>
---
drivers/iio/adc/stm32-dfsdm-adc.c | 39 +++++++++++++++++++++------------------
1 file changed, 21 insertions(+), 18 deletions(-)

diff --git a/drivers/iio/adc/stm32-dfsdm-adc.c b/drivers/iio/adc/stm32-dfsdm-adc.c
index 0eff811..01422d1 100644
--- a/drivers/iio/adc/stm32-dfsdm-adc.c
+++ b/drivers/iio/adc/stm32-dfsdm-adc.c
@@ -54,7 +54,6 @@ struct stm32_dfsdm_adc {
struct stm32_dfsdm *dfsdm;
const struct stm32_dfsdm_dev_data *dev_data;
unsigned int fl_id;
- unsigned int ch_id;

/* ADC specific */
unsigned int oversamp;
@@ -384,7 +383,7 @@ static ssize_t dfsdm_adc_audio_set_spiclk(struct iio_dev *indio_dev,
{
struct stm32_dfsdm_adc *adc = iio_priv(indio_dev);
struct stm32_dfsdm_filter *fl = &adc->dfsdm->fl_list[adc->fl_id];
- struct stm32_dfsdm_channel *ch = &adc->dfsdm->ch_list[adc->ch_id];
+ struct stm32_dfsdm_channel *ch = &adc->dfsdm->ch_list[chan->channel];
unsigned int sample_freq = adc->sample_freq;
unsigned int spi_freq;
int ret;
@@ -419,18 +418,20 @@ static ssize_t dfsdm_adc_audio_set_spiclk(struct iio_dev *indio_dev,
return len;
}

-static int stm32_dfsdm_start_conv(struct stm32_dfsdm_adc *adc, bool dma)
+static int stm32_dfsdm_start_conv(struct stm32_dfsdm_adc *adc,
+ const struct iio_chan_spec *chan,
+ bool dma)
{
struct regmap *regmap = adc->dfsdm->regmap;
int ret;
unsigned int dma_en = 0, cont_en = 0;

- ret = stm32_dfsdm_start_channel(adc->dfsdm, adc->ch_id);
+ ret = stm32_dfsdm_start_channel(adc->dfsdm, chan->channel);
if (ret < 0)
return ret;

ret = stm32_dfsdm_filter_configure(adc->dfsdm, adc->fl_id,
- adc->ch_id);
+ chan->channel);
if (ret < 0)
goto stop_channels;

@@ -464,12 +465,13 @@ static int stm32_dfsdm_start_conv(struct stm32_dfsdm_adc *adc, bool dma)

regmap_update_bits(regmap, DFSDM_CR1(adc->fl_id),
DFSDM_CR1_RCONT_MASK, 0);
- stm32_dfsdm_stop_channel(adc->dfsdm, adc->ch_id);
+ stm32_dfsdm_stop_channel(adc->dfsdm, chan->channel);

return ret;
}

-static void stm32_dfsdm_stop_conv(struct stm32_dfsdm_adc *adc)
+static void stm32_dfsdm_stop_conv(struct stm32_dfsdm_adc *adc,
+ const struct iio_chan_spec *chan)
{
struct regmap *regmap = adc->dfsdm->regmap;

@@ -482,7 +484,7 @@ static void stm32_dfsdm_stop_conv(struct stm32_dfsdm_adc *adc)
regmap_update_bits(regmap, DFSDM_CR1(adc->fl_id),
DFSDM_CR1_RCONT_MASK, 0);

- stm32_dfsdm_stop_channel(adc->dfsdm, adc->ch_id);
+ stm32_dfsdm_stop_channel(adc->dfsdm, chan->channel);
}

static int stm32_dfsdm_set_watermark(struct iio_dev *indio_dev,
@@ -609,6 +611,7 @@ static int stm32_dfsdm_adc_dma_start(struct iio_dev *indio_dev)
static int stm32_dfsdm_postenable(struct iio_dev *indio_dev)
{
struct stm32_dfsdm_adc *adc = iio_priv(indio_dev);
+ const struct iio_chan_spec *chan = &indio_dev->channels[0];
int ret;

/* Reset adc buffer index */
@@ -618,7 +621,7 @@ static int stm32_dfsdm_postenable(struct iio_dev *indio_dev)
if (ret < 0)
return ret;

- ret = stm32_dfsdm_start_conv(adc, true);
+ ret = stm32_dfsdm_start_conv(adc, chan, true);
if (ret) {
dev_err(&indio_dev->dev, "Can't start conversion\n");
goto stop_dfsdm;
@@ -635,7 +638,7 @@ static int stm32_dfsdm_postenable(struct iio_dev *indio_dev)
return 0;

err_stop_conv:
- stm32_dfsdm_stop_conv(adc);
+ stm32_dfsdm_stop_conv(adc, chan);
stop_dfsdm:
stm32_dfsdm_stop_dfsdm(adc->dfsdm);

@@ -645,11 +648,12 @@ 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);
+ const struct iio_chan_spec *chan = &indio_dev->channels[0];

if (adc->dma_chan)
dmaengine_terminate_all(adc->dma_chan);

- stm32_dfsdm_stop_conv(adc);
+ stm32_dfsdm_stop_conv(adc, chan);

stm32_dfsdm_stop_dfsdm(adc->dfsdm);

@@ -730,7 +734,7 @@ static int stm32_dfsdm_single_conv(struct iio_dev *indio_dev,
if (ret < 0)
goto stop_dfsdm;

- ret = stm32_dfsdm_start_conv(adc, false);
+ ret = stm32_dfsdm_start_conv(adc, chan, false);
if (ret < 0) {
regmap_update_bits(adc->dfsdm->regmap, DFSDM_CR2(adc->fl_id),
DFSDM_CR2_REOCIE_MASK, DFSDM_CR2_REOCIE(0));
@@ -751,7 +755,7 @@ static int stm32_dfsdm_single_conv(struct iio_dev *indio_dev,
else
ret = IIO_VAL_INT;

- stm32_dfsdm_stop_conv(adc);
+ stm32_dfsdm_stop_conv(adc, chan);

stop_dfsdm:
stm32_dfsdm_stop_dfsdm(adc->dfsdm);
@@ -765,7 +769,7 @@ static int stm32_dfsdm_write_raw(struct iio_dev *indio_dev,
{
struct stm32_dfsdm_adc *adc = iio_priv(indio_dev);
struct stm32_dfsdm_filter *fl = &adc->dfsdm->fl_list[adc->fl_id];
- struct stm32_dfsdm_channel *ch = &adc->dfsdm->ch_list[adc->ch_id];
+ struct stm32_dfsdm_channel *ch = &adc->dfsdm->ch_list[chan->channel];
unsigned int spi_freq = adc->spi_freq;
int ret = -EINVAL;

@@ -972,7 +976,6 @@ static int stm32_dfsdm_adc_chan_init_one(struct iio_dev *indio_dev,
}
ch->scan_type.realbits = 24;
ch->scan_type.storagebits = 32;
- adc->ch_id = ch->channel;

return stm32_dfsdm_chan_configure(adc->dfsdm,
&adc->dfsdm->ch_list[ch->channel]);
@@ -1001,7 +1004,7 @@ static int stm32_dfsdm_audio_init(struct iio_dev *indio_dev)
}
ch->info_mask_separate = BIT(IIO_CHAN_INFO_SAMP_FREQ);

- d_ch = &adc->dfsdm->ch_list[adc->ch_id];
+ d_ch = &adc->dfsdm->ch_list[ch->channel];
if (d_ch->src != DFSDM_CHANNEL_SPI_CLOCK_EXTERNAL)
adc->spi_freq = adc->dfsdm->spi_master_freq;

@@ -1042,8 +1045,8 @@ static int stm32_dfsdm_adc_init(struct iio_dev *indio_dev)
return -ENOMEM;

for (chan_idx = 0; chan_idx < num_ch; chan_idx++) {
- ch->scan_index = chan_idx;
- ret = stm32_dfsdm_adc_chan_init_one(indio_dev, ch);
+ ch[chan_idx].scan_index = chan_idx;
+ ret = stm32_dfsdm_adc_chan_init_one(indio_dev, &ch[chan_idx]);
if (ret < 0) {
dev_err(&indio_dev->dev, "Channels init failed\n");
return ret;
--
1.9.1


2018-02-23 12:54:48

by Fabrice Gasnier

[permalink] [raw]
Subject: [PATCH 5/7] iio: adc: stm32-dfsdm: misc style improvements and fixes

Misc fixes & style improvements:
- checkpatch warns about line over 80 characters.
- remove extra spaces and a blank line (e.g. checkpatch --strict)
- remove bad error message always printed in probe routine.

Signed-off-by: Fabrice Gasnier <[email protected]>
---
drivers/iio/adc/stm32-dfsdm-adc.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/adc/stm32-dfsdm-adc.c b/drivers/iio/adc/stm32-dfsdm-adc.c
index 01422d1..57bcb45 100644
--- a/drivers/iio/adc/stm32-dfsdm-adc.c
+++ b/drivers/iio/adc/stm32-dfsdm-adc.c
@@ -253,7 +253,8 @@ static int stm32_dfsdm_start_filter(struct stm32_dfsdm *dfsdm,
DFSDM_CR1_RSWSTART(1));
}

-static void stm32_dfsdm_stop_filter(struct stm32_dfsdm *dfsdm, unsigned int fl_id)
+static void stm32_dfsdm_stop_filter(struct stm32_dfsdm *dfsdm,
+ unsigned int fl_id)
{
/* Disable conversion */
regmap_update_bits(dfsdm->regmap, DFSDM_CR1(fl_id),
@@ -337,7 +338,7 @@ static int stm32_dfsdm_channel_parse_of(struct stm32_dfsdm *dfsdm,
"st,adc-channel-types", chan_idx,
&of_str);
if (!ret) {
- val = stm32_dfsdm_str2val(of_str, stm32_dfsdm_chan_type);
+ val = stm32_dfsdm_str2val(of_str, stm32_dfsdm_chan_type);
if (val < 0)
return val;
} else {
@@ -349,7 +350,7 @@ static int stm32_dfsdm_channel_parse_of(struct stm32_dfsdm *dfsdm,
"st,adc-channel-clk-src", chan_idx,
&of_str);
if (!ret) {
- val = stm32_dfsdm_str2val(of_str, stm32_dfsdm_chan_src);
+ val = stm32_dfsdm_str2val(of_str, stm32_dfsdm_chan_src);
if (val < 0)
return val;
} else {
@@ -1093,7 +1094,6 @@ static int stm32_dfsdm_adc_probe(struct platform_device *pdev)
char *name;
int ret, irq, val;

-
dev_data = of_device_get_match_data(dev);
iio = devm_iio_device_alloc(dev, sizeof(*adc));
if (!iio) {
@@ -1161,7 +1161,6 @@ static int stm32_dfsdm_adc_probe(struct platform_device *pdev)
if (ret < 0)
goto err_cleanup;

- dev_err(dev, "of_platform_populate\n");
if (dev_data->type == DFSDM_AUDIO) {
ret = of_platform_populate(np, NULL, NULL, dev);
if (ret < 0) {
--
1.9.1


2018-02-23 12:55:07

by Fabrice Gasnier

[permalink] [raw]
Subject: [PATCH 3/7] iio: adc: stm32-dfsdm: fix clock source selection

Add missing clock source selection. In case "audio" clock is provided,
it's unused currently: "dfsdm" clock is wrongly used by default.

Fixes: bed73904e76f ("IIO: ADC: add stm32 DFSDM core support")

Signed-off-by: Fabrice Gasnier <[email protected]>
---
drivers/iio/adc/stm32-dfsdm-core.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/adc/stm32-dfsdm-core.c b/drivers/iio/adc/stm32-dfsdm-core.c
index 0635f93..e50efdc 100644
--- a/drivers/iio/adc/stm32-dfsdm-core.c
+++ b/drivers/iio/adc/stm32-dfsdm-core.c
@@ -83,7 +83,7 @@ int stm32_dfsdm_start_dfsdm(struct stm32_dfsdm *dfsdm)
{
struct dfsdm_priv *priv = container_of(dfsdm, struct dfsdm_priv, dfsdm);
struct device *dev = &priv->pdev->dev;
- unsigned int clk_div = priv->spi_clk_out_div;
+ unsigned int clk_div = priv->spi_clk_out_div, clk_src;
int ret;

if (atomic_inc_return(&priv->n_active_ch) == 1) {
@@ -100,6 +100,14 @@ int stm32_dfsdm_start_dfsdm(struct stm32_dfsdm *dfsdm)
}
}

+ /* select clock source, e.g. 0 for "dfsdm" or 1 for "audio" */
+ clk_src = priv->aclk ? 1 : 0;
+ ret = regmap_update_bits(dfsdm->regmap, DFSDM_CHCFGR1(0),
+ DFSDM_CHCFGR1_CKOUTSRC_MASK,
+ DFSDM_CHCFGR1_CKOUTSRC(clk_src));
+ if (ret < 0)
+ goto disable_aclk;
+
/* Output the SPI CLKOUT (if clk_div == 0 clock if OFF) */
ret = regmap_update_bits(dfsdm->regmap, DFSDM_CHCFGR1(0),
DFSDM_CHCFGR1_CKOUTDIV_MASK,
--
1.9.1


2018-02-23 13:51:22

by Arnaud POULIQUEN

[permalink] [raw]
Subject: Re: [PATCH 0/7] iio: adc: stm32-dfsdm: misc fixes and improvements

Hello,

For the series:
Acked-by: Arnaud Pouliquen <[email protected]>

Thanks,
Arnaud

On 02/23/2018 01:50 PM, Fabrice Gasnier wrote:
> This series brings some fixes and improvements to STM32 DFSDM driver.
>
> Fabrice Gasnier (7):
> iio: adc: stm32-dfsdm: fix compatible data use
> iio: adc: stm32-dfsdm: fix call to stop channel
> iio: adc: stm32-dfsdm: fix clock source selection
> iio: adc: stm32-dfsdm: fix multiple channel initialization
> iio: adc: stm32-dfsdm: misc style improvements and fixes
> iio: adc: stm32-dfsdm: add check on max filter id
> iio: adc: stm32-dfsdm: add check on spi-max-frequency
>
> drivers/iio/adc/stm32-dfsdm-adc.c | 52 ++++++++++++++++++++------------------
> drivers/iio/adc/stm32-dfsdm-core.c | 17 +++++++++++--
> 2 files changed, 42 insertions(+), 27 deletions(-)
>

2018-02-24 12:57:34

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH 1/7] iio: adc: stm32-dfsdm: fix compatible data use

On Fri, 23 Feb 2018 13:50:55 +0100
Fabrice Gasnier <[email protected]> wrote:

> Fix use of compatible data: stm32h7 regmap configuration is statically
> used. Rather use regmap_cfg from compatible data.
>
> Fixes: bed73904e76f ("IIO: ADC: add stm32 DFSDM core support")
>
> Signed-off-by: Fabrice Gasnier <[email protected]>
Applied to the fixes-togreg branch of iio.git

Thanks,

Jonathan

> ---
> drivers/iio/adc/stm32-dfsdm-core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/iio/adc/stm32-dfsdm-core.c b/drivers/iio/adc/stm32-dfsdm-core.c
> index 6290332..0635f93 100644
> --- a/drivers/iio/adc/stm32-dfsdm-core.c
> +++ b/drivers/iio/adc/stm32-dfsdm-core.c
> @@ -274,7 +274,7 @@ static int stm32_dfsdm_probe(struct platform_device *pdev)
>
> dfsdm->regmap = devm_regmap_init_mmio_clk(&pdev->dev, "dfsdm",
> dfsdm->base,
> - &stm32h7_dfsdm_regmap_cfg);
> + dev_data->regmap_cfg);
> if (IS_ERR(dfsdm->regmap)) {
> ret = PTR_ERR(dfsdm->regmap);
> dev_err(&pdev->dev, "%s: Failed to allocate regmap: %d\n",


2018-02-24 12:58:59

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH 2/7] iio: adc: stm32-dfsdm: fix call to stop channel

On Fri, 23 Feb 2018 13:50:56 +0100
Fabrice Gasnier <[email protected]> wrote:

> stm32_dfsdm_stop_channel must be called with channel id, not filter id.
>
> Fixes: e2e6771c6462 ("IIO: ADC: add STM32 DFSDM sigma delta ADC support")
>
> Signed-off-by: Fabrice Gasnier <[email protected]>
Applied to the fixes-togreg branch of iio.git

Thanks,

Jonathan

> ---
> drivers/iio/adc/stm32-dfsdm-adc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/iio/adc/stm32-dfsdm-adc.c b/drivers/iio/adc/stm32-dfsdm-adc.c
> index daa026d..0eff811 100644
> --- a/drivers/iio/adc/stm32-dfsdm-adc.c
> +++ b/drivers/iio/adc/stm32-dfsdm-adc.c
> @@ -464,7 +464,7 @@ static int stm32_dfsdm_start_conv(struct stm32_dfsdm_adc *adc, bool dma)
>
> regmap_update_bits(regmap, DFSDM_CR1(adc->fl_id),
> DFSDM_CR1_RCONT_MASK, 0);
> - stm32_dfsdm_stop_channel(adc->dfsdm, adc->fl_id);
> + stm32_dfsdm_stop_channel(adc->dfsdm, adc->ch_id);
>
> return ret;
> }


2018-02-24 13:00:51

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH 3/7] iio: adc: stm32-dfsdm: fix clock source selection

On Fri, 23 Feb 2018 13:50:57 +0100
Fabrice Gasnier <[email protected]> wrote:

> Add missing clock source selection. In case "audio" clock is provided,
> it's unused currently: "dfsdm" clock is wrongly used by default.
>
> Fixes: bed73904e76f ("IIO: ADC: add stm32 DFSDM core support")
>
> Signed-off-by: Fabrice Gasnier <[email protected]>
Applied to the fixes-togreg branch of iio.git.

Thanks,

Jonathan

> ---
> drivers/iio/adc/stm32-dfsdm-core.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iio/adc/stm32-dfsdm-core.c b/drivers/iio/adc/stm32-dfsdm-core.c
> index 0635f93..e50efdc 100644
> --- a/drivers/iio/adc/stm32-dfsdm-core.c
> +++ b/drivers/iio/adc/stm32-dfsdm-core.c
> @@ -83,7 +83,7 @@ int stm32_dfsdm_start_dfsdm(struct stm32_dfsdm *dfsdm)
> {
> struct dfsdm_priv *priv = container_of(dfsdm, struct dfsdm_priv, dfsdm);
> struct device *dev = &priv->pdev->dev;
> - unsigned int clk_div = priv->spi_clk_out_div;
> + unsigned int clk_div = priv->spi_clk_out_div, clk_src;
> int ret;
>
> if (atomic_inc_return(&priv->n_active_ch) == 1) {
> @@ -100,6 +100,14 @@ int stm32_dfsdm_start_dfsdm(struct stm32_dfsdm *dfsdm)
> }
> }
>
> + /* select clock source, e.g. 0 for "dfsdm" or 1 for "audio" */
> + clk_src = priv->aclk ? 1 : 0;
> + ret = regmap_update_bits(dfsdm->regmap, DFSDM_CHCFGR1(0),
> + DFSDM_CHCFGR1_CKOUTSRC_MASK,
> + DFSDM_CHCFGR1_CKOUTSRC(clk_src));
> + if (ret < 0)
> + goto disable_aclk;
> +
> /* Output the SPI CLKOUT (if clk_div == 0 clock if OFF) */
> ret = regmap_update_bits(dfsdm->regmap, DFSDM_CHCFGR1(0),
> DFSDM_CHCFGR1_CKOUTDIV_MASK,


2018-02-24 13:02:35

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH 4/7] iio: adc: stm32-dfsdm: fix multiple channel initialization

On Fri, 23 Feb 2018 13:50:58 +0100
Fabrice Gasnier <[email protected]> wrote:

> When several channels are registered (e.g. via st,adc-channels property):
> - channels array is wrongly filled in. Only 1st element in array is being
> initialized with last registered channel.
> Fix it by passing reference to relevant channel (e.g. array[index]).
> - only last initialized channel can work properly (e.g. unique 'ch_id'
> is used). Converting any other channel result in conversion timeout.
> Fix it by getting rid of 'ch_id', use chan->channel instead.
>
> Signed-off-by: Fabrice Gasnier <[email protected]>
Applied to the fixes-togreg branch of iio.git.

Thanks,

Jonathan

> ---
> drivers/iio/adc/stm32-dfsdm-adc.c | 39 +++++++++++++++++++++------------------
> 1 file changed, 21 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/iio/adc/stm32-dfsdm-adc.c b/drivers/iio/adc/stm32-dfsdm-adc.c
> index 0eff811..01422d1 100644
> --- a/drivers/iio/adc/stm32-dfsdm-adc.c
> +++ b/drivers/iio/adc/stm32-dfsdm-adc.c
> @@ -54,7 +54,6 @@ struct stm32_dfsdm_adc {
> struct stm32_dfsdm *dfsdm;
> const struct stm32_dfsdm_dev_data *dev_data;
> unsigned int fl_id;
> - unsigned int ch_id;
>
> /* ADC specific */
> unsigned int oversamp;
> @@ -384,7 +383,7 @@ static ssize_t dfsdm_adc_audio_set_spiclk(struct iio_dev *indio_dev,
> {
> struct stm32_dfsdm_adc *adc = iio_priv(indio_dev);
> struct stm32_dfsdm_filter *fl = &adc->dfsdm->fl_list[adc->fl_id];
> - struct stm32_dfsdm_channel *ch = &adc->dfsdm->ch_list[adc->ch_id];
> + struct stm32_dfsdm_channel *ch = &adc->dfsdm->ch_list[chan->channel];
> unsigned int sample_freq = adc->sample_freq;
> unsigned int spi_freq;
> int ret;
> @@ -419,18 +418,20 @@ static ssize_t dfsdm_adc_audio_set_spiclk(struct iio_dev *indio_dev,
> return len;
> }
>
> -static int stm32_dfsdm_start_conv(struct stm32_dfsdm_adc *adc, bool dma)
> +static int stm32_dfsdm_start_conv(struct stm32_dfsdm_adc *adc,
> + const struct iio_chan_spec *chan,
> + bool dma)
> {
> struct regmap *regmap = adc->dfsdm->regmap;
> int ret;
> unsigned int dma_en = 0, cont_en = 0;
>
> - ret = stm32_dfsdm_start_channel(adc->dfsdm, adc->ch_id);
> + ret = stm32_dfsdm_start_channel(adc->dfsdm, chan->channel);
> if (ret < 0)
> return ret;
>
> ret = stm32_dfsdm_filter_configure(adc->dfsdm, adc->fl_id,
> - adc->ch_id);
> + chan->channel);
> if (ret < 0)
> goto stop_channels;
>
> @@ -464,12 +465,13 @@ static int stm32_dfsdm_start_conv(struct stm32_dfsdm_adc *adc, bool dma)
>
> regmap_update_bits(regmap, DFSDM_CR1(adc->fl_id),
> DFSDM_CR1_RCONT_MASK, 0);
> - stm32_dfsdm_stop_channel(adc->dfsdm, adc->ch_id);
> + stm32_dfsdm_stop_channel(adc->dfsdm, chan->channel);
>
> return ret;
> }
>
> -static void stm32_dfsdm_stop_conv(struct stm32_dfsdm_adc *adc)
> +static void stm32_dfsdm_stop_conv(struct stm32_dfsdm_adc *adc,
> + const struct iio_chan_spec *chan)
> {
> struct regmap *regmap = adc->dfsdm->regmap;
>
> @@ -482,7 +484,7 @@ static void stm32_dfsdm_stop_conv(struct stm32_dfsdm_adc *adc)
> regmap_update_bits(regmap, DFSDM_CR1(adc->fl_id),
> DFSDM_CR1_RCONT_MASK, 0);
>
> - stm32_dfsdm_stop_channel(adc->dfsdm, adc->ch_id);
> + stm32_dfsdm_stop_channel(adc->dfsdm, chan->channel);
> }
>
> static int stm32_dfsdm_set_watermark(struct iio_dev *indio_dev,
> @@ -609,6 +611,7 @@ static int stm32_dfsdm_adc_dma_start(struct iio_dev *indio_dev)
> static int stm32_dfsdm_postenable(struct iio_dev *indio_dev)
> {
> struct stm32_dfsdm_adc *adc = iio_priv(indio_dev);
> + const struct iio_chan_spec *chan = &indio_dev->channels[0];
> int ret;
>
> /* Reset adc buffer index */
> @@ -618,7 +621,7 @@ static int stm32_dfsdm_postenable(struct iio_dev *indio_dev)
> if (ret < 0)
> return ret;
>
> - ret = stm32_dfsdm_start_conv(adc, true);
> + ret = stm32_dfsdm_start_conv(adc, chan, true);
> if (ret) {
> dev_err(&indio_dev->dev, "Can't start conversion\n");
> goto stop_dfsdm;
> @@ -635,7 +638,7 @@ static int stm32_dfsdm_postenable(struct iio_dev *indio_dev)
> return 0;
>
> err_stop_conv:
> - stm32_dfsdm_stop_conv(adc);
> + stm32_dfsdm_stop_conv(adc, chan);
> stop_dfsdm:
> stm32_dfsdm_stop_dfsdm(adc->dfsdm);
>
> @@ -645,11 +648,12 @@ 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);
> + const struct iio_chan_spec *chan = &indio_dev->channels[0];
>
> if (adc->dma_chan)
> dmaengine_terminate_all(adc->dma_chan);
>
> - stm32_dfsdm_stop_conv(adc);
> + stm32_dfsdm_stop_conv(adc, chan);
>
> stm32_dfsdm_stop_dfsdm(adc->dfsdm);
>
> @@ -730,7 +734,7 @@ static int stm32_dfsdm_single_conv(struct iio_dev *indio_dev,
> if (ret < 0)
> goto stop_dfsdm;
>
> - ret = stm32_dfsdm_start_conv(adc, false);
> + ret = stm32_dfsdm_start_conv(adc, chan, false);
> if (ret < 0) {
> regmap_update_bits(adc->dfsdm->regmap, DFSDM_CR2(adc->fl_id),
> DFSDM_CR2_REOCIE_MASK, DFSDM_CR2_REOCIE(0));
> @@ -751,7 +755,7 @@ static int stm32_dfsdm_single_conv(struct iio_dev *indio_dev,
> else
> ret = IIO_VAL_INT;
>
> - stm32_dfsdm_stop_conv(adc);
> + stm32_dfsdm_stop_conv(adc, chan);
>
> stop_dfsdm:
> stm32_dfsdm_stop_dfsdm(adc->dfsdm);
> @@ -765,7 +769,7 @@ static int stm32_dfsdm_write_raw(struct iio_dev *indio_dev,
> {
> struct stm32_dfsdm_adc *adc = iio_priv(indio_dev);
> struct stm32_dfsdm_filter *fl = &adc->dfsdm->fl_list[adc->fl_id];
> - struct stm32_dfsdm_channel *ch = &adc->dfsdm->ch_list[adc->ch_id];
> + struct stm32_dfsdm_channel *ch = &adc->dfsdm->ch_list[chan->channel];
> unsigned int spi_freq = adc->spi_freq;
> int ret = -EINVAL;
>
> @@ -972,7 +976,6 @@ static int stm32_dfsdm_adc_chan_init_one(struct iio_dev *indio_dev,
> }
> ch->scan_type.realbits = 24;
> ch->scan_type.storagebits = 32;
> - adc->ch_id = ch->channel;
>
> return stm32_dfsdm_chan_configure(adc->dfsdm,
> &adc->dfsdm->ch_list[ch->channel]);
> @@ -1001,7 +1004,7 @@ static int stm32_dfsdm_audio_init(struct iio_dev *indio_dev)
> }
> ch->info_mask_separate = BIT(IIO_CHAN_INFO_SAMP_FREQ);
>
> - d_ch = &adc->dfsdm->ch_list[adc->ch_id];
> + d_ch = &adc->dfsdm->ch_list[ch->channel];
> if (d_ch->src != DFSDM_CHANNEL_SPI_CLOCK_EXTERNAL)
> adc->spi_freq = adc->dfsdm->spi_master_freq;
>
> @@ -1042,8 +1045,8 @@ static int stm32_dfsdm_adc_init(struct iio_dev *indio_dev)
> return -ENOMEM;
>
> for (chan_idx = 0; chan_idx < num_ch; chan_idx++) {
> - ch->scan_index = chan_idx;
> - ret = stm32_dfsdm_adc_chan_init_one(indio_dev, ch);
> + ch[chan_idx].scan_index = chan_idx;
> + ret = stm32_dfsdm_adc_chan_init_one(indio_dev, &ch[chan_idx]);
> if (ret < 0) {
> dev_err(&indio_dev->dev, "Channels init failed\n");
> return ret;


2018-02-24 13:04:13

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH 5/7] iio: adc: stm32-dfsdm: misc style improvements and fixes

On Fri, 23 Feb 2018 13:50:59 +0100
Fabrice Gasnier <[email protected]> wrote:

> Misc fixes & style improvements:
> - checkpatch warns about line over 80 characters.
> - remove extra spaces and a blank line (e.g. checkpatch --strict)
> - remove bad error message always printed in probe routine.
>
> Signed-off-by: Fabrice Gasnier <[email protected]>
I'd have preferred this as 3 patches doing one type fo fix each.

Anyhow, I'll need to hold this until the fixes have made there
way upstream. Give me a poke if I seem to have forgotten
and the dependencies have made it back to my togreg branch.

Thanks,

Jonathan

> ---
> drivers/iio/adc/stm32-dfsdm-adc.c | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/iio/adc/stm32-dfsdm-adc.c b/drivers/iio/adc/stm32-dfsdm-adc.c
> index 01422d1..57bcb45 100644
> --- a/drivers/iio/adc/stm32-dfsdm-adc.c
> +++ b/drivers/iio/adc/stm32-dfsdm-adc.c
> @@ -253,7 +253,8 @@ static int stm32_dfsdm_start_filter(struct stm32_dfsdm *dfsdm,
> DFSDM_CR1_RSWSTART(1));
> }
>
> -static void stm32_dfsdm_stop_filter(struct stm32_dfsdm *dfsdm, unsigned int fl_id)
> +static void stm32_dfsdm_stop_filter(struct stm32_dfsdm *dfsdm,
> + unsigned int fl_id)
> {
> /* Disable conversion */
> regmap_update_bits(dfsdm->regmap, DFSDM_CR1(fl_id),
> @@ -337,7 +338,7 @@ static int stm32_dfsdm_channel_parse_of(struct stm32_dfsdm *dfsdm,
> "st,adc-channel-types", chan_idx,
> &of_str);
> if (!ret) {
> - val = stm32_dfsdm_str2val(of_str, stm32_dfsdm_chan_type);
> + val = stm32_dfsdm_str2val(of_str, stm32_dfsdm_chan_type);
> if (val < 0)
> return val;
> } else {
> @@ -349,7 +350,7 @@ static int stm32_dfsdm_channel_parse_of(struct stm32_dfsdm *dfsdm,
> "st,adc-channel-clk-src", chan_idx,
> &of_str);
> if (!ret) {
> - val = stm32_dfsdm_str2val(of_str, stm32_dfsdm_chan_src);
> + val = stm32_dfsdm_str2val(of_str, stm32_dfsdm_chan_src);
> if (val < 0)
> return val;
> } else {
> @@ -1093,7 +1094,6 @@ static int stm32_dfsdm_adc_probe(struct platform_device *pdev)
> char *name;
> int ret, irq, val;
>
> -
> dev_data = of_device_get_match_data(dev);
> iio = devm_iio_device_alloc(dev, sizeof(*adc));
> if (!iio) {
> @@ -1161,7 +1161,6 @@ static int stm32_dfsdm_adc_probe(struct platform_device *pdev)
> if (ret < 0)
> goto err_cleanup;
>
> - dev_err(dev, "of_platform_populate\n");
> if (dev_data->type == DFSDM_AUDIO) {
> ret = of_platform_populate(np, NULL, NULL, dev);
> if (ret < 0) {


2018-02-27 08:24:18

by Fabrice Gasnier

[permalink] [raw]
Subject: Re: [PATCH 5/7] iio: adc: stm32-dfsdm: misc style improvements and fixes

On 02/24/2018 02:03 PM, Jonathan Cameron wrote:
> On Fri, 23 Feb 2018 13:50:59 +0100
> Fabrice Gasnier <[email protected]> wrote:
>
>> Misc fixes & style improvements:
>> - checkpatch warns about line over 80 characters.
>> - remove extra spaces and a blank line (e.g. checkpatch --strict)
>> - remove bad error message always printed in probe routine.
>>
>> Signed-off-by: Fabrice Gasnier <[email protected]>
> I'd have preferred this as 3 patches doing one type fo fix each.

Hi Jonathan,

I was wondering about this as well. Do you wish I send a new series with
this patch, split into (3 or maybe 4) separate patches ?

In this case, do I need also resend two last patches of this series ?

Please let me know.
Many thanks,
Fabrice
>
> Anyhow, I'll need to hold this until the fixes have made there
> way upstream. Give me a poke if I seem to have forgotten
> and the dependencies have made it back to my togreg branch.
>
> Thanks,
>
> Jonathan
>
>> ---
>> drivers/iio/adc/stm32-dfsdm-adc.c | 9 ++++-----
>> 1 file changed, 4 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/iio/adc/stm32-dfsdm-adc.c b/drivers/iio/adc/stm32-dfsdm-adc.c
>> index 01422d1..57bcb45 100644
>> --- a/drivers/iio/adc/stm32-dfsdm-adc.c
>> +++ b/drivers/iio/adc/stm32-dfsdm-adc.c
>> @@ -253,7 +253,8 @@ static int stm32_dfsdm_start_filter(struct stm32_dfsdm *dfsdm,
>> DFSDM_CR1_RSWSTART(1));
>> }
>>
>> -static void stm32_dfsdm_stop_filter(struct stm32_dfsdm *dfsdm, unsigned int fl_id)
>> +static void stm32_dfsdm_stop_filter(struct stm32_dfsdm *dfsdm,
>> + unsigned int fl_id)
>> {
>> /* Disable conversion */
>> regmap_update_bits(dfsdm->regmap, DFSDM_CR1(fl_id),
>> @@ -337,7 +338,7 @@ static int stm32_dfsdm_channel_parse_of(struct stm32_dfsdm *dfsdm,
>> "st,adc-channel-types", chan_idx,
>> &of_str);
>> if (!ret) {
>> - val = stm32_dfsdm_str2val(of_str, stm32_dfsdm_chan_type);
>> + val = stm32_dfsdm_str2val(of_str, stm32_dfsdm_chan_type);
>> if (val < 0)
>> return val;
>> } else {
>> @@ -349,7 +350,7 @@ static int stm32_dfsdm_channel_parse_of(struct stm32_dfsdm *dfsdm,
>> "st,adc-channel-clk-src", chan_idx,
>> &of_str);
>> if (!ret) {
>> - val = stm32_dfsdm_str2val(of_str, stm32_dfsdm_chan_src);
>> + val = stm32_dfsdm_str2val(of_str, stm32_dfsdm_chan_src);
>> if (val < 0)
>> return val;
>> } else {
>> @@ -1093,7 +1094,6 @@ static int stm32_dfsdm_adc_probe(struct platform_device *pdev)
>> char *name;
>> int ret, irq, val;
>>
>> -
>> dev_data = of_device_get_match_data(dev);
>> iio = devm_iio_device_alloc(dev, sizeof(*adc));
>> if (!iio) {
>> @@ -1161,7 +1161,6 @@ static int stm32_dfsdm_adc_probe(struct platform_device *pdev)
>> if (ret < 0)
>> goto err_cleanup;
>>
>> - dev_err(dev, "of_platform_populate\n");
>> if (dev_data->type == DFSDM_AUDIO) {
>> ret = of_platform_populate(np, NULL, NULL, dev);
>> if (ret < 0) {
>

2018-03-03 15:11:06

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH 5/7] iio: adc: stm32-dfsdm: misc style improvements and fixes

On Tue, 27 Feb 2018 09:21:00 +0100
Fabrice Gasnier <[email protected]> wrote:

> On 02/24/2018 02:03 PM, Jonathan Cameron wrote:
> > On Fri, 23 Feb 2018 13:50:59 +0100
> > Fabrice Gasnier <[email protected]> wrote:
> >
> >> Misc fixes & style improvements:
> >> - checkpatch warns about line over 80 characters.
> >> - remove extra spaces and a blank line (e.g. checkpatch --strict)
> >> - remove bad error message always printed in probe routine.
> >>
> >> Signed-off-by: Fabrice Gasnier <[email protected]>
> > I'd have preferred this as 3 patches doing one type fo fix each.
>
> Hi Jonathan,
>
> I was wondering about this as well. Do you wish I send a new series with
> this patch, split into (3 or maybe 4) separate patches ?
>
> In this case, do I need also resend two last patches of this series ?
It's probably not worth respining given just how small this is so
don't worry about it.

Jonathan

>
> Please let me know.
> Many thanks,
> Fabrice
> >
> > Anyhow, I'll need to hold this until the fixes have made there
> > way upstream. Give me a poke if I seem to have forgotten
> > and the dependencies have made it back to my togreg branch.
> >
> > Thanks,
> >
> > Jonathan
> >
> >> ---
> >> drivers/iio/adc/stm32-dfsdm-adc.c | 9 ++++-----
> >> 1 file changed, 4 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/drivers/iio/adc/stm32-dfsdm-adc.c b/drivers/iio/adc/stm32-dfsdm-adc.c
> >> index 01422d1..57bcb45 100644
> >> --- a/drivers/iio/adc/stm32-dfsdm-adc.c
> >> +++ b/drivers/iio/adc/stm32-dfsdm-adc.c
> >> @@ -253,7 +253,8 @@ static int stm32_dfsdm_start_filter(struct stm32_dfsdm *dfsdm,
> >> DFSDM_CR1_RSWSTART(1));
> >> }
> >>
> >> -static void stm32_dfsdm_stop_filter(struct stm32_dfsdm *dfsdm, unsigned int fl_id)
> >> +static void stm32_dfsdm_stop_filter(struct stm32_dfsdm *dfsdm,
> >> + unsigned int fl_id)
> >> {
> >> /* Disable conversion */
> >> regmap_update_bits(dfsdm->regmap, DFSDM_CR1(fl_id),
> >> @@ -337,7 +338,7 @@ static int stm32_dfsdm_channel_parse_of(struct stm32_dfsdm *dfsdm,
> >> "st,adc-channel-types", chan_idx,
> >> &of_str);
> >> if (!ret) {
> >> - val = stm32_dfsdm_str2val(of_str, stm32_dfsdm_chan_type);
> >> + val = stm32_dfsdm_str2val(of_str, stm32_dfsdm_chan_type);
> >> if (val < 0)
> >> return val;
> >> } else {
> >> @@ -349,7 +350,7 @@ static int stm32_dfsdm_channel_parse_of(struct stm32_dfsdm *dfsdm,
> >> "st,adc-channel-clk-src", chan_idx,
> >> &of_str);
> >> if (!ret) {
> >> - val = stm32_dfsdm_str2val(of_str, stm32_dfsdm_chan_src);
> >> + val = stm32_dfsdm_str2val(of_str, stm32_dfsdm_chan_src);
> >> if (val < 0)
> >> return val;
> >> } else {
> >> @@ -1093,7 +1094,6 @@ static int stm32_dfsdm_adc_probe(struct platform_device *pdev)
> >> char *name;
> >> int ret, irq, val;
> >>
> >> -
> >> dev_data = of_device_get_match_data(dev);
> >> iio = devm_iio_device_alloc(dev, sizeof(*adc));
> >> if (!iio) {
> >> @@ -1161,7 +1161,6 @@ static int stm32_dfsdm_adc_probe(struct platform_device *pdev)
> >> if (ret < 0)
> >> goto err_cleanup;
> >>
> >> - dev_err(dev, "of_platform_populate\n");
> >> if (dev_data->type == DFSDM_AUDIO) {
> >> ret = of_platform_populate(np, NULL, NULL, dev);
> >> if (ret < 0) {
> >


2018-04-23 07:51:12

by Fabrice Gasnier

[permalink] [raw]
Subject: Re: [PATCH 5/7] iio: adc: stm32-dfsdm: misc style improvements and fixes

On 02/24/2018 02:03 PM, Jonathan Cameron wrote:
> On Fri, 23 Feb 2018 13:50:59 +0100
> Fabrice Gasnier <[email protected]> wrote:
>
>> Misc fixes & style improvements:
>> - checkpatch warns about line over 80 characters.
>> - remove extra spaces and a blank line (e.g. checkpatch --strict)
>> - remove bad error message always printed in probe routine.
>>
>> Signed-off-by: Fabrice Gasnier <[email protected]>
> I'd have preferred this as 3 patches doing one type fo fix each.
>
> Anyhow, I'll need to hold this until the fixes have made there
> way upstream. Give me a poke if I seem to have forgotten
> and the dependencies have made it back to my togreg branch.

Hi Jonathan,

There are few pending patches in this series. I'm not sure everything
have been merged yet.
Do you wish I resend those patches as separate series ?

Please let me know,
Thanks in advance,
BR,
Fabrice

>
> Thanks,
>
> Jonathan
>
>> ---
>> drivers/iio/adc/stm32-dfsdm-adc.c | 9 ++++-----
>> 1 file changed, 4 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/iio/adc/stm32-dfsdm-adc.c b/drivers/iio/adc/stm32-dfsdm-adc.c
>> index 01422d1..57bcb45 100644
>> --- a/drivers/iio/adc/stm32-dfsdm-adc.c
>> +++ b/drivers/iio/adc/stm32-dfsdm-adc.c
>> @@ -253,7 +253,8 @@ static int stm32_dfsdm_start_filter(struct stm32_dfsdm *dfsdm,
>> DFSDM_CR1_RSWSTART(1));
>> }
>>
>> -static void stm32_dfsdm_stop_filter(struct stm32_dfsdm *dfsdm, unsigned int fl_id)
>> +static void stm32_dfsdm_stop_filter(struct stm32_dfsdm *dfsdm,
>> + unsigned int fl_id)
>> {
>> /* Disable conversion */
>> regmap_update_bits(dfsdm->regmap, DFSDM_CR1(fl_id),
>> @@ -337,7 +338,7 @@ static int stm32_dfsdm_channel_parse_of(struct stm32_dfsdm *dfsdm,
>> "st,adc-channel-types", chan_idx,
>> &of_str);
>> if (!ret) {
>> - val = stm32_dfsdm_str2val(of_str, stm32_dfsdm_chan_type);
>> + val = stm32_dfsdm_str2val(of_str, stm32_dfsdm_chan_type);
>> if (val < 0)
>> return val;
>> } else {
>> @@ -349,7 +350,7 @@ static int stm32_dfsdm_channel_parse_of(struct stm32_dfsdm *dfsdm,
>> "st,adc-channel-clk-src", chan_idx,
>> &of_str);
>> if (!ret) {
>> - val = stm32_dfsdm_str2val(of_str, stm32_dfsdm_chan_src);
>> + val = stm32_dfsdm_str2val(of_str, stm32_dfsdm_chan_src);
>> if (val < 0)
>> return val;
>> } else {
>> @@ -1093,7 +1094,6 @@ static int stm32_dfsdm_adc_probe(struct platform_device *pdev)
>> char *name;
>> int ret, irq, val;
>>
>> -
>> dev_data = of_device_get_match_data(dev);
>> iio = devm_iio_device_alloc(dev, sizeof(*adc));
>> if (!iio) {
>> @@ -1161,7 +1161,6 @@ static int stm32_dfsdm_adc_probe(struct platform_device *pdev)
>> if (ret < 0)
>> goto err_cleanup;
>>
>> - dev_err(dev, "of_platform_populate\n");
>> if (dev_data->type == DFSDM_AUDIO) {
>> ret = of_platform_populate(np, NULL, NULL, dev);
>> if (ret < 0) {
>

2018-04-28 15:09:44

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH 6/7] iio: adc: stm32-dfsdm: add check on max filter id

On Fri, 23 Feb 2018 13:51:00 +0100
Fabrice Gasnier <[email protected]> wrote:

> reg property should be checked against number of available filters.
> BTW, dfsdm->num_fls wasn't used. But it can be used for this purpose.
> This prevents using data out of allocated dfsdm->fl_list array.
>
> Signed-off-by: Fabrice Gasnier <[email protected]>
Applied to the togreg branch of iio.git and pushed out as testing for
the autobuilders to play with it.

Thanks,

Jonathan

> ---
> drivers/iio/adc/stm32-dfsdm-adc.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iio/adc/stm32-dfsdm-adc.c b/drivers/iio/adc/stm32-dfsdm-adc.c
> index 57bcb45..1b78bec 100644
> --- a/drivers/iio/adc/stm32-dfsdm-adc.c
> +++ b/drivers/iio/adc/stm32-dfsdm-adc.c
> @@ -1111,8 +1111,8 @@ static int stm32_dfsdm_adc_probe(struct platform_device *pdev)
> platform_set_drvdata(pdev, adc);
>
> ret = of_property_read_u32(dev->of_node, "reg", &adc->fl_id);
> - if (ret != 0) {
> - dev_err(dev, "Missing reg property\n");
> + if (ret != 0 || adc->fl_id >= adc->dfsdm->num_fls) {
> + dev_err(dev, "Missing or bad reg property\n");
> return -EINVAL;
> }
>


2018-04-28 15:09:58

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH 7/7] iio: adc: stm32-dfsdm: add check on spi-max-frequency

On Fri, 23 Feb 2018 13:51:01 +0100
Fabrice Gasnier <[email protected]> wrote:

> spi-max-frequency is requested for SPI master mode (only), to tune output
> clock. It may happen requested frequency isn't reachable.
> Add explicit check, so probe fails with error in this case. Otherwise,
> output clock may simply be silently turned off (conversions fail).
>
> Signed-off-by: Fabrice Gasnier <[email protected]>
Applied to the togreg branch of iio.git and pushed out as testing
for the autobuilders to play with.

Thanks,

Jonathan

> ---
> drivers/iio/adc/stm32-dfsdm-core.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/iio/adc/stm32-dfsdm-core.c b/drivers/iio/adc/stm32-dfsdm-core.c
> index e50efdc..1d0d823 100644
> --- a/drivers/iio/adc/stm32-dfsdm-core.c
> +++ b/drivers/iio/adc/stm32-dfsdm-core.c
> @@ -227,6 +227,11 @@ static int stm32_dfsdm_parse_of(struct platform_device *pdev,
> }
>
> priv->spi_clk_out_div = div_u64_rem(clk_freq, spi_freq, &rem) - 1;
> + if (!priv->spi_clk_out_div) {
> + /* spi_clk_out_div == 0 means ckout is OFF */
> + dev_err(&pdev->dev, "spi-max-frequency not achievable\n");
> + return -EINVAL;
> + }
> priv->dfsdm.spi_master_freq = spi_freq;
>
> if (rem) {


2018-04-28 15:11:00

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH 5/7] iio: adc: stm32-dfsdm: misc style improvements and fixes

On Mon, 23 Apr 2018 09:48:56 +0200
Fabrice Gasnier <[email protected]> wrote:

> On 02/24/2018 02:03 PM, Jonathan Cameron wrote:
> > On Fri, 23 Feb 2018 13:50:59 +0100
> > Fabrice Gasnier <[email protected]> wrote:
> >
> >> Misc fixes & style improvements:
> >> - checkpatch warns about line over 80 characters.
> >> - remove extra spaces and a blank line (e.g. checkpatch --strict)
> >> - remove bad error message always printed in probe routine.
> >>
> >> Signed-off-by: Fabrice Gasnier <[email protected]>
> > I'd have preferred this as 3 patches doing one type fo fix each.
> >
> > Anyhow, I'll need to hold this until the fixes have made there
> > way upstream. Give me a poke if I seem to have forgotten
> > and the dependencies have made it back to my togreg branch.
>
> Hi Jonathan,
>
> There are few pending patches in this series. I'm not sure everything
> have been merged yet.
> Do you wish I resend those patches as separate series ?
>
> Please let me know,
> Thanks in advance,
> BR,
> Fabrice
Thanks for the reminder, I'll pick them up from this original series.

For this one, applied to the togreg branch of iio.git and pushed out as
testing for the autobuilders to play with it.

Thanks,

Jonathan

>
> >
> > Thanks,
> >
> > Jonathan
> >
> >> ---
> >> drivers/iio/adc/stm32-dfsdm-adc.c | 9 ++++-----
> >> 1 file changed, 4 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/drivers/iio/adc/stm32-dfsdm-adc.c b/drivers/iio/adc/stm32-dfsdm-adc.c
> >> index 01422d1..57bcb45 100644
> >> --- a/drivers/iio/adc/stm32-dfsdm-adc.c
> >> +++ b/drivers/iio/adc/stm32-dfsdm-adc.c
> >> @@ -253,7 +253,8 @@ static int stm32_dfsdm_start_filter(struct stm32_dfsdm *dfsdm,
> >> DFSDM_CR1_RSWSTART(1));
> >> }
> >>
> >> -static void stm32_dfsdm_stop_filter(struct stm32_dfsdm *dfsdm, unsigned int fl_id)
> >> +static void stm32_dfsdm_stop_filter(struct stm32_dfsdm *dfsdm,
> >> + unsigned int fl_id)
> >> {
> >> /* Disable conversion */
> >> regmap_update_bits(dfsdm->regmap, DFSDM_CR1(fl_id),
> >> @@ -337,7 +338,7 @@ static int stm32_dfsdm_channel_parse_of(struct stm32_dfsdm *dfsdm,
> >> "st,adc-channel-types", chan_idx,
> >> &of_str);
> >> if (!ret) {
> >> - val = stm32_dfsdm_str2val(of_str, stm32_dfsdm_chan_type);
> >> + val = stm32_dfsdm_str2val(of_str, stm32_dfsdm_chan_type);
> >> if (val < 0)
> >> return val;
> >> } else {
> >> @@ -349,7 +350,7 @@ static int stm32_dfsdm_channel_parse_of(struct stm32_dfsdm *dfsdm,
> >> "st,adc-channel-clk-src", chan_idx,
> >> &of_str);
> >> if (!ret) {
> >> - val = stm32_dfsdm_str2val(of_str, stm32_dfsdm_chan_src);
> >> + val = stm32_dfsdm_str2val(of_str, stm32_dfsdm_chan_src);
> >> if (val < 0)
> >> return val;
> >> } else {
> >> @@ -1093,7 +1094,6 @@ static int stm32_dfsdm_adc_probe(struct platform_device *pdev)
> >> char *name;
> >> int ret, irq, val;
> >>
> >> -
> >> dev_data = of_device_get_match_data(dev);
> >> iio = devm_iio_device_alloc(dev, sizeof(*adc));
> >> if (!iio) {
> >> @@ -1161,7 +1161,6 @@ static int stm32_dfsdm_adc_probe(struct platform_device *pdev)
> >> if (ret < 0)
> >> goto err_cleanup;
> >>
> >> - dev_err(dev, "of_platform_populate\n");
> >> if (dev_data->type == DFSDM_AUDIO) {
> >> ret = of_platform_populate(np, NULL, NULL, dev);
> >> if (ret < 0) {
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html