On Tuesday, September 01, 2015 at 02:48:27 PM, Siva Durga Prasad Paladugu wrote:
HI Marek,
Hi,
-----Original Message----- From: Marek Vasut [mailto:marex@denx.de] Sent: Tuesday, September 01, 2015 5:09 PM To: Siva Durga Prasad Paladugu Cc: u-boot@lists.denx.de; Siva Durga Prasad Paladugu; monstr@monstr.eu Subject: Re: [PATCH v2 1/2] usb: zynqmp: Add XHCI driver support
On Tuesday, September 01, 2015 at 12:31:02 PM, Siva Durga Prasad Paladugu
wrote:
Added USB XHCI driver support for zynqmp.
Signed-off-by: Siva Durga Prasad Paladugu sivadur@xilinx.com
Hi, looks almost good, a few minor nits though ...
[...]
+unsigned long ctr_addr[] = {ZYNQMP_USB0_XHCI_BASEADDR,
static const void __iomem *ctl_addr[]
ZYNQMP_USB1_XHCI_BASEADDR};I guess you can define something like CONFIG_ZYNQMP_XHCI_LIST { address ... } in your board config file and then use static const unsigned long ctl_addr[] = CONFIG_ZYNQMP... ; This will cover board which only use one controller.
Yeah DT is the ideal way, For now, I can modify it to be like this static const void __iomem *ctl_addr[] = { ZYNQMP_USB0_XHCI_BASEADDR, ZYNQMP_USB1_XHCI_BASEADDR};
But to define a macro in board config file, I may have to include hardware.h, where iam defining all base addresses of the IP's into the board config file just for this.
Is that a problem ?
Is it fine if I can keep as I mentioned above?
I am not very fond of it, since this is broken for boards which don't use both controllers.
The ideal way would be to obtain these information from DT though.
+__weak int __board_usb_init(int index, enum usb_init_type init) {
- return 0;
+}
+void usb_phy_reset(struct dwc3 *dwc3_reg) {
- /* Assert USB3 PHY reset */
- setbits_le32(&dwc3_reg->g_usb3pipectl[0],
+DWC3_GUSB3PIPECTL_PHYSOFTRST);
- /* Assert USB2 PHY reset */
- setbits_le32(&dwc3_reg->g_usb2phycfg,
DWC3_GUSB2PHYCFG_PHYSOFTRST);
- mdelay(200);
That's some lazy crappy controller. Is this long delay needed ?
Yeah can you just keep it as is for some time. This is how we tested on our emulation platforms. I will anyway modify it at later point of time.
Why can't you modify it now ? 200mS is just too long in my opinion, what's the reason for such a long delay ?
[...]