qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 00/15] raspi: add the bcm2835 cprman clock manager
@ 2020-10-10 13:57 Luc Michel
  2020-10-10 13:57 ` [PATCH v3 01/15] hw/core/clock: provide the VMSTATE_ARRAY_CLOCK macro Luc Michel
                   ` (17 more replies)
  0 siblings, 18 replies; 35+ messages in thread
From: Luc Michel @ 2020-10-10 13:57 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Luc Michel, Philippe Mathieu-Daudé,
	Andrew Baumann, Paul Zimmerman, Niek Linnenbank, qemu-arm,
	Havard Skinnemoen

v2 -> v3:
  - patch 03: moved clock_new definition to hw/core/clock.c [Phil]
  - patch 03: commit message typo [Clement]
  - patch 10: clarifications around the CM_CTL/CM_DIBV mux registers.
              reg_cm replaced with reg_ctl and reg_div. Add some
              comments for clarity. [Phil]
  - patch 10: fixed update_mux_from_cm not matching the CM_DIV offset
              correctly. [Phil]
  - patch 11: replaced manual bitfield extraction with extract32 [Phil]
  - patch 11: added a visual representation of CM_DIV for clarity [Phil]
  - patch 11: added a missing return in clock_mux_update.

v1 -> v2:
  - patch 05: Added a comment about MMIO .valid constraints [Phil]
  - patch 05: Added MMIO .impl [Phil]
  - patch 05: Moved init_internal_clock to the public clock API, renamed
    clock_new (new patch 03) [Phil]
  - patch 11: use muldiv64 for clock mux frequency output computation [Phil]
  - patch 11: add a check for null divisor (Phil: I dropped your r-b)
  - Typos, formatting, naming, style [Phil]

Patches without review: 03, 11, 13

Hi,

This series add the BCM2835 CPRMAN clock manager peripheral to the
Raspberry Pi machine.

Patches 1-4 are preliminary changes, patches 5-13 are the actual
implementation.

The two last patches add a clock input to the PL011 and
connect it to the CPRMAN.

This series has been tested with Linux 5.4.61 (the current raspios
version). It fixes the kernel Oops at boot time due to invalid UART
clock value, and other warnings/errors here and there because of bad
clocks or lack of CPRMAN.

Here is the clock tree as seen by Linux when booted in QEMU:
(/sys/kernel/debug/clk/clk_summary with some columns removed)

                        enable  prepare              
   clock                 count    count          rate
-----------------------------------------------------
 otg                         0        0     480000000
 osc                         5        5      19200000
    gp2                      1        1         32768
    tsens                    0        0       1920000
    otp                      0        0       4800000
    timer                    0        0       1000002
    pllh                     4        4     864000000
       pllh_pix_prediv       1        1       3375000
          pllh_pix           0        0        337500
       pllh_aux              1        1     216000000
          vec                0        0     108000000
       pllh_rcal_prediv      1        1       3375000
          pllh_rcal          0        0        337500
    plld                     3        3    2000000024
       plld_dsi1             0        0       7812501
       plld_dsi0             0        0       7812501
       plld_per              3        3     500000006
          gp1                1        1      25000000
          uart               1        2      47999625
       plld_core             2        2     500000006
          sdram              0        0     166666668
    pllc                     3        3    2400000000
       pllc_per              1        1    1200000000
          emmc               0        0     200000000
       pllc_core2            0        0       9375000
       pllc_core1            0        0       9375000
       pllc_core0            2        2    1200000000
          vpu                1        1     700000000
             aux_spi2        0        0     700000000
             aux_spi1        0        0     700000000
             aux_uart        0        0     700000000
             peri_image      0        0     700000000
    plla                     2        2    2250000000
       plla_ccp2             0        0       8789063
       plla_dsi0             0        0       8789063
       plla_core             1        1     750000000
          h264               0        0     250000000
          isp                0        0     250000000
 dsi1p                       0        0             0
 dsi0p                       0        0             0
 dsi1e                       0        0             0
 dsi0e                       0        0             0
 cam1                        0        0             0
 cam0                        0        0             0
 dpi                         0        0             0
 tec                         0        0             0
 smi                         0        0             0
 slim                        0        0             0
 gp0                         0        0             0
 dft                         0        0             0
 aveo                        0        0             0
 pcm                         0        0             0
 pwm                         0        0             0
 hsm                         0        0             0

It shows small differences with real hardware due other missing
peripherals for which the driver turn the clock off (like tsens).

Luc Michel (15):
  hw/core/clock: provide the VMSTATE_ARRAY_CLOCK macro
  hw/core/clock: trace clock values in Hz instead of ns
  hw/core/clock: add the clock_new helper function
  hw/arm/raspi: fix CPRMAN base address
  hw/arm/raspi: add a skeleton implementation of the CPRMAN
  hw/misc/bcm2835_cprman: add a PLL skeleton implementation
  hw/misc/bcm2835_cprman: implement PLLs behaviour
  hw/misc/bcm2835_cprman: add a PLL channel skeleton implementation
  hw/misc/bcm2835_cprman: implement PLL channels behaviour
  hw/misc/bcm2835_cprman: add a clock mux skeleton implementation
  hw/misc/bcm2835_cprman: implement clock mux behaviour
  hw/misc/bcm2835_cprman: add the DSI0HSCK multiplexer
  hw/misc/bcm2835_cprman: add sane reset values to the registers
  hw/char/pl011: add a clock input
  hw/arm/bcm2835_peripherals: connect the UART clock

 include/hw/arm/bcm2835_peripherals.h       |    5 +-
 include/hw/arm/raspi_platform.h            |    5 +-
 include/hw/char/pl011.h                    |    1 +
 include/hw/clock.h                         |   18 +
 include/hw/misc/bcm2835_cprman.h           |  210 ++++
 include/hw/misc/bcm2835_cprman_internals.h | 1019 ++++++++++++++++++++
 hw/arm/bcm2835_peripherals.c               |   15 +-
 hw/char/pl011.c                            |   45 +
 hw/core/clock.c                            |   21 +-
 hw/misc/bcm2835_cprman.c                   |  808 ++++++++++++++++
 hw/char/trace-events                       |    1 +
 hw/core/trace-events                       |    4 +-
 hw/misc/meson.build                        |    1 +
 hw/misc/trace-events                       |    5 +
 14 files changed, 2146 insertions(+), 12 deletions(-)
 create mode 100644 include/hw/misc/bcm2835_cprman.h
 create mode 100644 include/hw/misc/bcm2835_cprman_internals.h
 create mode 100644 hw/misc/bcm2835_cprman.c

-- 
2.28.0



^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PATCH v3 01/15] hw/core/clock: provide the VMSTATE_ARRAY_CLOCK macro
  2020-10-10 13:57 [PATCH v3 00/15] raspi: add the bcm2835 cprman clock manager Luc Michel
@ 2020-10-10 13:57 ` Luc Michel
  2020-10-10 13:57 ` [PATCH v3 02/15] hw/core/clock: trace clock values in Hz instead of ns Luc Michel
                   ` (16 subsequent siblings)
  17 siblings, 0 replies; 35+ messages in thread
From: Luc Michel @ 2020-10-10 13:57 UTC (permalink / raw)
  To: qemu-devel
  Cc: Damien Hedde, Peter Maydell, Luc Michel,
	Philippe Mathieu-Daudé,
	Andrew Baumann, Paul Zimmerman, Niek Linnenbank, qemu-arm,
	Havard Skinnemoen

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Damien Hedde <damien.hedde@greensocs.com>
Signed-off-by: Luc Michel <luc@lmichel.fr>
---
 include/hw/clock.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/hw/clock.h b/include/hw/clock.h
index d357594df9..c93e6113cd 100644
--- a/include/hw/clock.h
+++ b/include/hw/clock.h
@@ -79,10 +79,15 @@ struct Clock {
 extern const VMStateDescription vmstate_clock;
 #define VMSTATE_CLOCK(field, state) \
     VMSTATE_CLOCK_V(field, state, 0)
 #define VMSTATE_CLOCK_V(field, state, version) \
     VMSTATE_STRUCT_POINTER_V(field, state, version, vmstate_clock, Clock)
+#define VMSTATE_ARRAY_CLOCK(field, state, num) \
+    VMSTATE_ARRAY_CLOCK_V(field, state, num, 0)
+#define VMSTATE_ARRAY_CLOCK_V(field, state, num, version)          \
+    VMSTATE_ARRAY_OF_POINTER_TO_STRUCT(field, state, num, version, \
+                                       vmstate_clock, Clock)
 
 /**
  * clock_setup_canonical_path:
  * @clk: clock
  *
-- 
2.28.0



^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v3 02/15] hw/core/clock: trace clock values in Hz instead of ns
  2020-10-10 13:57 [PATCH v3 00/15] raspi: add the bcm2835 cprman clock manager Luc Michel
  2020-10-10 13:57 ` [PATCH v3 01/15] hw/core/clock: provide the VMSTATE_ARRAY_CLOCK macro Luc Michel
@ 2020-10-10 13:57 ` Luc Michel
  2020-10-10 15:48   ` Philippe Mathieu-Daudé
  2020-10-10 13:57 ` [PATCH v3 03/15] hw/core/clock: add the clock_new helper function Luc Michel
                   ` (15 subsequent siblings)
  17 siblings, 1 reply; 35+ messages in thread
From: Luc Michel @ 2020-10-10 13:57 UTC (permalink / raw)
  To: qemu-devel
  Cc: Damien Hedde, Peter Maydell, Luc Michel,
	Philippe Mathieu-Daudé,
	Andrew Baumann, Paul Zimmerman, Niek Linnenbank, qemu-arm,
	Havard Skinnemoen

The nanosecond unit greatly limits the dynamic range we can display in
clock value traces, for values in the order of 1GHz and more. The
internal representation can go way beyond this value and it is quite
common for today's clocks to be within those ranges.

For example, a frequency between 500MHz+ and 1GHz will be displayed as
1ns. Beyond 1GHz, it will show up as 0ns.

Replace nanosecond periods traces with frequencies in the Hz unit
to have more dynamic range in the trace output.

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Damien Hedde <damien.hedde@greensocs.com>
Signed-off-by: Luc Michel <luc@lmichel.fr>
---
 hw/core/clock.c      | 6 +++---
 hw/core/trace-events | 4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/core/clock.c b/hw/core/clock.c
index 7066282f7b..81184734e0 100644
--- a/hw/core/clock.c
+++ b/hw/core/clock.c
@@ -37,12 +37,12 @@ void clock_clear_callback(Clock *clk)
 bool clock_set(Clock *clk, uint64_t period)
 {
     if (clk->period == period) {
         return false;
     }
-    trace_clock_set(CLOCK_PATH(clk), CLOCK_PERIOD_TO_NS(clk->period),
-                    CLOCK_PERIOD_TO_NS(period));
+    trace_clock_set(CLOCK_PATH(clk), CLOCK_PERIOD_TO_HZ(clk->period),
+                    CLOCK_PERIOD_TO_HZ(period));
     clk->period = period;
 
     return true;
 }
 
@@ -52,11 +52,11 @@ static void clock_propagate_period(Clock *clk, bool call_callbacks)
 
     QLIST_FOREACH(child, &clk->children, sibling) {
         if (child->period != clk->period) {
             child->period = clk->period;
             trace_clock_update(CLOCK_PATH(child), CLOCK_PATH(clk),
-                               CLOCK_PERIOD_TO_NS(clk->period),
+                               CLOCK_PERIOD_TO_HZ(clk->period),
                                call_callbacks);
             if (call_callbacks && child->callback) {
                 child->callback(child->callback_opaque);
             }
             clock_propagate_period(child, call_callbacks);
diff --git a/hw/core/trace-events b/hw/core/trace-events
index 1ac60ede6b..360ddeb2c8 100644
--- a/hw/core/trace-events
+++ b/hw/core/trace-events
@@ -29,8 +29,8 @@ resettable_phase_exit_end(void *obj, const char *objtype, unsigned count) "obj=%
 resettable_transitional_function(void *obj, const char *objtype) "obj=%p(%s)"
 
 # clock.c
 clock_set_source(const char *clk, const char *src) "'%s', src='%s'"
 clock_disconnect(const char *clk) "'%s'"
-clock_set(const char *clk, uint64_t old, uint64_t new) "'%s', ns=%"PRIu64"->%"PRIu64
+clock_set(const char *clk, uint64_t old, uint64_t new) "'%s', %"PRIu64"Hz->%"PRIu64"Hz"
 clock_propagate(const char *clk) "'%s'"
-clock_update(const char *clk, const char *src, uint64_t val, int cb) "'%s', src='%s', ns=%"PRIu64", cb=%d"
+clock_update(const char *clk, const char *src, uint64_t hz, int cb) "'%s', src='%s', val=%"PRIu64"Hz cb=%d"
-- 
2.28.0



^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v3 03/15] hw/core/clock: add the clock_new helper function
  2020-10-10 13:57 [PATCH v3 00/15] raspi: add the bcm2835 cprman clock manager Luc Michel
  2020-10-10 13:57 ` [PATCH v3 01/15] hw/core/clock: provide the VMSTATE_ARRAY_CLOCK macro Luc Michel
  2020-10-10 13:57 ` [PATCH v3 02/15] hw/core/clock: trace clock values in Hz instead of ns Luc Michel
@ 2020-10-10 13:57 ` Luc Michel
  2020-10-10 15:17   ` Philippe Mathieu-Daudé
  2020-10-10 13:57 ` [PATCH v3 04/15] hw/arm/raspi: fix CPRMAN base address Luc Michel
                   ` (14 subsequent siblings)
  17 siblings, 1 reply; 35+ messages in thread
From: Luc Michel @ 2020-10-10 13:57 UTC (permalink / raw)
  To: qemu-devel
  Cc: Damien Hedde, Peter Maydell, Luc Michel,
	Philippe Mathieu-Daudé,
	Andrew Baumann, Paul Zimmerman, Niek Linnenbank, qemu-arm,
	Havard Skinnemoen

This function creates a clock and parents it to another object with a given
name. It calls clock_setup_canonical_path before returning the new
clock.

This function is useful to create clocks in devices when one doesn't
want to expose it at the qdev level (as an input or an output).

Suggested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Luc Michel <luc@lmichel.fr>
---
 include/hw/clock.h | 13 +++++++++++++
 hw/core/clock.c    | 15 +++++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/include/hw/clock.h b/include/hw/clock.h
index c93e6113cd..81bcf3e505 100644
--- a/include/hw/clock.h
+++ b/include/hw/clock.h
@@ -93,10 +93,23 @@ extern const VMStateDescription vmstate_clock;
  *
  * compute the canonical path of the clock (used by log messages)
  */
 void clock_setup_canonical_path(Clock *clk);
 
+/**
+ * clock_new:
+ * @parent: the clock parent
+ * @name: the clock object name
+ *
+ * Helper function to create a new clock and parent it to @parent. There is no
+ * need to call clock_setup_canonical_path on the returned clock as it is done
+ * by this function.
+ *
+ * @return the newly created clock
+ */
+Clock *clock_new(Object *parent, const char *name);
+
 /**
  * clock_set_callback:
  * @clk: the clock to register the callback into
  * @cb: the callback function
  * @opaque: the argument to the callback
diff --git a/hw/core/clock.c b/hw/core/clock.c
index 81184734e0..8c6af223e7 100644
--- a/hw/core/clock.c
+++ b/hw/core/clock.c
@@ -21,10 +21,25 @@ void clock_setup_canonical_path(Clock *clk)
 {
     g_free(clk->canonical_path);
     clk->canonical_path = object_get_canonical_path(OBJECT(clk));
 }
 
+Clock *clock_new(Object *parent, const char *name)
+{
+    Object *obj;
+    Clock *clk;
+
+    obj = object_new(TYPE_CLOCK);
+    object_property_add_child(parent, name, obj);
+    object_unref(obj);
+
+    clk = CLOCK(obj);
+    clock_setup_canonical_path(clk);
+
+    return clk;
+}
+
 void clock_set_callback(Clock *clk, ClockCallback *cb, void *opaque)
 {
     clk->callback = cb;
     clk->callback_opaque = opaque;
 }
-- 
2.28.0



^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v3 04/15] hw/arm/raspi: fix CPRMAN base address
  2020-10-10 13:57 [PATCH v3 00/15] raspi: add the bcm2835 cprman clock manager Luc Michel
                   ` (2 preceding siblings ...)
  2020-10-10 13:57 ` [PATCH v3 03/15] hw/core/clock: add the clock_new helper function Luc Michel
@ 2020-10-10 13:57 ` Luc Michel
  2020-10-10 13:57 ` [PATCH v3 05/15] hw/arm/raspi: add a skeleton implementation of the CPRMAN Luc Michel
                   ` (13 subsequent siblings)
  17 siblings, 0 replies; 35+ messages in thread
From: Luc Michel @ 2020-10-10 13:57 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Luc Michel, Philippe Mathieu-Daudé,
	Andrew Baumann, Paul Zimmerman, Niek Linnenbank, qemu-arm,
	Havard Skinnemoen

The CPRMAN (clock controller) was mapped at the watchdog/power manager
address. It was also split into two unimplemented peripherals (CM and
A2W) but this is really the same one, as shown by this extract of the
Raspberry Pi 3 Linux device tree:

    watchdog@7e100000 {
            compatible = "brcm,bcm2835-pm\0brcm,bcm2835-pm-wdt";
            [...]
            reg = <0x7e100000 0x114 0x7e00a000 0x24>;
            [...]
    };

    [...]
    cprman@7e101000 {
            compatible = "brcm,bcm2835-cprman";
            [...]
            reg = <0x7e101000 0x2000>;
            [...]
    };

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Luc Michel <luc@lmichel.fr>
---
 include/hw/arm/bcm2835_peripherals.h | 2 +-
 include/hw/arm/raspi_platform.h      | 5 ++---
 hw/arm/bcm2835_peripherals.c         | 4 ++--
 3 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/include/hw/arm/bcm2835_peripherals.h b/include/hw/arm/bcm2835_peripherals.h
index c9ac941a82..6aa94184eb 100644
--- a/include/hw/arm/bcm2835_peripherals.h
+++ b/include/hw/arm/bcm2835_peripherals.h
@@ -45,12 +45,12 @@ struct BCM2835PeripheralState {
 
     BCM2835SystemTimerState systmr;
     BCM2835MphiState mphi;
     UnimplementedDeviceState txp;
     UnimplementedDeviceState armtmr;
+    UnimplementedDeviceState powermgt;
     UnimplementedDeviceState cprman;
-    UnimplementedDeviceState a2w;
     PL011State uart0;
     BCM2835AuxState aux;
     BCM2835FBState fb;
     BCM2835DMAState dma;
     BCM2835ICState ic;
diff --git a/include/hw/arm/raspi_platform.h b/include/hw/arm/raspi_platform.h
index c7f50b260f..e0e6c8ce94 100644
--- a/include/hw/arm/raspi_platform.h
+++ b/include/hw/arm/raspi_platform.h
@@ -43,13 +43,12 @@
 #define ARMCTRL_OFFSET          (ARM_OFFSET + 0x000)
 #define ARMCTRL_IC_OFFSET       (ARM_OFFSET + 0x200) /* Interrupt controller */
 #define ARMCTRL_TIMER0_1_OFFSET (ARM_OFFSET + 0x400) /* Timer 0 and 1 (SP804) */
 #define ARMCTRL_0_SBM_OFFSET    (ARM_OFFSET + 0x800) /* User 0 (ARM) Semaphores
                                                       * Doorbells & Mailboxes */
-#define CPRMAN_OFFSET           0x100000 /* Power Management, Watchdog */
-#define CM_OFFSET               0x101000 /* Clock Management */
-#define A2W_OFFSET              0x102000 /* Reset controller */
+#define PM_OFFSET               0x100000 /* Power Management */
+#define CPRMAN_OFFSET           0x101000 /* Clock Management */
 #define AVS_OFFSET              0x103000 /* Audio Video Standard */
 #define RNG_OFFSET              0x104000
 #define GPIO_OFFSET             0x200000
 #define UART0_OFFSET            0x201000 /* PL011 */
 #define MMCI0_OFFSET            0x202000 /* Legacy MMC */
diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c
index 15c5c72e46..10e9f501d2 100644
--- a/hw/arm/bcm2835_peripherals.c
+++ b/hw/arm/bcm2835_peripherals.c
@@ -343,12 +343,12 @@ static void bcm2835_peripherals_realize(DeviceState *dev, Error **errp)
         qdev_get_gpio_in_named(DEVICE(&s->ic), BCM2835_IC_GPU_IRQ,
                                INTERRUPT_USB));
 
     create_unimp(s, &s->txp, "bcm2835-txp", TXP_OFFSET, 0x1000);
     create_unimp(s, &s->armtmr, "bcm2835-sp804", ARMCTRL_TIMER0_1_OFFSET, 0x40);
-    create_unimp(s, &s->cprman, "bcm2835-cprman", CPRMAN_OFFSET, 0x1000);
-    create_unimp(s, &s->a2w, "bcm2835-a2w", A2W_OFFSET, 0x1000);
+    create_unimp(s, &s->powermgt, "bcm2835-powermgt", PM_OFFSET, 0x114);
+    create_unimp(s, &s->cprman, "bcm2835-cprman", CPRMAN_OFFSET, 0x2000);
     create_unimp(s, &s->i2s, "bcm2835-i2s", I2S_OFFSET, 0x100);
     create_unimp(s, &s->smi, "bcm2835-smi", SMI_OFFSET, 0x100);
     create_unimp(s, &s->spi[0], "bcm2835-spi0", SPI0_OFFSET, 0x20);
     create_unimp(s, &s->bscsl, "bcm2835-spis", BSC_SL_OFFSET, 0x100);
     create_unimp(s, &s->i2c[0], "bcm2835-i2c0", BSC0_OFFSET, 0x20);
-- 
2.28.0



^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v3 05/15] hw/arm/raspi: add a skeleton implementation of the CPRMAN
  2020-10-10 13:57 [PATCH v3 00/15] raspi: add the bcm2835 cprman clock manager Luc Michel
                   ` (3 preceding siblings ...)
  2020-10-10 13:57 ` [PATCH v3 04/15] hw/arm/raspi: fix CPRMAN base address Luc Michel
@ 2020-10-10 13:57 ` Luc Michel
  2020-10-10 13:57 ` [PATCH v3 06/15] hw/misc/bcm2835_cprman: add a PLL skeleton implementation Luc Michel
                   ` (12 subsequent siblings)
  17 siblings, 0 replies; 35+ messages in thread
From: Luc Michel @ 2020-10-10 13:57 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Luc Michel, Philippe Mathieu-Daudé,
	Andrew Baumann, Paul Zimmerman, Niek Linnenbank, qemu-arm,
	Havard Skinnemoen

The BCM2835 CPRMAN is the clock manager of the SoC. It is composed of a
main oscillator, and several sub-components (PLLs, multiplexers, ...) to
generate the BCM2835 clock tree.

This commit adds a skeleton of the CPRMAN, with a dummy register
read/write implementation. It embeds the main oscillator (xosc) from
which all the clocks will be derived.

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Luc Michel <luc@lmichel.fr>
---
 include/hw/arm/bcm2835_peripherals.h       |   3 +-
 include/hw/misc/bcm2835_cprman.h           |  37 +++++
 include/hw/misc/bcm2835_cprman_internals.h |  24 +++
 hw/arm/bcm2835_peripherals.c               |  11 +-
 hw/misc/bcm2835_cprman.c                   | 163 +++++++++++++++++++++
 hw/misc/meson.build                        |   1 +
 hw/misc/trace-events                       |   5 +
 7 files changed, 242 insertions(+), 2 deletions(-)
 create mode 100644 include/hw/misc/bcm2835_cprman.h
 create mode 100644 include/hw/misc/bcm2835_cprman_internals.h
 create mode 100644 hw/misc/bcm2835_cprman.c

diff --git a/include/hw/arm/bcm2835_peripherals.h b/include/hw/arm/bcm2835_peripherals.h
index 6aa94184eb..479e2346e8 100644
--- a/include/hw/arm/bcm2835_peripherals.h
+++ b/include/hw/arm/bcm2835_peripherals.h
@@ -21,10 +21,11 @@
 #include "hw/misc/bcm2835_property.h"
 #include "hw/misc/bcm2835_rng.h"
 #include "hw/misc/bcm2835_mbox.h"
 #include "hw/misc/bcm2835_mphi.h"
 #include "hw/misc/bcm2835_thermal.h"
+#include "hw/misc/bcm2835_cprman.h"
 #include "hw/sd/sdhci.h"
 #include "hw/sd/bcm2835_sdhost.h"
 #include "hw/gpio/bcm2835_gpio.h"
 #include "hw/timer/bcm2835_systmr.h"
 #include "hw/usb/hcd-dwc2.h"
@@ -46,11 +47,11 @@ struct BCM2835PeripheralState {
     BCM2835SystemTimerState systmr;
     BCM2835MphiState mphi;
     UnimplementedDeviceState txp;
     UnimplementedDeviceState armtmr;
     UnimplementedDeviceState powermgt;
-    UnimplementedDeviceState cprman;
+    BCM2835CprmanState cprman;
     PL011State uart0;
     BCM2835AuxState aux;
     BCM2835FBState fb;
     BCM2835DMAState dma;
     BCM2835ICState ic;
diff --git a/include/hw/misc/bcm2835_cprman.h b/include/hw/misc/bcm2835_cprman.h
new file mode 100644
index 0000000000..8ae2d4d17c
--- /dev/null
+++ b/include/hw/misc/bcm2835_cprman.h
@@ -0,0 +1,37 @@
+/*
+ * BCM2835 CPRMAN clock manager
+ *
+ * Copyright (c) 2020 Luc Michel <luc@lmichel.fr>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef HW_MISC_CPRMAN_H
+#define HW_MISC_CPRMAN_H
+
+#include "hw/sysbus.h"
+#include "hw/qdev-clock.h"
+
+#define TYPE_BCM2835_CPRMAN "bcm2835-cprman"
+
+typedef struct BCM2835CprmanState BCM2835CprmanState;
+
+DECLARE_INSTANCE_CHECKER(BCM2835CprmanState, CPRMAN,
+                         TYPE_BCM2835_CPRMAN)
+
+#define CPRMAN_NUM_REGS (0x2000 / sizeof(uint32_t))
+
+struct BCM2835CprmanState {
+    /*< private >*/
+    SysBusDevice parent_obj;
+
+    /*< public >*/
+    MemoryRegion iomem;
+
+    uint32_t regs[CPRMAN_NUM_REGS];
+    uint32_t xosc_freq;
+
+    Clock *xosc;
+};
+
+#endif
diff --git a/include/hw/misc/bcm2835_cprman_internals.h b/include/hw/misc/bcm2835_cprman_internals.h
new file mode 100644
index 0000000000..8fcc6d1d09
--- /dev/null
+++ b/include/hw/misc/bcm2835_cprman_internals.h
@@ -0,0 +1,24 @@
+/*
+ * BCM2835 CPRMAN clock manager
+ *
+ * Copyright (c) 2020 Luc Michel <luc@lmichel.fr>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef HW_MISC_CPRMAN_INTERNALS_H
+#define HW_MISC_CPRMAN_INTERNALS_H
+
+#include "hw/registerfields.h"
+#include "hw/misc/bcm2835_cprman.h"
+
+/* Register map */
+
+/*
+ * This field is common to all registers. Each register write value must match
+ * the CPRMAN_PASSWORD magic value in its 8 MSB.
+ */
+FIELD(CPRMAN, PASSWORD, 24, 8)
+#define CPRMAN_PASSWORD 0x5a
+
+#endif
diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c
index 10e9f501d2..9d6190042d 100644
--- a/hw/arm/bcm2835_peripherals.c
+++ b/hw/arm/bcm2835_peripherals.c
@@ -119,10 +119,13 @@ static void bcm2835_peripherals_init(Object *obj)
     object_initialize_child(obj, "mphi", &s->mphi, TYPE_BCM2835_MPHI);
 
     /* DWC2 */
     object_initialize_child(obj, "dwc2", &s->dwc2, TYPE_DWC2_USB);
 
+    /* CPRMAN clock manager */
+    object_initialize_child(obj, "cprman", &s->cprman, TYPE_BCM2835_CPRMAN);
+
     object_property_add_const_link(OBJECT(&s->dwc2), "dma-mr",
                                    OBJECT(&s->gpu_bus_mr));
 }
 
 static void bcm2835_peripherals_realize(DeviceState *dev, Error **errp)
@@ -158,10 +161,17 @@ static void bcm2835_peripherals_realize(DeviceState *dev, Error **errp)
     /* Interrupt Controller */
     if (!sysbus_realize(SYS_BUS_DEVICE(&s->ic), errp)) {
         return;
     }
 
+    /* CPRMAN clock manager */
+    if (!sysbus_realize(SYS_BUS_DEVICE(&s->cprman), errp)) {
+        return;
+    }
+    memory_region_add_subregion(&s->peri_mr, CPRMAN_OFFSET,
+                sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->cprman), 0));
+
     memory_region_add_subregion(&s->peri_mr, ARMCTRL_IC_OFFSET,
                 sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->ic), 0));
     sysbus_pass_irq(SYS_BUS_DEVICE(s), SYS_BUS_DEVICE(&s->ic));
 
     /* Sys Timer */
@@ -344,11 +354,10 @@ static void bcm2835_peripherals_realize(DeviceState *dev, Error **errp)
                                INTERRUPT_USB));
 
     create_unimp(s, &s->txp, "bcm2835-txp", TXP_OFFSET, 0x1000);
     create_unimp(s, &s->armtmr, "bcm2835-sp804", ARMCTRL_TIMER0_1_OFFSET, 0x40);
     create_unimp(s, &s->powermgt, "bcm2835-powermgt", PM_OFFSET, 0x114);
-    create_unimp(s, &s->cprman, "bcm2835-cprman", CPRMAN_OFFSET, 0x2000);
     create_unimp(s, &s->i2s, "bcm2835-i2s", I2S_OFFSET, 0x100);
     create_unimp(s, &s->smi, "bcm2835-smi", SMI_OFFSET, 0x100);
     create_unimp(s, &s->spi[0], "bcm2835-spi0", SPI0_OFFSET, 0x20);
     create_unimp(s, &s->bscsl, "bcm2835-spis", BSC_SL_OFFSET, 0x100);
     create_unimp(s, &s->i2c[0], "bcm2835-i2c0", BSC0_OFFSET, 0x20);
diff --git a/hw/misc/bcm2835_cprman.c b/hw/misc/bcm2835_cprman.c
new file mode 100644
index 0000000000..57ab9910b5
--- /dev/null
+++ b/hw/misc/bcm2835_cprman.c
@@ -0,0 +1,163 @@
+/*
+ * BCM2835 CPRMAN clock manager
+ *
+ * Copyright (c) 2020 Luc Michel <luc@lmichel.fr>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+/*
+ * This peripheral is roughly divided into 3 main parts:
+ *   - the PLLs
+ *   - the PLL channels
+ *   - the clock muxes
+ *
+ * A main oscillator (xosc) feeds all the PLLs. Each PLLs has one or more
+ * channels. Those channel are then connected to the clock muxes. Each mux has
+ * multiples sources (usually the xosc, some of the PLL channels and some "test
+ * debug" clocks). A mux is configured to select a given source through its
+ * control register. Each mux has one output clock that also goes out of the
+ * CPRMAN. This output clock usually connects to another peripheral in the SoC
+ * (so a given mux is dedicated to a peripheral).
+ *
+ * At each level (PLL, channel and mux), the clock can be altered through
+ * dividers (and multipliers in case of the PLLs), and can be disabled (in this
+ * case, the next levels see no clock).
+ *
+ * This can be sum-up as follows (this is an example and not the actual BCM2835
+ * clock tree):
+ *
+ *          /-->[PLL]-|->[PLL channel]--...            [mux]--> to peripherals
+ *          |         |->[PLL channel]  muxes takes    [mux]
+ *          |         \->[PLL channel]  inputs from    [mux]
+ *          |                           some channels  [mux]
+ * [xosc]---|-->[PLL]-|->[PLL channel]  and other srcs [mux]
+ *          |         \->[PLL channel]           ...-->[mux]
+ *          |                                          [mux]
+ *          \-->[PLL]--->[PLL channel]                 [mux]
+ *
+ * The page at https://elinux.org/The_Undocumented_Pi gives the actual clock
+ * tree configuration.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "migration/vmstate.h"
+#include "hw/qdev-properties.h"
+#include "hw/misc/bcm2835_cprman.h"
+#include "hw/misc/bcm2835_cprman_internals.h"
+#include "trace.h"
+
+/* CPRMAN "top level" model */
+
+static uint64_t cprman_read(void *opaque, hwaddr offset,
+                            unsigned size)
+{
+    BCM2835CprmanState *s = CPRMAN(opaque);
+    uint64_t r = 0;
+    size_t idx = offset / sizeof(uint32_t);
+
+    switch (idx) {
+    default:
+        r = s->regs[idx];
+    }
+
+    trace_bcm2835_cprman_read(offset, r);
+    return r;
+}
+
+static void cprman_write(void *opaque, hwaddr offset,
+                         uint64_t value, unsigned size)
+{
+    BCM2835CprmanState *s = CPRMAN(opaque);
+    size_t idx = offset / sizeof(uint32_t);
+
+    if (FIELD_EX32(value, CPRMAN, PASSWORD) != CPRMAN_PASSWORD) {
+        trace_bcm2835_cprman_write_invalid_magic(offset, value);
+        return;
+    }
+
+    value &= ~R_CPRMAN_PASSWORD_MASK;
+
+    trace_bcm2835_cprman_write(offset, value);
+    s->regs[idx] = value;
+
+}
+
+static const MemoryRegionOps cprman_ops = {
+    .read = cprman_read,
+    .write = cprman_write,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+    .valid = {
+        /*
+         * Although this hasn't been checked against real hardware, nor the
+         * information can be found in a datasheet, it seems reasonable because
+         * of the "PASSWORD" magic value found in every registers.
+         */
+        .min_access_size        = 4,
+        .max_access_size        = 4,
+        .unaligned              = false,
+    },
+    .impl = {
+        .max_access_size = 4,
+    },
+};
+
+static void cprman_reset(DeviceState *dev)
+{
+    BCM2835CprmanState *s = CPRMAN(dev);
+
+    memset(s->regs, 0, sizeof(s->regs));
+
+    clock_update_hz(s->xosc, s->xosc_freq);
+}
+
+static void cprman_init(Object *obj)
+{
+    BCM2835CprmanState *s = CPRMAN(obj);
+
+    s->xosc = clock_new(obj, "xosc");
+
+    memory_region_init_io(&s->iomem, obj, &cprman_ops,
+                          s, "bcm2835-cprman", 0x2000);
+    sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->iomem);
+}
+
+static const VMStateDescription cprman_vmstate = {
+    .name = TYPE_BCM2835_CPRMAN,
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32_ARRAY(regs, BCM2835CprmanState, CPRMAN_NUM_REGS),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static Property cprman_properties[] = {
+    DEFINE_PROP_UINT32("xosc-freq-hz", BCM2835CprmanState, xosc_freq, 19200000),
+    DEFINE_PROP_END_OF_LIST()
+};
+
+static void cprman_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->reset = cprman_reset;
+    dc->vmsd = &cprman_vmstate;
+    device_class_set_props(dc, cprman_properties);
+}
+
+static const TypeInfo cprman_info = {
+    .name = TYPE_BCM2835_CPRMAN,
+    .parent = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(BCM2835CprmanState),
+    .class_init = cprman_class_init,
+    .instance_init = cprman_init,
+};
+
+static void cprman_register_types(void)
+{
+    type_register_static(&cprman_info);
+}
+
+type_init(cprman_register_types);
diff --git a/hw/misc/meson.build b/hw/misc/meson.build
index 793d45b1dc..c94cf70e82 100644
--- a/hw/misc/meson.build
+++ b/hw/misc/meson.build
@@ -71,10 +71,11 @@ softmmu_ss.add(when: 'CONFIG_RASPI', if_true: files(
   'bcm2835_mbox.c',
   'bcm2835_mphi.c',
   'bcm2835_property.c',
   'bcm2835_rng.c',
   'bcm2835_thermal.c',
+  'bcm2835_cprman.c',
 ))
 softmmu_ss.add(when: 'CONFIG_SLAVIO', if_true: files('slavio_misc.c'))
 softmmu_ss.add(when: 'CONFIG_ZYNQ', if_true: files('zynq_slcr.c', 'zynq-xadc.c'))
 softmmu_ss.add(when: 'CONFIG_STM32F2XX_SYSCFG', if_true: files('stm32f2xx_syscfg.c'))
 softmmu_ss.add(when: 'CONFIG_STM32F4XX_SYSCFG', if_true: files('stm32f4xx_syscfg.c'))
diff --git a/hw/misc/trace-events b/hw/misc/trace-events
index 6054f9adf3..d718a2b177 100644
--- a/hw/misc/trace-events
+++ b/hw/misc/trace-events
@@ -224,5 +224,10 @@ grlib_ahb_pnp_read(uint64_t addr, uint32_t value) "AHB PnP read addr:0x%03"PRIx6
 grlib_apb_pnp_read(uint64_t addr, uint32_t value) "APB PnP read addr:0x%03"PRIx64" data:0x%08x"
 
 # pca9552.c
 pca955x_gpio_status(const char *description, const char *buf) "%s GPIOs 0-15 [%s]"
 pca955x_gpio_change(const char *description, unsigned id, unsigned prev_state, unsigned current_state) "%s GPIO id:%u status: %u -> %u"
+
+# bcm2835_cprman.c
+bcm2835_cprman_read(uint64_t offset, uint64_t value) "offset:0x%" PRIx64 " value:0x%" PRIx64
+bcm2835_cprman_write(uint64_t offset, uint64_t value) "offset:0x%" PRIx64 " value:0x%" PRIx64
+bcm2835_cprman_write_invalid_magic(uint64_t offset, uint64_t value) "offset:0x%" PRIx64 " value:0x%" PRIx64
-- 
2.28.0



^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v3 06/15] hw/misc/bcm2835_cprman: add a PLL skeleton implementation
  2020-10-10 13:57 [PATCH v3 00/15] raspi: add the bcm2835 cprman clock manager Luc Michel
                   ` (4 preceding siblings ...)
  2020-10-10 13:57 ` [PATCH v3 05/15] hw/arm/raspi: add a skeleton implementation of the CPRMAN Luc Michel
@ 2020-10-10 13:57 ` Luc Michel
  2020-10-10 13:57 ` [PATCH v3 07/15] hw/misc/bcm2835_cprman: implement PLLs behaviour Luc Michel
                   ` (11 subsequent siblings)
  17 siblings, 0 replies; 35+ messages in thread
