Re: [PATCH v12 1/3] misc: Add versal-usb2-ctrl-regs module

[Peter Maydell]

On Thu, 22 Oct 2020 at 13:11, Sai Pavan Boddu
<sai.pavan.boddu@xilinx.com> wrote:
>
> This module emulates control registers of versal usb2 controller, this is 
> added
> just to make guest happy. In general this module would control the phy-reset
> signal from usb controller, data coherency of the transactions, signals
> the host system errors received from controller.
>
> Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
> Signed-off-by: Vikram Garhwal <fnu.vikram@xilinx.com>
> ---
>  hw/misc/meson.build                          |   1 +
>  hw/misc/xlnx-versal-usb2-ctrl-regs.c         | 229 
> +++++++++++++++++++++++++++
>  include/hw/misc/xlnx-versal-usb2-ctrl-regs.h |  45 ++++++

This seems a bit odd. If it's a USB device (or part of a USB
device) then it should be under hw/usb, shouldn't it?

> +static void ir_status_postw(RegisterInfo *reg, uint64_t val64)
> +{
> +    VersalUsb2CtrlRegs *s = XILINX_VERSAL_USB2_CTRL_REGS(reg->opaque);
> +    /*
> +     * TODO: This should also clear USBSTS.HSE field in USB XHCI register.
> +     * May be combine both the modules.
> +     */

What does the hardware for this look like? You've modelled
it as two completely separate devices (this one and the
TYPE_USB_DWC3) but would it be closer to the hardware
structure to have a top-level device which has-a DWC3 ?

thanks
-- PMM