On Fri, 2017-06-02 at 15:00 -0700, Stephen Boyd wrote:
> On 05/26, Fabien Lahoudere wrote:
> > Hello
> >
> > I modify ci_hrdc_imx_probe to bypass "data->phy =
> > devm_usb_get_phy_by_phandle(&pdev->dev,
> > "fsl,usbphy", 0);". Everything works as expected and call ci_ulpi_init.
> >
> > The problem is that in ci_ulpi_init, before calling "ci->ulpi =
> > ulpi_register_interface(ci->dev,
> > &ci->ulpi_ops);" (to initialize our phy), "hw_phymode_configure(ci);" is
> > called which is the
> > original function that make our system to hang.
> >
> > Our phy is not initialised before calling ulpi_register_interface so I
> > don't understand how the
> > phy
> > can reply if it is not out of reset state.
>
> I haven't see any problem in hw_phymode_configure(). What's the
> value of ci->platdata->phy_mode? USBPHY_INTERFACE_MODE_ULPI? If
> you phy needs to be taken out of reset to reply to the ulpi reads
> of the vendor/product ids, then it sounds like you have a similar
> situation to what I had. I needed to turn on some regulators to
> get those reads to work, otherwise they would fail, but knowing
> what needed to be turned on basically meant I needed to probe the
> ulpi driver so probing the ids wasn't going to be useful. So on
> my device the reads for the ids go through, but they get all
> zeroes back, which is actually ok because there aren't any bits
> set on my devices anyway. After the reads see 0, we fallback to
> DT matching, which avoids the "bring it out of reset/power it on"
> sorts of problems entirely.
>
Yes the phy mode is configured to USBPHY_INTERFACE_MODE_ULPI.
Indeed, this phy need to be out of reset to work. For example everything works
fine if I call
"_ci_usb_phy_init(ci);" before calling "hw_phymode_configure(ci);"
This function only init reset GPIO and clock.
For information, the original patch I have to fix the issue:
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 79ad8e9..21aaff1 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -391,6 +391,7 @@ static int ci_usb_phy_init(struct ci_hdrc *ci)
case USBPHY_INTERFACE_MODE_UTMI:
case USBPHY_INTERFACE_MODE_UTMIW:
case USBPHY_INTERFACE_MODE_HSIC:
+ case USBPHY_INTERFACE_MODE_ULPI:
ret = _ci_usb_phy_init(ci);
if (!ret)
hw_wait_phy_stable();
@@ -398,7 +399,6 @@ static int ci_usb_phy_init(struct ci_hdrc *ci)
return ret;
hw_phymode_configure(ci);
break;
- case USBPHY_INTERFACE_MODE_ULPI:
case USBPHY_INTERFACE_MODE_SERIAL:
hw_phymode_configure(ci);
ret = _ci_usb_phy_init(ci);
--
So if some ULPI phys need to be initialised before calling
"hw_phymode_configure", is it acceptable
if I separate "case USBPHY_INTERFACE_MODE_ULPI:" and add a DT binding
("init_phy_first") to define
the order to call both functions?
Something like:
case USBPHY_INTERFACE_MODE_ULPI:
if (ci->platdata->init_phy_first) {
ret = _ci_usb_phy_init(ci);
if (!ret)
hw_wait_phy_stable();
else
return ret;
}
hw_phymode_configure(ci);
if (!ci->platdata->init_phy_first) {
ret = _ci_usb_phy_init(ci);
if (ret)
return ret;
}
break;
This approach will not modify current behaviour but allow to initialize phy
first on demand.
