Re: [PATCH for-6.2 12/25] hw/arm/stm32f205: Wire up sysclk and refclk

[Luc Michel]

On 10:33 Thu 12 Aug     , Peter Maydell wrote:
> Wire up the sysclk and refclk for the stm32f205 SoC.  This SoC always
> runs the systick refclk at 1/8 the frequency of the main CPU clock,
> so the board code only needs to provide a single sysclk clock.
> 
> Because there is only one board using this SoC, we convert the SoC
> and the board together, rather than splitting it into "add clock to
> SoC; connect clock in board; add error check in SoC code that clock
> is wired up".
> 
> When the systick device starts honouring its clock inputs, this will
> fix an emulation inaccuracy in the netduino2 board where the systick
> reference clock was running at 1MHz rather than 15MHz.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Luc Michel <luc@lmichel.fr>

> ---
>  include/hw/arm/stm32f205_soc.h |  4 ++++
>  hw/arm/netduino2.c             | 12 +++++++-----
>  hw/arm/stm32f205_soc.c         | 30 ++++++++++++++++++++++++++++++
>  3 files changed, 41 insertions(+), 5 deletions(-)
> 
> diff --git a/include/hw/arm/stm32f205_soc.h b/include/hw/arm/stm32f205_soc.h
> index 75251494917..849d3ed8891 100644
> --- a/include/hw/arm/stm32f205_soc.h
> +++ b/include/hw/arm/stm32f205_soc.h
> @@ -32,6 +32,7 @@
>  #include "hw/or-irq.h"
>  #include "hw/ssi/stm32f2xx_spi.h"
>  #include "hw/arm/armv7m.h"
> +#include "hw/clock.h"
>  #include "qom/object.h"
>  
>  #define TYPE_STM32F205_SOC "stm32f205-soc"
> @@ -67,6 +68,9 @@ struct STM32F205State {
>      MemoryRegion sram;
>      MemoryRegion flash;
>      MemoryRegion flash_alias;
> +
> +    Clock *sysclk;
> +    Clock *refclk;
>  };
>  
>  #endif
> diff --git a/hw/arm/netduino2.c b/hw/arm/netduino2.c
> index 1733b71507c..b5c0ba23ee5 100644
> --- a/hw/arm/netduino2.c
> +++ b/hw/arm/netduino2.c
> @@ -26,6 +26,7 @@
>  #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/stm32f205_soc.h"
>  #include "hw/arm/boot.h"
> @@ -36,16 +37,17 @@
>  static void netduino2_init(MachineState *machine)
>  {
>      DeviceState *dev;
> +    Clock *sysclk;
>  
> -    /*
> -     * TODO: ideally we would model the SoC RCC and let it handle
> -     * system_clock_scale, including its ability to define different
> -     * possible SYSCLK sources.
> -     */
>      system_clock_scale = NANOSECONDS_PER_SECOND / SYSCLK_FRQ;
>  
> +    /* 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_STM32F205_SOC);
>      qdev_prop_set_string(dev, "cpu-type", ARM_CPU_TYPE_NAME("cortex-m3"));
> +    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,
> diff --git a/hw/arm/stm32f205_soc.c b/hw/arm/stm32f205_soc.c
> index 0bd215aebd7..c6b75a381d9 100644
> --- a/hw/arm/stm32f205_soc.c
> +++ b/hw/arm/stm32f205_soc.c
> @@ -29,6 +29,7 @@
>  #include "exec/address-spaces.h"
>  #include "hw/arm/stm32f205_soc.h"
>  #include "hw/qdev-properties.h"
> +#include "hw/qdev-clock.h"
>  #include "sysemu/sysemu.h"
>  
>  /* At the moment only Timer 2 to 5 are modelled */
> @@ -74,6 +75,9 @@ static void stm32f205_soc_initfn(Object *obj)
>      for (i = 0; i < STM_NUM_SPIS; i++) {
>          object_initialize_child(obj, "spi[*]", &s->spi[i], 
> TYPE_STM32F2XX_SPI);
>      }
> +
> +    s->sysclk = qdev_init_clock_in(DEVICE(s), "sysclk", NULL, NULL, 0);
> +    s->refclk = qdev_init_clock_in(DEVICE(s), "refclk", NULL, NULL, 0);
>  }
>  
>  static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
> @@ -85,6 +89,30 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, 
> Error **errp)
>  
>      MemoryRegion *system_memory = get_system_memory();
>  
> +    /*
> +     * We use s->refclk internally and only define it with 
> qdev_init_clock_in()
> +     * so it is correctly parented and not leaked on an init/deinit; it is 
> not
> +     * intended as an externally exposed clock.
> +     */
> +    if (clock_has_source(s->refclk)) {
> +        error_setg(errp, "refclk clock must not be wired up by the board 
> code");
> +        return;
> +    }
> +
> +    if (!clock_has_source(s->sysclk)) {
> +        error_setg(errp, "sysclk clock must be wired up by the board code");
> +        return;
> +    }
> +
> +    /*
> +     * TODO: ideally we should model the SoC RCC and its ability to
> +     * change the sysclk frequency and define different sysclk sources.
> +     */
> +
> +    /* The refclk always runs at frequency HCLK / 8 */
> +    clock_set_mul_div(s->refclk, 8, 1);
> +    clock_set_source(s->refclk, s->sysclk);
> +
>      memory_region_init_rom(&s->flash, OBJECT(dev_soc), "STM32F205.flash",
>                             FLASH_SIZE, &error_fatal);
>      memory_region_init_alias(&s->flash_alias, OBJECT(dev_soc),
> @@ -101,6 +129,8 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, 
> Error **errp)
>      qdev_prop_set_uint32(armv7m, "num-irq", 96);
>      qdev_prop_set_string(armv7m, "cpu-type", s->cpu_type);
>      qdev_prop_set_bit(armv7m, "enable-bitband", true);
> +    qdev_connect_clock_in(armv7m, "cpuclk", s->sysclk);
> +    qdev_connect_clock_in(armv7m, "refclk", s->refclk);
>      object_property_set_link(OBJECT(&s->armv7m), "memory",
>                               OBJECT(get_system_memory()), &error_abort);
>      if (!sysbus_realize(SYS_BUS_DEVICE(&s->armv7m), errp)) {
> -- 
> 2.20.1
> 

--