Re: [PATCH v8 2/3] hw/arm: Connect STM32L4x5 EXTI to STM32L4x5 SoC

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

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.