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).
>> +}
[...]
