Re: [PATCH v6 6/9] docs/clocks: add device's clock documentation

[Damien Hedde]

On 12/2/19 4:17 PM, Peter Maydell wrote:
> On Wed, 4 Sep 2019 at 13:56, Damien Hedde <address@hidden> wrote:
>>
>> Add the documentation about the clock inputs and outputs in devices.
>>
>> This is based on the original work of Frederic Konrad.
>>
>> Signed-off-by: Damien Hedde <address@hidden>
>> ---
>>  docs/devel/clock.txt | 246 +++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 246 insertions(+)
>>  create mode 100644 docs/devel/clock.txt
> 
> Could you convert this to rst format, please?Yes.
> 
> 
> 
>> +Changing a clock output
>> +=======================
>> +
>> +A device can change its outputs using the clock_set_frequency function. It
>> +will trigger updates on every connected inputs.
> 
> "input"
> 
>> +
>> +For example, let's say that we have an output clock "clkout" and we have a
>> +pointer to it in the device state because we did the following in init 
>> phase:
>> +dev->clkout = qdev_init_clock_out(DEVICE(dev), "clkout");
>> +
>> +Then at any time (apart from the cases listed below), it is possible to
>> +change the clock value by doing:
>> +clock_set_frequency(dev->clkout, 1000 * 1000 * 1000); /* 1Ghz */
>> +This operation must be done while holding the qemu io lock.
>> +
>> +One can change clocks only when it is allowed to have side effects on other
>> +objects. In consequence, it is forbidden:
>> ++ during migration,
>> ++ and in the init phase of reset.
>> +
>> +Forwarding clocks
>> +=================
>> +
>> +Sometimes, one needs to forward, or inherit, a clock from another device.
>> +Typically, when doing device composition, a device might expose a 
>> sub-device's
>> +clock without interfering with it.
>> +The function qdev_pass_clock() can be used to achieve this behaviour. Note, 
>> that
> 
> "Note that"
> 
>> +it is possible to expose the clock under a different name. This works for 
>> both
>> +inputs or outputs.
> 
> "inputs and outputs"
> 
> 
>> +Migration
>> +=========
>> +
>> +Only the ClockIn object has a state. ClockOut is not concerned by migration.
> 
> "has any state".
> 
> "ClockOut has no state and does not need special handling for migration."
> 
>> +
>> +In case the frequency of in input clock is needed for a device's migration,
>> +this state must be migrated.
> 
> Are you trying to say that if an input clock is known to be a
> fixed frequency we don't need to migrate anything? I wonder
> if we need to worry about that or if we could/should just say that
> input clocks should always be migrated.

What I wanted to say is that there are indeed probably cases where
migrating the frequency is unnecessary. For example if we only use the
callback and never fetch the frequency outside it: if the frequency is
only used to compute something which is already saved/loaded during
migration.

But yes we could just do as you say. It's probably less confusing.

> 
>> The VMSTATE_CLOCKIN macro defines an entry to
>> +be added in a vmstate description.
>> +
>> +For example, if a device has a clock input and the device state looks like:
>> +MyDeviceState {
>> +    DeviceState parent_obj;
>> +    ClockIn *clk;
>> +};
>> +
>> +Then, to add the clock frequency to the device's migrated state, the vmstate
>> +description is:
>> +VMStateDescription my_device_vmstate = {
>> +    .name = "my_device",
>> +    .fields = (VMStateField[]) {
>> +        VMSTATE_CLOCKIN(clk, MyDeviceState),
>> +        VMSTATE_END_OF_LIST()
>> +    }
>> +};
>> +
>> +When adding a input clock support to an existing device, you must care about
>> +migration compatibility. To this end, you can use the clock_init_frequency 
>> in
>> +a pre_load function to setup a default value in case the source vm does not
>> +migrate the frequency.
> 
> thanks
> -- PMM
>