Re: [Qemu-devel] [PATCH v2 05/11] usb/ehci: seperate out PCIisms

[Peter Crosthwaite]

On Fri, Oct 26, 2012 at 10:24 PM, Gerd Hoffmann <address@hidden> wrote:
>   Hi,
>
>> +typedef struct EHCItfState {
>> +    union {
>> +        PCIDevice pcidev;
>> +    };
>> +    struct EHCIState ehci;
>> +} EHCIItfState;
>
> I still think we should have EHCIPCIState here, then add a
> EHCISysbusState variant for sysbus.  Everybody else does it this way
> (ohci, esp, serial, ...) and I'd like ehci follow.

There still has to be a way to share the Property[] array (currently
contains maxframes). Duplicating the properties array to all
definitions is verbose and fragile. If I want to add a new properties
to EHCI i need to put it in the props array of every subclass. serial
has this problem, with the "chardev" prop appearing in both isa and
pci variants (and the device variant out of tree that Anthony has). If
we decide to add a new prop to serial we have to DEFINE_PROP_FOO it 3
times.

Whats the real answer here? Can we get the shared init function to add
to properties explicify? Blow away the dc->properties = foo and
replace with code that parses to prop array?

I just think its a false economy to have to replicate your code for
the sake of the anit-union movement,

Regards,
Peter

Also makes it easier
> to split the source code into core, pci and sysbus pieces, i.e. move all
> the pci bits to hcd-ehci-pci.c and compile only for targets with pci
> support.
>
> cheers,
>   Gerd
>
>