On 3/6/23 20:12, Mark Cave-Ayland wrote:
On 03/06/2023 19: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é <phi...@linaro.org>
---
hw/timer/arm_timer.c | 109 +++++++++++++++++++++++++++----------------
1 file changed, 70 insertions(+), 39 deletions(-)
diff --git a/hw/timer/arm_timer.c b/hw/timer/arm_timer.c
index 82123b40c0..a929fbba62 100644
--- a/hw/timer/arm_timer.c
+++ b/hw/timer/arm_timer.c
@@ -17,6 +17,7 @@
#include "qemu/module.h"
#include "qemu/log.h"
#include "qom/object.h"
+#include "qapi/error.h"
/* Common timer implementation. */
@@ -29,14 +30,18 @@
#define TIMER_CTRL_PERIODIC (1 << 6)
#define TIMER_CTRL_ENABLE (1 << 7)
-typedef struct {
+#define TYPE_ARM_TIMER "arm-timer"
+OBJECT_DECLARE_SIMPLE_TYPE(ArmTimerState, ARM_TIMER)
As per our QOM guidelines ArmTimerState and the OBJECT_* macro should
live in a separate header file.
Ah wait: does "ArmTimerState is directly embedded into
SP804State/IcpPitState, and is initialized as a QOM child." mean that
ARM_TIMER is never instantiated externally?
Correct, while the type is exposed as any QOM type, it is internal to
the two devices, thus local to this unit.
I don't mind exposing the state to have a consistent QOM style.
What was discussed with Alex is:
- We don't need to convert all non-QOM devices, but
- Heterogeneous machines must contain only QOM devices;
- If a non-QOM device forces incorrect API use or abuses,
better convert it.
+struct ArmTimerState {
+ SysBusDevice parent_obj;
And don't forget to add a blank line here too.
OK.
ptimer_state *timer;
uint32_t control;
uint32_t limit;
uint32_t freq;
int int_level;
qemu_irq irq;
-} ArmTimerState;
+};
