On 8/12/21 11:33 AM, Peter Maydell wrote:
> Instead of passing the MSF2 SoC an integer property specifying the
> CPU clock rate, pass it a Clock instead. This lets us wire that
> clock up to the armv7m object.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> include/hw/arm/msf2-soc.h | 3 ++-
> hw/arm/msf2-soc.c | 28 +++++++++++++++++-----------
> hw/arm/msf2-som.c | 7 ++++++-
> 3 files changed, 25 insertions(+), 13 deletions(-)
>
> diff --git a/include/hw/arm/msf2-soc.h b/include/hw/arm/msf2-soc.h
> index 38e10ce20aa..01f904cec47 100644
> --- a/include/hw/arm/msf2-soc.h
> +++ b/include/hw/arm/msf2-soc.h
> @@ -30,6 +30,7 @@
> #include "hw/misc/msf2-sysreg.h"
> #include "hw/ssi/mss-spi.h"
> #include "hw/net/msf2-emac.h"
> +#include "hw/clock.h"
> #include "qom/object.h"
>
> #define TYPE_MSF2_SOC "msf2-soc"
> @@ -57,7 +58,7 @@ struct MSF2State {
> uint64_t envm_size;
> uint64_t esram_size;
>
> - uint32_t m3clk;
> + Clock *m3clk;
> uint8_t apb0div;
> uint8_t apb1div;
>
> diff --git a/hw/arm/msf2-soc.c b/hw/arm/msf2-soc.c
> index f36788054b3..0a1e594aee6 100644
> --- a/hw/arm/msf2-soc.c
> +++ b/hw/arm/msf2-soc.c
> @@ -29,6 +29,7 @@
> #include "hw/char/serial.h"
> #include "hw/arm/msf2-soc.h"
> #include "hw/misc/unimp.h"
> +#include "hw/qdev-clock.h"
> #include "sysemu/sysemu.h"
>
> #define MSF2_TIMER_BASE 0x40004000
> @@ -73,6 +74,8 @@ static void m2sxxx_soc_initfn(Object *obj)
> }
>
> object_initialize_child(obj, "emac", &s->emac, TYPE_MSS_EMAC);
> +
> + s->m3clk = qdev_init_clock_in(DEVICE(obj), "m3clk", NULL, NULL, 0);
> }
>
> static void m2sxxx_soc_realize(DeviceState *dev_soc, Error **errp)
> @@ -84,6 +87,11 @@ static void m2sxxx_soc_realize(DeviceState *dev_soc, Error **errp)
>
> MemoryRegion *system_memory = get_system_memory();
>
> + if (!clock_has_source(s->m3clk)) {
> + error_setg(errp, "m3clk must be wired up by the board code");
> + return;
> + }
> +
> memory_region_init_rom(&s->nvm, OBJECT(dev_soc), "MSF2.eNVM", s->envm_size,
> &error_fatal);
> /*
> @@ -106,19 +114,14 @@ static void m2sxxx_soc_realize(DeviceState *dev_soc, Error **errp)
> qdev_prop_set_uint32(armv7m, "num-irq", 81);
> 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->m3clk);
> object_property_set_link(OBJECT(&s->armv7m), "memory",
> OBJECT(get_system_memory()), &error_abort);
> if (!sysbus_realize(SYS_BUS_DEVICE(&s->armv7m), errp)) {
> return;
> }
>
> - if (!s->m3clk) {
> - error_setg(errp, "Invalid m3clk value");
> - error_append_hint(errp, "m3clk can not be zero\n");
> - return;
> - }
> -
> - system_clock_scale = NANOSECONDS_PER_SECOND / s->m3clk;
> + system_clock_scale = clock_ticks_to_ns(s->m3clk, 1);
>
> for (i = 0; i < MSF2_NUM_UARTS; i++) {
> if (serial_hd(i)) {
> @@ -129,8 +132,13 @@ static void m2sxxx_soc_realize(DeviceState *dev_soc, Error **errp)
> }
>
> dev = DEVICE(&s->timer);
> - /* APB0 clock is the timer input clock */
> - qdev_prop_set_uint32(dev, "clock-frequency", s->m3clk / s->apb0div);
> + /*
> + * APB0 clock is the timer input clock.
> + * TODO: ideally the MSF2 timer device should use a Clock rather than a
> + * clock-frequency integer property.
> + */
> + qdev_prop_set_uint32(dev, "clock-frequency",
> + clock_get_hz(s->m3clk) / s->apb0div);
> if (!sysbus_realize(SYS_BUS_DEVICE(&s->timer), errp)) {
> return;
> }
> @@ -207,8 +215,6 @@ static Property m2sxxx_soc_properties[] = {
> DEFINE_PROP_UINT64("eNVM-size", MSF2State, envm_size, MSF2_ENVM_MAX_SIZE),
> DEFINE_PROP_UINT64("eSRAM-size", MSF2State, esram_size,
> MSF2_ESRAM_MAX_SIZE),
> - /* Libero GUI shows 100Mhz as default for clocks */
> - DEFINE_PROP_UINT32("m3clk", MSF2State, m3clk, 100 * 1000000),
> /* default divisors in Libero GUI */
> DEFINE_PROP_UINT8("apb0div", MSF2State, apb0div, 2),
> DEFINE_PROP_UINT8("apb1div", MSF2State, apb1div, 2),
> diff --git a/hw/arm/msf2-som.c b/hw/arm/msf2-som.c
> index 343ec977c07..396e8b99138 100644
> --- a/hw/arm/msf2-som.c
> +++ b/hw/arm/msf2-som.c
> @@ -29,6 +29,7 @@
> #include "hw/boards.h"
> #include "hw/qdev-properties.h"
> #include "hw/arm/boot.h"
> +#include "hw/qdev-clock.h"
> #include "exec/address-spaces.h"
> #include "hw/arm/msf2-soc.h"
>
> @@ -49,6 +50,7 @@ static void emcraft_sf2_s2s010_init(MachineState *machine)
> BusState *spi_bus;
> MemoryRegion *sysmem = get_system_memory();
> MemoryRegion *ddr = g_new(MemoryRegion, 1);
> + Clock *m3clk;
>
> if (strcmp(machine->cpu_type, mc->default_cpu_type) != 0) {
> error_report("This board can only be used with CPU %s",
> @@ -72,7 +74,10 @@ static void emcraft_sf2_s2s010_init(MachineState *machine)
> * in Libero. CPU clock is divided by APB0 and APB1 divisors for
> * peripherals. Emcraft's SoM kit comes with these settings by default.
> */
> - qdev_prop_set_uint32(dev, "m3clk", 142 * 1000000);
> + /* This clock doesn't need migration because it is fixed-frequency */
> + m3clk = clock_new(OBJECT(machine), "m3clk");
> + clock_set_hz(m3clk, 142 * 1000000);
Maybe something could be added in the commit message to say that M3_CLK
is changed from 100MHz to 142MHz. I do not know the SmartFusion2 but the
clocking guide seems to agree with 142MHz:
https://www.microsemi.com/document-portal/doc_download/132012-ug0449-smartfusion2-and-igloo2-clocking-resources-user-guide
> + qdev_connect_clock_in(dev, "m3clk", m3clk);
> qdev_prop_set_uint32(dev, "apb0div", 2);
> qdev_prop_set_uint32(dev, "apb1div", 2);
>
>
Reviewed-by: Alexandre Iooss <erdnaxe@crans.org>
Thanks,
-- Alexandre