RE: [PATCH v12 3/3] Versal: Connect DWC3 controller with virt-versal

[Sai Pavan Boddu]

Hi Peter

> -----Original Message-----
> From: Peter Maydell <peter.maydell@linaro.org>
> Sent: Monday, October 26, 2020 9:35 PM
> To: Sai Pavan Boddu <saipava@xilinx.com>
> Cc: Markus Armbruster <armbru@redhat.com>; Marc-André Lureau
> <marcandre.lureau@redhat.com>; Paolo Bonzini <pbonzini@redhat.com>;
> Gerd Hoffmann <kraxel@redhat.com>; Edgar Iglesias <edgari@xilinx.com>;
> Francisco Eduardo Iglesias <figlesia@xilinx.com>; QEMU Developers <qemu-
> devel@nongnu.org>; Alistair Francis <alistair.francis@wdc.com>; Eduardo
> Habkost <ehabkost@redhat.com>; Ying Fang <fangying1@huawei.com>;
> Philippe Mathieu-Daudé <philmd@redhat.com>; Vikram Garhwal
> <fnuv@xilinx.com>; Paul Zimmerman <pauldzim@gmail.com>; Sai Pavan Boddu
> <saipava@xilinx.com>
> Subject: Re: [PATCH v12 3/3] Versal: Connect DWC3 controller with virt-versal
> 
> On Thu, 22 Oct 2020 at 13:11, Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
> wrote:
> >
> > From: Vikram Garhwal <fnu.vikram@xilinx.com>
> >
> > Connect dwc3 controller and usb2-reg module to xlnx-versal SOC, its
> > placed in iou of lpd domain and configure it as dual port host
> > controller. Add the respective guest dts nodes for "xlnx-versal-virt" 
> > machine.
> >
> > Signed-off-by: Vikram Garhwal <fnu.vikram@xilinx.com>
> > Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
> 
> 
> > +static void versal_create_usbs(Versal *s, qemu_irq *pic) {
> > +    DeviceState *dev, *xhci_dev;
> > +    MemoryRegion *mr;
> > +
> > +    object_initialize_child(OBJECT(s), "dwc3-0", &s->lpd.iou.usb.dwc3,
> > +                            TYPE_USB_DWC3);
> > +    dev = DEVICE(&s->lpd.iou.usb.dwc3);
> > +    xhci_dev = DEVICE(&s->lpd.iou.usb.dwc3.sysbus_xhci);
> 
> If you find yourself fishing around in the internals of another device, 
> especially
> to this depth, then something's probably not right in the structure of that
> device.
[Sai Pavan Boddu] Yeah, it look pretty long. But, "s->lpd.iou.usb" defines soc 
domains for us.
Ex. Usb is part of low-power domain(i.e lpd) in IOU module.
And " dwc3.sysbus_xhci ", is specific to usb and is result of parenting dwc3 to 
xhci.

> 
> > +
> > +    object_property_set_link(OBJECT(xhci_dev), "dma", OBJECT(&s->mr_ps),
> > +                             &error_abort);
> > +    qdev_prop_set_uint32(xhci_dev, "intrs", 1);
> > +    qdev_prop_set_uint32(xhci_dev, "slots", 2);
> > +
> > +    sysbus_realize(SYS_BUS_DEVICE(dev), &error_fatal);
> > +
> > +    mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0);
> > +    memory_region_add_subregion(&s->mr_ps,
> MM_USB_XHCI_0_DWC3_GLOBAL, mr);
> > +    mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(xhci_dev), 0);
> > +    memory_region_add_subregion(&s->mr_ps, MM_USB_XHCI_0, mr);
> 
> For instance, rather than having to find the xhci device and map its memory
> regions and connect its IRQs directly, the
> usb-dwc3 device could provide and pass through those MRs and IRQs, so that
> board code is only wiring up what the usb-dwc3 provides and doesn't need to
> know about its internals.
[Sai Pavan Boddu] Yes, xhci registers can be mapped internally. We can make 
those changes.

Regards,
Sai Pavan
> 
> 
> thanks
> -- PMM