[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [PATCH 2/2] i82378: cleanup implementation
From: |
Andreas Färber |
Subject: |
Re: [Qemu-ppc] [PATCH 2/2] i82378: cleanup implementation |
Date: |
Wed, 31 Jul 2013 19:06:24 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130620 Thunderbird/17.0.7 |
Am 30.07.2013 22:06, schrieb Hervé Poussineau:
> Andreas Färber a écrit :
>> Am 23.07.2013 23:16, schrieb Hervé Poussineau:
>>> }
>>>
>>> -static void i82378_init(DeviceState *dev, I82378State *s)
>>> +static void i82378_realize(DeviceState *dev, Error **errp)
>>> {
>>> - ISABus *isabus = ISA_BUS(qdev_get_child_bus(dev, "isa.0"));
>>> - ISADevice *pit;
>>> + PCIDevice *pci = PCI_DEVICE(dev);
>>> + I82378State *s = I82378(dev);
>>> + DeviceClass *dc;
>>> + uint8_t *pci_conf;
>>> + ISABus *isabus;
>>> ISADevice *isa;
>>> qemu_irq *out0_irq;
>>>
>>> + dc =
>>> DEVICE_CLASS(object_class_get_parent(object_get_class(OBJECT(dev))));
>>
>> This is going into uncharted territories. ;) I consider it wrong to use
>> object_get_class() - we should use object_class_by_name() to allow for
>> derived types and I'll put it into a macro that I'll try to align with
>> Peter C.'s and my QOM work.
>
> OK
While this gave me an inspiration for my virtio refactoring (it is
possible to convert device by device by calling the parent's
DeviceClass::realize as done here, as long as the *Class::init is called
conditionally through the DeviceClass::init implementation), I am
concerned about a single device deviating from initialization order here
(hw/pci/pci.c:pci_qdev_init() does things after calling PCIDevice::init,
namely ROM handling and bus hotplug) and will revert this to an
old-style qdev initfn for 1.6 bugfix.
[...]
>>> + dc->no_user = 1;
>>
>> Why do you do this? For one, according to Anthony it should no longer be
>> used, and for another, Paolo's endianness-test (make check) is using
>> -device i82378 for various other ppc and sh4 machines IIUC. make check
>> still succeeds for ppc with this patch though, so that might be due to
>> -device ignoring DeviceClass::no_user?
>
> I probably copied it from another chipset device, maybe i440fx.
> I don't really mind removing it.
Good.
> Yes, I double-checked that make check still works for all architectures.
>
>> Hoping to get this in shape for -rc1.
>
> Sure. Should I send a v2, as it seems you already queued it?
Since -rc1 is due tomorrow, I'll put together a pull myself tonight.
Andreas