Re: [PATCH 1/3] hw/gpio: Implement STM32L4x5 GPIO

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

Hello Inès,

On 22/1/24 10:18, Inès Varhol wrote:
> Features supported :
> - the 8 STM32L4x5 GPIOs are initialized with their reset values
>      (except IDR, see below)
> - input mode : setting a pin in input mode "externally" (using input
>      irqs) results in an out irq (transmitted to SYSCFG)
> - output mode : setting a bit in ODR sets the corresponding out irq
>      (if this line is configured in output mode)
> - pull-up, pull-down
> - push-pull, open-drain
> 
> Difference with the real GPIOs :
> - Alternate Function and Analog mode aren't implemented :
>      pins in AF/Analog behave like pins in input mode
> - floating pins stay at their last value
> - register IDR reset values differ from the real one :
>      values are coherent with the other registers reset values
>      and the fact that AF/Analog modes aren't implemented
> - setting I/O output speed isn't supported
> - locking port bits isn't supported
> - ADC function isn't supported
> - GPIOH has 16 pins instead of 2 pins
> - writing to registers LCKR, AFRL, AFRH and ASCR is ineffective
> 
> Signed-off-by: Arnaud Minier <arnaud.minier@telecom-paris.fr>
> Signed-off-by: Inès Varhol <ines.varhol@telecom-paris.fr>
> ---
>   MAINTAINERS                        |   1 +
>   docs/system/arm/b-l475e-iot01a.rst |   2 +-
>   hw/gpio/Kconfig                    |   3 +
>   hw/gpio/meson.build                |   1 +
>   hw/gpio/stm32l4x5_gpio.c           | 537 +++++++++++++++++++++++++++++
>   hw/gpio/trace-events               |   6 +
>   include/hw/gpio/stm32l4x5_gpio.h   |  80 +++++
>   7 files changed, 629 insertions(+), 1 deletion(-)
>   create mode 100644 hw/gpio/stm32l4x5_gpio.c
>   create mode 100644 include/hw/gpio/stm32l4x5_gpio.h


