Re: [RFC PATCH-for-6.1 0/9] hw/clock: Strengthen machine (non-qdev) clock propagation

[Luc Michel <luc@lmichel.fr>]

Hi Philippe,

On 08:23 Fri 09 Apr     , Philippe Mathieu-Daudé wrote:
> Hi Damian, Luc, Peter.
> 
> I've been debugging some odd issue with the clocks:
> a clock created in the machine (IOW, not a qdev clock) isn't
> always resetted, thus propagating its value.
> "not always" is the odd part. In the MPS2 board, the machine
> clock is propagated. Apparently because the peripherals are
> created directly in the machine_init() handler. When moving
> them out in a SoC QOM container, the clock isn't... I'm still
> having hard time to understand what is going on.

I think there is a misunderstanding on how the clock API works. If I
understand correctly your issue, you expect the callback of an input
clock connected to your constant "main oscillator" clock to be called on
machine reset.

If I'm not mistaken this is not the way the API has been designed. The
callback is called only when the clock period changes. A constant clock
does not change on reset, so the callback of child clocks should not be
called.

However devices that care about this clock value (e.g. a device that
has a clock input connected to this constant clock) should see their
standard reset callback called during reset. And they can effectively read
the clock value here and do what they need to do.

Note that clock propagation during reset has always been a complicated
problem. Calling clock_propagate is forbidden during the reset's enter
phase because of the side effects it can introduce.

> 
> Alternatively I tried to strengthen the clock API by reducing
> the clock creation in 2 cases: machine/device. This way clocks
> aren't left dangling around alone. The qdev clocks are properly
> resetted, and for the machine clocks I register a generic reset
> handler. This way is safer, but I don't think we want to keep
> adding generic reset handlers, instead we'd like to remove them.

I find your API modification a bit restrictive. I think creating a
standalone clock can be useful, e.g. in complicated devices that may
want to use internal "intermediate" clocks. I would not remove this
possibility to the API users.

Thanks,

-- 
Luc