<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, May 13, 2022 at 9:14 AM Fabrice Gasnier <<a href="mailto:fabrice.gasnier@foss.st.com">fabrice.gasnier@foss.st.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On 5/7/22 00:56, Yannick Brosseau wrote:<br>
> The check for spurious IRQs introduced in 695e2f5c289bb assumed that the bits<br>
> in the control and status registers are aligned. This is true for the H7 and MP1<br>
> version, but not the F4.<br>
> <br>
> Instead of comparing both registers bitwise, we check the bit in the status and control<br>
> for each interrupt we are interested in.<br>
> <br>
<br>
Hi Yannick,<br>
<br>
I propose a different approach, see here after.<br>
<br>
Same as for patch one,<br>
Fixes: 695e2f5c289b ("iio: adc: stm32-adc: fix a regression when using<br>
dma and irq")<br>
<br>
> Signed-off-by: Yannick Brosseau <<a href="mailto:yannick.brosseau@gmail.com" target="_blank">yannick.brosseau@gmail.com</a>><br>
> ---<br>
>  drivers/iio/adc/stm32-adc.c | 9 ++++++---<br>
>  1 file changed, 6 insertions(+), 3 deletions(-)<br>
> <br>
> diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c<br>
> index a68ecbda6480..5b0f138333ee 100644<br>
> --- a/drivers/iio/adc/stm32-adc.c<br>
> +++ b/drivers/iio/adc/stm32-adc.c<br>
> @@ -1422,9 +1422,10 @@ static irqreturn_t stm32_adc_threaded_isr(int irq, void *data)<br>
>               return IRQ_HANDLED;<br>
>       }<br>
>  <br>
> -     if (!(status & mask))<br>
> +     if(!((status & regs->isr_eoc.mask) && (mask & regs->ier_eoc.mask)) ||<br>
> +           ((status & regs->isr_ovr.mask) && (mask & regs->ier_ovr.mask)))<br>
>               dev_err_ratelimited(&indio_dev->dev,<br>
> -                                 "Unexpected IRQ: IER=0x%08x, ISR=0x%08x\n",<br>
> +                                 "Unexpected IRQ: CR1/IER=0x%08x, SR/ISR=0x%08x\n",<br>
>                                   mask, status);<br>
<br>
<br>
Here, a slightly different approach could be used... There's a long<br>
pending discussion, where Olivier or I should push further patches to<br>
support threadirqs (hopefully soon).<br>
In this discussion with Jonathan [1], he exposed the need to remove this<br>
message. Words from Jonathan:<br>
"This seems 'unusual'.  If this is a spurious interrupt we should be<br>
returning IRQ_NONE and letting the spurious interrupt protection<br>
stuff kick in."<br>
<br>
[1]<br>
<a href="https://lore.kernel.org/linux-arm-kernel/20210116175333.4d8684c5@archlinux/" rel="noreferrer" target="_blank">https://lore.kernel.org/linux-arm-kernel/20210116175333.4d8684c5@archlinux/</a><br>
<br>
So basically, I suggest to completely get rid of this message:<br>
<br>
-       if (!(status & mask))<br>
-               dev_err_ratelimited(&indio_dev->dev,<br>
-                                   "Unexpected IRQ: IER=0x%08x, ISR=0x%08x\n",<br>
-                                   mask, status);<br>
<br>
>  <br>
>       return IRQ_NONE;<br>
> @@ -1438,7 +1439,9 @@ static irqreturn_t stm32_adc_isr(int irq, void *data)<br>
>       u32 status = stm32_adc_readl(adc, regs->isr_eoc.reg);<br>
>       u32 mask = stm32_adc_readl(adc, regs->ier_eoc.reg);<br>
>  <br>
> -     if (!(status & mask))<br>
> +     /* Check that we have the interrupt we care about are enabled and active */<br>
> +        if(!((status & regs->isr_eoc.mask) && (mask & regs->ier_eoc.mask)) ||<br>
> +           ((status & regs->isr_ovr.mask) && (mask & regs->ier_ovr.mask)))<br>
>               return IRQ_WAKE_THREAD;<br>
<br>
Here the statement becomes useless, so it could be removed:<br>
-       u32 mask = stm32_adc_readl(adc, regs->ier_eoc.reg);<br>
-<br>
-       if (!(status & mask))<br>
-               return IRQ_WAKE_THREAD;<br>
<br>
This would avoid some complexity here (and so headaches or regressions<br>
like the one you've hit).<br>
<br>
This also should serve the two purposes:<br>
- fall into kernel generic handler for spurious IRQs (by returning<br>
IRQ_NONE below)<br>
- by the way fix current issue in stm32f4<br>
<br></blockquote><div><br></div><div>I agree with this. I was a bit unsure if this code was necessary. I ran my systems with those line removed for a few months without issues. </div><div>I'll spin up an updated patchset. </div><div><br></div><div>Yannick</div><div> </div></div></div>