Re: [PATCH] iio: adc: stm32-adc: fix erroneous handling of spurious IRQs
From: Ahmad Fatoum <a.fatoum@pengutronix.de>
Date: 2021-01-18 14:52:00
Also in:
linux-iio, lkml
Hello Jonathan, On 16.01.21 18:53, Jonathan Cameron wrote:
On Tue, 12 Jan 2021 16:24:42 +0100 Ahmad Fatoum [off-list ref] wrote:quoted
1c6c69525b40 ("genirq: Reject bogus threaded irq requests") makes sure that threaded IRQs either - have IRQF_ONESHOT set - don't have the default just return IRQ_WAKE_THREAD primary handler This is necessary because level-triggered interrupts need to be masked, either at device or irqchip, to avoid an interrupt storm. For spurious interrupts, the STM32 ADC driver still does this bogus request though: - It doesn't set IRQF_ONESHOT - Its primary handler just returns IRQ_WAKE_THREAD if the interrupt is unexpected, i.e. !(status & enabled_mask)This seems 'unusual'. If this is a spurious interrupt we should be returning IRQ_NONE and letting the spurious interrupt protection stuff kick in. The only reason I can see that it does this is print an error message. I'm not sure why we need to go into the thread to do that given it's not supposed to happen. If we need that message at all, I'd suggest doing it in the interrupt handler then return IRQ_NONE;
As described, I run into the spurious IRQ case, so I think the message is still useful (until that's properly fixed), but yes, it should've returned IRQ_NONE in that case. With these changes, IRQF_ONESHOT shouldn't be necessary, but in practice the driver doesn't function correctly with the primary IRQ handler threaded. Olivier, Fabrice: Are you aware of this problem? Cheers, Ahmad
quoted
- stm32mp151.dtsi describes the ADC interrupt as level-triggered Fix this by setting IRQF_ONESHOT to have the irqchip mask the IRQ until the IRQ thread has finished. IRQF_ONESHOT also has the effect that the primary handler is no longer forced into a thread. This makes the issue with spurious interrupts interrupts disappear when reading the ADC on a threadirqs=1 kernel. This used to result in following kernel error message: iio iio:device1: Unexpected IRQ: IER=0x00000000, ISR=0x0000100e or iio iio:device1: Unexpected IRQ: IER=0x00000004, ISR=0x0000100a But with this patch applied (or threaded IRQs disabled), this no longer occurs. Cc: Lucas Stach <l.stach@pengutronix.de> Reported-by: Holger Assmann <redacted> Fixes: 695e2f5c289b ("iio: adc: stm32-adc: fix a regression when using dma and irq") Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de> --- drivers/iio/adc/stm32-adc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c index c067c994dae2..7e0e21c79ac8 100644 --- a/drivers/iio/adc/stm32-adc.c +++ b/drivers/iio/adc/stm32-adc.c@@ -1910,7 +1910,7 @@ static int stm32_adc_probe(struct platform_device *pdev) ret = devm_request_threaded_irq(&pdev->dev, adc->irq, stm32_adc_isr, stm32_adc_threaded_isr, - 0, pdev->name, indio_dev); + IRQF_ONESHOT, pdev->name, indio_dev); if (ret) { dev_err(&pdev->dev, "failed to request IRQ\n"); return ret;
-- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel