> On 07.11.2014 02:16, Peter Chen wrote:
> >
> >> Hi Peter,
> >>
> >> On 06.11.2014 09:15, Peter Chen wrote:
> >>> On Wed, Nov 05, 2014 at 08:52:25PM +0200, Vladimir Zapolskiy wrote:
> >>>> On some boards powered by iMX6Q rev1.0 I get non-working USB Host 1
> >>>> (connected to a hub, no other devices are connected to this hub)
> >>>> and repeating resets from the chipidea host driver (with ported pm
> >>>> support from Freescale):
> >>>>
> >>>> ....
> >>>> [ 25.481714] usb 2-1: reset high-speed USB device number 2 using
> ci_hdrc
> >>>> [ 27.491716] usb 2-1: reset high-speed USB device number 2 using
> ci_hdrc
> >>>> [ 29.501721] usb 2-1: reset high-speed USB device number 2 using
> ci_hdrc
> >>>> ....
> >>>>
> >>>> As for me it seems that usb phy test clock gating may be done
> >>>> incorrectly, on disconnection it is running, and on connection it
> >>>> is gated, but may be it is the intention, unfortunately iMX6Q RM is
> >>>> not a satisfactory source of information on the topic.
> >>>>
> >>>> Either complete disabling of loopback for the host or inverting
> >>>> test clock gating solves my problem, this change proposes to invert
> >>>> test clock gating setting.
> >>>>
> >>>> Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@xxxxxxxxxx>
> >>>> Cc: Peter Chen <peter.chen@xxxxxxxxxxxxx>
> >>>> ---
> >>>> drivers/usb/phy/phy-mxs-usb.c | 4 ++--
> >>>> 1 file changed, 2 insertions(+), 2 deletions(-)
> >>>>
> >>>> diff --git a/drivers/usb/phy/phy-mxs-usb.c
> >>>> b/drivers/usb/phy/phy-mxs-usb.c index 0e0c415..fbd079f 100644
> >>>> --- a/drivers/usb/phy/phy-mxs-usb.c
> >>>> +++ b/drivers/usb/phy/phy-mxs-usb.c
> >>>> @@ -231,7 +231,7 @@ static void __mxs_phy_disconnect_line(struct
> >>>> mxs_phy *mxs_phy, bool disconnect)
> >>>>
> >>>> if (disconnect)
> >>>> writel_relaxed(BM_USBPHY_DEBUG_CLKGATE,
> >>>> - base + HW_USBPHY_DEBUG_CLR);
> >>>> + base + HW_USBPHY_DEBUG_SET);
> >>>>
> >>>> if (mxs_phy->port_id == 0) {
> >>>> reg = disconnect ? ANADIG_USB1_LOOPBACK_SET @@ -249,7
> >> +249,7 @@
> >>>> static void __mxs_phy_disconnect_line(struct mxs_phy *mxs_phy, bool
> >>>> disconnect)
> >>>>
> >>>> if (!disconnect)
> >>>> writel_relaxed(BM_USBPHY_DEBUG_CLKGATE,
> >>>> - base + HW_USBPHY_DEBUG_SET);
> >>>> + base + HW_USBPHY_DEBUG_CLR);
> >>>>
> >>>> /* Delay some time, and let Linestate be SE0 for controller */
> >>>> if (disconnect)
> >>>> --
> >>>> 1.7.10.4
> >>>>
> >>>
> >>> Hi Vladimir,
> >>>
> >>> The thing you have changed is not for this issue. The current
> >>> mainline code can support imx6q revision 1.2+ SoC which is used for real
> production.
> >>> I just tried my imx6q sabresd (revision 1.2) board with high speed
> >>> hub connected at felipe's newest next tree, it works ok with basic test.
> >>
> >> thank you for review and response.
> >>
> >> Just for your (and anyone's who occasionally meets the same problem)
> >> information, this strange behaviour is noticed only on some iMX6Q
> >> rev1.0 powered boards (the hardware of boards is the same), and I
> >> didn't succeed in grasping what may be the difference...
> >>
> >>> For rev 1.0 Soc, try to set MXS_PHY_ABNORMAL_IN_SUSPEND at
> >>> imx6q_phy_data like imx23's.
> >>>
> >>
> >> I've tried it, but unfortunately no luck with the quirk, only disabling "test
> clocks"
> >> or removing loopback logic for host1 helps...
> >>
> >> Since vanilla kernel has exported imx_get_soc_revision() interface,
> >> does it make sense to check for SoC revision and add a
> >> quirk/exception for iMX6Q rev
> >> 1.0 (and probably rev 1.1) powered boards in the phy driver?
> >>
> >
> > I suspect that your pm support for chipidea may not work well, if your
> > rev 1.0 chip runs freescale internal bsp (3.0.35 or 3.10), do you meet the same
> problem?
>
> I haven't tried to run Freescale internal BSP, I run vanilla kernel with chipidea
> host pm ops from Freescale internal BSP, and I suppose there is a plan to port it
> into the upstream eventually.
>
> > If this problem isn't existed at upstream kernel, so I can't accept that change.
>
> Well, my intention is to inform you that the problem exists. I think that the
> vanilla kernel is used by iMX6Q rev1.0 and rev1.1 holders as well, and if there is
> an actual problem in usb phy code, after porting chipidea host runtime pm it
> will hit all such users. There is a chance to analyze/fix the problem in advance
> though.
>
> And the problem is hidden in the upstream kernel, because simply there is no
> callers of mxs_phy_set_wakeup() right at the moment, I don't know if it is
> called an existing problem or non-existing one.
>
Thanks for pointing it, but things are better step by step. You have found this problem
after porting chipidea pm code to upstream kernel, so first you should post the chipidea
pm patch set, then post this patch if the problem is still existed.
I think your host port should work ok if disable runtime at chipidea (just delete CI_HDRC_SUPPORTS_RUNTIME_PM
at imx6q's platform flag at ci_hdrc_imx.c).
> > For your internal use, you can disable
> MXS_PHY_DISCONNECT_LINE_WITHOUT_VBUS.
>
> Right, this would work since it completely disables the problematic code in
> __mxs_phy_disconnect_line().
>
> Theoretically one day does it make sense to add an exception disabling
> MXS_PHY_DISCONNECT_LINE_WITHOUT_VBUS for iMX6Q rev1.0 and rev1.1 in
> the vanilla kernel or not? Or is there a better technical solution?
>
Just like I said, It is better we upstream chipidea pm code first. When I worked at Rev 1.0 chipidea before,
I did not meet the problem you described, so I am wonder if it is the root cause for your problem.
Peter
��.n��������+%������w��{.n�����{���)�� jg��� �����ݢj����G���� ���j:+v���w�m������w�������h�����٥
