Hi Luc, On 4/10/21 3:19 PM, Luc Michel wrote: > On 08:23 Fri 09 Apr , Philippe Mathieu-Daudé wrote: >> 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.
They why the children of a clock tree fed with constant clock stay with a clock of 0? Who is responsible of setting their clock to the constant value? > 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. Ah... Maybe this is related to the generic reset problem in QEMU :( >> 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. Well, this is the point. I can't see a justification to have a clock on a non-qdev object. We should be able to model complicated devices with qdev. We are having various problems with the CPUs which are non-qdev devices, or recently even with the LED model...: https://www.mail-archive.com/qemu-devel@nongnu.org/msg798031.html Phil.
