On Sun, 17 Mar 2024 at 10:42, Arnaud Minier
<arnaud.min...@telecom-paris.fr> wrote:
>
> Add the USART to the SoC and connect it to the other implemented devices.
>
> Signed-off-by: Arnaud Minier <arnaud.min...@telecom-paris.fr>
> Signed-off-by: Inès Varhol <ines.var...@telecom-paris.fr>
> @@ -143,6 +172,7 @@ static void stm32l4x5_soc_realize(DeviceState *dev_soc,
> Error **errp)
> DeviceState *armv7m, *dev;
> SysBusDevice *busdev;
> uint32_t pin_index;
> + g_autofree char *name;
The idea with g_autofree is that you put it in a variable scope
which is the right size so that the automatic free on going out
of the scope is all that you need for freeing it. You've put this
one at the top function level, which then means you needed
to add extra manual g_free() calls.
>
> if (!memory_region_init_rom(&s->flash, OBJECT(dev_soc), "flash",
> sc->flash_size, errp)) {
> @@ -185,7 +215,7 @@ static void stm32l4x5_soc_realize(DeviceState *dev_soc,
> Error **errp)
>
> /* GPIOs */
> for (unsigned i = 0; i < NUM_GPIOS; i++) {
> - g_autofree char *name = g_strdup_printf("%c", 'A' + i);
> + name = g_strdup_printf("%c", 'A' + i);
> dev = DEVICE(&s->gpio[i]);
> qdev_prop_set_string(dev, "name", name);
> qdev_prop_set_uint32(dev, "mode-reset",
> @@ -199,6 +229,7 @@ static void stm32l4x5_soc_realize(DeviceState *dev_soc,
> Error **errp)
> name = g_strdup_printf("gpio%c-out", 'a' + i);
> qdev_connect_clock_in(DEVICE(&s->gpio[i]), "clk",
> qdev_get_clock_out(DEVICE(&(s->rcc)), name));
> + g_free(name);
> if (!sysbus_realize(busdev, errp)) {
> return;
> }
For instance this code was correctly using the g_autofree, and
no longer is.
> @@ -279,6 +310,55 @@ static void stm32l4x5_soc_realize(DeviceState *dev_soc,
> Error **errp)
> sysbus_mmio_map(busdev, 0, RCC_BASE_ADDRESS);
> sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(armv7m, RCC_IRQ));
>
> + /* USART devices */
> + for (int i = 0; i < STM_NUM_USARTS; i++) {
> + dev = DEVICE(&(s->usart[i]));
> + qdev_prop_set_chr(dev, "chardev", serial_hd(i));
> + name = g_strdup_printf("usart%d-out", i + 1);
> + qdev_connect_clock_in(dev, "clk",
> + qdev_get_clock_out(DEVICE(&(s->rcc)), name));
> + g_free(name);
You should declare a new local g_autofree char *name inside
this loop body, and then the g_free() isn't needed.
> + busdev = SYS_BUS_DEVICE(dev);
> + if (!sysbus_realize(busdev, errp)) {
> + return;
> + }
> + sysbus_mmio_map(busdev, 0, usart_addr[i]);
> + sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(armv7m,
> usart_irq[i]));
> + }
> +
> + /*
> + * TODO: Connect the USARTs, UARTs and LPUART to the EXTI once the EXTI
> + * can handle other gpio-in than the gpios. (e.g. Direct Lines for the
> usarts)
> + */
> +
> + /* UART devices */
> + for (int i = 0; i < STM_NUM_UARTS; i++) {
> + dev = DEVICE(&(s->uart[i]));
> + qdev_prop_set_chr(dev, "chardev", serial_hd(STM_NUM_USARTS + i));
> + name = g_strdup_printf("uart%d-out", STM_NUM_USARTS + i + 1);
> + qdev_connect_clock_in(dev, "clk",
> + qdev_get_clock_out(DEVICE(&(s->rcc)), name));
> + g_free(name);
Similarly here.
> + busdev = SYS_BUS_DEVICE(dev);
> + if (!sysbus_realize(busdev, errp)) {
> + return;
> + }
> + sysbus_mmio_map(busdev, 0, uart_addr[i]);
> + sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(armv7m, uart_irq[i]));
> + }
> +
> + /* LPUART device*/
> + dev = DEVICE(&(s->lpuart));
> + qdev_prop_set_chr(dev, "chardev", serial_hd(STM_NUM_USARTS +
> STM_NUM_UARTS));
> + qdev_connect_clock_in(dev, "clk",
> + qdev_get_clock_out(DEVICE(&(s->rcc)), "lpuart1-out"));
> + busdev = SYS_BUS_DEVICE(dev);
> + if (!sysbus_realize(busdev, errp)) {
> + return;
> + }
> + sysbus_mmio_map(busdev, 0, LPUART_BASE_ADDRESS);
> + sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(armv7m, LPUART_IRQ));
> +
> /* APB1 BUS */
> create_unimplemented_device("TIM2", 0x40000000, 0x400);
> create_unimplemented_device("TIM3", 0x40000400, 0x400);
thanks
-- PMM
