Re: [PATCH 15/15] hw/timer/arm_timer: QOM'ify ARM_TIMER

[Philippe Mathieu-Daudé]

On 3/6/23 20:07, Mark Cave-Ayland wrote:
> On 31/05/2023 21:35, Philippe Mathieu-Daudé wrote:
>
> > Introduce the ARM_TIMER sysbus device.
> >
> > arm_timer_new() is converted as QOM instance init()/finalize()
> > handlers. Note in arm_timer_finalize() we release a ptimer handle
> > which was previously leaked.
> >
> > ArmTimerState is directly embedded into SP804State/IcpPitState,
> > and is initialized as a QOM child.
> >
> > Since the timer frequency belongs to ARM_TIMER, have it hold the
> > QOM property. SP804State/IcpPitState directly access it.
> >
> > Similarly the SP804State/IcpPitState input IRQ becomes the
> > ARM_TIMER sysbus output IRQ.
> >
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > ---
> >   hw/timer/arm_timer.c | 109 +++++++++++++++++++++++++++----------------
> >   1 file changed, 70 insertions(+), 39 deletions(-)


> > -static void arm_timer_reset(ArmTimerState *s)
> > +static void arm_timer_reset(DeviceState *dev)
> >   {
> > +    ArmTimerState *s = ARM_TIMER(dev);
> > +
> >       s->control = TIMER_CTRL_IE;
> >   }
>
> If you're currently set up to test the ARM timers with these changes, is 
> it worth considering converting this to use the Resettable interface at 
> the same time?

Good point. Then ARM_TIMER doesn't need to inherit from SysBus: if the
parent device resets it explicitly, it can be a plan QDev.

Peter, what do you think?

Even generically, I wonder if a QDev could resets all its QOM children,
propagating each ResetType.