On Tue, Dec 01, 2020 at 11:34:25AM +0000, Peter Maydell wrote:
> On Tue, 1 Dec 2020 at 08:34, Sai Pavan Boddu <sai.pavan.bo...@xilinx.com>
> wrote:
> >
> > From: Vikram Garhwal <fnu.vik...@xilinx.com>
> >
> > Connect VersalUbs2 subsystem to xlnx-versal SOC, its placed
>
> Typo : "VersalUSB2".
>
>
> > 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.vik...@xilinx.com>
> > Signed-off-by: Sai Pavan Boddu <sai.pavan.bo...@xilinx.com>
>
> Code looks OK but I'll let somebody else from Xilinx review the detail.
>
> > +static void fdt_add_usb_xhci_nodes(VersalVirt *s)
> > +{
> > + const char clocknames[] = "bus_clk\0ref_clk";
> > + char *name = g_strdup_printf("/usb@%" PRIx32, MM_USB2_CTRL_REGS);
> > + const char compat[] = "xlnx,versal-dwc3";
> > +
> > + qemu_fdt_add_subnode(s->fdt, name);
> > + qemu_fdt_setprop(s->fdt, name, "compatible",
> > + compat, sizeof(compat));
> > + qemu_fdt_setprop_sized_cells(s->fdt, name, "reg",
> > + 2, MM_USB2_CTRL_REGS,
> > + 2, MM_USB2_CTRL_REGS_SIZE);
> > + qemu_fdt_setprop(s->fdt, name, "clock-names",
> > + clocknames, sizeof(clocknames));
> > + qemu_fdt_setprop_cells(s->fdt, name, "clocks",
> > + s->phandle.clk_25Mhz,
> > s->phandle.clk_125Mhz);
> > + qemu_fdt_setprop(s->fdt, name, "ranges", NULL, 0);
> > + qemu_fdt_setprop_cell(s->fdt, name, "#address-cells", 2);
> > + qemu_fdt_setprop_cell(s->fdt, name, "#size-cells", 2);
> > + qemu_fdt_setprop_cell(s->fdt, name, "phandle", s->phandle.usb);
> > + g_free(name);
> > +
> > + {
> > + const char irq_name[] = "dwc_usb3";
> > + const char compat[] = "snps,dwc3";
>
> Minor coding style side note, but I'm not hugely fond of
> code blocks in the middle of functions just for declaring
> variables. You could either put these variable declarations
> at the top of the function, or if you think the code in the
> block is self contained and worth putting in its own function
> you could do that.
>
Hi Sai, I beleive I had already reviewed a previous version of this
patch so after you fix the stuff the Peter pointed out feel free to add my
Rb:
Reviewed-by: Edgar E. Iglesias <edgar.igles...@xilinx.com>
Cheers,
Edgar