From: Luc Michel @ 2020-10-10 13:57 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Luc Michel, Philippe Mathieu-Daudé,
	Andrew Baumann, Paul Zimmerman, Niek Linnenbank, qemu-arm,
	Havard Skinnemoen

There are 5 PLLs in the CPRMAN, namely PLL A, C, D, H and B. All of them
take the xosc clock as input and produce a new clock.

This commit adds a skeleton implementation for the PLLs as sub-devices
of the CPRMAN. The PLLs are instantiated and connected internally to the
main oscillator.

Each PLL has 6 registers : CM, A2W_CTRL, A2W_ANA[0,1,2,3], A2W_FRAC. A
write to any of them triggers a call to the (not yet implemented)
pll_update function.

If the main oscillator changes frequency, an update is also triggered.

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Luc Michel <luc@lmichel.fr>
---
 include/hw/misc/bcm2835_cprman.h           |  29 +++++
 include/hw/misc/bcm2835_cprman_internals.h | 144 +++++++++++++++++++++
 hw/misc/bcm2835_cprman.c                   | 108 ++++++++++++++++
 3 files changed, 281 insertions(+)

diff --git a/include/hw/misc/bcm2835_cprman.h b/include/hw/misc/bcm2835_cprman.h
index 8ae2d4d17c..5c442e6ff9 100644
--- a/include/hw/misc/bcm2835_cprman.h
+++ b/include/hw/misc/bcm2835_cprman.h
@@ -19,17 +19,46 @@ typedef struct BCM2835CprmanState BCM2835CprmanState;
 DECLARE_INSTANCE_CHECKER(BCM2835CprmanState, CPRMAN,
                          TYPE_BCM2835_CPRMAN)
 
 #define CPRMAN_NUM_REGS (0x2000 / sizeof(uint32_t))
 
+typedef enum CprmanPll {
+    CPRMAN_PLLA = 0,
+    CPRMAN_PLLC,
+    CPRMAN_PLLD,
+    CPRMAN_PLLH,
+    CPRMAN_PLLB,
+
+    CPRMAN_NUM_PLL
+} CprmanPll;
+
+typedef struct CprmanPllState {
+    /*< private >*/
+    DeviceState parent_obj;
+
+    /*< public >*/
+    CprmanPll id;
+
+    uint32_t *reg_cm;
+    uint32_t *reg_a2w_ctrl;
+    uint32_t *reg_a2w_ana; /* ANA[0] .. ANA[3] */
+    uint32_t prediv_mask; /* prediv bit in ana[1] */
+    uint32_t *reg_a2w_frac;
+
+    Clock *xosc_in;
+    Clock *out;
+} CprmanPllState;
+
 struct BCM2835CprmanState {
     /*< private >*/
     SysBusDevice parent_obj;
 
     /*< public >*/
     MemoryRegion iomem;
 
+    CprmanPllState plls[CPRMAN_NUM_PLL];
+
     uint32_t regs[CPRMAN_NUM_REGS];
     uint32_t xosc_freq;
 
     Clock *xosc;
 };
diff --git a/include/hw/misc/bcm2835_cprman_internals.h b/include/hw/misc/bcm2835_cprman_internals.h
index 8fcc6d1d09..340ad623bb 100644
--- a/include/hw/misc/bcm2835_cprman_internals.h
+++ b/include/hw/misc/bcm2835_cprman_internals.h
@@ -10,15 +10,159 @@
 #define HW_MISC_CPRMAN_INTERNALS_H
 
 #include "hw/registerfields.h"
 #include "hw/misc/bcm2835_cprman.h"
 
+#define TYPE_CPRMAN_PLL "bcm2835-cprman-pll"
+
+DECLARE_INSTANCE_CHECKER(CprmanPllState, CPRMAN_PLL,
+                         TYPE_CPRMAN_PLL)
+
 /* Register map */
 
+/* PLLs */
+REG32(CM_PLLA, 0x104)
+    FIELD(CM_PLLA, LOADDSI0, 0, 1)
+    FIELD(CM_PLLA, HOLDDSI0, 1, 1)
+    FIELD(CM_PLLA, LOADCCP2, 2, 1)
+    FIELD(CM_PLLA, HOLDCCP2, 3, 1)
+    FIELD(CM_PLLA, LOADCORE, 4, 1)
+    FIELD(CM_PLLA, HOLDCORE, 5, 1)
+    FIELD(CM_PLLA, LOADPER, 6, 1)
+    FIELD(CM_PLLA, HOLDPER, 7, 1)
+    FIELD(CM_PLLx, ANARST, 8, 1)
+REG32(CM_PLLC, 0x108)
+    FIELD(CM_PLLC, LOADCORE0, 0, 1)
+    FIELD(CM_PLLC, HOLDCORE0, 1, 1)
+    FIELD(CM_PLLC, LOADCORE1, 2, 1)
+    FIELD(CM_PLLC, HOLDCORE1, 3, 1)
+    FIELD(CM_PLLC, LOADCORE2, 4, 1)
+    FIELD(CM_PLLC, HOLDCORE2, 5, 1)
+    FIELD(CM_PLLC, LOADPER, 6, 1)
+    FIELD(CM_PLLC, HOLDPER, 7, 1)
+REG32(CM_PLLD, 0x10c)
+    FIELD(CM_PLLD, LOADDSI0, 0, 1)
+    FIELD(CM_PLLD, HOLDDSI0, 1, 1)
+    FIELD(CM_PLLD, LOADDSI1, 2, 1)
+    FIELD(CM_PLLD, HOLDDSI1, 3, 1)
+    FIELD(CM_PLLD, LOADCORE, 4, 1)
+    FIELD(CM_PLLD, HOLDCORE, 5, 1)
+    FIELD(CM_PLLD, LOADPER, 6, 1)
+    FIELD(CM_PLLD, HOLDPER, 7, 1)
+REG32(CM_PLLH, 0x110)
+    FIELD(CM_PLLH, LOADPIX, 0, 1)
+    FIELD(CM_PLLH, LOADAUX, 1, 1)
+    FIELD(CM_PLLH, LOADRCAL, 2, 1)
+REG32(CM_PLLB, 0x170)
+    FIELD(CM_PLLB, LOADARM, 0, 1)
+    FIELD(CM_PLLB, HOLDARM, 1, 1)
+
+REG32(A2W_PLLA_CTRL, 0x1100)
+    FIELD(A2W_PLLx_CTRL, NDIV, 0, 10)
+    FIELD(A2W_PLLx_CTRL, PDIV, 12, 3)
+    FIELD(A2W_PLLx_CTRL, PWRDN, 16, 1)
+    FIELD(A2W_PLLx_CTRL, PRST_DISABLE, 17, 1)
+REG32(A2W_PLLC_CTRL, 0x1120)
+REG32(A2W_PLLD_CTRL, 0x1140)
+REG32(A2W_PLLH_CTRL, 0x1160)
+REG32(A2W_PLLB_CTRL, 0x11e0)
+
+REG32(A2W_PLLA_ANA0, 0x1010)
+REG32(A2W_PLLA_ANA1, 0x1014)
+    FIELD(A2W_PLLx_ANA1, FB_PREDIV, 14, 1)
+REG32(A2W_PLLA_ANA2, 0x1018)
+REG32(A2W_PLLA_ANA3, 0x101c)
+
+REG32(A2W_PLLC_ANA0, 0x1030)
+REG32(A2W_PLLC_ANA1, 0x1034)
+REG32(A2W_PLLC_ANA2, 0x1038)
+REG32(A2W_PLLC_ANA3, 0x103c)
+
+REG32(A2W_PLLD_ANA0, 0x1050)
+REG32(A2W_PLLD_ANA1, 0x1054)
+REG32(A2W_PLLD_ANA2, 0x1058)
+REG32(A2W_PLLD_ANA3, 0x105c)
+
+REG32(A2W_PLLH_ANA0, 0x1070)
+REG32(A2W_PLLH_ANA1, 0x1074)
+    FIELD(A2W_PLLH_ANA1, FB_PREDIV, 11, 1)
+REG32(A2W_PLLH_ANA2, 0x1078)
+REG32(A2W_PLLH_ANA3, 0x107c)
+
+REG32(A2W_PLLB_ANA0, 0x10f0)
+REG32(A2W_PLLB_ANA1, 0x10f4)
+REG32(A2W_PLLB_ANA2, 0x10f8)
+REG32(A2W_PLLB_ANA3, 0x10fc)
+
+REG32(A2W_PLLA_FRAC, 0x1200)
+    FIELD(A2W_PLLx_FRAC, FRAC, 0, 20)
+REG32(A2W_PLLC_FRAC, 0x1220)
+REG32(A2W_PLLD_FRAC, 0x1240)
+REG32(A2W_PLLH_FRAC, 0x1260)
+REG32(A2W_PLLB_FRAC, 0x12e0)
+
 /*
  * This field is common to all registers. Each register write value must match
  * the CPRMAN_PASSWORD magic value in its 8 MSB.
  */
 FIELD(CPRMAN, PASSWORD, 24, 8)
 #define CPRMAN_PASSWORD 0x5a
 
+/* PLL init info */
+typedef struct PLLInitInfo {
+    const char *name;
+    size_t cm_offset;
+    size_t a2w_ctrl_offset;
+    size_t a2w_ana_offset;
+    uint32_t prediv_mask; /* Prediv bit in ana[1] */
+    size_t a2w_frac_offset;
+} PLLInitInfo;
+
+#define FILL_PLL_INIT_INFO(pll_)                \
+    .cm_offset = R_CM_ ## pll_,                 \
+    .a2w_ctrl_offset = R_A2W_ ## pll_ ## _CTRL, \
+    .a2w_ana_offset = R_A2W_ ## pll_ ## _ANA0,  \
+    .a2w_frac_offset = R_A2W_ ## pll_ ## _FRAC
+
+static const PLLInitInfo PLL_INIT_INFO[] = {
+    [CPRMAN_PLLA] = {
+        .name = "plla",
+        .prediv_mask = R_A2W_PLLx_ANA1_FB_PREDIV_MASK,
+        FILL_PLL_INIT_INFO(PLLA),
+    },
+    [CPRMAN_PLLC] = {
+        .name = "pllc",
+        .prediv_mask = R_A2W_PLLx_ANA1_FB_PREDIV_MASK,
+        FILL_PLL_INIT_INFO(PLLC),
+    },
+    [CPRMAN_PLLD] = {
+        .name = "plld",
+        .prediv_mask = R_A2W_PLLx_ANA1_FB_PREDIV_MASK,
+        FILL_PLL_INIT_INFO(PLLD),
+    },
+    [CPRMAN_PLLH] = {
+        .name = "pllh",
+        .prediv_mask = R_A2W_PLLH_ANA1_FB_PREDIV_MASK,
+        FILL_PLL_INIT_INFO(PLLH),
+    },
+    [CPRMAN_PLLB] = {
+        .name = "pllb",
+        .prediv_mask = R_A2W_PLLx_ANA1_FB_PREDIV_MASK,
+        FILL_PLL_INIT_INFO(PLLB),
+    },
+};
+
+#undef FILL_PLL_CHANNEL_INIT_INFO
+
+static inline void set_pll_init_info(BCM2835CprmanState *s,
+                                     CprmanPllState *pll,
+                                     CprmanPll id)
+{
+    pll->id = id;
+    pll->reg_cm = &s->regs[PLL_INIT_INFO[id].cm_offset];
+    pll->reg_a2w_ctrl = &s->regs[PLL_INIT_INFO[id].a2w_ctrl_offset];
+    pll->reg_a2w_ana = &s->regs[PLL_INIT_INFO[id].a2w_ana_offset];
+    pll->prediv_mask = PLL_INIT_INFO[id].prediv_mask;
+    pll->reg_a2w_frac = &s->regs[PLL_INIT_INFO[id].a2w_frac_offset];
+}
+
 #endif
diff --git a/hw/misc/bcm2835_cprman.c b/hw/misc/bcm2835_cprman.c
index 57ab9910b5..b86f5901b8 100644
--- a/hw/misc/bcm2835_cprman.c
+++ b/hw/misc/bcm2835_cprman.c
@@ -46,10 +46,56 @@
 #include "hw/qdev-properties.h"
 #include "hw/misc/bcm2835_cprman.h"
 #include "hw/misc/bcm2835_cprman_internals.h"
 #include "trace.h"
 
