* [PATCH v8 0/3] Add device STM32L4x5 EXTI
@ 2024-01-09 16:06 Inès Varhol
2024-01-09 16:06 ` [PATCH v8 1/3] hw/misc: Implement " Inès Varhol
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Inès Varhol @ 2024-01-09 16:06 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Philippe Mathieu-Daudé,
Peter Maydell, Thomas Huth, Laurent Vivier, Arnaud Minier,
qemu-arm, Inès Varhol, Samuel Tardieu, Alistair Francis
This patch adds a new device STM32L4x5 EXTI device and is part
of a series implementing the STM32L4x5 with a few peripherals.
Hello,
I see that the previous patch v7 couldn't be applied to master,
the log is :
...
Applying: hw/arm: Connect STM32L4x5 EXTI to STM32L4x5 SoC
error: sha1 information is lacking or useless (hw/arm/stm32l4x5_soc.c).
error: could not build fake ancestor
...
I notice there was a wrong Based-on message ID in v7 and I figure
that's the issue (since the relevant commit
"hw/arm: Connect STM32L4x5 EXTI to STM32L4x5 SoC" didn't even change
from v6). If that's not the case, I apologize in advance.
Changes from v7 to v8:
- none except for "Based-on" message-id correction
Changes from v6 to v7:
- in `stm32l4x5_exti_reset_hold`: unify register initialization
using new array `exti_romask`
- in `stm32l4x5_exti_set_irq`: rename `n` to `bank`, use new helper
function `regbank_index_by_irq` and new macro `EXTI_MAX_IRQ_PER_BANK`,
explain why there's no `else` clause
- in `stm32l4x5_exti_read`: rename `n` to `bank`, use new helper
function `regbank_index_by_irq`, use `HWADDR_PRIx` formatting in error log
- in `stm32l4x5_exti_write`: rename `n` to `bank`, use new helper
function `regbank_index_by_irq`, use `HWADDR_PRIx` formatting in error log,
unify writes where only bank differs using new helper functions
`valid_mask` and `configurable_mask`
- in `stm32l4x5_exti_init`: use `size_t` instead of `int` in for loop
- in `stm32l4x5_exti-test.c`: instead of declaring intermediate
variables, using `exti_readl` directly in `g_assert_cmpuint`
so that QEMU coding style is respected
Changes from v5 to v6:
- in `stm32l4x5_exti-test.c` : using a helper function
`exti_set_irq()` to help readability
- in `stm32l4x5_exti-test.c` : correct a mistake in test
`test_edge_selector` (adding a necessary NVIC interrupt unpend
so that the assert actually checks something)
- in `stm32l4x5_soc.c` : reducing scope of `i` used in for loops
- in `stm32l4x5_soc.c` : removing useless variable `dev`
- swapping commit 2 (add tests) and commit 3 (connects exti to SoC)
so that the tests pass in the commit they're added
Changes from v4 to v5:
- update the documentation file
Changes from v3 to v4:
- add a test to check that irq trigger selection works correctly
(`test_edge_selector`) and correct `stm32l4x5_exti_set_irq` accordingly
Changes from v2 to v3:
- corrected the license to GPL
Changes from v1 to v2:
- correct the commit messages
- remove a misleading comment
Changes from v3 to v1:
- separating the patch in 3 commits
- justifying in the commit message why we implement a new model instead
of changing the existing stm32f4xx_exti
- changed irq_raise to irq_pulse in register SWIERx write
(in `stm32l4x5_exti_write`) to be consistent with the irq_pulse in
`stm32l4x5_exti_set_irq` (and also both these interrupts are
edge-triggered)
- changed the license to GPL
Changes from v2 to v3:
- adding more tests writing/reading in exti registers
- adding tests checking that interrupt work by reading NVIC registers
- correcting exti_write in SWIER (so it sets an irq only when a bit
goes from '0' to '1')
- correcting exti_set_irq (so it never writes in PR when the relevant
bit in IMR is '0')
Changes from v1 to v2:
- use arrays to deduplicate code and logic
- move internal constant `EXTI_NUM_GPIO_EVENT_IN_LINES` from the header
to the .c file
- Improve copyright headers
- replace `static const` with `#define`
- use the DEFINE_TYPES macro
- fill the `impl` and `valid` field of the exti's `MemoryRegionOps`
- fix invalid test caused by a last minute change
Based-on: 20240108135849.351719-1-ines.varhol@telecom-paris.fr
([PATCH v6 0/2] Add minimal support for the B-L475E-IOT01A board)
Inès Varhol (3):
hw/misc: Implement STM32L4x5 EXTI
hw/arm: Connect STM32L4x5 EXTI to STM32L4x5 SoC
tests/qtest: Add STM32L4x5 EXTI QTest testcase
docs/system/arm/b-l475e-iot01a.rst | 5 +-
hw/arm/Kconfig | 1 +
hw/arm/stm32l4x5_soc.c | 52 ++-
hw/misc/Kconfig | 3 +
hw/misc/meson.build | 1 +
hw/misc/stm32l4x5_exti.c | 290 ++++++++++++++++
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 | 524 +++++++++++++++++++++++++++++
11 files changed, 936 insertions(+), 4 deletions(-)
create mode 100644 hw/misc/stm32l4x5_exti.c
create mode 100644 include/hw/misc/stm32l4x5_exti.h
create mode 100644 tests/qtest/stm32l4x5_exti-test.c
--
2.43.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v8 1/3] hw/misc: Implement STM32L4x5 EXTI
2024-01-09 16:06 [PATCH v8 0/3] Add device STM32L4x5 EXTI Inès Varhol
@ 2024-01-09 16:06 ` Inès Varhol
2024-01-09 17:13 ` Philippe Mathieu-Daudé
2024-01-09 16:06 ` [PATCH v8 2/3] hw/arm: Connect STM32L4x5 EXTI to STM32L4x5 SoC Inès Varhol
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Inès Varhol @ 2024-01-09 16:06 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Philippe Mathieu-Daudé,
Peter Maydell, Thomas Huth, Laurent Vivier, Arnaud Minier,
qemu-arm, Inès Varhol, Samuel Tardieu, Alistair Francis,
Alistair Francis
Although very similar to the STM32F4xx EXTI, STM32L4x5 EXTI generates
more than 32 event/interrupt requests and thus uses more registers
than STM32F4xx EXTI which generates 23 event/interrupt requests.
Acked-by: Alistair Francis <alistair.francis@wdc.com>
Signed-off-by: Arnaud Minier <arnaud.minier@telecom-paris.fr>
Signed-off-by: Inès Varhol <ines.varhol@telecom-paris.fr>
---
docs/system/arm/b-l475e-iot01a.rst | 5 +-
hw/misc/Kconfig | 3 +
hw/misc/meson.build | 1 +
hw/misc/stm32l4x5_exti.c | 290 +++++++++++++++++++++++++++++
hw/misc/trace-events | 5 +
include/hw/misc/stm32l4x5_exti.h | 51 +++++
6 files changed, 352 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..9fd859160d
--- /dev/null
+++ b/hw/misc/stm32l4x5_exti.c
@@ -0,0 +1,290 @@
+/*
+ * STM32L4x5 EXTI (Extended interrupts and events controller)
+ *
+ * Copyright (c) 2023 Arnaud Minier <arnaud.minier@telecom-paris.fr>
+ * Copyright (c) 2023 Samuel Tardieu <samuel.tardieu@telecom-paris.fr>
+ * Copyright (c) 2023 Inès Varhol <ines.varhol@telecom-paris.fr>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ * This work is based on the stm32f4xx_exti by Alistair Francis.
+ * Original code is licensed under the MIT License:
+ *
+ * Copyright (c) 2014 Alistair Francis <alistair@alistair23.me>
+ */
+
+/*
+ * The reference used is the STMicroElectronics RM0351 Reference manual
+ * for STM32L4x5 and STM32L4x6 advanced Arm ® -based 32-bit MCUs.
+ * https://www.st.com/en/microcontrollers-microprocessors/stm32l4x5/documentation.html
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "trace.h"
+#include "hw/irq.h"
+#include "migration/vmstate.h"
+#include "hw/misc/stm32l4x5_exti.h"
+
+#define EXTI_IMR1 0x00
+#define EXTI_EMR1 0x04
+#define EXTI_RTSR1 0x08
+#define EXTI_FTSR1 0x0C
+#define EXTI_SWIER1 0x10
+#define EXTI_PR1 0x14
+#define EXTI_IMR2 0x20
+#define EXTI_EMR2 0x24
+#define EXTI_RTSR2 0x28
+#define EXTI_FTSR2 0x2C
+#define EXTI_SWIER2 0x30
+#define EXTI_PR2 0x34
+
+#define EXTI_NUM_GPIO_EVENT_IN_LINES 16
+#define EXTI_MAX_IRQ_PER_BANK 32
+#define EXTI_IRQS_BANK0 32
+#define EXTI_IRQS_BANK1 8
+
+static const unsigned irqs_per_bank[EXTI_NUM_REGISTER] = {
+ EXTI_IRQS_BANK0,
+ EXTI_IRQS_BANK1,
+};
+
+static const uint32_t exti_romask[EXTI_NUM_REGISTER] = {
+ 0xff820000, /* 0b11111111_10000010_00000000_00000000 */
+ 0x00000087, /* 0b00000000_00000000_00000000_10000111 */
+};
+
+static unsigned regbank_index_by_irq(unsigned irq)
+{
+ return irq >= EXTI_MAX_IRQ_PER_BANK ? 1 : 0;
+}
+
+static unsigned regbank_index_by_addr(hwaddr addr)
+{
+ return addr >= EXTI_IMR2 ? 1 : 0;
+}
+
+static unsigned valid_mask(unsigned bank)
+{
+ return MAKE_64BIT_MASK(0, irqs_per_bank[bank]);
+}
+
+static unsigned configurable_mask(unsigned bank)
+{
+ return valid_mask(bank) & ~exti_romask[bank];
+}
+
+static void stm32l4x5_exti_reset_hold(Object *obj)
+{
+ Stm32l4x5ExtiState *s = STM32L4X5_EXTI(obj);
+
+ for (unsigned bank = 0; bank < EXTI_NUM_REGISTER; bank++) {
+ s->imr[bank] = exti_romask[bank];
+ s->emr[bank] = 0x00000000;
+ s->rtsr[bank] = 0x00000000;
+ s->ftsr[bank] = 0x00000000;
+ s->swier[bank] = 0x00000000;
+ s->pr[bank] = 0x00000000;
+ }
+}
+
+static void stm32l4x5_exti_set_irq(void *opaque, int irq, int level)
+{
+ Stm32l4x5ExtiState *s = opaque;
+ const unsigned bank = regbank_index_by_irq(irq);
+ const int oirq = irq;
+
+ trace_stm32l4x5_exti_set_irq(irq, level);
+
+ /* Shift the value to enable access in x2 registers. */
+ irq %= EXTI_MAX_IRQ_PER_BANK;
+
+ /* If the interrupt is masked, pr won't be raised */
+ if (!extract32(s->imr[bank], irq, 1)) {
+ return;
+ }
+
+ if (((1 << irq) & s->rtsr[bank]) && level) {
+ /* Rising Edge */
+ s->pr[bank] |= 1 << irq;
+ qemu_irq_pulse(s->irq[oirq]);
+ } else if (((1 << irq) & s->ftsr[bank]) && !level) {
+ /* Falling Edge */
+ s->pr[bank] |= 1 << irq;
+ qemu_irq_pulse(s->irq[oirq]);
+ }
+ /*
+ * In the following situations :
+ * - falling edge but rising trigger selected
+ * - rising edge but falling trigger selected
+ * - no trigger selected
+ * No action is required
+ */
+}
+
+static uint64_t stm32l4x5_exti_read(void *opaque, hwaddr addr,
+ unsigned int size)
+{
+ Stm32l4x5ExtiState *s = opaque;
+ uint32_t r = 0;
+ const unsigned bank = regbank_index_by_addr(addr);
+
+ switch (addr) {
+ case EXTI_IMR1:
+ case EXTI_IMR2:
+ r = s->imr[bank];
+ break;
+ case EXTI_EMR1:
+ case EXTI_EMR2:
+ r = s->emr[bank];
+ break;
+ case EXTI_RTSR1:
+ case EXTI_RTSR2:
+ r = s->rtsr[bank];
+ break;
+ case EXTI_FTSR1:
+ case EXTI_FTSR2:
+ r = s->ftsr[bank];
+ break;
+ case EXTI_SWIER1:
+ case EXTI_SWIER2:
+ r = s->swier[bank];
+ break;
+ case EXTI_PR1:
+ case EXTI_PR2:
+ r = s->pr[bank];
+ break;
+
+ default:
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "STM32L4X5_exti_read: Bad offset 0x%" HWADDR_PRIx "\n",
+ 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 unsigned bank = regbank_index_by_addr(addr);
+
+ trace_stm32l4x5_exti_write(addr, val64);
+
+ switch (addr) {
+ case EXTI_IMR1:
+ case EXTI_IMR2:
+ s->imr[bank] = val64 & valid_mask(bank);
+ return;
+ case EXTI_EMR1:
+ case EXTI_EMR2:
+ s->emr[bank] = val64 & valid_mask(bank);
+ return;
+ case EXTI_RTSR1:
+ case EXTI_RTSR2:
+ s->rtsr[bank] = val64 & configurable_mask(bank);
+ return;
+ case EXTI_FTSR1:
+ case EXTI_FTSR2:
+ s->ftsr[bank] = val64 & configurable_mask(bank);
+ return;
+ case EXTI_SWIER1:
+ case EXTI_SWIER2: {
+ const uint32_t set = val64 & configurable_mask(bank);
+ const uint32_t pend = set & ~s->swier[bank] & s->imr[bank] &
+ ~s->pr[bank];
+ s->swier[bank] = set;
+ s->pr[bank] |= pend;
+ for (unsigned i = 0; i < irqs_per_bank[bank]; i++) {
+ if (extract32(pend, i, 1)) {
+ qemu_irq_pulse(s->irq[i + 32 * bank]);
+ }
+ }
+ return;
+ }
+ case EXTI_PR1:
+ case EXTI_PR2: {
+ const uint32_t cleared = s->pr[bank] & val64 & configurable_mask(bank);
+ /* This bit is cleared by writing a 1 to it */
+ s->pr[bank] &= ~cleared;
+ /* Software triggered interrupts are cleared as well */
+ s->swier[bank] &= ~cleared;
+ return;
+ }
+ default:
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "STM32L4X5_exti_write: Bad offset 0x%" HWADDR_PRIx "\n",
+ 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);
+
+ for (size_t i = 0; i < EXTI_NUM_INTERRUPT_OUT_LINES; i++) {
+ sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->irq[i]);
+ }
+
+ memory_region_init_io(&s->mmio, obj, &stm32l4x5_exti_ops, s,
+ TYPE_STM32L4X5_EXTI, 0x400);
+ sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->mmio);
+
+ qdev_init_gpio_in(DEVICE(obj), stm32l4x5_exti_set_irq,
+ EXTI_NUM_GPIO_EVENT_IN_LINES);
+}
+
+static const VMStateDescription vmstate_stm32l4x5_exti = {
+ .name = TYPE_STM32L4X5_EXTI,
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .fields = (VMStateField[]) {
+ VMSTATE_UINT32_ARRAY(imr, Stm32l4x5ExtiState, EXTI_NUM_REGISTER),
+ VMSTATE_UINT32_ARRAY(emr, Stm32l4x5ExtiState, EXTI_NUM_REGISTER),
+ VMSTATE_UINT32_ARRAY(rtsr, Stm32l4x5ExtiState, EXTI_NUM_REGISTER),
+ VMSTATE_UINT32_ARRAY(ftsr, Stm32l4x5ExtiState, EXTI_NUM_REGISTER),
+ VMSTATE_UINT32_ARRAY(swier, Stm32l4x5ExtiState, EXTI_NUM_REGISTER),
+ VMSTATE_UINT32_ARRAY(pr, Stm32l4x5ExtiState, EXTI_NUM_REGISTER),
+ VMSTATE_END_OF_LIST()
+ }
+};
+
+static void stm32l4x5_exti_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+ ResettableClass *rc = RESETTABLE_CLASS(klass);
+
+ dc->vmsd = &vmstate_stm32l4x5_exti;
+ rc->phases.hold = stm32l4x5_exti_reset_hold;
+}
+
+static const TypeInfo stm32l4x5_exti_types[] = {
+ {
+ .name = TYPE_STM32L4X5_EXTI,
+ .parent = TYPE_SYS_BUS_DEVICE,
+ .instance_size = sizeof(Stm32l4x5ExtiState),
+ .instance_init = stm32l4x5_exti_init,
+ .class_init = stm32l4x5_exti_class_init,
+ }
+};
+
+DEFINE_TYPES(stm32l4x5_exti_types)
diff --git a/hw/misc/trace-events b/hw/misc/trace-events
index 85725506bf..fccd3204bf 100644
--- a/hw/misc/trace-events
+++ b/hw/misc/trace-events
@@ -163,6 +163,11 @@ stm32f4xx_exti_set_irq(int irq, int level) "Set EXTI: %d to %d"
stm32f4xx_exti_read(uint64_t addr) "reg read: addr: 0x%" PRIx64 " "
stm32f4xx_exti_write(uint64_t addr, uint64_t data) "reg write: addr: 0x%" PRIx64 " val: 0x%" PRIx64 ""
+# stm32l4x5_exti.c
+stm32l4x5_exti_set_irq(int irq, int level) "Set EXTI: %d to %d"
+stm32l4x5_exti_read(uint64_t addr, uint64_t data) "reg read: addr: 0x%" PRIx64 " val: 0x%" PRIx64 ""
+stm32l4x5_exti_write(uint64_t addr, uint64_t data) "reg write: addr: 0x%" PRIx64 " val: 0x%" PRIx64 ""
+
# tz-mpc.c
tz_mpc_reg_read(uint32_t offset, uint64_t data, unsigned size) "TZ MPC regs read: offset 0x%x data 0x%" PRIx64 " size %u"
tz_mpc_reg_write(uint32_t offset, uint64_t data, unsigned size) "TZ MPC regs write: offset 0x%x data 0x%" PRIx64 " size %u"
diff --git a/include/hw/misc/stm32l4x5_exti.h b/include/hw/misc/stm32l4x5_exti.h
new file mode 100644
index 0000000000..be961d2f01
--- /dev/null
+++ b/include/hw/misc/stm32l4x5_exti.h
@@ -0,0 +1,51 @@
+/*
+ * STM32L4x5 EXTI (Extended interrupts and events controller)
+ *
+ * Copyright (c) 2023 Arnaud Minier <arnaud.minier@telecom-paris.fr>
+ * Copyright (c) 2023 Inès Varhol <ines.varhol@telecom-paris.fr>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ * This work is based on the stm32f4xx_exti by Alistair Francis.
+ * Original code is licensed under the MIT License:
+ *
+ * Copyright (c) 2014 Alistair Francis <alistair@alistair23.me>
+ */
+
+/*
+ * The reference used is the STMicroElectronics RM0351 Reference manual
+ * for STM32L4x5 and STM32L4x6 advanced Arm ® -based 32-bit MCUs.
+ * https://www.st.com/en/microcontrollers-microprocessors/stm32l4x5/documentation.html
+ */
+
+#ifndef HW_STM32L4X5_EXTI_H
+#define HW_STM32L4X5_EXTI_H
+
+#include "hw/sysbus.h"
+#include "qom/object.h"
+
+#define TYPE_STM32L4X5_EXTI "stm32l4x5-exti"
+OBJECT_DECLARE_SIMPLE_TYPE(Stm32l4x5ExtiState, STM32L4X5_EXTI)
+
+#define EXTI_NUM_INTERRUPT_OUT_LINES 40
+#define EXTI_NUM_REGISTER 2
+
+struct Stm32l4x5ExtiState {
+ SysBusDevice parent_obj;
+
+ MemoryRegion mmio;
+
+ uint32_t imr[EXTI_NUM_REGISTER];
+ uint32_t emr[EXTI_NUM_REGISTER];
+ uint32_t rtsr[EXTI_NUM_REGISTER];
+ uint32_t ftsr[EXTI_NUM_REGISTER];
+ uint32_t swier[EXTI_NUM_REGISTER];
+ uint32_t pr[EXTI_NUM_REGISTER];
+
+ qemu_irq irq[EXTI_NUM_INTERRUPT_OUT_LINES];
+};
+
+#endif
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v8 2/3] hw/arm: Connect STM32L4x5 EXTI to STM32L4x5 SoC
2024-01-09 16:06 [PATCH v8 0/3] Add device STM32L4x5 EXTI Inès Varhol
2024-01-09 16:06 ` [PATCH v8 1/3] hw/misc: Implement " Inès Varhol
@ 2024-01-09 16:06 ` Inès Varhol
2024-02-07 22:02 ` Philippe Mathieu-Daudé
2024-01-09 16:06 ` [PATCH v8 3/3] tests/qtest: Add STM32L4x5 EXTI QTest testcase Inès Varhol
2024-01-12 18:13 ` [PATCH v8 0/3] Add device STM32L4x5 EXTI Peter Maydell
3 siblings, 1 reply; 9+ messages in thread
From: Inès Varhol @ 2024-01-09 16:06 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Philippe Mathieu-Daudé,
Peter Maydell, Thomas Huth, Laurent Vivier, Arnaud Minier,
qemu-arm, Inès Varhol, Samuel Tardieu, Alistair Francis,
Alistair Francis
Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Signed-off-by: Arnaud Minier <arnaud.minier@telecom-paris.fr>
Signed-off-by: Inès Varhol <ines.varhol@telecom-paris.fr>
---
hw/arm/Kconfig | 1 +
hw/arm/stm32l4x5_soc.c | 52 +++++++++++++++++++++++++++++++++-
include/hw/arm/stm32l4x5_soc.h | 3 ++
3 files changed, 55 insertions(+), 1 deletion(-)
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 4ae2073a1d..8c8488a70a 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -459,6 +459,7 @@ config STM32L4X5_SOC
bool
select ARM_V7M
select OR_IRQ
+ select STM32L4X5_EXTI
config XLNX_ZYNQMP_ARM
bool
diff --git a/hw/arm/stm32l4x5_soc.c b/hw/arm/stm32l4x5_soc.c
index 70609a6dac..fe46b7c6c0 100644
--- a/hw/arm/stm32l4x5_soc.c
+++ b/hw/arm/stm32l4x5_soc.c
@@ -36,10 +36,51 @@
#define SRAM2_BASE_ADDRESS 0x10000000
#define SRAM2_SIZE (32 * KiB)
+#define EXTI_ADDR 0x40010400
+
+#define NUM_EXTI_IRQ 40
+/* Match exti line connections with their CPU IRQ number */
+/* See Vector Table (Reference Manual p.396) */
+static const int exti_irq[NUM_EXTI_IRQ] = {
+ 6, /* GPIO[0] */
+ 7, /* GPIO[1] */
+ 8, /* GPIO[2] */
+ 9, /* GPIO[3] */
+ 10, /* GPIO[4] */
+ 23, 23, 23, 23, 23, /* GPIO[5..9] */
+ 40, 40, 40, 40, 40, 40, /* GPIO[10..15] */
+ 1, /* PVD */
+ 67, /* OTG_FS_WKUP, Direct */
+ 41, /* RTC_ALARM */
+ 2, /* RTC_TAMP_STAMP2/CSS_LSE */
+ 3, /* RTC wakeup timer */
+ 63, /* COMP1 */
+ 63, /* COMP2 */
+ 31, /* I2C1 wakeup, Direct */
+ 33, /* I2C2 wakeup, Direct */
+ 72, /* I2C3 wakeup, Direct */
+ 37, /* USART1 wakeup, Direct */
+ 38, /* USART2 wakeup, Direct */
+ 39, /* USART3 wakeup, Direct */
+ 52, /* UART4 wakeup, Direct */
+ 53, /* UART4 wakeup, Direct */
+ 70, /* LPUART1 wakeup, Direct */
+ 65, /* LPTIM1, Direct */
+ 66, /* LPTIM2, Direct */
+ 76, /* SWPMI1 wakeup, Direct */
+ 1, /* PVM1 wakeup */
+ 1, /* PVM2 wakeup */
+ 1, /* PVM3 wakeup */
+ 1, /* PVM4 wakeup */
+ 78 /* LCD wakeup, Direct */
+};
+
static void stm32l4x5_soc_initfn(Object *obj)
{
Stm32l4x5SocState *s = STM32L4X5_SOC(obj);
+ object_initialize_child(obj, "exti", &s->exti, TYPE_STM32L4X5_EXTI);
+
s->sysclk = qdev_init_clock_in(DEVICE(s), "sysclk", NULL, NULL, 0);
s->refclk = qdev_init_clock_in(DEVICE(s), "refclk", NULL, NULL, 0);
}
@@ -51,6 +92,7 @@ static void stm32l4x5_soc_realize(DeviceState *dev_soc, Error **errp)
const Stm32l4x5SocClass *sc = STM32L4X5_SOC_GET_CLASS(dev_soc);
MemoryRegion *system_memory = get_system_memory();
DeviceState *armv7m;
+ SysBusDevice *busdev;
/*
* We use s->refclk internally and only define it with qdev_init_clock_in()
@@ -112,6 +154,15 @@ static void stm32l4x5_soc_realize(DeviceState *dev_soc, Error **errp)
return;
}
+ busdev = SYS_BUS_DEVICE(&s->exti);
+ if (!sysbus_realize(busdev, errp)) {
+ return;
+ }
+ sysbus_mmio_map(busdev, 0, EXTI_ADDR);
+ for (unsigned i = 0; i < NUM_EXTI_IRQ; i++) {
+ sysbus_connect_irq(busdev, i, qdev_get_gpio_in(armv7m, exti_irq[i]));
+ }
+
/* APB1 BUS */
create_unimplemented_device("TIM2", 0x40000000, 0x400);
create_unimplemented_device("TIM3", 0x40000400, 0x400);
@@ -152,7 +203,6 @@ static void stm32l4x5_soc_realize(DeviceState *dev_soc, Error **errp)
create_unimplemented_device("SYSCFG", 0x40010000, 0x30);
create_unimplemented_device("VREFBUF", 0x40010030, 0x1D0);
create_unimplemented_device("COMP", 0x40010200, 0x200);
- create_unimplemented_device("EXTI", 0x40010400, 0x400);
/* RESERVED: 0x40010800, 0x1400 */
create_unimplemented_device("FIREWALL", 0x40011C00, 0x400);
/* RESERVED: 0x40012000, 0x800 */
diff --git a/include/hw/arm/stm32l4x5_soc.h b/include/hw/arm/stm32l4x5_soc.h
index 2fd44a36a9..f7305568dc 100644
--- a/include/hw/arm/stm32l4x5_soc.h
+++ b/include/hw/arm/stm32l4x5_soc.h
@@ -26,6 +26,7 @@
#include "exec/memory.h"
#include "hw/arm/armv7m.h"
+#include "hw/misc/stm32l4x5_exti.h"
#include "qom/object.h"
#define TYPE_STM32L4X5_SOC "stm32l4x5-soc"
@@ -39,6 +40,8 @@ struct Stm32l4x5SocState {
ARMv7MState armv7m;
+ Stm32l4x5ExtiState exti;
+
MemoryRegion sram1;
MemoryRegion sram2;
MemoryRegion flash;
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v8 3/3] tests/qtest: Add STM32L4x5 EXTI QTest testcase
2024-01-09 16:06 [PATCH v8 0/3] Add device STM32L4x5 EXTI Inès Varhol
2024-01-09 16:06 ` [PATCH v8 1/3] hw/misc: Implement " Inès Varhol
2024-01-09 16:06 ` [PATCH v8 2/3] hw/arm: Connect STM32L4x5 EXTI to STM32L4x5 SoC Inès Varhol
@ 2024-01-09 16:06 ` Inès Varhol
2024-01-12 18:13 ` [PATCH v8 0/3] Add device STM32L4x5 EXTI Peter Maydell
3 siblings, 0 replies; 9+ messages in thread
From: Inès Varhol @ 2024-01-09 16:06 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Philippe Mathieu-Daudé,
Peter Maydell, Thomas Huth, Laurent Vivier, Arnaud Minier,
qemu-arm, Inès Varhol, Samuel Tardieu, Alistair Francis,
Alistair Francis
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Acked-by: Alistair Francis <alistair.francis@wdc.com>
Signed-off-by: Arnaud Minier <arnaud.minier@telecom-paris.fr>
Signed-off-by: Inès Varhol <ines.varhol@telecom-paris.fr>
---
tests/qtest/meson.build | 5 +
tests/qtest/stm32l4x5_exti-test.c | 524 ++++++++++++++++++++++++++++++
2 files changed, 529 insertions(+)
create mode 100644 tests/qtest/stm32l4x5_exti-test.c
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index f25bffcc20..d890b6f333 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -194,6 +194,10 @@ qtests_aspeed = \
['aspeed_hace-test',
'aspeed_smc-test',
'aspeed_gpio-test']
+
+qtests_stm32l4x5 = \
+ ['stm32l4x5_exti-test']
+
qtests_arm = \
(config_all_devices.has_key('CONFIG_MPS2') ? ['sse-timer-test'] : []) + \
(config_all_devices.has_key('CONFIG_CMSDK_APB_DUALTIMER') ? ['cmsdk-apb-dualtimer-test'] : []) + \
@@ -207,6 +211,7 @@ qtests_arm = \
(config_all_devices.has_key('CONFIG_TPM_TIS_I2C') ? ['tpm-tis-i2c-test'] : []) + \
(config_all_devices.has_key('CONFIG_VEXPRESS') ? ['test-arm-mptimer'] : []) + \
(config_all_devices.has_key('CONFIG_MICROBIT') ? ['microbit-test'] : []) + \
+ (config_all_devices.has_key('CONFIG_STM32L4X5_SOC') ? qtests_stm32l4x5 : []) + \
['arm-cpu-features',
'boot-serial-test']
diff --git a/tests/qtest/stm32l4x5_exti-test.c b/tests/qtest/stm32l4x5_exti-test.c
new file mode 100644
index 0000000000..c390077713
--- /dev/null
+++ b/tests/qtest/stm32l4x5_exti-test.c
@@ -0,0 +1,524 @@
+/*
+ * QTest testcase for STM32L4x5_EXTI
+ *
+ * Copyright (c) 2023 Arnaud Minier <arnaud.minier@telecom-paris.fr>
+ * Copyright (c) 2023 Inès Varhol <ines.varhol@telecom-paris.fr>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "libqtest-single.h"
+
+#define EXTI_BASE_ADDR 0x40010400
+#define EXTI_IMR1 0x00
+#define EXTI_EMR1 0x04
+#define EXTI_RTSR1 0x08
+#define EXTI_FTSR1 0x0C
+#define EXTI_SWIER1 0x10
+#define EXTI_PR1 0x14
+#define EXTI_IMR2 0x20
+#define EXTI_EMR2 0x24
+#define EXTI_RTSR2 0x28
+#define EXTI_FTSR2 0x2C
+#define EXTI_SWIER2 0x30
+#define EXTI_PR2 0x34
+
+#define NVIC_ISER 0xE000E100
+#define NVIC_ISPR 0xE000E200
+#define NVIC_ICPR 0xE000E280
+
+#define EXTI0_IRQ 6
+#define EXTI1_IRQ 7
+#define EXTI35_IRQ 1
+
+static void enable_nvic_irq(unsigned int n)
+{
+ writel(NVIC_ISER, 1 << n);
+}
+
+static void unpend_nvic_irq(unsigned int n)
+{
+ writel(NVIC_ICPR, 1 << n);
+}
+
+static bool check_nvic_pending(unsigned int n)
+{
+ return readl(NVIC_ISPR) & (1 << n);
+}
+
+static void exti_writel(unsigned int offset, uint32_t value)
+{
+ writel(EXTI_BASE_ADDR + offset, value);
+}
+
+static uint32_t exti_readl(unsigned int offset)
+{
+ return readl(EXTI_BASE_ADDR + offset);
+}
+
+static void exti_set_irq(int num, int level)
+{
+ qtest_set_irq_in(global_qtest, "/machine/soc/exti", NULL,
+ num, level);
+}
+
+static void test_reg_write_read(void)
+{
+ /* Test that non-reserved bits in xMR and xTSR can be set and cleared */
+
+ exti_writel(EXTI_IMR1, 0xFFFFFFFF);
+ g_assert_cmpuint(exti_readl(EXTI_IMR1), ==, 0xFFFFFFFF);
+ exti_writel(EXTI_IMR1, 0x00000000);
+ g_assert_cmpuint(exti_readl(EXTI_IMR1), ==, 0x00000000);
+
+ exti_writel(EXTI_EMR1, 0xFFFFFFFF);
+ g_assert_cmpuint(exti_readl(EXTI_EMR1), ==, 0xFFFFFFFF);
+ exti_writel(EXTI_EMR1, 0x00000000);
+ g_assert_cmpuint(exti_readl(EXTI_EMR1), ==, 0x00000000);
+
+ exti_writel(EXTI_RTSR1, 0xFFFFFFFF);
+ g_assert_cmpuint(exti_readl(EXTI_RTSR1), ==, 0x007DFFFF);
+ exti_writel(EXTI_RTSR1, 0x00000000);
+ g_assert_cmpuint(exti_readl(EXTI_RTSR1), ==, 0x00000000);
+
+ exti_writel(EXTI_FTSR1, 0xFFFFFFFF);
+ g_assert_cmpuint(exti_readl(EXTI_FTSR1), ==, 0x007DFFFF);
+ exti_writel(EXTI_FTSR1, 0x00000000);
+ g_assert_cmpuint(exti_readl(EXTI_FTSR1), ==, 0x00000000);
+
+ exti_writel(EXTI_IMR2, 0xFFFFFFFF);
+ g_assert_cmpuint(exti_readl(EXTI_IMR2), ==, 0x000000FF);
+ exti_writel(EXTI_IMR2, 0x00000000);
+ g_assert_cmpuint(exti_readl(EXTI_IMR2), ==, 0x00000000);
+
+ exti_writel(EXTI_EMR2, 0xFFFFFFFF);
+ g_assert_cmpuint(exti_readl(EXTI_EMR2), ==, 0x000000FF);
+ exti_writel(EXTI_EMR2, 0x00000000);
+ g_assert_cmpuint(exti_readl(EXTI_EMR2), ==, 0x00000000);
+
+ exti_writel(EXTI_RTSR2, 0xFFFFFFFF);
+ g_assert_cmpuint(exti_readl(EXTI_RTSR2), ==, 0x00000078);
+ exti_writel(EXTI_RTSR2, 0x00000000);
+ g_assert_cmpuint(exti_readl(EXTI_RTSR2), ==, 0x00000000);
+
+ exti_writel(EXTI_FTSR2, 0xFFFFFFFF);
+ g_assert_cmpuint(exti_readl(EXTI_FTSR2), ==, 0x00000078);
+ exti_writel(EXTI_FTSR2, 0x00000000);
+ g_assert_cmpuint(exti_readl(EXTI_FTSR2), ==, 0x00000000);
+}
+
+static void test_direct_lines_write(void)
+{
+ /* Test that direct lines reserved bits are not written to */
+
+ exti_writel(EXTI_RTSR1, 0xFF820000);
+ g_assert_cmpuint(exti_readl(EXTI_RTSR1), ==, 0x00000000);
+
+ exti_writel(EXTI_FTSR1, 0xFF820000);
+ g_assert_cmpuint(exti_readl(EXTI_FTSR1), ==, 0x00000000);
+
+ exti_writel(EXTI_SWIER1, 0xFF820000);
+ g_assert_cmpuint(exti_readl(EXTI_SWIER1), ==, 0x00000000);
+
+ exti_writel(EXTI_PR1, 0xFF820000);
+ g_assert_cmpuint(exti_readl(EXTI_PR1), ==, 0x00000000);
+
+ exti_writel(EXTI_RTSR2, 0x00000087);
+ g_assert_cmpuint(exti_readl(EXTI_RTSR2), ==, 0x00000000);
+
+ exti_writel(EXTI_FTSR2, 0x00000087);
+ g_assert_cmpuint(exti_readl(EXTI_FTSR2), ==, 0x00000000);
+
+ exti_writel(EXTI_SWIER2, 0x00000087);
+ g_assert_cmpuint(exti_readl(EXTI_SWIER2), ==, 0x00000000);
+
+ exti_writel(EXTI_PR2, 0x00000087);
+ g_assert_cmpuint(exti_readl(EXTI_PR2), ==, 0x00000000);
+}
+
+static void test_reserved_bits_write(void)
+{
+ /* Test that reserved bits stay are not written to */
+
+ exti_writel(EXTI_IMR2, 0xFFFFFF00);
+ g_assert_cmpuint(exti_readl(EXTI_IMR2), ==, 0x00000000);
+
+ exti_writel(EXTI_EMR2, 0xFFFFFF00);
+ g_assert_cmpuint(exti_readl(EXTI_EMR2), ==, 0x00000000);
+
+ exti_writel(EXTI_RTSR2, 0xFFFFFF00);
+ g_assert_cmpuint(exti_readl(EXTI_RTSR2), ==, 0x00000000);
+
+ exti_writel(EXTI_FTSR2, 0xFFFFFF00);
+ g_assert_cmpuint(exti_readl(EXTI_FTSR2), ==, 0x00000000);
+
+ exti_writel(EXTI_SWIER2, 0xFFFFFF00);
+ g_assert_cmpuint(exti_readl(EXTI_SWIER2), ==, 0x00000000);
+
+ exti_writel(EXTI_PR2, 0xFFFFFF00);
+ g_assert_cmpuint(exti_readl(EXTI_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 */
+ g_assert_cmpuint(exti_readl(EXTI_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 */
+ g_assert_cmpuint(exti_readl(EXTI_SWIER1), ==, 0x00000001);
+ /* Check that the corresponding pending bit in PR is set */
+ g_assert_cmpuint(exti_readl(EXTI_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 */
+ g_assert_cmpuint(exti_readl(EXTI_PR1), ==, 0x00000000);
+ /* Check that the corresponding bit in SWIER was cleared */
+ g_assert_cmpuint(exti_readl(EXTI_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 */
+ g_assert_cmpuint(exti_readl(EXTI_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 */
+ g_assert_cmpuint(exti_readl(EXTI_SWIER2), ==, 0x00000008);
+ /* Check that the corresponding pending bit in PR is set */
+ g_assert_cmpuint(exti_readl(EXTI_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 */
+ g_assert_cmpuint(exti_readl(EXTI_PR2), ==, 0x00000000);
+ /* Check that the corresponding bit in SWIER was cleared */
+ g_assert_cmpuint(exti_readl(EXTI_SWIER2), ==, 0x00000000);
+ /* Check that the interrupt is still pending in the NVIC */
+ g_assert_true(check_nvic_pending(EXTI35_IRQ));
+
+ /* Clean NVIC */
+ unpend_nvic_irq(EXTI0_IRQ);
+ g_assert_false(check_nvic_pending(EXTI0_IRQ));
+ unpend_nvic_irq(EXTI35_IRQ);
+ g_assert_false(check_nvic_pending(EXTI35_IRQ));
+}
+
+static void test_edge_selector(void)
+{
+ enable_nvic_irq(EXTI0_IRQ);
+
+ /* Configure EXTI line 0 irq on rising edge */
+ exti_set_irq(0, 1);
+ exti_writel(EXTI_IMR1, 0x00000001);
+ exti_writel(EXTI_RTSR1, 0x00000001);
+ exti_writel(EXTI_FTSR1, 0x00000000);
+
+ /* Test that an irq is raised on rising edge only */
+ exti_set_irq(0, 0);
+ g_assert_cmpuint(exti_readl(EXTI_PR1), ==, 0x00000000);
+ g_assert_false(check_nvic_pending(EXTI0_IRQ));
+
+ exti_set_irq(0, 1);
+ g_assert_cmpuint(exti_readl(EXTI_PR1), ==, 0x00000001);
+ g_assert_true(check_nvic_pending(EXTI0_IRQ));
+
+ /* Clean the test */
+ exti_writel(EXTI_PR1, 0x00000001);
+ g_assert_cmpuint(exti_readl(EXTI_PR1), ==, 0x00000000);
+ unpend_nvic_irq(EXTI0_IRQ);
+ g_assert_false(check_nvic_pending(EXTI0_IRQ));
+
+ /* Configure EXTI line 0 irq on falling edge */
+ exti_set_irq(0, 0);
+ exti_writel(EXTI_IMR1, 0x00000001);
+ exti_writel(EXTI_RTSR1, 0x00000000);
+ exti_writel(EXTI_FTSR1, 0x00000001);
+
+ /* Test that an irq is raised on falling edge only */
+ exti_set_irq(0, 1);
+ g_assert_cmpuint(exti_readl(EXTI_PR1), ==, 0x00000000);
+ g_assert_false(check_nvic_pending(EXTI0_IRQ));
+
+ exti_set_irq(0, 0);
+ g_assert_cmpuint(exti_readl(EXTI_PR1), ==, 0x00000001);
+ g_assert_true(check_nvic_pending(EXTI0_IRQ));
+
+ /* Clean the test */
+ exti_writel(EXTI_PR1, 0x00000001);
+ g_assert_cmpuint(exti_readl(EXTI_PR1), ==, 0x00000000);
+ unpend_nvic_irq(EXTI0_IRQ);
+ g_assert_false(check_nvic_pending(EXTI0_IRQ));
+
+ /* Configure EXTI line 0 irq on falling and rising edge */
+ exti_writel(EXTI_IMR1, 0x00000001);
+ exti_writel(EXTI_RTSR1, 0x00000001);
+ exti_writel(EXTI_FTSR1, 0x00000001);
+
+ /* Test that an irq is raised on rising edge */
+ exti_set_irq(0, 1);
+ g_assert_cmpuint(exti_readl(EXTI_PR1), ==, 0x00000001);
+ g_assert_true(check_nvic_pending(EXTI0_IRQ));
+
+ /* Clean the test */
+ exti_writel(EXTI_PR1, 0x00000001);
+ g_assert_cmpuint(exti_readl(EXTI_PR1), ==, 0x00000000);
+ unpend_nvic_irq(EXTI0_IRQ);
+ g_assert_false(check_nvic_pending(EXTI0_IRQ));
+
+ /* Test that an irq is raised on falling edge */
+ exti_set_irq(0, 0);
+ g_assert_cmpuint(exti_readl(EXTI_PR1), ==, 0x00000001);
+ g_assert_true(check_nvic_pending(EXTI0_IRQ));
+
+ /* Clean the test */
+ exti_writel(EXTI_PR1, 0x00000001);
+ g_assert_cmpuint(exti_readl(EXTI_PR1), ==, 0x00000000);
+ unpend_nvic_irq(EXTI0_IRQ);
+ g_assert_false(check_nvic_pending(EXTI0_IRQ));
+
+ /* Configure EXTI line 0 irq without selecting an edge trigger */
+ exti_writel(EXTI_IMR1, 0x00000001);
+ exti_writel(EXTI_RTSR1, 0x00000000);
+ exti_writel(EXTI_FTSR1, 0x00000000);
+
+ /* Test that no irq is raised */
+ exti_set_irq(0, 1);
+ g_assert_cmpuint(exti_readl(EXTI_PR1), ==, 0x00000000);
+ g_assert_false(check_nvic_pending(EXTI0_IRQ));
+
+ exti_set_irq(0, 0);
+ g_assert_cmpuint(exti_readl(EXTI_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 */
+ g_assert_cmpuint(exti_readl(EXTI_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 */
+ g_assert_cmpuint(exti_readl(EXTI_SWIER1), ==, 0x00000001);
+ /* Check that the pending bit in PR wasn't set */
+ g_assert_cmpuint(exti_readl(EXTI_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 */
+ g_assert_cmpuint(exti_readl(EXTI_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 */
+ g_assert_cmpuint(exti_readl(EXTI_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 */
+ g_assert_cmpuint(exti_readl(EXTI_SWIER2), ==, 0x00000008);
+ /* Check that the pending bit in PR wasn't set */
+ g_assert_cmpuint(exti_readl(EXTI_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 */
+ g_assert_cmpuint(exti_readl(EXTI_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 */
+ g_assert_cmpuint(exti_readl(EXTI_PR1), ==, 0x00000000);
+ /* Check that this specific interrupt isn't pending in NVIC */
+ g_assert_false(check_nvic_pending(EXTI1_IRQ));
+
+ /* Mask interrupt line EXTI1 */
+ exti_writel(EXTI_IMR1, 0x00000000);
+
+ /* Configure interrupt on rising edge */
+ exti_writel(EXTI_RTSR1, 0x00000002);
+
+ /* Simulate rising edge from GPIO line 1 */
+ exti_set_irq(1, 1);
+
+ /* Check that the pending bit in PR wasn't set */
+ g_assert_cmpuint(exti_readl(EXTI_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 */
+ g_assert_cmpuint(exti_readl(EXTI_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 */
+ g_assert_cmpuint(exti_readl(EXTI_PR1), ==, 0x00000000);
+ /* Check that this specific interrupt isn't pending in NVIC */
+ g_assert_false(check_nvic_pending(EXTI1_IRQ));
+
+ /* Enable interrupt line EXTI1 */
+ exti_writel(EXTI_IMR1, 0x00000002);
+
+ /* Configure interrupt on rising edge */
+ exti_writel(EXTI_RTSR1, 0x00000002);
+
+ /* Simulate rising edge from GPIO line 1 */
+ exti_set_irq(1, 1);
+
+ /* Check that the pending bit in PR was set */
+ g_assert_cmpuint(exti_readl(EXTI_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 */
+ g_assert_cmpuint(exti_readl(EXTI_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] 9+ messages in thread
* Re: [PATCH v8 1/3] hw/misc: Implement STM32L4x5 EXTI
2024-01-09 16:06 ` [PATCH v8 1/3] hw/misc: Implement " Inès Varhol
@ 2024-01-09 17:13 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 9+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-01-09 17:13 UTC (permalink / raw)
To: Inès Varhol, qemu-devel
Cc: Paolo Bonzini, Peter Maydell, Thomas Huth, Laurent Vivier,
Arnaud Minier, qemu-arm, Samuel Tardieu, Alistair Francis,
Alistair Francis
On 9/1/24 17:06, Inès Varhol wrote:
> Although very similar to the STM32F4xx EXTI, STM32L4x5 EXTI generates
> more than 32 event/interrupt requests and thus uses more registers
> than STM32F4xx EXTI which generates 23 event/interrupt requests.
>
> Acked-by: Alistair Francis <alistair.francis@wdc.com>
> Signed-off-by: Arnaud Minier <arnaud.minier@telecom-paris.fr>
> Signed-off-by: Inès Varhol <ines.varhol@telecom-paris.fr>
> ---
> docs/system/arm/b-l475e-iot01a.rst | 5 +-
> hw/misc/Kconfig | 3 +
> hw/misc/meson.build | 1 +
> hw/misc/stm32l4x5_exti.c | 290 +++++++++++++++++++++++++++++
> hw/misc/trace-events | 5 +
> include/hw/misc/stm32l4x5_exti.h | 51 +++++
> 6 files changed, 352 insertions(+), 3 deletions(-)
> create mode 100644 hw/misc/stm32l4x5_exti.c
> create mode 100644 include/hw/misc/stm32l4x5_exti.h
> +static unsigned configurable_mask(unsigned bank)
> +{
> + return valid_mask(bank) & ~exti_romask[bank];
> +}
Excellent, I'm glad of all the improvement you made over
the review process, great work!
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> +static void stm32l4x5_exti_write(void *opaque, hwaddr addr,
> + uint64_t val64, unsigned int size)
> +{
> + Stm32l4x5ExtiState *s = opaque;
> + const unsigned bank = regbank_index_by_addr(addr);
> +
> + trace_stm32l4x5_exti_write(addr, val64);
> +
> + switch (addr) {
> + case EXTI_IMR1:
> + case EXTI_IMR2:
> + s->imr[bank] = val64 & valid_mask(bank);
> + return;
> + case EXTI_EMR1:
> + case EXTI_EMR2:
> + s->emr[bank] = val64 & valid_mask(bank);
> + return;
> + case EXTI_RTSR1:
> + case EXTI_RTSR2:
> + s->rtsr[bank] = val64 & configurable_mask(bank);
> + return;
> + case EXTI_FTSR1:
> + case EXTI_FTSR2:
> + s->ftsr[bank] = val64 & configurable_mask(bank);
> + return;
> + case EXTI_SWIER1:
> + case EXTI_SWIER2: {
> + const uint32_t set = val64 & configurable_mask(bank);
> + const uint32_t pend = set & ~s->swier[bank] & s->imr[bank] &
> + ~s->pr[bank];
> + s->swier[bank] = set;
> + s->pr[bank] |= pend;
> + for (unsigned i = 0; i < irqs_per_bank[bank]; i++) {
> + if (extract32(pend, i, 1)) {
> + qemu_irq_pulse(s->irq[i + 32 * bank]);
> + }
> + }
> + return;
> + }
[...]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v8 0/3] Add device STM32L4x5 EXTI
2024-01-09 16:06 [PATCH v8 0/3] Add device STM32L4x5 EXTI Inès Varhol
` (2 preceding siblings ...)
2024-01-09 16:06 ` [PATCH v8 3/3] tests/qtest: Add STM32L4x5 EXTI QTest testcase Inès Varhol
@ 2024-01-12 18:13 ` Peter Maydell
3 siblings, 0 replies; 9+ messages in thread
From: Peter Maydell @ 2024-01-12 18:13 UTC (permalink / raw)
To: Inès Varhol
Cc: qemu-devel, Paolo Bonzini, Philippe Mathieu-Daudé,
Thomas Huth, Laurent Vivier, Arnaud Minier, qemu-arm,
Samuel Tardieu, Alistair Francis
On Tue, 9 Jan 2024 at 16:07, Inès Varhol <ines.varhol@telecom-paris.fr> wrote:
>
> This patch adds a new device STM32L4x5 EXTI device and is part
> of a series implementing the STM32L4x5 with a few peripherals.
>
Applied to target-arm.next, thanks.
-- PMM
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v8 2/3] hw/arm: Connect STM32L4x5 EXTI to STM32L4x5 SoC
2024-01-09 16:06 ` [PATCH v8 2/3] hw/arm: Connect STM32L4x5 EXTI to STM32L4x5 SoC Inès Varhol
@ 2024-02-07 22:02 ` Philippe Mathieu-Daudé
2024-02-08 10:34 ` Inès Varhol
0 siblings, 1 reply; 9+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-02-07 22:02 UTC (permalink / raw)
To: Arnaud Minier, Inès Varhol, qemu-devel
Cc: Paolo Bonzini, Peter Maydell, Thomas Huth, Laurent Vivier,
qemu-arm, Samuel Tardieu, Alistair Francis, Alistair Francis
Hi Inès,
(this is now commit 52671f69f7).
On 9/1/24 17:06, Inès Varhol wrote:
> Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> Signed-off-by: Arnaud Minier <arnaud.minier@telecom-paris.fr>
> Signed-off-by: Inès Varhol <ines.varhol@telecom-paris.fr>
> ---
> hw/arm/Kconfig | 1 +
> hw/arm/stm32l4x5_soc.c | 52 +++++++++++++++++++++++++++++++++-
> include/hw/arm/stm32l4x5_soc.h | 3 ++
> 3 files changed, 55 insertions(+), 1 deletion(-)
> +#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] */
I'm sorry because I missed that earlier, and I'm surprised
you aren't chasing weird bugs. Due to how QEMU IRQs are
implemented, we can not wire multiple input lines to the same
output without using an intermediate "OR gate". We model it
as TYPE_OR_IRQ. See the comment in "hw/qdev-core.h" added in
commit cd07d7f9f5 ("qdev: Document GPIO related functions"):
* It is not valid to try to connect one outbound GPIO to multiple
* qemu_irqs at once, or to connect multiple outbound GPIOs to the
* same qemu_irq. (Warning: there is no assertion or other guard to
* catch this error: the model will just not do the right thing.)
* Instead, for fan-out you can use the TYPE_SPLIT_IRQ device: connect
* a device's outbound GPIO to the splitter's input, and connect each
* of the splitter's outputs to a different device. For fan-in you
* can use the TYPE_OR_IRQ device, which is a model of a logical OR
* gate with multiple inputs and one output.
So for example for the GPIO[10..15] you need to create a 6-line
OR gate as (totally untested):
/* 6-line OR IRQ gate */
Object *orgate40 = object_new(TYPE_OR_IRQ);
object_property_set_int(orgate40, "num-lines", 6, &error_fatal);
qdev_realize(DEVICE(orgate), NULL, &error_fatal);
/* OR gate -> IRQ #40 */
qdev_connect_gpio_out(DEVICE(orgate40), 0,
qdev_get_gpio_in(armv7m, 40));
/* EXTI GPIO[10..15] -> OR gate */
for (unsigned i = 0; i < 6; i++) {
sysbus_connect_irq(SYS_BUS_DEVICE(&s->exti), 10 + i,
qdev_get_gpio_in(DEVICE(orgate40), i));
}
> + 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 */
> +};
> + busdev = SYS_BUS_DEVICE(&s->exti);
> + if (!sysbus_realize(busdev, errp)) {
> + return;
> + }
> + sysbus_mmio_map(busdev, 0, EXTI_ADDR);
> + for (unsigned i = 0; i < NUM_EXTI_IRQ; i++) {
> + sysbus_connect_irq(busdev, i, qdev_get_gpio_in(armv7m, exti_irq[i]));
^^^^^^^^^^
> + }
Regards,
Phil.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v8 2/3] hw/arm: Connect STM32L4x5 EXTI to STM32L4x5 SoC
2024-02-07 22:02 ` Philippe Mathieu-Daudé
@ 2024-02-08 10:34 ` Inès Varhol
2024-02-08 12:35 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 9+ messages in thread
From: Inès Varhol @ 2024-02-08 10:34 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Arnaud Minier, qemu-devel, Paolo Bonzini, Peter Maydell,
Thomas Huth, Laurent Vivier, qemu-arm, Samuel Tardieu,
Alistair Francis, Alistair Francis
[-- Attachment #1: Type: text/plain, Size: 4345 bytes --]
Hi,
> De: Philippe <philmd@linaro.org>
> Envoyé: mercredi 7 février 2024 23:02 CET
>
> Hi Inès,
>
> (this is now commit 52671f69f7).
>
> On 9/1/24 17:06, Inès Varhol wrote:
> > Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> > Signed-off-by: Arnaud Minier <arnaud.minier@telecom-paris.fr>
> > Signed-off-by: Inès Varhol <ines.varhol@telecom-paris.fr>
> > ---
> > hw/arm/Kconfig | 1 +
> > hw/arm/stm32l4x5_soc.c | 52 +++++++++++++++++++++++++++++++++-
> > include/hw/arm/stm32l4x5_soc.h | 3 ++
> > 3 files changed, 55 insertions(+), 1 deletion(-)
>
>
> > +#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] */
>
> I'm sorry because I missed that earlier, and I'm surprised
> you aren't chasing weird bugs. Due to how QEMU IRQs are
> implemented, we can not wire multiple input lines to the same
> output without using an intermediate "OR gate". We model it
> as TYPE_OR_IRQ. See the comment in "hw/qdev-core.h" added in
> commit cd07d7f9f5 ("qdev: Document GPIO related functions"):
Better fixing it now than later :)
I must admit I didn't pay attention to the particularity of EXTI5 to 15.
Current exti tests don't even use these lines, a testcase will have
to be added. Otherwise we mostly ran executables using GPIOs as output,
so no weird bugs encountered.
Thank you for noticing !
Ines
>
> * It is not valid to try to connect one outbound GPIO to multiple
> * qemu_irqs at once, or to connect multiple outbound GPIOs to the
> * same qemu_irq. (Warning: there is no assertion or other guard to
> * catch this error: the model will just not do the right thing.)
> * Instead, for fan-out you can use the TYPE_SPLIT_IRQ device: connect
> * a device's outbound GPIO to the splitter's input, and connect each
> * of the splitter's outputs to a different device. For fan-in you
> * can use the TYPE_OR_IRQ device, which is a model of a logical OR
> * gate with multiple inputs and one output.
>
> So for example for the GPIO[10..15] you need to create a 6-line
> OR gate as (totally untested):
>
> /* 6-line OR IRQ gate */
> Object *orgate40 = object_new(TYPE_OR_IRQ);
> object_property_set_int(orgate40, "num-lines", 6, &error_fatal);
> qdev_realize(DEVICE(orgate), NULL, &error_fatal);
>
> /* OR gate -> IRQ #40 */
> qdev_connect_gpio_out(DEVICE(orgate40), 0,
> qdev_get_gpio_in(armv7m, 40));
>
> /* EXTI GPIO[10..15] -> OR gate */
> for (unsigned i = 0; i < 6; i++) {
> sysbus_connect_irq(SYS_BUS_DEVICE(&s->exti), 10 + i,
> qdev_get_gpio_in(DEVICE(orgate40), i));
> }
>
> > + 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 */
> > +};
>
> > + busdev = SYS_BUS_DEVICE(&s->exti);
> > + if (!sysbus_realize(busdev, errp)) {
> > + return;
> > + }
> > + sysbus_mmio_map(busdev, 0, EXTI_ADDR);
> > + for (unsigned i = 0; i < NUM_EXTI_IRQ; i++) {
> > + sysbus_connect_irq(busdev, i, qdev_get_gpio_in(armv7m, exti_irq[i]));
>
> ^^^^^^^^^^
> > + }
> Regards,
>
> Phil.
[-- Attachment #2: Type: text/html, Size: 6282 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v8 2/3] hw/arm: Connect STM32L4x5 EXTI to STM32L4x5 SoC
2024-02-08 10:34 ` Inès Varhol
@ 2024-02-08 12:35 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 9+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-02-08 12:35 UTC (permalink / raw)
To: Inès Varhol
Cc: Arnaud Minier, qemu-devel, Paolo Bonzini, Peter Maydell,
Thomas Huth, Laurent Vivier, qemu-arm, Samuel Tardieu,
Alistair Francis, Alistair Francis
On 8/2/24 11:34, Inès Varhol wrote:
> Hi,
>
> > De: Philippe <philmd@linaro.org>
> > Envoyé: mercredi 7 février 2024 23:02 CET
> >
> > Hi Inès,
> >
> > (this is now commit 52671f69f7).
> >
> > On 9/1/24 17:06, Inès Varhol wrote:
> > > Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > > Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> > > Signed-off-by: Arnaud Minier <arnaud.minier@telecom-paris.fr>
> > > Signed-off-by: Inès Varhol <ines.varhol@telecom-paris.fr>
> > > ---
> > > hw/arm/Kconfig | 1 +
> > > hw/arm/stm32l4x5_soc.c | 52 +++++++++++++++++++++++++++++++++-
> > > include/hw/arm/stm32l4x5_soc.h | 3 ++
> > > 3 files changed, 55 insertions(+), 1 deletion(-)
> >
> >
> > > +#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] */
BTW I only mentioned #40, but note other patterns, here:
GPIO[5..9] -> #23
> > > + 40, 40, 40, 40, 40, 40, /* GPIO[10..15] */
> >
> > I'm sorry because I missed that earlier, and I'm surprised
> > you aren't chasing weird bugs. Due to how QEMU IRQs are
> > implemented, we can not wire multiple input lines to the same
> > output without using an intermediate "OR gate". We model it
> > as TYPE_OR_IRQ. See the comment in "hw/qdev-core.h" added in
> > commit cd07d7f9f5 ("qdev: Document GPIO related functions"):
> Better fixing it now than later :)
> I must admit I didn't pay attention to the particularity of EXTI5 to 15.
> Current exti tests don't even use these lines, a testcase will have
> to be added. Otherwise we mostly ran executables using GPIOs as output,
> so no weird bugs encountered.
> Thank you for noticing !
> Ines
> >
> > * It is not valid to try to connect one outbound GPIO to multiple
> > * qemu_irqs at once, or to connect multiple outbound GPIOs to the
> > * same qemu_irq. (Warning: there is no assertion or other guard to
> > * catch this error: the model will just not do the right thing.)
> > * Instead, for fan-out you can use the TYPE_SPLIT_IRQ device: connect
> > * a device's outbound GPIO to the splitter's input, and connect each
> > * of the splitter's outputs to a different device. For fan-in you
> > * can use the TYPE_OR_IRQ device, which is a model of a logical OR
> > * gate with multiple inputs and one output.
> >
> > So for example for the GPIO[10..15] you need to create a 6-line
> > OR gate as (totally untested):
> >
> > /* 6-line OR IRQ gate */
> > Object *orgate40 = object_new(TYPE_OR_IRQ);
> > object_property_set_int(orgate40, "num-lines", 6, &error_fatal);
> > qdev_realize(DEVICE(orgate), NULL, &error_fatal);
> >
> > /* OR gate -> IRQ #40 */
> > qdev_connect_gpio_out(DEVICE(orgate40), 0,
> > qdev_get_gpio_in(armv7m, 40));
> >
> > /* EXTI GPIO[10..15] -> OR gate */
> > for (unsigned i = 0; i < 6; i++) {
> > sysbus_connect_irq(SYS_BUS_DEVICE(&s->exti), 10 + i,
> > qdev_get_gpio_in(DEVICE(orgate40), i));
> > }
> >
> > > + 1, /* PVD */
PVD, PVM[1-4] -> #1
> > > + 67, /* OTG_FS_WKUP, Direct */
> > > + 41, /* RTC_ALARM */
> > > + 2, /* RTC_TAMP_STAMP2/CSS_LSE */
> > > + 3, /* RTC wakeup timer */
> > > + 63, /* COMP1 */
> > > + 63, /* COMP2 */
COMP[1-2] -> #63
> > > + 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 */
> > > +};
> >
> > > + busdev = SYS_BUS_DEVICE(&s->exti);
> > > + if (!sysbus_realize(busdev, errp)) {
> > > + return;
> > > + }
> > > + sysbus_mmio_map(busdev, 0, EXTI_ADDR);
> > > + for (unsigned i = 0; i < NUM_EXTI_IRQ; i++) {
> > > + sysbus_connect_irq(busdev, i, qdev_get_gpio_in(armv7m, exti_irq[i]));
> >
> > ^^^^^^^^^^
> > > + }
> > Regards,
> >
> > Phil.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2024-02-08 12:36 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-09 16:06 [PATCH v8 0/3] Add device STM32L4x5 EXTI Inès Varhol
2024-01-09 16:06 ` [PATCH v8 1/3] hw/misc: Implement " Inès Varhol
2024-01-09 17:13 ` Philippe Mathieu-Daudé
2024-01-09 16:06 ` [PATCH v8 2/3] hw/arm: Connect STM32L4x5 EXTI to STM32L4x5 SoC Inès Varhol
2024-02-07 22:02 ` Philippe Mathieu-Daudé
2024-02-08 10:34 ` Inès Varhol
2024-02-08 12:35 ` Philippe Mathieu-Daudé
2024-01-09 16:06 ` [PATCH v8 3/3] tests/qtest: Add STM32L4x5 EXTI QTest testcase Inès Varhol
2024-01-12 18:13 ` [PATCH v8 0/3] Add device STM32L4x5 EXTI Peter Maydell
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.