On 8/19/20 8:22 PM, Tom Rini wrote: [...] >> +#include <common.h> >> +#include <dm.h> >> +#include <usb.h> >> +#include <linux/compat.h> >> +#include <linux/errno.h> >> +#include <linux/delay.h> >> +#include <linux/usb/dwc3.h> >> +#include <usb/xhci.h>
Please keep the list sorted. >> +/* Declare global data pointer */ >> +DECLARE_GLOBAL_DATA_PTR; >> + >> +struct ipq_xhci_platdata { >> + fdt_addr_t hcd_base; >> + unsigned int rst_ctrl; >> + unsigned int hs_only; bool ... >> +}; >> + >> +struct ipq_xhci { >> + struct ipq_xhci_platdata usb_plat; >> + struct xhci_ctrl ctrl; >> + struct udevice* dev; >> + struct xhci_hccr *hcd; >> + struct dwc3 *dwc3_reg; >> +}; >> + >> +void ipq_reset_usb_phy(void *data) >> +{ >> + unsigned int gcc_rst_ctrl; >> + struct ipq_xhci_platdata *platdata; >> + struct ipq_xhci *ipq = (struct ipq_xhci *)data; Pass struct ipg_xhci pointer in instead of void *. >> + platdata = dev_get_platdata(ipq->dev); >> + if (platdata == NULL) { >> + printf("Error: %s Failed\n", __func__); dev_err() here. >> + return; >> + } Shouldn't this be part of a PHY driver ? >> + gcc_rst_ctrl = platdata->rst_ctrl; >> + >> + if (gcc_rst_ctrl) { >> + /* assert HS PHY POR reset */ >> + setbits_le32(gcc_rst_ctrl, 0x10); >> + mdelay(10); Does it really need such lengthy delays here ? >> + /* assert HS PHY SRIF reset */ >> + setbits_le32(gcc_rst_ctrl, 0x4); >> + mdelay(10); >> + >> + /* deassert HS PHY SRIF reset and program HS PHY registers */ >> + clrbits_le32(gcc_rst_ctrl, 0x4); >> + mdelay(10); >> + >> + /* de-assert USB3 HS PHY POR reset */ >> + clrbits_le32(gcc_rst_ctrl, 0x10); This BIT(4) should likely be #define'd as a macro , same for the others. >> + mdelay(10); >> + >> + if (!platdata->hs_only) { >> + /* assert SS PHY POR reset */ >> + setbits_le32(gcc_rst_ctrl, 0x20); >> + mdelay(10); >> + >> + /* deassert SS PHY POR reset */ >> + clrbits_le32(gcc_rst_ctrl, 0x20); >> + } >> + } >> +} >> + >> +static int ipq_xhci_core_init(struct ipq_xhci *ipq) >> +{ >> + int ret = 0; >> + >> + ipq_reset_usb_phy((void *)ipq); >> + >> + ret = dwc3_core_init(ipq->dwc3_reg); >> + if (ret) { >> + debug("%s:failed to initialize core\n", __func__); dev_dbg() >> + return ret; >> + } >> + >> + /* We are hard-coding DWC3 core to Host Mode */ >> + dwc3_set_mode(ipq->dwc3_reg, DWC3_GCTL_PRTCAP_HOST); >> + >> + return ret; >> +} >> + >> +static void ipq_xhci_core_exit(struct ipq_xhci *ipq) >> +{ Is some code missing here ? >> +} >> + >> +static int xhci_usb_remove(struct udevice *dev) >> +{ >> + int ret; >> + ret = xhci_deregister(dev); >> + >> + if (ret != 0) { >> + debug("%s:xhci deregistration failed\n", __func__); dev_dbg() >> + return ret; >> + } >> + >> + ipq_xhci_core_exit(dev_get_priv(dev)); >> + >> + return 0; return ipg_xhci_core_exit() to propagate the error value (if any). >> +} [...]