Re: [PATCH v3 00/15] raspi: add the bcm2835 cprman clock manager

[Guenter Roeck]

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