Re: [Qemu-devel] [PATCH v1 5/8] xilinx_zynq: add USB controllers

[Gerd Hoffmann]

On 10/25/12 14:56, Peter Crosthwaite wrote:
> On Thu, Oct 25, 2012 at 10:16 PM, Peter Maydell
> <address@hidden> wrote:
>> On 25 October 2012 13:12, Gerd Hoffmann <address@hidden> wrote:
>>>> +static inline void zynq_init_usb(uint32_t base_addr, qemu_irq irq)
>>>> +{
>>>> +    DeviceState *dev = qdev_create(NULL, "ehci-sysbus");
>>>
>>> I'd suggest to have a "ehci-sysbus-zynq" device instead which sets
>>> capsbase & opregbase in ->init() ...
>>>
>>>> +    qdev_prop_set_uint16(dev, "capabase", 0x100);
>>>> +    qdev_prop_set_uint32(dev, "opregbase", 0x140);
>>>
>>> ... then drop these lines.
>>
>> That sounds weird to me -- properties are exactly the mechanism
>> for having a device which is configurable. Why have two differently
>> named devices which only differ in the value of a configurable
>> property?

> Yes I agree. Creating a now QOM definition for every variant of a
> device is tedious. EHCI provides a nice abstraction which should not
> have awareness of its particular implementations.

Maybe "zynq" is the wrong abstraction and this should be named by the
actual ehci chip implementation (which could be the same for a bunch of
sysbus boards).

But, yes, different chips should have different QOM definitions.  Like
we have a bunch of different uhci variants with a QOM definition for
each of them.

cheers,
  Gerd