Re: [PATCH v2 5/6] hw/arm: Add the USART to the stm32l4x5 SoC

[Peter Maydell]

On Sun, 24 Mar 2024 at 16:57, Arnaud Minier
<arnaud.minier@telecom-paris.fr> wrote:
>
> Add the USART to the SoC and connect it to the other implemented devices.
>
> Signed-off-by: Arnaud Minier <arnaud.minier@telecom-paris.fr>
> Signed-off-by: Inès Varhol <ines.varhol@telecom-paris.fr>
> ---
>  docs/system/arm/b-l475e-iot01a.rst |  2 +-
>  hw/arm/Kconfig                     |  1 +
>  hw/arm/stm32l4x5_soc.c             | 82 +++++++++++++++++++++++++++---
>  include/hw/arm/stm32l4x5_soc.h     | 13 +++++
>  4 files changed, 91 insertions(+), 7 deletions(-)
>
> diff --git a/docs/system/arm/b-l475e-iot01a.rst 
> b/docs/system/arm/b-l475e-iot01a.rst
> index 0afef8e4f4..a76c9976c5 100644
> --- a/docs/system/arm/b-l475e-iot01a.rst
> +++ b/docs/system/arm/b-l475e-iot01a.rst
> @@ -19,13 +19,13 @@ Currently B-L475E-IOT01A machine's only supports the 
> following devices:
>  - STM32L4x5 SYSCFG (System configuration controller)
>  - STM32L4x5 RCC (Reset and clock control)
>  - STM32L4x5 GPIOs (General-purpose I/Os)
> +- STM32L4x5 USARTs, UARTs and LPUART (Serial ports)
>
>  Missing devices
>  """""""""""""""
>
>  The B-L475E-IOT01A does *not* support the following devices:
>
> -- Serial ports (UART)
>  - Analog to Digital Converter (ADC)
>  - SPI controller
>  - Timer controller (TIMER)
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index 893a7bff66..098d043375 100644
> --- a/hw/arm/Kconfig
> +++ b/hw/arm/Kconfig
> @@ -477,6 +477,7 @@ config STM32L4X5_SOC
>      select STM32L4X5_SYSCFG
>      select STM32L4X5_RCC
>      select STM32L4X5_GPIO
> +    select STM32L4X5_USART
>
>  config XLNX_ZYNQMP_ARM
>      bool
> diff --git a/hw/arm/stm32l4x5_soc.c b/hw/arm/stm32l4x5_soc.c
> index 40e294f838..ae0868dcab 100644
> --- a/hw/arm/stm32l4x5_soc.c
> +++ b/hw/arm/stm32l4x5_soc.c
> @@ -28,6 +28,7 @@
>  #include "sysemu/sysemu.h"
>  #include "hw/or-irq.h"
>  #include "hw/arm/stm32l4x5_soc.h"
> +#include "hw/char/stm32l4x5_usart.h"
>  #include "hw/gpio/stm32l4x5_gpio.h"
>  #include "hw/qdev-clock.h"
>  #include "hw/misc/unimp.h"
> @@ -116,6 +117,22 @@ static const struct {
>      { 0x48001C00, 0x0000000F, 0x00000000, 0x00000000 },
>  };
>
> +static const hwaddr usart_addr[] = {
> +    0x40013800, /* "USART1", 0x400 */
> +    0x40004400, /* "USART2", 0x400 */
> +    0x40004800, /* "USART3", 0x400 */
> +};
> +static const hwaddr uart_addr[] = {
> +    0x40004C00, /* "UART4" , 0x400 */
> +    0x40005000  /* "UART5" , 0x400 */
> +};
> +
> +#define LPUART_BASE_ADDRESS 0x40008000
> +
> +static const int usart_irq[] = { 37, 38, 39 };
> +static const int uart_irq[] = { 52, 53 };
> +#define LPUART_IRQ 70
> +
>  static void stm32l4x5_soc_initfn(Object *obj)
>  {
>      Stm32l4x5SocState *s = STM32L4X5_SOC(obj);
> @@ -132,6 +149,18 @@ static void stm32l4x5_soc_initfn(Object *obj)
>          g_autofree char *name = g_strdup_printf("gpio%c", 'a' + i);
>          object_initialize_child(obj, name, &s->gpio[i], TYPE_STM32L4X5_GPIO);
>      }
> +
> +    for (int i = 0; i < STM_NUM_USARTS; i++) {
> +        object_initialize_child(obj, "usart[*]", &s->usart[i],
> +                                TYPE_STM32L4X5_USART);
> +    }
> +
> +    for (int i = 0; i < STM_NUM_UARTS; i++) {
> +        object_initialize_child(obj, "uart[*]", &s->uart[i],
> +                                TYPE_STM32L4X5_UART);
> +    }
> +    object_initialize_child(obj, "lpuart1", &s->lpuart,
> +                            TYPE_STM32L4X5_LPUART);
>  }
>
>  static void stm32l4x5_soc_realize(DeviceState *dev_soc, Error **errp)
> @@ -279,6 +308,53 @@ 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));
> +        g_autofree char *name = g_strdup_printf("usart%d-out", i + 1);

Variable declarations should always be at the start of a code block.
Similarly below.

> +        qdev_connect_clock_in(dev, "clk",
> +            qdev_get_clock_out(DEVICE(&(s->rcc)), name));
> +        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));
> +        g_autofree char *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));
> +        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);
> @@ -294,10 +370,6 @@ static void stm32l4x5_soc_realize(DeviceState *dev_soc, 
> Error **errp)
>      create_unimplemented_device("SPI2",      0x40003800, 0x400);
>      create_unimplemented_device("SPI3",      0x40003C00, 0x400);
>      /* RESERVED:    0x40004000, 0x400 */
> -    create_unimplemented_device("USART2",    0x40004400, 0x400);
> -    create_unimplemented_device("USART3",    0x40004800, 0x400);
> -    create_unimplemented_device("UART4",     0x40004C00, 0x400);
> -    create_unimplemented_device("UART5",     0x40005000, 0x400);
>      create_unimplemented_device("I2C1",      0x40005400, 0x400);
>      create_unimplemented_device("I2C2",      0x40005800, 0x400);
>      create_unimplemented_device("I2C3",      0x40005C00, 0x400);
> @@ -308,7 +380,6 @@ static void stm32l4x5_soc_realize(DeviceState *dev_soc, 
> Error **errp)
>      create_unimplemented_device("DAC1",      0x40007400, 0x400);
>      create_unimplemented_device("OPAMP",     0x40007800, 0x400);
>      create_unimplemented_device("LPTIM1",    0x40007C00, 0x400);
> -    create_unimplemented_device("LPUART1",   0x40008000, 0x400);
>      /* RESERVED:    0x40008400, 0x400 */
>      create_unimplemented_device("SWPMI1",    0x40008800, 0x400);
>      /* RESERVED:    0x40008C00, 0x800 */
> @@ -325,7 +396,6 @@ static void stm32l4x5_soc_realize(DeviceState *dev_soc, 
> Error **errp)
>      create_unimplemented_device("TIM1",      0x40012C00, 0x400);
>      create_unimplemented_device("SPI1",      0x40013000, 0x400);
>      create_unimplemented_device("TIM8",      0x40013400, 0x400);
> -    create_unimplemented_device("USART1",    0x40013800, 0x400);
>      /* RESERVED:    0x40013C00, 0x400 */
>      create_unimplemented_device("TIM15",     0x40014000, 0x400);
>      create_unimplemented_device("TIM16",     0x40014400, 0x400);
> diff --git a/include/hw/arm/stm32l4x5_soc.h b/include/hw/arm/stm32l4x5_soc.h
> index ee5f362405..a94ddbd19c 100644
> --- a/include/hw/arm/stm32l4x5_soc.h
> +++ b/include/hw/arm/stm32l4x5_soc.h
> @@ -21,6 +21,12 @@
>   * 
> https://www.st.com/en/microcontrollers-microprocessors/stm32l4x5/documentation.html
>   */
>
> +/*
> + * The STM32L4X5 is heavily inspired by the stm32f405 by Alistair Francis.
> + * The reference used is the STMicroElectronics RM0351 Reference manual
> + * for STM32L4x5 and STM32L4x6 advanced Arm ® -based 32-bit MCUs.
> + */
> +

Doesn't the comment above with the URL already have this
text about the RM0351 manual ?

>  #ifndef HW_ARM_STM32L4x5_SOC_H
>  #define HW_ARM_STM32L4x5_SOC_H

Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM