[PATCH 2/3] hw/misc: In STM32L4x5 EXTI, handle direct line interrupts

Inès Varhol posted 3 patches 6 months, 2 weeks ago
Maintainers: Arnaud Minier <arnaud.minier@telecom-paris.fr>, "Inès Varhol" <ines.varhol@telecom-paris.fr>, Peter Maydell <peter.maydell@linaro.org>, Alistair Francis <alistair@alistair23.me>
There is a newer version of this series
[PATCH 2/3] hw/misc: In STM32L4x5 EXTI, handle direct line interrupts
Posted by Inès Varhol 6 months, 2 weeks ago
The previous implementation for EXTI interrupts only handled
"configurable" interrupts, like those originating from STM32L4x5 SYSCFG
(the only device currently connected to the EXTI up until now).
In order to connect STM32L4x5 USART to the EXTI, this commit adds
handling for direct interrupts (interrupts without configurable edge),
as well as a comment that will be useful to connect other devices to the
EXTI.

Signed-off-by: Inès Varhol <ines.varhol@telecom-paris.fr>
---
 hw/misc/stm32l4x5_exti.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/hw/misc/stm32l4x5_exti.c b/hw/misc/stm32l4x5_exti.c
index eebefc6cd3..1817bbdad2 100644
--- a/hw/misc/stm32l4x5_exti.c
+++ b/hw/misc/stm32l4x5_exti.c
@@ -106,6 +106,27 @@ static void stm32l4x5_exti_set_irq(void *opaque, int irq, int level)
         return;
     }
 
+    /* In case of a direct line interrupt */
+    if (extract32(exti_romask[bank], irq, 1)) {
+        if (level) {
+            qemu_irq_raise(s->irq[oirq]);
+        } else {
+            qemu_irq_lower(s->irq[oirq]);
+        }
+        return;
+    }
+
+    /*
+     * In case of a configurable interrupt
+     *
+     * Note that while the real EXTI uses edge detection to tell
+     * apart a line rising (the level changes from 0 to 1) and a line
+     * staying high (the level was 1 and is set to 1), the current
+     * implementation relies on the fact that this handler will only
+     * be called when there's a level change. That means that the
+     * devices creating a configurable interrupt (like STM32L4x5 GPIO)
+     * have to set their IRQs only on a change.
+     */
     if (((1 << irq) & s->rtsr[bank]) && level) {
         /* Rising Edge */
         s->pr[bank] |= 1 << irq;
@@ -116,7 +137,7 @@ static void stm32l4x5_exti_set_irq(void *opaque, int irq, int level)
         qemu_irq_pulse(s->irq[oirq]);
     }
     /*
-     * In the following situations :
+     * In the following situations (for configurable interrupts) :
      * - falling edge but rising trigger selected
      * - rising edge but falling trigger selected
      * - no trigger selected
-- 
2.43.2
Re: [PATCH 2/3] hw/misc: In STM32L4x5 EXTI, handle direct line interrupts
Posted by Peter Maydell 6 months, 1 week ago
On Sun, 12 May 2024 at 11:20, Inès Varhol <ines.varhol@telecom-paris.fr> wrote:
>
> The previous implementation for EXTI interrupts only handled
> "configurable" interrupts, like those originating from STM32L4x5 SYSCFG
> (the only device currently connected to the EXTI up until now).
> In order to connect STM32L4x5 USART to the EXTI, this commit adds
> handling for direct interrupts (interrupts without configurable edge),
> as well as a comment that will be useful to connect other devices to the
> EXTI.
>
> Signed-off-by: Inès Varhol <ines.varhol@telecom-paris.fr>
> ---
>  hw/misc/stm32l4x5_exti.c | 23 ++++++++++++++++++++++-
>  1 file changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/hw/misc/stm32l4x5_exti.c b/hw/misc/stm32l4x5_exti.c
> index eebefc6cd3..1817bbdad2 100644
> --- a/hw/misc/stm32l4x5_exti.c
> +++ b/hw/misc/stm32l4x5_exti.c
> @@ -106,6 +106,27 @@ static void stm32l4x5_exti_set_irq(void *opaque, int irq, int level)
>          return;
>      }
>
> +    /* In case of a direct line interrupt */
> +    if (extract32(exti_romask[bank], irq, 1)) {
> +        if (level) {
> +            qemu_irq_raise(s->irq[oirq]);
> +        } else {
> +            qemu_irq_lower(s->irq[oirq]);
> +        }

You can say this more concisely as
           qemu_set_irq(s->irq[oirq], level);

> +        return;
> +    }
> +
> +    /*
> +     * In case of a configurable interrupt
> +     *
> +     * Note that while the real EXTI uses edge detection to tell
> +     * apart a line rising (the level changes from 0 to 1) and a line
> +     * staying high (the level was 1 and is set to 1), the current
> +     * implementation relies on the fact that this handler will only
> +     * be called when there's a level change. That means that the
> +     * devices creating a configurable interrupt (like STM32L4x5 GPIO)
> +     * have to set their IRQs only on a change.
> +     */

You cannot rely on this in QEMU's qemu_irq API. The set
function may be called multiple times with the same input
level value. If you need to detect rising edges then this
device needs to have a state field that records the current
value so it can compare the 'level' argument here against
what it was previously.

>      if (((1 << irq) & s->rtsr[bank]) && level) {
>          /* Rising Edge */
>          s->pr[bank] |= 1 << irq;
> @@ -116,7 +137,7 @@ static void stm32l4x5_exti_set_irq(void *opaque, int irq, int level)
>          qemu_irq_pulse(s->irq[oirq]);
>      }
>      /*
> -     * In the following situations :
> +     * In the following situations (for configurable interrupts) :
>       * - falling edge but rising trigger selected
>       * - rising edge but falling trigger selected
>       * - no trigger selected

thanks
-- PMM