| Message ID | 20240109160658.311932-3-ines.varhol@telecom-paris.fr (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add device STM32L4x5 EXTI | expand |
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.
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.
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.
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;