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 <ma...@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 <ma...@denx.de> wrote:
> >>
> >> On 8/27/20 5:01 PM, Robert Marko wrote:
> >>> On Wed, Aug 19, 2020 at 9:34 PM Marek Vasut <ma...@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.
