Hi Peter/Edgar,
> -----Original Message-----
> From: Edgar E. Iglesias <edgar.igles...@xilinx.com>
> Sent: Thursday, December 3, 2020 11:35 PM
> To: Peter Maydell <peter.mayd...@linaro.org>
> Cc: Sai Pavan Boddu <saip...@xilinx.com>; Markus Armbruster
> <arm...@redhat.com>; Marc-André Lureau <marcandre.lur...@redhat.com>;
> Paolo Bonzini <pbonz...@redhat.com>; Gerd Hoffmann <kra...@redhat.com>;
> Edgar Iglesias <edg...@xilinx.com>; Francisco Eduardo Iglesias
> <figle...@xilinx.com>; Alistair Francis <alistair.fran...@wdc.com>; Eduardo
> Habkost <ehabk...@redhat.com>; Ying Fang <fangyi...@huawei.com>;
> Philippe Mathieu-Daudé <phi...@redhat.com>; Vikram Garhwal
> <f...@xilinx.com>; Paul Zimmerman <pauld...@gmail.com>; Sai Pavan Boddu
> <saip...@xilinx.com>; QEMU Developers <qemu-devel@nongnu.org>
> Subject: Re: [PATCH v14 4/4] arm: xlnx-versal: Connect usb to virt-versal
>
> 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.
[Sai Pavan Boddu] Yeah. I could fix this in V15, Thanks.
> >
>
> 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>
[Sai Pavan Boddu] Thanks Edgar.
Regards,
Sai Pavan
>
> Cheers,
> Edgar
