* [PATCH v5 0/3] Add device STM32L4x5 EXTI
@ 2023-12-28 16:19 Inès Varhol
2023-12-28 16:19 ` [PATCH v5 1/3] hw/misc: Implement " Inès Varhol
` (2 more replies)
0 siblings, 3 replies; 23+ messages in thread
From: Inès Varhol @ 2023-12-28 16:19 UTC (permalink / raw)
To: qemu-devel
Cc: Alistair Francis, Arnaud Minier, Philippe Mathieu-Daudé,
Peter Maydell, Paolo Bonzini, Inès Varhol, Laurent Vivier,
Thomas Huth, qemu-arm
ERRATUM : I mistakenly sent an incorrect version v4 of this patch.
This version v5 rectifies the error and replaces the erroneous v4.
All my apologies.
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: 20231221213838.54944-1-ines.varhol@telecom-paris.fr
([PATCH v4 0/2] Add minimal support for the B-L475E-IOT01A board)
Inès Varhol (3):
hw/misc: Implement STM32L4x5 EXTI
tests/qtest: Add STM32L4x5 EXTI QTest testcase
hw/arm: Connect STM32L4x5 EXTI to STM32L4x5 SoC
docs/system/arm/b-l475e-iot01a.rst | 5 +-
hw/arm/Kconfig | 1 +
hw/arm/stm32l4x5_soc.c | 56 ++-
hw/misc/Kconfig | 3 +
hw/misc/meson.build | 1 +
hw/misc/stm32l4x5_exti.c | 288 ++++++++++++++
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 | 596 +++++++++++++++++++++++++++++
11 files changed, 1009 insertions(+), 5 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] 23+ messages in thread
* [PATCH v5 1/3] hw/misc: Implement STM32L4x5 EXTI
2023-12-28 16:19 [PATCH v5 0/3] Add device STM32L4x5 EXTI Inès Varhol
@ 2023-12-28 16:19 ` Inès Varhol
2024-01-04 13:14 ` Philippe Mathieu-Daudé
2023-12-28 16:19 ` [PATCH v5 2/3] tests/qtest: Add STM32L4x5 EXTI QTest testcase Inès Varhol
2023-12-28 16:19 ` [PATCH v5 3/3] hw/arm: Connect STM32L4x5 EXTI to STM32L4x5 SoC Inès Varhol
2 siblings, 1 reply; 23+ messages in thread
From: Inès Varhol @ 2023-12-28 16:19 UTC (permalink / raw)
To: qemu-devel
Cc: Alistair Francis, Arnaud Minier, Philippe Mathieu-Daudé,
Peter Maydell, Paolo Bonzini, Inès Varhol, Laurent Vivier,
Thomas Huth, qemu-arm
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.
Signed-off-by: Arnaud Minier <arnaud.minier@telecom-paris.fr>
Signed-off-by: Inès Varhol <ines.varhol@telecom-paris.fr>
---
docs/system/arm/b-l475e-iot01a.rst | 5 +-
hw/misc/Kconfig | 3 +
hw/misc/meson.build | 1 +
hw/misc/stm32l4x5_exti.c | 288 +++++++++++++++++++++++++++++
hw/misc/trace-events | 5 +
include/hw/misc/stm32l4x5_exti.h | 51 +++++
6 files changed, 350 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..81c901d3e5
--- /dev/null
+++ b/hw/misc/stm32l4x5_exti.c
@@ -0,0 +1,288 @@
+/*
+ * 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 05ff692441..2f01c62c0e 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] 23+ messages in thread
* [PATCH v5 2/3] tests/qtest: Add STM32L4x5 EXTI QTest testcase
2023-12-28 16:19 [PATCH v5 0/3] Add device STM32L4x5 EXTI Inès Varhol
2023-12-28 16:19 ` [PATCH v5 1/3] hw/misc: Implement " Inès Varhol
@ 2023-12-28 16:19 ` Inès Varhol
2024-01-04 3:39 ` Alistair Francis
` (2 more replies)
2023-12-28 16:19 ` [PATCH v5 3/3] hw/arm: Connect STM32L4x5 EXTI to STM32L4x5 SoC Inès Varhol
2 siblings, 3 replies; 23+ messages in thread
From: Inès Varhol @ 2023-12-28 16:19 UTC (permalink / raw)
To: qemu-devel
Cc: Alistair Francis, Arnaud Minier, Philippe Mathieu-Daudé,
Peter Maydell, Paolo Bonzini, Inès Varhol, Laurent Vivier,
Thomas Huth, qemu-arm
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 | 596 ++++++++++++++++++++++++++++++
2 files changed, 601 insertions(+)
create mode 100644 tests/qtest/stm32l4x5_exti-test.c
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 47dabf91d0..d5126f4d86 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..60c8297246
--- /dev/null
+++ b/tests/qtest/stm32l4x5_exti-test.c
@@ -0,0 +1,596 @@
+/*
+ * 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 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 */
+ qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
+ NULL, 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 */
+ qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
+ NULL, 0, 0);
+
+ uint32_t pr1 = exti_readl(EXTI_PR1);
+ g_assert_cmpuint(pr1, ==, 0x00000000);
+ g_assert_false(check_nvic_pending(EXTI0_IRQ));
+
+ qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
+ NULL, 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 */
+ qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
+ NULL, 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 */
+ qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
+ NULL, 0, 1);
+
+ pr1 = exti_readl(EXTI_PR1);
+ g_assert_cmpuint(pr1, ==, 0x00000000);
+ g_assert_false(check_nvic_pending(EXTI0_IRQ));
+
+ qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
+ NULL, 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, 0x00000000);
+
+ /* Test that an irq is raised on rising and falling edge */
+ qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
+ NULL, 0, 1);
+
+ pr1 = exti_readl(EXTI_PR1);
+ g_assert_cmpuint(pr1, ==, 0x00000001);
+ g_assert_true(check_nvic_pending(EXTI0_IRQ));
+
+ qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
+ NULL, 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 */
+ qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
+ NULL, 0, 1);
+
+ pr1 = exti_readl(EXTI_PR1);
+ g_assert_cmpuint(pr1, ==, 0x00000000);
+ g_assert_false(check_nvic_pending(EXTI0_IRQ));
+
+ qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
+ NULL, 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 */
+ qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
+ NULL, 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 */
+ qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
+ NULL, 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] 23+ messages in thread
* [PATCH v5 3/3] hw/arm: Connect STM32L4x5 EXTI to STM32L4x5 SoC
2023-12-28 16:19 [PATCH v5 0/3] Add device STM32L4x5 EXTI Inès Varhol
2023-12-28 16:19 ` [PATCH v5 1/3] hw/misc: Implement " Inès Varhol
2023-12-28 16:19 ` [PATCH v5 2/3] tests/qtest: Add STM32L4x5 EXTI QTest testcase Inès Varhol
@ 2023-12-28 16:19 ` Inès Varhol
2024-01-04 3:42 ` Alistair Francis
2024-01-04 13:17 ` Philippe Mathieu-Daudé
2 siblings, 2 replies; 23+ messages in thread
From: Inès Varhol @ 2023-12-28 16:19 UTC (permalink / raw)
To: qemu-devel
Cc: Alistair Francis, Arnaud Minier, Philippe Mathieu-Daudé,
Peter Maydell, Paolo Bonzini, Inès Varhol, Laurent Vivier,
Thomas Huth, qemu-arm
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 | 56 ++++++++++++++++++++++++++++++++--
include/hw/arm/stm32l4x5_soc.h | 3 ++
3 files changed, 58 insertions(+), 2 deletions(-)
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 7520dc5cc0..9c9d5bb541 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -458,6 +458,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 7513db0d6a..08b8a4c2ed 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);
}
@@ -50,7 +91,9 @@ static void stm32l4x5_soc_realize(DeviceState *dev_soc, Error **errp)
Stm32l4x5SocState *s = STM32L4X5_SOC(dev_soc);
const Stm32l4x5SocClass *sc = STM32L4X5_SOC_GET_CLASS(dev_soc);
MemoryRegion *system_memory = get_system_memory();
- DeviceState *armv7m;
+ DeviceState *dev, *armv7m;
+ SysBusDevice *busdev;
+ int i;
/*
* We use s->refclk internally and only define it with qdev_init_clock_in()
@@ -115,6 +158,16 @@ static void stm32l4x5_soc_realize(DeviceState *dev_soc, Error **errp)
return;
}
+ dev = DEVICE(&s->exti);
+ if (!sysbus_realize(SYS_BUS_DEVICE(&s->exti), errp)) {
+ return;
+ }
+ busdev = SYS_BUS_DEVICE(dev);
+ sysbus_mmio_map(busdev, 0, EXTI_ADDR);
+ for (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);
@@ -155,7 +208,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 dce13a023d..6cba566a31 100644
--- a/include/hw/arm/stm32l4x5_soc.h
+++ b/include/hw/arm/stm32l4x5_soc.h
@@ -28,6 +28,7 @@
#include "qemu/units.h"
#include "hw/qdev-core.h"
#include "hw/arm/armv7m.h"
+#include "hw/misc/stm32l4x5_exti.h"
#include "qom/object.h"
#define TYPE_STM32L4X5_SOC "stm32l4x5-soc"
@@ -41,6 +42,8 @@ struct Stm32l4x5SocState {
ARMv7MState armv7m;
+ Stm32l4x5ExtiState exti;
+
MemoryRegion sram1;
MemoryRegion sram2;
MemoryRegion flash;
--
2.43.0
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v5 2/3] tests/qtest: Add STM32L4x5 EXTI QTest testcase
2023-12-28 16:19 ` [PATCH v5 2/3] tests/qtest: Add STM32L4x5 EXTI QTest testcase Inès Varhol
@ 2024-01-04 3:39 ` Alistair Francis
2024-01-04 13:05 ` Philippe Mathieu-Daudé
2024-01-04 13:33 ` Philippe Mathieu-Daudé
2 siblings, 0 replies; 23+ messages in thread
From: Alistair Francis @ 2024-01-04 3:39 UTC (permalink / raw)
To: Inès Varhol
Cc: qemu-devel, Alistair Francis, Arnaud Minier,
Philippe Mathieu-Daudé, Peter Maydell, Paolo Bonzini,
Laurent Vivier, Thomas Huth, qemu-arm
On Fri, Dec 29, 2023 at 3:34 AM Inès Varhol
<ines.varhol@telecom-paris.fr> wrote:
>
> Signed-off-by: Arnaud Minier <arnaud.minier@telecom-paris.fr>
> Signed-off-by: Inès Varhol <ines.varhol@telecom-paris.fr>
Acked-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> tests/qtest/meson.build | 5 +
> tests/qtest/stm32l4x5_exti-test.c | 596 ++++++++++++++++++++++++++++++
> 2 files changed, 601 insertions(+)
> create mode 100644 tests/qtest/stm32l4x5_exti-test.c
>
> diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
> index 47dabf91d0..d5126f4d86 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..60c8297246
> --- /dev/null
> +++ b/tests/qtest/stm32l4x5_exti-test.c
> @@ -0,0 +1,596 @@
> +/*
> + * 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 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 */
> + qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
> + NULL, 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 */
> + qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
> + NULL, 0, 0);
> +
> + uint32_t pr1 = exti_readl(EXTI_PR1);
> + g_assert_cmpuint(pr1, ==, 0x00000000);
> + g_assert_false(check_nvic_pending(EXTI0_IRQ));
> +
> + qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
> + NULL, 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 */
> + qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
> + NULL, 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 */
> + qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
> + NULL, 0, 1);
> +
> + pr1 = exti_readl(EXTI_PR1);
> + g_assert_cmpuint(pr1, ==, 0x00000000);
> + g_assert_false(check_nvic_pending(EXTI0_IRQ));
> +
> + qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
> + NULL, 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, 0x00000000);
> +
> + /* Test that an irq is raised on rising and falling edge */
> + qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
> + NULL, 0, 1);
> +
> + pr1 = exti_readl(EXTI_PR1);
> + g_assert_cmpuint(pr1, ==, 0x00000001);
> + g_assert_true(check_nvic_pending(EXTI0_IRQ));
> +
> + qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
> + NULL, 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 */
> + qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
> + NULL, 0, 1);
> +
> + pr1 = exti_readl(EXTI_PR1);
> + g_assert_cmpuint(pr1, ==, 0x00000000);
> + g_assert_false(check_nvic_pending(EXTI0_IRQ));
> +
> + qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
> + NULL, 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 */
> + qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
> + NULL, 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 */
> + qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
> + NULL, 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 [flat|nested] 23+ messages in thread
* Re: [PATCH v5 3/3] hw/arm: Connect STM32L4x5 EXTI to STM32L4x5 SoC
2023-12-28 16:19 ` [PATCH v5 3/3] hw/arm: Connect STM32L4x5 EXTI to STM32L4x5 SoC Inès Varhol
@ 2024-01-04 3:42 ` Alistair Francis
2024-01-04 13:17 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 23+ messages in thread
From: Alistair Francis @ 2024-01-04 3:42 UTC (permalink / raw)
To: Inès Varhol
Cc: qemu-devel, Alistair Francis, Arnaud Minier,
Philippe Mathieu-Daudé, Peter Maydell, Paolo Bonzini,
Laurent Vivier, Thomas Huth, qemu-arm
On Fri, Dec 29, 2023 at 2:21 AM Inès Varhol
<ines.varhol@telecom-paris.fr> wrote:
>
> Signed-off-by: Arnaud Minier <arnaud.minier@telecom-paris.fr>
> Signed-off-by: Inès Varhol <ines.varhol@telecom-paris.fr>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> hw/arm/Kconfig | 1 +
> hw/arm/stm32l4x5_soc.c | 56 ++++++++++++++++++++++++++++++++--
> include/hw/arm/stm32l4x5_soc.h | 3 ++
> 3 files changed, 58 insertions(+), 2 deletions(-)
>
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index 7520dc5cc0..9c9d5bb541 100644
> --- a/hw/arm/Kconfig
> +++ b/hw/arm/Kconfig
> @@ -458,6 +458,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 7513db0d6a..08b8a4c2ed 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);
> }
> @@ -50,7 +91,9 @@ static void stm32l4x5_soc_realize(DeviceState *dev_soc, Error **errp)
> Stm32l4x5SocState *s = STM32L4X5_SOC(dev_soc);
> const Stm32l4x5SocClass *sc = STM32L4X5_SOC_GET_CLASS(dev_soc);
> MemoryRegion *system_memory = get_system_memory();
> - DeviceState *armv7m;
> + DeviceState *dev, *armv7m;
> + SysBusDevice *busdev;
> + int i;
>
> /*
> * We use s->refclk internally and only define it with qdev_init_clock_in()
> @@ -115,6 +158,16 @@ static void stm32l4x5_soc_realize(DeviceState *dev_soc, Error **errp)
> return;
> }
>
> + dev = DEVICE(&s->exti);
> + if (!sysbus_realize(SYS_BUS_DEVICE(&s->exti), errp)) {
> + return;
> + }
> + busdev = SYS_BUS_DEVICE(dev);
> + sysbus_mmio_map(busdev, 0, EXTI_ADDR);
> + for (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);
> @@ -155,7 +208,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 dce13a023d..6cba566a31 100644
> --- a/include/hw/arm/stm32l4x5_soc.h
> +++ b/include/hw/arm/stm32l4x5_soc.h
> @@ -28,6 +28,7 @@
> #include "qemu/units.h"
> #include "hw/qdev-core.h"
> #include "hw/arm/armv7m.h"
> +#include "hw/misc/stm32l4x5_exti.h"
> #include "qom/object.h"
>
> #define TYPE_STM32L4X5_SOC "stm32l4x5-soc"
> @@ -41,6 +42,8 @@ struct Stm32l4x5SocState {
>
> ARMv7MState armv7m;
>
> + Stm32l4x5ExtiState exti;
> +
> MemoryRegion sram1;
> MemoryRegion sram2;
> MemoryRegion flash;
> --
> 2.43.0
>
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v5 2/3] tests/qtest: Add STM32L4x5 EXTI QTest testcase
2023-12-28 16:19 ` [PATCH v5 2/3] tests/qtest: Add STM32L4x5 EXTI QTest testcase Inès Varhol
2024-01-04 3:39 ` Alistair Francis
@ 2024-01-04 13:05 ` Philippe Mathieu-Daudé
2024-01-04 13:37 ` inesvarhol
2024-01-04 13:33 ` Philippe Mathieu-Daudé
2 siblings, 1 reply; 23+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-01-04 13:05 UTC (permalink / raw)
To: Inès Varhol, qemu-devel, Markus Armbruster
Cc: Alistair Francis, Arnaud Minier, Peter Maydell, Paolo Bonzini,
Laurent Vivier, Thomas Huth, qemu-arm
+Markus for QOM tree
On 28/12/23 17:19, Inès Varhol wrote:
> 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 | 596 ++++++++++++++++++++++++++++++
> 2 files changed, 601 insertions(+)
> create mode 100644 tests/qtest/stm32l4x5_exti-test.c
>
> diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
> index 47dabf91d0..d5126f4d86 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..60c8297246
> --- /dev/null
> +++ b/tests/qtest/stm32l4x5_exti-test.c
> @@ -0,0 +1,596 @@
> +/*
> + * 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 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 */
> + qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
Markus, this qtest use seems to expect some stability in QOM path...
Inès, Arnaud, having the SoC unattached is dubious, it belongs to
the machine.
(qemu) info qom-tree
/machine (b-l475e-iot01a-machine)
/SYSCLK (clock)
/peripheral (container)
/peripheral-anon (container)
/unattached (container)
/device[0] (stm32l4x5xg-soc)
Eh I don't see the 'exti' here...
Indeed the test fails:
17/35 qemu:qtest+qtest-arm / qtest-arm/test-arm-mptimer
OK 0.44s 61 subtests passed
▶ 18/35 /arm/stm32l4x5/exti/reg_write_read
FAIL
▶ 18/35 /arm/stm32l4x5/exti/no_software_interrupt
FAIL
▶ 18/35 /arm/stm32l4x5/exti/software_interrupt
FAIL
▶ 18/35 /arm/stm32l4x5/exti/masked_interrupt
FAIL
▶ 18/35 /arm/stm32l4x5/exti/interrupt
FAIL
▶ 18/35 /arm/stm32l4x5/exti/test_edge_selector
FAIL
Listing only the last 100 lines from a long log.
**
ERROR:../../tests/qtest/stm32l4x5_exti-test.c:102:test_reg_write_read:
code should not be reached
**
ERROR:../../tests/qtest/stm32l4x5_exti-test.c:109:test_reg_write_read:
assertion failed (rtsr2 == 0x00000078): (0 == 120)
**
ERROR:../../tests/qtest/stm32l4x5_exti-test.c:109:test_reg_write_read:
code should not be reached
**
ERROR:../../tests/qtest/stm32l4x5_exti-test.c:116:test_reg_write_read:
assertion failed (ftsr2 == 0x00000078): (0 == 120)
**
ERROR:../../tests/qtest/stm32l4x5_exti-test.c:116:test_reg_write_read:
code should not be reached
**
ERROR:../../tests/qtest/stm32l4x5_exti-test.c:421:test_no_software_interrupt:
assertion failed (swier1 == 0x00000001): (0 == 1)
...
You should re-order patches 2 <-> 3.
> + NULL, 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 */
> + qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
> + NULL, 0, 0);
> +
> + uint32_t pr1 = exti_readl(EXTI_PR1);
> + g_assert_cmpuint(pr1, ==, 0x00000000);
> + g_assert_false(check_nvic_pending(EXTI0_IRQ));
> +
> + qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
> + NULL, 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 */
> + qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
> + NULL, 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 */
> + qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
> + NULL, 0, 1);
> +
> + pr1 = exti_readl(EXTI_PR1);
> + g_assert_cmpuint(pr1, ==, 0x00000000);
> + g_assert_false(check_nvic_pending(EXTI0_IRQ));
> +
> + qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
> + NULL, 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, 0x00000000);
> +
> + /* Test that an irq is raised on rising and falling edge */
> + qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
> + NULL, 0, 1);
> +
> + pr1 = exti_readl(EXTI_PR1);
> + g_assert_cmpuint(pr1, ==, 0x00000001);
> + g_assert_true(check_nvic_pending(EXTI0_IRQ));
> +
> + qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
> + NULL, 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 */
> + qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
> + NULL, 0, 1);
> +
> + pr1 = exti_readl(EXTI_PR1);
> + g_assert_cmpuint(pr1, ==, 0x00000000);
> + g_assert_false(check_nvic_pending(EXTI0_IRQ));
> +
> + qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
> + NULL, 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 */
> + qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
> + NULL, 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 */
> + qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
> + NULL, 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;
> +}
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v5 1/3] hw/misc: Implement STM32L4x5 EXTI
2023-12-28 16:19 ` [PATCH v5 1/3] hw/misc: Implement " Inès Varhol
@ 2024-01-04 13:14 ` Philippe Mathieu-Daudé
2024-01-04 13:23 ` Samuel Tardieu
0 siblings, 1 reply; 23+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-01-04 13:14 UTC (permalink / raw)
To: Inès Varhol, qemu-devel
Cc: Alistair Francis, Arnaud Minier, Peter Maydell, Paolo Bonzini,
Laurent Vivier, Thomas Huth, qemu-arm
On 28/12/23 17:19, 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.
>
> Signed-off-by: Arnaud Minier <arnaud.minier@telecom-paris.fr>
> Signed-off-by: Inès Varhol <ines.varhol@telecom-paris.fr>
> ---
> docs/system/arm/b-l475e-iot01a.rst | 5 +-
> hw/misc/Kconfig | 3 +
> hw/misc/meson.build | 1 +
> hw/misc/stm32l4x5_exti.c | 288 +++++++++++++++++++++++++++++
> hw/misc/trace-events | 5 +
> include/hw/misc/stm32l4x5_exti.h | 51 +++++
> 6 files changed, 350 insertions(+), 3 deletions(-)
> create mode 100644 hw/misc/stm32l4x5_exti.c
> create mode 100644 include/hw/misc/stm32l4x5_exti.h
> +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);
> + }
> +}
This doesn't build:
../../hw/misc/stm32l4x5_exti.c:172:9: error: expected expression
const uint32_t set1 = value & ~DIRECT_LINE_MASK1;
^
../../hw/misc/stm32l4x5_exti.c:173:32: error: use of undeclared
identifier 'set1'
const uint32_t pend1 = set1 & ~s->swier[0] & s->imr[0] & ~s->pr[0];
^
../../hw/misc/stm32l4x5_exti.c:174:23: error: use of undeclared
identifier 'set1'
s->swier[0] = set1;
^
../../hw/misc/stm32l4x5_exti.c:183:9: error: expected expression
const uint32_t cleared1 = s->pr[0] & value & ~DIRECT_LINE_MASK1;
^
../../hw/misc/stm32l4x5_exti.c:185:22: error: use of undeclared
identifier 'cleared1'
s->pr[0] &= ~cleared1;
^
../../hw/misc/stm32l4x5_exti.c:187:25: error: use of undeclared
identifier 'cleared1'
s->swier[0] &= ~cleared1;
^
../../hw/misc/stm32l4x5_exti.c:202:9: error: expected expression
const uint32_t set2 = value & ACTIVABLE_MASK2;
^
../../hw/misc/stm32l4x5_exti.c:203:32: error: use of undeclared
identifier 'set2'
const uint32_t pend2 = set2 & ~s->swier[1] & s->imr[1] & ~s->pr[1];
^
../../hw/misc/stm32l4x5_exti.c:204:23: error: use of undeclared
identifier 'set2'
s->swier[1] = set2;
^
../../hw/misc/stm32l4x5_exti.c:213:9: error: expected expression
const uint32_t cleared = s->pr[1] & value & ACTIVABLE_MASK2;
^
../../hw/misc/stm32l4x5_exti.c:215:22: error: use of undeclared
identifier 'cleared'
s->pr[1] &= ~cleared;
^
../../hw/misc/stm32l4x5_exti.c:217:25: error: use of undeclared
identifier 'cleared'
s->swier[1] &= ~cleared;
^
14 errors generated.
I could build using:
-- >8 --
diff --git a/hw/misc/stm32l4x5_exti.c b/hw/misc/stm32l4x5_exti.c
index 81c901d3e5..aedf1fb370 100644
--- a/hw/misc/stm32l4x5_exti.c
+++ b/hw/misc/stm32l4x5_exti.c
@@ -170,3 +170,3 @@ static void stm32l4x5_exti_write(void *opaque,
hwaddr addr,
return;
- case EXTI_SWIER1:
+ case EXTI_SWIER1: {
const uint32_t set1 = value & ~DIRECT_LINE_MASK1;
@@ -181,3 +181,4 @@ static void stm32l4x5_exti_write(void *opaque,
hwaddr addr,
return;
- case EXTI_PR1:
+ }
+ case EXTI_PR1: {
const uint32_t cleared1 = s->pr[0] & value & ~DIRECT_LINE_MASK1;
@@ -188,2 +189,3 @@ static void stm32l4x5_exti_write(void *opaque,
hwaddr addr,
return;
+ }
case EXTI_IMR2:
@@ -200,3 +202,3 @@ static void stm32l4x5_exti_write(void *opaque,
hwaddr addr,
return;
- case EXTI_SWIER2:
+ case EXTI_SWIER2: {
const uint32_t set2 = value & ACTIVABLE_MASK2;
@@ -211,3 +213,4 @@ static void stm32l4x5_exti_write(void *opaque,
hwaddr addr,
return;
- case EXTI_PR2:
+ }
+ case EXTI_PR2: {
const uint32_t cleared = s->pr[1] & value & ACTIVABLE_MASK2;
@@ -218,2 +221,3 @@ static void stm32l4x5_exti_write(void *opaque,
hwaddr addr,
return;
+ }
default:
---
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v5 3/3] hw/arm: Connect STM32L4x5 EXTI to STM32L4x5 SoC
2023-12-28 16:19 ` [PATCH v5 3/3] hw/arm: Connect STM32L4x5 EXTI to STM32L4x5 SoC Inès Varhol
2024-01-04 3:42 ` Alistair Francis
@ 2024-01-04 13:17 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 23+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-01-04 13:17 UTC (permalink / raw)
To: Inès Varhol, qemu-devel
Cc: Alistair Francis, Arnaud Minier, Peter Maydell, Paolo Bonzini,
Laurent Vivier, Thomas Huth, qemu-arm
On 28/12/23 17:19, Inès Varhol wrote:
> 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 | 56 ++++++++++++++++++++++++++++++++--
> include/hw/arm/stm32l4x5_soc.h | 3 ++
> 3 files changed, 58 insertions(+), 2 deletions(-)
> 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);
> }
> @@ -50,7 +91,9 @@ static void stm32l4x5_soc_realize(DeviceState *dev_soc, Error **errp)
> Stm32l4x5SocState *s = STM32L4X5_SOC(dev_soc);
> const Stm32l4x5SocClass *sc = STM32L4X5_SOC_GET_CLASS(dev_soc);
> MemoryRegion *system_memory = get_system_memory();
> - DeviceState *armv7m;
> + DeviceState *dev, *armv7m;
> + SysBusDevice *busdev;
> + int i;
>
> /*
> * We use s->refclk internally and only define it with qdev_init_clock_in()
> @@ -115,6 +158,16 @@ static void stm32l4x5_soc_realize(DeviceState *dev_soc, Error **errp)
> return;
> }
>
> + dev = DEVICE(&s->exti);
'dev' isn't used, you can drop it and directly use:
busdev = SYS_BUS_DEVICE(&s->exti);
> + if (!sysbus_realize(SYS_BUS_DEVICE(&s->exti), errp)) {
Here you can use 'busdev' directly.
> + return;
> + }
> + busdev = SYS_BUS_DEVICE(dev);
> + sysbus_mmio_map(busdev, 0, EXTI_ADDR);
> + for (i = 0; i < NUM_EXTI_IRQ; i++) {
Preferably reduce 'i' scope:
for (unsigned i = 0; ...) {
> + sysbus_connect_irq(busdev, i, qdev_get_gpio_in(armv7m, exti_irq[i]));
> + }
Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
With the comments addressed:
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v5 1/3] hw/misc: Implement STM32L4x5 EXTI
2024-01-04 13:14 ` Philippe Mathieu-Daudé
@ 2024-01-04 13:23 ` Samuel Tardieu
2024-01-04 13:40 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 23+ messages in thread
From: Samuel Tardieu @ 2024-01-04 13:23 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Inès Varhol, Alistair Francis, Arnaud Minier, Peter Maydell,
Paolo Bonzini, Laurent Vivier, Thomas Huth, qemu-arm, qemu-devel
Philippe Mathieu-Daudé <philmd@linaro.org> writes:
> This doesn't build:
>
> ../../hw/misc/stm32l4x5_exti.c:172:9: error: expected expression
> const uint32_t set1 = value & ~DIRECT_LINE_MASK1;
> […]
> I could build using:
> - case EXTI_SWIER1:
> + case EXTI_SWIER1: {
Out or curiosity, which C compiler or option do you use for
checking? I have no problem building this using "./configure
--target-list=arm-softmmu" with GCC 12.3.0.
Sam
--
Samuel Tardieu
Télécom Paris - Institut Polytechnique de Paris
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v5 2/3] tests/qtest: Add STM32L4x5 EXTI QTest testcase
2023-12-28 16:19 ` [PATCH v5 2/3] tests/qtest: Add STM32L4x5 EXTI QTest testcase Inès Varhol
2024-01-04 3:39 ` Alistair Francis
2024-01-04 13:05 ` Philippe Mathieu-Daudé
@ 2024-01-04 13:33 ` Philippe Mathieu-Daudé
2024-01-04 13:45 ` inesvarhol
2 siblings, 1 reply; 23+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-01-04 13:33 UTC (permalink / raw)
To: Inès Varhol, qemu-devel
Cc: Alistair Francis, Arnaud Minier, Peter Maydell, Paolo Bonzini,
Laurent Vivier, Thomas Huth, qemu-arm
On 28/12/23 17:19, Inès Varhol wrote:
> 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 | 596 ++++++++++++++++++++++++++++++
> 2 files changed, 601 insertions(+)
> create mode 100644 tests/qtest/stm32l4x5_exti-test.c
Once the SoC parentship fixed in based series, this patch
requires:
-- >8 --
diff --git a/tests/qtest/stm32l4x5_exti-test.c
b/tests/qtest/stm32l4x5_exti-test.c
index 60c8297246..543199cd4d 100644
--- a/tests/qtest/stm32l4x5_exti-test.c
+++ b/tests/qtest/stm32l4x5_exti-test.c
@@ -287,4 +287,3 @@ static void test_edge_selector(void)
/* Configure EXTI line 0 irq on rising edge */
- qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
- NULL, 0, 1);
+ qtest_set_irq_in(global_qtest, "/machine/soc/exti", NULL, 0, 1);
exti_writel(EXTI_IMR1, 0x00000001);
@@ -294,4 +293,3 @@ static void test_edge_selector(void)
/* Test that an irq is raised on rising edge only */
- qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
- NULL, 0, 0);
+ qtest_set_irq_in(global_qtest, "/machine/soc/exti", NULL, 0, 0);
@@ -301,4 +299,3 @@ static void test_edge_selector(void)
- qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
- NULL, 0, 1);
+ qtest_set_irq_in(global_qtest, "/machine/soc/exti", NULL, 0, 1);
@@ -316,4 +313,3 @@ static void test_edge_selector(void)
/* Configure EXTI line 0 irq on falling edge */
- qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
- NULL, 0, 0);
+ qtest_set_irq_in(global_qtest, "/machine/soc/exti", NULL, 0, 0);
exti_writel(EXTI_IMR1, 0x00000001);
@@ -323,4 +319,3 @@ static void test_edge_selector(void)
/* Test that an irq is raised on falling edge only */
- qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
- NULL, 0, 1);
+ qtest_set_irq_in(global_qtest, "/machine/soc/exti", NULL, 0, 1);
@@ -330,4 +325,3 @@ static void test_edge_selector(void)
- qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
- NULL, 0, 0);
+ qtest_set_irq_in(global_qtest, "/machine/soc/exti", NULL, 0, 0);
@@ -350,4 +344,3 @@ static void test_edge_selector(void)
/* Test that an irq is raised on rising and falling edge */
- qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
- NULL, 0, 1);
+ qtest_set_irq_in(global_qtest, "/machine/soc/exti", NULL, 0, 1);
@@ -357,4 +350,3 @@ static void test_edge_selector(void)
- qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
- NULL, 0, 0);
+ qtest_set_irq_in(global_qtest, "/machine/soc/exti", NULL, 0, 0);
@@ -377,4 +369,3 @@ static void test_edge_selector(void)
/* Test that no irq is raised */
- qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
- NULL, 0, 1);
+ qtest_set_irq_in(global_qtest, "/machine/soc/exti", NULL, 0, 1);
@@ -384,4 +375,3 @@ static void test_edge_selector(void)
- qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
- NULL, 0, 0);
+ qtest_set_irq_in(global_qtest, "/machine/soc/exti", NULL, 0, 0);
@@ -500,4 +490,3 @@ static void test_masked_interrupt(void)
/* Simulate rising edge from GPIO line 1 */
- qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
- NULL, 1, 1);
+ qtest_set_irq_in(global_qtest, "/machine/soc/exti", NULL, 1, 1);
@@ -550,4 +539,3 @@ static void test_interrupt(void)
/* Simulate rising edge from GPIO line 1 */
- qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
- NULL, 1, 1);
+ qtest_set_irq_in(global_qtest, "/machine/soc/exti", NULL, 1, 1);
---
Note you could use a helper to ease readability:
static void exti_set_irq(int num, int lvl)
{
qtest_set_irq_in(global_qtest, "/machine/soc/exti", NULL, num, lvl);
}
Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v5 2/3] tests/qtest: Add STM32L4x5 EXTI QTest testcase
2024-01-04 13:05 ` Philippe Mathieu-Daudé
@ 2024-01-04 13:37 ` inesvarhol
2024-01-05 10:13 ` Philippe Mathieu-Daudé
2024-01-05 10:24 ` Daniel P. Berrangé
0 siblings, 2 replies; 23+ messages in thread
From: inesvarhol @ 2024-01-04 13:37 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Inès Varhol, qemu-devel, Markus Armbruster, Alistair Francis,
Arnaud Minier, Peter Maydell, Paolo Bonzini, Laurent Vivier,
Thomas Huth, qemu-arm
Le jeudi 4 janvier 2024 à 14:05, Philippe Mathieu-Daudé <philmd@linaro.org> a écrit :
Hello,
> > +static void test_edge_selector(void)
> > +{
> > + enable_nvic_irq(EXTI0_IRQ);
> > +
> > + / Configure EXTI line 0 irq on rising edge */
> > + qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
>
>
> Markus, this qtest use seems to expect some stability in QOM path...
>
> Inès, Arnaud, having the SoC unattached is dubious, it belongs to
> the machine.
Noted, we will fix that.
Should we be concerned about the "stability in QOM path" ?
>
> (qemu) info qom-tree
> /machine (b-l475e-iot01a-machine)
> /SYSCLK (clock)
> /peripheral (container)
> /peripheral-anon (container)
> /unattached (container)
> /device[0] (stm32l4x5xg-soc)
>
> Eh I don't see the 'exti' here...
>
> Indeed the test fails:
>
> 17/35 qemu:qtest+qtest-arm / qtest-arm/test-arm-mptimer
> OK 0.44s 61 subtests passed
> ▶ 18/35 /arm/stm32l4x5/exti/reg_write_read
> FAIL
> ▶ 18/35 /arm/stm32l4x5/exti/no_software_interrupt
> FAIL
> ▶ 18/35 /arm/stm32l4x5/exti/software_interrupt
> FAIL
> ▶ 18/35 /arm/stm32l4x5/exti/masked_interrupt
> FAIL
> ▶ 18/35 /arm/stm32l4x5/exti/interrupt
> FAIL
> ▶ 18/35 /arm/stm32l4x5/exti/test_edge_selector
> FAIL
> Listing only the last 100 lines from a long log.
Yes indeed, the tests fail in this 2nd commit as the EXTI device isn't connected to the SoC yet (3rd commit).
I forgot to mention it in this in this version :/
Swapping the 2nd and 3rd commmit seems more straightforward to do ?
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v5 1/3] hw/misc: Implement STM32L4x5 EXTI
2024-01-04 13:23 ` Samuel Tardieu
@ 2024-01-04 13:40 ` Philippe Mathieu-Daudé
2024-01-04 13:59 ` Samuel Tardieu
2024-01-04 14:06 ` inesvarhol
0 siblings, 2 replies; 23+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-01-04 13:40 UTC (permalink / raw)
To: Samuel Tardieu
Cc: Inès Varhol, Alistair Francis, Arnaud Minier, Peter Maydell,
Paolo Bonzini, Laurent Vivier, Thomas Huth, qemu-arm, qemu-devel
On 4/1/24 14:23, Samuel Tardieu wrote:
>
> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
>
>> This doesn't build:
>>
>> ../../hw/misc/stm32l4x5_exti.c:172:9: error: expected expression
>> const uint32_t set1 = value & ~DIRECT_LINE_MASK1;
>> […]
>> I could build using:
>> - case EXTI_SWIER1:
>> + case EXTI_SWIER1: {
>
> Out or curiosity, which C compiler or option do you use for checking? I
> have no problem building this using "./configure
> --target-list=arm-softmmu" with GCC 12.3.0.
C compiler for the host machine: clang (clang 15.0.0 "Apple clang
version 15.0.0 (clang-1500.0.40.1)")
C linker for the host machine: clang ld64 1015.7
If you don't have access to similar compiler, you can fork on GitLab
and push a branch to trigger the CI; I expect it to fail.
Regards,
Phil.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v5 2/3] tests/qtest: Add STM32L4x5 EXTI QTest testcase
2024-01-04 13:33 ` Philippe Mathieu-Daudé
@ 2024-01-04 13:45 ` inesvarhol
0 siblings, 0 replies; 23+ messages in thread
From: inesvarhol @ 2024-01-04 13:45 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Inès Varhol, qemu-devel, Alistair Francis, Arnaud Minier,
Peter Maydell, Paolo Bonzini, Laurent Vivier, Thomas Huth,
qemu-arm
Le jeudi 4 janvier 2024 à 14:33, Philippe Mathieu-Daudé <philmd@linaro.org> a écrit :
> On 28/12/23 17:19, Inès Varhol wrote:
>
> > 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 | 596 ++++++++++++++++++++++++++++++
> > 2 files changed, 601 insertions(+)
> > create mode 100644 tests/qtest/stm32l4x5_exti-test.c
>
>
> Once the SoC parentship fixed in based series, this patch
> requires:
>
> -- >8 --
>
> diff --git a/tests/qtest/stm32l4x5_exti-test.c
> b/tests/qtest/stm32l4x5_exti-test.c
> index 60c8297246..543199cd4d 100644
> --- a/tests/qtest/stm32l4x5_exti-test.c
> +++ b/tests/qtest/stm32l4x5_exti-test.c
> @@ -287,4 +287,3 @@ static void test_edge_selector(void)
> /* Configure EXTI line 0 irq on rising edge /
> - qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
> - NULL, 0, 1);
> + qtest_set_irq_in(global_qtest, "/machine/soc/exti", NULL, 0, 1);
> exti_writel(EXTI_IMR1, 0x00000001);
> @@ -294,4 +293,3 @@ static void test_edge_selector(void)
> / Test that an irq is raised on rising edge only /
> - qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
> - NULL, 0, 0);
> + qtest_set_irq_in(global_qtest, "/machine/soc/exti", NULL, 0, 0);
>
> @@ -301,4 +299,3 @@ static void test_edge_selector(void)
>
> - qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
> - NULL, 0, 1);
> + qtest_set_irq_in(global_qtest, "/machine/soc/exti", NULL, 0, 1);
>
> @@ -316,4 +313,3 @@ static void test_edge_selector(void)
> / Configure EXTI line 0 irq on falling edge /
> - qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
> - NULL, 0, 0);
> + qtest_set_irq_in(global_qtest, "/machine/soc/exti", NULL, 0, 0);
> exti_writel(EXTI_IMR1, 0x00000001);
> @@ -323,4 +319,3 @@ static void test_edge_selector(void)
> / Test that an irq is raised on falling edge only /
> - qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
> - NULL, 0, 1);
> + qtest_set_irq_in(global_qtest, "/machine/soc/exti", NULL, 0, 1);
>
> @@ -330,4 +325,3 @@ static void test_edge_selector(void)
>
> - qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
> - NULL, 0, 0);
> + qtest_set_irq_in(global_qtest, "/machine/soc/exti", NULL, 0, 0);
>
> @@ -350,4 +344,3 @@ static void test_edge_selector(void)
> / Test that an irq is raised on rising and falling edge /
> - qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
> - NULL, 0, 1);
> + qtest_set_irq_in(global_qtest, "/machine/soc/exti", NULL, 0, 1);
>
> @@ -357,4 +350,3 @@ static void test_edge_selector(void)
>
> - qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
> - NULL, 0, 0);
> + qtest_set_irq_in(global_qtest, "/machine/soc/exti", NULL, 0, 0);
>
> @@ -377,4 +369,3 @@ static void test_edge_selector(void)
> / Test that no irq is raised /
> - qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
> - NULL, 0, 1);
> + qtest_set_irq_in(global_qtest, "/machine/soc/exti", NULL, 0, 1);
>
> @@ -384,4 +375,3 @@ static void test_edge_selector(void)
>
> - qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
> - NULL, 0, 0);
> + qtest_set_irq_in(global_qtest, "/machine/soc/exti", NULL, 0, 0);
>
> @@ -500,4 +490,3 @@ static void test_masked_interrupt(void)
> / Simulate rising edge from GPIO line 1 /
> - qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
> - NULL, 1, 1);
> + qtest_set_irq_in(global_qtest, "/machine/soc/exti", NULL, 1, 1);
>
> @@ -550,4 +539,3 @@ static void test_interrupt(void)
> / Simulate rising edge from GPIO line 1 */
> - qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
> - NULL, 1, 1);
> + qtest_set_irq_in(global_qtest, "/machine/soc/exti", NULL, 1, 1);
> ---
>
> Note you could use a helper to ease readability:
>
> static void exti_set_irq(int num, int lvl)
> {
> qtest_set_irq_in(global_qtest, "/machine/soc/exti", NULL, num, lvl);
> }
>
> Tested-by: Philippe Mathieu-Daudé philmd@linaro.org
Ok thank you a lot !
And yes, we'll swap the 2nd and 3rd commit as you said.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v5 1/3] hw/misc: Implement STM32L4x5 EXTI
2024-01-04 13:40 ` Philippe Mathieu-Daudé
@ 2024-01-04 13:59 ` Samuel Tardieu
2024-01-04 14:06 ` inesvarhol
1 sibling, 0 replies; 23+ messages in thread
From: Samuel Tardieu @ 2024-01-04 13:59 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Inès Varhol, Alistair Francis, Arnaud Minier, Peter Maydell,
Paolo Bonzini, Laurent Vivier, Thomas Huth, qemu-arm, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 928 bytes --]
On 04/01/2024 14:40, Philippe Mathieu-Daudé wrote:
> On 4/1/24 14:23, Samuel Tardieu wrote:
>>
>> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
>>
>>> This doesn't build:
>>>
>>> ../../hw/misc/stm32l4x5_exti.c:172:9: error: expected expression
>>> const uint32_t set1 = value & ~DIRECT_LINE_MASK1;
>>> […]
>>> I could build using:
>>> - case EXTI_SWIER1:
>>> + case EXTI_SWIER1: {
>>
>> Out or curiosity, which C compiler or option do you use for checking?
>> I have no problem building this using "./configure
>> --target-list=arm-softmmu" with GCC 12.3.0.
>
> C compiler for the host machine: clang (clang 15.0.0 "Apple clang
> version 15.0.0 (clang-1500.0.40.1)")
> C linker for the host machine: clang ld64 1015.7
Indeed, it looks like mixing labels, declarations and statements is a
GCC extension. I've switched to clang and can reproduce the failure. Thanks!
[-- Attachment #2: Type: text/html, Size: 1965 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v5 1/3] hw/misc: Implement STM32L4x5 EXTI
2024-01-04 13:40 ` Philippe Mathieu-Daudé
2024-01-04 13:59 ` Samuel Tardieu
@ 2024-01-04 14:06 ` inesvarhol
1 sibling, 0 replies; 23+ messages in thread
From: inesvarhol @ 2024-01-04 14:06 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Samuel Tardieu, Inès Varhol, Alistair Francis, Arnaud Minier,
Peter Maydell, Paolo Bonzini, Laurent Vivier, Thomas Huth,
qemu-arm, qemu-devel
Le jeudi 4 janvier 2024 à 14:40, Philippe Mathieu-Daudé <philmd@linaro.org> a écrit :
> If you don't have access to similar compiler, you can fork on GitLab
> and push a branch to trigger the CI; I expect it to fail.
Thanks for the tip !
Ines Varhol
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v5 2/3] tests/qtest: Add STM32L4x5 EXTI QTest testcase
2024-01-04 13:37 ` inesvarhol
@ 2024-01-05 10:13 ` Philippe Mathieu-Daudé
2024-01-05 10:19 ` Philippe Mathieu-Daudé
2024-01-07 14:04 ` Mark Cave-Ayland
2024-01-05 10:24 ` Daniel P. Berrangé
1 sibling, 2 replies; 23+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-01-05 10:13 UTC (permalink / raw)
To: inesvarhol, Inès Varhol, Mark Cave-Ayland, Markus Armbruster,
Peter Maydell, Eduardo Habkost, Paolo Bonzini
Cc: qemu-devel, Alistair Francis, Arnaud Minier, Laurent Vivier,
Thomas Huth, qemu-arm, Daniel P. Berrangé
(+Mark & Eduardo)
On 4/1/24 14:37, inesvarhol wrote:
>
> Le jeudi 4 janvier 2024 à 14:05, Philippe Mathieu-Daudé <philmd@linaro.org> a écrit :
>
> Hello,
>
>>> +static void test_edge_selector(void)
>>> +{
>>> + enable_nvic_irq(EXTI0_IRQ);
>>> +
>>> + / Configure EXTI line 0 irq on rising edge */
>>> + qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
>>
>>
>> Markus, this qtest use seems to expect some stability in QOM path...
>>
>> Inès, Arnaud, having the SoC unattached is dubious, it belongs to
>> the machine.
>
> Noted, we will fix that.
> Should we be concerned about the "stability in QOM path" ?
Don't worry about this Inès, I wanted to raise Markus attention on this.
You showed a legit use of stable QOM path, and Markus told me recently
there is no contract for QOM paths (it shouldn't be considered as a
stable API). IIRC Markus explanation, "/unattached" container was
added as a temporary hack to allow migrating QDev objects to QOM (see
around commit da57febfed "qdev: give all devices a canonical path",
11 years ago).
I agree anything under "/unattached" can be expected to be stable
(but we need a community consensus). Then the big question remaining
is "can any qom-path out of /unattached be considered stable?"
Regards,
Phil.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v5 2/3] tests/qtest: Add STM32L4x5 EXTI QTest testcase
2024-01-05 10:13 ` Philippe Mathieu-Daudé
@ 2024-01-05 10:19 ` Philippe Mathieu-Daudé
2024-01-08 16:15 ` Markus Armbruster
2024-01-07 14:04 ` Mark Cave-Ayland
1 sibling, 1 reply; 23+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-01-05 10:19 UTC (permalink / raw)
To: inesvarhol, Inès Varhol, Mark Cave-Ayland, Markus Armbruster,
Peter Maydell, Eduardo Habkost, Paolo Bonzini
Cc: qemu-devel, Alistair Francis, Arnaud Minier, Laurent Vivier,
Thomas Huth, qemu-arm, Daniel P. Berrangé
On 5/1/24 11:13, Philippe Mathieu-Daudé wrote:
> (+Mark & Eduardo)
>
> On 4/1/24 14:37, inesvarhol wrote:
>>
>> Le jeudi 4 janvier 2024 à 14:05, Philippe Mathieu-Daudé
>> <philmd@linaro.org> a écrit :
>>
>> Hello,
>>
>>>> +static void test_edge_selector(void)
>>>> +{
>>>> + enable_nvic_irq(EXTI0_IRQ);
>>>> +
>>>> + / Configure EXTI line 0 irq on rising edge */
>>>> + qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
>>>
>>>
>>> Markus, this qtest use seems to expect some stability in QOM path...
>>>
>>> Inès, Arnaud, having the SoC unattached is dubious, it belongs to
>>> the machine.
>>
>> Noted, we will fix that.
>> Should we be concerned about the "stability in QOM path" ?
>
> Don't worry about this Inès, I wanted to raise Markus attention on this.
>
> You showed a legit use of stable QOM path, and Markus told me recently
> there is no contract for QOM paths (it shouldn't be considered as a
> stable API). IIRC Markus explanation, "/unattached" container was
> added as a temporary hack to allow migrating QDev objects to QOM (see
> around commit da57febfed "qdev: give all devices a canonical path",
> 11 years ago).
Hmm am I getting confused with "/peripheral-anon" (commit 8eb02831af
"dev: add an anonymous peripheral container")?
> I agree anything under "/unattached" can be expected to be stable
> (but we need a community consensus). Then the big question remaining
> is "can any qom-path out of /unattached be considered stable?"
>
> Regards,
>
> Phil.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v5 2/3] tests/qtest: Add STM32L4x5 EXTI QTest testcase
2024-01-04 13:37 ` inesvarhol
2024-01-05 10:13 ` Philippe Mathieu-Daudé
@ 2024-01-05 10:24 ` Daniel P. Berrangé
2024-01-05 22:16 ` Philippe Mathieu-Daudé
1 sibling, 1 reply; 23+ messages in thread
From: Daniel P. Berrangé @ 2024-01-05 10:24 UTC (permalink / raw)
To: inesvarhol
Cc: Philippe Mathieu-Daudé, Inès Varhol, qemu-devel,
Markus Armbruster, Alistair Francis, Arnaud Minier, Peter Maydell,
Paolo Bonzini, Laurent Vivier, Thomas Huth, qemu-arm
On Thu, Jan 04, 2024 at 01:37:22PM +0000, inesvarhol wrote:
>
> Le jeudi 4 janvier 2024 à 14:05, Philippe Mathieu-Daudé <philmd@linaro.org> a écrit :
>
> Hello,
>
> > > +static void test_edge_selector(void)
> > > +{
> > > + enable_nvic_irq(EXTI0_IRQ);
> > > +
> > > + / Configure EXTI line 0 irq on rising edge */
> > > + qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
> >
> >
> > Markus, this qtest use seems to expect some stability in QOM path...
> >
> > Inès, Arnaud, having the SoC unattached is dubious, it belongs to
> > the machine.
>
> Noted, we will fix that.
> Should we be concerned about the "stability in QOM path" ?
QTest is a functional test harness that intentionally has knowledge
about QEMU internals.
IOW, usage of particular QOM path in qtest does *not* imply that
QOM path needs to be stable. If QEMU internals change for whatever
reason, it is expected that QTests may need some updates to match.
QOM path stability only matters if there's a mgmt app facing use
case, which requires the app to have hardcoded knowledge of the
path.
Even a mgmt app can use unstable QOM paths, provided it has a way
to dynamically detect the path to be used, instead of hardcoding
it.
None the less, you may still choose to move it out of /unattached
at your discretion.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v5 2/3] tests/qtest: Add STM32L4x5 EXTI QTest testcase
2024-01-05 10:24 ` Daniel P. Berrangé
@ 2024-01-05 22:16 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 23+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-01-05 22:16 UTC (permalink / raw)
To: Daniel P. Berrangé, inesvarhol
Cc: Inès Varhol, qemu-devel, Markus Armbruster, Alistair Francis,
Arnaud Minier, Peter Maydell, Paolo Bonzini, Laurent Vivier,
Thomas Huth, qemu-arm
On 5/1/24 11:24, Daniel P. Berrangé wrote:
> On Thu, Jan 04, 2024 at 01:37:22PM +0000, inesvarhol wrote:
>>
>> Le jeudi 4 janvier 2024 à 14:05, Philippe Mathieu-Daudé <philmd@linaro.org> a écrit :
>>
>> Hello,
>>
>>>> +static void test_edge_selector(void)
>>>> +{
>>>> + enable_nvic_irq(EXTI0_IRQ);
>>>> +
>>>> + / Configure EXTI line 0 irq on rising edge */
>>>> + qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
>>>
>>>
>>> Markus, this qtest use seems to expect some stability in QOM path...
>>>
>>> Inès, Arnaud, having the SoC unattached is dubious, it belongs to
>>> the machine.
>>
>> Noted, we will fix that.
>> Should we be concerned about the "stability in QOM path" ?
>
> QTest is a functional test harness that intentionally has knowledge
> about QEMU internals.
>
> IOW, usage of particular QOM path in qtest does *not* imply that
> QOM path needs to be stable. If QEMU internals change for whatever
> reason, it is expected that QTests may need some updates to match.
Good point.
> QOM path stability only matters if there's a mgmt app facing use
> case, which requires the app to have hardcoded knowledge of the
> path.
>
> Even a mgmt app can use unstable QOM paths, provided it has a way
> to dynamically detect the path to be used, instead of hardcoding
> it.
I can understand this use to lookup "on which CDROM tray is
inserted the blkdrv named FOO", but to find a component on a
well specified system on chip, this is overkill.
> None the less, you may still choose to move it out of /unattached
> at your discretion.
Yeah we should clean those...
Thanks for clarifying,
Phil.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v5 2/3] tests/qtest: Add STM32L4x5 EXTI QTest testcase
2024-01-05 10:13 ` Philippe Mathieu-Daudé
2024-01-05 10:19 ` Philippe Mathieu-Daudé
@ 2024-01-07 14:04 ` Mark Cave-Ayland
2024-01-08 16:21 ` Markus Armbruster
1 sibling, 1 reply; 23+ messages in thread
From: Mark Cave-Ayland @ 2024-01-07 14:04 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, inesvarhol, Inès Varhol,
Markus Armbruster, Peter Maydell, Eduardo Habkost, Paolo Bonzini
Cc: qemu-devel, Alistair Francis, Arnaud Minier, Laurent Vivier,
Thomas Huth, qemu-arm, Daniel P. Berrangé
On 05/01/2024 10:13, Philippe Mathieu-Daudé wrote:
> (+Mark & Eduardo)
>
> On 4/1/24 14:37, inesvarhol wrote:
>>
>> Le jeudi 4 janvier 2024 à 14:05, Philippe Mathieu-Daudé <philmd@linaro.org> a écrit :
>>
>> Hello,
>>
>>>> +static void test_edge_selector(void)
>>>> +{
>>>> + enable_nvic_irq(EXTI0_IRQ);
>>>> +
>>>> + / Configure EXTI line 0 irq on rising edge */
>>>> + qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
>>>
>>>
>>> Markus, this qtest use seems to expect some stability in QOM path...
>>>
>>> Inès, Arnaud, having the SoC unattached is dubious, it belongs to
>>> the machine.
>>
>> Noted, we will fix that.
>> Should we be concerned about the "stability in QOM path" ?
>
> Don't worry about this Inès, I wanted to raise Markus attention on this.
>
> You showed a legit use of stable QOM path, and Markus told me recently
> there is no contract for QOM paths (it shouldn't be considered as a
> stable API). IIRC Markus explanation, "/unattached" container was
> added as a temporary hack to allow migrating QDev objects to QOM (see
> around commit da57febfed "qdev: give all devices a canonical path",
> 11 years ago).
>
> I agree anything under "/unattached" can be expected to be stable
> (but we need a community consensus). Then the big question remaining
> is "can any qom-path out of /unattached be considered stable?"
For the moment I would definitely say no, and that is mainly because if we were to
assume that QOM paths were stable today then I can see it being a barrier to updating
older code to meet our current guidelines.
These days I think more about QOM paths being related to the lifecycle of the objects
e.g. a machine object has child devices, which may also consist of a number of other
children in the case of a multi-function device. For me this means that using
object_resolve_path_component() to look up a child object seems reasonable, in
contrast with expecting the entire path to be stable.
One thing I think about often is whether the use of device[n] is suitable within QOM
tree. For example, if I have a command line like:
-device foo,myprop=prop0,id=fooid0 -device foo,myprop=prop1,id=fooid1
currently they would appear in "info qom-tree" as:
/machine
/unattached
/device[0] (foo)
/device[1] (foo)
whereas it feels this could be done better as:
/machine
/unattached
/foo[0] (fooid0)
/foo[1] (fooid1)
This would automatically place devices of the same type within a QOM array to allow
them to be accessed separately by type, or even directly via the "id" if we assume
they are unique. In particular if you have a machine with 2 foo in-built devices you
could then potentially configure them separately using -global foo[0].myprop=newprop0
and/or -global foo[1].myprop=newprop1 which is something that currently isn't possible.
ATB,
Mark.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v5 2/3] tests/qtest: Add STM32L4x5 EXTI QTest testcase
2024-01-05 10:19 ` Philippe Mathieu-Daudé
@ 2024-01-08 16:15 ` Markus Armbruster
0 siblings, 0 replies; 23+ messages in thread
From: Markus Armbruster @ 2024-01-08 16:15 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: inesvarhol, Inès Varhol, Mark Cave-Ayland, Peter Maydell,
Eduardo Habkost, Paolo Bonzini, qemu-devel, Alistair Francis,
Arnaud Minier, Laurent Vivier, Thomas Huth, qemu-arm,
Daniel P. Berrangé
Philippe Mathieu-Daudé <philmd@linaro.org> writes:
> On 5/1/24 11:13, Philippe Mathieu-Daudé wrote:
>> (+Mark & Eduardo)
>> On 4/1/24 14:37, inesvarhol wrote:
>>>
>>> Le jeudi 4 janvier 2024 à 14:05, Philippe Mathieu-Daudé <philmd@linaro.org> a écrit :
>>>
>>> Hello,
>>>
>>>>> +static void test_edge_selector(void)
>>>>> +{
>>>>> + enable_nvic_irq(EXTI0_IRQ);
>>>>> +
>>>>> + / Configure EXTI line 0 irq on rising edge */
>>>>> + qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
>>>>
>>>>
>>>> Markus, this qtest use seems to expect some stability in QOM path...
>>>>
>>>> Inès, Arnaud, having the SoC unattached is dubious, it belongs to
>>>> the machine.
>>>
>>> Noted, we will fix that.
>>> Should we be concerned about the "stability in QOM path" ?
>>
>> Don't worry about this Inès, I wanted to raise Markus attention on this.
>>
>> You showed a legit use of stable QOM path, and Markus told me recently
>> there is no contract for QOM paths (it shouldn't be considered as a
>> stable API). IIRC Markus explanation, "/unattached" container was
>> added as a temporary hack to allow migrating QDev objects to QOM (see
>> around commit da57febfed "qdev: give all devices a canonical path",
>> 11 years ago).
I'm not sure the hack was intended to be temporary. Doesn't matter now.
The connection between parent and child is a child property of the
parent. Like all properties, it has a name. These names are the
components of the canonical path.
When the code that creates the child makes the connection, it can give
the property a sensible name.
When it doesn't, we sometimes do it in generic code, using the
/machine/unattached orphanage, and a name that contains a counter, like
/machine/unattached/device[N]
/machine/unattached/non-qdev-gpio[N]
The actual name depends on execution order, because the counter value
does. Brittle.
> Hmm am I getting confused with "/peripheral-anon" (commit 8eb02831af
> "dev: add an anonymous peripheral container")?
Not the same, but related. Devices added with -device / device_add go
into /machine/peripheral/ID when they have id=ID, else into
/machine/peripheral/anon/device[N]. Before the commit you quoted, the
latter were orphaned I believe.
>> I agree anything under "/unattached" can be expected to be stable
>> (but we need a community consensus). Then the big question remaining
>> is "can any qom-path out of /unattached be considered stable?"
Backwards? Keeping /machine/unattached/FOO[N] stable is harder then the
paths the code picks explicitly.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v5 2/3] tests/qtest: Add STM32L4x5 EXTI QTest testcase
2024-01-07 14:04 ` Mark Cave-Ayland
@ 2024-01-08 16:21 ` Markus Armbruster
0 siblings, 0 replies; 23+ messages in thread
From: Markus Armbruster @ 2024-01-08 16:21 UTC (permalink / raw)
To: Mark Cave-Ayland
Cc: Philippe Mathieu-Daudé, inesvarhol, Inès Varhol,
Peter Maydell, Eduardo Habkost, Paolo Bonzini, qemu-devel,
Alistair Francis, Arnaud Minier, Laurent Vivier, Thomas Huth,
qemu-arm, Daniel P. Berrangé
Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> writes:
> On 05/01/2024 10:13, Philippe Mathieu-Daudé wrote:
>
>> (+Mark & Eduardo)
>> On 4/1/24 14:37, inesvarhol wrote:
>>>
>>> Le jeudi 4 janvier 2024 à 14:05, Philippe Mathieu-Daudé <philmd@linaro.org> a écrit :
>>>
>>> Hello,
>>>
>>>>> +static void test_edge_selector(void)
>>>>> +{
>>>>> + enable_nvic_irq(EXTI0_IRQ);
>>>>> +
>>>>> + / Configure EXTI line 0 irq on rising edge */
>>>>> + qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
>>>>
>>>>
>>>> Markus, this qtest use seems to expect some stability in QOM path...
>>>>
>>>> Inès, Arnaud, having the SoC unattached is dubious, it belongs to
>>>> the machine.
>>>
>>> Noted, we will fix that.
>>> Should we be concerned about the "stability in QOM path" ?
>>
>> Don't worry about this Inès, I wanted to raise Markus attention on this.
>>
>> You showed a legit use of stable QOM path, and Markus told me recently
>> there is no contract for QOM paths (it shouldn't be considered as a
>> stable API). IIRC Markus explanation, "/unattached" container was
>> added as a temporary hack to allow migrating QDev objects to QOM (see
>> around commit da57febfed "qdev: give all devices a canonical path",
>> 11 years ago).
>>
>> I agree anything under "/unattached" can be expected to be stable
>> (but we need a community consensus). Then the big question remaining
>> is "can any qom-path out of /unattached be considered stable?"
>
> For the moment I would definitely say no, and that is mainly because if we were to assume that QOM paths were stable today then I can see it being a barrier to updating older code to meet our current guidelines.
>
> These days I think more about QOM paths being related to the lifecycle of the objects e.g. a machine object has child devices, which may also consist of a number of other children in the case of a multi-function device. For me this means that using object_resolve_path_component() to look up a child object seems reasonable, in contrast with expecting the entire path to be stable.
>
> One thing I think about often is whether the use of device[n] is suitable within QOM tree. For example, if I have a command line like:
>
> -device foo,myprop=prop0,id=fooid0 -device foo,myprop=prop1,id=fooid1
>
> currently they would appear in "info qom-tree" as:
>
> /machine
> /unattached
> /device[0] (foo)
> /device[1] (foo)
Actually
/machine
/peripheral (container)
/fooid0 (foo
/fooid1 (foo)
If you omit id=..., you get
/machine
/peripheral-anon (container)
/device[2] (usb-mouse)
/device[3] (usb-mouse)
or similar; the actual numbers in [brackets] depend on the board.
> whereas it feels this could be done better as:
>
> /machine
> /unattached
> /foo[0] (fooid0)
> /foo[1] (fooid1)
>
> This would automatically place devices of the same type within a QOM array to allow them to be accessed separately by type, or even directly via the "id" if we assume they are unique. In particular if you have a machine with 2 foo in-built devices you could then potentially configure them separately using -global foo[0].myprop=newprop0 and/or -global foo[1].myprop=newprop1 which is something that currently isn't possible.
>
>
> ATB,
>
> Mark.
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2024-01-08 16:22 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-12-28 16:19 [PATCH v5 0/3] Add device STM32L4x5 EXTI Inès Varhol
2023-12-28 16:19 ` [PATCH v5 1/3] hw/misc: Implement " Inès Varhol
2024-01-04 13:14 ` Philippe Mathieu-Daudé
2024-01-04 13:23 ` Samuel Tardieu
2024-01-04 13:40 ` Philippe Mathieu-Daudé
2024-01-04 13:59 ` Samuel Tardieu
2024-01-04 14:06 ` inesvarhol
2023-12-28 16:19 ` [PATCH v5 2/3] tests/qtest: Add STM32L4x5 EXTI QTest testcase Inès Varhol
2024-01-04 3:39 ` Alistair Francis
2024-01-04 13:05 ` Philippe Mathieu-Daudé
2024-01-04 13:37 ` inesvarhol
2024-01-05 10:13 ` Philippe Mathieu-Daudé
2024-01-05 10:19 ` Philippe Mathieu-Daudé
2024-01-08 16:15 ` Markus Armbruster
2024-01-07 14:04 ` Mark Cave-Ayland
2024-01-08 16:21 ` Markus Armbruster
2024-01-05 10:24 ` Daniel P. Berrangé
2024-01-05 22:16 ` Philippe Mathieu-Daudé
2024-01-04 13:33 ` Philippe Mathieu-Daudé
2024-01-04 13:45 ` inesvarhol
2023-12-28 16:19 ` [PATCH v5 3/3] hw/arm: Connect STM32L4x5 EXTI to STM32L4x5 SoC Inès Varhol
2024-01-04 3:42 ` Alistair Francis
2024-01-04 13:17 ` 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).