[PATCH 4/5] usb: xhci: Add Qualcomm DWC3 xHCI driver

[Robert Marko]

I don't think that this has anything to do with Qualcomm HW.
I find it weird that hooking into the non-DM DWC3 driver and simply
calling core init has both ports working while DM version only has
USB2.0 one working.

On Thu, Aug 27, 2020 at 8:06 PM Marek Vasut <marex at denx.de> wrote:
>
> On 8/27/20 5:01 PM, Robert Marko wrote:
> > On Wed, Aug 19, 2020 at 9:34 PM Marek Vasut <marex at denx.de> wrote:
> >>
> >> 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).
> >>
> >>>> +}
> >>
> >> [...]
> >
> > Hi Marek,
> > thanks for the review.
> >
> > I have moved the USB PHY-s into a separate driver as Linux does.
> > I have also dropped this custom driver and moved to use of the DWC3
> > generic glue one,
> > that way nodes and everything will match Linux as much as possible.
> > Unfortunately, it will most likely take a while for me to send a v2 of
> > this patchset as USB3.0 capable port is not working.
> > USB2.0 one is working fine and detecting the TI CC2540 I have
> > connected to it, but USB3.0 external Type-A port won't detect
> > USB mass storage or WLAN adapters at all.
> >
> > Do you maybe have advice on how to debug this?
>
> I don't know anything about the qualcomm hardware and I never worked
> with any, sorry.