qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v6 0/3] Add device STM32L4x5 EXTI
@ 2024-01-08 18:03 Inès Varhol
  2024-01-08 18:03 ` [PATCH v6 1/3] hw/misc: Implement " Inès Varhol
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Inès Varhol @ 2024-01-08 18:03 UTC (permalink / raw)
  To: qemu-devel
  Cc: Samuel Tardieu, Paolo Bonzini, Philippe Mathieu-Daudé,
	Inès Varhol, qemu-arm, Arnaud Minier, Alistair Francis,
	Laurent Vivier, Thomas Huth, Peter Maydell

This patch adds a new device STM32L4x5 EXTI device and is part
of a series implementing the STM32L4x5 with a few peripherals.

Changes from v5 to v6:
- in `stm32l4x5_exti.c` : adding {} so the code builds with clang
- in `stm32l4x5_exti-test.c` : using a helper function
`exti_set_irq()` to help readability
- in `stm32l4x5_exti-test.c` : correct a mistake in test
`test_edge_selector` (adding a necessary NVIC interrupt unpend
so that the following assertion actually checks something)
- in `stm32l4x5_soc.c` : reducing scope of `i` used in for loops
- in `stm32l4x5_soc.c` : removing useless variable `dev`
- swapping commit 2 (add tests) and commit 3 (connects exti to SoC)
so that the tests pass in the commit they're added

Changes from v4 to v5:
- update the documentation file

Changes from v3 to v4:
- add a test to check that irq trigger selection works correctly
(`test_edge_selector`) and correct `stm32l4x5_exti_set_irq` accordingly

Changes from v2 to v3:
- corrected the license to GPL

Changes from v1 to v2:
- correct the commit messages
- remove a misleading comment

Changes from v3 to v1:
- separating the patch in 3 commits
- justifying in the commit message why we implement a new model instead
of changing the existing stm32f4xx_exti
- changed irq_raise to irq_pulse in register SWIERx write
(in `stm32l4x5_exti_write`) to be consistent with the irq_pulse in
`stm32l4x5_exti_set_irq` (and also both these interrupts are
edge-triggered)
- changed the license to GPL

Changes from v2 to v3:
- adding more tests writing/reading in exti registers
- adding tests checking that interrupt work by reading NVIC registers
- correcting exti_write in SWIER (so it sets an irq only when a bit
goes from '0' to '1')
- correcting exti_set_irq (so it never writes in PR when the relevant
bit in IMR is '0')

Changes from v1 to v2:
- use arrays to deduplicate code and logic
- move internal constant `EXTI_NUM_GPIO_EVENT_IN_LINES` from the header
to the .c file
- Improve copyright headers
- replace `static const` with `#define`
- use the DEFINE_TYPES macro
- fill the `impl` and `valid` field of the exti's `MemoryRegionOps`
- fix invalid test caused by a last minute change

Based-on: 20240108135849.351719-1-ines.varhol@telecom-paris.fr
([PATCH v6 0/2] Add minimal support for the B-L475E-IOT01A board)

Inès Varhol (3):
  hw/misc: Implement STM32L4x5 EXTI
  hw/arm: Connect STM32L4x5 EXTI to STM32L4x5 SoC
  tests/qtest: Add STM32L4x5 EXTI QTest testcase

 docs/system/arm/b-l475e-iot01a.rst |   5 +-
 hw/arm/Kconfig                     |   1 +
 hw/arm/stm32l4x5_soc.c             |  52 ++-
 hw/misc/Kconfig                    |   3 +
 hw/misc/meson.build                |   1 +
 hw/misc/stm32l4x5_exti.c           | 292 ++++++++++++++
 hw/misc/trace-events               |   5 +
 include/hw/arm/stm32l4x5_soc.h     |   3 +
 include/hw/misc/stm32l4x5_exti.h   |  51 +++
 tests/qtest/meson.build            |   5 +
 tests/qtest/stm32l4x5_exti-test.c  | 590 +++++++++++++++++++++++++++++
 11 files changed, 1004 insertions(+), 4 deletions(-)
 create mode 100644 hw/misc/stm32l4x5_exti.c
 create mode 100644 include/hw/misc/stm32l4x5_exti.h
 create mode 100644 tests/qtest/stm32l4x5_exti-test.c

-- 
2.43.0



^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v6 1/3] hw/misc: Implement STM32L4x5 EXTI
  2024-01-08 18:03 [PATCH v6 0/3] Add device STM32L4x5 EXTI Inès Varhol
@ 2024-01-08 18:03 ` Inès Varhol
  2024-01-08 22:56   ` Philippe Mathieu-Daudé
  2024-01-08 18:03 ` [PATCH v6 2/3] hw/arm: Connect STM32L4x5 EXTI to STM32L4x5 SoC Inès Varhol
  2024-01-08 18:03 ` [PATCH v6 3/3] tests/qtest: Add STM32L4x5 EXTI QTest testcase Inès Varhol
  2 siblings, 1 reply; 6+ messages in thread
From: Inès Varhol @ 2024-01-08 18:03 UTC (permalink / raw)
  To: qemu-devel
  Cc: Samuel Tardieu, Paolo Bonzini, Philippe Mathieu-Daudé,
	Inès Varhol, qemu-arm, Arnaud Minier, Alistair Francis,
	Laurent Vivier, Thomas Huth, Peter Maydell

Although very similar to the STM32F4xx EXTI, STM32L4x5 EXTI generates
more than 32 event/interrupt requests and thus uses more registers
than STM32F4xx EXTI which generates 23 event/interrupt requests.

Acked-by: Alistair Francis <alistair.francis@wdc.com>
Signed-off-by: Arnaud Minier <arnaud.minier@telecom-paris.fr>
Signed-off-by: Inès Varhol <ines.varhol@telecom-paris.fr>
---

Should the for loop variables be `unsigned` rather than `int` ?

 docs/system/arm/b-l475e-iot01a.rst |   5 +-
 hw/misc/Kconfig                    |   3 +
 hw/misc/meson.build                |   1 +
 hw/misc/stm32l4x5_exti.c           | 292 +++++++++++++++++++++++++++++
 hw/misc/trace-events               |   5 +
 include/hw/misc/stm32l4x5_exti.h   |  51 +++++
 6 files changed, 354 insertions(+), 3 deletions(-)
 create mode 100644 hw/misc/stm32l4x5_exti.c
 create mode 100644 include/hw/misc/stm32l4x5_exti.h

diff --git a/docs/system/arm/b-l475e-iot01a.rst b/docs/system/arm/b-l475e-iot01a.rst
index 2b128e6b84..72f256ace7 100644
--- a/docs/system/arm/b-l475e-iot01a.rst
+++ b/docs/system/arm/b-l475e-iot01a.rst
@@ -12,17 +12,16 @@ USART, I2C, SPI, CAN and USB OTG, as well as a variety of sensors.
 Supported devices
 """""""""""""""""
 
-Currently, B-L475E-IOT01A machine's implementation is minimal,
-it only supports the following device:
+Currently B-L475E-IOT01A machine's only supports the following devices:
 
 - Cortex-M4F based STM32L4x5 SoC
+- STM32L4x5 EXTI (Extended interrupts and events controller)
 
 Missing devices
 """""""""""""""
 
 The B-L475E-IOT01A does *not* support the following devices:
 
-- Extended interrupts and events controller (EXTI)
 - Reset and clock control (RCC)
 - Serial ports (UART)
 - System configuration controller (SYSCFG)
diff --git a/hw/misc/Kconfig b/hw/misc/Kconfig
index cc8a8c1418..3efe3dc2cc 100644
--- a/hw/misc/Kconfig
+++ b/hw/misc/Kconfig
@@ -87,6 +87,9 @@ config STM32F4XX_SYSCFG
 config STM32F4XX_EXTI
     bool
 
+config STM32L4X5_EXTI
+    bool
+
 config MIPS_ITU
     bool
 
