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

["Inès Varhol" <ines.varhol@telecom-paris.fr>]

[-- 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 --]