From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marek Vasut Date: Mon, 16 Nov 2015 10:26:01 +0100 Subject: [U-Boot] [PATCH v5 1/2] usb: zynqmp: Add XHCI driver support In-Reply-To: References: <1447653554-36345-1-git-send-email-sivadur@xilinx.com> <201511160709.09633.marex@denx.de> Message-ID: <201511161026.01800.marex@denx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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