On Thu, Oct 25, 2012 at 10:16 PM, Peter Maydell <[email protected]> wrote: > On 25 October 2012 13:12, Gerd Hoffmann <[email protected]> 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. The way I have done it here also maps to how it is handled in the linux kernel as well. capabase and opregbase are left as parameters and EHCI implementations wrap around and set them as needed. hcd-ehci.c in the kernel has no awareness of zynq and I think the same hold for hcd-ehci.c in QEMU. > [I haven't looked at whether these specific properties make > sense or if there's some other higher-level-of-abstraction > knob that would be better to expose.) > Im interested to see what up with Tegra here. Might have a look through the kernel drivers to see what kinda diffs there are from one EHCI variant to the next tmrw. Regards, Peter > -- PMM >