> +static void stm32l4x5_gpio_a_class_init(ObjectClass *klass, void *data)
> +{
> +    Stm32l4x5GpioClass *sc = STM32L4X5_GPIO_CLASS(klass);
> +
> +    sc->moder_reset = 0xABFFFFFF;
> +    sc->ospeedr_reset = 0x0C000000;
> +    sc->pupdr_reset = 0x64000000;
> +}
> +
> +static void stm32l4x5_gpio_b_class_init(ObjectClass *klass, void *data)
> +{
> +    Stm32l4x5GpioClass *sc = STM32L4X5_GPIO_CLASS(klass);
> +
> +    sc->moder_reset = 0xFFFFFEBF;
> +    sc->ospeedr_reset = 0x00000000;
> +    sc->pupdr_reset = 0x00000100;
> +}
> +
> +static void stm32l4x5_gpio_c_class_init(ObjectClass *klass, void *data)
> +{
> +    Stm32l4x5GpioClass *sc = STM32L4X5_GPIO_CLASS(klass);
> +
> +    sc->moder_reset = 0xFFFFFFFF;
> +    sc->ospeedr_reset = 0x00000000;
> +    sc->pupdr_reset = 0x00000000;
> +}
> +
> +static void stm32l4x5_gpio_d_class_init(ObjectClass *klass, void *data)
> +{
> +    Stm32l4x5GpioClass *sc = STM32L4X5_GPIO_CLASS(klass);
> +
> +    sc->moder_reset = 0xFFFFFFFF;
> +    sc->ospeedr_reset = 0x00000000;
> +    sc->pupdr_reset = 0x00000000;
> +}
> +
> +static void stm32l4x5_gpio_e_class_init(ObjectClass *klass, void *data)
> +{
> +    Stm32l4x5GpioClass *sc = STM32L4X5_GPIO_CLASS(klass);
> +
> +    sc->moder_reset = 0xFFFFFFFF;
> +    sc->ospeedr_reset = 0x00000000;
> +    sc->pupdr_reset = 0x00000000;
> +}
> +
> +static void stm32l4x5_gpio_f_class_init(ObjectClass *klass, void *data)
> +{
> +    Stm32l4x5GpioClass *sc = STM32L4X5_GPIO_CLASS(klass);
> +
> +    sc->moder_reset = 0xFFFFFFFF;
> +    sc->ospeedr_reset = 0x00000000;
> +    sc->pupdr_reset = 0x00000000;
> +}
> +
> +static void stm32l4x5_gpio_g_class_init(ObjectClass *klass, void *data)
> +{
> +    Stm32l4x5GpioClass *sc = STM32L4X5_GPIO_CLASS(klass);
> +
> +    sc->moder_reset = 0xFFFFFFFF;
> +    sc->ospeedr_reset = 0x00000000;
> +    sc->pupdr_reset = 0x00000000;
> +}
> +
> +static void stm32l4x5_gpio_h_class_init(ObjectClass *klass, void *data)
> +{
> +    Stm32l4x5GpioClass *sc = STM32L4X5_GPIO_CLASS(klass);
> +
> +    sc->moder_reset = 0x0000000F;
> +    sc->ospeedr_reset = 0x00000000;
> +    sc->pupdr_reset = 0x00000000;
> +}
> +
> +static const TypeInfo stm32l4x5_gpio_types[] = {
> +    {
> +        .name = TYPE_STM32L4X5_GPIO,
> +        .parent = TYPE_SYS_BUS_DEVICE,
> +        .instance_size = sizeof(Stm32l4x5GpioState),
> +        .instance_init = stm32l4x5_gpio_init,
> +        .class_size     = sizeof(Stm32l4x5GpioClass),
> +        .class_init = stm32l4x5_gpio_class_init,
> +        .abstract = true,
> +    }, {
> +        .name = TYPE_STM32L4X5_GPIO_A,
> +        .parent = TYPE_STM32L4X5_GPIO,
> +        .class_init = stm32l4x5_gpio_a_class_init,
> +    }, {
> +        .name = TYPE_STM32L4X5_GPIO_B,
> +        .parent = TYPE_STM32L4X5_GPIO,
> +        .class_init = stm32l4x5_gpio_b_class_init,
> +    }, {
> +        .name = TYPE_STM32L4X5_GPIO_C,
> +        .parent = TYPE_STM32L4X5_GPIO,
> +        .class_init = stm32l4x5_gpio_c_class_init,
> +    }, {
> +        .name = TYPE_STM32L4X5_GPIO_D,
> +        .parent = TYPE_STM32L4X5_GPIO,
> +        .class_init = stm32l4x5_gpio_d_class_init,
> +    }, {

My understanding is the same GPIO block is used for the STM32L4*
family, and the reset values are specific to each SoC.

Therefore I'd keep a generic GPIO model in this file, and set the
reset values in stm32l4x5_soc.c, likely stm32l4x5_soc_realize().

Something like:

static const struct {
     uint32_t moder;
     uint32_t ospeedr;
     uint32_t pupdr;
} stm32l4x5_gpio_initval[NUM_GPIOS] = {
     { 0xABFFFFFF, 0x0C000000, 0x64000000 },
     { 0xFFFFFEBF, 0x00000000, 0x00000100 },
     { 0xFFFFFFFF, 0x00000000, 0x00000000 },
     ...
};

   for (unsigned i = 0; i < NUM_GPIOS; i++) {
       DeviceState *dev = DEVICE(&s->gpio[i]);
       g_autofree char *name = g_strdup_printf("%c", 'A' + i);

       qdev_prop_set_string(dev, "name", name);
       qdev_prop_set_uint32(dev, "mode-reset",
                            stm32l4x5_gpio_initval[i].moder);
       qdev_prop_set_uint32(dev, "ospeed-reset",
                            stm32l4x5_gpio_initval[i].ospeedr);
       qdev_prop_set_uint32(dev, "pupd-reset",
                            stm32l4x5_gpio_initval[i].pupdr);
       ...
   }

(Having "mode-reset", "ospeed-reset", "pupd-reset" exposed as
  DEFINE_PROP_UINT32() properties).

Regards,

Phil.