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

[Peter Crosthwaite]

On Sat, Oct 27, 2012 at 10:32 AM, Peter Crosthwaite
<address@hidden> wrote:
> 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.
>

In the interest of hopefully getting something merged before freeze I
had remade it this way (With EHCIPCIState). Can revisit this code
replication issue another day. v3 will be up shortly.

Regards,
Peter

> 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
>>
>>