On 24 May 2017 at 08:35, KONRAD Frederic <address@hidden> wrote:
Le 28/02/2017 à 11:02, address@hidden a écrit :
This is the third version of the clock framework API it contains:
* The first 6 patches which introduce the framework.
* The 7th patch which introduces a fixed-clock model.
* The rest which gives an example how to model a PLL from the existing
zynqmp-crf extracted from the qemu xilinx tree.
No specific behavior is expected yet when the CRF register set is accessed
but
the user can see for example the dp_video_ref and vpll_to_lpd rate
changing in
the monitor with the "info qtree" command when the vpll_ctrl register is
modified.
Some top-level review:
* I like the documentation, this is very helpful
* consider tracepoints rather than DPRINTF macro
* qemu_clk_device_add_clock() does a g_strdup, but when is this freed?
(consider devices which are hot-unpluggable)
* similarly, what if a device with a clock with a lot of child nodes
is destroyed? how are the ClkList structs freed?
* interaction with migration -- how is the "this clock is at this rate"
state intended to be migrated?
* I'll leave the review of the xilinx patches to the xilinx folk
* the 'introduce zynqmp_crf' patch is missing any signoffs
(in particular if it's from the xilinx tree it will need
signoff from them)
thanks
-- PMM