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

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

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.