Re: [PATCH v2 2/2] hw/arm: Add nucleo-g071rb board

[Peter Maydell]

On Wed, 20 Mar 2024 at 20:21, Felipe Balbi <balbi@kernel.org> wrote:
>
> This board is based around STM32G071RB SoC, a Cortex-M0 based
> device. More information can be found at:
>
> https://www.st.com/en/product/nucleo-g071rb.html

Could you put this URL in a comment in the source file too, please?

>
> Signed-off-by: Felipe Balbi <balbi@kernel.org>
> ---
>
> Changes since v1:
>
>     - Convert tabs to spaces (checkpatch.pl)
>
>  MAINTAINERS            |  6 ++++
>  hw/arm/Kconfig         |  6 ++++
>  hw/arm/meson.build     |  1 +
>  hw/arm/nucleo-g071rb.c | 70 ++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 83 insertions(+)
>  create mode 100644 hw/arm/nucleo-g071rb.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index bce2eb3ad70b..052ce4dcfb97 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1116,6 +1116,12 @@ L: qemu-arm@nongnu.org
>  S: Maintained
>  F: hw/arm/netduinoplus2.c
>
> +Nucleo G071RB
> +M: Felipe Balbi <balbi@kernel.org>
> +L: qemu-arm@nongnu.org
> +S: Maintained
> +F: hw/arm/nucleo-g071rb.c
> +
>  Olimex STM32 H405
>  M: Felipe Balbi <balbi@kernel.org>
>  L: qemu-arm@nongnu.org
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index 28a46d2b1ad3..5938bb8208a1 100644
> --- a/hw/arm/Kconfig
> +++ b/hw/arm/Kconfig
> @@ -310,6 +310,12 @@ config STM32VLDISCOVERY
>      depends on TCG && ARM
>      select STM32F100_SOC
>
> +config NUCLEO_G071RB
> +    bool
> +    default y
> +    depends on TCG && ARM
> +    select STM32G000_SOC
> +
>  config STRONGARM
>      bool
>      select PXA2XX
> diff --git a/hw/arm/meson.build b/hw/arm/meson.build
> index 9c4137a988e1..580c2d55fc3f 100644
> --- a/hw/arm/meson.build
> +++ b/hw/arm/meson.build
> @@ -18,6 +18,7 @@ arm_ss.add(when: 'CONFIG_REALVIEW', if_true: 
> files('realview.c'))
>  arm_ss.add(when: 'CONFIG_SBSA_REF', if_true: files('sbsa-ref.c'))
>  arm_ss.add(when: 'CONFIG_STELLARIS', if_true: files('stellaris.c'))
>  arm_ss.add(when: 'CONFIG_STM32VLDISCOVERY', if_true: 
> files('stm32vldiscovery.c'))
> +arm_ss.add(when: 'CONFIG_NUCLEO_G071RB', if_true: files('nucleo-g071rb.c'))
>  arm_ss.add(when: 'CONFIG_ZYNQ', if_true: files('xilinx_zynq.c'))
>  arm_ss.add(when: 'CONFIG_SABRELITE', if_true: files('sabrelite.c'))
>
> diff --git a/hw/arm/nucleo-g071rb.c b/hw/arm/nucleo-g071rb.c
> new file mode 100644
> index 000000000000..580b52bacf2c
> --- /dev/null
> +++ b/hw/arm/nucleo-g071rb.c
> @@ -0,0 +1,70 @@
> +/*
> + * ST Nucleo G071RB
> + *
> + * Copyright (c) 2024 Felipe Balbi <felipe@balbi.sh>
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a 
> copy
> + * of this software and associated documentation files (the "Software"), to 
> deal
> + * in the Software without restriction, including without limitation the 
> rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
> FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qapi/error.h"
> +#include "hw/boards.h"
> +#include "hw/qdev-properties.h"
> +#include "hw/qdev-clock.h"
> +#include "qemu/error-report.h"
> +#include "hw/arm/stm32g000_soc.h"
> +#include "hw/arm/boot.h"
> +
> +/* nucleo_g071rb implementation is derived from olimex-stm32-h405.c */
> +
> +/* Main SYSCLK frequency in Hz (48MHz) */
> +#define SYSCLK_FRQ 48000000ULL
> +
> +static void nucleo_g071rb_init(MachineState *machine)
> +{
> +    DeviceState *dev;
> +    Clock *sysclk;
> +
> +    /* This clock doesn't need migration because it is fixed-frequency */
> +    sysclk = clock_new(OBJECT(machine), "SYSCLK");
> +    clock_set_hz(sysclk, SYSCLK_FRQ);
> +
> +    dev = qdev_new(TYPE_STM32G000_SOC);
> +    object_property_add_child(OBJECT(machine), "soc", OBJECT(dev));
> +    qdev_connect_clock_in(dev, "sysclk", sysclk);
> +    sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
> +
> +    armv7m_load_kernel(ARM_CPU(first_cpu),
> +                       machine->kernel_filename,
> +                       0, FLASH_SIZE);
> +}
> +
> +static void nucleo_g071rb_machine_init(MachineClass *mc)
> +{
> +    static const char * const valid_cpu_types[] = {
> +        ARM_CPU_TYPE_NAME("cortex-m0"),
> +        NULL
> +    };
> +
> +    mc->desc = "ST Nucleo-G071RB (Cortex-M0)";
> +    mc->init = nucleo_g071rb_init;
> +    mc->valid_cpu_types = valid_cpu_types;
> +}

Like the olimex code, the SoC here allocates the SRAM in the
SoC instantiation, so I think we should set
mc->default_ram_size = 0 the same way that code does.

Otherwise this looks good.

thanks
-- PMM