+/* PLL */
+
+static void pll_update(CprmanPllState *pll)
+{
+    clock_update(pll->out, 0);
+}
+
+static void pll_xosc_update(void *opaque)
+{
+    pll_update(CPRMAN_PLL(opaque));
+}
+
+static void pll_init(Object *obj)
+{
+    CprmanPllState *s = CPRMAN_PLL(obj);
+
+    s->xosc_in = qdev_init_clock_in(DEVICE(s), "xosc-in", pll_xosc_update, s);
+    s->out = qdev_init_clock_out(DEVICE(s), "out");
+}
+
+static const VMStateDescription pll_vmstate = {
+    .name = TYPE_CPRMAN_PLL,
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_CLOCK(xosc_in, CprmanPllState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static void pll_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->vmsd = &pll_vmstate;
+}
+
+static const TypeInfo cprman_pll_info = {
+    .name = TYPE_CPRMAN_PLL,
+    .parent = TYPE_DEVICE,
+    .instance_size = sizeof(CprmanPllState),
+    .class_init = pll_class_init,
+    .instance_init = pll_init,
+};
+
+
 /* CPRMAN "top level" model */
 
 static uint64_t cprman_read(void *opaque, hwaddr offset,
                             unsigned size)
 {
@@ -64,10 +110,19 @@ static uint64_t cprman_read(void *opaque, hwaddr offset,
 
     trace_bcm2835_cprman_read(offset, r);
     return r;
 }
 
+#define CASE_PLL_REGS(pll_)       \
+    case R_CM_ ## pll_:           \
+    case R_A2W_ ## pll_ ## _CTRL: \
+    case R_A2W_ ## pll_ ## _ANA0: \
+    case R_A2W_ ## pll_ ## _ANA1: \
+    case R_A2W_ ## pll_ ## _ANA2: \
+    case R_A2W_ ## pll_ ## _ANA3: \
+    case R_A2W_ ## pll_ ## _FRAC
+
 static void cprman_write(void *opaque, hwaddr offset,
                          uint64_t value, unsigned size)
 {
     BCM2835CprmanState *s = CPRMAN(opaque);
     size_t idx = offset / sizeof(uint32_t);
@@ -80,12 +135,35 @@ static void cprman_write(void *opaque, hwaddr offset,
     value &= ~R_CPRMAN_PASSWORD_MASK;
 
     trace_bcm2835_cprman_write(offset, value);
     s->regs[idx] = value;
 
+    switch (idx) {
+    CASE_PLL_REGS(PLLA) :
+        pll_update(&s->plls[CPRMAN_PLLA]);
+        break;
+
+    CASE_PLL_REGS(PLLC) :
+        pll_update(&s->plls[CPRMAN_PLLC]);
+        break;
+
+    CASE_PLL_REGS(PLLD) :
+        pll_update(&s->plls[CPRMAN_PLLD]);
+        break;
+
+    CASE_PLL_REGS(PLLH) :
+        pll_update(&s->plls[CPRMAN_PLLH]);
+        break;
+
+    CASE_PLL_REGS(PLLB) :
+        pll_update(&s->plls[CPRMAN_PLLB]);
+        break;
+    }
 }
 
+#undef CASE_PLL_REGS
+
 static const MemoryRegionOps cprman_ops = {
     .read = cprman_read,
     .write = cprman_write,
     .endianness = DEVICE_LITTLE_ENDIAN,
     .valid = {
@@ -104,27 +182,55 @@ static const MemoryRegionOps cprman_ops = {
 };
 
 static void cprman_reset(DeviceState *dev)
 {
     BCM2835CprmanState *s = CPRMAN(dev);
+    size_t i;
 
     memset(s->regs, 0, sizeof(s->regs));
 
+    for (i = 0; i < CPRMAN_NUM_PLL; i++) {
+        device_cold_reset(DEVICE(&s->plls[i]));
+    }
+
     clock_update_hz(s->xosc, s->xosc_freq);
 }
 
 static void cprman_init(Object *obj)
 {
     BCM2835CprmanState *s = CPRMAN(obj);
+    size_t i;
+
+    for (i = 0; i < CPRMAN_NUM_PLL; i++) {
+        object_initialize_child(obj, PLL_INIT_INFO[i].name,
+                                &s->plls[i], TYPE_CPRMAN_PLL);
+        set_pll_init_info(s, &s->plls[i], i);
+    }
 
     s->xosc = clock_new(obj, "xosc");
 
     memory_region_init_io(&s->iomem, obj, &cprman_ops,
                           s, "bcm2835-cprman", 0x2000);
     sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->iomem);
 }
 
+static void cprman_realize(DeviceState *dev, Error **errp)
+{
+    BCM2835CprmanState *s = CPRMAN(dev);
+    size_t i;
+
+    for (i = 0; i < CPRMAN_NUM_PLL; i++) {
+        CprmanPllState *pll = &s->plls[i];
+
+        clock_set_source(pll->xosc_in, s->xosc);
+
+        if (!qdev_realize(DEVICE(pll), NULL, errp)) {
+            return;
+        }
+    }
+}
+
 static const VMStateDescription cprman_vmstate = {
     .name = TYPE_BCM2835_CPRMAN,
     .version_id = 1,
     .minimum_version_id = 1,
     .fields = (VMStateField[]) {
@@ -140,10 +246,11 @@ static Property cprman_properties[] = {
 
 static void cprman_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
 
+    dc->realize = cprman_realize;
     dc->reset = cprman_reset;
     dc->vmsd = &cprman_vmstate;
     device_class_set_props(dc, cprman_properties);
 }
 
@@ -156,8 +263,9 @@ static const TypeInfo cprman_info = {
 };
 
 static void cprman_register_types(void)
 {
     type_register_static(&cprman_info);
+    type_register_static(&cprman_pll_info);
 }
 
 type_init(cprman_register_types);
-- 
2.28.0



^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v3 07/15] hw/misc/bcm2835_cprman: implement PLLs behaviour
  2020-10-10 13:57 [PATCH v3 00/15] raspi: add the bcm2835 cprman clock manager Luc Michel
                   ` (5 preceding siblings ...)
  2020-10-10 13:57 ` [PATCH v3 06/15] hw/misc/bcm2835_cprman: add a PLL skeleton implementation Luc Michel
@ 2020-10-10 13:57 ` Luc Michel
  2020-10-10 13:57 ` [PATCH v3 08/15] hw/misc/bcm2835_cprman: add a PLL channel skeleton implementation Luc Michel
                   ` (10 subsequent siblings)
  17 siblings, 0 replies; 35+ messages in thread
From: Luc Michel @ 2020-10-10 13:57 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Luc Michel, Philippe Mathieu-Daudé,
	Andrew Baumann, Paul Zimmerman, Niek Linnenbank, qemu-arm,
	Havard Skinnemoen

The CPRMAN PLLs generate a clock based on a prescaler, a multiplier and
a divider. The prescaler doubles the parent (xosc) frequency, then the
multiplier/divider are applied. The multiplier has an integer and a
fractional part.

This commit also implements the CPRMAN CM_LOCK register. This register
reports which PLL is currently locked. We consider a PLL has being
locked as soon as it is enabled (on real hardware, there is a delay
after turning a PLL on, for it to stabilize).

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Luc Michel <luc@lmichel.fr>
---
 include/hw/misc/bcm2835_cprman_internals.h |  8 +++
 hw/misc/bcm2835_cprman.c                   | 64 +++++++++++++++++++++-
 2 files changed, 71 insertions(+), 1 deletion(-)

diff --git a/include/hw/misc/bcm2835_cprman_internals.h b/include/hw/misc/bcm2835_cprman_internals.h
index 340ad623bb..7aa46c6e18 100644
--- a/include/hw/misc/bcm2835_cprman_internals.h
+++ b/include/hw/misc/bcm2835_cprman_internals.h
@@ -98,10 +98,18 @@ REG32(A2W_PLLA_FRAC, 0x1200)
 REG32(A2W_PLLC_FRAC, 0x1220)
 REG32(A2W_PLLD_FRAC, 0x1240)
 REG32(A2W_PLLH_FRAC, 0x1260)
 REG32(A2W_PLLB_FRAC, 0x12e0)
 
+/* misc registers */
+REG32(CM_LOCK, 0x114)
+    FIELD(CM_LOCK, FLOCKH, 12, 1)
+    FIELD(CM_LOCK, FLOCKD, 11, 1)
+    FIELD(CM_LOCK, FLOCKC, 10, 1)
+    FIELD(CM_LOCK, FLOCKB, 9, 1)
+    FIELD(CM_LOCK, FLOCKA, 8, 1)
+
 /*
  * This field is common to all registers. Each register write value must match
  * the CPRMAN_PASSWORD magic value in its 8 MSB.
  */
 FIELD(CPRMAN, PASSWORD, 24, 8)
diff --git a/hw/misc/bcm2835_cprman.c b/hw/misc/bcm2835_cprman.c
index b86f5901b8..144bcc289d 100644
--- a/hw/misc/bcm2835_cprman.c
+++ b/hw/misc/bcm2835_cprman.c
@@ -48,13 +48,51 @@
 #include "hw/misc/bcm2835_cprman_internals.h"
 #include "trace.h"
 
 /* PLL */
 
+static bool pll_is_locked(const CprmanPllState *pll)
+{
+    return !FIELD_EX32(*pll->reg_a2w_ctrl, A2W_PLLx_CTRL, PWRDN)
+        && !FIELD_EX32(*pll->reg_cm, CM_PLLx, ANARST);
+}
+
 static void pll_update(CprmanPllState *pll)
 {
-    clock_update(pll->out, 0);
+    uint64_t freq, ndiv, fdiv, pdiv;
+
+    if (!pll_is_locked(pll)) {
+        clock_update(pll->out, 0);
+        return;
+    }
+
+    pdiv = FIELD_EX32(*pll->reg_a2w_ctrl, A2W_PLLx_CTRL, PDIV);
+
+    if (!pdiv) {
+        clock_update(pll->out, 0);
+        return;
+    }
+
+    ndiv = FIELD_EX32(*pll->reg_a2w_ctrl, A2W_PLLx_CTRL, NDIV);
+    fdiv = FIELD_EX32(*pll->reg_a2w_frac, A2W_PLLx_FRAC, FRAC);
+
+    if (pll->reg_a2w_ana[1] & pll->prediv_mask) {
+        /* The prescaler doubles the parent frequency */
+        ndiv *= 2;
+        fdiv *= 2;
+    }
+
+    /*
+     * We have a multiplier with an integer part (ndiv) and a fractional part
+     * (fdiv), and a divider (pdiv).
+     */
+    freq = clock_get_hz(pll->xosc_in) *
+        ((ndiv << R_A2W_PLLx_FRAC_FRAC_LENGTH) + fdiv);
+    freq /= pdiv;
+    freq >>= R_A2W_PLLx_FRAC_FRAC_LENGTH;
+
+    clock_update_hz(pll->out, freq);
 }
 
 static void pll_xosc_update(void *opaque)
 {
     pll_update(CPRMAN_PLL(opaque));
@@ -94,18 +132,42 @@ static const TypeInfo cprman_pll_info = {
 };
 
 
 /* CPRMAN "top level" model */
 
+static uint32_t get_cm_lock(const BCM2835CprmanState *s)
+{
+    static const int CM_LOCK_MAPPING[CPRMAN_NUM_PLL] = {
+        [CPRMAN_PLLA] = R_CM_LOCK_FLOCKA_SHIFT,
+        [CPRMAN_PLLC] = R_CM_LOCK_FLOCKC_SHIFT,
+        [CPRMAN_PLLD] = R_CM_LOCK_FLOCKD_SHIFT,
+        [CPRMAN_PLLH] = R_CM_LOCK_FLOCKH_SHIFT,
+        [CPRMAN_PLLB] = R_CM_LOCK_FLOCKB_SHIFT,
+    };
+
+    uint32_t r = 0;
+    size_t i;
+
+    for (i = 0; i < CPRMAN_NUM_PLL; i++) {
+        r |= pll_is_locked(&s->plls[i]) << CM_LOCK_MAPPING[i];
+    }
+
+    return r;
+}
+
 static uint64_t cprman_read(void *opaque, hwaddr offset,
                             unsigned size)
 {
     BCM2835CprmanState *s = CPRMAN(opaque);
     uint64_t r = 0;
     size_t idx = offset / sizeof(uint32_t);
 
     switch (idx) {
+    case R_CM_LOCK:
+        r = get_cm_lock(s);
+        break;
+
     default:
         r = s->regs[idx];
     }
 
     trace_bcm2835_cprman_read(offset, r);
-- 
2.28.0



^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v3 08/15] hw/misc/bcm2835_cprman: add a PLL channel skeleton implementation
  2020-10-10 13:57 [PATCH v3 00/15] raspi: add the bcm2835 cprman clock manager Luc Michel
                   ` (6 preceding siblings ...)
  2020-10-10 13:57 ` [PATCH v3 07/15] hw/misc/bcm2835_cprman: implement PLLs behaviour Luc Michel
@ 2020-10-10 13:57 ` Luc Michel
  2020-10-10 16:05   ` Philippe Mathieu-Daudé
  2020-10-10 13:57 ` [PATCH v3 09/15] hw/misc/bcm2835_cprman: implement PLL channels behaviour Luc Michel
                   ` (9 subsequent siblings)
  17 siblings, 1 reply; 35+ messages in thread
From: Luc Michel @ 2020-10-10 13:57 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Luc Michel, Philippe Mathieu-Daudé,
	Andrew Baumann, Paul Zimmerman, Niek Linnenbank, qemu-arm,
	Havard Skinnemoen

PLLs are composed of multiple channels. Each channel outputs one clock
signal. They are modeled as one device taking the PLL generated clock as
input, and outputting a new clock.

A channel shares the CM register with its parent PLL, and has its own
A2W_CTRL register. A write to the CM register will trigger an update of
the PLL and all its channels, while a write to an A2W_CTRL channel
register will update the required channel only.

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Luc Michel <luc@lmichel.fr>
---
 include/hw/misc/bcm2835_cprman.h           |  44 ++++++
 include/hw/misc/bcm2835_cprman_internals.h | 146 +++++++++++++++++++
 hw/misc/bcm2835_cprman.c                   | 155 +++++++++++++++++++--
 3 files changed, 337 insertions(+), 8 deletions(-)

diff --git a/include/hw/misc/bcm2835_cprman.h b/include/hw/misc/bcm2835_cprman.h
index 5c442e6ff9..e1a1b33f8b 100644
--- a/include/hw/misc/bcm2835_cprman.h
+++ b/include/hw/misc/bcm2835_cprman.h
@@ -29,10 +29,35 @@ typedef enum CprmanPll {
     CPRMAN_PLLB,
 
     CPRMAN_NUM_PLL
 } CprmanPll;
 
+typedef enum CprmanPllChannel {
+    CPRMAN_PLLA_CHANNEL_DSI0 = 0,
+    CPRMAN_PLLA_CHANNEL_CORE,
+    CPRMAN_PLLA_CHANNEL_PER,
+    CPRMAN_PLLA_CHANNEL_CCP2,
+
+    CPRMAN_PLLC_CHANNEL_CORE2,
+    CPRMAN_PLLC_CHANNEL_CORE1,
+    CPRMAN_PLLC_CHANNEL_PER,
+    CPRMAN_PLLC_CHANNEL_CORE0,
+
+    CPRMAN_PLLD_CHANNEL_DSI0,
+    CPRMAN_PLLD_CHANNEL_CORE,
+    CPRMAN_PLLD_CHANNEL_PER,
+    CPRMAN_PLLD_CHANNEL_DSI1,
+
+    CPRMAN_PLLH_CHANNEL_AUX,
+    CPRMAN_PLLH_CHANNEL_RCAL,
+    CPRMAN_PLLH_CHANNEL_PIX,
+
+    CPRMAN_PLLB_CHANNEL_ARM,
+
+    CPRMAN_NUM_PLL_CHANNEL,
+} CprmanPllChannel;
+
 typedef struct CprmanPllState {
     /*< private >*/
     DeviceState parent_obj;
 
     /*< public >*/
@@ -46,18 +71,37 @@ typedef struct CprmanPllState {
 
     Clock *xosc_in;
     Clock *out;
 } CprmanPllState;
 
+typedef struct CprmanPllChannelState {
+    /*< private >*/
+    DeviceState parent_obj;
+
+    /*< public >*/
+    CprmanPllChannel id;
+    CprmanPll parent;
+
+    uint32_t *reg_cm;
+    uint32_t hold_mask;
+    uint32_t load_mask;
+    uint32_t *reg_a2w_ctrl;
+    int fixed_divider;
+
+    Clock *pll_in;
+    Clock *out;
+} CprmanPllChannelState;
+
 struct BCM2835CprmanState {
     /*< private >*/
     SysBusDevice parent_obj;
 
     /*< public >*/
     MemoryRegion iomem;
 
     CprmanPllState plls[CPRMAN_NUM_PLL];
+    CprmanPllChannelState channels[CPRMAN_NUM_PLL_CHANNEL];
 
     uint32_t regs[CPRMAN_NUM_REGS];
     uint32_t xosc_freq;
 
     Clock *xosc;
diff --git a/include/hw/misc/bcm2835_cprman_internals.h b/include/hw/misc/bcm2835_cprman_internals.h
index 7aa46c6e18..7409ddb024 100644
--- a/include/hw/misc/bcm2835_cprman_internals.h
+++ b/include/hw/misc/bcm2835_cprman_internals.h
@@ -11,13 +11,16 @@
 
 #include "hw/registerfields.h"
 #include "hw/misc/bcm2835_cprman.h"
 
 #define TYPE_CPRMAN_PLL "bcm2835-cprman-pll"
+#define TYPE_CPRMAN_PLL_CHANNEL "bcm2835-cprman-pll-channel"
 
 DECLARE_INSTANCE_CHECKER(CprmanPllState, CPRMAN_PLL,
                          TYPE_CPRMAN_PLL)
+DECLARE_INSTANCE_CHECKER(CprmanPllChannelState, CPRMAN_PLL_CHANNEL,
+                         TYPE_CPRMAN_PLL_CHANNEL)
 
 /* Register map */
 
 /* PLLs */
 REG32(CM_PLLA, 0x104)
@@ -98,10 +101,35 @@ REG32(A2W_PLLA_FRAC, 0x1200)
 REG32(A2W_PLLC_FRAC, 0x1220)
 REG32(A2W_PLLD_FRAC, 0x1240)
 REG32(A2W_PLLH_FRAC, 0x1260)
 REG32(A2W_PLLB_FRAC, 0x12e0)
 
+/* PLL channels */
+REG32(A2W_PLLA_DSI0, 0x1300)
+    FIELD(A2W_PLLx_CHANNELy, DIV, 0, 8)
+    FIELD(A2W_PLLx_CHANNELy, DISABLE, 8, 1)
+REG32(A2W_PLLA_CORE, 0x1400)
+REG32(A2W_PLLA_PER, 0x1500)
+REG32(A2W_PLLA_CCP2, 0x1600)
+
+REG32(A2W_PLLC_CORE2, 0x1320)
+REG32(A2W_PLLC_CORE1, 0x1420)
+REG32(A2W_PLLC_PER, 0x1520)
+REG32(A2W_PLLC_CORE0, 0x1620)
+
+REG32(A2W_PLLD_DSI0, 0x1340)
+REG32(A2W_PLLD_CORE, 0x1440)
+REG32(A2W_PLLD_PER, 0x1540)
+REG32(A2W_PLLD_DSI1, 0x1640)
+
+REG32(A2W_PLLH_AUX, 0x1360)
+REG32(A2W_PLLH_RCAL, 0x1460)
+REG32(A2W_PLLH_PIX, 0x1560)
+REG32(A2W_PLLH_STS, 0x1660)
+
+REG32(A2W_PLLB_ARM, 0x13e0)
+
 /* misc registers */
 REG32(CM_LOCK, 0x114)
     FIELD(CM_LOCK, FLOCKH, 12, 1)
     FIELD(CM_LOCK, FLOCKD, 11, 1)
     FIELD(CM_LOCK, FLOCKC, 10, 1)
@@ -171,6 +199,124 @@ static inline void set_pll_init_info(BCM2835CprmanState *s,
     pll->reg_a2w_ana = &s->regs[PLL_INIT_INFO[id].a2w_ana_offset];
     pll->prediv_mask = PLL_INIT_INFO[id].prediv_mask;
     pll->reg_a2w_frac = &s->regs[PLL_INIT_INFO[id].a2w_frac_offset];
 }
 
+
+/* PLL channel init info */
+typedef struct PLLChannelInitInfo {
+    const char *name;
+    CprmanPll parent;
+    size_t cm_offset;
+    uint32_t cm_hold_mask;
+    uint32_t cm_load_mask;
+    size_t a2w_ctrl_offset;
+    unsigned int fixed_divider;
+} PLLChannelInitInfo;
+
+#define FILL_PLL_CHANNEL_INIT_INFO_common(pll_, channel_)            \
+    .parent = CPRMAN_ ## pll_,                                       \
+    .cm_offset = R_CM_ ## pll_,                                      \
+    .cm_load_mask = R_CM_ ## pll_ ## _ ## LOAD ## channel_ ## _MASK, \
+    .a2w_ctrl_offset = R_A2W_ ## pll_ ## _ ## channel_
+
+#define FILL_PLL_CHANNEL_INIT_INFO(pll_, channel_)                   \
+    FILL_PLL_CHANNEL_INIT_INFO_common(pll_, channel_),               \
+    .cm_hold_mask = R_CM_ ## pll_ ## _ ## HOLD ## channel_ ## _MASK, \
+    .fixed_divider = 1
+
+#define FILL_PLL_CHANNEL_INIT_INFO_nohold(pll_, channel_) \
+    FILL_PLL_CHANNEL_INIT_INFO_common(pll_, channel_),    \
+    .cm_hold_mask = 0
+
+static PLLChannelInitInfo PLL_CHANNEL_INIT_INFO[] = {
+    [CPRMAN_PLLA_CHANNEL_DSI0] = {
+        .name = "plla-dsi0",
+        FILL_PLL_CHANNEL_INIT_INFO(PLLA, DSI0),
+    },
+    [CPRMAN_PLLA_CHANNEL_CORE] = {
+        .name = "plla-core",
+        FILL_PLL_CHANNEL_INIT_INFO(PLLA, CORE),
+    },
+    [CPRMAN_PLLA_CHANNEL_PER] = {
+        .name = "plla-per",
+        FILL_PLL_CHANNEL_INIT_INFO(PLLA, PER),
+    },
+    [CPRMAN_PLLA_CHANNEL_CCP2] = {
+        .name = "plla-ccp2",
+        FILL_PLL_CHANNEL_INIT_INFO(PLLA, CCP2),
+    },
+
+    [CPRMAN_PLLC_CHANNEL_CORE2] = {
+        .name = "pllc-core2",
+        FILL_PLL_CHANNEL_INIT_INFO(PLLC, CORE2),
+    },
+    [CPRMAN_PLLC_CHANNEL_CORE1] = {
+        .name = "pllc-core1",
+        FILL_PLL_CHANNEL_INIT_INFO(PLLC, CORE1),
+    },
+    [CPRMAN_PLLC_CHANNEL_PER] = {
+        .name = "pllc-per",
+        FILL_PLL_CHANNEL_INIT_INFO(PLLC, PER),
+    },
+    [CPRMAN_PLLC_CHANNEL_CORE0] = {
+        .name = "pllc-core0",
+        FILL_PLL_CHANNEL_INIT_INFO(PLLC, CORE0),
+    },
+
+    [CPRMAN_PLLD_CHANNEL_DSI0] = {
+        .name = "plld-dsi0",
+        FILL_PLL_CHANNEL_INIT_INFO(PLLD, DSI0),
+    },
+    [CPRMAN_PLLD_CHANNEL_CORE] = {
+        .name = "plld-core",
+        FILL_PLL_CHANNEL_INIT_INFO(PLLD, CORE),
+    },
+    [CPRMAN_PLLD_CHANNEL_PER] = {
+        .name = "plld-per",
+        FILL_PLL_CHANNEL_INIT_INFO(PLLD, PER),
+    },
+    [CPRMAN_PLLD_CHANNEL_DSI1] = {
+        .name = "plld-dsi1",
+        FILL_PLL_CHANNEL_INIT_INFO(PLLD, DSI1),
+    },
+
+    [CPRMAN_PLLH_CHANNEL_AUX] = {
+        .name = "pllh-aux",
+        .fixed_divider = 1,
+        FILL_PLL_CHANNEL_INIT_INFO_nohold(PLLH, AUX),
+    },
+    [CPRMAN_PLLH_CHANNEL_RCAL] = {
+        .name = "pllh-rcal",
+        .fixed_divider = 10,
+        FILL_PLL_CHANNEL_INIT_INFO_nohold(PLLH, RCAL),
+    },
+    [CPRMAN_PLLH_CHANNEL_PIX] = {
+        .name = "pllh-pix",
+        .fixed_divider = 10,
+        FILL_PLL_CHANNEL_INIT_INFO_nohold(PLLH, PIX),
+    },
+
+    [CPRMAN_PLLB_CHANNEL_ARM] = {
+        .name = "pllb-arm",
+        FILL_PLL_CHANNEL_INIT_INFO(PLLB, ARM),
+    },
+};
+
+#undef FILL_PLL_CHANNEL_INIT_INFO_nohold
+#undef FILL_PLL_CHANNEL_INIT_INFO
+#undef FILL_PLL_CHANNEL_INIT_INFO_common
+
+static inline void set_pll_channel_init_info(BCM2835CprmanState *s,
+                                             CprmanPllChannelState *channel,
+                                             CprmanPllChannel id)
+{
+    channel->id = id;
+    channel->parent = PLL_CHANNEL_INIT_INFO[id].parent;
+    channel->reg_cm = &s->regs[PLL_CHANNEL_INIT_INFO[id].cm_offset];
+    channel->hold_mask = PLL_CHANNEL_INIT_INFO[id].cm_hold_mask;
+    channel->load_mask = PLL_CHANNEL_INIT_INFO[id].cm_load_mask;
+    channel->reg_a2w_ctrl = &s->regs[PLL_CHANNEL_INIT_INFO[id].a2w_ctrl_offset];
+    channel->fixed_divider = PLL_CHANNEL_INIT_INFO[id].fixed_divider;
+}
+
 #endif
diff --git a/hw/misc/bcm2835_cprman.c b/hw/misc/bcm2835_cprman.c
index 144bcc289d..12fa78181b 100644
--- a/hw/misc/bcm2835_cprman.c
+++ b/hw/misc/bcm2835_cprman.c
@@ -130,10 +130,73 @@ static const TypeInfo cprman_pll_info = {
     .class_init = pll_class_init,
     .instance_init = pll_init,
 };
 
 
+/* PLL channel */
+
+static void pll_channel_update(CprmanPllChannelState *channel)
+{
+    clock_update(channel->out, 0);
+}
+
+/* Update a PLL and all its channels */
+static void pll_update_all_channels(BCM2835CprmanState *s,
+                                    CprmanPllState *pll)
+{
+    size_t i;
+
+    pll_update(pll);
+
+    for (i = 0; i < CPRMAN_NUM_PLL_CHANNEL; i++) {
+        CprmanPllChannelState *channel = &s->channels[i];
+        if (channel->parent == pll->id) {
+            pll_channel_update(channel);
+        }
+    }
+}
+
+static void pll_channel_pll_in_update(void *opaque)
+{
+    pll_channel_update(CPRMAN_PLL_CHANNEL(opaque));
+}
+
+static void pll_channel_init(Object *obj)
+{
+    CprmanPllChannelState *s = CPRMAN_PLL_CHANNEL(obj);
+
+    s->pll_in = qdev_init_clock_in(DEVICE(s), "pll-in",
+                                   pll_channel_pll_in_update, s);
+    s->out = qdev_init_clock_out(DEVICE(s), "out");
+}
+
+static const VMStateDescription pll_channel_vmstate = {
+    .name = TYPE_CPRMAN_PLL_CHANNEL,
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_CLOCK(pll_in, CprmanPllChannelState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static void pll_channel_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->vmsd = &pll_channel_vmstate;
+}
+
+static const TypeInfo cprman_pll_channel_info = {
+    .name = TYPE_CPRMAN_PLL_CHANNEL,
+    .parent = TYPE_DEVICE,
+    .instance_size = sizeof(CprmanPllChannelState),
+    .class_init = pll_channel_class_init,
+    .instance_init = pll_channel_init,
+};
+
+
 /* CPRMAN "top level" model */
 
 static uint32_t get_cm_lock(const BCM2835CprmanState *s)
 {
     static const int CM_LOCK_MAPPING[CPRMAN_NUM_PLL] = {
@@ -172,12 +235,36 @@ static uint64_t cprman_read(void *opaque, hwaddr offset,
 
     trace_bcm2835_cprman_read(offset, r);
     return r;
 }
 
-#define CASE_PLL_REGS(pll_)       \
-    case R_CM_ ## pll_:           \
+static inline void update_pll_and_channels_from_cm(BCM2835CprmanState *s,
+                                                   size_t idx)
+{
+    size_t i;
+
+    for (i = 0; i < CPRMAN_NUM_PLL; i++) {
+        if (PLL_INIT_INFO[i].cm_offset == idx) {
+            pll_update_all_channels(s, &s->plls[i]);
+            return;
+        }
+    }
+}
+
+static inline void update_channel_from_a2w(BCM2835CprmanState *s, size_t idx)
+{
+    size_t i;
+
+    for (i = 0; i < CPRMAN_NUM_PLL_CHANNEL; i++) {
+        if (PLL_CHANNEL_INIT_INFO[i].a2w_ctrl_offset == idx) {
+            pll_channel_update(&s->channels[i]);
+            return;
+        }
+    }
+}
+
+#define CASE_PLL_A2W_REGS(pll_) \
     case R_A2W_ ## pll_ ## _CTRL: \
     case R_A2W_ ## pll_ ## _ANA0: \
     case R_A2W_ ## pll_ ## _ANA1: \
     case R_A2W_ ## pll_ ## _ANA2: \
     case R_A2W_ ## pll_ ## _ANA3: \
@@ -198,33 +285,61 @@ static void cprman_write(void *opaque, hwaddr offset,
 
     trace_bcm2835_cprman_write(offset, value);
     s->regs[idx] = value;
 
     switch (idx) {
-    CASE_PLL_REGS(PLLA) :
+    case R_CM_PLLA ... R_CM_PLLH:
+    case R_CM_PLLB:
+        /*
+         * A given CM_PLLx register is shared by both the PLL and the channels
+         * of this PLL.
+         */
+        update_pll_and_channels_from_cm(s, idx);
+        break;
+
+    CASE_PLL_A2W_REGS(PLLA) :
         pll_update(&s->plls[CPRMAN_PLLA]);
         break;
 
-    CASE_PLL_REGS(PLLC) :
+    CASE_PLL_A2W_REGS(PLLC) :
         pll_update(&s->plls[CPRMAN_PLLC]);
         break;
 
-    CASE_PLL_REGS(PLLD) :
+    CASE_PLL_A2W_REGS(PLLD) :
         pll_update(&s->plls[CPRMAN_PLLD]);
         break;
 
-    CASE_PLL_REGS(PLLH) :
+    CASE_PLL_A2W_REGS(PLLH) :
         pll_update(&s->plls[CPRMAN_PLLH]);
         break;
 
-    CASE_PLL_REGS(PLLB) :
+    CASE_PLL_A2W_REGS(PLLB) :
         pll_update(&s->plls[CPRMAN_PLLB]);
         break;
+
+    case R_A2W_PLLA_DSI0:
+    case R_A2W_PLLA_CORE:
+    case R_A2W_PLLA_PER:
+    case R_A2W_PLLA_CCP2:
+    case R_A2W_PLLC_CORE2:
+    case R_A2W_PLLC_CORE1:
+    case R_A2W_PLLC_PER:
+    case R_A2W_PLLC_CORE0:
+    case R_A2W_PLLD_DSI0:
+    case R_A2W_PLLD_CORE:
+    case R_A2W_PLLD_PER:
+    case R_A2W_PLLD_DSI1:
+    case R_A2W_PLLH_AUX:
+    case R_A2W_PLLH_RCAL:
+    case R_A2W_PLLH_PIX:
+    case R_A2W_PLLB_ARM:
+        update_channel_from_a2w(s, idx);
+        break;
     }
 }
 
-#undef CASE_PLL_REGS
+#undef CASE_PLL_A2W_REGS
 
 static const MemoryRegionOps cprman_ops = {
     .read = cprman_read,
     .write = cprman_write,
     .endianness = DEVICE_LITTLE_ENDIAN,
@@ -252,10 +367,14 @@ static void cprman_reset(DeviceState *dev)
 
     for (i = 0; i < CPRMAN_NUM_PLL; i++) {
         device_cold_reset(DEVICE(&s->plls[i]));
     }
 
+    for (i = 0; i < CPRMAN_NUM_PLL_CHANNEL; i++) {
+        device_cold_reset(DEVICE(&s->channels[i]));
+    }
+
     clock_update_hz(s->xosc, s->xosc_freq);
 }
 
 static void cprman_init(Object *obj)
 {
@@ -266,10 +385,17 @@ static void cprman_init(Object *obj)
         object_initialize_child(obj, PLL_INIT_INFO[i].name,
                                 &s->plls[i], TYPE_CPRMAN_PLL);
         set_pll_init_info(s, &s->plls[i], i);
     }
 
+    for (i = 0; i < CPRMAN_NUM_PLL_CHANNEL; i++) {
+        object_initialize_child(obj, PLL_CHANNEL_INIT_INFO[i].name,
+                                &s->channels[i],
+                                TYPE_CPRMAN_PLL_CHANNEL);
+        set_pll_channel_init_info(s, &s->channels[i], i);
+    }
+
     s->xosc = clock_new(obj, "xosc");
 
     memory_region_init_io(&s->iomem, obj, &cprman_ops,
                           s, "bcm2835-cprman", 0x2000);
     sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->iomem);
@@ -287,10 +413,22 @@ static void cprman_realize(DeviceState *dev, Error **errp)
 
         if (!qdev_realize(DEVICE(pll), NULL, errp)) {
             return;
         }
     }
+
+    for (i = 0; i < CPRMAN_NUM_PLL_CHANNEL; i++) {
+        CprmanPllChannelState *channel = &s->channels[i];
+        CprmanPll parent = PLL_CHANNEL_INIT_INFO[i].parent;
+        Clock *parent_clk = s->plls[parent].out;
+
+        clock_set_source(channel->pll_in, parent_clk);
+
+        if (!qdev_realize(DEVICE(channel), NULL, errp)) {
+            return;
+        }
+    }
 }
 
 static const VMStateDescription cprman_vmstate = {
     .name = TYPE_BCM2835_CPRMAN,
     .version_id = 1,
@@ -326,8 +464,9 @@ static const TypeInfo cprman_info = {
 
 static void cprman_register_types(void)
 {
     type_register_static(&cprman_info);
     type_register_static(&cprman_pll_info);
+    type_register_static(&cprman_pll_channel_info);
 }
 
 type_init(cprman_register_types);
-- 
2.28.0



^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v3 09/15] hw/misc/bcm2835_cprman: implement PLL channels behaviour
  2020-10-10 13:57 [PATCH v3 00/15] raspi: add the bcm2835 cprman clock manager Luc Michel
                   ` (7 preceding siblings ...)
  2020-10-10 13:57 ` [PATCH v3 08/15] hw/misc/bcm2835_cprman: add a PLL channel skeleton implementation Luc Michel
@ 2020-10-10 13:57 ` Luc Michel
  2020-10-10 13:57 ` [PATCH v3 10/15] hw/misc/bcm2835_cprman: add a clock mux skeleton implementation Luc Michel
                   ` (8 subsequent siblings)
  17 siblings, 0 replies; 35+ messages in thread
From: Luc Michel @ 2020-10-10 13:57 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Luc Michel, Philippe Mathieu-Daudé,
	Andrew Baumann, Paul Zimmerman, Niek Linnenbank, qemu-arm,
	Havard Skinnemoen

A PLL channel is able to further divide the generated PLL frequency.
The divider is given in the CTRL_A2W register. Some channels have an
additional fixed divider which is always applied to the signal.

Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Luc Michel <luc@lmichel.fr>
---
 hw/misc/bcm2835_cprman.c | 33 ++++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/hw/misc/bcm2835_cprman.c b/hw/misc/bcm2835_cprman.c
index 12fa78181b..71c1d7b9e7 100644
--- a/hw/misc/bcm2835_cprman.c
+++ b/hw/misc/bcm2835_cprman.c
@@ -132,13 +132,44 @@ static const TypeInfo cprman_pll_info = {
 };
 
 
 /* PLL channel */
 
+static bool pll_channel_is_enabled(CprmanPllChannelState *channel)
+{
+    /*
+     * XXX I'm not sure of the purpose of the LOAD field. The Linux driver does
+     * not set it when enabling the channel, but does clear it when disabling
+     * it.
+     */
+    return !FIELD_EX32(*channel->reg_a2w_ctrl, A2W_PLLx_CHANNELy, DISABLE)
+        && !(*channel->reg_cm & channel->hold_mask);
+}
+
 static void pll_channel_update(CprmanPllChannelState *channel)
 {
-    clock_update(channel->out, 0);
+    uint64_t freq, div;
+
+    if (!pll_channel_is_enabled(channel)) {
+        clock_update(channel->out, 0);
+        return;
+    }
+
+    div = FIELD_EX32(*channel->reg_a2w_ctrl, A2W_PLLx_CHANNELy, DIV);
+
+    if (!div) {
+        /*
+         * It seems that when the divider value is 0, it is considered as
+         * being maximum by the hardware (see the Linux driver).
+         */
+        div = R_A2W_PLLx_CHANNELy_DIV_MASK;
+    }
+
+    /* Some channels have an additional fixed divider */
+    freq = clock_get_hz(channel->pll_in) / (div * channel->fixed_divider);
+
+    clock_update_hz(channel->out, freq);
 }
 
 /* Update a PLL and all its channels */
 static void pll_update_all_channels(BCM2835CprmanState *s,
                                     CprmanPllState *pll)
-- 
2.28.0



^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v3 10/15] hw/misc/bcm2835_cprman: add a clock mux skeleton implementation
  2020-10-10 13:57 [PATCH v3 00/15] raspi: add the bcm2835 cprman clock manager Luc Michel
                   ` (8 preceding siblings ...)
  2020-10-10 13:57 ` [PATCH v3 09/15] hw/misc/bcm2835_cprman: implement PLL channels behaviour Luc Michel
@ 2020-10-10 13:57 ` Luc Michel
  2020-10-10 13:57 ` [PATCH v3 11/15] hw/misc/bcm2835_cprman: implement clock mux behaviour Luc Michel
                   ` (7 subsequent siblings)
  17 siblings, 0 replies; 35+ messages in thread
From: Luc Michel @ 2020-10-10 13:57 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Luc Michel, Philippe Mathieu-Daudé,
	Andrew Baumann, Paul Zimmerman, Niek Linnenbank, qemu-arm,
	Havard Skinnemoen

The clock multiplexers are the last clock stage in the CPRMAN. Each mux
outputs one clock signal that goes out of the CPRMAN to the SoC
peripherals.

Each mux has at most 10 sources. The sources 0 to 3 are common to all
muxes. They are:
   0. ground (no clock signal)
   1. the main oscillator (xosc)
   2. "test debug 0" clock
   3. "test debug 1" clock

Test debug 0 and 1 are actual clock muxes that can be used as sources to
other muxes (for debug purpose).

Sources 4 to 9 are mux specific and can be unpopulated (grounded). Those
sources are fed by the PLL channels outputs.

One corner case exists for DSI0E and DSI0P muxes. They have their source
number 4 connected to an intermediate multiplexer that can select
between PLLA-DSI0 and PLLD-DSI0 channel. This multiplexer is called
DSI0HSCK and is not a clock mux as such. It is really a simple mux from
the hardware point of view (see https://elinux.org/The_Undocumented_Pi).
This mux is not implemented in this commit.

Note that there is some muxes for which sources are unknown (because of
a lack of documentation). For those cases all the sources are connected
to ground in this implementation.

Each clock mux output is exported by the CPRMAN at the qdev level,
adding the suffix '-out' to the mux name to form the output clock name.
(E.g. the 'uart' mux sees its output exported as 'uart-out' at the
CPRMAN level.)

Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Luc Michel <luc@lmichel.fr>
---
 include/hw/misc/bcm2835_cprman.h           |  85 +++++
 include/hw/misc/bcm2835_cprman_internals.h | 422 +++++++++++++++++++++
 hw/misc/bcm2835_cprman.c                   | 151 ++++++++
 3 files changed, 658 insertions(+)

diff --git a/include/hw/misc/bcm2835_cprman.h b/include/hw/misc/bcm2835_cprman.h
index e1a1b33f8b..0fc8f68845 100644
--- a/include/hw/misc/bcm2835_cprman.h
+++ b/include/hw/misc/bcm2835_cprman.h
@@ -52,12 +52,73 @@ typedef enum CprmanPllChannel {
     CPRMAN_PLLH_CHANNEL_PIX,
 
     CPRMAN_PLLB_CHANNEL_ARM,
 
     CPRMAN_NUM_PLL_CHANNEL,
+
+    /* Special values used when connecting clock sources to clocks */
+    CPRMAN_CLOCK_SRC_NORMAL = -1,
+    CPRMAN_CLOCK_SRC_FORCE_GROUND = -2,
+    CPRMAN_CLOCK_SRC_DSI0HSCK = -3,
 } CprmanPllChannel;
 
+typedef enum CprmanClockMux {
+    CPRMAN_CLOCK_GNRIC,
+    CPRMAN_CLOCK_VPU,
+    CPRMAN_CLOCK_SYS,
+    CPRMAN_CLOCK_PERIA,
+    CPRMAN_CLOCK_PERII,
+    CPRMAN_CLOCK_H264,
+    CPRMAN_CLOCK_ISP,
+    CPRMAN_CLOCK_V3D,
+    CPRMAN_CLOCK_CAM0,
+    CPRMAN_CLOCK_CAM1,
+    CPRMAN_CLOCK_CCP2,
+    CPRMAN_CLOCK_DSI0E,
+    CPRMAN_CLOCK_DSI0P,
+    CPRMAN_CLOCK_DPI,
+    CPRMAN_CLOCK_GP0,
+    CPRMAN_CLOCK_GP1,
+    CPRMAN_CLOCK_GP2,
+    CPRMAN_CLOCK_HSM,
+    CPRMAN_CLOCK_OTP,
+    CPRMAN_CLOCK_PCM,
+    CPRMAN_CLOCK_PWM,
+    CPRMAN_CLOCK_SLIM,
+    CPRMAN_CLOCK_SMI,
+    CPRMAN_CLOCK_TEC,
+    CPRMAN_CLOCK_TD0,
+    CPRMAN_CLOCK_TD1,
+    CPRMAN_CLOCK_TSENS,
+    CPRMAN_CLOCK_TIMER,
+    CPRMAN_CLOCK_UART,
+    CPRMAN_CLOCK_VEC,
+    CPRMAN_CLOCK_PULSE,
+    CPRMAN_CLOCK_SDC,
+    CPRMAN_CLOCK_ARM,
+    CPRMAN_CLOCK_AVEO,
+    CPRMAN_CLOCK_EMMC,
+    CPRMAN_CLOCK_EMMC2,
+
+    CPRMAN_NUM_CLOCK_MUX
+} CprmanClockMux;
+
+typedef enum CprmanClockMuxSource {
+    CPRMAN_CLOCK_SRC_GND = 0,
+    CPRMAN_CLOCK_SRC_XOSC,
+    CPRMAN_CLOCK_SRC_TD0,
+    CPRMAN_CLOCK_SRC_TD1,
+    CPRMAN_CLOCK_SRC_PLLA,
+    CPRMAN_CLOCK_SRC_PLLC,
+    CPRMAN_CLOCK_SRC_PLLD,
+    CPRMAN_CLOCK_SRC_PLLH,
+    CPRMAN_CLOCK_SRC_PLLC_CORE1,
+    CPRMAN_CLOCK_SRC_PLLC_CORE2,
+
+    CPRMAN_NUM_CLOCK_MUX_SRC
+} CprmanClockMuxSource;
+
 typedef struct CprmanPllState {
     /*< private >*/
     DeviceState parent_obj;
 
     /*< public >*/
@@ -89,22 +150,46 @@ typedef struct CprmanPllChannelState {
 
     Clock *pll_in;
     Clock *out;
 } CprmanPllChannelState;
 
+typedef struct CprmanClockMuxState {
+    /*< private >*/
+    DeviceState parent_obj;
+
+    /*< public >*/
+    CprmanClockMux id;
+
+    uint32_t *reg_ctl;
+    uint32_t *reg_div;
+    int int_bits;
+    int frac_bits;
+
+    Clock *srcs[CPRMAN_NUM_CLOCK_MUX_SRC];
+    Clock *out;
+
+    /*
+     * Used by clock srcs update callback to retrieve both the clock and the
+     * source number.
+     */
+    struct CprmanClockMuxState *backref[CPRMAN_NUM_CLOCK_MUX_SRC];
+} CprmanClockMuxState;
+
 struct BCM2835CprmanState {
     /*< private >*/
     SysBusDevice parent_obj;
 
     /*< public >*/
     MemoryRegion iomem;
 
     CprmanPllState plls[CPRMAN_NUM_PLL];
     CprmanPllChannelState channels[CPRMAN_NUM_PLL_CHANNEL];
+    CprmanClockMuxState clock_muxes[CPRMAN_NUM_CLOCK_MUX];
 
     uint32_t regs[CPRMAN_NUM_REGS];
     uint32_t xosc_freq;
 
     Clock *xosc;
+    Clock *gnd;
 };
 
 #endif
diff --git a/include/hw/misc/bcm2835_cprman_internals.h b/include/hw/misc/bcm2835_cprman_internals.h
index 7409ddb024..0305448bbc 100644
--- a/include/hw/misc/bcm2835_cprman_internals.h
+++ b/include/hw/misc/bcm2835_cprman_internals.h
@@ -12,15 +12,18 @@
 #include "hw/registerfields.h"
 #include "hw/misc/bcm2835_cprman.h"
 
 #define TYPE_CPRMAN_PLL "bcm2835-cprman-pll"
 #define TYPE_CPRMAN_PLL_CHANNEL "bcm2835-cprman-pll-channel"
+#define TYPE_CPRMAN_CLOCK_MUX "bcm2835-cprman-clock-mux"
 
 DECLARE_INSTANCE_CHECKER(CprmanPllState, CPRMAN_PLL,
                          TYPE_CPRMAN_PLL)
 DECLARE_INSTANCE_CHECKER(CprmanPllChannelState, CPRMAN_PLL_CHANNEL,
                          TYPE_CPRMAN_PLL_CHANNEL)
+DECLARE_INSTANCE_CHECKER(CprmanClockMuxState, CPRMAN_CLOCK_MUX,
+                         TYPE_CPRMAN_CLOCK_MUX)
 
 /* Register map */
 
 /* PLLs */
 REG32(CM_PLLA, 0x104)
@@ -126,10 +129,94 @@ REG32(A2W_PLLH_RCAL, 0x1460)
 REG32(A2W_PLLH_PIX, 0x1560)
 REG32(A2W_PLLH_STS, 0x1660)
 
 REG32(A2W_PLLB_ARM, 0x13e0)
 
+/* Clock muxes */
+REG32(CM_GNRICCTL, 0x000)
+    FIELD(CM_CLOCKx_CTL, SRC, 0, 4)
+    FIELD(CM_CLOCKx_CTL, ENABLE, 4, 1)
+    FIELD(CM_CLOCKx_CTL, KILL, 5, 1)
+    FIELD(CM_CLOCKx_CTL, GATE, 6, 1)
+    FIELD(CM_CLOCKx_CTL, BUSY, 7, 1)
+    FIELD(CM_CLOCKx_CTL, BUSYD, 8, 1)
+    FIELD(CM_CLOCKx_CTL, MASH, 9, 2)
+    FIELD(CM_CLOCKx_CTL, FLIP, 11, 1)
+REG32(CM_GNRICDIV, 0x004)
+    FIELD(CM_CLOCKx_DIV, FRAC, 0, 12)
+REG32(CM_VPUCTL, 0x008)
+REG32(CM_VPUDIV, 0x00c)
+REG32(CM_SYSCTL, 0x010)
+REG32(CM_SYSDIV, 0x014)
+REG32(CM_PERIACTL, 0x018)
+REG32(CM_PERIADIV, 0x01c)
+REG32(CM_PERIICTL, 0x020)
+REG32(CM_PERIIDIV, 0x024)
+REG32(CM_H264CTL, 0x028)
+REG32(CM_H264DIV, 0x02c)
+REG32(CM_ISPCTL, 0x030)
+REG32(CM_ISPDIV, 0x034)
+REG32(CM_V3DCTL, 0x038)
+REG32(CM_V3DDIV, 0x03c)
+REG32(CM_CAM0CTL, 0x040)
+REG32(CM_CAM0DIV, 0x044)
+REG32(CM_CAM1CTL, 0x048)
+REG32(CM_CAM1DIV, 0x04c)
+REG32(CM_CCP2CTL, 0x050)
+REG32(CM_CCP2DIV, 0x054)
+REG32(CM_DSI0ECTL, 0x058)
+REG32(CM_DSI0EDIV, 0x05c)
+REG32(CM_DSI0PCTL, 0x060)
+REG32(CM_DSI0PDIV, 0x064)
+REG32(CM_DPICTL, 0x068)
+REG32(CM_DPIDIV, 0x06c)
+REG32(CM_GP0CTL, 0x070)
+REG32(CM_GP0DIV, 0x074)
+REG32(CM_GP1CTL, 0x078)
+REG32(CM_GP1DIV, 0x07c)
+REG32(CM_GP2CTL, 0x080)
+REG32(CM_GP2DIV, 0x084)
+REG32(CM_HSMCTL, 0x088)
+REG32(CM_HSMDIV, 0x08c)
+REG32(CM_OTPCTL, 0x090)
+REG32(CM_OTPDIV, 0x094)
+REG32(CM_PCMCTL, 0x098)
+REG32(CM_PCMDIV, 0x09c)
+REG32(CM_PWMCTL, 0x0a0)
+REG32(CM_PWMDIV, 0x0a4)
+REG32(CM_SLIMCTL, 0x0a8)
+REG32(CM_SLIMDIV, 0x0ac)
+REG32(CM_SMICTL, 0x0b0)
+REG32(CM_SMIDIV, 0x0b4)
+REG32(CM_TCNTCTL, 0x0c0)
+REG32(CM_TCNTCNT, 0x0c4)
+REG32(CM_TECCTL, 0x0c8)
+REG32(CM_TECDIV, 0x0cc)
+REG32(CM_TD0CTL, 0x0d0)
+REG32(CM_TD0DIV, 0x0d4)
+REG32(CM_TD1CTL, 0x0d8)
+REG32(CM_TD1DIV, 0x0dc)
+REG32(CM_TSENSCTL, 0x0e0)
+REG32(CM_TSENSDIV, 0x0e4)
+REG32(CM_TIMERCTL, 0x0e8)
+REG32(CM_TIMERDIV, 0x0ec)
+REG32(CM_UARTCTL, 0x0f0)
+REG32(CM_UARTDIV, 0x0f4)
+REG32(CM_VECCTL, 0x0f8)
+REG32(CM_VECDIV, 0x0fc)
+REG32(CM_PULSECTL, 0x190)
+REG32(CM_PULSEDIV, 0x194)
+REG32(CM_SDCCTL, 0x1a8)
+REG32(CM_SDCDIV, 0x1ac)
+REG32(CM_ARMCTL, 0x1b0)
+REG32(CM_AVEOCTL, 0x1b8)
+REG32(CM_AVEODIV, 0x1bc)
+REG32(CM_EMMCCTL, 0x1c0)
+REG32(CM_EMMCDIV, 0x1c4)
+REG32(CM_EMMC2CTL, 0x1d0)
+REG32(CM_EMMC2DIV, 0x1d4)
+
 /* misc registers */
 REG32(CM_LOCK, 0x114)
     FIELD(CM_LOCK, FLOCKH, 12, 1)
     FIELD(CM_LOCK, FLOCKD, 11, 1)
     FIELD(CM_LOCK, FLOCKC, 10, 1)
@@ -317,6 +404,341 @@ static inline void set_pll_channel_init_info(BCM2835CprmanState *s,
     channel->load_mask = PLL_CHANNEL_INIT_INFO[id].cm_load_mask;
     channel->reg_a2w_ctrl = &s->regs[PLL_CHANNEL_INIT_INFO[id].a2w_ctrl_offset];
     channel->fixed_divider = PLL_CHANNEL_INIT_INFO[id].fixed_divider;
 }
 
+/* Clock mux init info */
+typedef struct ClockMuxInitInfo {
+    const char *name;
+    size_t cm_offset; /* cm_offset[0]->CM_CTL, cm_offset[1]->CM_DIV */
+    int int_bits;
+    int frac_bits;
+
+    CprmanPllChannel src_mapping[CPRMAN_NUM_CLOCK_MUX_SRC];
+} ClockMuxInitInfo;
+
+/*
+ * Each clock mux can have up to 10 sources. Sources 0 to 3 are always the
+ * same (ground, xosc, td0, td1). Sources 4 to 9 are mux specific, and are not
+ * always populated. The following macros catch all those cases.
+ */
+
+/* Unknown mapping. Connect everything to ground */
+#define SRC_MAPPING_INFO_unknown                          \
+    .src_mapping = {                                      \
+        CPRMAN_CLOCK_SRC_FORCE_GROUND, /* gnd */          \
+        CPRMAN_CLOCK_SRC_FORCE_GROUND, /* xosc */         \
+        CPRMAN_CLOCK_SRC_FORCE_GROUND, /* test debug 0 */ \
+        CPRMAN_CLOCK_SRC_FORCE_GROUND, /* test debug 1 */ \
+        CPRMAN_CLOCK_SRC_FORCE_GROUND, /* pll a */        \
+        CPRMAN_CLOCK_SRC_FORCE_GROUND, /* pll c */        \
+        CPRMAN_CLOCK_SRC_FORCE_GROUND, /* pll d */        \
+        CPRMAN_CLOCK_SRC_FORCE_GROUND, /* pll h */        \
+        CPRMAN_CLOCK_SRC_FORCE_GROUND, /* pll c, core1 */ \
+        CPRMAN_CLOCK_SRC_FORCE_GROUND, /* pll c, core2 */ \
+    }
+
+/* Only the oscillator and the two test debug clocks */
+#define SRC_MAPPING_INFO_xosc          \
+    .src_mapping = {                   \
+        CPRMAN_CLOCK_SRC_NORMAL,       \
+        CPRMAN_CLOCK_SRC_NORMAL,       \
+        CPRMAN_CLOCK_SRC_NORMAL,       \
+        CPRMAN_CLOCK_SRC_NORMAL,       \
+        CPRMAN_CLOCK_SRC_FORCE_GROUND, \
+        CPRMAN_CLOCK_SRC_FORCE_GROUND, \
+        CPRMAN_CLOCK_SRC_FORCE_GROUND, \
+        CPRMAN_CLOCK_SRC_FORCE_GROUND, \
+        CPRMAN_CLOCK_SRC_FORCE_GROUND, \
+        CPRMAN_CLOCK_SRC_FORCE_GROUND, \
+    }
+
+/* All the PLL "core" channels */
+#define SRC_MAPPING_INFO_core      \
+    .src_mapping = {               \
+        CPRMAN_CLOCK_SRC_NORMAL,   \
+        CPRMAN_CLOCK_SRC_NORMAL,   \
+        CPRMAN_CLOCK_SRC_NORMAL,   \
+        CPRMAN_CLOCK_SRC_NORMAL,   \
+        CPRMAN_PLLA_CHANNEL_CORE,  \
+        CPRMAN_PLLC_CHANNEL_CORE0, \
+        CPRMAN_PLLD_CHANNEL_CORE,  \
+        CPRMAN_PLLH_CHANNEL_AUX,   \
+        CPRMAN_PLLC_CHANNEL_CORE1, \
+        CPRMAN_PLLC_CHANNEL_CORE2, \
+    }
+
+/* All the PLL "per" channels */
+#define SRC_MAPPING_INFO_periph        \
+    .src_mapping = {                   \
+        CPRMAN_CLOCK_SRC_NORMAL,       \
+        CPRMAN_CLOCK_SRC_NORMAL,       \
+        CPRMAN_CLOCK_SRC_NORMAL,       \
+        CPRMAN_CLOCK_SRC_NORMAL,       \
+        CPRMAN_PLLA_CHANNEL_PER,       \
+        CPRMAN_PLLC_CHANNEL_PER,       \
+        CPRMAN_PLLD_CHANNEL_PER,       \
+        CPRMAN_CLOCK_SRC_FORCE_GROUND, \
+        CPRMAN_CLOCK_SRC_FORCE_GROUND, \
+        CPRMAN_CLOCK_SRC_FORCE_GROUND, \
+    }
+
+/*
+ * The DSI0 channels. This one got an intermediate mux between the PLL channels
+ * and the clock input.
+ */
+#define SRC_MAPPING_INFO_dsi0          \
+    .src_mapping = {                   \
+        CPRMAN_CLOCK_SRC_NORMAL,       \
+        CPRMAN_CLOCK_SRC_NORMAL,       \
+        CPRMAN_CLOCK_SRC_NORMAL,       \
+        CPRMAN_CLOCK_SRC_NORMAL,       \
+        CPRMAN_CLOCK_SRC_DSI0HSCK,     \
+        CPRMAN_CLOCK_SRC_FORCE_GROUND, \
+        CPRMAN_CLOCK_SRC_FORCE_GROUND, \
+        CPRMAN_CLOCK_SRC_FORCE_GROUND, \
+        CPRMAN_CLOCK_SRC_FORCE_GROUND, \
+        CPRMAN_CLOCK_SRC_FORCE_GROUND, \
+    }
+
+/* The DSI1 channel */
+#define SRC_MAPPING_INFO_dsi1          \
+    .src_mapping = {                   \
+        CPRMAN_CLOCK_SRC_NORMAL,       \
+        CPRMAN_CLOCK_SRC_NORMAL,       \
+        CPRMAN_CLOCK_SRC_NORMAL,       \
+        CPRMAN_CLOCK_SRC_NORMAL,       \
+        CPRMAN_PLLD_CHANNEL_DSI1,      \
+        CPRMAN_CLOCK_SRC_FORCE_GROUND, \
+        CPRMAN_CLOCK_SRC_FORCE_GROUND, \
+        CPRMAN_CLOCK_SRC_FORCE_GROUND, \
+        CPRMAN_CLOCK_SRC_FORCE_GROUND, \
+        CPRMAN_CLOCK_SRC_FORCE_GROUND, \
+    }
+
+#define FILL_CLOCK_MUX_SRC_MAPPING_INIT_INFO(kind_) \
+    SRC_MAPPING_INFO_ ## kind_
+
+#define FILL_CLOCK_MUX_INIT_INFO(clock_, kind_) \
+    .cm_offset = R_CM_ ## clock_ ## CTL,        \
+    FILL_CLOCK_MUX_SRC_MAPPING_INIT_INFO(kind_)
+
+static ClockMuxInitInfo CLOCK_MUX_INIT_INFO[] = {
+    [CPRMAN_CLOCK_GNRIC] = {
+        .name = "gnric",
+        FILL_CLOCK_MUX_INIT_INFO(GNRIC, unknown),
+    },
+    [CPRMAN_CLOCK_VPU] = {
+        .name = "vpu",
+        .int_bits = 12,
+        .frac_bits = 8,
+        FILL_CLOCK_MUX_INIT_INFO(VPU, core),
+    },
+    [CPRMAN_CLOCK_SYS] = {
+        .name = "sys",
+        FILL_CLOCK_MUX_INIT_INFO(SYS, unknown),
+    },
+    [CPRMAN_CLOCK_PERIA] = {
+        .name = "peria",
+        FILL_CLOCK_MUX_INIT_INFO(PERIA, unknown),
+    },
+    [CPRMAN_CLOCK_PERII] = {
+        .name = "perii",
+        FILL_CLOCK_MUX_INIT_INFO(PERII, unknown),
+    },
+    [CPRMAN_CLOCK_H264] = {
+        .name = "h264",
+        .int_bits = 4,
+        .frac_bits = 8,
+        FILL_CLOCK_MUX_INIT_INFO(H264, core),
+    },
+    [CPRMAN_CLOCK_ISP] = {
+        .name = "isp",
+        .int_bits = 4,
+        .frac_bits = 8,
+        FILL_CLOCK_MUX_INIT_INFO(ISP, core),
+    },
+    [CPRMAN_CLOCK_V3D] = {
+        .name = "v3d",
+        FILL_CLOCK_MUX_INIT_INFO(V3D, core),
+    },
+    [CPRMAN_CLOCK_CAM0] = {
+        .name = "cam0",
+        .int_bits = 4,
+        .frac_bits = 8,
+        FILL_CLOCK_MUX_INIT_INFO(CAM0, periph),
+    },
+    [CPRMAN_CLOCK_CAM1] = {
+        .name = "cam1",
+        .int_bits = 4,
+        .frac_bits = 8,
+        FILL_CLOCK_MUX_INIT_INFO(CAM1, periph),
+    },
+    [CPRMAN_CLOCK_CCP2] = {
+        .name = "ccp2",
+        FILL_CLOCK_MUX_INIT_INFO(CCP2, unknown),
+    },
+    [CPRMAN_CLOCK_DSI0E] = {
+        .name = "dsi0e",
+        .int_bits = 4,
+        .frac_bits = 8,
+        FILL_CLOCK_MUX_INIT_INFO(DSI0E, dsi0),
+    },
+    [CPRMAN_CLOCK_DSI0P] = {
+        .name = "dsi0p",
+        .int_bits = 0,
+        .frac_bits = 0,
+        FILL_CLOCK_MUX_INIT_INFO(DSI0P, dsi0),
+    },
+    [CPRMAN_CLOCK_DPI] = {
+        .name = "dpi",
+        .int_bits = 4,
+        .frac_bits = 8,
+        FILL_CLOCK_MUX_INIT_INFO(DPI, periph),
+    },
+    [CPRMAN_CLOCK_GP0] = {
+        .name = "gp0",
+        .int_bits = 12,
+        .frac_bits = 12,
+        FILL_CLOCK_MUX_INIT_INFO(GP0, periph),
+    },
+    [CPRMAN_CLOCK_GP1] = {
+        .name = "gp1",
+        .int_bits = 12,
+        .frac_bits = 12,
+        FILL_CLOCK_MUX_INIT_INFO(GP1, periph),
+    },
+    [CPRMAN_CLOCK_GP2] = {
+        .name = "gp2",
+        .int_bits = 12,
+        .frac_bits = 12,
+        FILL_CLOCK_MUX_INIT_INFO(GP2, periph),
+    },
+    [CPRMAN_CLOCK_HSM] = {
+        .name = "hsm",
+        .int_bits = 4,
+        .frac_bits = 8,
+        FILL_CLOCK_MUX_INIT_INFO(HSM, periph),
+    },
+    [CPRMAN_CLOCK_OTP] = {
+        .name = "otp",
+        .int_bits = 4,
+        .frac_bits = 0,
+        FILL_CLOCK_MUX_INIT_INFO(OTP, xosc),
+    },
+    [CPRMAN_CLOCK_PCM] = {
+        .name = "pcm",
+        .int_bits = 12,
+        .frac_bits = 12,
+        FILL_CLOCK_MUX_INIT_INFO(PCM, periph),
+    },
+    [CPRMAN_CLOCK_PWM] = {
+        .name = "pwm",
+        .int_bits = 12,
+        .frac_bits = 12,
+        FILL_CLOCK_MUX_INIT_INFO(PWM, periph),
+    },
+    [CPRMAN_CLOCK_SLIM] = {
+        .name = "slim",
+        .int_bits = 12,
+        .frac_bits = 12,
+        FILL_CLOCK_MUX_INIT_INFO(SLIM, periph),
+    },
+    [CPRMAN_CLOCK_SMI] = {
+        .name = "smi",
+        .int_bits = 4,
+        .frac_bits = 8,
+        FILL_CLOCK_MUX_INIT_INFO(SMI, periph),
+    },
+    [CPRMAN_CLOCK_TEC] = {
+        .name = "tec",
+        .int_bits = 6,
+        .frac_bits = 0,
+        FILL_CLOCK_MUX_INIT_INFO(TEC, xosc),
+    },
+    [CPRMAN_CLOCK_TD0] = {
+        .name = "td0",
+        FILL_CLOCK_MUX_INIT_INFO(TD0, unknown),
+    },
+    [CPRMAN_CLOCK_TD1] = {
+        .name = "td1",
+        FILL_CLOCK_MUX_INIT_INFO(TD1, unknown),
+    },
+    [CPRMAN_CLOCK_TSENS] = {
+        .name = "tsens",
+        .int_bits = 5,
+        .frac_bits = 0,
+        FILL_CLOCK_MUX_INIT_INFO(TSENS, xosc),
+    },
+    [CPRMAN_CLOCK_TIMER] = {
+        .name = "timer",
+        .int_bits = 6,
+        .frac_bits = 12,
+        FILL_CLOCK_MUX_INIT_INFO(TIMER, xosc),
+    },
+    [CPRMAN_CLOCK_UART] = {
+        .name = "uart",
+        .int_bits = 10,
+        .frac_bits = 12,
+        FILL_CLOCK_MUX_INIT_INFO(UART, periph),
+    },
+    [CPRMAN_CLOCK_VEC] = {
+        .name = "vec",
+        .int_bits = 4,
+        .frac_bits = 0,
+        FILL_CLOCK_MUX_INIT_INFO(VEC, periph),
+    },
+    [CPRMAN_CLOCK_PULSE] = {
+        .name = "pulse",
+        FILL_CLOCK_MUX_INIT_INFO(PULSE, xosc),
+    },
+    [CPRMAN_CLOCK_SDC] = {
+        .name = "sdram",
+        .int_bits = 6,
+        .frac_bits = 0,
+        FILL_CLOCK_MUX_INIT_INFO(SDC, core),
+    },
+    [CPRMAN_CLOCK_ARM] = {
+        .name = "arm",
+        FILL_CLOCK_MUX_INIT_INFO(ARM, unknown),
+    },
+    [CPRMAN_CLOCK_AVEO] = {
+        .name = "aveo",
+        .int_bits = 4,
+        .frac_bits = 0,
+        FILL_CLOCK_MUX_INIT_INFO(AVEO, periph),
+    },
+    [CPRMAN_CLOCK_EMMC] = {
+        .name = "emmc",
+        .int_bits = 4,
+        .frac_bits = 8,
+        FILL_CLOCK_MUX_INIT_INFO(EMMC, periph),
+    },
+    [CPRMAN_CLOCK_EMMC2] = {
+        .name = "emmc2",
+        .int_bits = 4,
+        .frac_bits = 8,
+        FILL_CLOCK_MUX_INIT_INFO(EMMC2, unknown),
+    },
+};
+
+#undef FILL_CLOCK_MUX_INIT_INFO
+#undef FILL_CLOCK_MUX_SRC_MAPPING_INIT_INFO
+#undef SRC_MAPPING_INFO_dsi1
+#undef SRC_MAPPING_INFO_dsi0
+#undef SRC_MAPPING_INFO_periph
+#undef SRC_MAPPING_INFO_core
+#undef SRC_MAPPING_INFO_xosc
+#undef SRC_MAPPING_INFO_unknown
+
+static inline void set_clock_mux_init_info(BCM2835CprmanState *s,
+                                           CprmanClockMuxState *mux,
+                                           CprmanClockMux id)
+{
+    mux->id = id;
+    mux->reg_ctl = &s->regs[CLOCK_MUX_INIT_INFO[id].cm_offset];
+    mux->reg_div = &s->regs[CLOCK_MUX_INIT_INFO[id].cm_offset + 1];
+    mux->int_bits = CLOCK_MUX_INIT_INFO[id].int_bits;
+    mux->frac_bits = CLOCK_MUX_INIT_INFO[id].frac_bits;
+}
+
 #endif
diff --git a/hw/misc/bcm2835_cprman.c b/hw/misc/bcm2835_cprman.c
index 71c1d7b9e7..b22170f3bc 100644
--- a/hw/misc/bcm2835_cprman.c
+++ b/hw/misc/bcm2835_cprman.c
@@ -36,10 +36,13 @@
  *          |                                          [mux]
  *          \-->[PLL]--->[PLL channel]                 [mux]
  *
  * The page at https://elinux.org/The_Undocumented_Pi gives the actual clock
  * tree configuration.
+ *
+ * The CPRMAN exposes clock outputs with the name of the clock mux suffixed
+ * with "-out" (e.g. "uart-out", "h264-out", ...).
  */
 
 #include "qemu/osdep.h"
 #include "qemu/log.h"
 #include "migration/vmstate.h"
@@ -224,10 +227,69 @@ static const TypeInfo cprman_pll_channel_info = {
     .class_init = pll_channel_class_init,
     .instance_init = pll_channel_init,
 };
 
 
+/* clock mux */
+
+static void clock_mux_update(CprmanClockMuxState *mux)
+{
+    clock_update(mux->out, 0);
+}
+
+static void clock_mux_src_update(void *opaque)
+{
+    CprmanClockMuxState **backref = opaque;
+    CprmanClockMuxState *s = *backref;
+
+    clock_mux_update(s);
+}
+
+static void clock_mux_init(Object *obj)
+{
+    CprmanClockMuxState *s = CPRMAN_CLOCK_MUX(obj);
+    size_t i;
+
+    for (i = 0; i < CPRMAN_NUM_CLOCK_MUX_SRC; i++) {
+        char *name = g_strdup_printf("srcs[%zu]", i);
+        s->backref[i] = s;
+        s->srcs[i] = qdev_init_clock_in(DEVICE(s), name,
+                                        clock_mux_src_update,
+                                        &s->backref[i]);
+        g_free(name);
+    }
+
+    s->out = qdev_init_clock_out(DEVICE(s), "out");
+}
+
+static const VMStateDescription clock_mux_vmstate = {
+    .name = TYPE_CPRMAN_CLOCK_MUX,
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_ARRAY_CLOCK(srcs, CprmanClockMuxState,
+                            CPRMAN_NUM_CLOCK_MUX_SRC),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static void clock_mux_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->vmsd = &clock_mux_vmstate;
+}
+
+static const TypeInfo cprman_clock_mux_info = {
+    .name = TYPE_CPRMAN_CLOCK_MUX,
+    .parent = TYPE_DEVICE,
+    .instance_size = sizeof(CprmanClockMuxState),
+    .class_init = clock_mux_class_init,
+    .instance_init = clock_mux_init,
+};
+
+
 /* CPRMAN "top level" model */
 
 static uint32_t get_cm_lock(const BCM2835CprmanState *s)
 {
     static const int CM_LOCK_MAPPING[CPRMAN_NUM_PLL] = {
@@ -291,10 +353,24 @@ static inline void update_channel_from_a2w(BCM2835CprmanState *s, size_t idx)
             return;
         }
     }
 }
 
+static inline void update_mux_from_cm(BCM2835CprmanState *s, size_t idx)
+{
+    size_t i;
+
+    for (i = 0; i < CPRMAN_NUM_CLOCK_MUX; i++) {
+        if ((CLOCK_MUX_INIT_INFO[i].cm_offset == idx) ||
+            (CLOCK_MUX_INIT_INFO[i].cm_offset + 4 == idx)) {
+            /* matches CM_CTL or CM_DIV mux register */
+            clock_mux_update(&s->clock_muxes[i]);
+            return;
+        }
+    }
+}
+
 #define CASE_PLL_A2W_REGS(pll_) \
     case R_A2W_ ## pll_ ## _CTRL: \
     case R_A2W_ ## pll_ ## _ANA0: \
     case R_A2W_ ## pll_ ## _ANA1: \
     case R_A2W_ ## pll_ ## _ANA2: \
@@ -363,10 +439,19 @@ static void cprman_write(void *opaque, hwaddr offset,
     case R_A2W_PLLH_RCAL:
     case R_A2W_PLLH_PIX:
     case R_A2W_PLLB_ARM:
         update_channel_from_a2w(s, idx);
         break;
+
+    case R_CM_GNRICCTL ... R_CM_SMIDIV:
+    case R_CM_TCNTCNT ... R_CM_VECDIV:
+    case R_CM_PULSECTL ... R_CM_PULSEDIV:
+    case R_CM_SDCCTL ... R_CM_ARMCTL:
+    case R_CM_AVEOCTL ... R_CM_EMMCDIV:
+    case R_CM_EMMC2CTL ... R_CM_EMMC2DIV:
+        update_mux_from_cm(s, idx);
+        break;
     }
 }
 
 #undef CASE_PLL_A2W_REGS
 
@@ -402,10 +487,14 @@ static void cprman_reset(DeviceState *dev)
 
     for (i = 0; i < CPRMAN_NUM_PLL_CHANNEL; i++) {
         device_cold_reset(DEVICE(&s->channels[i]));
     }
 
+    for (i = 0; i < CPRMAN_NUM_CLOCK_MUX; i++) {
+        device_cold_reset(DEVICE(&s->clock_muxes[i]));
+    }
+
     clock_update_hz(s->xosc, s->xosc_freq);
 }
 
 static void cprman_init(Object *obj)
 {
@@ -423,17 +512,68 @@ static void cprman_init(Object *obj)
                                 &s->channels[i],
                                 TYPE_CPRMAN_PLL_CHANNEL);
         set_pll_channel_init_info(s, &s->channels[i], i);
     }
 
+    for (i = 0; i < CPRMAN_NUM_CLOCK_MUX; i++) {
+        char *alias;
+
+        object_initialize_child(obj, CLOCK_MUX_INIT_INFO[i].name,
+                                &s->clock_muxes[i],
+                                TYPE_CPRMAN_CLOCK_MUX);
+        set_clock_mux_init_info(s, &s->clock_muxes[i], i);
+
+        /* Expose muxes output as CPRMAN outputs */
+        alias = g_strdup_printf("%s-out", CLOCK_MUX_INIT_INFO[i].name);
+        qdev_alias_clock(DEVICE(&s->clock_muxes[i]), "out", DEVICE(obj), alias);
+        g_free(alias);
+    }
+
     s->xosc = clock_new(obj, "xosc");
+    s->gnd = clock_new(obj, "gnd");
+
+    clock_set(s->gnd, 0);
 
     memory_region_init_io(&s->iomem, obj, &cprman_ops,
                           s, "bcm2835-cprman", 0x2000);
     sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->iomem);
 }
 
+static void connect_mux_sources(BCM2835CprmanState *s,
+                                CprmanClockMuxState *mux,
+                                const CprmanPllChannel *clk_mapping)
+{
+    size_t i;
+    Clock *td0 = s->clock_muxes[CPRMAN_CLOCK_TD0].out;
+    Clock *td1 = s->clock_muxes[CPRMAN_CLOCK_TD1].out;
+
+    /* For sources from 0 to 3. Source 4 to 9 are mux specific */
+    Clock * const CLK_SRC_MAPPING[] = {
+        [CPRMAN_CLOCK_SRC_GND] = s->gnd,
+        [CPRMAN_CLOCK_SRC_XOSC] = s->xosc,
+        [CPRMAN_CLOCK_SRC_TD0] = td0,
+        [CPRMAN_CLOCK_SRC_TD1] = td1,
+    };
+
+    for (i = 0; i < CPRMAN_NUM_CLOCK_MUX_SRC; i++) {
+        CprmanPllChannel mapping = clk_mapping[i];
+        Clock *src;
+
+        if (mapping == CPRMAN_CLOCK_SRC_FORCE_GROUND) {
+            src = s->gnd;
+        } else if (mapping == CPRMAN_CLOCK_SRC_DSI0HSCK) {
+            src = s->gnd; /* TODO */
+        } else if (i < CPRMAN_CLOCK_SRC_PLLA) {
+            src = CLK_SRC_MAPPING[i];
+        } else {
+            src = s->channels[mapping].out;
+        }
+
+        clock_set_source(mux->srcs[i], src);
+    }
+}
+
 static void cprman_realize(DeviceState *dev, Error **errp)
 {
     BCM2835CprmanState *s = CPRMAN(dev);
     size_t i;
 
@@ -456,10 +596,20 @@ static void cprman_realize(DeviceState *dev, Error **errp)
 
         if (!qdev_realize(DEVICE(channel), NULL, errp)) {
             return;
         }
     }
+
+    for (i = 0; i < CPRMAN_NUM_CLOCK_MUX; i++) {
+        CprmanClockMuxState *clock_mux = &s->clock_muxes[i];
+
+        connect_mux_sources(s, clock_mux, CLOCK_MUX_INIT_INFO[i].src_mapping);
+
+        if (!qdev_realize(DEVICE(clock_mux), NULL, errp)) {
+            return;
+        }
+    }
 }
 
 static const VMStateDescription cprman_vmstate = {
     .name = TYPE_BCM2835_CPRMAN,
     .version_id = 1,
@@ -496,8 +646,9 @@ static const TypeInfo cprman_info = {
 static void cprman_register_types(void)
 {
     type_register_static(&cprman_info);
     type_register_static(&cprman_pll_info);
     type_register_static(&cprman_pll_channel_info);
+    type_register_static(&cprman_clock_mux_info);
 }
 
 type_init(cprman_register_types);
-- 
2.28.0



^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v3 11/15] hw/misc/bcm2835_cprman: implement clock mux behaviour
  2020-10-10 13:57 [PATCH v3 00/15] raspi: add the bcm2835 cprman clock manager Luc Michel
                   ` (9 preceding siblings ...)
  2020-10-10 13:57 ` [PATCH v3 10/15] hw/misc/bcm2835_cprman: add a clock mux skeleton implementation Luc Michel
@ 2020-10-10 13:57 ` Luc Michel
  2020-10-10 15:52   ` Philippe Mathieu-Daudé
  2020-10-10 13:57 ` [PATCH v3 12/15] hw/misc/bcm2835_cprman: add the DSI0HSCK multiplexer Luc Michel
                   ` (6 subsequent siblings)
  17 siblings, 1 reply; 35+ messages in thread
From: Luc Michel @ 2020-10-10 13:57 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Luc Michel, Philippe Mathieu-Daudé,
	Andrew Baumann, Paul Zimmerman, Niek Linnenbank, qemu-arm,
	Havard Skinnemoen

A clock mux can be configured to select one of its 10 sources through
the CM_CTL register. It also embeds yet another clock divider, composed
of an integer part and a fractional part. The number of bits of each
part is mux dependent.

Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Luc Michel <luc@lmichel.fr>
---
 hw/misc/bcm2835_cprman.c | 53 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 52 insertions(+), 1 deletion(-)

diff --git a/hw/misc/bcm2835_cprman.c b/hw/misc/bcm2835_cprman.c
index b22170f3bc..919a55aa23 100644
--- a/hw/misc/bcm2835_cprman.c
+++ b/hw/misc/bcm2835_cprman.c
@@ -229,19 +229,70 @@ static const TypeInfo cprman_pll_channel_info = {
 };
 
 
 /* clock mux */
 
+static bool clock_mux_is_enabled(CprmanClockMuxState *mux)
+{
+    return FIELD_EX32(*mux->reg_ctl, CM_CLOCKx_CTL, ENABLE);
+}
+
 static void clock_mux_update(CprmanClockMuxState *mux)
 {
-    clock_update(mux->out, 0);
+    uint64_t freq;
+    uint32_t div, src = FIELD_EX32(*mux->reg_ctl, CM_CLOCKx_CTL, SRC);
+    bool enabled = clock_mux_is_enabled(mux);
+
+    *mux->reg_ctl = FIELD_DP32(*mux->reg_ctl, CM_CLOCKx_CTL, BUSY, enabled);
+
+    if (!enabled) {
+        clock_update(mux->out, 0);
+        return;
+    }
+
+    freq = clock_get_hz(mux->srcs[src]);
+
+    if (mux->int_bits == 0 && mux->frac_bits == 0) {
+        clock_update_hz(mux->out, freq);
+        return;
+    }
+
+    /*
+     * The divider has an integer and a fractional part. The size of each part
+     * varies with the muxes (int_bits and frac_bits). Both parts are
+     * concatenated, with the integer part always starting at bit 12.
+     *
+     *         31          12 11          0
+     *        ------------------------------
+     * CM_DIV |      |  int  |  frac  |    |
+     *        ------------------------------
+     *                <-----> <------>
+     *                int_bits frac_bits
+     */
+    div = extract32(*mux->reg_div,
+                    R_CM_CLOCKx_DIV_FRAC_LENGTH - mux->frac_bits,
+                    mux->int_bits + mux->frac_bits);
+
+    if (!div) {
+        clock_update(mux->out, 0);
+        return;
+    }
+
+    freq = muldiv64(freq, 1 << mux->frac_bits, div);
+
+    clock_update_hz(mux->out, freq);
 }
 
 static void clock_mux_src_update(void *opaque)
 {
     CprmanClockMuxState **backref = opaque;
     CprmanClockMuxState *s = *backref;
+    CprmanClockMuxSource src = backref - s->backref;
+
+    if (FIELD_EX32(*s->reg_ctl, CM_CLOCKx_CTL, SRC) != src) {
+        return;
+    }
 
     clock_mux_update(s);
 }
 
 static void clock_mux_init(Object *obj)
-- 
2.28.0



^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v3 12/15] hw/misc/bcm2835_cprman: add the DSI0HSCK multiplexer
  2020-10-10 13:57 [PATCH v3 00/15] raspi: add the bcm2835 cprman clock manager Luc Michel
                   ` (10 preceding siblings ...)
  2020-10-10 13:57 ` [PATCH v3 11/15] hw/misc/bcm2835_cprman: implement clock mux behaviour Luc Michel
@ 2020-10-10 13:57 ` Luc Michel
  2020-10-10 13:57 ` [PATCH v3 13/15] hw/misc/bcm2835_cprman: add sane reset values to the registers Luc Michel
                   ` (5 subsequent siblings)
  17 siblings, 0 replies; 35+ messages in thread
From: Luc Michel @ 2020-10-10 13:57 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Luc Michel, Philippe Mathieu-Daudé,
	Andrew Baumann, Paul Zimmerman, Niek Linnenbank, qemu-arm,
	Havard Skinnemoen

This simple mux sits between the PLL channels and the DSI0E and DSI0P
clock muxes. This mux selects between PLLA-DSI0 and PLLD-DSI0 channel
and outputs the selected signal to source number 4 of DSI0E/P clock
muxes. It is controlled by the cm_dsi0hsck register.

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Luc Michel <luc@lmichel.fr>
---
 include/hw/misc/bcm2835_cprman.h           | 15 +++++
 include/hw/misc/bcm2835_cprman_internals.h |  6 ++
 hw/misc/bcm2835_cprman.c                   | 74 +++++++++++++++++++++-
 3 files changed, 94 insertions(+), 1 deletion(-)

diff --git a/include/hw/misc/bcm2835_cprman.h b/include/hw/misc/bcm2835_cprman.h
index 0fc8f68845..3df4ceedd2 100644
--- a/include/hw/misc/bcm2835_cprman.h
+++ b/include/hw/misc/bcm2835_cprman.h
@@ -172,20 +172,35 @@ typedef struct CprmanClockMuxState {
      * source number.
      */
     struct CprmanClockMuxState *backref[CPRMAN_NUM_CLOCK_MUX_SRC];
 } CprmanClockMuxState;
 
+typedef struct CprmanDsi0HsckMuxState {
+    /*< private >*/
+    DeviceState parent_obj;
+
+    /*< public >*/
+    CprmanClockMux id;
+
+    uint32_t *reg_cm;
+
+    Clock *plla_in;
+    Clock *plld_in;
+    Clock *out;
+} CprmanDsi0HsckMuxState;
+
 struct BCM2835CprmanState {
     /*< private >*/
     SysBusDevice parent_obj;
 
     /*< public >*/
     MemoryRegion iomem;
 
     CprmanPllState plls[CPRMAN_NUM_PLL];
     CprmanPllChannelState channels[CPRMAN_NUM_PLL_CHANNEL];
     CprmanClockMuxState clock_muxes[CPRMAN_NUM_CLOCK_MUX];
+    CprmanDsi0HsckMuxState dsi0hsck_mux;
 
     uint32_t regs[CPRMAN_NUM_REGS];
     uint32_t xosc_freq;
 
     Clock *xosc;
diff --git a/include/hw/misc/bcm2835_cprman_internals.h b/include/hw/misc/bcm2835_cprman_internals.h
index 0305448bbc..a6e799075f 100644
--- a/include/hw/misc/bcm2835_cprman_internals.h
+++ b/include/hw/misc/bcm2835_cprman_internals.h
@@ -13,17 +13,20 @@
 #include "hw/misc/bcm2835_cprman.h"
 
 #define TYPE_CPRMAN_PLL "bcm2835-cprman-pll"
 #define TYPE_CPRMAN_PLL_CHANNEL "bcm2835-cprman-pll-channel"
 #define TYPE_CPRMAN_CLOCK_MUX "bcm2835-cprman-clock-mux"
+#define TYPE_CPRMAN_DSI0HSCK_MUX "bcm2835-cprman-dsi0hsck-mux"
 
 DECLARE_INSTANCE_CHECKER(CprmanPllState, CPRMAN_PLL,
                          TYPE_CPRMAN_PLL)
 DECLARE_INSTANCE_CHECKER(CprmanPllChannelState, CPRMAN_PLL_CHANNEL,
                          TYPE_CPRMAN_PLL_CHANNEL)
 DECLARE_INSTANCE_CHECKER(CprmanClockMuxState, CPRMAN_CLOCK_MUX,
                          TYPE_CPRMAN_CLOCK_MUX)
+DECLARE_INSTANCE_CHECKER(CprmanDsi0HsckMuxState, CPRMAN_DSI0HSCK_MUX,
+                         TYPE_CPRMAN_DSI0HSCK_MUX)
 
 /* Register map */
 
 /* PLLs */
 REG32(CM_PLLA, 0x104)
@@ -221,10 +224,13 @@ REG32(CM_LOCK, 0x114)
     FIELD(CM_LOCK, FLOCKD, 11, 1)
     FIELD(CM_LOCK, FLOCKC, 10, 1)
     FIELD(CM_LOCK, FLOCKB, 9, 1)
     FIELD(CM_LOCK, FLOCKA, 8, 1)
 
+REG32(CM_DSI0HSCK, 0x120)
+    FIELD(CM_DSI0HSCK, SELPLLD, 0, 1)
+
 /*
  * This field is common to all registers. Each register write value must match
  * the CPRMAN_PASSWORD magic value in its 8 MSB.
  */
 FIELD(CPRMAN, PASSWORD, 24, 8)
diff --git a/hw/misc/bcm2835_cprman.c b/hw/misc/bcm2835_cprman.c
index 919a55aa23..7a7401963d 100644
--- a/hw/misc/bcm2835_cprman.c
+++ b/hw/misc/bcm2835_cprman.c
@@ -337,10 +337,62 @@ static const TypeInfo cprman_clock_mux_info = {
     .class_init = clock_mux_class_init,
     .instance_init = clock_mux_init,
 };
 
 
+/* DSI0HSCK mux */
+
+static void dsi0hsck_mux_update(CprmanDsi0HsckMuxState *s)
+{
+    bool src_is_plld = FIELD_EX32(*s->reg_cm, CM_DSI0HSCK, SELPLLD);
+    Clock *src = src_is_plld ? s->plld_in : s->plla_in;
+
+    clock_update(s->out, clock_get(src));
+}
+
+static void dsi0hsck_mux_in_update(void *opaque)
+{
+    dsi0hsck_mux_update(CPRMAN_DSI0HSCK_MUX(opaque));
+}
+
+static void dsi0hsck_mux_init(Object *obj)
+{
+    CprmanDsi0HsckMuxState *s = CPRMAN_DSI0HSCK_MUX(obj);
+    DeviceState *dev = DEVICE(obj);
+
+    s->plla_in = qdev_init_clock_in(dev, "plla-in", dsi0hsck_mux_in_update, s);
+    s->plld_in = qdev_init_clock_in(dev, "plld-in", dsi0hsck_mux_in_update, s);
+    s->out = qdev_init_clock_out(DEVICE(s), "out");
+}
+
+static const VMStateDescription dsi0hsck_mux_vmstate = {
+    .name = TYPE_CPRMAN_DSI0HSCK_MUX,
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_CLOCK(plla_in, CprmanDsi0HsckMuxState),
+        VMSTATE_CLOCK(plld_in, CprmanDsi0HsckMuxState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static void dsi0hsck_mux_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->vmsd = &dsi0hsck_mux_vmstate;
+}
+
+static const TypeInfo cprman_dsi0hsck_mux_info = {
+    .name = TYPE_CPRMAN_DSI0HSCK_MUX,
+    .parent = TYPE_DEVICE,
+    .instance_size = sizeof(CprmanDsi0HsckMuxState),
+    .class_init = dsi0hsck_mux_class_init,
+    .instance_init = dsi0hsck_mux_init,
+};
+
+
 /* CPRMAN "top level" model */
 
 static uint32_t get_cm_lock(const BCM2835CprmanState *s)
 {
     static const int CM_LOCK_MAPPING[CPRMAN_NUM_PLL] = {
@@ -499,10 +551,14 @@ static void cprman_write(void *opaque, hwaddr offset,
     case R_CM_SDCCTL ... R_CM_ARMCTL:
     case R_CM_AVEOCTL ... R_CM_EMMCDIV:
     case R_CM_EMMC2CTL ... R_CM_EMMC2DIV:
         update_mux_from_cm(s, idx);
         break;
+
+    case R_CM_DSI0HSCK:
+        dsi0hsck_mux_update(&s->dsi0hsck_mux);
+        break;
     }
 }
 
 #undef CASE_PLL_A2W_REGS
 
@@ -538,10 +594,12 @@ static void cprman_reset(DeviceState *dev)
 
     for (i = 0; i < CPRMAN_NUM_PLL_CHANNEL; i++) {
         device_cold_reset(DEVICE(&s->channels[i]));
     }
 
+    device_cold_reset(DEVICE(&s->dsi0hsck_mux));
+
     for (i = 0; i < CPRMAN_NUM_CLOCK_MUX; i++) {
         device_cold_reset(DEVICE(&s->clock_muxes[i]));
     }
 
     clock_update_hz(s->xosc, s->xosc_freq);
@@ -563,10 +621,14 @@ static void cprman_init(Object *obj)
                                 &s->channels[i],
                                 TYPE_CPRMAN_PLL_CHANNEL);
         set_pll_channel_init_info(s, &s->channels[i], i);
     }
 
+    object_initialize_child(obj, "dsi0hsck-mux",
+                            &s->dsi0hsck_mux, TYPE_CPRMAN_DSI0HSCK_MUX);
+    s->dsi0hsck_mux.reg_cm = &s->regs[R_CM_DSI0HSCK];
+
     for (i = 0; i < CPRMAN_NUM_CLOCK_MUX; i++) {
         char *alias;
 
         object_initialize_child(obj, CLOCK_MUX_INIT_INFO[i].name,
                                 &s->clock_muxes[i],
@@ -610,11 +672,11 @@ static void connect_mux_sources(BCM2835CprmanState *s,
         Clock *src;
 
         if (mapping == CPRMAN_CLOCK_SRC_FORCE_GROUND) {
             src = s->gnd;
         } else if (mapping == CPRMAN_CLOCK_SRC_DSI0HSCK) {
-            src = s->gnd; /* TODO */
+            src = s->dsi0hsck_mux.out;
         } else if (i < CPRMAN_CLOCK_SRC_PLLA) {
             src = CLK_SRC_MAPPING[i];
         } else {
             src = s->channels[mapping].out;
         }
@@ -648,10 +710,19 @@ static void cprman_realize(DeviceState *dev, Error **errp)
         if (!qdev_realize(DEVICE(channel), NULL, errp)) {
             return;
         }
     }
 
+    clock_set_source(s->dsi0hsck_mux.plla_in,
+                     s->channels[CPRMAN_PLLA_CHANNEL_DSI0].out);
+    clock_set_source(s->dsi0hsck_mux.plld_in,
+                     s->channels[CPRMAN_PLLD_CHANNEL_DSI0].out);
+
+    if (!qdev_realize(DEVICE(&s->dsi0hsck_mux), NULL, errp)) {
+        return;
+    }
+
     for (i = 0; i < CPRMAN_NUM_CLOCK_MUX; i++) {
         CprmanClockMuxState *clock_mux = &s->clock_muxes[i];
 
         connect_mux_sources(s, clock_mux, CLOCK_MUX_INIT_INFO[i].src_mapping);
 
@@ -698,8 +769,9 @@ static void cprman_register_types(void)
 {
     type_register_static(&cprman_info);
     type_register_static(&cprman_pll_info);
     type_register_static(&cprman_pll_channel_info);
     type_register_static(&cprman_clock_mux_info);
+    type_register_static(&cprman_dsi0hsck_mux_info);
 }
 
 type_init(cprman_register_types);
-- 
2.28.0



^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v3 13/15] hw/misc/bcm2835_cprman: add sane reset values to the registers
  2020-10-10 13:57 [PATCH v3 00/15] raspi: add the bcm2835 cprman clock manager Luc Michel
                   ` (11 preceding siblings ...)
  2020-10-10 13:57 ` [PATCH v3 12/15] hw/misc/bcm2835_cprman: add the DSI0HSCK multiplexer Luc Michel
@ 2020-10-10 13:57 ` Luc Michel
  2020-10-10 16:18   ` Philippe Mathieu-Daudé
  2020-10-10 13:57 ` [PATCH v3 14/15] hw/char/pl011: add a clock input Luc Michel
                   ` (4 subsequent siblings)
  17 siblings, 1 reply; 35+ messages in thread
From: Luc Michel @ 2020-10-10 13:57 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Luc Michel, Philippe Mathieu-Daudé,
	Andrew Baumann, Paul Zimmerman, Niek Linnenbank, qemu-arm,
	Havard Skinnemoen

Those reset values have been extracted from a Raspberry Pi 3 model B
v1.2, using the 2020-08-20 version of raspios. The dump was done using
the debugfs interface of the CPRMAN driver in Linux (under
'/sys/kernel/debug/clk'). Each exposed clock tree stage (PLLs, channels
and muxes) can be observed by reading the 'regdump' file (e.g.
'plla/regdump').

Those values are set by the Raspberry Pi firmware at boot time (Linux
expects them to be set when it boots up).

Some stages are not exposed by the Linux driver (e.g. the PLL B). For
those, the reset values are unknown and left to 0 which implies a
disabled output.

Once booted in QEMU, the final clock tree is very similar to the one
visible on real hardware. The differences come from some unimplemented
devices for which the driver simply disable the corresponding clock.

Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Luc Michel <luc@lmichel.fr>
---
 include/hw/misc/bcm2835_cprman_internals.h | 269 +++++++++++++++++++++
 hw/misc/bcm2835_cprman.c                   |  31 +++
 2 files changed, 300 insertions(+)

diff --git a/include/hw/misc/bcm2835_cprman_internals.h b/include/hw/misc/bcm2835_cprman_internals.h
index a6e799075f..339759b307 100644
--- a/include/hw/misc/bcm2835_cprman_internals.h
+++ b/include/hw/misc/bcm2835_cprman_internals.h
@@ -745,6 +745,275 @@ static inline void set_clock_mux_init_info(BCM2835CprmanState *s,
     mux->reg_div = &s->regs[CLOCK_MUX_INIT_INFO[id].cm_offset + 1];
     mux->int_bits = CLOCK_MUX_INIT_INFO[id].int_bits;
     mux->frac_bits = CLOCK_MUX_INIT_INFO[id].frac_bits;
 }
 
+
+/*
+ * Object reset info
+ * Those values have been dumped from a Raspberry Pi 3 Model B v1.2 using the
+ * clk debugfs interface in Linux.
+ */
+typedef struct PLLResetInfo {
+    uint32_t cm;
+    uint32_t a2w_ctrl;
+    uint32_t a2w_ana[4];
+    uint32_t a2w_frac;
+} PLLResetInfo;
+
+static const PLLResetInfo PLL_RESET_INFO[] = {
+    [CPRMAN_PLLA] = {
+        .cm = 0x0000008a,
+        .a2w_ctrl = 0x0002103a,
+        .a2w_frac = 0x00098000,
+        .a2w_ana = { 0x00000000, 0x00144000, 0x00000000, 0x00000100 }
+    },
+
+    [CPRMAN_PLLC] = {
+        .cm = 0x00000228,
+        .a2w_ctrl = 0x0002103e,
+        .a2w_frac = 0x00080000,
+        .a2w_ana = { 0x00000000, 0x00144000, 0x00000000, 0x00000100 }
+    },
+
+    [CPRMAN_PLLD] = {
+        .cm = 0x0000020a,
+        .a2w_ctrl = 0x00021034,
+        .a2w_frac = 0x00015556,
+        .a2w_ana = { 0x00000000, 0x00144000, 0x00000000, 0x00000100 }
+    },
+
+    [CPRMAN_PLLH] = {
+        .cm = 0x00000000,
+        .a2w_ctrl = 0x0002102d,
+        .a2w_frac = 0x00000000,
+        .a2w_ana = { 0x00900000, 0x0000000c, 0x00000000, 0x00000000 }
+    },
+
+    [CPRMAN_PLLB] = {
+        /* unknown */
+        .cm = 0x00000000,
+        .a2w_ctrl = 0x00000000,
+        .a2w_frac = 0x00000000,
+        .a2w_ana = { 0x00000000, 0x00000000, 0x00000000, 0x00000000 }
+    }
+};
+
+typedef struct PLLChannelResetInfo {
+    /*
+     * Even though a PLL channel has a CM register, it shares it with its
+     * parent PLL. The parent already takes care of the reset value.
+     */
+    uint32_t a2w_ctrl;
+} PLLChannelResetInfo;
+
+static const PLLChannelResetInfo PLL_CHANNEL_RESET_INFO[] = {
+    [CPRMAN_PLLA_CHANNEL_DSI0] = { .a2w_ctrl = 0x00000100 },
+    [CPRMAN_PLLA_CHANNEL_CORE] = { .a2w_ctrl = 0x00000003 },
+    [CPRMAN_PLLA_CHANNEL_PER] = { .a2w_ctrl = 0x00000000 }, /* unknown */
+    [CPRMAN_PLLA_CHANNEL_CCP2] = { .a2w_ctrl = 0x00000100 },
+
+    [CPRMAN_PLLC_CHANNEL_CORE2] = { .a2w_ctrl = 0x00000100 },
+    [CPRMAN_PLLC_CHANNEL_CORE1] = { .a2w_ctrl = 0x00000100 },
+    [CPRMAN_PLLC_CHANNEL_PER] = { .a2w_ctrl = 0x00000002 },
+    [CPRMAN_PLLC_CHANNEL_CORE0] = { .a2w_ctrl = 0x00000002 },
+
+    [CPRMAN_PLLD_CHANNEL_DSI0] = { .a2w_ctrl = 0x00000100 },
+    [CPRMAN_PLLD_CHANNEL_CORE] = { .a2w_ctrl = 0x00000004 },
+    [CPRMAN_PLLD_CHANNEL_PER] = { .a2w_ctrl = 0x00000004 },
+    [CPRMAN_PLLD_CHANNEL_DSI1] = { .a2w_ctrl = 0x00000100 },
+
+    [CPRMAN_PLLH_CHANNEL_AUX] = { .a2w_ctrl = 0x00000004 },
+    [CPRMAN_PLLH_CHANNEL_RCAL] = { .a2w_ctrl = 0x00000000 },
+    [CPRMAN_PLLH_CHANNEL_PIX] = { .a2w_ctrl = 0x00000000 },
+
+    [CPRMAN_PLLB_CHANNEL_ARM] = { .a2w_ctrl = 0x00000000 }, /* unknown */
+};
+
+typedef struct ClockMuxResetInfo {
+    uint32_t cm_ctl;
+    uint32_t cm_div;
+} ClockMuxResetInfo;
+
+static const ClockMuxResetInfo CLOCK_MUX_RESET_INFO[] = {
+    [CPRMAN_CLOCK_GNRIC] = {
+        .cm_ctl = 0, /* unknown */
+        .cm_div = 0
+    },
+
+    [CPRMAN_CLOCK_VPU] = {
+        .cm_ctl = 0x00000245,
+        .cm_div = 0x00003000,
+    },
+
+    [CPRMAN_CLOCK_SYS] = {
+        .cm_ctl = 0, /* unknown */
+        .cm_div = 0
+    },
+
+    [CPRMAN_CLOCK_PERIA] = {
+        .cm_ctl = 0, /* unknown */
+        .cm_div = 0
+    },
+
+    [CPRMAN_CLOCK_PERII] = {
+        .cm_ctl = 0, /* unknown */
+        .cm_div = 0
+    },
+
+    [CPRMAN_CLOCK_H264] = {
+        .cm_ctl = 0x00000244,
+        .cm_div = 0x00003000,
+    },
+
+    [CPRMAN_CLOCK_ISP] = {
+        .cm_ctl = 0x00000244,
+        .cm_div = 0x00003000,
+    },
+
+    [CPRMAN_CLOCK_V3D] = {
+        .cm_ctl = 0, /* unknown */
+        .cm_div = 0
+    },
+
+    [CPRMAN_CLOCK_CAM0] = {
+        .cm_ctl = 0x00000000,
+        .cm_div = 0x00000000,
+    },
+
+    [CPRMAN_CLOCK_CAM1] = {
+        .cm_ctl = 0x00000000,
+        .cm_div = 0x00000000,
+    },
+
+    [CPRMAN_CLOCK_CCP2] = {
+        .cm_ctl = 0, /* unknown */
+        .cm_div = 0
+    },
+
+    [CPRMAN_CLOCK_DSI0E] = {
+        .cm_ctl = 0x00000000,
+        .cm_div = 0x00000000,
+    },
+
+    [CPRMAN_CLOCK_DSI0P] = {
+        .cm_ctl = 0x00000000,
+        .cm_div = 0x00000000,
+    },
+
+    [CPRMAN_CLOCK_DPI] = {
+        .cm_ctl = 0x00000000,
+        .cm_div = 0x00000000,
+    },
+
+    [CPRMAN_CLOCK_GP0] = {
+        .cm_ctl = 0x00000200,
+        .cm_div = 0x00000000,
+    },
+
+    [CPRMAN_CLOCK_GP1] = {
+        .cm_ctl = 0x00000096,
+        .cm_div = 0x00014000,
+    },
+
+    [CPRMAN_CLOCK_GP2] = {
+        .cm_ctl = 0x00000291,
+        .cm_div = 0x00249f00,
+    },
+
+    [CPRMAN_CLOCK_HSM] = {
+        .cm_ctl = 0x00000000,
+        .cm_div = 0x00000000,
+    },
+
+    [CPRMAN_CLOCK_OTP] = {
+        .cm_ctl = 0x00000091,
+        .cm_div = 0x00004000,
+    },
+
+    [CPRMAN_CLOCK_PCM] = {
+        .cm_ctl = 0x00000200,
+        .cm_div = 0x00000000,
+    },
+
+    [CPRMAN_CLOCK_PWM] = {
+        .cm_ctl = 0x00000200,
+        .cm_div = 0x00000000,
+    },
+
+    [CPRMAN_CLOCK_SLIM] = {
+        .cm_ctl = 0x00000200,
+        .cm_div = 0x00000000,
+    },
+
+    [CPRMAN_CLOCK_SMI] = {
+        .cm_ctl = 0x00000000,
+        .cm_div = 0x00000000,
+    },
+
+    [CPRMAN_CLOCK_TEC] = {
+        .cm_ctl = 0x00000000,
+        .cm_div = 0x00000000,
+    },
+
+    [CPRMAN_CLOCK_TD0] = {
+        .cm_ctl = 0, /* unknown */
+        .cm_div = 0
+    },
+
+    [CPRMAN_CLOCK_TD1] = {
+        .cm_ctl = 0, /* unknown */
+        .cm_div = 0
+    },
+
+    [CPRMAN_CLOCK_TSENS] = {
+        .cm_ctl = 0x00000091,
+        .cm_div = 0x0000a000,
+    },
+
+    [CPRMAN_CLOCK_TIMER] = {
+        .cm_ctl = 0x00000291,
+        .cm_div = 0x00013333,
+    },
+
+    [CPRMAN_CLOCK_UART] = {
+        .cm_ctl = 0x00000296,
+        .cm_div = 0x0000a6ab,
+    },
+
+    [CPRMAN_CLOCK_VEC] = {
+        .cm_ctl = 0x00000097,
+        .cm_div = 0x00002000,
+    },
+
+    [CPRMAN_CLOCK_PULSE] = {
+        .cm_ctl = 0, /* unknown */
+        .cm_div = 0
+    },
+
+    [CPRMAN_CLOCK_SDC] = {
+        .cm_ctl = 0x00004006,
+        .cm_div = 0x00003000,
+    },
+
+    [CPRMAN_CLOCK_ARM] = {
+        .cm_ctl = 0, /* unknown */
+        .cm_div = 0
+    },
+
+    [CPRMAN_CLOCK_AVEO] = {
+        .cm_ctl = 0x00000000,
+        .cm_div = 0x00000000,
+    },
+
+    [CPRMAN_CLOCK_EMMC] = {
+        .cm_ctl = 0x00000295,
+        .cm_div = 0x00006000,
+    },
+
+    [CPRMAN_CLOCK_EMMC2] = {
+        .cm_ctl = 0, /* unknown */
+        .cm_div = 0
+    },
+};
+
 #endif
diff --git a/hw/misc/bcm2835_cprman.c b/hw/misc/bcm2835_cprman.c
index 7a7401963d..7e415a017c 100644
--- a/hw/misc/bcm2835_cprman.c
+++ b/hw/misc/bcm2835_cprman.c
@@ -51,10 +51,21 @@
 #include "hw/misc/bcm2835_cprman_internals.h"
 #include "trace.h"
 
 /* PLL */
 
+static void pll_reset(DeviceState *dev)
+{
+    CprmanPllState *s = CPRMAN_PLL(dev);
+    const PLLResetInfo *info = &PLL_RESET_INFO[s->id];
+
+    *s->reg_cm = info->cm;
+    *s->reg_a2w_ctrl = info->a2w_ctrl;
+    memcpy(s->reg_a2w_ana, info->a2w_ana, sizeof(info->a2w_ana));
+    *s->reg_a2w_frac = info->a2w_frac;
+}
+
 static bool pll_is_locked(const CprmanPllState *pll)
 {
     return !FIELD_EX32(*pll->reg_a2w_ctrl, A2W_PLLx_CTRL, PWRDN)
         && !FIELD_EX32(*pll->reg_cm, CM_PLLx, ANARST);
 }
@@ -121,10 +132,11 @@ static const VMStateDescription pll_vmstate = {
 
 static void pll_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
 
+    dc->reset = pll_reset;
     dc->vmsd = &pll_vmstate;
 }
 
 static const TypeInfo cprman_pll_info = {
     .name = TYPE_CPRMAN_PLL,
@@ -135,10 +147,18 @@ static const TypeInfo cprman_pll_info = {
 };
 
 
 /* PLL channel */
 
+static void pll_channel_reset(DeviceState *dev)
+{
+    CprmanPllChannelState *s = CPRMAN_PLL_CHANNEL(dev);
+    const PLLChannelResetInfo *info = &PLL_CHANNEL_RESET_INFO[s->id];
+
+    *s->reg_a2w_ctrl = info->a2w_ctrl;
+}
+
 static bool pll_channel_is_enabled(CprmanPllChannelState *channel)
 {
     /*
      * XXX I'm not sure of the purpose of the LOAD field. The Linux driver does
      * not set it when enabling the channel, but does clear it when disabling
@@ -215,10 +235,11 @@ static const VMStateDescription pll_channel_vmstate = {
 
 static void pll_channel_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
 
+    dc->reset = pll_channel_reset;
     dc->vmsd = &pll_channel_vmstate;
 }
 
 static const TypeInfo cprman_pll_channel_info = {
     .name = TYPE_CPRMAN_PLL_CHANNEL,
@@ -293,10 +314,19 @@ static void clock_mux_src_update(void *opaque)
     }
 
     clock_mux_update(s);
 }
 
+static void clock_mux_reset(DeviceState *dev)
+{
+    CprmanClockMuxState *clock = CPRMAN_CLOCK_MUX(dev);
+    const ClockMuxResetInfo *info = &CLOCK_MUX_RESET_INFO[clock->id];
+
+    *clock->reg_ctl = info->cm_ctl;
+    *clock->reg_div = info->cm_div;
+}
+
 static void clock_mux_init(Object *obj)
 {
     CprmanClockMuxState *s = CPRMAN_CLOCK_MUX(obj);
     size_t i;
 
@@ -325,10 +355,11 @@ static const VMStateDescription clock_mux_vmstate = {
 
 static void clock_mux_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
 
+    dc->reset = clock_mux_reset;
     dc->vmsd = &clock_mux_vmstate;
 }
 
 static const TypeInfo cprman_clock_mux_info = {
     .name = TYPE_CPRMAN_CLOCK_MUX,
-- 
2.28.0



^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v3 14/15] hw/char/pl011: add a clock input
  2020-10-10 13:57 [PATCH v3 00/15] raspi: add the bcm2835 cprman clock manager Luc Michel
                   ` (12 preceding siblings ...)
  2020-10-10 13:57 ` [PATCH v3 13/15] hw/misc/bcm2835_cprman: add sane reset values to the registers Luc Michel
@ 2020-10-10 13:57 ` Luc Michel
  2020-10-10 13:57 ` [PATCH v3 15/15] hw/arm/bcm2835_peripherals: connect the UART clock Luc Michel
                   ` (3 subsequent siblings)
  17 siblings, 0 replies; 35+ messages in thread
From: Luc Michel @ 2020-10-10 13:57 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Luc Michel, Philippe Mathieu-Daudé,
	Andrew Baumann, Paul Zimmerman, Niek Linnenbank, qemu-arm,
	Havard Skinnemoen

Add a clock input to the PL011 UART so we can compute the current baud
rate and trace it. This is intended for developers who wish to use QEMU
to e.g. debug their firmware or to figure out the baud rate configured
by an unknown/closed source binary.

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Luc Michel <luc@lmichel.fr>
---
 include/hw/char/pl011.h |  1 +
 hw/char/pl011.c         | 45 +++++++++++++++++++++++++++++++++++++++++
 hw/char/trace-events    |  1 +
 3 files changed, 47 insertions(+)

diff --git a/include/hw/char/pl011.h b/include/hw/char/pl011.h
index a91ea50e11..33e5e5317b 100644
--- a/include/hw/char/pl011.h
+++ b/include/hw/char/pl011.h
@@ -47,10 +47,11 @@ struct PL011State {
     int read_pos;
     int read_count;
     int read_trigger;
     CharBackend chr;
     qemu_irq irq[6];
+    Clock *clk;
     const unsigned char *id;
 };
 
 static inline DeviceState *pl011_create(hwaddr addr,
                                         qemu_irq irq,
diff --git a/hw/char/pl011.c b/hw/char/pl011.c
index 13e784f9d9..ede16c781c 100644
--- a/hw/char/pl011.c
+++ b/hw/char/pl011.c
@@ -20,10 +20,11 @@
 
 #include "qemu/osdep.h"
 #include "hw/char/pl011.h"
 #include "hw/irq.h"
 #include "hw/sysbus.h"
+#include "hw/qdev-clock.h"
 #include "migration/vmstate.h"
 #include "chardev/char-fe.h"
 #include "qemu/log.h"
 #include "qemu/module.h"
 #include "trace.h"
@@ -167,10 +168,29 @@ static void pl011_set_read_trigger(PL011State *s)
     else
 #endif
         s->read_trigger = 1;
 }
 
+static unsigned int pl011_get_baudrate(const PL011State *s)
+{
+    uint64_t clk;
+
+    if (s->fbrd == 0) {
+        return 0;
+    }
+
+    clk = clock_get_hz(s->clk);
+    return (clk / ((s->ibrd << 6) + s->fbrd)) << 2;
+}
+
+static void pl011_trace_baudrate_change(const PL011State *s)
+{
+    trace_pl011_baudrate_change(pl011_get_baudrate(s),
+                                clock_get_hz(s->clk),
+                                s->ibrd, s->fbrd);
+}
+
 static void pl011_write(void *opaque, hwaddr offset,
                         uint64_t value, unsigned size)
 {
     PL011State *s = (PL011State *)opaque;
     unsigned char ch;
@@ -196,13 +216,15 @@ static void pl011_write(void *opaque, hwaddr offset,
     case 8: /* UARTUARTILPR */
         s->ilpr = value;
         break;
     case 9: /* UARTIBRD */
         s->ibrd = value;
+        pl011_trace_baudrate_change(s);
         break;
     case 10: /* UARTFBRD */
         s->fbrd = value;
+        pl011_trace_baudrate_change(s);
         break;
     case 11: /* UARTLCR_H */
         /* Reset the FIFO state on FIFO enable or disable */
         if ((s->lcr ^ value) & 0x10) {
             s->read_count = 0;
@@ -284,16 +306,33 @@ static void pl011_event(void *opaque, QEMUChrEvent event)
 {
     if (event == CHR_EVENT_BREAK)
         pl011_put_fifo(opaque, 0x400);
 }
 
+static void pl011_clock_update(void *opaque)
+{
+    PL011State *s = PL011(opaque);
+
+    pl011_trace_baudrate_change(s);
+}
+
 static const MemoryRegionOps pl011_ops = {
     .read = pl011_read,
     .write = pl011_write,
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
+static const VMStateDescription vmstate_pl011_clock = {
+    .name = "pl011/clock",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_CLOCK(clk, PL011State),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static const VMStateDescription vmstate_pl011 = {
     .name = "pl011",
     .version_id = 2,
     .minimum_version_id = 2,
     .fields = (VMStateField[]) {
@@ -312,10 +351,14 @@ static const VMStateDescription vmstate_pl011 = {
         VMSTATE_UINT32(ifl, PL011State),
         VMSTATE_INT32(read_pos, PL011State),
         VMSTATE_INT32(read_count, PL011State),
         VMSTATE_INT32(read_trigger, PL011State),
         VMSTATE_END_OF_LIST()
+    },
+    .subsections = (const VMStateDescription * []) {
+        &vmstate_pl011_clock,
+        NULL
     }
 };
 
 static Property pl011_properties[] = {
     DEFINE_PROP_CHR("chardev", PL011State, chr),
@@ -332,10 +375,12 @@ static void pl011_init(Object *obj)
     sysbus_init_mmio(sbd, &s->iomem);
     for (i = 0; i < ARRAY_SIZE(s->irq); i++) {
         sysbus_init_irq(sbd, &s->irq[i]);
     }
 
+    s->clk = qdev_init_clock_in(DEVICE(obj), "clk", pl011_clock_update, s);
+
     s->read_trigger = 1;
     s->ifl = 0x12;
     s->cr = 0x300;
     s->flags = 0x90;
 
diff --git a/hw/char/trace-events b/hw/char/trace-events
index 609df10fed..81026f6612 100644
--- a/hw/char/trace-events
+++ b/hw/char/trace-events
@@ -63,10 +63,11 @@ pl011_read(uint32_t addr, uint32_t value) "addr 0x%08x value 0x%08x"
 pl011_read_fifo(int read_count) "FIFO read, read_count now %d"
 pl011_write(uint32_t addr, uint32_t value) "addr 0x%08x value 0x%08x"
 pl011_can_receive(uint32_t lcr, int read_count, int r) "LCR 0x%08x read_count %d returning %d"
 pl011_put_fifo(uint32_t c, int read_count) "new char 0x%x read_count now %d"
 pl011_put_fifo_full(void) "FIFO now full, RXFF set"
+pl011_baudrate_change(unsigned int baudrate, uint64_t clock, uint32_t ibrd, uint32_t fbrd) "new baudrate %u (clk: %" PRIu64 "hz, ibrd: %" PRIu32 ", fbrd: %" PRIu32 ")"
 
 # cmsdk-apb-uart.c
 cmsdk_apb_uart_read(uint64_t offset, uint64_t data, unsigned size) "CMSDK APB UART read: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u"
 cmsdk_apb_uart_write(uint64_t offset, uint64_t data, unsigned size) "CMSDK APB UART write: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u"
 cmsdk_apb_uart_reset(void) "CMSDK APB UART: reset"
-- 
2.28.0



^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v3 15/15] hw/arm/bcm2835_peripherals: connect the UART clock
  2020-10-10 13:57 [PATCH v3 00/15] raspi: add the bcm2835 cprman clock manager Luc Michel
                   ` (13 preceding siblings ...)
  2020-10-10 13:57 ` [PATCH v3 14/15] hw/char/pl011: add a clock input Luc Michel
@ 2020-10-10 13:57 ` Luc Michel
  2020-10-16 17:11 ` [PATCH v3 00/15] raspi: add the bcm2835 cprman clock manager Philippe Mathieu-Daudé
                   ` (2 subsequent siblings)
  17 siblings, 0 replies; 35+ messages in thread
From: Luc Michel @ 2020-10-10 13:57 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Luc Michel, Philippe Mathieu-Daudé,
	Andrew Baumann, Paul Zimmerman, Niek Linnenbank, qemu-arm,
	Havard Skinnemoen

Connect the 'uart-out' clock from the CPRMAN to the PL011 instance.

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Luc Michel <luc@lmichel.fr>
---
 hw/arm/bcm2835_peripherals.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c
index 9d6190042d..d8f28b1ae2 100644
--- a/hw/arm/bcm2835_peripherals.c
+++ b/hw/arm/bcm2835_peripherals.c
@@ -167,10 +167,12 @@ static void bcm2835_peripherals_realize(DeviceState *dev, Error **errp)
     if (!sysbus_realize(SYS_BUS_DEVICE(&s->cprman), errp)) {
         return;
     }
     memory_region_add_subregion(&s->peri_mr, CPRMAN_OFFSET,
                 sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->cprman), 0));
+    qdev_connect_clock_in(DEVICE(&s->uart0), "clk",
+                          qdev_get_clock_out(DEVICE(&s->cprman), "uart-out"));
 
     memory_region_add_subregion(&s->peri_mr, ARMCTRL_IC_OFFSET,
                 sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->ic), 0));
     sysbus_pass_irq(SYS_BUS_DEVICE(s), SYS_BUS_DEVICE(&s->ic));
 
-- 
2.28.0



^ permalink raw reply related	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 03/15] hw/core/clock: add the clock_new helper function
  2020-10-10 13:57 ` [PATCH v3 03/15] hw/core/clock: add the clock_new helper function Luc Michel
@ 2020-10-10 15:17   ` Philippe Mathieu-Daudé
  2020-10-10 15:44     ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 35+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-10-10 15:17 UTC (permalink / raw)
  To: Luc Michel, qemu-devel
  Cc: Damien Hedde, Peter Maydell, Havard Skinnemoen, Andrew Baumann,
	Paul Zimmerman, Niek Linnenbank, qemu-arm

On 10/10/20 3:57 PM, Luc Michel wrote:
> This function creates a clock and parents it to another object with a given
> name. It calls clock_setup_canonical_path before returning the new
> clock.
> 
> This function is useful to create clocks in devices when one doesn't
> want to expose it at the qdev level (as an input or an output).
> 
> Suggested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Signed-off-by: Luc Michel <luc@lmichel.fr>
> ---
>   include/hw/clock.h | 13 +++++++++++++
>   hw/core/clock.c    | 15 +++++++++++++++
>   2 files changed, 28 insertions(+)

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>



^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 03/15] hw/core/clock: add the clock_new helper function
  2020-10-10 15:17   ` Philippe Mathieu-Daudé
@ 2020-10-10 15:44     ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 35+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-10-10 15:44 UTC (permalink / raw)
  To: Luc Michel, qemu-devel
  Cc: Damien Hedde, Peter Maydell, Havard Skinnemoen, Andrew Baumann,
	Paul Zimmerman, Niek Linnenbank, qemu-arm

On 10/10/20 5:17 PM, Philippe Mathieu-Daudé wrote:
> On 10/10/20 3:57 PM, Luc Michel wrote:
>> This function creates a clock and parents it to another object with a 
>> given
>> name. It calls clock_setup_canonical_path before returning the new
>> clock.
>>
>> This function is useful to create clocks in devices when one doesn't
>> want to expose it at the qdev level (as an input or an output).
>>
>> Suggested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> Signed-off-by: Luc Michel <luc@lmichel.fr>
>> ---
>>   include/hw/clock.h | 13 +++++++++++++
>>   hw/core/clock.c    | 15 +++++++++++++++
>>   2 files changed, 28 insertions(+)
> 
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>



^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 02/15] hw/core/clock: trace clock values in Hz instead of ns
  2020-10-10 13:57 ` [PATCH v3 02/15] hw/core/clock: trace clock values in Hz instead of ns Luc Michel
@ 2020-10-10 15:48   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 35+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-10-10 15:48 UTC (permalink / raw)
  To: Luc Michel, qemu-devel
  Cc: Damien Hedde, Peter Maydell, Havard Skinnemoen, Andrew Baumann,
	Paul Zimmerman, Niek Linnenbank, qemu-arm

On 10/10/20 3:57 PM, Luc Michel wrote:
> The nanosecond unit greatly limits the dynamic range we can display in
> clock value traces, for values in the order of 1GHz and more. The
> internal representation can go way beyond this value and it is quite
> common for today's clocks to be within those ranges.
> 
> For example, a frequency between 500MHz+ and 1GHz will be displayed as
> 1ns. Beyond 1GHz, it will show up as 0ns.
> 
> Replace nanosecond periods traces with frequencies in the Hz unit
> to have more dynamic range in the trace output.
> 
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Reviewed-by: Damien Hedde <damien.hedde@greensocs.com>
> Signed-off-by: Luc Michel <luc@lmichel.fr>
> ---
>   hw/core/clock.c      | 6 +++---
>   hw/core/trace-events | 4 ++--
>   2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/core/clock.c b/hw/core/clock.c
> index 7066282f7b..81184734e0 100644
> --- a/hw/core/clock.c
> +++ b/hw/core/clock.c
> @@ -37,12 +37,12 @@ void clock_clear_callback(Clock *clk)
>   bool clock_set(Clock *clk, uint64_t period)
>   {
>       if (clk->period == period) {
>           return false;
>       }
> -    trace_clock_set(CLOCK_PATH(clk), CLOCK_PERIOD_TO_NS(clk->period),
> -                    CLOCK_PERIOD_TO_NS(period));
> +    trace_clock_set(CLOCK_PATH(clk), CLOCK_PERIOD_TO_HZ(clk->period),
> +                    CLOCK_PERIOD_TO_HZ(period));
>       clk->period = period;
>   
>       return true;
>   }
>   
> @@ -52,11 +52,11 @@ static void clock_propagate_period(Clock *clk, bool call_callbacks)
>   
>       QLIST_FOREACH(child, &clk->children, sibling) {
>           if (child->period != clk->period) {
>               child->period = clk->period;
>               trace_clock_update(CLOCK_PATH(child), CLOCK_PATH(clk),
> -                               CLOCK_PERIOD_TO_NS(clk->period),
> +                               CLOCK_PERIOD_TO_HZ(clk->period),
>                                  call_callbacks);
>               if (call_callbacks && child->callback) {
>                   child->callback(child->callback_opaque);
>               }
>               clock_propagate_period(child, call_callbacks);
> diff --git a/hw/core/trace-events b/hw/core/trace-events
> index 1ac60ede6b..360ddeb2c8 100644
> --- a/hw/core/trace-events
> +++ b/hw/core/trace-events
> @@ -29,8 +29,8 @@ resettable_phase_exit_end(void *obj, const char *objtype, unsigned count) "obj=%
>   resettable_transitional_function(void *obj, const char *objtype) "obj=%p(%s)"
>   
>   # clock.c
>   clock_set_source(const char *clk, const char *src) "'%s', src='%s'"
>   clock_disconnect(const char *clk) "'%s'"
> -clock_set(const char *clk, uint64_t old, uint64_t new) "'%s', ns=%"PRIu64"->%"PRIu64
> +clock_set(const char *clk, uint64_t old, uint64_t new) "'%s', %"PRIu64"Hz->%"PRIu64"Hz"

I find it easier to read/follow as: "'%s', %"PRIu64" -> %"PRIu64" Hz",
you see the number directly, no need for the brain to extract the
trailing "Hz".

Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

>   clock_propagate(const char *clk) "'%s'"
> -clock_update(const char *clk, const char *src, uint64_t val, int cb) "'%s', src='%s', ns=%"PRIu64", cb=%d"
> +clock_update(const char *clk, const char *src, uint64_t hz, int cb) "'%s', src='%s', val=%"PRIu64"Hz cb=%d"
> 


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 11/15] hw/misc/bcm2835_cprman: implement clock mux behaviour
  2020-10-10 13:57 ` [PATCH v3 11/15] hw/misc/bcm2835_cprman: implement clock mux behaviour Luc Michel
@ 2020-10-10 15:52   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 35+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-10-10 15:52 UTC (permalink / raw)
  To: Luc Michel, qemu-devel
  Cc: Peter Maydell, Havard Skinnemoen, Andrew Baumann, Paul Zimmerman,
	Niek Linnenbank, qemu-arm

On 10/10/20 3:57 PM, Luc Michel wrote:
> A clock mux can be configured to select one of its 10 sources through
> the CM_CTL register. It also embeds yet another clock divider, composed
> of an integer part and a fractional part. The number of bits of each
> part is mux dependent.
> 
> Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Signed-off-by: Luc Michel <luc@lmichel.fr>
> ---
>   hw/misc/bcm2835_cprman.c | 53 +++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 52 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/misc/bcm2835_cprman.c b/hw/misc/bcm2835_cprman.c
> index b22170f3bc..919a55aa23 100644
> --- a/hw/misc/bcm2835_cprman.c
> +++ b/hw/misc/bcm2835_cprman.c
> @@ -229,19 +229,70 @@ static const TypeInfo cprman_pll_channel_info = {
>   };
>   
>   
>   /* clock mux */
>   
> +static bool clock_mux_is_enabled(CprmanClockMuxState *mux)
> +{
> +    return FIELD_EX32(*mux->reg_ctl, CM_CLOCKx_CTL, ENABLE);
> +}
> +
>   static void clock_mux_update(CprmanClockMuxState *mux)
>   {
> -    clock_update(mux->out, 0);
> +    uint64_t freq;
> +    uint32_t div, src = FIELD_EX32(*mux->reg_ctl, CM_CLOCKx_CTL, SRC);
> +    bool enabled = clock_mux_is_enabled(mux);
> +
> +    *mux->reg_ctl = FIELD_DP32(*mux->reg_ctl, CM_CLOCKx_CTL, BUSY, enabled);
> +
> +    if (!enabled) {
> +        clock_update(mux->out, 0);
> +        return;
> +    }
> +
> +    freq = clock_get_hz(mux->srcs[src]);
> +
> +    if (mux->int_bits == 0 && mux->frac_bits == 0) {
> +        clock_update_hz(mux->out, freq);
> +        return;
> +    }
> +
> +    /*
> +     * The divider has an integer and a fractional part. The size of each part
> +     * varies with the muxes (int_bits and frac_bits). Both parts are
> +     * concatenated, with the integer part always starting at bit 12.
> +     *
> +     *         31          12 11          0
> +     *        ------------------------------
> +     * CM_DIV |      |  int  |  frac  |    |
> +     *        ------------------------------
> +     *                <-----> <------>
> +     *                int_bits frac_bits
> +     */
> +    div = extract32(*mux->reg_div,
> +                    R_CM_CLOCKx_DIV_FRAC_LENGTH - mux->frac_bits,
> +                    mux->int_bits + mux->frac_bits);
> +
> +    if (!div) {
> +        clock_update(mux->out, 0);
> +        return;
> +    }
> +
> +    freq = muldiv64(freq, 1 << mux->frac_bits, div);
> +
> +    clock_update_hz(mux->out, freq);
>   }

Much nicer now, thanks :)

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

>   
>   static void clock_mux_src_update(void *opaque)
>   {
>       CprmanClockMuxState **backref = opaque;
>       CprmanClockMuxState *s = *backref;
> +    CprmanClockMuxSource src = backref - s->backref;
> +
> +    if (FIELD_EX32(*s->reg_ctl, CM_CLOCKx_CTL, SRC) != src) {
> +        return;
> +    }
>   
>       clock_mux_update(s);
>   }
>   
>   static void clock_mux_init(Object *obj)
> 


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 08/15] hw/misc/bcm2835_cprman: add a PLL channel skeleton implementation
  2020-10-10 13:57 ` [PATCH v3 08/15] hw/misc/bcm2835_cprman: add a PLL channel skeleton implementation Luc Michel
@ 2020-10-10 16:05   ` Philippe Mathieu-Daudé
  2020-10-17 10:00     ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 35+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-10-10 16:05 UTC (permalink / raw)
  To: Luc Michel, qemu-devel
  Cc: Peter Maydell, Havard Skinnemoen, Andrew Baumann, Paul Zimmerman,
	Niek Linnenbank, qemu-arm

On 10/10/20 3:57 PM, Luc Michel wrote:
> PLLs are composed of multiple channels. Each channel outputs one clock
> signal. They are modeled as one device taking the PLL generated clock as
> input, and outputting a new clock.
> 
> A channel shares the CM register with its parent PLL, and has its own
> A2W_CTRL register. A write to the CM register will trigger an update of
> the PLL and all its channels, while a write to an A2W_CTRL channel
> register will update the required channel only.
> 
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Signed-off-by: Luc Michel <luc@lmichel.fr>
> ---
>   include/hw/misc/bcm2835_cprman.h           |  44 ++++++
>   include/hw/misc/bcm2835_cprman_internals.h | 146 +++++++++++++++++++
>   hw/misc/bcm2835_cprman.c                   | 155 +++++++++++++++++++--
>   3 files changed, 337 insertions(+), 8 deletions(-)
[...]

> diff --git a/include/hw/misc/bcm2835_cprman_internals.h b/include/hw/misc/bcm2835_cprman_internals.h
> index 7aa46c6e18..7409ddb024 100644
> --- a/include/hw/misc/bcm2835_cprman_internals.h
> +++ b/include/hw/misc/bcm2835_cprman_internals.h
> @@ -11,13 +11,16 @@
>   
>   #include "hw/registerfields.h"
>   #include "hw/misc/bcm2835_cprman.h"
>   
>   #define TYPE_CPRMAN_PLL "bcm2835-cprman-pll"
> +#define TYPE_CPRMAN_PLL_CHANNEL "bcm2835-cprman-pll-channel"
>   
>   DECLARE_INSTANCE_CHECKER(CprmanPllState, CPRMAN_PLL,
>                            TYPE_CPRMAN_PLL)
> +DECLARE_INSTANCE_CHECKER(CprmanPllChannelState, CPRMAN_PLL_CHANNEL,
> +                         TYPE_CPRMAN_PLL_CHANNEL)
>   
>   /* Register map */
>   
>   /* PLLs */
>   REG32(CM_PLLA, 0x104)
> @@ -98,10 +101,35 @@ REG32(A2W_PLLA_FRAC, 0x1200)
>   REG32(A2W_PLLC_FRAC, 0x1220)
>   REG32(A2W_PLLD_FRAC, 0x1240)
>   REG32(A2W_PLLH_FRAC, 0x1260)
>   REG32(A2W_PLLB_FRAC, 0x12e0)
>   
> +/* PLL channels */
> +REG32(A2W_PLLA_DSI0, 0x1300)
> +    FIELD(A2W_PLLx_CHANNELy, DIV, 0, 8)
> +    FIELD(A2W_PLLx_CHANNELy, DISABLE, 8, 1)
> +REG32(A2W_PLLA_CORE, 0x1400)
> +REG32(A2W_PLLA_PER, 0x1500)
> +REG32(A2W_PLLA_CCP2, 0x1600)
> +
> +REG32(A2W_PLLC_CORE2, 0x1320)
> +REG32(A2W_PLLC_CORE1, 0x1420)
> +REG32(A2W_PLLC_PER, 0x1520)
> +REG32(A2W_PLLC_CORE0, 0x1620)
> +
> +REG32(A2W_PLLD_DSI0, 0x1340)
> +REG32(A2W_PLLD_CORE, 0x1440)
> +REG32(A2W_PLLD_PER, 0x1540)
> +REG32(A2W_PLLD_DSI1, 0x1640)
> +
> +REG32(A2W_PLLH_AUX, 0x1360)
> +REG32(A2W_PLLH_RCAL, 0x1460)
> +REG32(A2W_PLLH_PIX, 0x1560)
> +REG32(A2W_PLLH_STS, 0x1660)
> +
> +REG32(A2W_PLLB_ARM, 0x13e0)
> +
>   /* misc registers */
>   REG32(CM_LOCK, 0x114)
>       FIELD(CM_LOCK, FLOCKH, 12, 1)
>       FIELD(CM_LOCK, FLOCKD, 11, 1)
>       FIELD(CM_LOCK, FLOCKC, 10, 1)
> @@ -171,6 +199,124 @@ static inline void set_pll_init_info(BCM2835CprmanState *s,
>       pll->reg_a2w_ana = &s->regs[PLL_INIT_INFO[id].a2w_ana_offset];
>       pll->prediv_mask = PLL_INIT_INFO[id].prediv_mask;
>       pll->reg_a2w_frac = &s->regs[PLL_INIT_INFO[id].a2w_frac_offset];
>   }
>   
> +
> +/* PLL channel init info */
> +typedef struct PLLChannelInitInfo {
> +    const char *name;
> +    CprmanPll parent;
> +    size_t cm_offset;
> +    uint32_t cm_hold_mask;
> +    uint32_t cm_load_mask;
> +    size_t a2w_ctrl_offset;
> +    unsigned int fixed_divider;
> +} PLLChannelInitInfo;
> +
> +#define FILL_PLL_CHANNEL_INIT_INFO_common(pll_, channel_)            \
> +    .parent = CPRMAN_ ## pll_,                                       \
> +    .cm_offset = R_CM_ ## pll_,                                      \
> +    .cm_load_mask = R_CM_ ## pll_ ## _ ## LOAD ## channel_ ## _MASK, \
> +    .a2w_ctrl_offset = R_A2W_ ## pll_ ## _ ## channel_
> +
> +#define FILL_PLL_CHANNEL_INIT_INFO(pll_, channel_)                   \
> +    FILL_PLL_CHANNEL_INIT_INFO_common(pll_, channel_),               \
> +    .cm_hold_mask = R_CM_ ## pll_ ## _ ## HOLD ## channel_ ## _MASK, \
> +    .fixed_divider = 1
> +
> +#define FILL_PLL_CHANNEL_INIT_INFO_nohold(pll_, channel_) \
> +    FILL_PLL_CHANNEL_INIT_INFO_common(pll_, channel_),    \
> +    .cm_hold_mask = 0
> +
> +static PLLChannelInitInfo PLL_CHANNEL_INIT_INFO[] = {

Hmm I missed this static definition in an header.
As it is local and only include once, I'd prefer match the
rest of the source tree style and name it .c.inc:

-- >8 --
diff --git a/hw/misc/bcm2835_cprman.c b/hw/misc/bcm2835_cprman.c
index 7e415a017c9..9d4c0ee6c73 100644
--- a/hw/misc/bcm2835_cprman.c
+++ b/hw/misc/bcm2835_cprman.c
@@ -48,7 +48,7 @@
  #include "migration/vmstate.h"
  #include "hw/qdev-properties.h"
  #include "hw/misc/bcm2835_cprman.h"
-#include "hw/misc/bcm2835_cprman_internals.h"
+#include "bcm2835_cprman_internals.c.inc"
  #include "trace.h"

  /* PLL */
diff --git a/include/hw/misc/bcm2835_cprman_internals.h 
b/hw/misc/bcm2835_cprman_internals.c.inc
similarity index 100%
rename from include/hw/misc/bcm2835_cprman_internals.h
rename to hw/misc/bcm2835_cprman_internals.c.inc
---
> +    [CPRMAN_PLLA_CHANNEL_DSI0] = {
> +        .name = "plla-dsi0",
> +        FILL_PLL_CHANNEL_INIT_INFO(PLLA, DSI0),
> +    },
> +    [CPRMAN_PLLA_CHANNEL_CORE] = {
> +        .name = "plla-core",
> +        FILL_PLL_CHANNEL_INIT_INFO(PLLA, CORE),
> +    },
> +    [CPRMAN_PLLA_CHANNEL_PER] = {
> +        .name = "plla-per",
> +        FILL_PLL_CHANNEL_INIT_INFO(PLLA, PER),
> +    },
> +    [CPRMAN_PLLA_CHANNEL_CCP2] = {
> +        .name = "plla-ccp2",
> +        FILL_PLL_CHANNEL_INIT_INFO(PLLA, CCP2),
> +    },
> +
> +    [CPRMAN_PLLC_CHANNEL_CORE2] = {
> +        .name = "pllc-core2",
> +        FILL_PLL_CHANNEL_INIT_INFO(PLLC, CORE2),
> +    },
> +    [CPRMAN_PLLC_CHANNEL_CORE1] = {
> +        .name = "pllc-core1",
> +        FILL_PLL_CHANNEL_INIT_INFO(PLLC, CORE1),
> +    },
> +    [CPRMAN_PLLC_CHANNEL_PER] = {
> +        .name = "pllc-per",
> +        FILL_PLL_CHANNEL_INIT_INFO(PLLC, PER),
> +    },
> +    [CPRMAN_PLLC_CHANNEL_CORE0] = {
> +        .name = "pllc-core0",
> +        FILL_PLL_CHANNEL_INIT_INFO(PLLC, CORE0),
> +    },
> +
> +    [CPRMAN_PLLD_CHANNEL_DSI0] = {
> +        .name = "plld-dsi0",
> +        FILL_PLL_CHANNEL_INIT_INFO(PLLD, DSI0),
> +    },
> +    [CPRMAN_PLLD_CHANNEL_CORE] = {
> +        .name = "plld-core",
> +        FILL_PLL_CHANNEL_INIT_INFO(PLLD, CORE),
> +    },
> +    [CPRMAN_PLLD_CHANNEL_PER] = {
> +        .name = "plld-per",
> +        FILL_PLL_CHANNEL_INIT_INFO(PLLD, PER),
> +    },
> +    [CPRMAN_PLLD_CHANNEL_DSI1] = {
> +        .name = "plld-dsi1",
> +        FILL_PLL_CHANNEL_INIT_INFO(PLLD, DSI1),
> +    },
> +
> +    [CPRMAN_PLLH_CHANNEL_AUX] = {
> +        .name = "pllh-aux",
> +        .fixed_divider = 1,
> +        FILL_PLL_CHANNEL_INIT_INFO_nohold(PLLH, AUX),
> +    },
> +    [CPRMAN_PLLH_CHANNEL_RCAL] = {
> +        .name = "pllh-rcal",
> +        .fixed_divider = 10,
> +        FILL_PLL_CHANNEL_INIT_INFO_nohold(PLLH, RCAL),
> +    },
> +    [CPRMAN_PLLH_CHANNEL_PIX] = {
> +        .name = "pllh-pix",
> +        .fixed_divider = 10,
> +        FILL_PLL_CHANNEL_INIT_INFO_nohold(PLLH, PIX),
> +    },
> +
> +    [CPRMAN_PLLB_CHANNEL_ARM] = {
> +        .name = "pllb-arm",
> +        FILL_PLL_CHANNEL_INIT_INFO(PLLB, ARM),
> +    },
> +};
> +
> +#undef FILL_PLL_CHANNEL_INIT_INFO_nohold
> +#undef FILL_PLL_CHANNEL_INIT_INFO
> +#undef FILL_PLL_CHANNEL_INIT_INFO_common
> +
> +static inline void set_pll_channel_init_info(BCM2835CprmanState *s,
> +                                             CprmanPllChannelState *channel,
> +                                             CprmanPllChannel id)
> +{
> +    channel->id = id;
> +    channel->parent = PLL_CHANNEL_INIT_INFO[id].parent;
> +    channel->reg_cm = &s->regs[PLL_CHANNEL_INIT_INFO[id].cm_offset];
> +    channel->hold_mask = PLL_CHANNEL_INIT_INFO[id].cm_hold_mask;
> +    channel->load_mask = PLL_CHANNEL_INIT_INFO[id].cm_load_mask;
> +    channel->reg_a2w_ctrl = &s->regs[PLL_CHANNEL_INIT_INFO[id].a2w_ctrl_offset];
> +    channel->fixed_divider = PLL_CHANNEL_INIT_INFO[id].fixed_divider;
> +}
> +
>   #endif
[...]


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 13/15] hw/misc/bcm2835_cprman: add sane reset values to the registers
  2020-10-10 13:57 ` [PATCH v3 13/15] hw/misc/bcm2835_cprman: add sane reset values to the registers Luc Michel
@ 2020-10-10 16:18   ` Philippe Mathieu-Daudé
  2020-10-11 18:26     ` Luc Michel
  0 siblings, 1 reply; 35+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-10-10 16:18 UTC (permalink / raw)
  To: Luc Michel, qemu-devel
  Cc: Peter Maydell, Havard Skinnemoen, Andrew Baumann, Paul Zimmerman,
	Niek Linnenbank, qemu-arm

On 10/10/20 3:57 PM, Luc Michel wrote:
> Those reset values have been extracted from a Raspberry Pi 3 model B
> v1.2, using the 2020-08-20 version of raspios. The dump was done using
> the debugfs interface of the CPRMAN driver in Linux (under
> '/sys/kernel/debug/clk'). Each exposed clock tree stage (PLLs, channels
> and muxes) can be observed by reading the 'regdump' file (e.g.
> 'plla/regdump').
> 
> Those values are set by the Raspberry Pi firmware at boot time (Linux
> expects them to be set when it boots up).
> 
> Some stages are not exposed by the Linux driver (e.g. the PLL B). For
> those, the reset values are unknown and left to 0 which implies a
> disabled output.
> 
> Once booted in QEMU, the final clock tree is very similar to the one
> visible on real hardware. The differences come from some unimplemented
> devices for which the driver simply disable the corresponding clock.
> 
> Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Signed-off-by: Luc Michel <luc@lmichel.fr>
> ---
>   include/hw/misc/bcm2835_cprman_internals.h | 269 +++++++++++++++++++++
>   hw/misc/bcm2835_cprman.c                   |  31 +++
>   2 files changed, 300 insertions(+)
> 
> diff --git a/include/hw/misc/bcm2835_cprman_internals.h b/include/hw/misc/bcm2835_cprman_internals.h
> index a6e799075f..339759b307 100644
> --- a/include/hw/misc/bcm2835_cprman_internals.h
> +++ b/include/hw/misc/bcm2835_cprman_internals.h
> @@ -745,6 +745,275 @@ static inline void set_clock_mux_init_info(BCM2835CprmanState *s,
>       mux->reg_div = &s->regs[CLOCK_MUX_INIT_INFO[id].cm_offset + 1];
>       mux->int_bits = CLOCK_MUX_INIT_INFO[id].int_bits;
>       mux->frac_bits = CLOCK_MUX_INIT_INFO[id].frac_bits;
>   }
>   
> +
> +/*
> + * Object reset info
> + * Those values have been dumped from a Raspberry Pi 3 Model B v1.2 using the
> + * clk debugfs interface in Linux.
> + */
> +typedef struct PLLResetInfo {
> +    uint32_t cm;
> +    uint32_t a2w_ctrl;
> +    uint32_t a2w_ana[4];
> +    uint32_t a2w_frac;
> +} PLLResetInfo;
> +
> +static const PLLResetInfo PLL_RESET_INFO[] = {
> +    [CPRMAN_PLLA] = {
> +        .cm = 0x0000008a,
> +        .a2w_ctrl = 0x0002103a,
> +        .a2w_frac = 0x00098000,
> +        .a2w_ana = { 0x00000000, 0x00144000, 0x00000000, 0x00000100 }
> +    },
> +
> +    [CPRMAN_PLLC] = {
> +        .cm = 0x00000228,
> +        .a2w_ctrl = 0x0002103e,
> +        .a2w_frac = 0x00080000,
> +        .a2w_ana = { 0x00000000, 0x00144000, 0x00000000, 0x00000100 }
> +    },
> +
> +    [CPRMAN_PLLD] = {
> +        .cm = 0x0000020a,
> +        .a2w_ctrl = 0x00021034,
> +        .a2w_frac = 0x00015556,
> +        .a2w_ana = { 0x00000000, 0x00144000, 0x00000000, 0x00000100 }
> +    },
> +
> +    [CPRMAN_PLLH] = {
> +        .cm = 0x00000000,
> +        .a2w_ctrl = 0x0002102d,
> +        .a2w_frac = 0x00000000,
> +        .a2w_ana = { 0x00900000, 0x0000000c, 0x00000000, 0x00000000 }
> +    },
> +
> +    [CPRMAN_PLLB] = {
> +        /* unknown */
> +        .cm = 0x00000000,
> +        .a2w_ctrl = 0x00000000,
> +        .a2w_frac = 0x00000000,
> +        .a2w_ana = { 0x00000000, 0x00000000, 0x00000000, 0x00000000 }
> +    }
> +};
> +
> +typedef struct PLLChannelResetInfo {
> +    /*
> +     * Even though a PLL channel has a CM register, it shares it with its
> +     * parent PLL. The parent already takes care of the reset value.
> +     */
> +    uint32_t a2w_ctrl;
> +} PLLChannelResetInfo;
> +
> +static const PLLChannelResetInfo PLL_CHANNEL_RESET_INFO[] = {
> +    [CPRMAN_PLLA_CHANNEL_DSI0] = { .a2w_ctrl = 0x00000100 },
> +    [CPRMAN_PLLA_CHANNEL_CORE] = { .a2w_ctrl = 0x00000003 },
> +    [CPRMAN_PLLA_CHANNEL_PER] = { .a2w_ctrl = 0x00000000 }, /* unknown */
> +    [CPRMAN_PLLA_CHANNEL_CCP2] = { .a2w_ctrl = 0x00000100 },
> +
> +    [CPRMAN_PLLC_CHANNEL_CORE2] = { .a2w_ctrl = 0x00000100 },
> +    [CPRMAN_PLLC_CHANNEL_CORE1] = { .a2w_ctrl = 0x00000100 },
> +    [CPRMAN_PLLC_CHANNEL_PER] = { .a2w_ctrl = 0x00000002 },
> +    [CPRMAN_PLLC_CHANNEL_CORE0] = { .a2w_ctrl = 0x00000002 },
> +
> +    [CPRMAN_PLLD_CHANNEL_DSI0] = { .a2w_ctrl = 0x00000100 },
> +    [CPRMAN_PLLD_CHANNEL_CORE] = { .a2w_ctrl = 0x00000004 },
> +    [CPRMAN_PLLD_CHANNEL_PER] = { .a2w_ctrl = 0x00000004 },
> +    [CPRMAN_PLLD_CHANNEL_DSI1] = { .a2w_ctrl = 0x00000100 },
> +
> +    [CPRMAN_PLLH_CHANNEL_AUX] = { .a2w_ctrl = 0x00000004 },
> +    [CPRMAN_PLLH_CHANNEL_RCAL] = { .a2w_ctrl = 0x00000000 },
> +    [CPRMAN_PLLH_CHANNEL_PIX] = { .a2w_ctrl = 0x00000000 },
> +
> +    [CPRMAN_PLLB_CHANNEL_ARM] = { .a2w_ctrl = 0x00000000 }, /* unknown */
> +};
> +
> +typedef struct ClockMuxResetInfo {
> +    uint32_t cm_ctl;
> +    uint32_t cm_div;
> +} ClockMuxResetInfo;
> +
> +static const ClockMuxResetInfo CLOCK_MUX_RESET_INFO[] = {
> +    [CPRMAN_CLOCK_GNRIC] = {
> +        .cm_ctl = 0, /* unknown */
> +        .cm_div = 0
> +    },
> +
[...]
> +};
> +
>   #endif
> diff --git a/hw/misc/bcm2835_cprman.c b/hw/misc/bcm2835_cprman.c
> index 7a7401963d..7e415a017c 100644
> --- a/hw/misc/bcm2835_cprman.c
> +++ b/hw/misc/bcm2835_cprman.c
> @@ -51,10 +51,21 @@
>   #include "hw/misc/bcm2835_cprman_internals.h"
>   #include "trace.h"
>   
>   /* PLL */
>   
> +static void pll_reset(DeviceState *dev)
> +{
> +    CprmanPllState *s = CPRMAN_PLL(dev);
> +    const PLLResetInfo *info = &PLL_RESET_INFO[s->id];

Hmm so we overwrite various values from PLL_INIT_INFO.
> +
> +    *s->reg_cm = info->cm;
> +    *s->reg_a2w_ctrl = info->a2w_ctrl;
> +    memcpy(s->reg_a2w_ana, info->a2w_ana, sizeof(info->a2w_ana));
> +    *s->reg_a2w_frac = info->a2w_frac;

set_pll_init_info() can be simplified as:

     pll->id = id;
     pll->prediv_mask = PLL_INIT_INFO[id].prediv_mask;

Or directly in cprman_init():

     &s->plls[i]->id = i;
     &s->plls[i]->prediv_mask = PLL_INIT_INFO[i].prediv_mask;

And the rest directly implemented in pll_reset().

Maybe not, but having pll_reset() added in patch #8/15
"bcm2835_cprman: add a PLL channel skeleton implementation"
would make this patch review easier ;)

> +}
> +
>   static bool pll_is_locked(const CprmanPllState *pll)
>   {
>       return !FIELD_EX32(*pll->reg_a2w_ctrl, A2W_PLLx_CTRL, PWRDN)
>           && !FIELD_EX32(*pll->reg_cm, CM_PLLx, ANARST);
>   }
> @@ -121,10 +132,11 @@ static const VMStateDescription pll_vmstate = {
>   
>   static void pll_class_init(ObjectClass *klass, void *data)
>   {
>       DeviceClass *dc = DEVICE_CLASS(klass);
>   
> +    dc->reset = pll_reset;
>       dc->vmsd = &pll_vmstate;
>   }
[...]
Similarly, implement clock_mux_reset() in patch #10/15
"bcm2835_cprman: add a clock mux skeleton implementation".

Regards,

Phil.


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 13/15] hw/misc/bcm2835_cprman: add sane reset values to the registers
  2020-10-10 16:18   ` Philippe Mathieu-Daudé
@ 2020-10-11 18:26     ` Luc Michel
  2020-10-16 17:10       ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 35+ messages in thread
From: Luc Michel @ 2020-10-11 18:26 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Peter Maydell, qemu-devel, Andrew Baumann, Paul Zimmerman,
	Niek Linnenbank, qemu-arm, Havard Skinnemoen

On 18:18 Sat 10 Oct     , Philippe Mathieu-Daudé wrote:
> On 10/10/20 3:57 PM, Luc Michel wrote:
> > Those reset values have been extracted from a Raspberry Pi 3 model B
> > v1.2, using the 2020-08-20 version of raspios. The dump was done using
> > the debugfs interface of the CPRMAN driver in Linux (under
> > '/sys/kernel/debug/clk'). Each exposed clock tree stage (PLLs, channels
> > and muxes) can be observed by reading the 'regdump' file (e.g.
> > 'plla/regdump').
> > 
> > Those values are set by the Raspberry Pi firmware at boot time (Linux
> > expects them to be set when it boots up).
> > 
> > Some stages are not exposed by the Linux driver (e.g. the PLL B). For
> > those, the reset values are unknown and left to 0 which implies a
> > disabled output.
> > 
> > Once booted in QEMU, the final clock tree is very similar to the one
> > visible on real hardware. The differences come from some unimplemented
> > devices for which the driver simply disable the corresponding clock.
> > 
> > Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> > Signed-off-by: Luc Michel <luc@lmichel.fr>
> > ---
> >   include/hw/misc/bcm2835_cprman_internals.h | 269 +++++++++++++++++++++
> >   hw/misc/bcm2835_cprman.c                   |  31 +++
> >   2 files changed, 300 insertions(+)
> > 
> > diff --git a/include/hw/misc/bcm2835_cprman_internals.h b/include/hw/misc/bcm2835_cprman_internals.h
> > index a6e799075f..339759b307 100644
> > --- a/include/hw/misc/bcm2835_cprman_internals.h
> > +++ b/include/hw/misc/bcm2835_cprman_internals.h
> > @@ -745,6 +745,275 @@ static inline void set_clock_mux_init_info(BCM2835CprmanState *s,
> >       mux->reg_div = &s->regs[CLOCK_MUX_INIT_INFO[id].cm_offset + 1];
> >       mux->int_bits = CLOCK_MUX_INIT_INFO[id].int_bits;
> >       mux->frac_bits = CLOCK_MUX_INIT_INFO[id].frac_bits;
> >   }
> > +
> > +/*
> > + * Object reset info
> > + * Those values have been dumped from a Raspberry Pi 3 Model B v1.2 using the
> > + * clk debugfs interface in Linux.
> > + */
> > +typedef struct PLLResetInfo {
> > +    uint32_t cm;
> > +    uint32_t a2w_ctrl;
> > +    uint32_t a2w_ana[4];
> > +    uint32_t a2w_frac;
> > +} PLLResetInfo;
> > +
> > +static const PLLResetInfo PLL_RESET_INFO[] = {
> > +    [CPRMAN_PLLA] = {
> > +        .cm = 0x0000008a,
> > +        .a2w_ctrl = 0x0002103a,
> > +        .a2w_frac = 0x00098000,
> > +        .a2w_ana = { 0x00000000, 0x00144000, 0x00000000, 0x00000100 }
> > +    },
> > +
> > +    [CPRMAN_PLLC] = {
> > +        .cm = 0x00000228,
> > +        .a2w_ctrl = 0x0002103e,
> > +        .a2w_frac = 0x00080000,
> > +        .a2w_ana = { 0x00000000, 0x00144000, 0x00000000, 0x00000100 }
> > +    },
> > +
> > +    [CPRMAN_PLLD] = {
> > +        .cm = 0x0000020a,
> > +        .a2w_ctrl = 0x00021034,
> > +        .a2w_frac = 0x00015556,
> > +        .a2w_ana = { 0x00000000, 0x00144000, 0x00000000, 0x00000100 }
> > +    },
> > +
> > +    [CPRMAN_PLLH] = {
> > +        .cm = 0x00000000,
> > +        .a2w_ctrl = 0x0002102d,
> > +        .a2w_frac = 0x00000000,
> > +        .a2w_ana = { 0x00900000, 0x0000000c, 0x00000000, 0x00000000 }
> > +    },
> > +
> > +    [CPRMAN_PLLB] = {
> > +        /* unknown */
> > +        .cm = 0x00000000,
> > +        .a2w_ctrl = 0x00000000,
> > +        .a2w_frac = 0x00000000,
> > +        .a2w_ana = { 0x00000000, 0x00000000, 0x00000000, 0x00000000 }
> > +    }
> > +};
> > +
> > +typedef struct PLLChannelResetInfo {
> > +    /*
> > +     * Even though a PLL channel has a CM register, it shares it with its
> > +     * parent PLL. The parent already takes care of the reset value.
> > +     */
> > +    uint32_t a2w_ctrl;
> > +} PLLChannelResetInfo;
> > +
> > +static const PLLChannelResetInfo PLL_CHANNEL_RESET_INFO[] = {
> > +    [CPRMAN_PLLA_CHANNEL_DSI0] = { .a2w_ctrl = 0x00000100 },
> > +    [CPRMAN_PLLA_CHANNEL_CORE] = { .a2w_ctrl = 0x00000003 },
> > +    [CPRMAN_PLLA_CHANNEL_PER] = { .a2w_ctrl = 0x00000000 }, /* unknown */
> > +    [CPRMAN_PLLA_CHANNEL_CCP2] = { .a2w_ctrl = 0x00000100 },
> > +
> > +    [CPRMAN_PLLC_CHANNEL_CORE2] = { .a2w_ctrl = 0x00000100 },
> > +    [CPRMAN_PLLC_CHANNEL_CORE1] = { .a2w_ctrl = 0x00000100 },
> > +    [CPRMAN_PLLC_CHANNEL_PER] = { .a2w_ctrl = 0x00000002 },
> > +    [CPRMAN_PLLC_CHANNEL_CORE0] = { .a2w_ctrl = 0x00000002 },
> > +
> > +    [CPRMAN_PLLD_CHANNEL_DSI0] = { .a2w_ctrl = 0x00000100 },
> > +    [CPRMAN_PLLD_CHANNEL_CORE] = { .a2w_ctrl = 0x00000004 },
> > +    [CPRMAN_PLLD_CHANNEL_PER] = { .a2w_ctrl = 0x00000004 },
> > +    [CPRMAN_PLLD_CHANNEL_DSI1] = { .a2w_ctrl = 0x00000100 },
> > +
> > +    [CPRMAN_PLLH_CHANNEL_AUX] = { .a2w_ctrl = 0x00000004 },
> > +    [CPRMAN_PLLH_CHANNEL_RCAL] = { .a2w_ctrl = 0x00000000 },
> > +    [CPRMAN_PLLH_CHANNEL_PIX] = { .a2w_ctrl = 0x00000000 },
> > +
> > +    [CPRMAN_PLLB_CHANNEL_ARM] = { .a2w_ctrl = 0x00000000 }, /* unknown */
> > +};
> > +
> > +typedef struct ClockMuxResetInfo {
> > +    uint32_t cm_ctl;
> > +    uint32_t cm_div;
> > +} ClockMuxResetInfo;
> > +
> > +static const ClockMuxResetInfo CLOCK_MUX_RESET_INFO[] = {
> > +    [CPRMAN_CLOCK_GNRIC] = {
> > +        .cm_ctl = 0, /* unknown */
> > +        .cm_div = 0
> > +    },
> > +
> [...]
> > +};
> > +
> >   #endif
> > diff --git a/hw/misc/bcm2835_cprman.c b/hw/misc/bcm2835_cprman.c
> > index 7a7401963d..7e415a017c 100644
> > --- a/hw/misc/bcm2835_cprman.c
> > +++ b/hw/misc/bcm2835_cprman.c
> > @@ -51,10 +51,21 @@
> >   #include "hw/misc/bcm2835_cprman_internals.h"
> >   #include "trace.h"
> >   /* PLL */
> > +static void pll_reset(DeviceState *dev)
> > +{
> > +    CprmanPllState *s = CPRMAN_PLL(dev);
> > +    const PLLResetInfo *info = &PLL_RESET_INFO[s->id];
> 
> Hmm so we overwrite various values from PLL_INIT_INFO.
> > +
> > +    *s->reg_cm = info->cm;
> > +    *s->reg_a2w_ctrl = info->a2w_ctrl;
> > +    memcpy(s->reg_a2w_ana, info->a2w_ana, sizeof(info->a2w_ana));
> > +    *s->reg_a2w_frac = info->a2w_frac;
> 
> set_pll_init_info() can be simplified as:
> 
>     pll->id = id;
>     pll->prediv_mask = PLL_INIT_INFO[id].prediv_mask;
> 
> Or directly in cprman_init():
> 
>     &s->plls[i]->id = i;
>     &s->plls[i]->prediv_mask = PLL_INIT_INFO[i].prediv_mask;
> 
> And the rest directly implemented in pll_reset().
> 
> Maybe not, but having pll_reset() added in patch #8/15
> "bcm2835_cprman: add a PLL channel skeleton implementation"
> would make this patch review easier ;)

Hi Phil,

I think there is a misunderstanding here:
  - set_xxx_init_info functions set (among others) register pointers
    to alias the common register array "regs" in BCM2835CprmanState.
    This is really an initialization step (in the sense of the QOM
    object). Those pointers won't move during the object's lifetime.
  - xxx_reset however (like e.g. xxx_update) _dereferences_ those
    pointers to access the registers data (in this case to set their
    reset values).

Doing so greatly decreases code complexity because:
  - read/write functions can directly access the common "regs" array
    without further decoding.
  - Each PLL shares a register with all its channels (A2W_CTRL). With
    this scheme, they simply all have a pointer aliasing the same data.
  - A lot a registers are unknown/unimplemented.

Thanks !

-- 
Luc

> 
> > +}
> > +
> >   static bool pll_is_locked(const CprmanPllState *pll)
> >   {
> >       return !FIELD_EX32(*pll->reg_a2w_ctrl, A2W_PLLx_CTRL, PWRDN)
> >           && !FIELD_EX32(*pll->reg_cm, CM_PLLx, ANARST);
> >   }
> > @@ -121,10 +132,11 @@ static const VMStateDescription pll_vmstate = {
> >   static void pll_class_init(ObjectClass *klass, void *data)
> >   {
> >       DeviceClass *dc = DEVICE_CLASS(klass);
> > +    dc->reset = pll_reset;
> >       dc->vmsd = &pll_vmstate;
> >   }
> [...]
> Similarly, implement clock_mux_reset() in patch #10/15
> "bcm2835_cprman: add a clock mux skeleton implementation".
> 
> Regards,
> 
> Phil.

-- 


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 13/15] hw/misc/bcm2835_cprman: add sane reset values to the registers
  2020-10-11 18:26     ` Luc Michel
@ 2020-10-16 17:10       ` Philippe Mathieu-Daudé
  2020-10-19 15:44         ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 35+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-10-16 17:10 UTC (permalink / raw)
  To: Luc Michel
  Cc: Peter Maydell, qemu-devel, Andrew Baumann, Paul Zimmerman,
	Niek Linnenbank, qemu-arm, Havard Skinnemoen

On 10/11/20 8:26 PM, Luc Michel wrote:
> On 18:18 Sat 10 Oct     , Philippe Mathieu-Daudé wrote:
>> On 10/10/20 3:57 PM, Luc Michel wrote:
>>> Those reset values have been extracted from a Raspberry Pi 3 model B
>>> v1.2, using the 2020-08-20 version of raspios. The dump was done using
>>> the debugfs interface of the CPRMAN driver in Linux (under
>>> '/sys/kernel/debug/clk'). Each exposed clock tree stage (PLLs, channels
>>> and muxes) can be observed by reading the 'regdump' file (e.g.
>>> 'plla/regdump').
>>>
>>> Those values are set by the Raspberry Pi firmware at boot time (Linux
>>> expects them to be set when it boots up).
>>>
>>> Some stages are not exposed by the Linux driver (e.g. the PLL B). For
>>> those, the reset values are unknown and left to 0 which implies a
>>> disabled output.
>>>
>>> Once booted in QEMU, the final clock tree is very similar to the one
>>> visible on real hardware. The differences come from some unimplemented
>>> devices for which the driver simply disable the corresponding clock.
>>>
>>> Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>> Signed-off-by: Luc Michel <luc@lmichel.fr>
>>> ---
>>>    include/hw/misc/bcm2835_cprman_internals.h | 269 +++++++++++++++++++++
>>>    hw/misc/bcm2835_cprman.c                   |  31 +++
>>>    2 files changed, 300 insertions(+)
>>>
>>> diff --git a/include/hw/misc/bcm2835_cprman_internals.h b/include/hw/misc/bcm2835_cprman_internals.h
>>> index a6e799075f..339759b307 100644
>>> --- a/include/hw/misc/bcm2835_cprman_internals.h
>>> +++ b/include/hw/misc/bcm2835_cprman_internals.h
>>> @@ -745,6 +745,275 @@ static inline void set_clock_mux_init_info(BCM2835CprmanState *s,
>>>        mux->reg_div = &s->regs[CLOCK_MUX_INIT_INFO[id].cm_offset + 1];
>>>        mux->int_bits = CLOCK_MUX_INIT_INFO[id].int_bits;
>>>        mux->frac_bits = CLOCK_MUX_INIT_INFO[id].frac_bits;
>>>    }
>>> +
>>> +/*
>>> + * Object reset info
>>> + * Those values have been dumped from a Raspberry Pi 3 Model B v1.2 using the
>>> + * clk debugfs interface in Linux.
>>> + */
>>> +typedef struct PLLResetInfo {
>>> +    uint32_t cm;
>>> +    uint32_t a2w_ctrl;
>>> +    uint32_t a2w_ana[4];
>>> +    uint32_t a2w_frac;
>>> +} PLLResetInfo;
>>> +
>>> +static const PLLResetInfo PLL_RESET_INFO[] = {
>>> +    [CPRMAN_PLLA] = {
>>> +        .cm = 0x0000008a,
>>> +        .a2w_ctrl = 0x0002103a,
>>> +        .a2w_frac = 0x00098000,
>>> +        .a2w_ana = { 0x00000000, 0x00144000, 0x00000000, 0x00000100 }
>>> +    },
>>> +
>>> +    [CPRMAN_PLLC] = {
>>> +        .cm = 0x00000228,
>>> +        .a2w_ctrl = 0x0002103e,
>>> +        .a2w_frac = 0x00080000,
>>> +        .a2w_ana = { 0x00000000, 0x00144000, 0x00000000, 0x00000100 }
>>> +    },
>>> +
>>> +    [CPRMAN_PLLD] = {
>>> +        .cm = 0x0000020a,
>>> +        .a2w_ctrl = 0x00021034,
>>> +        .a2w_frac = 0x00015556,
>>> +        .a2w_ana = { 0x00000000, 0x00144000, 0x00000000, 0x00000100 }
>>> +    },
>>> +
>>> +    [CPRMAN_PLLH] = {
>>> +        .cm = 0x00000000,
>>> +        .a2w_ctrl = 0x0002102d,
>>> +        .a2w_frac = 0x00000000,
>>> +        .a2w_ana = { 0x00900000, 0x0000000c, 0x00000000, 0x00000000 }
>>> +    },
>>> +
>>> +    [CPRMAN_PLLB] = {
>>> +        /* unknown */
>>> +        .cm = 0x00000000,
>>> +        .a2w_ctrl = 0x00000000,
>>> +        .a2w_frac = 0x00000000,
>>> +        .a2w_ana = { 0x00000000, 0x00000000, 0x00000000, 0x00000000 }
>>> +    }
>>> +};
>>> +
>>> +typedef struct PLLChannelResetInfo {
>>> +    /*
>>> +     * Even though a PLL channel has a CM register, it shares it with its
>>> +     * parent PLL. The parent already takes care of the reset value.
>>> +     */
>>> +    uint32_t a2w_ctrl;
>>> +} PLLChannelResetInfo;
>>> +
>>> +static const PLLChannelResetInfo PLL_CHANNEL_RESET_INFO[] = {
>>> +    [CPRMAN_PLLA_CHANNEL_DSI0] = { .a2w_ctrl = 0x00000100 },
>>> +    [CPRMAN_PLLA_CHANNEL_CORE] = { .a2w_ctrl = 0x00000003 },
>>> +    [CPRMAN_PLLA_CHANNEL_PER] = { .a2w_ctrl = 0x00000000 }, /* unknown */
>>> +    [CPRMAN_PLLA_CHANNEL_CCP2] = { .a2w_ctrl = 0x00000100 },
>>> +
>>> +    [CPRMAN_PLLC_CHANNEL_CORE2] = { .a2w_ctrl = 0x00000100 },
>>> +    [CPRMAN_PLLC_CHANNEL_CORE1] = { .a2w_ctrl = 0x00000100 },
>>> +    [CPRMAN_PLLC_CHANNEL_PER] = { .a2w_ctrl = 0x00000002 },
>>> +    [CPRMAN_PLLC_CHANNEL_CORE0] = { .a2w_ctrl = 0x00000002 },
>>> +
>>> +    [CPRMAN_PLLD_CHANNEL_DSI0] = { .a2w_ctrl = 0x00000100 },
>>> +    [CPRMAN_PLLD_CHANNEL_CORE] = { .a2w_ctrl = 0x00000004 },
>>> +    [CPRMAN_PLLD_CHANNEL_PER] = { .a2w_ctrl = 0x00000004 },
>>> +    [CPRMAN_PLLD_CHANNEL_DSI1] = { .a2w_ctrl = 0x00000100 },
>>> +
>>> +    [CPRMAN_PLLH_CHANNEL_AUX] = { .a2w_ctrl = 0x00000004 },
>>> +    [CPRMAN_PLLH_CHANNEL_RCAL] = { .a2w_ctrl = 0x00000000 },
>>> +    [CPRMAN_PLLH_CHANNEL_PIX] = { .a2w_ctrl = 0x00000000 },
>>> +
>>> +    [CPRMAN_PLLB_CHANNEL_ARM] = { .a2w_ctrl = 0x00000000 }, /* unknown */
>>> +};
>>> +
>>> +typedef struct ClockMuxResetInfo {
>>> +    uint32_t cm_ctl;
>>> +    uint32_t cm_div;
>>> +} ClockMuxResetInfo;
>>> +
>>> +static const ClockMuxResetInfo CLOCK_MUX_RESET_INFO[] = {
>>> +    [CPRMAN_CLOCK_GNRIC] = {
>>> +        .cm_ctl = 0, /* unknown */
>>> +        .cm_div = 0
>>> +    },
>>> +
>> [...]
>>> +};
>>> +
>>>    #endif
>>> diff --git a/hw/misc/bcm2835_cprman.c b/hw/misc/bcm2835_cprman.c
>>> index 7a7401963d..7e415a017c 100644
>>> --- a/hw/misc/bcm2835_cprman.c
>>> +++ b/hw/misc/bcm2835_cprman.c
>>> @@ -51,10 +51,21 @@
>>>    #include "hw/misc/bcm2835_cprman_internals.h"
>>>    #include "trace.h"
>>>    /* PLL */
>>> +static void pll_reset(DeviceState *dev)
>>> +{
>>> +    CprmanPllState *s = CPRMAN_PLL(dev);
>>> +    const PLLResetInfo *info = &PLL_RESET_INFO[s->id];
>>
>> Hmm so we overwrite various values from PLL_INIT_INFO.
>>> +
>>> +    *s->reg_cm = info->cm;
>>> +    *s->reg_a2w_ctrl = info->a2w_ctrl;
>>> +    memcpy(s->reg_a2w_ana, info->a2w_ana, sizeof(info->a2w_ana));
>>> +    *s->reg_a2w_frac = info->a2w_frac;
>>
>> set_pll_init_info() can be simplified as:
>>
>>      pll->id = id;
>>      pll->prediv_mask = PLL_INIT_INFO[id].prediv_mask;
>>
>> Or directly in cprman_init():
>>
>>      &s->plls[i]->id = i;
>>      &s->plls[i]->prediv_mask = PLL_INIT_INFO[i].prediv_mask;
>>
>> And the rest directly implemented in pll_reset().
>>
>> Maybe not, but having pll_reset() added in patch #8/15
>> "bcm2835_cprman: add a PLL channel skeleton implementation"
>> would make this patch review easier ;)
> 
> Hi Phil,
> 
> I think there is a misunderstanding here:
>    - set_xxx_init_info functions set (among others) register pointers
>      to alias the common register array "regs" in BCM2835CprmanState.
>      This is really an initialization step (in the sense of the QOM
>      object). Those pointers won't move during the object's lifetime.
>    - xxx_reset however (like e.g. xxx_update) _dereferences_ those
>      pointers to access the registers data (in this case to set their
>      reset values).
> 
> Doing so greatly decreases code complexity because:
>    - read/write functions can directly access the common "regs" array
>      without further decoding.
>    - Each PLL shares a register with all its channels (A2W_CTRL). With
>      this scheme, they simply all have a pointer aliasing the same data.
>    - A lot a registers are unknown/unimplemented.

OK, thanks for clarifying.
(I wanted to split the boilerplate code from the dumped constants).


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 00/15] raspi: add the bcm2835 cprman clock manager
  2020-10-10 13:57 [PATCH v3 00/15] raspi: add the bcm2835 cprman clock manager Luc Michel
                   ` (14 preceding siblings ...)
  2020-10-10 13:57 ` [PATCH v3 15/15] hw/arm/bcm2835_peripherals: connect the UART clock Luc Michel
@ 2020-10-16 17:11 ` Philippe Mathieu-Daudé
  2020-10-19 15:45 ` Peter Maydell
  2020-10-22 22:06 ` Philippe Mathieu-Daudé
  17 siblings, 0 replies; 35+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-10-16 17:11 UTC (permalink / raw)
  To: Luc Michel, qemu-devel
  Cc: Peter Maydell, Havard Skinnemoen, Andrew Baumann, Paul Zimmerman,
	Niek Linnenbank, qemu-arm

On 10/10/20 3:57 PM, Luc Michel wrote:
[...]
> Hi,
> 
> This series add the BCM2835 CPRMAN clock manager peripheral to the
> Raspberry Pi machine.

Series:
Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 08/15] hw/misc/bcm2835_cprman: add a PLL channel skeleton implementation
  2020-10-10 16:05   ` Philippe Mathieu-Daudé
@ 2020-10-17 10:00     ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 35+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-10-17 10:00 UTC (permalink / raw)
  To: Luc Michel, qemu-devel
  Cc: Peter Maydell, Havard Skinnemoen, Andrew Baumann, Paul Zimmerman,
	Niek Linnenbank, qemu-arm

On 10/10/20 6:05 PM, Philippe Mathieu-Daudé wrote:
> On 10/10/20 3:57 PM, Luc Michel wrote:
>> PLLs are composed of multiple channels. Each channel outputs one clock
>> signal. They are modeled as one device taking the PLL generated clock as
>> input, and outputting a new clock.
>>
>> A channel shares the CM register with its parent PLL, and has its own
>> A2W_CTRL register. A write to the CM register will trigger an update of
>> the PLL and all its channels, while a write to an A2W_CTRL channel
>> register will update the required channel only.
>>
>> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> Signed-off-by: Luc Michel <luc@lmichel.fr>
>> ---
>>   include/hw/misc/bcm2835_cprman.h           |  44 ++++++
>>   include/hw/misc/bcm2835_cprman_internals.h | 146 +++++++++++++++++++
>>   hw/misc/bcm2835_cprman.c                   | 155 +++++++++++++++++++--
>>   3 files changed, 337 insertions(+), 8 deletions(-)
[...]

>> +#define FILL_PLL_CHANNEL_INIT_INFO_common(pll_, channel_)            \
>> +    .parent = CPRMAN_ ## pll_,                                       \
>> +    .cm_offset = R_CM_ ## pll_,                                      \
>> +    .cm_load_mask = R_CM_ ## pll_ ## _ ## LOAD ## channel_ ## _MASK, \
>> +    .a2w_ctrl_offset = R_A2W_ ## pll_ ## _ ## channel_
>> +
>> +#define FILL_PLL_CHANNEL_INIT_INFO(pll_, channel_)                   \
>> +    FILL_PLL_CHANNEL_INIT_INFO_common(pll_, channel_),               \
>> +    .cm_hold_mask = R_CM_ ## pll_ ## _ ## HOLD ## channel_ ## _MASK, \
>> +    .fixed_divider = 1
>> +
>> +#define FILL_PLL_CHANNEL_INIT_INFO_nohold(pll_, channel_) \
>> +    FILL_PLL_CHANNEL_INIT_INFO_common(pll_, channel_),    \
>> +    .cm_hold_mask = 0
>> +
>> +static PLLChannelInitInfo PLL_CHANNEL_INIT_INFO[] = {
> 
> Hmm I missed this static definition in an header.
> As it is local and only include once, I'd prefer match the
> rest of the source tree style and name it .c.inc:
> 
> -- >8 --
> diff --git a/hw/misc/bcm2835_cprman.c b/hw/misc/bcm2835_cprman.c
> index 7e415a017c9..9d4c0ee6c73 100644
> --- a/hw/misc/bcm2835_cprman.c
> +++ b/hw/misc/bcm2835_cprman.c
> @@ -48,7 +48,7 @@
>   #include "migration/vmstate.h"
>   #include "hw/qdev-properties.h"
>   #include "hw/misc/bcm2835_cprman.h"
> -#include "hw/misc/bcm2835_cprman_internals.h"
> +#include "bcm2835_cprman_internals.c.inc"
>   #include "trace.h"
> 
>   /* PLL */
> diff --git a/include/hw/misc/bcm2835_cprman_internals.h 
> b/hw/misc/bcm2835_cprman_internals.c.inc
> similarity index 100%
> rename from include/hw/misc/bcm2835_cprman_internals.h
> rename to hw/misc/bcm2835_cprman_internals.c.inc
> ---

This can be applied directly by Peter, or can
be cleaned later. This is not a blocker to get
this series merged.


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 13/15] hw/misc/bcm2835_cprman: add sane reset values to the registers
  2020-10-16 17:10       ` Philippe Mathieu-Daudé
@ 2020-10-19 15:44         ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 35+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-10-19 15:44 UTC (permalink / raw)
  To: Luc Michel
  Cc: Peter Maydell, qemu-devel, Andrew Baumann, Paul Zimmerman,
	Niek Linnenbank, qemu-arm, Havard Skinnemoen

On 10/16/20 7:10 PM, Philippe Mathieu-Daudé wrote:
> On 10/11/20 8:26 PM, Luc Michel wrote:
>> On 18:18 Sat 10 Oct     , Philippe Mathieu-Daudé wrote:
>>> On 10/10/20 3:57 PM, Luc Michel wrote:
>>>> Those reset values have been extracted from a Raspberry Pi 3 model B
>>>> v1.2, using the 2020-08-20 version of raspios. The dump was done using
>>>> the debugfs interface of the CPRMAN driver in Linux (under
>>>> '/sys/kernel/debug/clk'). Each exposed clock tree stage (PLLs, channels
>>>> and muxes) can be observed by reading the 'regdump' file (e.g.
>>>> 'plla/regdump').
>>>>
>>>> Those values are set by the Raspberry Pi firmware at boot time (Linux
>>>> expects them to be set when it boots up).
>>>>
>>>> Some stages are not exposed by the Linux driver (e.g. the PLL B). For
>>>> those, the reset values are unknown and left to 0 which implies a
>>>> disabled output.
>>>>
>>>> Once booted in QEMU, the final clock tree is very similar to the one
>>>> visible on real hardware. The differences come from some unimplemented
>>>> devices for which the driver simply disable the corresponding clock.
>>>>
>>>> Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>>> Signed-off-by: Luc Michel <luc@lmichel.fr>
>>>> ---
>>>>    include/hw/misc/bcm2835_cprman_internals.h | 269 
>>>> +++++++++++++++++++++
>>>>    hw/misc/bcm2835_cprman.c                   |  31 +++
>>>>    2 files changed, 300 insertions(+)
[...]

I haven't verified the dumped values with real hardware,
but for the rest this patch shouldn't introduce regression,
and it helps to boot up to Linux kernel 5.7, so:

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 00/15] raspi: add the bcm2835 cprman clock manager
  2020-10-10 13:57 [PATCH v3 00/15] raspi: add the bcm2835 cprman clock manager Luc Michel
                   ` (15 preceding siblings ...)
  2020-10-16 17:11 ` [PATCH v3 00/15] raspi: add the bcm2835 cprman clock manager Philippe Mathieu-Daudé
@ 2020-10-19 15:45 ` Peter Maydell
  2020-10-19 19:31   ` Peter Maydell
  2020-10-22 22:06 ` Philippe Mathieu-Daudé
  17 siblings, 1 reply; 35+ messages in thread
From: Peter Maydell @ 2020-10-19 15:45 UTC (permalink / raw)
  To: Luc Michel
  Cc: QEMU Developers, Andrew Baumann, Paul Zimmerman, Niek Linnenbank,
	qemu-arm, Havard Skinnemoen, Philippe Mathieu-Daudé

On Sat, 10 Oct 2020 at 14:57, Luc Michel <luc@lmichel.fr> wrote:
>
> v2 -> v3:
>   - patch 03: moved clock_new definition to hw/core/clock.c [Phil]
>   - patch 03: commit message typo [Clement]
>   - patch 10: clarifications around the CM_CTL/CM_DIBV mux registers.
>               reg_cm replaced with reg_ctl and reg_div. Add some
>               comments for clarity. [Phil]
>   - patch 10: fixed update_mux_from_cm not matching the CM_DIV offset
>               correctly. [Phil]
>   - patch 11: replaced manual bitfield extraction with extract32 [Phil]
>   - patch 11: added a visual representation of CM_DIV for clarity [Phil]
>   - patch 11: added a missing return in clock_mux_update.



Applied to target-arm.next, thanks.

-- PMM


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 00/15] raspi: add the bcm2835 cprman clock manager
  2020-10-19 15:45 ` Peter Maydell
@ 2020-10-19 19:31   ` Peter Maydell
  2020-10-27  8:55     ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 35+ messages in thread
From: Peter Maydell @ 2020-10-19 19:31 UTC (permalink / raw)
  To: Luc Michel
  Cc: QEMU Developers, Andrew Baumann, Paul Zimmerman, Niek Linnenbank,
	qemu-arm, Havard Skinnemoen, Philippe Mathieu-Daudé

On Mon, 19 Oct 2020 at 16:45, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Sat, 10 Oct 2020 at 14:57, Luc Michel <luc@lmichel.fr> wrote:
> >
> > v2 -> v3:
> >   - patch 03: moved clock_new definition to hw/core/clock.c [Phil]
> >   - patch 03: commit message typo [Clement]
> >   - patch 10: clarifications around the CM_CTL/CM_DIBV mux registers.
> >               reg_cm replaced with reg_ctl and reg_div. Add some
> >               comments for clarity. [Phil]
> >   - patch 10: fixed update_mux_from_cm not matching the CM_DIV offset
> >               correctly. [Phil]
> >   - patch 11: replaced manual bitfield extraction with extract32 [Phil]
> >   - patch 11: added a visual representation of CM_DIV for clarity [Phil]
> >   - patch 11: added a missing return in clock_mux_update.
>
>
>
> Applied to target-arm.next, thanks.

Dropped again due to segv in 'make check' when running with
clang sanitizer, which I gather from irc that you're looking
into.

thanks
-- PMM


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 00/15] raspi: add the bcm2835 cprman clock manager
  2020-10-10 13:57 [PATCH v3 00/15] raspi: add the bcm2835 cprman clock manager Luc Michel
                   ` (16 preceding siblings ...)
  2020-10-19 15:45 ` Peter Maydell
@ 2020-10-22 22:06 ` Philippe Mathieu-Daudé
  2020-10-22 22:48   ` Guenter Roeck
  2020-10-23  3:55   ` Guenter Roeck
  17 siblings, 2 replies; 35+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-10-22 22:06 UTC (permalink / raw)
  To: Luc Michel, qemu-devel, Guenter Roeck
  Cc: Peter Maydell, Havard Skinnemoen, Andrew Baumann, Paul Zimmerman,
	Niek Linnenbank, qemu-arm

Cc'ing Guenter who had a similar patch and might be interested
to test :)

patch 16/15 fixup:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg752113.html

On 10/10/20 3:57 PM, Luc Michel wrote:
> v2 -> v3:
>    - patch 03: moved clock_new definition to hw/core/clock.c [Phil]
>    - patch 03: commit message typo [Clement]
>    - patch 10: clarifications around the CM_CTL/CM_DIBV mux registers.
>                reg_cm replaced with reg_ctl and reg_div. Add some
>                comments for clarity. [Phil]
>    - patch 10: fixed update_mux_from_cm not matching the CM_DIV offset
>                correctly. [Phil]
>    - patch 11: replaced manual bitfield extraction with extract32 [Phil]
>    - patch 11: added a visual representation of CM_DIV for clarity [Phil]
>    - patch 11: added a missing return in clock_mux_update.
> 
> v1 -> v2:
>    - patch 05: Added a comment about MMIO .valid constraints [Phil]
>    - patch 05: Added MMIO .impl [Phil]
>    - patch 05: Moved init_internal_clock to the public clock API, renamed
>      clock_new (new patch 03) [Phil]
>    - patch 11: use muldiv64 for clock mux frequency output computation [Phil]
>    - patch 11: add a check for null divisor (Phil: I dropped your r-b)
>    - Typos, formatting, naming, style [Phil]
> 
> Patches without review: 03, 11, 13
> 
> Hi,
> 
> This series add the BCM2835 CPRMAN clock manager peripheral to the
> Raspberry Pi machine.
> 
> Patches 1-4 are preliminary changes, patches 5-13 are the actual
> implementation.
> 
> The two last patches add a clock input to the PL011 and
> connect it to the CPRMAN.
> 
> This series has been tested with Linux 5.4.61 (the current raspios
> version). It fixes the kernel Oops at boot time due to invalid UART
> clock value, and other warnings/errors here and there because of bad
> clocks or lack of CPRMAN.
> 
> Here is the clock tree as seen by Linux when booted in QEMU:
> (/sys/kernel/debug/clk/clk_summary with some columns removed)
> 
>                          enable  prepare
>     clock                 count    count          rate
> -----------------------------------------------------
>   otg                         0        0     480000000
>   osc                         5        5      19200000
>      gp2                      1        1         32768
>      tsens                    0        0       1920000
>      otp                      0        0       4800000
>      timer                    0        0       1000002
>      pllh                     4        4     864000000
>         pllh_pix_prediv       1        1       3375000
>            pllh_pix           0        0        337500
>         pllh_aux              1        1     216000000
>            vec                0        0     108000000
>         pllh_rcal_prediv      1        1       3375000
>            pllh_rcal          0        0        337500
>      plld                     3        3    2000000024
>         plld_dsi1             0        0       7812501
>         plld_dsi0             0        0       7812501
>         plld_per              3        3     500000006
>            gp1                1        1      25000000
>            uart               1        2      47999625
>         plld_core             2        2     500000006
>            sdram              0        0     166666668
>      pllc                     3        3    2400000000
>         pllc_per              1        1    1200000000
>            emmc               0        0     200000000
>         pllc_core2            0        0       9375000
>         pllc_core1            0        0       9375000
>         pllc_core0            2        2    1200000000
>            vpu                1        1     700000000
>               aux_spi2        0        0     700000000
>               aux_spi1        0        0     700000000
>               aux_uart        0        0     700000000
>               peri_image      0        0     700000000
>      plla                     2        2    2250000000
>         plla_ccp2             0        0       8789063
>         plla_dsi0             0        0       8789063
>         plla_core             1        1     750000000
>            h264               0        0     250000000
>            isp                0        0     250000000
>   dsi1p                       0        0             0
>   dsi0p                       0        0             0
>   dsi1e                       0        0             0
>   dsi0e                       0        0             0
>   cam1                        0        0             0
>   cam0                        0        0             0
>   dpi                         0        0             0
>   tec                         0        0             0
>   smi                         0        0             0
>   slim                        0        0             0
>   gp0                         0        0             0
>   dft                         0        0             0
>   aveo                        0        0             0
>   pcm                         0        0             0
>   pwm                         0        0             0
>   hsm                         0        0             0
> 
> It shows small differences with real hardware due other missing
> peripherals for which the driver turn the clock off (like tsens).
> 
> Luc Michel (15):
>    hw/core/clock: provide the VMSTATE_ARRAY_CLOCK macro
>    hw/core/clock: trace clock values in Hz instead of ns
>    hw/core/clock: add the clock_new helper function
>    hw/arm/raspi: fix CPRMAN base address
>    hw/arm/raspi: add a skeleton implementation of the CPRMAN
>    hw/misc/bcm2835_cprman: add a PLL skeleton implementation
>    hw/misc/bcm2835_cprman: implement PLLs behaviour
>    hw/misc/bcm2835_cprman: add a PLL channel skeleton implementation
>    hw/misc/bcm2835_cprman: implement PLL channels behaviour
>    hw/misc/bcm2835_cprman: add a clock mux skeleton implementation
>    hw/misc/bcm2835_cprman: implement clock mux behaviour
>    hw/misc/bcm2835_cprman: add the DSI0HSCK multiplexer
>    hw/misc/bcm2835_cprman: add sane reset values to the registers
>    hw/char/pl011: add a clock input
>    hw/arm/bcm2835_peripherals: connect the UART clock
> 
>   include/hw/arm/bcm2835_peripherals.h       |    5 +-
>   include/hw/arm/raspi_platform.h            |    5 +-
>   include/hw/char/pl011.h                    |    1 +
>   include/hw/clock.h                         |   18 +
>   include/hw/misc/bcm2835_cprman.h           |  210 ++++
>   include/hw/misc/bcm2835_cprman_internals.h | 1019 ++++++++++++++++++++
>   hw/arm/bcm2835_peripherals.c               |   15 +-
>   hw/char/pl011.c                            |   45 +
>   hw/core/clock.c                            |   21 +-
>   hw/misc/bcm2835_cprman.c                   |  808 ++++++++++++++++
>   hw/char/trace-events                       |    1 +
>   hw/core/trace-events                       |    4 +-
>   hw/misc/meson.build                        |    1 +
>   hw/misc/trace-events                       |    5 +
>   14 files changed, 2146 insertions(+), 12 deletions(-)
>   create mode 100644 include/hw/misc/bcm2835_cprman.h
>   create mode 100644 include/hw/misc/bcm2835_cprman_internals.h
>   create mode 100644 hw/misc/bcm2835_cprman.c
> 


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 00/15] raspi: add the bcm2835 cprman clock manager
  2020-10-22 22:06 ` Philippe Mathieu-Daudé
@ 2020-10-22 22:48   ` Guenter Roeck
  2020-10-23  3:55   ` Guenter Roeck
  1 sibling, 0 replies; 35+ messages in thread
From: Guenter Roeck @ 2020-10-22 22:48 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, Luc Michel, qemu-devel
  Cc: Peter Maydell, Havard Skinnemoen, Andrew Baumann, Paul Zimmerman,
	Niek Linnenbank, qemu-arm

Hi,

On 10/22/20 3:06 PM, Philippe Mathieu-Daudé wrote:
> Cc'ing Guenter who had a similar patch and might be interested
> to test :)
> 

great. I think my patch doesn't work anymore since qemu 5.0 (at least not
for raspi3), and it was pretty hackish anyway. I'll give the series a try.

Guenter

> patch 16/15 fixup:
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg752113.html
> 
> On 10/10/20 3:57 PM, Luc Michel wrote:
>> v2 -> v3:
>>    - patch 03: moved clock_new definition to hw/core/clock.c [Phil]
>>    - patch 03: commit message typo [Clement]
>>    - patch 10: clarifications around the CM_CTL/CM_DIBV mux registers.
>>                reg_cm replaced with reg_ctl and reg_div. Add some
>>                comments for clarity. [Phil]
>>    - patch 10: fixed update_mux_from_cm not matching the CM_DIV offset
>>                correctly. [Phil]
>>    - patch 11: replaced manual bitfield extraction with extract32 [Phil]
>>    - patch 11: added a visual representation of CM_DIV for clarity [Phil]
>>    - patch 11: added a missing return in clock_mux_update.
>>
>> v1 -> v2:
>>    - patch 05: Added a comment about MMIO .valid constraints [Phil]
>>    - patch 05: Added MMIO .impl [Phil]
>>    - patch 05: Moved init_internal_clock to the public clock API, renamed
>>      clock_new (new patch 03) [Phil]
>>    - patch 11: use muldiv64 for clock mux frequency output computation [Phil]
>>    - patch 11: add a check for null divisor (Phil: I dropped your r-b)
>>    - Typos, formatting, naming, style [Phil]
>>
>> Patches without review: 03, 11, 13
>>
>> Hi,
>>
>> This series add the BCM2835 CPRMAN clock manager peripheral to the
>> Raspberry Pi machine.
>>
>> Patches 1-4 are preliminary changes, patches 5-13 are the actual
>> implementation.
>>
>> The two last patches add a clock input to the PL011 and
>> connect it to the CPRMAN.
>>
>> This series has been tested with Linux 5.4.61 (the current raspios
>> version). It fixes the kernel Oops at boot time due to invalid UART
>> clock value, and other warnings/errors here and there because of bad
>> clocks or lack of CPRMAN.
>>
>> Here is the clock tree as seen by Linux when booted in QEMU:
>> (/sys/kernel/debug/clk/clk_summary with some columns removed)
>>
>>                          enable  prepare
>>     clock                 count    count          rate
>> -----------------------------------------------------
>>   otg                         0        0     480000000
>>   osc                         5        5      19200000
>>      gp2                      1        1         32768
>>      tsens                    0        0       1920000
>>      otp                      0        0       4800000
>>      timer                    0        0       1000002
>>      pllh                     4        4     864000000
>>         pllh_pix_prediv       1        1       3375000
>>            pllh_pix           0        0        337500
>>         pllh_aux              1        1     216000000
>>            vec                0        0     108000000
>>         pllh_rcal_prediv      1        1       3375000
>>            pllh_rcal          0        0        337500
>>      plld                     3        3    2000000024
>>         plld_dsi1             0        0       7812501
>>         plld_dsi0             0        0       7812501
>>         plld_per              3        3     500000006
>>            gp1                1        1      25000000
>>            uart               1        2      47999625
>>         plld_core             2        2     500000006
>>            sdram              0        0     166666668
>>      pllc                     3        3    2400000000
>>         pllc_per              1        1    1200000000
>>            emmc               0        0     200000000
>>         pllc_core2            0        0       9375000
>>         pllc_core1            0        0       9375000
>>         pllc_core0            2        2    1200000000
>>            vpu                1        1     700000000
>>               aux_spi2        0        0     700000000
>>               aux_spi1        0        0     700000000
>>               aux_uart        0        0     700000000
>>               peri_image      0        0     700000000
>>      plla                     2        2    2250000000
>>         plla_ccp2             0        0       8789063
>>         plla_dsi0             0        0       8789063
>>         plla_core             1        1     750000000
>>            h264               0        0     250000000
>>            isp                0        0     250000000
>>   dsi1p                       0        0             0
>>   dsi0p                       0        0             0
>>   dsi1e                       0        0             0
>>   dsi0e                       0        0             0
>>   cam1                        0        0             0
>>   cam0                        0        0             0
>>   dpi                         0        0             0
>>   tec                         0        0             0
>>   smi                         0        0             0
>>   slim                        0        0             0
>>   gp0                         0        0             0
>>   dft                         0        0             0
>>   aveo                        0        0             0
>>   pcm                         0        0             0
>>   pwm                         0        0             0
>>   hsm                         0        0             0
>>
>> It shows small differences with real hardware due other missing
>> peripherals for which the driver turn the clock off (like tsens).
>>
>> Luc Michel (15):
>>    hw/core/clock: provide the VMSTATE_ARRAY_CLOCK macro
>>    hw/core/clock: trace clock values in Hz instead of ns
>>    hw/core/clock: add the clock_new helper function
>>    hw/arm/raspi: fix CPRMAN base address
>>    hw/arm/raspi: add a skeleton implementation of the CPRMAN
>>    hw/misc/bcm2835_cprman: add a PLL skeleton implementation
>>    hw/misc/bcm2835_cprman: implement PLLs behaviour
>>    hw/misc/bcm2835_cprman: add a PLL channel skeleton implementation
>>    hw/misc/bcm2835_cprman: implement PLL channels behaviour
>>    hw/misc/bcm2835_cprman: add a clock mux skeleton implementation
>>    hw/misc/bcm2835_cprman: implement clock mux behaviour
>>    hw/misc/bcm2835_cprman: add the DSI0HSCK multiplexer
>>    hw/misc/bcm2835_cprman: add sane reset values to the registers
>>    hw/char/pl011: add a clock input
>>    hw/arm/bcm2835_peripherals: connect the UART clock
>>
>>   include/hw/arm/bcm2835_peripherals.h       |    5 +-
>>   include/hw/arm/raspi_platform.h            |    5 +-
>>   include/hw/char/pl011.h                    |    1 +
>>   include/hw/clock.h                         |   18 +
>>   include/hw/misc/bcm2835_cprman.h           |  210 ++++
>>   include/hw/misc/bcm2835_cprman_internals.h | 1019 ++++++++++++++++++++
>>   hw/arm/bcm2835_peripherals.c               |   15 +-
>>   hw/char/pl011.c                            |   45 +
>>   hw/core/clock.c                            |   21 +-
>>   hw/misc/bcm2835_cprman.c                   |  808 ++++++++++++++++
>>   hw/char/trace-events                       |    1 +
>>   hw/core/trace-events                       |    4 +-
>>   hw/misc/meson.build                        |    1 +
>>   hw/misc/trace-events                       |    5 +
>>   14 files changed, 2146 insertions(+), 12 deletions(-)
>>   create mode 100644 include/hw/misc/bcm2835_cprman.h
>>   create mode 100644 include/hw/misc/bcm2835_cprman_internals.h
>>   create mode 100644 hw/misc/bcm2835_cprman.c
>>



^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 00/15] raspi: add the bcm2835 cprman clock manager
  2020-10-22 22:06 ` Philippe Mathieu-Daudé
  2020-10-22 22:48   ` Guenter Roeck
@ 2020-10-23  3:55   ` Guenter Roeck
  2020-10-23 11:13     ` Philippe Mathieu-Daudé
  1 sibling, 1 reply; 35+ messages in thread
From: Guenter Roeck @ 2020-10-23  3:55 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, Luc Michel, qemu-devel
  Cc: Peter Maydell, Havard Skinnemoen, Andrew Baumann, Paul Zimmerman,
	Niek Linnenbank, qemu-arm

On 10/22/20 3:06 PM, Philippe Mathieu-Daudé wrote:
> Cc'ing Guenter who had a similar patch and might be interested
> to test :)
> 
I applied the series on top of qemu mainline and ran all my test with it
(raspi2 with qemu-system-arm as well as qemu-system-aarch64, and raspi3
in both big endian and little endian mode with qemu-system-aarch64.
All tests passed without error or kernel warning.

For the series:

Tested-by: Guenter Roeck <linux@roeck-us.net>

Guenter

> patch 16/15 fixup:
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg752113.html
> 
> On 10/10/20 3:57 PM, Luc Michel wrote:
>> v2 -> v3:
>>    - patch 03: moved clock_new definition to hw/core/clock.c [Phil]
>>    - patch 03: commit message typo [Clement]
>>    - patch 10: clarifications around the CM_CTL/CM_DIBV mux registers.
>>                reg_cm replaced with reg_ctl and reg_div. Add some
>>                comments for clarity. [Phil]
>>    - patch 10: fixed update_mux_from_cm not matching the CM_DIV offset
>>                correctly. [Phil]
>>    - patch 11: replaced manual bitfield extraction with extract32 [Phil]
>>    - patch 11: added a visual representation of CM_DIV for clarity [Phil]
>>    - patch 11: added a missing return in clock_mux_update.
>>
>> v1 -> v2:
>>    - patch 05: Added a comment about MMIO .valid constraints [Phil]
>>    - patch 05: Added MMIO .impl [Phil]
>>    - patch 05: Moved init_internal_clock to the public clock API, renamed
>>      clock_new (new patch 03) [Phil]
>>    - patch 11: use muldiv64 for clock mux frequency output computation [Phil]
>>    - patch 11: add a check for null divisor (Phil: I dropped your r-b)
>>    - Typos, formatting, naming, style [Phil]
>>
>> Patches without review: 03, 11, 13
>>
>> Hi,
>>
>> This series add the BCM2835 CPRMAN clock manager peripheral to the
>> Raspberry Pi machine.
>>
>> Patches 1-4 are preliminary changes, patches 5-13 are the actual
>> implementation.
>>
>> The two last patches add a clock input to the PL011 and
>> connect it to the CPRMAN.
>>
>> This series has been tested with Linux 5.4.61 (the current raspios
>> version). It fixes the kernel Oops at boot time due to invalid UART
>> clock value, and other warnings/errors here and there because of bad
>> clocks or lack of CPRMAN.
>>
>> Here is the clock tree as seen by Linux when booted in QEMU:
>> (/sys/kernel/debug/clk/clk_summary with some columns removed)
>>
>>                          enable  prepare
>>     clock                 count    count          rate
>> -----------------------------------------------------
>>   otg                         0        0     480000000
>>   osc                         5        5      19200000
>>      gp2                      1        1         32768
>>      tsens                    0        0       1920000
>>      otp                      0        0       4800000
>>      timer                    0        0       1000002
>>      pllh                     4        4     864000000
>>         pllh_pix_prediv       1        1       3375000
>>            pllh_pix           0        0        337500
>>         pllh_aux              1        1     216000000
>>            vec                0        0     108000000
>>         pllh_rcal_prediv      1        1       3375000
>>            pllh_rcal          0        0        337500
>>      plld                     3        3    2000000024
>>         plld_dsi1             0        0       7812501
>>         plld_dsi0             0        0       7812501
>>         plld_per              3        3     500000006
>>            gp1                1        1      25000000
>>            uart               1        2      47999625
>>         plld_core             2        2     500000006
>>            sdram              0        0     166666668
>>      pllc                     3        3    2400000000
>>         pllc_per              1        1    1200000000
>>            emmc               0        0     200000000
>>         pllc_core2            0        0       9375000
>>         pllc_core1            0        0       9375000
>>         pllc_core0            2        2    1200000000
>>            vpu                1        1     700000000
>>               aux_spi2        0        0     700000000
>>               aux_spi1        0        0     700000000
>>               aux_uart        0        0     700000000
>>               peri_image      0        0     700000000
>>      plla                     2        2    2250000000
>>         plla_ccp2             0        0       8789063
>>         plla_dsi0             0        0       8789063
>>         plla_core             1        1     750000000
>>            h264               0        0     250000000
>>            isp                0        0     250000000
>>   dsi1p                       0        0             0
>>   dsi0p                       0        0             0
>>   dsi1e                       0        0             0
>>   dsi0e                       0        0             0
>>   cam1                        0        0             0
>>   cam0                        0        0             0
>>   dpi                         0        0             0
>>   tec                         0        0             0
>>   smi                         0        0             0
>>   slim                        0        0             0
>>   gp0                         0        0             0
>>   dft                         0        0             0
>>   aveo                        0        0             0
>>   pcm                         0        0             0
>>   pwm                         0        0             0
>>   hsm                         0        0             0
>>
>> It shows small differences with real hardware due other missing
>> peripherals for which the driver turn the clock off (like tsens).
>>
>> Luc Michel (15):
>>    hw/core/clock: provide the VMSTATE_ARRAY_CLOCK macro
>>    hw/core/clock: trace clock values in Hz instead of ns
>>    hw/core/clock: add the clock_new helper function
>>    hw/arm/raspi: fix CPRMAN base address
>>    hw/arm/raspi: add a skeleton implementation of the CPRMAN
>>    hw/misc/bcm2835_cprman: add a PLL skeleton implementation
>>    hw/misc/bcm2835_cprman: implement PLLs behaviour
>>    hw/misc/bcm2835_cprman: add a PLL channel skeleton implementation
>>    hw/misc/bcm2835_cprman: implement PLL channels behaviour
>>    hw/misc/bcm2835_cprman: add a clock mux skeleton implementation
>>    hw/misc/bcm2835_cprman: implement clock mux behaviour
>>    hw/misc/bcm2835_cprman: add the DSI0HSCK multiplexer
>>    hw/misc/bcm2835_cprman: add sane reset values to the registers
>>    hw/char/pl011: add a clock input
>>    hw/arm/bcm2835_peripherals: connect the UART clock
>>
>>   include/hw/arm/bcm2835_peripherals.h       |    5 +-
>>   include/hw/arm/raspi_platform.h            |    5 +-
>>   include/hw/char/pl011.h                    |    1 +
>>   include/hw/clock.h                         |   18 +
>>   include/hw/misc/bcm2835_cprman.h           |  210 ++++
>>   include/hw/misc/bcm2835_cprman_internals.h | 1019 ++++++++++++++++++++
>>   hw/arm/bcm2835_peripherals.c               |   15 +-
>>   hw/char/pl011.c                            |   45 +
>>   hw/core/clock.c                            |   21 +-
>>   hw/misc/bcm2835_cprman.c                   |  808 ++++++++++++++++
>>   hw/char/trace-events                       |    1 +
>>   hw/core/trace-events                       |    4 +-
>>   hw/misc/meson.build                        |    1 +
>>   hw/misc/trace-events                       |    5 +
>>   14 files changed, 2146 insertions(+), 12 deletions(-)
>>   create mode 100644 include/hw/misc/bcm2835_cprman.h
>>   create mode 100644 include/hw/misc/bcm2835_cprman_internals.h
>>   create mode 100644 hw/misc/bcm2835_cprman.c
>>



^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 00/15] raspi: add the bcm2835 cprman clock manager
  2020-10-23  3:55   ` Guenter Roeck
@ 2020-10-23 11:13     ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 35+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-10-23 11:13 UTC (permalink / raw)
  To: Guenter Roeck, Luc Michel, qemu-devel
  Cc: Peter Maydell, Havard Skinnemoen, Andrew Baumann, Paul Zimmerman,
	Niek Linnenbank, qemu-arm

On 10/23/20 5:55 AM, Guenter Roeck wrote:
> On 10/22/20 3:06 PM, Philippe Mathieu-Daudé wrote:
>> Cc'ing Guenter who had a similar patch and might be interested
>> to test :)
>>
> I applied the series on top of qemu mainline and ran all my test with it
> (raspi2 with qemu-system-arm as well as qemu-system-aarch64, and raspi3
> in both big endian and little endian mode with qemu-system-aarch64.
> All tests passed without error or kernel warning.

Awesome :) I'm glad we finally get this complex part implemented.

Regards,

Phil.

> 
> For the series:
> 
> Tested-by: Guenter Roeck <linux@roeck-us.net>
> 
> Guenter


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 00/15] raspi: add the bcm2835 cprman clock manager
  2020-10-19 19:31   ` Peter Maydell
@ 2020-10-27  8:55     ` Philippe Mathieu-Daudé
  2020-10-27 11:11       ` Peter Maydell
  0 siblings, 1 reply; 35+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-10-27  8:55 UTC (permalink / raw)
  To: Peter Maydell, Luc Michel
  Cc: Havard Skinnemoen, Andrew Baumann, QEMU Developers,
	Niek Linnenbank, qemu-arm, Paul Zimmerman

Hi Peter,

On 10/19/20 9:31 PM, Peter Maydell wrote:
> On Mon, 19 Oct 2020 at 16:45, Peter Maydell <peter.maydell@linaro.org> wrote:
>>
>> On Sat, 10 Oct 2020 at 14:57, Luc Michel <luc@lmichel.fr> wrote:
>>>
>>> v2 -> v3:
>>>   - patch 03: moved clock_new definition to hw/core/clock.c [Phil]
>>>   - patch 03: commit message typo [Clement]
>>>   - patch 10: clarifications around the CM_CTL/CM_DIBV mux registers.
>>>               reg_cm replaced with reg_ctl and reg_div. Add some
>>>               comments for clarity. [Phil]
>>>   - patch 10: fixed update_mux_from_cm not matching the CM_DIV offset
>>>               correctly. [Phil]
>>>   - patch 11: replaced manual bitfield extraction with extract32 [Phil]
>>>   - patch 11: added a visual representation of CM_DIV for clarity [Phil]
>>>   - patch 11: added a missing return in clock_mux_update.
>>
>>
>>
>> Applied to target-arm.next, thanks.
> 
> Dropped again due to segv in 'make check' when running with
> clang sanitizer, which I gather from irc that you're looking
> into.

The fix has been merged as commit a6e9b9123e7
("hw/core/qdev-clock: add a reference on aliased clocks")
and the series also got:
Tested-by: Guenter Roeck <linux@roeck-us.net>

Hopefully it will make it for 5.2 :)

Thanks,

Phil.


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 00/15] raspi: add the bcm2835 cprman clock manager
  2020-10-27  8:55     ` Philippe Mathieu-Daudé
@ 2020-10-27 11:11       ` Peter Maydell
  0 siblings, 0 replies; 35+ messages in thread
From: Peter Maydell @ 2020-10-27 11:11 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Luc Michel, QEMU Developers, Andrew Baumann, Paul Zimmerman,
	Niek Linnenbank, qemu-arm, Havard Skinnemoen

On Tue, 27 Oct 2020 at 08:55, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> Hi Peter,
>
> On 10/19/20 9:31 PM, Peter Maydell wrote:
> > On Mon, 19 Oct 2020 at 16:45, Peter Maydell <peter.maydell@linaro.org> wrote:
> >>
> >> On Sat, 10 Oct 2020 at 14:57, Luc Michel <luc@lmichel.fr> wrote:
> >>>
> >>> v2 -> v3:
> >>>   - patch 03: moved clock_new definition to hw/core/clock.c [Phil]
> >>>   - patch 03: commit message typo [Clement]
> >>>   - patch 10: clarifications around the CM_CTL/CM_DIBV mux registers.
> >>>               reg_cm replaced with reg_ctl and reg_div. Add some
> >>>               comments for clarity. [Phil]
> >>>   - patch 10: fixed update_mux_from_cm not matching the CM_DIV offset
> >>>               correctly. [Phil]
> >>>   - patch 11: replaced manual bitfield extraction with extract32 [Phil]
> >>>   - patch 11: added a visual representation of CM_DIV for clarity [Phil]
> >>>   - patch 11: added a missing return in clock_mux_update.
> >>
> >>
> >>
> >> Applied to target-arm.next, thanks.
> >
> > Dropped again due to segv in 'make check' when running with
> > clang sanitizer, which I gather from irc that you're looking
> > into.
>
> The fix has been merged as commit a6e9b9123e7
> ("hw/core/qdev-clock: add a reference on aliased clocks")
> and the series also got:
> Tested-by: Guenter Roeck <linux@roeck-us.net>
>
> Hopefully it will make it for 5.2 :)



Applied to target-arm.next, thanks.

-- PMM


^ permalink raw reply	[flat|nested] 35+ messages in thread

end of thread, other threads:[~2020-10-27 12:17 UTC | newest]

Thread overview: 35+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-10 13:57 [PATCH v3 00/15] raspi: add the bcm2835 cprman clock manager Luc Michel
2020-10-10 13:57 ` [PATCH v3 01/15] hw/core/clock: provide the VMSTATE_ARRAY_CLOCK macro Luc Michel
2020-10-10 13:57 ` [PATCH v3 02/15] hw/core/clock: trace clock values in Hz instead of ns Luc Michel
2020-10-10 15:48   ` Philippe Mathieu-Daudé
2020-10-10 13:57 ` [PATCH v3 03/15] hw/core/clock: add the clock_new helper function Luc Michel
2020-10-10 15:17   ` Philippe Mathieu-Daudé
2020-10-10 15:44     ` Philippe Mathieu-Daudé
2020-10-10 13:57 ` [PATCH v3 04/15] hw/arm/raspi: fix CPRMAN base address Luc Michel
2020-10-10 13:57 ` [PATCH v3 05/15] hw/arm/raspi: add a skeleton implementation of the CPRMAN Luc Michel
2020-10-10 13:57 ` [PATCH v3 06/15] hw/misc/bcm2835_cprman: add a PLL skeleton implementation Luc Michel
2020-10-10 13:57 ` [PATCH v3 07/15] hw/misc/bcm2835_cprman: implement PLLs behaviour Luc Michel
2020-10-10 13:57 ` [PATCH v3 08/15] hw/misc/bcm2835_cprman: add a PLL channel skeleton implementation Luc Michel
2020-10-10 16:05   ` Philippe Mathieu-Daudé
2020-10-17 10:00     ` Philippe Mathieu-Daudé
2020-10-10 13:57 ` [PATCH v3 09/15] hw/misc/bcm2835_cprman: implement PLL channels behaviour Luc Michel
2020-10-10 13:57 ` [PATCH v3 10/15] hw/misc/bcm2835_cprman: add a clock mux skeleton implementation Luc Michel
2020-10-10 13:57 ` [PATCH v3 11/15] hw/misc/bcm2835_cprman: implement clock mux behaviour Luc Michel
2020-10-10 15:52   ` Philippe Mathieu-Daudé
2020-10-10 13:57 ` [PATCH v3 12/15] hw/misc/bcm2835_cprman: add the DSI0HSCK multiplexer Luc Michel
2020-10-10 13:57 ` [PATCH v3 13/15] hw/misc/bcm2835_cprman: add sane reset values to the registers Luc Michel
2020-10-10 16:18   ` Philippe Mathieu-Daudé
2020-10-11 18:26     ` Luc Michel
2020-10-16 17:10       ` Philippe Mathieu-Daudé
2020-10-19 15:44         ` Philippe Mathieu-Daudé
2020-10-10 13:57 ` [PATCH v3 14/15] hw/char/pl011: add a clock input Luc Michel
2020-10-10 13:57 ` [PATCH v3 15/15] hw/arm/bcm2835_peripherals: connect the UART clock Luc Michel
2020-10-16 17:11 ` [PATCH v3 00/15] raspi: add the bcm2835 cprman clock manager Philippe Mathieu-Daudé
2020-10-19 15:45 ` Peter Maydell
2020-10-19 19:31   ` Peter Maydell
2020-10-27  8:55     ` Philippe Mathieu-Daudé
2020-10-27 11:11       ` Peter Maydell
2020-10-22 22:06 ` Philippe Mathieu-Daudé
2020-10-22 22:48   ` Guenter Roeck
2020-10-23  3:55   ` Guenter Roeck
2020-10-23 11:13     ` Philippe Mathieu-Daudé

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).