On Wed, 2013-10-30 at 17:28 +0100, arm...@redhat.com wrote: > From: Markus Armbruster <arm...@redhat.com> > > Drop it when there's no obvious reason why device_add could not work. > Else keep and document why. > > * isa-fdc: drop > > * i8042: drop, even though its I/O base is hardcoded (because you > could conceivably still add one to a board that has none), and even > though PC board code wires up the A20 line (because that wiring is > optional) > > * port92: keep because it needs additional wiring by port92_init() > > * mc146818rtc: keep because it needs to be wired up by rtc_init() > > * m48t59_isa: keep because needs to be wired up by m48t59_init_isa() > > * isa-pit, kvm-pit: keep (in their abstract base pic-common) because > the PIT needs additional wiring by board code, depending on HPET > presence > > * pcspk: keep because of pointer property pit, and because realize > sets global pcspk_state > > * vmmouse: keep because of pointer property ps2_mouse > > * vmport: keep because realize sets global port_state > > * isa-i8259, kvm-i8259: keep (in their abstract base pic-common), > because the PICs' IRQ input lines are set up by board code, and the > wiring of the slave to the master is hard-coded in device model code
I agree with the intend of this patch, but I am not familiar with this code, so I am not comfortable reviewing it, anyone else? Thanks, Marcel > > Signed-off-by: Markus Armbruster <arm...@redhat.com> > --- > hw/audio/pcspk.c | 3 ++- > hw/block/fdc.c | 1 - > hw/i386/pc.c | 7 ++++++- > hw/input/pckbd.c | 1 - > hw/input/vmmouse.c | 3 ++- > hw/intc/i8259_common.c | 8 +++++++- > hw/misc/vmport.c | 3 ++- > hw/timer/i8254_common.c | 7 ++++++- > hw/timer/m48t59.c | 3 ++- > hw/timer/mc146818rtc.c | 3 ++- > 10 files changed, 29 insertions(+), 10 deletions(-) > > diff --git a/hw/audio/pcspk.c b/hw/audio/pcspk.c > index 8e3e178..f980d66 100644 > --- a/hw/audio/pcspk.c > +++ b/hw/audio/pcspk.c > @@ -192,8 +192,9 @@ static void pcspk_class_initfn(ObjectClass *klass, void > *data) > > dc->realize = pcspk_realizefn; > set_bit(DEVICE_CATEGORY_SOUND, dc->categories); > - dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why > */ > dc->props = pcspk_properties; > + /* Reason: pointer property "pit", realize sets global pcspk_state */ > + dc->cannot_instantiate_with_device_add_yet = true; > } > > static const TypeInfo pcspk_info = { > diff --git a/hw/block/fdc.c b/hw/block/fdc.c > index 86f4920..592b58f 100644 > --- a/hw/block/fdc.c > +++ b/hw/block/fdc.c > @@ -2234,7 +2234,6 @@ static void isabus_fdc_class_init(ObjectClass *klass, > void *data) > > dc->realize = isabus_fdc_realize; > dc->fw_name = "fdc"; > - dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why > */ > dc->reset = fdctrl_external_reset_isa; > dc->vmsd = &vmstate_isa_fdc; > dc->props = isa_fdc_properties; > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index fe33843..20402ba 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -544,10 +544,15 @@ static void port92_class_initfn(ObjectClass *klass, > void *data) > { > DeviceClass *dc = DEVICE_CLASS(klass); > > - dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why > */ > dc->realize = port92_realizefn; > dc->reset = port92_reset; > dc->vmsd = &vmstate_port92_isa; > + /* > + * Reason: unlike ordinary ISA devices, this one needs additional > + * wiring: its A20 output line needs to be wired up by > + * port92_init(). > + */ > + dc->cannot_instantiate_with_device_add_yet = true; > } > > static const TypeInfo port92_info = { > diff --git a/hw/input/pckbd.c b/hw/input/pckbd.c > index dee31a6..655b8c5 100644 > --- a/hw/input/pckbd.c > +++ b/hw/input/pckbd.c > @@ -522,7 +522,6 @@ static void i8042_class_initfn(ObjectClass *klass, void > *data) > DeviceClass *dc = DEVICE_CLASS(klass); > > dc->realize = i8042_realizefn; > - dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why > */ > dc->vmsd = &vmstate_kbd_isa; > } > > diff --git a/hw/input/vmmouse.c b/hw/input/vmmouse.c > index 600e4a2..6a50533 100644 > --- a/hw/input/vmmouse.c > +++ b/hw/input/vmmouse.c > @@ -282,10 +282,11 @@ static void vmmouse_class_initfn(ObjectClass *klass, > void *data) > DeviceClass *dc = DEVICE_CLASS(klass); > > dc->realize = vmmouse_realizefn; > - dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why > */ > dc->reset = vmmouse_reset; > dc->vmsd = &vmstate_vmmouse; > dc->props = vmmouse_properties; > + /* Reason: pointer property "ps2_mouse" */ > + dc->cannot_instantiate_with_device_add_yet = true; > } > > static const TypeInfo vmmouse_info = { > diff --git a/hw/intc/i8259_common.c b/hw/intc/i8259_common.c > index 2acdbfe..9d29399 100644 > --- a/hw/intc/i8259_common.c > +++ b/hw/intc/i8259_common.c > @@ -135,9 +135,15 @@ static void pic_common_class_init(ObjectClass *klass, > void *data) > DeviceClass *dc = DEVICE_CLASS(klass); > > dc->vmsd = &vmstate_pic_common; > - dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why > */ > dc->props = pic_properties_common; > dc->realize = pic_common_realize; > + /* > + * Reason: unlike ordinary ISA devices, the PICs need additional > + * wiring: its IRQ input lines are set up by board code, and the > + * wiring of the slave to the master is hard-coded in device model > + * code. > + */ > + dc->cannot_instantiate_with_device_add_yet = true; > } > > static const TypeInfo pic_common_type = { > diff --git a/hw/misc/vmport.c b/hw/misc/vmport.c > index 94ae6ae..cd5716a 100644 > --- a/hw/misc/vmport.c > +++ b/hw/misc/vmport.c > @@ -162,7 +162,8 @@ static void vmport_class_initfn(ObjectClass *klass, void > *data) > DeviceClass *dc = DEVICE_CLASS(klass); > > dc->realize = vmport_realizefn; > - dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why > */ > + /* Reason: realize sets global port_state */ > + dc->cannot_instantiate_with_device_add_yet = true; > } > > static const TypeInfo vmport_info = { > diff --git a/hw/timer/i8254_common.c b/hw/timer/i8254_common.c > index dc2196c..9db5c9d 100644 > --- a/hw/timer/i8254_common.c > +++ b/hw/timer/i8254_common.c > @@ -282,7 +282,12 @@ static void pit_common_class_init(ObjectClass *klass, > void *data) > > dc->realize = pit_common_realize; > dc->vmsd = &vmstate_pit_common; > - dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why > */ > + /* > + * Reason: unlike ordinary ISA devices, the PIT may need to be > + * wired to the HPET, and because of that, some wiring is always > + * done by board code. > + */ > + dc->cannot_instantiate_with_device_add_yet = true; > } > > static const TypeInfo pit_common_type = { > diff --git a/hw/timer/m48t59.c b/hw/timer/m48t59.c > index f81cf48..8005503 100644 > --- a/hw/timer/m48t59.c > +++ b/hw/timer/m48t59.c > @@ -750,9 +750,10 @@ static void m48t59_isa_class_init(ObjectClass *klass, > void *data) > DeviceClass *dc = DEVICE_CLASS(klass); > > dc->realize = m48t59_isa_realize; > - dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why > */ > dc->reset = m48t59_reset_isa; > dc->props = m48t59_isa_properties; > + /* Reason: needs to be wired up by m48t59_init_isa() */ > + dc->cannot_instantiate_with_device_add_yet = true; > } > > static const TypeInfo m48t59_isa_info = { > diff --git a/hw/timer/mc146818rtc.c b/hw/timer/mc146818rtc.c > index 2f58220..17e1907 100644 > --- a/hw/timer/mc146818rtc.c > +++ b/hw/timer/mc146818rtc.c > @@ -906,9 +906,10 @@ static void rtc_class_initfn(ObjectClass *klass, void > *data) > DeviceClass *dc = DEVICE_CLASS(klass); > > dc->realize = rtc_realizefn; > - dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why > */ > dc->vmsd = &vmstate_rtc; > dc->props = mc146818rtc_properties; > + /* Reason: needs to be wired up by rtc_init() */ > + dc->cannot_instantiate_with_device_add_yet = true; > } > > static const TypeInfo mc146818rtc_info = {