[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Qemu-devel] [PATCH v4 00/10] Clock framework API.

From: damien . hedde
Subject: [Qemu-devel] [PATCH v4 00/10] Clock framework API.
Date: Mon, 17 Sep 2018 10:40:06 +0200

From: Damien Hedde <address@hidden>

This series corresponds to the v4 of "clock framework api" patches
which were discussed in 2017, here:

It is a big refactoring trying to respond to comments and to integrate the
clock gating cases I sent recently here:
Note that, for now, the power cases are not considered in this patchset.

For the user, the framework is now very similar to the device's gpio API.
Clocks inputs and outputs can be added in devices during initialization phase.
Then an input can be connected to an output: it means every time the output
clock changes, a callback in the input is trigerred allowing any action to be
taken. A difference with gpios is that several inputs can be connected to a
single output without doing any glue.

Compared to v3, the following notable changes happen:
  - Bindings are now fixed during machine realisation,
  - Input clock objects have been removed from the point of view of the
user, i.e. nothing need to be added in the device state. A device can
now declare a input clock by calling qdev_init_clock_in() (similarly of
what is done for GPIOs),
  - Callbacks called on clock change now return void. The input owner is
responsible of making necessary updates accordingly (e.g. update another
clock output, or modify its internal state) (again, mostly like GPIOs).
  - For now, internal state have been removed. It could re-added as a
cached value to print it in info qtree, or to avoid unnecessary updates.

Behind the scene, there is 2 objects: a clock input which is a placeholder for
a callback, and a clock output which is a list of inputs. The value transferred
between an output and an input is a ClockState which contains 2 fields:
  - an integer to store the frequency
  - a boolean to indicate whether the clock domain reset is asserted or not
The reset flag has been added because it seems both signals are closely
related and very often controlled by the same device.

Regarding the migration strategy, clocks do not hold the clock state
internally, so there is nothing to migrate there. The consequence is that
a device must update its output clocks in its post_load to propagate the
migrated clock state. This allows migration from old-qemu-with-no-clock
to new-qemu-with-clock-support: newly added clocks will be correctly
initialized during migration.
But it is more complex for input clocks handling: there is no order
guarantee between a device state migration and the update of its inputs clocks
which will occur during other device state migrations.
I think that, for most the cases, this does not rise problems, although there
might be some jitter/glitch during migration before hitting the right value
(with consequences such as the baudrate of a character device changing several
times during migration, I don't think it is a problem but may well be wrong

For example, if we have 2 devices A and B with a clock between them:
| dev A  clk|>----->| dev B |
There is 2 migration scenarios (this can happen in our example implementation
between the slcr and cadence_uart devices):
  1. A before B:
     - A is migrated:
        + A's state is loaded
        + A's post_load restore the clock which is propagated to B
        + B reacts to the clock change (eg: update serial baudrate with
        migrated clock value but old B state)
     - B is migrated:
        + B's state is loaded
        + B's post_load reacts to new B's state (eg: update serial baudrate
        to the final right value)
  2. B before A:
     - B is migrated:
        + B's state is loaded
        + B's post_load reacts to ne B's state (eg: update serial baudrate with
        migrated B state but old clock value)
     - A is migrated:
        + A's state is loaded
        + A's post_load restore the clock which is propagated to B
        + B reacts to the clock change (eg: update serial baudrate to the final
        right value)

Regarding clock gating. The 0 frequency value means the clock is gated.
If need be, a gate device can be built taking an input gpio and clock and
generating an output clock.

We are considering switching to a generic payload evolution of this API.
For example by specifying the qom carried type when adding an input/output to
a device. The current implementation is poorly dependent on what the data type
is since only pointers are transferred between output and inputs. Changes would
probably be minor.
This would allow us, for example, to add a power input port to handle power
gating. We could also have the basic clock port (frequency-only) and the one we
have here (frequency-reset).

Concerning this frequency-reset port, we can obviously go back to the simple 
frequency-only one if you think it is not a good idea.

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 theorical 115200) which is nice. "cadence_uart*" and
"clock*" traces can be enabled to see what's going on in this platform.

Any comments and suggestion are welcomed.

The patches are organised as follows:
+ Patches 1 to 4 adds the clock support in qemu.
+ Patch 5 add some documentation in docs/devel
+ Patch 6 adds support for a default clock in sysbus devices which control
the mmios visibility.
+ Patches 7 to 10 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.

Thanks to the Xilinx QEMU team who sponsored this development.

Damien Hedde (10):
  hw/core/clock-port: introduce clock port objects
  qdev: add clock input&output support to devices.
  qdev-monitor: print the device's clock with info qtree
  qdev-clock: introduce an init array to ease the device construction
  docs/clocks: add device's clock documentation
  sysbus: add bus_interface_clock feature to sysbus devices
  hw/misc/zynq_slcr: use standard register definition
  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

 docs/devel/clock.txt           | 144 ++++++++
 Makefile.objs                  |   1 +
 include/hw/char/cadence_uart.h |   2 +
 include/hw/clock-port.h        | 153 ++++++++
 include/hw/qdev-clock.h        | 129 +++++++
 include/hw/qdev-core.h         |  14 +
 include/hw/qdev.h              |   1 +
 include/hw/sysbus.h            |  22 ++
 hw/arm/xilinx_zynq.c           |  17 +-
 hw/char/cadence_uart.c         |  73 +++-
 hw/core/clock-port.c           | 145 ++++++++
 hw/core/qdev-clock.c           | 166 +++++++++
 hw/core/qdev.c                 |  29 ++
 hw/core/sysbus.c               |  25 ++
 hw/misc/zynq_slcr.c            | 637 ++++++++++++++++++++-------------
 qdev-monitor.c                 |   6 +
 hw/char/trace-events           |   3 +
 hw/core/Makefile.objs          |   3 +-
 hw/core/trace-events           |   6 +
 19 files changed, 1324 insertions(+), 252 deletions(-)
 create mode 100644 docs/devel/clock.txt
 create mode 100644 include/hw/clock-port.h
 create mode 100644 include/hw/qdev-clock.h
 create mode 100644 hw/core/clock-port.c
 create mode 100644 hw/core/qdev-clock.c
 create mode 100644 hw/core/trace-events


reply via email to

[Prev in Thread] Current Thread [Next in Thread]