Messages in this thread | | | Subject | Re: [PATCH] iio: adc: stm32: fix sleep inside atomic section when using DMA | From | Mukesh Ojha <> | Date | Wed, 27 Mar 2019 18:52:53 +0530 |
| |
On 3/27/2019 6:21 PM, Mukesh Ojha wrote: > > On 3/26/2019 6:14 PM, Fabrice Gasnier wrote: >> Enabling CONFIG_DEBUG_ATOMIC_SLEEP=y triggers this BUG message: >> BUG: sleeping function called from invalid context at >> kernel/irq/chip.c... >> >> Call stack is as follows: >> - __might_sleep >> - handle_nested_irq <-- Expects threaded irq >> - iio_trigger_poll_chained >> - stm32_adc_dma_buffer_done >> - vchan_complete >> - tasklet_action_common >> - tasklet_action >> - __do_softirq <-- DMA completion raises a tasklet >> - irq_exit >> - __handle_domain_irq <-- DMA IRQ >> - gic_handle_irq >> >> IIO expects threaded interrupt context when calling: >> - iio_trigger_poll_chained() >> Or it expects interrupt context when calling: >> - iio_trigger_poll() >> >> This patch triggers an IRQ upon stm32_adc_dma_buffer_done() DMA callback >> call, so the IIO trigger poll API gets called from IRQ context. >> >> Fixes: 2763ea0585c9 ("iio: adc: stm32: add optional dma support") >> >> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com> > Reviewed-by: Mukesh Ojha <mojha@codeaurora.org> > > -Mukesh > > >> --- >> drivers/iio/adc/Kconfig | 1 + >> drivers/iio/adc/stm32-adc.c | 32 ++++++++++++++++++++++++++++++-- >> 2 files changed, 31 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig >> index 76db6e5..059407a 100644 >> --- a/drivers/iio/adc/Kconfig >> +++ b/drivers/iio/adc/Kconfig >> @@ -775,6 +775,7 @@ config STM32_ADC_CORE >> select MFD_STM32_TIMERS >> select IIO_STM32_TIMER_TRIGGER >> select IIO_TRIGGERED_BUFFER >> + select IRQ_WORK >> help >> Select this option to enable the core driver for >> STMicroelectronics >> STM32 analog-to-digital converter (ADC). >> diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c >> index 205e169..1aa3189 100644 >> --- a/drivers/iio/adc/stm32-adc.c >> +++ b/drivers/iio/adc/stm32-adc.c >> @@ -20,6 +20,7 @@ >> #include <linux/interrupt.h> >> #include <linux/io.h> >> #include <linux/iopoll.h> >> +#include <linux/irq_work.h> >> #include <linux/module.h> >> #include <linux/platform_device.h> >> #include <linux/pm_runtime.h> >> @@ -297,6 +298,7 @@ struct stm32_adc_cfg { >> * @smpr_val: sampling time settings (e.g. smpr1 / smpr2) >> * @cal: optional calibration data on some devices >> * @chan_name: channel name array >> + * @work: irq work used to call trigger poll routine >> */ >> struct stm32_adc { >> struct stm32_adc_common *common; >> @@ -320,6 +322,7 @@ struct stm32_adc { >> u32 smpr_val[2]; >> struct stm32_adc_calib cal; >> char chan_name[STM32_ADC_CH_MAX][STM32_ADC_CH_SZ]; >> + struct irq_work work; >> }; >> struct stm32_adc_diff_channel { >> @@ -1473,11 +1476,32 @@ static unsigned int >> stm32_adc_dma_residue(struct stm32_adc *adc) >> return 0; >> } >> +static void stm32_adc_dma_irq_work(struct irq_work *work) >> +{ >> + struct stm32_adc *adc = container_of(work, struct stm32_adc, work); >> + struct iio_dev *indio_dev = iio_priv_to_dev(adc); >> + >> + /* >> + * iio_trigger_poll calls generic_handle_irq(). So, it requires >> hard >> + * irq context, and cannot be called directly from dma callback, >> + * dma cb has to schedule this work instead. >> + */ >> + iio_trigger_poll(indio_dev->trig); >> +} >> + >> static void stm32_adc_dma_buffer_done(void *data) >> { >> struct iio_dev *indio_dev = data; >> + struct stm32_adc *adc = iio_priv(indio_dev); >> - iio_trigger_poll_chained(indio_dev->trig); >> + /* >> + * Invoques iio_trigger_poll() from hard irq context: We can't
s/Invoques/invoke
overlooked this last time. Take mine Review tag if you make above change. Reviewed-by: Mukesh Ojha <mojha@codeaurora.org>
-Mukesh
>> + * call iio_trigger_poll() nor iio_trigger_poll_chained() >> + * directly from DMA callback (under tasklet e.g. softirq). >> + * They require respectively HW IRQ and threaded IRQ context >> + * as it might sleep. >> + */ >> + irq_work_queue(&adc->work); >> } >> static int stm32_adc_dma_start(struct iio_dev *indio_dev) >> @@ -1585,8 +1609,10 @@ static void >> __stm32_adc_buffer_predisable(struct iio_dev *indio_dev) >> if (!adc->dma_chan) >> stm32_adc_conv_irq_disable(adc); >> - if (adc->dma_chan) >> + if (adc->dma_chan) { >> dmaengine_terminate_all(adc->dma_chan); >> + irq_work_sync(&adc->work); >> + } >> if (stm32_adc_set_trig(indio_dev, NULL)) >> dev_err(&indio_dev->dev, "Can't clear trigger\n"); >> @@ -1872,6 +1898,8 @@ static int stm32_adc_dma_request(struct iio_dev >> *indio_dev) >> if (ret) >> goto err_free; >> + init_irq_work(&adc->work, stm32_adc_dma_irq_work); >> + >> return 0; >> err_free:
| |