diff --git a/hw/misc/meson.build b/hw/misc/meson.build
index 36c20d5637..16db6e228d 100644
--- a/hw/misc/meson.build
+++ b/hw/misc/meson.build
@@ -110,6 +110,7 @@ system_ss.add(when: 'CONFIG_XLNX_VERSAL_TRNG', if_true: files(
 system_ss.add(when: 'CONFIG_STM32F2XX_SYSCFG', if_true: files('stm32f2xx_syscfg.c'))
 system_ss.add(when: 'CONFIG_STM32F4XX_SYSCFG', if_true: files('stm32f4xx_syscfg.c'))
 system_ss.add(when: 'CONFIG_STM32F4XX_EXTI', if_true: files('stm32f4xx_exti.c'))
+system_ss.add(when: 'CONFIG_STM32L4X5_EXTI', if_true: files('stm32l4x5_exti.c'))
 system_ss.add(when: 'CONFIG_MPS2_FPGAIO', if_true: files('mps2-fpgaio.c'))
 system_ss.add(when: 'CONFIG_MPS2_SCC', if_true: files('mps2-scc.c'))
 
diff --git a/hw/misc/stm32l4x5_exti.c b/hw/misc/stm32l4x5_exti.c
new file mode 100644
index 0000000000..aedf1fb370
--- /dev/null
+++ b/hw/misc/stm32l4x5_exti.c
@@ -0,0 +1,292 @@
+/*
+ * STM32L4x5 EXTI (Extended interrupts and events controller)
+ *
+ * Copyright (c) 2023 Arnaud Minier <arnaud.minier@telecom-paris.fr>
+ * Copyright (c) 2023 Samuel Tardieu <samuel.tardieu@telecom-paris.fr>
+ * Copyright (c) 2023 Inès Varhol <ines.varhol@telecom-paris.fr>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ * This work is based on the stm32f4xx_exti by Alistair Francis.
+ * Original code is licensed under the MIT License:
+ *
+ * Copyright (c) 2014 Alistair Francis <alistair@alistair23.me>
+ */
+
+/*
+ * The reference used is the STMicroElectronics RM0351 Reference manual
+ * for STM32L4x5 and STM32L4x6 advanced Arm ® -based 32-bit MCUs.
+ * https://www.st.com/en/microcontrollers-microprocessors/stm32l4x5/documentation.html
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "trace.h"
+#include "hw/irq.h"
+#include "migration/vmstate.h"
+#include "hw/misc/stm32l4x5_exti.h"
+
+#define EXTI_IMR1   0x00
+#define EXTI_EMR1   0x04
+#define EXTI_RTSR1  0x08
+#define EXTI_FTSR1  0x0C
+#define EXTI_SWIER1 0x10
+#define EXTI_PR1    0x14
+#define EXTI_IMR2   0x20
+#define EXTI_EMR2   0x24
+#define EXTI_RTSR2  0x28
+#define EXTI_FTSR2  0x2C
+#define EXTI_SWIER2 0x30
+#define EXTI_PR2    0x34
+
+#define EXTI_NUM_GPIO_EVENT_IN_LINES 16
+
+/* 0b11111111_10000010_00000000_00000000 */
+#define DIRECT_LINE_MASK1 0xFF820000
+/* 0b00000000_00000000_00000000_10000111 */
+#define DIRECT_LINE_MASK2 0x00000087
+/* 0b11111111_11111111_11111111_00000000 */
+#define RESERVED_BITS_MASK2 0xFFFFFF00
+
+/* 0b00000000_00000000_00000000_01111000 */
+#define ACTIVABLE_MASK2 (~DIRECT_LINE_MASK2 & ~RESERVED_BITS_MASK2)
+
+static void stm32l4x5_exti_reset_hold(Object *obj)
+{
+    Stm32l4x5ExtiState *s = STM32L4X5_EXTI(obj);
+
+    s->imr[0] = DIRECT_LINE_MASK1;
+    s->emr[0] = 0x00000000;
+    s->rtsr[0] = 0x00000000;
+    s->ftsr[0] = 0x00000000;
+    s->swier[0] = 0x00000000;
+    s->pr[0] = 0x00000000;
+
+    s->imr[1] = DIRECT_LINE_MASK2;
+    s->emr[1] = 0x00000000;
+    s->rtsr[1] = 0x00000000;
+    s->ftsr[1] = 0x00000000;
+    s->swier[1] = 0x00000000;
+    s->pr[1] = 0x00000000;
+}
+
+static void stm32l4x5_exti_set_irq(void *opaque, int irq, int level)
+{
+    Stm32l4x5ExtiState *s = opaque;
+    const unsigned n = irq >= 32;
+    const int oirq = irq;
+
+    trace_stm32l4x5_exti_set_irq(irq, level);
+
+    if (irq >= 32) {
+        /* Shift the value to enable access in x2 registers. */
+        irq -= 32;
+    }
+
+    /* If the interrupt is masked, pr won't be raised */
+    if (!((1 << irq) & s->imr[n])) {
+        return;
+    }
+
+    if (((1 << irq) & s->rtsr[n]) && level) {
+        /* Rising Edge */
+        s->pr[n] |= 1 << irq;
+        qemu_irq_pulse(s->irq[oirq]);
+    } else if (((1 << irq) & s->ftsr[n]) && !level) {
+        /* Falling Edge */
+        s->pr[n] |= 1 << irq;
+        qemu_irq_pulse(s->irq[oirq]);
+    }
+
+}
+
+static uint64_t stm32l4x5_exti_read(void *opaque, hwaddr addr,
+                                    unsigned int size)
+{
+    Stm32l4x5ExtiState *s = opaque;
+    uint32_t r = 0;
+    const unsigned n = addr >= EXTI_IMR2;
+
+    switch (addr) {
+    case EXTI_IMR1:
+    case EXTI_IMR2:
+        r = s->imr[n];
+        break;
+    case EXTI_EMR1:
+    case EXTI_EMR2:
+        r = s->emr[n];
+        break;
+    case EXTI_RTSR1:
+    case EXTI_RTSR2:
+        r = s->rtsr[n];
+        break;
+    case EXTI_FTSR1:
+    case EXTI_FTSR2:
+        r = s->ftsr[n];
+        break;
+    case EXTI_SWIER1:
+    case EXTI_SWIER2:
+        r = s->swier[n];
+        break;
+    case EXTI_PR1:
+    case EXTI_PR2:
+        r = s->pr[n];
+        break;
+
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "STM32L4X5_exti_read: Bad offset 0x%x\n", (int)addr);
+        break;
+    }
+
+    trace_stm32l4x5_exti_read(addr, r);
+
+    return r;
+}
+
+static void stm32l4x5_exti_write(void *opaque, hwaddr addr,
+                                 uint64_t val64, unsigned int size)
+{
+    Stm32l4x5ExtiState *s = opaque;
+    const uint32_t value = (uint32_t)val64;
+
+    trace_stm32l4x5_exti_write(addr, value);
+
+    switch (addr) {
+    case EXTI_IMR1:
+        s->imr[0] = value;
+        return;
+    case EXTI_EMR1:
+        s->emr[0] = value;
+        return;
+    case EXTI_RTSR1:
+        s->rtsr[0] = value & ~DIRECT_LINE_MASK1;
+        return;
+    case EXTI_FTSR1:
+        s->ftsr[0] = value & ~DIRECT_LINE_MASK1;
+        return;
+    case EXTI_SWIER1: {
+        const uint32_t set1 = value & ~DIRECT_LINE_MASK1;
+        const uint32_t pend1 = set1 & ~s->swier[0] & s->imr[0] & ~s->pr[0];
+        s->swier[0] = set1;
+        s->pr[0] |= pend1;
+        for (int i = 0; i < 32; i++) {
+            if (pend1 & (1 << i)) {
+                qemu_irq_pulse(s->irq[i]);
+            }
+        }
+        return;
+    }
+    case EXTI_PR1: {
+        const uint32_t cleared1 = s->pr[0] & value & ~DIRECT_LINE_MASK1;
+        /* This bit is cleared by writing a 1 to it */
+        s->pr[0] &= ~cleared1;
+        /* Software triggered interrupts are cleared as well */
+        s->swier[0] &= ~cleared1;
+        return;
+    }
+    case EXTI_IMR2:
+        s->imr[1] = value & ~RESERVED_BITS_MASK2;
+        return;
+    case EXTI_EMR2:
+        s->emr[1] = value & ~RESERVED_BITS_MASK2;
+        return;
+    case EXTI_RTSR2:
+        s->rtsr[1] = value & ACTIVABLE_MASK2;
+        return;
+    case EXTI_FTSR2:
+        s->ftsr[1] = value & ACTIVABLE_MASK2;
+        return;
+    case EXTI_SWIER2: {
+        const uint32_t set2 = value & ACTIVABLE_MASK2;
+        const uint32_t pend2 = set2 & ~s->swier[1] & s->imr[1] & ~s->pr[1];
+        s->swier[1] = set2;
+        s->pr[1] |= pend2;
+        for (int i = 0; i < 8; i++) {
+            if (pend2 & (1 << i)) {
+                qemu_irq_pulse(s->irq[32 + i]);
+            }
+        }
+        return;
+    }
+    case EXTI_PR2: {
+        const uint32_t cleared = s->pr[1] & value & ACTIVABLE_MASK2;
+        /* This bit is cleared by writing a 1 to it */
+        s->pr[1] &= ~cleared;
+        /* Software triggered interrupts are cleared as well */
+        s->swier[1] &= ~cleared;
+        return;
+    }
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "STM32L4X5_exti_write: Bad offset 0x%x\n", (int)addr);
+    }
+}
+
+static const MemoryRegionOps stm32l4x5_exti_ops = {
+    .read = stm32l4x5_exti_read,
+    .write = stm32l4x5_exti_write,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+    .impl.min_access_size = 4,
+    .impl.max_access_size = 4,
+    .impl.unaligned = false,
+    .valid.min_access_size = 4,
+    .valid.max_access_size = 4,
+    .valid.unaligned = false,
+};
+
+static void stm32l4x5_exti_init(Object *obj)
+{
+    Stm32l4x5ExtiState *s = STM32L4X5_EXTI(obj);
+    int i;
+
+    for (i = 0; i < EXTI_NUM_INTERRUPT_OUT_LINES; i++) {
+        sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->irq[i]);
+    }
+
+    memory_region_init_io(&s->mmio, obj, &stm32l4x5_exti_ops, s,
+                          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);
+}
+
+static const VMStateDescription vmstate_stm32l4x5_exti = {
+    .name = TYPE_STM32L4X5_EXTI,
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32_ARRAY(imr, Stm32l4x5ExtiState, EXTI_NUM_REGISTER),
+        VMSTATE_UINT32_ARRAY(emr, Stm32l4x5ExtiState, EXTI_NUM_REGISTER),
+        VMSTATE_UINT32_ARRAY(rtsr, Stm32l4x5ExtiState, EXTI_NUM_REGISTER),
+        VMSTATE_UINT32_ARRAY(ftsr, Stm32l4x5ExtiState, EXTI_NUM_REGISTER),
+        VMSTATE_UINT32_ARRAY(swier, Stm32l4x5ExtiState, EXTI_NUM_REGISTER),
+        VMSTATE_UINT32_ARRAY(pr, Stm32l4x5ExtiState, EXTI_NUM_REGISTER),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static void stm32l4x5_exti_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    ResettableClass *rc = RESETTABLE_CLASS(klass);
+
+    dc->vmsd = &vmstate_stm32l4x5_exti;
+    rc->phases.hold = stm32l4x5_exti_reset_hold;
+}
+
+static const TypeInfo stm32l4x5_exti_types[] = {
+    {
+        .name          = TYPE_STM32L4X5_EXTI,
+        .parent        = TYPE_SYS_BUS_DEVICE,
+        .instance_size = sizeof(Stm32l4x5ExtiState),
+        .instance_init = stm32l4x5_exti_init,
+        .class_init    = stm32l4x5_exti_class_init,
+    }
+};
+
+DEFINE_TYPES(stm32l4x5_exti_types)
diff --git a/hw/misc/trace-events b/hw/misc/trace-events
index 85725506bf..fccd3204bf 100644
--- a/hw/misc/trace-events
+++ b/hw/misc/trace-events
@@ -163,6 +163,11 @@ stm32f4xx_exti_set_irq(int irq, int level) "Set EXTI: %d to %d"
 stm32f4xx_exti_read(uint64_t addr) "reg read: addr: 0x%" PRIx64 " "
 stm32f4xx_exti_write(uint64_t addr, uint64_t data) "reg write: addr: 0x%" PRIx64 " val: 0x%" PRIx64 ""
 
