On Monday, November 16, 2015 at 08:23:24 AM, Siva Durga Prasad Paladugu wrote:
> Hi Marek,
Hi,
[...]
> > > + ctx->hcd = (struct xhci_hccr *)ctr_addr[index];
> > > + ctx->dwc3_reg = (struct dwc3 *)((char *)(ctx->hcd) +
> > > +DWC3_REG_OFFSET);
> >
> > Should be void *, not char * I think. Also, the parenthesis around
> > ctx->hcd are not needed.
>
> I think keeping char* would be good for easy understanding and readability.
Using bogus casts is never good for anything.
> > > + ret = board_usb_init(index, USB_INIT_HOST);
> > > + if (ret != 0) {
> > > + puts("Failed to initialize board for USB\n");
> > > + return ret;
> > > + }
> > > +
> > > + ret = zynqmp_xhci_core_init(ctx);
> > > + if (ret < 0) {
> > > + puts("Failed to initialize xhci\n");
> > > + return ret;
> > > + }
> > > +
> > > + *hccr = (struct xhci_hccr *)ctx->hcd;
> > > + *hcor = (struct xhci_hcor *)((uint32_t) *hccr
> > > + + HC_LENGTH(xhci_readl(&(*hccr)-
> > >
> > >cr_capbase)));
> >
> > You want to use uintptr_t for the same of 64bit parts.
>
> Ok
>
> > > + debug("zynqmp-xhci: init hccr %x and hcor %x hc_length %d\n",
> > > + (uint32_t)*hccr, (uint32_t)*hcor,
> > > + (uint32_t)HC_LENGTH(xhci_readl(&(*hccr)->cr_capbase)));
> >
> > We have %p for printing pointers, so drop the casts.
> >
> > Also, since you're using HC_LENGTH() here twice, you might as well put
> > it's value into temporary variable and use it in both places, it might
> > make the code slightly more readable.
>
> Ok, will take care in next version
Thanks!
Best regards,
Marek Vasut
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
