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

[Robert Marko]

Hi,
I did some more testing and I can confirm that USB3.0 port works fine
with the PHY driver and old XHCI-DWC3 while with DWC3 generic one it
does not work.
I have been unable to figure out why is that exactly.

Regards
Robert

On Thu, Aug 27, 2020 at 9:13 PM Marek Vasut <marex at denx.de> wrote:
>
> On 8/27/20 9:08 PM, Robert Marko wrote:
> > 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.
>
>
> In that case, +CC Bin.