+# stm32l4x5_exti.c
+stm32l4x5_exti_set_irq(int irq, int level) "Set EXTI: %d to %d"
+stm32l4x5_exti_read(uint64_t addr, uint64_t data) "reg read: addr: 0x%" PRIx64 " val: 0x%" PRIx64 ""
+stm32l4x5_exti_write(uint64_t addr, uint64_t data) "reg write: addr: 0x%" PRIx64 " val: 0x%" PRIx64 ""
+
 # tz-mpc.c
 tz_mpc_reg_read(uint32_t offset, uint64_t data, unsigned size) "TZ MPC regs read: offset 0x%x data 0x%" PRIx64 " size %u"
 tz_mpc_reg_write(uint32_t offset, uint64_t data, unsigned size) "TZ MPC regs write: offset 0x%x data 0x%" PRIx64 " size %u"
diff --git a/include/hw/misc/stm32l4x5_exti.h b/include/hw/misc/stm32l4x5_exti.h
new file mode 100644
index 0000000000..be961d2f01
--- /dev/null
+++ b/include/hw/misc/stm32l4x5_exti.h
@@ -0,0 +1,51 @@
+/*
+ * STM32L4x5 EXTI (Extended interrupts and events controller)
+ *
+ * Copyright (c) 2023 Arnaud Minier <arnaud.minier@telecom-paris.fr>
+ * Copyright (c) 2023 Inès Varhol <ines.varhol@telecom-paris.fr>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ * This work is based on the stm32f4xx_exti by Alistair Francis.
+ * Original code is licensed under the MIT License:
+ *
+ * Copyright (c) 2014 Alistair Francis <alistair@alistair23.me>
+ */
+
+/*
+ * The reference used is the STMicroElectronics RM0351 Reference manual
+ * for STM32L4x5 and STM32L4x6 advanced Arm ® -based 32-bit MCUs.
+ * https://www.st.com/en/microcontrollers-microprocessors/stm32l4x5/documentation.html
+ */
+
+#ifndef HW_STM32L4X5_EXTI_H
+#define HW_STM32L4X5_EXTI_H
+
+#include "hw/sysbus.h"
+#include "qom/object.h"
+
+#define TYPE_STM32L4X5_EXTI "stm32l4x5-exti"
+OBJECT_DECLARE_SIMPLE_TYPE(Stm32l4x5ExtiState, STM32L4X5_EXTI)
+
+#define EXTI_NUM_INTERRUPT_OUT_LINES 40
+#define EXTI_NUM_REGISTER 2
+
+struct Stm32l4x5ExtiState {
+    SysBusDevice parent_obj;
+
+    MemoryRegion mmio;
+
+    uint32_t imr[EXTI_NUM_REGISTER];
+    uint32_t emr[EXTI_NUM_REGISTER];
+    uint32_t rtsr[EXTI_NUM_REGISTER];
+    uint32_t ftsr[EXTI_NUM_REGISTER];
+    uint32_t swier[EXTI_NUM_REGISTER];
+    uint32_t pr[EXTI_NUM_REGISTER];
+
+    qemu_irq irq[EXTI_NUM_INTERRUPT_OUT_LINES];
+};
+
+#endif
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v6 2/3] hw/arm: Connect STM32L4x5 EXTI to STM32L4x5 SoC
  2024-01-08 18:03 [PATCH v6 0/3] Add device STM32L4x5 EXTI Inès Varhol
  2024-01-08 18:03 ` [PATCH v6 1/3] hw/misc: Implement " Inès Varhol
@ 2024-01-08 18:03 ` Inès Varhol
  2024-01-08 18:03 ` [PATCH v6 3/3] tests/qtest: Add STM32L4x5 EXTI QTest testcase Inès Varhol
  2 siblings, 0 replies; 6+ messages in thread
From: Inès Varhol @ 2024-01-08 18:03 UTC (permalink / raw)
  To: qemu-devel
  Cc: Samuel Tardieu, Paolo Bonzini, Philippe Mathieu-Daudé,
	Inès Varhol, qemu-arm, Arnaud Minier, Alistair Francis,
	Laurent Vivier, Thomas Huth, Peter Maydell

Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Signed-off-by: Arnaud Minier <arnaud.minier@telecom-paris.fr>
Signed-off-by: Inès Varhol <ines.varhol@telecom-paris.fr>
---
 hw/arm/Kconfig                 |  1 +
 hw/arm/stm32l4x5_soc.c         | 52 +++++++++++++++++++++++++++++++++-
 include/hw/arm/stm32l4x5_soc.h |  3 ++
 3 files changed, 55 insertions(+), 1 deletion(-)

diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 4ae2073a1d..8c8488a70a 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -459,6 +459,7 @@ config STM32L4X5_SOC
     bool
     select ARM_V7M
     select OR_IRQ
+    select STM32L4X5_EXTI
 
 config XLNX_ZYNQMP_ARM
     bool
diff --git a/hw/arm/stm32l4x5_soc.c b/hw/arm/stm32l4x5_soc.c
index 70609a6dac..fe46b7c6c0 100644
--- a/hw/arm/stm32l4x5_soc.c
+++ b/hw/arm/stm32l4x5_soc.c
@@ -36,10 +36,51 @@
 #define SRAM2_BASE_ADDRESS 0x10000000
 #define SRAM2_SIZE (32 * KiB)
 
+#define EXTI_ADDR 0x40010400
+
+#define NUM_EXTI_IRQ 40
+/* Match exti line connections with their CPU IRQ number */
+/* See Vector Table (Reference Manual p.396) */
+static const int exti_irq[NUM_EXTI_IRQ] = {
+    6,                      /* GPIO[0]                 */
+    7,                      /* GPIO[1]                 */
+    8,                      /* GPIO[2]                 */
+    9,                      /* GPIO[3]                 */
+    10,                     /* GPIO[4]                 */
+    23, 23, 23, 23, 23,     /* GPIO[5..9]              */
+    40, 40, 40, 40, 40, 40, /* GPIO[10..15]            */
+    1,                      /* PVD                     */
+    67,                     /* OTG_FS_WKUP, Direct     */
+    41,                     /* RTC_ALARM               */
+    2,                      /* RTC_TAMP_STAMP2/CSS_LSE */
+    3,                      /* RTC wakeup timer        */
+    63,                     /* COMP1                   */
+    63,                     /* COMP2                   */
+    31,                     /* I2C1 wakeup, Direct     */
+    33,                     /* I2C2 wakeup, Direct     */
+    72,                     /* I2C3 wakeup, Direct     */
+    37,                     /* USART1 wakeup, Direct   */
+    38,                     /* USART2 wakeup, Direct   */
+    39,                     /* USART3 wakeup, Direct   */
+    52,                     /* UART4 wakeup, Direct    */
+    53,                     /* UART4 wakeup, Direct    */
+    70,                     /* LPUART1 wakeup, Direct  */
+    65,                     /* LPTIM1, Direct          */
+    66,                     /* LPTIM2, Direct          */
+    76,                     /* SWPMI1 wakeup, Direct   */
+    1,                      /* PVM1 wakeup             */
+    1,                      /* PVM2 wakeup             */
+    1,                      /* PVM3 wakeup             */
+    1,                      /* PVM4 wakeup             */
+    78                      /* LCD wakeup, Direct      */
+};
+
 static void stm32l4x5_soc_initfn(Object *obj)
 {
     Stm32l4x5SocState *s = STM32L4X5_SOC(obj);
 
+    object_initialize_child(obj, "exti", &s->exti, TYPE_STM32L4X5_EXTI);
+
     s->sysclk = qdev_init_clock_in(DEVICE(s), "sysclk", NULL, NULL, 0);
     s->refclk = qdev_init_clock_in(DEVICE(s), "refclk", NULL, NULL, 0);
 }
@@ -51,6 +92,7 @@ static void stm32l4x5_soc_realize(DeviceState *dev_soc, Error **errp)
     const Stm32l4x5SocClass *sc = STM32L4X5_SOC_GET_CLASS(dev_soc);
     MemoryRegion *system_memory = get_system_memory();
     DeviceState *armv7m;
+    SysBusDevice *busdev;
 
     /*
      * We use s->refclk internally and only define it with qdev_init_clock_in()
@@ -112,6 +154,15 @@ static void stm32l4x5_soc_realize(DeviceState *dev_soc, Error **errp)
         return;
     }
 
+    busdev = SYS_BUS_DEVICE(&s->exti);
+    if (!sysbus_realize(busdev, errp)) {
+        return;
+    }
+    sysbus_mmio_map(busdev, 0, EXTI_ADDR);
+    for (unsigned i = 0; i < NUM_EXTI_IRQ; i++) {
+        sysbus_connect_irq(busdev, i, qdev_get_gpio_in(armv7m, exti_irq[i]));
+    }
+
     /* APB1 BUS */
     create_unimplemented_device("TIM2",      0x40000000, 0x400);
     create_unimplemented_device("TIM3",      0x40000400, 0x400);
@@ -152,7 +203,6 @@ static void stm32l4x5_soc_realize(DeviceState *dev_soc, Error **errp)
     create_unimplemented_device("SYSCFG",    0x40010000, 0x30);
     create_unimplemented_device("VREFBUF",   0x40010030, 0x1D0);
     create_unimplemented_device("COMP",      0x40010200, 0x200);
-    create_unimplemented_device("EXTI",      0x40010400, 0x400);
     /* RESERVED:    0x40010800, 0x1400 */
     create_unimplemented_device("FIREWALL",  0x40011C00, 0x400);
     /* RESERVED:    0x40012000, 0x800 */
diff --git a/include/hw/arm/stm32l4x5_soc.h b/include/hw/arm/stm32l4x5_soc.h
index 2fd44a36a9..f7305568dc 100644
--- a/include/hw/arm/stm32l4x5_soc.h
+++ b/include/hw/arm/stm32l4x5_soc.h
@@ -26,6 +26,7 @@
 
 #include "exec/memory.h"
 #include "hw/arm/armv7m.h"
+#include "hw/misc/stm32l4x5_exti.h"
 #include "qom/object.h"
 
 #define TYPE_STM32L4X5_SOC "stm32l4x5-soc"
@@ -39,6 +40,8 @@ struct Stm32l4x5SocState {
 
     ARMv7MState armv7m;
 
+    Stm32l4x5ExtiState exti;
+
     MemoryRegion sram1;
     MemoryRegion sram2;
     MemoryRegion flash;
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v6 3/3] tests/qtest: Add STM32L4x5 EXTI QTest testcase
  2024-01-08 18:03 [PATCH v6 0/3] Add device STM32L4x5 EXTI Inès Varhol
  2024-01-08 18:03 ` [PATCH v6 1/3] hw/misc: Implement " Inès Varhol
  2024-01-08 18:03 ` [PATCH v6 2/3] hw/arm: Connect STM32L4x5 EXTI to STM32L4x5 SoC Inès Varhol
@ 2024-01-08 18:03 ` Inès Varhol
  2024-01-08 21:31   ` Philippe Mathieu-Daudé
  2 siblings, 1 reply; 6+ messages in thread
From: Inès Varhol @ 2024-01-08 18:03 UTC (permalink / raw)
  To: qemu-devel
  Cc: Samuel Tardieu, Paolo Bonzini, Philippe Mathieu-Daudé,
	Inès Varhol, qemu-arm, Arnaud Minier, Alistair Francis,
	Laurent Vivier, Thomas Huth, Peter Maydell

Acked-by: Alistair Francis <alistair.francis@wdc.com>
Signed-off-by: Arnaud Minier <arnaud.minier@telecom-paris.fr>
Signed-off-by: Inès Varhol <ines.varhol@telecom-paris.fr>
---
 tests/qtest/meson.build           |   5 +
 tests/qtest/stm32l4x5_exti-test.c | 590 ++++++++++++++++++++++++++++++
 2 files changed, 595 insertions(+)
 create mode 100644 tests/qtest/stm32l4x5_exti-test.c

diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index f25bffcc20..d890b6f333 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -194,6 +194,10 @@ qtests_aspeed = \
   ['aspeed_hace-test',
    'aspeed_smc-test',
    'aspeed_gpio-test']
+
+qtests_stm32l4x5 = \
+  ['stm32l4x5_exti-test']
+
 qtests_arm = \
   (config_all_devices.has_key('CONFIG_MPS2') ? ['sse-timer-test'] : []) + \
   (config_all_devices.has_key('CONFIG_CMSDK_APB_DUALTIMER') ? ['cmsdk-apb-dualtimer-test'] : []) + \
@@ -207,6 +211,7 @@ qtests_arm = \
   (config_all_devices.has_key('CONFIG_TPM_TIS_I2C') ? ['tpm-tis-i2c-test'] : []) + \
   (config_all_devices.has_key('CONFIG_VEXPRESS') ? ['test-arm-mptimer'] : []) + \
   (config_all_devices.has_key('CONFIG_MICROBIT') ? ['microbit-test'] : []) + \
+  (config_all_devices.has_key('CONFIG_STM32L4X5_SOC') ? qtests_stm32l4x5 : []) + \
   ['arm-cpu-features',
    'boot-serial-test']
 
diff --git a/tests/qtest/stm32l4x5_exti-test.c b/tests/qtest/stm32l4x5_exti-test.c
new file mode 100644
index 0000000000..faba530a24
--- /dev/null
+++ b/tests/qtest/stm32l4x5_exti-test.c
@@ -0,0 +1,590 @@
+/*
+ * QTest testcase for STM32L4x5_EXTI
+ *
+ * Copyright (c) 2023 Arnaud Minier <arnaud.minier@telecom-paris.fr>
+ * Copyright (c) 2023 Inès Varhol <ines.varhol@telecom-paris.fr>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "libqtest-single.h"
+
+#define EXTI_BASE_ADDR 0x40010400
+#define EXTI_IMR1 0x00
+#define EXTI_EMR1 0x04
+#define EXTI_RTSR1 0x08
+#define EXTI_FTSR1 0x0C
+#define EXTI_SWIER1 0x10
+#define EXTI_PR1 0x14
+#define EXTI_IMR2 0x20
+#define EXTI_EMR2 0x24
+#define EXTI_RTSR2 0x28
+#define EXTI_FTSR2 0x2C
+#define EXTI_SWIER2 0x30
+#define EXTI_PR2 0x34
+
+#define NVIC_ISER 0xE000E100
+#define NVIC_ISPR 0xE000E200
+#define NVIC_ICPR 0xE000E280
+
+#define EXTI0_IRQ 6
+#define EXTI1_IRQ 7
+#define EXTI35_IRQ 1
+
+static void enable_nvic_irq(unsigned int n)
+{
+    writel(NVIC_ISER, 1 << n);
+}
+
+static void unpend_nvic_irq(unsigned int n)
+{
+    writel(NVIC_ICPR, 1 << n);
+}
+
+static bool check_nvic_pending(unsigned int n)
+{
+    return readl(NVIC_ISPR) & (1 << n);
+}
+
+static void exti_writel(unsigned int offset, uint32_t value)
+{
+    writel(EXTI_BASE_ADDR + offset, value);
+}
+
+static uint32_t exti_readl(unsigned int offset)
+{
+    return readl(EXTI_BASE_ADDR + offset);
+}
+
+static void exti_set_irq(int num, int level)
+{
+   qtest_set_irq_in(global_qtest, "/machine/soc/exti", NULL,
+                    num, level);
+}
+
+static void test_reg_write_read(void)
+{
+    /* Test that non-reserved bits in xMR and xTSR can be set and cleared */
+
+    exti_writel(EXTI_IMR1, 0xFFFFFFFF);
+    uint32_t imr1 = exti_readl(EXTI_IMR1);
+    g_assert_cmpuint(imr1, ==, 0xFFFFFFFF);
+    exti_writel(EXTI_IMR1, 0x00000000);
+    imr1 = exti_readl(EXTI_IMR1);
+    g_assert_cmpuint(imr1, ==, 0x00000000);
+
+    exti_writel(EXTI_EMR1, 0xFFFFFFFF);
+    uint32_t emr1 = exti_readl(EXTI_EMR1);
+    g_assert_cmpuint(emr1, ==, 0xFFFFFFFF);
+    exti_writel(EXTI_EMR1, 0x00000000);
+    emr1 = exti_readl(EXTI_EMR1);
+    g_assert_cmpuint(emr1, ==, 0x00000000);
+
+    exti_writel(EXTI_RTSR1, 0xFFFFFFFF);
+    uint32_t rtsr1 = exti_readl(EXTI_RTSR1);
+    g_assert_cmpuint(rtsr1, ==, 0x007DFFFF);
+    exti_writel(EXTI_RTSR1, 0x00000000);
+    rtsr1 = exti_readl(EXTI_RTSR1);
+    g_assert_cmpuint(rtsr1, ==, 0x00000000);
+
+    exti_writel(EXTI_FTSR1, 0xFFFFFFFF);
+    uint32_t ftsr1 = exti_readl(EXTI_FTSR1);
+    g_assert_cmpuint(ftsr1, ==, 0x007DFFFF);
+    exti_writel(EXTI_FTSR1, 0x00000000);
+    ftsr1 = exti_readl(EXTI_FTSR1);
+    g_assert_cmpuint(ftsr1, ==, 0x00000000);
+
+    exti_writel(EXTI_IMR2, 0xFFFFFFFF);
+    uint32_t imr2 = exti_readl(EXTI_IMR2);
+    g_assert_cmpuint(imr2, ==, 0x000000FF);
+    exti_writel(EXTI_IMR2, 0x00000000);
+    imr2 = exti_readl(EXTI_IMR2);
+    g_assert_cmpuint(imr2, ==, 0x00000000);
+
+    exti_writel(EXTI_EMR2, 0xFFFFFFFF);
+    uint32_t emr2 = exti_readl(EXTI_EMR2);
+    g_assert_cmpuint(emr2, ==, 0x000000FF);
+    exti_writel(EXTI_EMR2, 0x00000000);
+    emr2 = exti_readl(EXTI_EMR2);
+    g_assert_cmpuint(emr2, ==, 0x00000000);
+
+    exti_writel(EXTI_RTSR2, 0xFFFFFFFF);
+    uint32_t rtsr2 = exti_readl(EXTI_RTSR2);
+    g_assert_cmpuint(rtsr2, ==, 0x00000078);
+    exti_writel(EXTI_RTSR2, 0x00000000);
+    rtsr2 = exti_readl(EXTI_RTSR2);
+    g_assert_cmpuint(rtsr2, ==, 0x00000000);
+
+    exti_writel(EXTI_FTSR2, 0xFFFFFFFF);
+    uint32_t ftsr2 = exti_readl(EXTI_FTSR2);
+    g_assert_cmpuint(ftsr2, ==, 0x00000078);
+    exti_writel(EXTI_FTSR2, 0x00000000);
+    ftsr2 = exti_readl(EXTI_FTSR2);
+    g_assert_cmpuint(ftsr2, ==, 0x00000000);
+}
+
+static void test_direct_lines_write(void)
+{
+    /* Test that direct lines reserved bits are not written to */
+
+    exti_writel(EXTI_RTSR1, 0xFF820000);
+    uint32_t rtsr1 = exti_readl(EXTI_RTSR1);
+    g_assert_cmpuint(rtsr1, ==, 0x00000000);
+
+    exti_writel(EXTI_FTSR1, 0xFF820000);
+    uint32_t ftsr1 = exti_readl(EXTI_FTSR1);
+    g_assert_cmpuint(ftsr1, ==, 0x00000000);
+
+    exti_writel(EXTI_SWIER1, 0xFF820000);
+    uint32_t swier1 = exti_readl(EXTI_SWIER1);
+    g_assert_cmpuint(swier1, ==, 0x00000000);
+
+    exti_writel(EXTI_PR1, 0xFF820000);
+    uint32_t pr1 = exti_readl(EXTI_PR1);
+    g_assert_cmpuint(pr1, ==, 0x00000000);
+
+    exti_writel(EXTI_RTSR2, 0x00000087);
+    const uint32_t rtsr2 = exti_readl(EXTI_RTSR2);
+    g_assert_cmpuint(rtsr2, ==, 0x00000000);
+
+    exti_writel(EXTI_FTSR2, 0x00000087);
+    const uint32_t ftsr2 = exti_readl(EXTI_FTSR2);
+    g_assert_cmpuint(ftsr2, ==, 0x00000000);
+
+    exti_writel(EXTI_SWIER2, 0x00000087);
+    const uint32_t swier2 = exti_readl(EXTI_SWIER2);
+    g_assert_cmpuint(swier2, ==, 0x00000000);
+
+    exti_writel(EXTI_PR2, 0x00000087);
+    const uint32_t pr2 = exti_readl(EXTI_PR2);
+    g_assert_cmpuint(pr2, ==, 0x00000000);
+}
+
+static void test_reserved_bits_write(void)
+{
+    /* Test that reserved bits stay are not written to */
+
+    exti_writel(EXTI_IMR2, 0xFFFFFF00);
+    uint32_t imr2 = exti_readl(EXTI_IMR2);
+    g_assert_cmpuint(imr2, ==, 0x00000000);
+
+    exti_writel(EXTI_EMR2, 0xFFFFFF00);
+    uint32_t emr2 = exti_readl(EXTI_EMR2);
+    g_assert_cmpuint(emr2, ==, 0x00000000);
+
+    exti_writel(EXTI_RTSR2, 0xFFFFFF00);
+    const uint32_t rtsr2 = exti_readl(EXTI_RTSR2);
+    g_assert_cmpuint(rtsr2, ==, 0x00000000);
+
+    exti_writel(EXTI_FTSR2, 0xFFFFFF00);
+    const uint32_t ftsr2 = exti_readl(EXTI_FTSR2);
+    g_assert_cmpuint(ftsr2, ==, 0x00000000);
+
+    exti_writel(EXTI_SWIER2, 0xFFFFFF00);
+    const uint32_t swier2 = exti_readl(EXTI_SWIER2);
+    g_assert_cmpuint(swier2, ==, 0x00000000);
+
+    exti_writel(EXTI_PR2, 0xFFFFFF00);
+    const uint32_t pr2 = exti_readl(EXTI_PR2);
+    g_assert_cmpuint(pr2, ==, 0x00000000);
+}
+
+static void test_software_interrupt(void)
+{
+    /*
+     * Test that we can launch a software irq by :
+     * - enabling its line in IMR
+     * - and then setting a bit from '0' to '1' in SWIER
+     *
+     * And that the interruption stays pending in NVIC
+     * even after clearing the pending bit in PR.
+     */
+
+    /*
+     * Testing interrupt line EXTI0
+     * Bit 0 in EXTI_*1 registers (EXTI0) corresponds to GPIO Px_0
+     */
+
+    enable_nvic_irq(EXTI0_IRQ);
+    /* Check that there are no interrupts already pending in PR */
+    uint32_t pr1 = exti_readl(EXTI_PR1);
+    g_assert_cmpuint(pr1, ==, 0x00000000);
+    /* Check that this specific interrupt isn't pending in NVIC */
+    g_assert_false(check_nvic_pending(EXTI0_IRQ));
+
+    /* Enable interrupt line EXTI0 */
+    exti_writel(EXTI_IMR1, 0x00000001);
+    /* Set the right SWIER bit from '0' to '1' */
+    exti_writel(EXTI_SWIER1, 0x00000000);
+    exti_writel(EXTI_SWIER1, 0x00000001);
+
+    /* Check that the write in SWIER was effective */
+    uint32_t swier1 = exti_readl(EXTI_SWIER1);
+    g_assert_cmpuint(swier1, ==, 0x00000001);
+    /* Check that the corresponding pending bit in PR is set */
+    pr1 = exti_readl(EXTI_PR1);
+    g_assert_cmpuint(pr1, ==, 0x00000001);
+    /* Check that the corresponding interrupt is pending in the NVIC */
+    g_assert_true(check_nvic_pending(EXTI0_IRQ));
+
+    /* Clear the pending bit in PR */
+    exti_writel(EXTI_PR1, 0x00000001);
+
+    /* Check that the write in PR was effective */
+    pr1 = exti_readl(EXTI_PR1);
+    g_assert_cmpuint(pr1, ==, 0x00000000);
+    /* Check that the corresponding bit in SWIER was cleared */
+    swier1 = exti_readl(EXTI_SWIER1);
+    g_assert_cmpuint(swier1, ==, 0x00000000);
+    /* Check that the interrupt is still pending in the NVIC */
+    g_assert_true(check_nvic_pending(EXTI0_IRQ));
+
+    /*
+     * Testing interrupt line EXTI35
+     * Bit 3 in EXTI_*2 registers (EXTI35) corresponds to PVM 1 Wakeup
+     */
+
+    enable_nvic_irq(EXTI35_IRQ);
+    /* Check that there are no interrupts already pending */
+    uint32_t pr2 = exti_readl(EXTI_PR2);
+    g_assert_cmpuint(pr2, ==, 0x00000000);
+    g_assert_false(check_nvic_pending(EXTI35_IRQ));
+
+    /* Enable interrupt line EXTI0 */
+    exti_writel(EXTI_IMR2, 0x00000008);
+    /* Set the right SWIER bit from '0' to '1' */
+    exti_writel(EXTI_SWIER2, 0x00000000);
+    exti_writel(EXTI_SWIER2, 0x00000008);
+
+    /* Check that the write in SWIER was effective */
+    uint32_t swier2 = exti_readl(EXTI_SWIER2);
+    g_assert_cmpuint(swier2, ==, 0x00000008);
+    /* Check that the corresponding pending bit in PR is set */
+    pr2 = exti_readl(EXTI_PR2);
+    g_assert_cmpuint(pr2, ==, 0x00000008);
+    /* Check that the corresponding interrupt is pending in the NVIC */
+    g_assert_true(check_nvic_pending(EXTI35_IRQ));
+
+    /* Clear the pending bit in PR */
+    exti_writel(EXTI_PR2, 0x00000008);
+
+    /* Check that the write in PR was effective */
+    pr2 = exti_readl(EXTI_PR2);
+    g_assert_cmpuint(pr2, ==, 0x00000000);
+    /* Check that the corresponding bit in SWIER was cleared */
+    swier2 = exti_readl(EXTI_SWIER2);
+    g_assert_cmpuint(swier2, ==, 0x00000000);
+    /* Check that the interrupt is still pending in the NVIC */
+    g_assert_true(check_nvic_pending(EXTI35_IRQ));
+
+    /* Clean NVIC */
+    unpend_nvic_irq(EXTI0_IRQ);
+    g_assert_false(check_nvic_pending(EXTI0_IRQ));
+    unpend_nvic_irq(EXTI35_IRQ);
+    g_assert_false(check_nvic_pending(EXTI35_IRQ));
+}
+
+static void test_edge_selector(void)
+{
+    enable_nvic_irq(EXTI0_IRQ);
+
+    /* Configure EXTI line 0 irq on rising edge */
+    exti_set_irq(0, 1);
+    exti_writel(EXTI_IMR1, 0x00000001);
+    exti_writel(EXTI_RTSR1, 0x00000001);
+    exti_writel(EXTI_FTSR1, 0x00000000);
+
+    /* Test that an irq is raised on rising edge only */
+    exti_set_irq(0, 0);
+    uint32_t pr1 = exti_readl(EXTI_PR1);
+    g_assert_cmpuint(pr1, ==, 0x00000000);
+    g_assert_false(check_nvic_pending(EXTI0_IRQ));
+
+    exti_set_irq(0, 1);
+    pr1 = exti_readl(EXTI_PR1);
+    g_assert_cmpuint(pr1, ==, 0x00000001);
+    g_assert_true(check_nvic_pending(EXTI0_IRQ));
+
+    /* Clean the test */
+    exti_writel(EXTI_PR1, 0x00000001);
+    pr1 = exti_readl(EXTI_PR1);
+    g_assert_cmpuint(pr1, ==, 0x00000000);
+    unpend_nvic_irq(EXTI0_IRQ);
+    g_assert_false(check_nvic_pending(EXTI0_IRQ));
+
+    /* Configure EXTI line 0 irq on falling edge */
+    exti_set_irq(0, 0);
+    exti_writel(EXTI_IMR1, 0x00000001);
+    exti_writel(EXTI_RTSR1, 0x00000000);
+    exti_writel(EXTI_FTSR1, 0x00000001);
+
+    /* Test that an irq is raised on falling edge only */
+    exti_set_irq(0, 1);
+    pr1 = exti_readl(EXTI_PR1);
+    g_assert_cmpuint(pr1, ==, 0x00000000);
+    g_assert_false(check_nvic_pending(EXTI0_IRQ));
+
+    exti_set_irq(0, 0);
+    pr1 = exti_readl(EXTI_PR1);
+    g_assert_cmpuint(pr1, ==, 0x00000001);
+    g_assert_true(check_nvic_pending(EXTI0_IRQ));
+
+    /* Clean the test */
+    exti_writel(EXTI_PR1, 0x00000001);
+    pr1 = exti_readl(EXTI_PR1);
+    g_assert_cmpuint(pr1, ==, 0x00000000);
+    unpend_nvic_irq(EXTI0_IRQ);
+    g_assert_false(check_nvic_pending(EXTI0_IRQ));
+
+    /* Configure EXTI line 0 irq on falling and rising edge */
+    exti_writel(EXTI_IMR1, 0x00000001);
+    exti_writel(EXTI_RTSR1, 0x00000001);
+    exti_writel(EXTI_FTSR1, 0x00000001);
+
+    /* Test that an irq is raised on rising edge */
+    exti_set_irq(0, 1);
+    pr1 = exti_readl(EXTI_PR1);
+    g_assert_cmpuint(pr1, ==, 0x00000001);
+    g_assert_true(check_nvic_pending(EXTI0_IRQ));
+
+    /* Clean the test */
+    exti_writel(EXTI_PR1, 0x00000001);
+    pr1 = exti_readl(EXTI_PR1);
+    g_assert_cmpuint(pr1, ==, 0x00000000);
+    unpend_nvic_irq(EXTI0_IRQ);
+    g_assert_false(check_nvic_pending(EXTI0_IRQ));
+
+    /* Test that an irq is raised on falling edge */
+    exti_set_irq(0, 0);
+    pr1 = exti_readl(EXTI_PR1);
+    g_assert_cmpuint(pr1, ==, 0x00000001);
+    g_assert_true(check_nvic_pending(EXTI0_IRQ));
+
+    /* Clean the test */
+    exti_writel(EXTI_PR1, 0x00000001);
+    pr1 = exti_readl(EXTI_PR1);
+    g_assert_cmpuint(pr1, ==, 0x00000000);
+    unpend_nvic_irq(EXTI0_IRQ);
+    g_assert_false(check_nvic_pending(EXTI0_IRQ));
+
+    /* Configure EXTI line 0 irq without selecting an edge trigger */
+    exti_writel(EXTI_IMR1, 0x00000001);
+    exti_writel(EXTI_RTSR1, 0x00000000);
+    exti_writel(EXTI_FTSR1, 0x00000000);
+
+    /* Test that no irq is raised */
+    exti_set_irq(0, 1);
+    pr1 = exti_readl(EXTI_PR1);
+    g_assert_cmpuint(pr1, ==, 0x00000000);
+    g_assert_false(check_nvic_pending(EXTI0_IRQ));
+
+    exti_set_irq(0, 0);
+    pr1 = exti_readl(EXTI_PR1);
+    g_assert_cmpuint(pr1, ==, 0x00000000);
+    g_assert_false(check_nvic_pending(EXTI0_IRQ));
+}
+
+static void test_no_software_interrupt(void)
+{
+    /*
+     * Test that software irq doesn't happen when :
+     * - corresponding bit in IMR isn't set
+     * - SWIER is set to 1 before IMR is set to 1
+     */
+
+    /*
+     * Testing interrupt line EXTI0
+     * Bit 0 in EXTI_*1 registers (EXTI0) corresponds to GPIO Px_0
+     */
+
+    enable_nvic_irq(EXTI0_IRQ);
+    /* Check that there are no interrupts already pending in PR */
+    uint32_t pr1 = exti_readl(EXTI_PR1);
+    g_assert_cmpuint(pr1, ==, 0x00000000);
+    /* Check that this specific interrupt isn't pending in NVIC */
+    g_assert_false(check_nvic_pending(EXTI0_IRQ));
+
+    /* Mask interrupt line EXTI0 */
+    exti_writel(EXTI_IMR1, 0x00000000);
+    /* Set the corresponding SWIER bit from '0' to '1' */
+    exti_writel(EXTI_SWIER1, 0x00000000);
+    exti_writel(EXTI_SWIER1, 0x00000001);
+
+    /* Check that the write in SWIER was effective */
+    uint32_t swier1 = exti_readl(EXTI_SWIER1);
+    g_assert_cmpuint(swier1, ==, 0x00000001);
+    /* Check that the pending bit in PR wasn't set */
+    pr1 = exti_readl(EXTI_PR1);
+    g_assert_cmpuint(pr1, ==, 0x00000000);
+    /* Check that the interrupt isn't pending in NVIC */
+    g_assert_false(check_nvic_pending(EXTI0_IRQ));
+
+    /* Enable interrupt line EXTI0 */
+    exti_writel(EXTI_IMR1, 0x00000001);
+
+    /* Check that the pending bit in PR wasn't set */
+    pr1 = exti_readl(EXTI_PR1);
+    g_assert_cmpuint(pr1, ==, 0x00000000);
+    /* Check that the interrupt isn't pending in NVIC */
+    g_assert_false(check_nvic_pending(EXTI0_IRQ));
+
+    /*
+     * Testing interrupt line EXTI35
+     * Bit 3 in EXTI_*2 registers (EXTI35) corresponds to PVM 1 Wakeup
+     */
+
+    enable_nvic_irq(EXTI35_IRQ);
+    /* Check that there are no interrupts already pending in PR */
+    uint32_t pr2 = exti_readl(EXTI_PR2);
+    g_assert_cmpuint(pr2, ==, 0x00000000);
+    /* Check that this specific interrupt isn't pending in NVIC */
+    g_assert_false(check_nvic_pending(EXTI35_IRQ));
+
+    /* Mask interrupt line EXTI35 */
+    exti_writel(EXTI_IMR2, 0x00000000);
+    /* Set the corresponding SWIER bit from '0' to '1' */
+    exti_writel(EXTI_SWIER2, 0x00000000);
+    exti_writel(EXTI_SWIER2, 0x00000008);
+
+    /* Check that the write in SWIER was effective */
+    uint32_t swier2 = exti_readl(EXTI_SWIER2);
+    g_assert_cmpuint(swier2, ==, 0x00000008);
+    /* Check that the pending bit in PR wasn't set */
+    pr2 = exti_readl(EXTI_PR2);
+    g_assert_cmpuint(pr2, ==, 0x00000000);
+    /* Check that the interrupt isn't pending in NVIC */
+    g_assert_false(check_nvic_pending(EXTI35_IRQ));
+
+    /* Enable interrupt line EXTI35 */
+    exti_writel(EXTI_IMR2, 0x00000008);
+
+    /* Check that the pending bit in PR wasn't set */
+    pr2 = exti_readl(EXTI_PR2);
+    g_assert_cmpuint(pr2, ==, 0x00000000);
+    /* Check that the interrupt isn't pending in NVIC */
+    g_assert_false(check_nvic_pending(EXTI35_IRQ));
+}
+
+static void test_masked_interrupt(void)
+{
+    /*
+     * Test that irq doesn't happen when :
+     * - corresponding bit in IMR isn't set
+     * - SWIER is set to 1 before IMR is set to 1
+     */
+
+    /*
+     * Testing interrupt line EXTI1
+     * with rising edge from GPIOx pin 1
+     */
+
+    enable_nvic_irq(EXTI1_IRQ);
+    /* Check that there are no interrupts already pending in PR */
+    uint32_t pr1 = exti_readl(EXTI_PR1);
+    g_assert_cmpuint(pr1, ==, 0x00000000);
+    /* Check that this specific interrupt isn't pending in NVIC */
+    g_assert_false(check_nvic_pending(EXTI1_IRQ));
+
+    /* Mask interrupt line EXTI1 */
+    exti_writel(EXTI_IMR1, 0x00000000);
+
+    /* Configure interrupt on rising edge */
+    exti_writel(EXTI_RTSR1, 0x00000002);
+
+    /* Simulate rising edge from GPIO line 1 */
+    exti_set_irq(1, 1);
+
+    /* Check that the pending bit in PR wasn't set */
+    pr1 = exti_readl(EXTI_PR1);
+    g_assert_cmpuint(pr1, ==, 0x00000000);
+    /* Check that the interrupt isn't pending in NVIC */
+    g_assert_false(check_nvic_pending(EXTI1_IRQ));
+
+    /* Enable interrupt line EXTI1 */
+    exti_writel(EXTI_IMR1, 0x00000002);
+
+    /* Check that the pending bit in PR wasn't set */
+    pr1 = exti_readl(EXTI_PR1);
+    g_assert_cmpuint(pr1, ==, 0x00000000);
+    /* Check that the interrupt isn't pending in NVIC */
+    g_assert_false(check_nvic_pending(EXTI1_IRQ));
+}
+
+static void test_interrupt(void)
+{
+    /*
+     * Test that we can launch an irq by :
+     * - enabling its line in IMR
+     * - configuring interrupt on rising edge
+     * - and then setting the input line from '0' to '1'
+     *
+     * And that the interruption stays pending in NVIC
+     * even after clearing the pending bit in PR.
+     */
+
+    /*
+     * Testing interrupt line EXTI1
+     * with rising edge from GPIOx pin 1
+     */
+
+    enable_nvic_irq(EXTI1_IRQ);
+    /* Check that there are no interrupts already pending in PR */
+    uint32_t pr1 = exti_readl(EXTI_PR1);
+    g_assert_cmpuint(pr1, ==, 0x00000000);
+    /* Check that this specific interrupt isn't pending in NVIC */
+    g_assert_false(check_nvic_pending(EXTI1_IRQ));
+
+    /* Enable interrupt line EXTI1 */
+    exti_writel(EXTI_IMR1, 0x00000002);
+
+    /* Configure interrupt on rising edge */
+    exti_writel(EXTI_RTSR1, 0x00000002);
+
+    /* Simulate rising edge from GPIO line 1 */
+    exti_set_irq(1, 1);
+
+    /* Check that the pending bit in PR was set */
+    pr1 = exti_readl(EXTI_PR1);
+    g_assert_cmpuint(pr1, ==, 0x00000002);
+    /* Check that the interrupt is pending in NVIC */
+    g_assert_true(check_nvic_pending(EXTI1_IRQ));
+
+    /* Clear the pending bit in PR */
+    exti_writel(EXTI_PR1, 0x00000002);
+
+    /* Check that the write in PR was effective */
+    pr1 = exti_readl(EXTI_PR1);
+    g_assert_cmpuint(pr1, ==, 0x00000000);
+    /* Check that the interrupt is still pending in the NVIC */
+    g_assert_true(check_nvic_pending(EXTI1_IRQ));
+
+    /* Clean NVIC */
+    unpend_nvic_irq(EXTI1_IRQ);
+    g_assert_false(check_nvic_pending(EXTI1_IRQ));
+}
+
+int main(int argc, char **argv)
+{
+    int ret;
+
+    g_test_init(&argc, &argv, NULL);
+    g_test_set_nonfatal_assertions();
+    qtest_add_func("stm32l4x5/exti/direct_lines", test_direct_lines_write);
+    qtest_add_func("stm32l4x5/exti/reserved_bits", test_reserved_bits_write);
+    qtest_add_func("stm32l4x5/exti/reg_write_read", test_reg_write_read);
+    qtest_add_func("stm32l4x5/exti/no_software_interrupt",
+                   test_no_software_interrupt);
+    qtest_add_func("stm32l4x5/exti/software_interrupt",
+                   test_software_interrupt);
+    qtest_add_func("stm32l4x5/exti/masked_interrupt", test_masked_interrupt);
+    qtest_add_func("stm32l4x5/exti/interrupt", test_interrupt);
+    qtest_add_func("stm32l4x5/exti/test_edge_selector", test_edge_selector);
+
+    qtest_start("-machine b-l475e-iot01a");
+    ret = g_test_run();
+    qtest_end();
+
+    return ret;
+}
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v6 3/3] tests/qtest: Add STM32L4x5 EXTI QTest testcase
  2024-01-08 18:03 ` [PATCH v6 3/3] tests/qtest: Add STM32L4x5 EXTI QTest testcase Inès Varhol
