Re: [Qemu-devel] [PATCH v5 0/9] Clock framework API.

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

Hi Damien,

On 02/10/2018 16:24, Damien Hedde wrote:
> This series aims to add a way to model clocks in qemu between devices.
> This 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.
> 
> This series is a reroll of the v4 sent recently without the reset feature as
> requested by Peter. I also took into account the reviews about migration and
> other suggestions.
> 
> This framework was originally discussed in 2017, here:
> https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg07218.html
> 
> 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 triggered 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.
> 
> 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 an integer representing the clock frequency.
> The input clock callback is called every time the clock frequency changes.
> The input side holds a cached value of the frequency (the output does not need
> one). This allows a device to fetch its input clock frequency at any time
> without caching it itself.
> 
> This internal state is added to handle migration in order not to update and
> propagate clocks during it (it adds cross-device and order-specific effects).
> Each device handles its input clock migration by adding the clock frequency in
> its own vmstate description.
> 
> Regarding the migration strategy, discussion started in the v4 patches.
> The problem is that we add some kind of wire between different devices and 
> we face propagation issue.
> The chosen solution allows migration compatibility from a platform version
> with no implemented clocks to a platform with clocks. A migrated device that
> have a new input clock is responsible to initialize its frequency during
> migration. Each input clock having its own state, such initialization will not
> have any side-effect on other input clock connected to the same output.
> Output clocks, having no state, are not set during migration: If a clock output
> frequency changes due to migration (eg: clock computation bug-fix), the effects
> will be delayed. Eventually the clock output will be updated after migration if
> some (software) event trigger a clock update, but it can not be guaranteed.
> 
> There is also the problem of initialization which is very much like the
> migration. Currently, in the zynq example, clocks outputs are initialized in
> the clock controller device_reset. According to Peter, outputs should not be
> set in device_reset, so we need a better way.
> 
> It is not simple, since we can have complicated cases with several propagation
> step.
> We can't initialize outputs (without propagating) during device init and uses
> inputs value in device_reset to complete the initialization.
> Consider the case where we have a device generating a frequency, another device
> doing a clock division, then a device using this clock.
> [DevClockGenerator] -> clk1 -> [DevClockDiv] -> clk2 -> [Dev]
> I don't see how we can handle such initialization without doing some
> propagation phase at some point.
> 
> 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.
> 
> 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.
> 
> 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. This would allow us, for example, to add a power input port to handle
> power gating or others ports without modifying the device state structure.
> 
> Any comments and suggestion are welcomed.

How would you instanciate devices and connect their clocks from the
command line (with the -device option)?

Should clocked devices have DeviceClass::user_creatable = false by default?

Thanks,

Phil.