Hi,
>> +static uint64_t
>> +ehci_faraday_read(void *ptr, hwaddr addr, unsigned size)
>> +{
>> + hwaddr off = 0x34 + addr;
>> +
>> + switch (off) {
>> + case 0x34: /* fusbh200: EOF/Async. Sleep Timer Register */
>> + return 0x00000041;
>> + case 0x40: /* fusbh200: Bus Monitor Control/Status Register */
>> + /* High-Speed, VBUS valid, interrupt level-high active */
>> + return (2 << 9) | (1 << 8) | (1 << 3);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static void
>> +ehci_faraday_write(void *ptr, hwaddr addr, uint64_t val, unsigned size)
>> +{
>> +}
>> +
>> +static const MemoryRegionOps ehci_mmio_faraday_ops = {
>> + .read = ehci_faraday_read,
>> + .write = ehci_faraday_write,
>> + .valid.min_access_size = 4,
>> + .valid.max_access_size = 4,
>> + .endianness = DEVICE_LITTLE_ENDIAN,
>> +};
This should go to hcd-ehci-sysbus.c
>> + memory_region_init_io(&s->mem_faraday, &ehci_mmio_faraday_ops, s,
>> + "faraday", 0x4c);
>
> I don't think this is good design... Can't you do the Faraday part from
> your own instance_init / initfn / realizefn function?
Yes, please. Just add your own usb_ehci_sysbus_${board}_initfn, call
usb_ehci_sysbus_initfn for the common stuff, then add your
hardware-specific memory region.
> The define -> class/state movements look fine to me as continuation of
> our previous effort. Optionally this patch could be split into two -
> preparing the fields and updating all define users, and then introduce
> your new model.
Agree, splitting would be nice: one patch making portbase + nports
runtime configurable, one patch adding the new ehci controller.
cheers,
Gerd