@ 2024-01-08 21:31   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-01-08 21:31 UTC (permalink / raw)
  To: Inès Varhol, qemu-devel, Thomas Huth
  Cc: Samuel Tardieu, Paolo Bonzini, qemu-arm, Arnaud Minier,
	Alistair Francis, Laurent Vivier, Peter Maydell

Hi Inès,

On 8/1/24 19:03, Inès Varhol wrote:
> Acked-by: Alistair Francis <alistair.francis@wdc.com>
> Signed-off-by: Arnaud Minier <arnaud.minier@telecom-paris.fr>
> Signed-off-by: Inès Varhol <ines.varhol@telecom-paris.fr>
> ---
>   tests/qtest/meson.build           |   5 +
>   tests/qtest/stm32l4x5_exti-test.c | 590 ++++++++++++++++++++++++++++++
>   2 files changed, 595 insertions(+)
>   create mode 100644 tests/qtest/stm32l4x5_exti-test.c


> +static void test_reg_write_read(void)
> +{
> +    /* Test that non-reserved bits in xMR and xTSR can be set and cleared */
> +
> +    exti_writel(EXTI_IMR1, 0xFFFFFFFF);
> +    uint32_t imr1 = exti_readl(EXTI_IMR1);
> +    g_assert_cmpuint(imr1, ==, 0xFFFFFFFF);
> +    exti_writel(EXTI_IMR1, 0x00000000);
> +    imr1 = exti_readl(EXTI_IMR1);
> +    g_assert_cmpuint(imr1, ==, 0x00000000);
> +
> +    exti_writel(EXTI_EMR1, 0xFFFFFFFF);
> +    uint32_t emr1 = exti_readl(EXTI_EMR1);

Per QEMU Coding Style [*]:

   Mixed declarations (interleaving statements and declarations within
   blocks) are generally not allowed; declarations should be at the
   beginning of blocks.

Up to the maintainers to accept as is or request a change. Personally
this looks good enough, so:

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

[*] https://qemu-project.gitlab.io/qemu/devel/style.html#declarations

> +    g_assert_cmpuint(emr1, ==, 0xFFFFFFFF);
> +    exti_writel(EXTI_EMR1, 0x00000000);
> +    emr1 = exti_readl(EXTI_EMR1);
> +    g_assert_cmpuint(emr1, ==, 0x00000000);
[...]


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v6 1/3] hw/misc: Implement STM32L4x5 EXTI
  2024-01-08 18:03 ` [PATCH v6 1/3] hw/misc: Implement " Inès Varhol
@ 2024-01-08 22:56   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-01-08 22:56 UTC (permalink / raw)
  To: Inès Varhol, qemu-devel
  Cc: Samuel Tardieu, Paolo Bonzini, qemu-arm, Arnaud Minier,
	Alistair Francis, Laurent Vivier, Thomas Huth, Peter Maydell

On 8/1/24 19:03, Inès Varhol wrote:
> Although very similar to the STM32F4xx EXTI, STM32L4x5 EXTI generates
> more than 32 event/interrupt requests and thus uses more registers
> than STM32F4xx EXTI which generates 23 event/interrupt requests.
> 
> Acked-by: Alistair Francis <alistair.francis@wdc.com>
> Signed-off-by: Arnaud Minier <arnaud.minier@telecom-paris.fr>
> Signed-off-by: Inès Varhol <ines.varhol@telecom-paris.fr>
> ---
> 
> Should the for loop variables be `unsigned` rather than `int` ?

It depends on the iterated range. Here you iterate over
ARRAY_SIZE(Stm32l4x5ExtiState::irq) which is a size_t type,
which is unsigned. Amusingly we use both similarly:

$ git grep 'for (size_t' | wc -l
       56
$ git grep 'for (unsigned' | wc -l
       59

>   docs/system/arm/b-l475e-iot01a.rst |   5 +-
>   hw/misc/Kconfig                    |   3 +
>   hw/misc/meson.build                |   1 +
>   hw/misc/stm32l4x5_exti.c           | 292 +++++++++++++++++++++++++++++
>   hw/misc/trace-events               |   5 +
>   include/hw/misc/stm32l4x5_exti.h   |  51 +++++
>   6 files changed, 354 insertions(+), 3 deletions(-)
>   create mode 100644 hw/misc/stm32l4x5_exti.c
>   create mode 100644 include/hw/misc/stm32l4x5_exti.h
> 
> diff --git a/docs/system/arm/b-l475e-iot01a.rst b/docs/system/arm/b-l475e-iot01a.rst
> index 2b128e6b84..72f256ace7 100644
> --- a/docs/system/arm/b-l475e-iot01a.rst
> +++ b/docs/system/arm/b-l475e-iot01a.rst
> @@ -12,17 +12,16 @@ USART, I2C, SPI, CAN and USB OTG, as well as a variety of sensors.
>   Supported devices
>   """""""""""""""""
>   
> -Currently, B-L475E-IOT01A machine's implementation is minimal,
> -it only supports the following device:
> +Currently B-L475E-IOT01A machine's only supports the following devices:
>   
>   - Cortex-M4F based STM32L4x5 SoC
> +- STM32L4x5 EXTI (Extended interrupts and events controller)
>   
>   Missing devices
>   """""""""""""""
>   
>   The B-L475E-IOT01A does *not* support the following devices:
>   
> -- Extended interrupts and events controller (EXTI)
>   - Reset and clock control (RCC)
>   - Serial ports (UART)
>   - System configuration controller (SYSCFG)
> diff --git a/hw/misc/Kconfig b/hw/misc/Kconfig
> index cc8a8c1418..3efe3dc2cc 100644
> --- a/hw/misc/Kconfig
> +++ b/hw/misc/Kconfig
> @@ -87,6 +87,9 @@ config STM32F4XX_SYSCFG
>   config STM32F4XX_EXTI
>       bool
>   
> +config STM32L4X5_EXTI
> +    bool
> +
>   config MIPS_ITU
>       bool
>   
> diff --git a/hw/misc/meson.build b/hw/misc/meson.build
> index 36c20d5637..16db6e228d 100644
> --- a/hw/misc/meson.build
> +++ b/hw/misc/meson.build
> @@ -110,6 +110,7 @@ system_ss.add(when: 'CONFIG_XLNX_VERSAL_TRNG', if_true: files(
>   system_ss.add(when: 'CONFIG_STM32F2XX_SYSCFG', if_true: files('stm32f2xx_syscfg.c'))
>   system_ss.add(when: 'CONFIG_STM32F4XX_SYSCFG', if_true: files('stm32f4xx_syscfg.c'))
>   system_ss.add(when: 'CONFIG_STM32F4XX_EXTI', if_true: files('stm32f4xx_exti.c'))
> +system_ss.add(when: 'CONFIG_STM32L4X5_EXTI', if_true: files('stm32l4x5_exti.c'))
>   system_ss.add(when: 'CONFIG_MPS2_FPGAIO', if_true: files('mps2-fpgaio.c'))
>   system_ss.add(when: 'CONFIG_MPS2_SCC', if_true: files('mps2-scc.c'))
>   
> diff --git a/hw/misc/stm32l4x5_exti.c b/hw/misc/stm32l4x5_exti.c
> new file mode 100644
> index 0000000000..aedf1fb370
> --- /dev/null
> +++ b/hw/misc/stm32l4x5_exti.c
> @@ -0,0 +1,292 @@
> +/*
> + * STM32L4x5 EXTI (Extended interrupts and events controller)
> + *
> + * Copyright (c) 2023 Arnaud Minier <arnaud.minier@telecom-paris.fr>
> + * Copyright (c) 2023 Samuel Tardieu <samuel.tardieu@telecom-paris.fr>
> + * Copyright (c) 2023 Inès Varhol <ines.varhol@telecom-paris.fr>
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + * This work is based on the stm32f4xx_exti by Alistair Francis.
> + * Original code is licensed under the MIT License:
> + *
> + * Copyright (c) 2014 Alistair Francis <alistair@alistair23.me>
> + */
> +
> +/*
> + * The reference used is the STMicroElectronics RM0351 Reference manual
> + * for STM32L4x5 and STM32L4x6 advanced Arm ® -based 32-bit MCUs.
> + * https://www.st.com/en/microcontrollers-microprocessors/stm32l4x5/documentation.html
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/log.h"
> +#include "trace.h"
> +#include "hw/irq.h"
> +#include "migration/vmstate.h"
> +#include "hw/misc/stm32l4x5_exti.h"
> +
> +#define EXTI_IMR1   0x00
> +#define EXTI_EMR1   0x04
> +#define EXTI_RTSR1  0x08
> +#define EXTI_FTSR1  0x0C
> +#define EXTI_SWIER1 0x10
> +#define EXTI_PR1    0x14

> +#define EXTI_IMR2   0x20
> +#define EXTI_EMR2   0x24
> +#define EXTI_RTSR2  0x28
> +#define EXTI_FTSR2  0x2C
> +#define EXTI_SWIER2 0x30
> +#define EXTI_PR2    0x34
> +
> +#define EXTI_NUM_GPIO_EVENT_IN_LINES 16

   #define EXTI_MAX_IRQ_PER_BANK 32

> +
> +/* 0b11111111_10000010_00000000_00000000 */
> +#define DIRECT_LINE_MASK1 0xFF820000
> +/* 0b00000000_00000000_00000000_10000111 */
> +#define DIRECT_LINE_MASK2 0x00000087
> +/* 0b11111111_11111111_11111111_00000000 */
> +#define RESERVED_BITS_MASK2 0xFFFFFF00
> +
> +/* 0b00000000_00000000_00000000_01111000 */
> +#define ACTIVABLE_MASK2 (~DIRECT_LINE_MASK2 & ~RESERVED_BITS_MASK2)


You might want to declare:

   #define EXTI_IRQS_BANK0  32
   #define EXTI_IRQS_BANK1  8

   static const unsigned irqs_per_bank[EXTI_NUM_REGISTER] = {
       EXTI_IRQS_BANK0,
       EXTI_IRQS_BANK1,
   };

   static const uint32_t exti_romask[EXTI_NUM_REGISTER] = {
       0xff820000, /* 0b11111111_10000010_00000000_00000000 */
       0x00000087, /* 0b00000000_00000000_00000000_10000111 */
   };

And add descriptive helpers such:

static unsigned regbank_index_by_irq(unsigned irq)
{
     return irq >= EXTI_MAX_IRQ_PER_BANK ? 1 : 0;
}

static unsigned regbank_index_by_addr(hwaddr addr)
{
     return addr >= EXTI_IMR2 ? 1 : 0;
}

static unsigned valid_mask(unsigned bank)
{
     return MAKE_64BIT_MASK(0, irqs_per_bank[bank]);
}

(then see below)

> +static void stm32l4x5_exti_reset_hold(Object *obj)
> +{
> +    Stm32l4x5ExtiState *s = STM32L4X5_EXTI(obj);
> +
> +    s->imr[0] = DIRECT_LINE_MASK1;
> +    s->emr[0] = 0x00000000;
> +    s->rtsr[0] = 0x00000000;
> +    s->ftsr[0] = 0x00000000;
> +    s->swier[0] = 0x00000000;
> +    s->pr[0] = 0x00000000;
> +
> +    s->imr[1] = DIRECT_LINE_MASK2;
> +    s->emr[1] = 0x00000000;
> +    s->rtsr[1] = 0x00000000;
> +    s->ftsr[1] = 0x00000000;
> +    s->swier[1] = 0x00000000;
> +    s->pr[1] = 0x00000000;
> +}

BTW stm32l4x5_exti_reset_hold() can also be unified:

    static void stm32l4x5_exti_reset_hold(Object *obj)
    {
        Stm32l4x5ExtiState *s = STM32L4X5_EXTI(obj);

        for (unsigned bank = 0; bank < EXTI_NUM_REGISTER; bank++) {
            s->imr[bank] = exti_romask[bank];
            s->emr[bank] = 0;
            s->rtsr[bank] = 0;
            s->ftsr[bank] = 0;
            s->swier[bank] = 0;
            s->pr[bank] = 0;
        }
    }

> +static void stm32l4x5_exti_set_irq(void *opaque, int irq, int level)
> +{
> +    Stm32l4x5ExtiState *s = opaque;
> +    const unsigned n = irq >= 32;

Then:

       n = regbank_index_by_irq(irq);

Maybe 'bank' is a better variable name instead of 'n', in particular
in stm32l4x5_exti_read() it makes it clear registers behave identically
for each bank of registers.

> +    const int oirq = irq;
> +
> +    trace_stm32l4x5_exti_set_irq(irq, level);
> +
> +    if (irq >= 32) {
> +        /* Shift the value to enable access in x2 registers. */
> +        irq -= 32;
> +    }

Alternatively:

       irq %= EXTI_MAX_IRQ_PER_BANK;

> +    /* If the interrupt is masked, pr won't be raised */
> +    if (!((1 << irq) & s->imr[n])) {

Alternatively:

        if (!extract32(s->imr[n], irq, 1)) {

> +        return;
> +    }
> +
> +    if (((1 << irq) & s->rtsr[n]) && level) {
> +        /* Rising Edge */
> +        s->pr[n] |= 1 << irq;
> +        qemu_irq_pulse(s->irq[oirq]);
> +    } else if (((1 << irq) & s->ftsr[n]) && !level) {
> +        /* Falling Edge */
> +        s->pr[n] |= 1 << irq;
> +        qemu_irq_pulse(s->irq[oirq]);
> +    }

        else { // Can we get here?

> +}
> +
> +static uint64_t stm32l4x5_exti_read(void *opaque, hwaddr addr,
> +                                    unsigned int size)
> +{
> +    Stm32l4x5ExtiState *s = opaque;
> +    uint32_t r = 0;
> +    const unsigned n = addr >= EXTI_IMR2;

       bank = regbank_index_by_addr(addr);

> +
> +    switch (addr) {
> +    case EXTI_IMR1:
> +    case EXTI_IMR2:
> +        r = s->imr[n];
> +        break;
> +    case EXTI_EMR1:
> +    case EXTI_EMR2:
> +        r = s->emr[n];
> +        break;
> +    case EXTI_RTSR1:
> +    case EXTI_RTSR2:
> +        r = s->rtsr[n];
> +        break;
> +    case EXTI_FTSR1:
> +    case EXTI_FTSR2:
> +        r = s->ftsr[n];
> +        break;
> +    case EXTI_SWIER1:
> +    case EXTI_SWIER2:
> +        r = s->swier[n];
> +        break;
> +    case EXTI_PR1:
> +    case EXTI_PR2:
> +        r = s->pr[n];
> +        break;
> +
> +    default:
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "STM32L4X5_exti_read: Bad offset 0x%x\n", (int)addr);

Here you can avoid the cast by using the HWADDR_PRIx format.

> +        break;
> +    }
> +
> +    trace_stm32l4x5_exti_read(addr, r);
> +
> +    return r;
> +}
> +
> +static void stm32l4x5_exti_write(void *opaque, hwaddr addr,
> +                                 uint64_t val64, unsigned int size)
> +{
> +    Stm32l4x5ExtiState *s = opaque;
> +    const uint32_t value = (uint32_t)val64;

I don't see the benefit of casting to 32-bits, besides we have
".valid.max_access_size = 4" so the caller always pass 32-bits.

> +
> +    trace_stm32l4x5_exti_write(addr, value);
> +
> +    switch (addr) {
> +    case EXTI_IMR1:
> +        s->imr[0] = value;
> +        return;

You can unify banks:

        case EXTI_IMR1:
        case EXTI_IMR2:
            s->imr[bank] = value & valid_mask(bank);
            return;

> +    case EXTI_EMR1:
> +        s->emr[0] = value;
> +        return;

        case EXTI_EMR1:
        case EXTI_EMR2:
            s->emr[bank] = value & valid_mask(bank);
            return;

> +    case EXTI_RTSR1:
> +        s->rtsr[0] = value & ~DIRECT_LINE_MASK1;
> +        return;

        case EXTI_RTSR1:
        case EXTI_RTSR2:
            s->rtsr[bank] = value & ~exti_romask[bank];
            return;

> +    case EXTI_FTSR1:
> +        s->ftsr[0] = value & ~DIRECT_LINE_MASK1;
> +        return;

        case EXTI_FTSR1:
        case EXTI_FTSR2:
            s->ftsr[bank] = value & ~exti_romask[bank];
            return;

> +    case EXTI_SWIER1: {
> +        const uint32_t set1 = value & ~DIRECT_LINE_MASK1;
> +        const uint32_t pend1 = set1 & ~s->swier[0] & s->imr[0] & ~s->pr[0];
> +        s->swier[0] = set1;
> +        s->pr[0] |= pend1;
> +        for (int i = 0; i < 32; i++) {
> +            if (pend1 & (1 << i)) {
> +                qemu_irq_pulse(s->irq[i]);
> +            }
> +        }
> +        return;

      case EXTI_SWIER1:
      case EXTI_SWIER2:
           set = value & ~exti_romask[bank];
           pend = set & ~s->swier[bank] & s->imr[bank] & ~s->pr[bank];
           s->swier[bank] = set;
           s->pr[bank] |= pend;
           for (unsigned i = 0; i < irqs_per_bank[bank]; i++) {
               if (pend & (1 << i)) { // or extract32()
                   qemu_irq_pulse(s->irq[i]);
               }
           }
           return;

> +    }
> +    case EXTI_PR1: {
> +        const uint32_t cleared1 = s->pr[0] & value & ~DIRECT_LINE_MASK1;
> +        /* This bit is cleared by writing a 1 to it */
> +        s->pr[0] &= ~cleared1;
> +        /* Software triggered interrupts are cleared as well */
> +        s->swier[0] &= ~cleared1;
> +        return;

Ditto.

> +    }
> +    case EXTI_IMR2:
> +        s->imr[1] = value & ~RESERVED_BITS_MASK2;
> +        return;
> +    case EXTI_EMR2:
> +        s->emr[1] = value & ~RESERVED_BITS_MASK2;
> +        return;
> +    case EXTI_RTSR2:
> +        s->rtsr[1] = value & ACTIVABLE_MASK2;
> +        return;
> +    case EXTI_FTSR2:
> +        s->ftsr[1] = value & ACTIVABLE_MASK2;
> +        return;
> +    case EXTI_SWIER2: {
> +        const uint32_t set2 = value & ACTIVABLE_MASK2;
> +        const uint32_t pend2 = set2 & ~s->swier[1] & s->imr[1] & ~s->pr[1];
> +        s->swier[1] = set2;
> +        s->pr[1] |= pend2;
> +        for (int i = 0; i < 8; i++) {
> +            if (pend2 & (1 << i)) {
> +                qemu_irq_pulse(s->irq[32 + i]);
> +            }
> +        }
> +        return;
> +    }
> +    case EXTI_PR2: {
> +        const uint32_t cleared = s->pr[1] & value & ACTIVABLE_MASK2;
> +        /* This bit is cleared by writing a 1 to it */
> +        s->pr[1] &= ~cleared;
> +        /* Software triggered interrupts are cleared as well */
> +        s->swier[1] &= ~cleared;
> +        return;
> +    }
> +    default:
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "STM32L4X5_exti_write: Bad offset 0x%x\n", (int)addr);
> +    }
> +}
> +
> +static const MemoryRegionOps stm32l4x5_exti_ops = {
> +    .read = stm32l4x5_exti_read,
> +    .write = stm32l4x5_exti_write,
> +    .endianness = DEVICE_NATIVE_ENDIAN,
> +    .impl.min_access_size = 4,
> +    .impl.max_access_size = 4,
> +    .impl.unaligned = false,
> +    .valid.min_access_size = 4,
> +    .valid.max_access_size = 4,
> +    .valid.unaligned = false,
> +};

Code totally untested =)

Regards,

Phil.


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2024-01-08 22:57 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-01-08 18:03 [PATCH v6 0/3] Add device STM32L4x5 EXTI Inès Varhol
2024-01-08 18:03 ` [PATCH v6 1/3] hw/misc: Implement " Inès Varhol
2024-01-08 22:56   ` Philippe Mathieu-Daudé
2024-01-08 18:03 ` [PATCH v6 2/3] hw/arm: Connect STM32L4x5 EXTI to STM32L4x5 SoC Inès Varhol
2024-01-08 18:03 ` [PATCH v6 3/3] tests/qtest: Add STM32L4x5 EXTI QTest testcase Inès Varhol
2024-01-08 21:31   ` Philippe Mathieu-Daudé

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).