Re: [Qemu-devel] [PATCH v1 0/8] Sysbus EHCI + Zynq USB.

[Gerd Hoffmann]

On 10/30/12 09:24, Peter Crosthwaite wrote:
> On Tue, Oct 30, 2012 at 5:20 PM, Gerd Hoffmann <address@hidden> wrote:
>> On 10/29/12 15:08, Peter Crosthwaite wrote:
>>> Ping!
>>>
>>> This is the first version of the EHCI sysbus series which takes a
>>> property based approach rather than the dynamic class approach.
>>>
>>> No refactoring of the PCI stuff is done here (introduced v2) but
>>> following on from the discussion on IRC about how to do and the
>>> suggestion we take a property based approach, could we get a review of
>>> this and mix and match between this and V3 for the solution?
>>
>> I like the EHCI{Pci,Sysbus}State approach in the v3 series, also the
>> sysbus restruction so sysbus_create_simple() can be used in v2+.
>>
>> I'd suggest to drop all the controversial stuff:
>>
>>  - v3 patch #1 (go with NULL like ohci does for the time being,
>>    when we finally figured+agreed on how to fix it we can change
>>    both ehci+ohci).
>>  - v3 patch #2 (class_data for pci).
>>
> 
> Hi Gerd,
> 
> Its difficult to drop this patch, as it defines struct EHCIPCIClass
> which is needed to hold the capabase and opregbase properties
> introduced later.

Just set them hard-coded in usb_ehci_pci_initfn (before calling into the
common usb_ehci_initfn function obviously), they are identical in all
pci variants.

The same works (for now) for sysbus, although that will probably change
in the future when we add more variants with other base values.

> If the only objection is the class_data then I can
> revert to the old code driven approach with separate class_init fns
> for each,

Yes, just keep the separate class_init fns (+drop EHCIPCIClass) for now.

For a quick merge I'd suggest to go with established practices here,
i.e. either just have the properties twice (once for pci, once for
sysfs) or use a #define like nic/block properties do.

Exploring ways to do this in a better way using QOM is fine too, but I
wouldn't attempt this for the initial merge.  QOM is at least partly
still in the design phase, thus causing lots of discussions when it
comes to hashing out the details as you've seen.

cheers,
  Gerd