Re: [PATCH v8 0/9] Clock framework API

["Edgar E. Iglesias" <edgar.iglesias@gmail.com>]

On Tue, Feb 25, 2020 at 02:14:13PM +0100, Damien Hedde wrote:
> This series aims to add a way to model clock distribution in qemu.
> The proposed objet and qdev API allows to model the clock tree of
> a platform allowing us to inspect clock configuration and detect
> problems such as disabled clock or bad configured pll.

On the series:
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>



> 
> This fast v8 fixes the documentation rst patchew errors and includes
> Alistair's review on v7.
> 
> There's 2 notable changes since v6:
> + The value stored in clock is now a period on 64 bits integer. The
>   unit is 2^-32ns to achieve the same precision as the ptimer internal
>   representation.
> + No more ClockIn/ClockOut base types, just a single Clock type. It
>   makes things simplier for everything else (standalone clocks,
>   possibility to read device's output clocks) and allow to chain
>   several clock inputs.
> 
> Regarding the internal represention. The precision is huge so that
> it is possible (in the future) to somehow connect a ptimer with a
> Clock with no loss of precision.
> The consequence is that we have a ~4seconds period upper bound only.
> Alternatives, allowing us to keep this precision, are to use a
> floating point or to extend the integer.
> 
> The added clock api is very similar the the GPIO API for devices. We
> can add input and output and connect them together.
> 
> Now that ressettable API is merged, the clock tree is properly
> initialized during the machine reset.
> I've tested this patchset running Xilinx's Linux on the xilinx-zynq-a9
> machine. Clocks are correctly updated and we ends up with a configured
> baudrate of 115601 on the console uart (for a theoretical 115200)
> which is nice. "cadence_uart*" and "clock*" traces can be enabled to
> see what's going on in this platform.
> 
> Any comments and suggestions are welcomed.
> 
> Patches 1, 3 and 5 to 8 still need some reviews.
> 
> The patches are organised as follows:
> + Patches 1 to 4 adds the clock support in qemu
> + Patch 5 adds some documentation in docs/devel
> + Patches 6 to 8 adds the uart's clocks to the xilinx_zynq platform
>   as an example for this framework. It updates the zynq's slcr clock
>   controller, the cadence_uart device, and the zynq toplevel platform.
> + Patch 9 adds clock info to monitor "info qtree" command
> 
> Changes since v7:
> https://lists.gnu.org/archive/html/qemu-devel/2020-02/msg06451.html
>  - rst errors in doc
>  - Alistair's comments on patch 1 and 3
> 
> Changes since v6:
> https://lists.gnu.org/archive/html/qemu-devel/2019-09/msg00599.html
>  - switch from frequency to period based clock state
>  - single Clock type (no more ClockIn and ClockOut)
>  - doc converted in rst format (Peter)
>  - various fixes (Peter and Philippe)
>  - better migration support for zynq devices (Peter)
> 
> Thanks to the Xilinx QEMU team who sponsored this development.
> 
> Damien Hedde (9):
>   hw/core/clock: introduce clock object
>   hw/core/clock-vmstate: define a vmstate entry for clock state
>   qdev: add clock input&output support to devices.
>   qdev-clock: introduce an init array to ease the device construction
>   docs/clocks: add device's clock documentation
>   hw/misc/zynq_slcr: add clock generation for uarts
>   hw/char/cadence_uart: add clock support
>   hw/arm/xilinx_zynq: connect uart clocks to slcr
>   qdev-monitor: print the device's clock with info qtree
> 
>  docs/devel/clocks.rst          | 360 +++++++++++++++++++++++++++++++++
>  docs/devel/index.rst           |   1 +
>  include/hw/char/cadence_uart.h |   1 +
>  include/hw/clock.h             | 225 +++++++++++++++++++++
>  include/hw/qdev-clock.h        | 159 +++++++++++++++
>  include/hw/qdev-core.h         |  12 ++
>  hw/arm/xilinx_zynq.c           |  57 +++++-
>  hw/char/cadence_uart.c         |  73 ++++++-
>  hw/core/clock-vmstate.c        |  25 +++
>  hw/core/clock.c                | 130 ++++++++++++
>  hw/core/qdev-clock.c           | 185 +++++++++++++++++
>  hw/core/qdev.c                 |  12 ++
>  hw/misc/zynq_slcr.c            | 172 +++++++++++++++-
>  qdev-monitor.c                 |   9 +
>  hw/char/trace-events           |   3 +
>  hw/core/Makefile.objs          |   2 +
>  hw/core/trace-events           |   7 +
>  tests/Makefile.include         |   1 +
>  18 files changed, 1412 insertions(+), 22 deletions(-)
>  create mode 100644 docs/devel/clocks.rst
>  create mode 100644 include/hw/clock.h
>  create mode 100644 include/hw/qdev-clock.h
>  create mode 100644 hw/core/clock-vmstate.c
>  create mode 100644 hw/core/clock.c
>  create mode 100644 hw/core/qdev-clock.c
> 
> -- 
> 2.25.1
>