Re: [PATCH for-6.2 19/25] hw/arm/msf2: Use Clock input to MSF2_SOC instead of m3clk property

[Alexandre IOOSS]

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.mayd...@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"

@@ -57,7 +58,7 @@ struct MSF2State {
      uint64_t envm_size;
      uint64_t esram_size;

+    Clock *m3clk;
      uint8_t apb0div;
      uint8_t apb1div;

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"

@@ -73,6 +74,8 @@ static void m2sxxx_soc_initfn(Object *obj)
      }

+
+    s->m3clk = qdev_init_clock_in(DEVICE(obj), "m3clk", NULL, NULL, 0);
  }

@@ -84,6 +87,11 @@ static void m2sxxx_soc_realize(DeviceState *dev_soc, Error 
**errp)

+        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;
      }

-        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);

          if (serial_hd(i)) {
@@ -129,8 +132,13 @@ static void m2sxxx_soc_realize(DeviceState *dev_soc, Error 
**errp)
      }

-    /* 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"

      BusState *spi_bus;
      MemoryRegion *sysmem = get_system_memory();
      MemoryRegion *ddr = g_new(MemoryRegion, 1);
+    Clock *m3clk;

          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);

#define TYPE_MSF2_SOC "msf2-soc" - uint32_t m3clk; diff --git a/hw/arm/msf2-soc.c b/hw/arm/msf2-soc.c #define MSF2_TIMER_BASE 0x40004000 object_initialize_child(obj, "emac", &s->emac, TYPE_MSS_EMAC); static void m2sxxx_soc_realize(DeviceState *dev_soc, Error **errp) MemoryRegion *system_memory = get_system_memory(); + if (!clock_has_source(s->m3clk)) { - if (!s->m3clk) { for (i = 0; i < MSF2_NUM_UARTS; i++) { dev = DEVICE(&s->timer); @@ -49,6 +50,7 @@ static void emcraft_sf2_s2s010_init(MachineState *machine) if (strcmp(machine->cpu_type, mc->default_cpu_type) != 0) {

https://www.microsemi.com/document-portal/doc_download/132012-ug0449-smartfusion2-and-igloo2-clocking-resources-user-guide

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:

+    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 <erdn...@crans.org>

Thanks,
-- Alexandre

OpenPGP_signature
Description: OpenPGP digital signature