| Message ID | 20240212163446.227802-2-ines.varhol@telecom-paris.fr |
|---|---|
| State | New |
| Headers | show |
| Series | hw/arm: Fix STM32L4x5 EXTI to CPU irq fan-in connections | expand |
On Mon, 12 Feb 2024 at 16:34, Inès Varhol <ines.varhol@telecom-paris.fr> wrote: > > Fixes: 52671f69f7a4 ("[PATCH v8 0/3] Add device STM32L4x5 EXTI") > Signed-off-by: Inès Varhol <ines.varhol@telecom-paris.fr> > --- > hw/arm/stm32l4x5_soc.c | 69 ++++++++++++++++++++++++++++++++++++------ > 1 file changed, 59 insertions(+), 10 deletions(-) > > diff --git a/hw/arm/stm32l4x5_soc.c b/hw/arm/stm32l4x5_soc.c > index f470ff74ec..df5bb02315 100644 > --- a/hw/arm/stm32l4x5_soc.c > +++ b/hw/arm/stm32l4x5_soc.c > @@ -26,6 +26,7 @@ > #include "qapi/error.h" > #include "exec/address-spaces.h" > #include "sysemu/sysemu.h" > +#include "hw/or-irq.h" > #include "hw/arm/stm32l4x5_soc.h" > #include "hw/qdev-clock.h" > #include "hw/misc/unimp.h" > @@ -48,15 +49,14 @@ static const int exti_irq[NUM_EXTI_IRQ] = { > 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 */ > + -1, -1, -1, -1, -1, /* GPIO[5..9] OR gate 23 */ > + -1, -1, -1, -1, -1, -1, /* GPIO[10..15] OR gate 40 */ > + -1, /* PVD OR gate 1 */ > 67, /* OTG_FS_WKUP, Direct */ > 41, /* RTC_ALARM */ > 2, /* RTC_TAMP_STAMP2/CSS_LSE */ > 3, /* RTC wakeup timer */ > - 63, /* COMP1 */ > - 63, /* COMP2 */ > + -1, -1, /* COMP[1..2] OR gate 63 */ > 31, /* I2C1 wakeup, Direct */ > 33, /* I2C2 wakeup, Direct */ > 72, /* I2C3 wakeup, Direct */ > @@ -69,13 +69,29 @@ static const int exti_irq[NUM_EXTI_IRQ] = { > 65, /* LPTIM1, Direct */ > 66, /* LPTIM2, Direct */ > 76, /* SWPMI1 wakeup, Direct */ > - 1, /* PVM1 wakeup */ > - 1, /* PVM2 wakeup */ > - 1, /* PVM3 wakeup */ > - 1, /* PVM4 wakeup */ > + -1, -1, -1, -1, /* PVM[1..4] OR gate 1 */ > 78 /* LCD wakeup, Direct */ > }; > > +#define NUM_EXTI_OR_GATES 4 > +static const int exti_or_gates_out[NUM_EXTI_OR_GATES] = { > + 23, 40, 63, 1, > +}; > + > +#define NUM_EXTI_SIMPLE_FANIN_IRQ 3 > +static const int exti_or_gates_num_lines_in[NUM_EXTI_SIMPLE_FANIN_IRQ] = { > + 5, 6, 2, > +}; This array only has three elements, but you always set the num-lines property on the OR gate using this array for all NUM_EXTI_OR_GATES gates. I think the array should be size NUM_EXTI_OR_GATES and have an extra element '5' at the end. > + > +static const int exti_or_gates_first_line_in[NUM_EXTI_SIMPLE_FANIN_IRQ] = { > + 5, 10, 21, > +}; > + > +#define NUM_EXTI_OR_GATE1_NUM_LINES_IN 5 > +static const int exti_or_gate1_lines_in[NUM_EXTI_OR_GATE1_NUM_LINES_IN] = { > + 16, 35, 36, 37, 38, > +}; > + > static void stm32l4x5_soc_initfn(Object *obj) > { > Stm32l4x5SocState *s = STM32L4X5_SOC(obj); > @@ -175,8 +191,41 @@ static void stm32l4x5_soc_realize(DeviceState *dev_soc, Error **errp) > return; > } > sysbus_mmio_map(busdev, 0, EXTI_ADDR); > + > + /* IRQs with fan-in that require an OR gate */ > + for (unsigned i = 0; i < NUM_EXTI_OR_GATES; i++) { > + Object *orgate = object_new(TYPE_OR_IRQ); > + object_property_set_int(orgate, "num-lines", > + exti_or_gates_num_lines_in[i], &error_fatal); > + /* Should unref be used? */ > + qdev_realize(DEVICE(orgate), NULL, &error_fatal); There are two patterns for create-and-realize: (1) create with qdev_new() etc, realize with qdev_realize_and_unref() (2) create with object_initialize_child(), realize with qdev_realize() (the doc comments in qdev-core.h try to explain these two patterns.) This is an SoC container object, so it's better to use the pattern where the sub-devices of it (like these OR gates) are given fields in the SoC's state struct, initialized with object_initialize_child() in the SoC init function, and then realized here with qdev_realize(). There's an example of doing it this way in hw/arm/armsse.c. > + > + qdev_connect_gpio_out(DEVICE(orgate), 0, > + qdev_get_gpio_in(armv7m, exti_or_gates_out[i])); > + > + /* consecutive inputs for OR gates 23, 40, 63 */ > + if (i < NUM_EXTI_SIMPLE_FANIN_IRQ) { > + for (unsigned j = 0; j < exti_or_gates_num_lines_in[i]; j++) { > + sysbus_connect_irq(SYS_BUS_DEVICE(&s->exti), > + exti_or_gates_first_line_in[i] + j, > + qdev_get_gpio_in(DEVICE(orgate), j)); > + } > + /* non-consecutive inputs for OR gate 1 */ This comment would be better inside the block of the 'else', I think. > + } else { > + for (unsigned j = 0; j < NUM_EXTI_OR_GATE1_NUM_LINES_IN; j++) { > + sysbus_connect_irq(SYS_BUS_DEVICE(&s->exti), > + exti_or_gate1_lines_in[j], > + qdev_get_gpio_in(DEVICE(orgate), j)); > + } > + } > + } > + > + /* IRQs that don't require fan-in */ > for (unsigned i = 0; i < NUM_EXTI_IRQ; i++) { > - sysbus_connect_irq(busdev, i, qdev_get_gpio_in(armv7m, exti_irq[i])); > + if (exti_irq[i] != -1) { > + sysbus_connect_irq(busdev, i, > + qdev_get_gpio_in(armv7m, exti_irq[i])); > + } > } > > for (unsigned i = 0; i < 16; i++) { > -- thanks -- PMM
diff --git a/hw/arm/stm32l4x5_soc.c b/hw/arm/stm32l4x5_soc.c index f470ff74ec..df5bb02315 100644 --- a/hw/arm/stm32l4x5_soc.c +++ b/hw/arm/stm32l4x5_soc.c @@ -26,6 +26,7 @@ #include "qapi/error.h" #include "exec/address-spaces.h" #include "sysemu/sysemu.h" +#include "hw/or-irq.h" #include "hw/arm/stm32l4x5_soc.h" #include "hw/qdev-clock.h" #include "hw/misc/unimp.h" @@ -48,15 +49,14 @@ static const int exti_irq[NUM_EXTI_IRQ] = { 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 */ + -1, -1, -1, -1, -1, /* GPIO[5..9] OR gate 23 */ + -1, -1, -1, -1, -1, -1, /* GPIO[10..15] OR gate 40 */ + -1, /* PVD OR gate 1 */ 67, /* OTG_FS_WKUP, Direct */ 41, /* RTC_ALARM */ 2, /* RTC_TAMP_STAMP2/CSS_LSE */ 3, /* RTC wakeup timer */ - 63, /* COMP1 */ - 63, /* COMP2 */ + -1, -1, /* COMP[1..2] OR gate 63 */ 31, /* I2C1 wakeup, Direct */ 33, /* I2C2 wakeup, Direct */ 72, /* I2C3 wakeup, Direct */ @@ -69,13 +69,29 @@ static const int exti_irq[NUM_EXTI_IRQ] = { 65, /* LPTIM1, Direct */ 66, /* LPTIM2, Direct */ 76, /* SWPMI1 wakeup, Direct */ - 1, /* PVM1 wakeup */ - 1, /* PVM2 wakeup */ - 1, /* PVM3 wakeup */ - 1, /* PVM4 wakeup */ + -1, -1, -1, -1, /* PVM[1..4] OR gate 1 */ 78 /* LCD wakeup, Direct */ }; +#define NUM_EXTI_OR_GATES 4 +static const int exti_or_gates_out[NUM_EXTI_OR_GATES] = { + 23, 40, 63, 1, +}; + +#define NUM_EXTI_SIMPLE_FANIN_IRQ 3 +static const int exti_or_gates_num_lines_in[NUM_EXTI_SIMPLE_FANIN_IRQ] = { + 5, 6, 2, +}; + +static const int exti_or_gates_first_line_in[NUM_EXTI_SIMPLE_FANIN_IRQ] = { + 5, 10, 21, +}; + +#define NUM_EXTI_OR_GATE1_NUM_LINES_IN 5 +static const int exti_or_gate1_lines_in[NUM_EXTI_OR_GATE1_NUM_LINES_IN] = { + 16, 35, 36, 37, 38, +}; + static void stm32l4x5_soc_initfn(Object *obj) { Stm32l4x5SocState *s = STM32L4X5_SOC(obj); @@ -175,8 +191,41 @@ static void stm32l4x5_soc_realize(DeviceState *dev_soc, Error **errp) return; } sysbus_mmio_map(busdev, 0, EXTI_ADDR); + + /* IRQs with fan-in that require an OR gate */ + for (unsigned i = 0; i < NUM_EXTI_OR_GATES; i++) { + Object *orgate = object_new(TYPE_OR_IRQ); + object_property_set_int(orgate, "num-lines", + exti_or_gates_num_lines_in[i], &error_fatal); + /* Should unref be used? */ + qdev_realize(DEVICE(orgate), NULL, &error_fatal); + + qdev_connect_gpio_out(DEVICE(orgate), 0, + qdev_get_gpio_in(armv7m, exti_or_gates_out[i])); + + /* consecutive inputs for OR gates 23, 40, 63 */ + if (i < NUM_EXTI_SIMPLE_FANIN_IRQ) { + for (unsigned j = 0; j < exti_or_gates_num_lines_in[i]; j++) { + sysbus_connect_irq(SYS_BUS_DEVICE(&s->exti), + exti_or_gates_first_line_in[i] + j, + qdev_get_gpio_in(DEVICE(orgate), j)); + } + /* non-consecutive inputs for OR gate 1 */ + } else { + for (unsigned j = 0; j < NUM_EXTI_OR_GATE1_NUM_LINES_IN; j++) { + sysbus_connect_irq(SYS_BUS_DEVICE(&s->exti), + exti_or_gate1_lines_in[j], + qdev_get_gpio_in(DEVICE(orgate), j)); + } + } + } + + /* IRQs that don't require fan-in */ for (unsigned i = 0; i < NUM_EXTI_IRQ; i++) { - sysbus_connect_irq(busdev, i, qdev_get_gpio_in(armv7m, exti_irq[i])); + if (exti_irq[i] != -1) { + sysbus_connect_irq(busdev, i, + qdev_get_gpio_in(armv7m, exti_irq[i])); + } } for (unsigned i = 0; i < 16; i++) {
Fixes: 52671f69f7a4 ("[PATCH v8 0/3] Add device STM32L4x5 EXTI") Signed-off-by: Inès Varhol <ines.varhol@telecom-paris.fr> --- hw/arm/stm32l4x5_soc.c | 69 ++++++++++++++++++++++++++++++++++++------ 1 file changed, 59 insertions(+), 10 deletions(-)