[PATCH 0/3] Connect STM32L4x5 USART devices to the EXTI

[PATCH 0/3] Connect STM32L4x5 USART devices to the EXTI

[Inès Varhol]

STM32L4x5 EXTI was handling only configurable interrupts
(such as those coming from STM32L4x5 SYSCFG which was the
only device connected to the EXTI).
This patch adds support for direct line interrupts and
connects the existing STM32L4x5 USART devices to the EXTI.

Signed-off-by: Inès Varhol <ines.varhol@telecom-paris.fr>

Inès Varhol (3):
  hw/misc: In STM32L4x5 EXTI, consolidate 2 constants
  hw/misc: In STM32L4x5 EXTI, handle direct line interrupts
  hw/arm: In STM32L4x5 SOC, connect USART devices to EXTI

 include/hw/misc/stm32l4x5_exti.h |  4 ++--
 hw/arm/stm32l4x5_soc.c           | 24 +++++++++++-------------
 hw/misc/stm32l4x5_exti.c         | 29 ++++++++++++++++++++++++-----
 3 files changed, 37 insertions(+), 20 deletions(-)

-- 
2.43.2

[PATCH 1/3] hw/misc: In STM32L4x5 EXTI, consolidate 2 constants

[Inès Varhol]

Up until now, the EXTI implementation had 16 inbound GPIOs connected to
the 16 outbound GPIOs of STM32L4x5 SYSCFG.
The EXTI actually handles 40 lines (namely 5 from STM32L4x5 USART
devices which are already implemented in QEMU).
In order to connect USART devices to EXTI, this commit consolidates
constants `EXTI_NUM_INTERRUPT_OUT_LINES` (40) and
`EXTI_NUM_GPIO_EVENT_IN_LINES` (16) into `EXTI_NUM_LINES` (40).

Signed-off-by: Inès Varhol <ines.varhol@telecom-paris.fr>
---
 include/hw/misc/stm32l4x5_exti.h | 4 ++--
 hw/misc/stm32l4x5_exti.c         | 6 ++----
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/include/hw/misc/stm32l4x5_exti.h b/include/hw/misc/stm32l4x5_exti.h
index be961d2f01..82f75a2417 100644
--- a/include/hw/misc/stm32l4x5_exti.h
+++ b/include/hw/misc/stm32l4x5_exti.h
@@ -30,7 +30,7 @@
 #define TYPE_STM32L4X5_EXTI "stm32l4x5-exti"
 OBJECT_DECLARE_SIMPLE_TYPE(Stm32l4x5ExtiState, STM32L4X5_EXTI)
 
-#define EXTI_NUM_INTERRUPT_OUT_LINES 40
+#define EXTI_NUM_LINES 40
 #define EXTI_NUM_REGISTER 2
 
 struct Stm32l4x5ExtiState {
@@ -45,7 +45,7 @@ struct Stm32l4x5ExtiState {
     uint32_t swier[EXTI_NUM_REGISTER];
     uint32_t pr[EXTI_NUM_REGISTER];
 
-    qemu_irq irq[EXTI_NUM_INTERRUPT_OUT_LINES];
+    qemu_irq irq[EXTI_NUM_LINES];
 };
 
 #endif
diff --git a/hw/misc/stm32l4x5_exti.c b/hw/misc/stm32l4x5_exti.c
index 495a0004ab..eebefc6cd3 100644
--- a/hw/misc/stm32l4x5_exti.c
+++ b/hw/misc/stm32l4x5_exti.c
@@ -42,7 +42,6 @@
 #define EXTI_SWIER2 0x30
 #define EXTI_PR2    0x34
 
-#define EXTI_NUM_GPIO_EVENT_IN_LINES 16
 #define EXTI_MAX_IRQ_PER_BANK 32
 #define EXTI_IRQS_BANK0  32
 #define EXTI_IRQS_BANK1  8
@@ -241,7 +240,7 @@ static void stm32l4x5_exti_init(Object *obj)
 {
     Stm32l4x5ExtiState *s = STM32L4X5_EXTI(obj);
 
-    for (size_t i = 0; i < EXTI_NUM_INTERRUPT_OUT_LINES; i++) {
+    for (size_t i = 0; i < EXTI_NUM_LINES; i++) {
         sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->irq[i]);
     }
 
@@ -249,8 +248,7 @@ static void stm32l4x5_exti_init(Object *obj)
                           TYPE_STM32L4X5_EXTI, 0x400);
     sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->mmio);
 
