On Sun, Mar 6, 2011 at 6:31 PM, Anthony Liguori <anth...@codemonkey.ws> wrote: > On 02/13/2011 03:10 PM, Blue Swirl wrote: >> >> Convert to qdev. Don't expose PITState. >> >> Signed-off-by: Blue Swirl<blauwir...@gmail.com> >> --- >> hw/i8254.c | 61 >> +++++++++++++++++++++++++++++++++++++-------------- >> hw/mips_fulong2e.c | 4 +- >> hw/mips_jazz.c | 4 +- >> hw/mips_malta.c | 4 +- >> hw/mips_r4k.c | 4 +- >> hw/pc.c | 5 +-- >> hw/pc.h | 25 ++++++++++++++------ >> hw/pcspk.c | 4 +- >> hw/ppc_prep.c | 4 +- >> 9 files changed, 75 insertions(+), 40 deletions(-) >> >> diff --git a/hw/i8254.c b/hw/i8254.c >> index 06b225c..680caab 100644 >> --- a/hw/i8254.c >> +++ b/hw/i8254.c >> @@ -53,9 +53,12 @@ typedef struct PITChannelState { >> qemu_irq irq; >> } PITChannelState; >> >> -struct PITState { >> +typedef struct PITState { >> + ISADevice dev; >> > > The PIT is not an ISA device. Modelling it as such is worse than leaving it > unmodelled.
No. These days, PIT is part of Super I/O chip, which is accessed via LPC bus. LPC is from software point of view equal to ISA. Therefore, in absence of LPC, ISA is correct. >> -PITState *pit_init(int base, qemu_irq irq); >> -void pit_set_gate(PITState *pit, int channel, int val); >> -int pit_get_gate(PITState *pit, int channel); >> -int pit_get_initial_count(PITState *pit, int channel); >> -int pit_get_mode(PITState *pit, int channel); >> -int pit_get_out(PITState *pit, int channel, int64_t current_time); >> +void pit_set_gate(ISADevice *dev, int channel, int val); >> +int pit_get_gate(ISADevice *dev, int channel); >> +int pit_get_initial_count(ISADevice *dev, int channel); >> +int pit_get_mode(ISADevice *dev, int channel); >> +int pit_get_out(ISADevice *dev, int channel, int64_t current_time); >> > > Making these functions take an ISADevice hurts type safety. They should > take a PITState. These functions should be removed anyway. For example, pcspk probably should be merged with i825, or a signal like interface could be used.