Messages in this thread | | | Subject | Re: [PATCH v2 5/7] iio: adc: stm32: add optional dma support | From | Fabrice Gasnier <> | Date | Mon, 30 Jan 2017 09:57:02 +0100 |
| |
On 01/28/2017 07:41 PM, Jonathan Cameron wrote: > On 26/01/17 14:28, Fabrice Gasnier wrote: >> Add DMA optional support to STM32 ADC, as there is a limited number DMA >> channels (request lines) that can be assigned to ADC. This way, driver >> may fall back using interrupts when all DMA channels are in use for >> other IPs. >> Use dma cyclic mode with two periods. Allow to tune period length by >> using watermark. Coherent memory is used for dma (max buffer size is >> fixed to PAGE_SIZE). >> >> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com> > I am happy with this whole series, except this patch gives me: > > drivers/iio/adc/stm32-adc.c:478:23: warning: symbol 'stm32_adc_trig_pol' was not declared. Should it be static? > drivers/iio/adc/stm32-adc.c:621:21: error: incompatible types in comparison expression (different type sizes) > CC [M] drivers/iio/adc/stm32-adc.o > In file included from ./include/linux/clk.h:16:0, > from drivers/iio/adc/stm32-adc.c:22: > drivers/iio/adc/stm32-adc.c: In function ‘stm32_adc_set_watermark’: > ./include/linux/kernel.h:753:16: warning: comparison of distinct pointer types lacks a cast > (void) (&min1 == &min2); \ > ^ > ./include/linux/kernel.h:756:2: note: in expansion of macro ‘__min’ > __min(typeof(x), typeof(y), \ > ^~~~~ > drivers/iio/adc/stm32-adc.c:621:14: note: in expansion of macro ‘min’ > watermark = min(watermark, val * sizeof(u16)); > ^~~ > > The static is obviously fine so I've added that. > The second looks to be because sizeof(u16) is a size_t which is signed IIRC. > Anyhow, a cast of that to unsigned should I think be harmless and fixes the > warning. > > Please check I did these right.
Hi Jonathan,
I just checked, this is ok for me.
Many thanks! Best Regards, Fabrice > > They are in the testing branch of iio.git. > > Thanks, > > Jonathan > >> --- >> Changes in v2: >> - Use iio_trigger_poll_chained() avoids to bounce back into irq context. >> Remove irq_work. >> - Rework dma buffer allocation and use. Allocation moved to probe time, >> fixed to PAGE_SIZE. Use hwfifo_set_watermark() routine to tune dma >> cyclic period length. >> --- >> drivers/iio/adc/Kconfig | 1 + >> drivers/iio/adc/stm32-adc-core.c | 1 + >> drivers/iio/adc/stm32-adc-core.h | 2 + >> drivers/iio/adc/stm32-adc.c | 227 ++++++++++++++++++++++++++++++++++++--- >> 4 files changed, 218 insertions(+), 13 deletions(-) >> >> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig >> index 9a7b090..03a73f9 100644 >> --- a/drivers/iio/adc/Kconfig >> +++ b/drivers/iio/adc/Kconfig >> @@ -444,6 +444,7 @@ config ROCKCHIP_SARADC >> config STM32_ADC_CORE >> tristate "STMicroelectronics STM32 adc core" >> depends on ARCH_STM32 || COMPILE_TEST >> + depends on HAS_DMA >> depends on OF >> depends on REGULATOR >> select IIO_BUFFER >> diff --git a/drivers/iio/adc/stm32-adc-core.c b/drivers/iio/adc/stm32-adc-core.c >> index 4214b0c..22b7c93 100644 >> --- a/drivers/iio/adc/stm32-adc-core.c >> +++ b/drivers/iio/adc/stm32-adc-core.c >> @@ -201,6 +201,7 @@ static int stm32_adc_probe(struct platform_device *pdev) >> priv->common.base = devm_ioremap_resource(&pdev->dev, res); >> if (IS_ERR(priv->common.base)) >> return PTR_ERR(priv->common.base); >> + priv->common.phys_base = res->start; >> >> priv->vref = devm_regulator_get(&pdev->dev, "vref"); >> if (IS_ERR(priv->vref)) { >> diff --git a/drivers/iio/adc/stm32-adc-core.h b/drivers/iio/adc/stm32-adc-core.h >> index 081fa5f..2ec7abb 100644 >> --- a/drivers/iio/adc/stm32-adc-core.h >> +++ b/drivers/iio/adc/stm32-adc-core.h >> @@ -42,10 +42,12 @@ >> /** >> * struct stm32_adc_common - stm32 ADC driver common data (for all instances) >> * @base: control registers base cpu addr >> + * @phys_base: control registers base physical addr >> * @vref_mv: vref voltage (mv) >> */ >> struct stm32_adc_common { >> void __iomem *base; >> + phys_addr_t phys_base; >> int vref_mv; >> }; >> >> diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c >> index 9a38f9a..8a30039 100644 >> --- a/drivers/iio/adc/stm32-adc.c >> +++ b/drivers/iio/adc/stm32-adc.c >> @@ -21,6 +21,8 @@ >> >> #include <linux/clk.h> >> #include <linux/delay.h> >> +#include <linux/dma-mapping.h> >> +#include <linux/dmaengine.h> >> #include <linux/iio/iio.h> >> #include <linux/iio/buffer.h> >> #include <linux/iio/timer/stm32-timer-trigger.h> >> @@ -68,12 +70,16 @@ >> #define STM32F4_EXTSEL_SHIFT 24 >> #define STM32F4_EXTSEL_MASK GENMASK(27, 24) >> #define STM32F4_EOCS BIT(10) >> +#define STM32F4_DDS BIT(9) >> +#define STM32F4_DMA BIT(8) >> #define STM32F4_ADON BIT(0) >> >> #define STM32_ADC_MAX_SQ 16 /* SQ1..SQ16 */ >> #define STM32_ADC_TIMEOUT_US 100000 >> #define STM32_ADC_TIMEOUT (msecs_to_jiffies(STM32_ADC_TIMEOUT_US / 1000)) >> >> +#define STM32_DMA_BUFFER_SIZE PAGE_SIZE >> + >> /* External trigger enable */ >> enum stm32_adc_exten { >> STM32_EXTEN_SWTRIG, >> @@ -136,6 +142,10 @@ struct stm32_adc_regs { >> * @bufi: data buffer index >> * @num_conv: expected number of scan conversions >> * @trigger_polarity: external trigger polarity (e.g. exten) >> + * @dma_chan: dma channel >> + * @rx_buf: dma rx buffer cpu address >> + * @rx_dma_buf: dma rx buffer bus address >> + * @rx_buf_sz: dma rx buffer size >> */ >> struct stm32_adc { >> struct stm32_adc_common *common; >> @@ -148,6 +158,10 @@ struct stm32_adc { >> unsigned int bufi; >> unsigned int num_conv; >> u32 trigger_polarity; >> + struct dma_chan *dma_chan; >> + u8 *rx_buf; >> + dma_addr_t rx_dma_buf; >> + unsigned int rx_buf_sz; >> }; >> >> /** >> @@ -291,10 +305,21 @@ static void stm32_adc_conv_irq_disable(struct stm32_adc *adc) >> /** >> * stm32_adc_start_conv() - Start conversions for regular channels. >> * @adc: stm32 adc instance >> + * @dma: use dma to transfer conversion result >> + * >> + * Start conversions for regular channels. >> + * Also take care of normal or DMA mode. Circular DMA may be used for regular >> + * conversions, in IIO buffer modes. Otherwise, use ADC interrupt with direct >> + * DR read instead (e.g. read_raw, or triggered buffer mode without DMA). >> */ >> -static void stm32_adc_start_conv(struct stm32_adc *adc) >> +static void stm32_adc_start_conv(struct stm32_adc *adc, bool dma) >> { >> stm32_adc_set_bits(adc, STM32F4_ADC_CR1, STM32F4_SCAN); >> + >> + if (dma) >> + stm32_adc_set_bits(adc, STM32F4_ADC_CR2, >> + STM32F4_DMA | STM32F4_DDS); >> + >> stm32_adc_set_bits(adc, STM32F4_ADC_CR2, STM32F4_EOCS | STM32F4_ADON); >> >> /* Wait for Power-up time (tSTAB from datasheet) */ >> @@ -311,7 +336,8 @@ static void stm32_adc_stop_conv(struct stm32_adc *adc) >> stm32_adc_clr_bits(adc, STM32F4_ADC_SR, STM32F4_STRT); >> >> stm32_adc_clr_bits(adc, STM32F4_ADC_CR1, STM32F4_SCAN); >> - stm32_adc_clr_bits(adc, STM32F4_ADC_CR2, STM32F4_ADON); >> + stm32_adc_clr_bits(adc, STM32F4_ADC_CR2, >> + STM32F4_ADON | STM32F4_DMA | STM32F4_DDS); >> } >> >> /** >> @@ -494,7 +520,7 @@ static int stm32_adc_single_conv(struct iio_dev *indio_dev, >> >> stm32_adc_conv_irq_enable(adc); >> >> - stm32_adc_start_conv(adc); >> + stm32_adc_start_conv(adc, false); >> >> timeout = wait_for_completion_interruptible_timeout( >> &adc->completion, STM32_ADC_TIMEOUT); >> @@ -581,6 +607,23 @@ static int stm32_adc_validate_trigger(struct iio_dev *indio_dev, >> return stm32_adc_get_trig_extsel(trig) < 0 ? -EINVAL : 0; >> } >> >> +static int stm32_adc_set_watermark(struct iio_dev *indio_dev, unsigned val) >> +{ >> + struct stm32_adc *adc = iio_priv(indio_dev); >> + unsigned int watermark = STM32_DMA_BUFFER_SIZE / 2; >> + >> + /* >> + * dma cyclic transfers are used, buffer is split into two periods. >> + * There should be : >> + * - always one buffer (period) dma is working on >> + * - one buffer (period) driver can push with iio_trigger_poll(). >> + */ >> + watermark = min(watermark, val * sizeof(u16)); >> + adc->rx_buf_sz = watermark * 2; >> + >> + return 0; >> +} >> + >> static int stm32_adc_update_scan_mode(struct iio_dev *indio_dev, >> const unsigned long *scan_mask) >> { >> @@ -635,12 +678,83 @@ static int stm32_adc_debugfs_reg_access(struct iio_dev *indio_dev, >> static const struct iio_info stm32_adc_iio_info = { >> .read_raw = stm32_adc_read_raw, >> .validate_trigger = stm32_adc_validate_trigger, >> + .hwfifo_set_watermark = stm32_adc_set_watermark, >> .update_scan_mode = stm32_adc_update_scan_mode, >> .debugfs_reg_access = stm32_adc_debugfs_reg_access, >> .of_xlate = stm32_adc_of_xlate, >> .driver_module = THIS_MODULE, >> }; >> >> +static unsigned int stm32_adc_dma_residue(struct stm32_adc *adc) >> +{ >> + struct dma_tx_state state; >> + enum dma_status status; >> + >> + status = dmaengine_tx_status(adc->dma_chan, >> + adc->dma_chan->cookie, >> + &state); >> + if (status == DMA_IN_PROGRESS) { >> + /* Residue is size in bytes from end of buffer */ >> + unsigned int i = adc->rx_buf_sz - state.residue; >> + unsigned int size; >> + >> + /* Return available bytes */ >> + if (i >= adc->bufi) >> + size = i - adc->bufi; >> + else >> + size = adc->rx_buf_sz + i - adc->bufi; >> + >> + return size; >> + } >> + >> + return 0; >> +} >> + >> +static void stm32_adc_dma_buffer_done(void *data) >> +{ >> + struct iio_dev *indio_dev = data; >> + >> + iio_trigger_poll_chained(indio_dev->trig); >> +} >> + >> +static int stm32_adc_dma_start(struct iio_dev *indio_dev) >> +{ >> + struct stm32_adc *adc = iio_priv(indio_dev); >> + struct dma_async_tx_descriptor *desc; >> + dma_cookie_t cookie; >> + int ret; >> + >> + if (!adc->dma_chan) >> + return 0; >> + >> + dev_dbg(&indio_dev->dev, "%s size=%d watermark=%d\n", __func__, >> + adc->rx_buf_sz, adc->rx_buf_sz / 2); >> + >> + /* Prepare a DMA cyclic transaction */ >> + desc = dmaengine_prep_dma_cyclic(adc->dma_chan, >> + adc->rx_dma_buf, >> + adc->rx_buf_sz, adc->rx_buf_sz / 2, >> + DMA_DEV_TO_MEM, >> + DMA_PREP_INTERRUPT); >> + if (!desc) >> + return -EBUSY; >> + >> + desc->callback = stm32_adc_dma_buffer_done; >> + desc->callback_param = indio_dev; >> + >> + cookie = dmaengine_submit(desc); >> + ret = dma_submit_error(cookie); >> + if (ret) { >> + dmaengine_terminate_all(adc->dma_chan); >> + return ret; >> + } >> + >> + /* Issue pending DMA requests */ >> + dma_async_issue_pending(adc->dma_chan); >> + >> + return 0; >> +} >> + >> static int stm32_adc_buffer_postenable(struct iio_dev *indio_dev) >> { >> struct stm32_adc *adc = iio_priv(indio_dev); >> @@ -652,18 +766,29 @@ static int stm32_adc_buffer_postenable(struct iio_dev *indio_dev) >> return ret; >> } >> >> + ret = stm32_adc_dma_start(indio_dev); >> + if (ret) { >> + dev_err(&indio_dev->dev, "Can't start dma\n"); >> + goto err_clr_trig; >> + } >> + >> ret = iio_triggered_buffer_postenable(indio_dev); >> if (ret < 0) >> - goto err_clr_trig; >> + goto err_stop_dma; >> >> /* Reset adc buffer index */ >> adc->bufi = 0; >> >> - stm32_adc_conv_irq_enable(adc); >> - stm32_adc_start_conv(adc); >> + if (!adc->dma_chan) >> + stm32_adc_conv_irq_enable(adc); >> + >> + stm32_adc_start_conv(adc, !!adc->dma_chan); >> >> return 0; >> >> +err_stop_dma: >> + if (adc->dma_chan) >> + dmaengine_terminate_all(adc->dma_chan); >> err_clr_trig: >> stm32_adc_set_trig(indio_dev, NULL); >> >> @@ -676,12 +801,16 @@ static int stm32_adc_buffer_predisable(struct iio_dev *indio_dev) >> int ret; >> >> stm32_adc_stop_conv(adc); >> - stm32_adc_conv_irq_disable(adc); >> + if (!adc->dma_chan) >> + stm32_adc_conv_irq_disable(adc); >> >> ret = iio_triggered_buffer_predisable(indio_dev); >> if (ret < 0) >> dev_err(&indio_dev->dev, "predisable failed\n"); >> >> + if (adc->dma_chan) >> + dmaengine_terminate_all(adc->dma_chan); >> + >> if (stm32_adc_set_trig(indio_dev, NULL)) >> dev_err(&indio_dev->dev, "Can't clear trigger\n"); >> >> @@ -701,15 +830,31 @@ static irqreturn_t stm32_adc_trigger_handler(int irq, void *p) >> >> dev_dbg(&indio_dev->dev, "%s bufi=%d\n", __func__, adc->bufi); >> >> - /* reset buffer index */ >> - adc->bufi = 0; >> - iio_push_to_buffers_with_timestamp(indio_dev, adc->buffer, >> - pf->timestamp); >> + if (!adc->dma_chan) { >> + /* reset buffer index */ >> + adc->bufi = 0; >> + iio_push_to_buffers_with_timestamp(indio_dev, adc->buffer, >> + pf->timestamp); >> + } else { >> + int residue = stm32_adc_dma_residue(adc); >> + >> + while (residue >= indio_dev->scan_bytes) { >> + u16 *buffer = (u16 *)&adc->rx_buf[adc->bufi]; >> + >> + iio_push_to_buffers_with_timestamp(indio_dev, buffer, >> + pf->timestamp); >> + residue -= indio_dev->scan_bytes; >> + adc->bufi += indio_dev->scan_bytes; >> + if (adc->bufi >= adc->rx_buf_sz) >> + adc->bufi = 0; >> + } >> + } >> >> iio_trigger_notify_done(indio_dev->trig); >> >> /* re-enable eoc irq */ >> - stm32_adc_conv_irq_enable(adc); >> + if (!adc->dma_chan) >> + stm32_adc_conv_irq_enable(adc); >> >> return IRQ_HANDLED; >> } >> @@ -781,6 +926,45 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev) >> return 0; >> } >> >> +static int stm32_adc_dma_request(struct iio_dev *indio_dev) >> +{ >> + struct stm32_adc *adc = iio_priv(indio_dev); >> + struct dma_slave_config config; >> + int ret; >> + >> + adc->dma_chan = dma_request_slave_channel(&indio_dev->dev, "rx"); >> + if (!adc->dma_chan) >> + return 0; >> + >> + adc->rx_buf = dma_alloc_coherent(adc->dma_chan->device->dev, >> + STM32_DMA_BUFFER_SIZE, >> + &adc->rx_dma_buf, GFP_KERNEL); >> + if (!adc->rx_buf) { >> + goto err_release; >> + ret = -ENOMEM; >> + } >> + >> + /* Configure DMA channel to read data register */ >> + memset(&config, 0, sizeof(config)); >> + config.src_addr = (dma_addr_t)adc->common->phys_base; >> + config.src_addr += adc->offset + STM32F4_ADC_DR; >> + config.src_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES; >> + >> + ret = dmaengine_slave_config(adc->dma_chan, &config); >> + if (ret) >> + goto err_free; >> + >> + return 0; >> + >> +err_free: >> + dma_free_coherent(adc->dma_chan->device->dev, STM32_DMA_BUFFER_SIZE, >> + adc->rx_buf, adc->rx_dma_buf); >> +err_release: >> + dma_release_channel(adc->dma_chan); >> + >> + return ret; >> +} >> + >> static int stm32_adc_probe(struct platform_device *pdev) >> { >> struct iio_dev *indio_dev; >> @@ -842,13 +1026,17 @@ static int stm32_adc_probe(struct platform_device *pdev) >> if (ret < 0) >> goto err_clk_disable; >> >> + ret = stm32_adc_dma_request(indio_dev); >> + if (ret < 0) >> + goto err_clk_disable; >> + >> ret = iio_triggered_buffer_setup(indio_dev, >> &iio_pollfunc_store_time, >> &stm32_adc_trigger_handler, >> &stm32_adc_buffer_setup_ops); >> if (ret) { >> dev_err(&pdev->dev, "buffer setup failed\n"); >> - goto err_clk_disable; >> + goto err_dma_disable; >> } >> >> ret = iio_device_register(indio_dev); >> @@ -862,6 +1050,13 @@ static int stm32_adc_probe(struct platform_device *pdev) >> err_buffer_cleanup: >> iio_triggered_buffer_cleanup(indio_dev); >> >> +err_dma_disable: >> + if (adc->dma_chan) { >> + dma_free_coherent(adc->dma_chan->device->dev, >> + STM32_DMA_BUFFER_SIZE, >> + adc->rx_buf, adc->rx_dma_buf); >> + dma_release_channel(adc->dma_chan); >> + } >> err_clk_disable: >> clk_disable_unprepare(adc->clk); >> >> @@ -875,6 +1070,12 @@ static int stm32_adc_remove(struct platform_device *pdev) >> >> iio_device_unregister(indio_dev); >> iio_triggered_buffer_cleanup(indio_dev); >> + if (adc->dma_chan) { >> + dma_free_coherent(adc->dma_chan->device->dev, >> + STM32_DMA_BUFFER_SIZE, >> + adc->rx_buf, adc->rx_dma_buf); >> + dma_release_channel(adc->dma_chan); >> + } >> clk_disable_unprepare(adc->clk); >> >> return 0; >>
| |