-    qdev_init_gpio_in(DEVICE(obj), stm32l4x5_exti_set_irq,
-                      EXTI_NUM_GPIO_EVENT_IN_LINES);
+    qdev_init_gpio_in(DEVICE(obj), stm32l4x5_exti_set_irq, EXTI_NUM_LINES);
 }
 
 static const VMStateDescription vmstate_stm32l4x5_exti = {
-- 
2.43.2

Re: [PATCH 1/3] hw/misc: In STM32L4x5 EXTI, consolidate 2 constants

[Peter Maydell]

On Sun, 12 May 2024 at 11:20, Inès Varhol <ines.varhol@telecom-paris.fr> wrote:
>
> Up until now, the EXTI implementation had 16 inbound GPIOs connected to
> the 16 outbound GPIOs of STM32L4x5 SYSCFG.
> The EXTI actually handles 40 lines (namely 5 from STM32L4x5 USART
> devices which are already implemented in QEMU).
> In order to connect USART devices to EXTI, this commit consolidates
> constants `EXTI_NUM_INTERRUPT_OUT_LINES` (40) and
> `EXTI_NUM_GPIO_EVENT_IN_LINES` (16) into `EXTI_NUM_LINES` (40).
>
> Signed-off-by: Inès Varhol <ines.varhol@telecom-paris.fr>
> ---

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

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

[Inès Varhol]

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

[Peter Maydell]

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

[PATCH 3/3] hw/arm: In STM32L4x5 SOC, connect USART devices to EXTI

[Inès Varhol]

The USART devices were previously connecting their outbound IRQs
directly to the CPU because the EXTI wasn't handling direct lines
interrupts.
Now the USART connects to the EXTI inbound GPIOs, and the EXTI connects
its IRQs to the CPU.
The existing QTest for the USART (tests/qtest/stm32l4x5_usart-test.c)
checks that USART1_IRQ in the CPU is pending when expected so it
confirms that the connection through the EXTI still works.

Signed-off-by: Inès Varhol <ines.varhol@telecom-paris.fr>
---
 hw/arm/stm32l4x5_soc.c | 24 +++++++++++-------------
 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/hw/arm/stm32l4x5_soc.c b/hw/arm/stm32l4x5_soc.c
index 38f7a2d5d9..fac83d349c 100644
--- a/hw/arm/stm32l4x5_soc.c
+++ b/hw/arm/stm32l4x5_soc.c
@@ -81,6 +81,10 @@ static const int exti_irq[NUM_EXTI_IRQ] = {
 #define RCC_BASE_ADDRESS 0x40021000
 #define RCC_IRQ 5
 
+#define EXTI_USART1_IRQ 26
+#define EXTI_UART4_IRQ 29
+#define EXTI_LPUART1_IRQ 31
+
 static const int exti_or_gates_out[NUM_EXTI_OR_GATES] = {
     23, 40, 63, 1,
 };
@@ -129,10 +133,6 @@ static const hwaddr uart_addr[] = {
 
 #define LPUART_BASE_ADDRESS 0x40008000
 
-static const int usart_irq[] = { 37, 38, 39 };
-static const int uart_irq[] = { 52, 53 };
-#define LPUART_IRQ 70
-
 static void stm32l4x5_soc_initfn(Object *obj)
 {
     Stm32l4x5SocState *s = STM32L4X5_SOC(obj);
@@ -297,6 +297,7 @@ static void stm32l4x5_soc_realize(DeviceState *dev_soc, Error **errp)
         }
     }
 
+    /* Connect SYSCFG to EXTI */
     for (unsigned i = 0; i < GPIO_NUM_PINS; i++) {
         qdev_connect_gpio_out(DEVICE(&s->syscfg), i,
                               qdev_get_gpio_in(DEVICE(&s->exti), i));
@@ -322,15 +323,10 @@ static void stm32l4x5_soc_realize(DeviceState *dev_soc, Error **errp)
             return;
         }
         sysbus_mmio_map(busdev, 0, usart_addr[i]);
-        sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(armv7m, usart_irq[i]));
+        sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(DEVICE(&s->exti),
+                                                       EXTI_USART1_IRQ + i));
     }
 
-    /*
-     * TODO: Connect the USARTs, UARTs and LPUART to the EXTI once the EXTI
-     * can handle other gpio-in than the gpios. (e.g. Direct Lines for the
-     * usarts)
-     */
-
     /* UART devices */
     for (int i = 0; i < STM_NUM_UARTS; i++) {
         g_autofree char *name = g_strdup_printf("uart%d-out", STM_NUM_USARTS + i + 1);
@@ -343,7 +339,8 @@ static void stm32l4x5_soc_realize(DeviceState *dev_soc, Error **errp)
             return;
         }
         sysbus_mmio_map(busdev, 0, uart_addr[i]);
-        sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(armv7m, uart_irq[i]));
+        sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(DEVICE(&s->exti),
+                                                       EXTI_UART4_IRQ + i));
     }
 
     /* LPUART device*/
@@ -356,7 +353,8 @@ static void stm32l4x5_soc_realize(DeviceState *dev_soc, Error **errp)
         return;
     }
     sysbus_mmio_map(busdev, 0, LPUART_BASE_ADDRESS);
-    sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(armv7m, LPUART_IRQ));
+    sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(DEVICE(&s->exti),
+                                                   EXTI_LPUART1_IRQ));
 
     /* APB1 BUS */
     create_unimplemented_device("TIM2",      0x40000000, 0x400);
-- 
2.43.2

Re: [PATCH 3/3] hw/arm: In STM32L4x5 SOC, connect USART devices to EXTI

[Peter Maydell]

On Sun, 12 May 2024 at 11:20, Inès Varhol <ines.varhol@telecom-paris.fr> wrote:
>
> The USART devices were previously connecting their outbound IRQs
> directly to the CPU because the EXTI wasn't handling direct lines
> interrupts.
> Now the USART connects to the EXTI inbound GPIOs, and the EXTI connects
> its IRQs to the CPU.
> The existing QTest for the USART (tests/qtest/stm32l4x5_usart-test.c)
> checks that USART1_IRQ in the CPU is pending when expected so it
> confirms that the connection through the EXTI still works.
>
> Signed-off-by: Inès Varhol <ines.varhol@telecom-paris.fr>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM