Regression caused by: usb: add a flag to skip PHY initialization to struct usb_hcd

Commit Message

Mats Karrman June 5, 2018, 11 p.m. UTC

On 2018-06-05 02:54, Peter Chen wrote:
>   
>>>> And this is what the "decompiled" device tree entry for the USB
>>>> controller and phy look like:
>>>>
>>>>                usb@2184200 {
>>>>                    compatible = "fsl,imx6q-usb", "fsl,imx27-usb";
>>>>                    reg = <0x2184200 0x200>;
>>>>                    interrupts = <0x0 0x28 0x4>;
>>>>                    clocks = <0x4 0xa2>;
>>>>                    fsl,usbphy = <0x2c>;
>>>>                    fsl,usbmisc = <0x29 0x1>;
>>>>                    dr_mode = "host";
>>>>                    ahb-burst-config = <0x0>;
>>>>                    tx-burst-size-dword = <0x10>;
>>>>                    rx-burst-size-dword = <0x10>;
>>>>                    status = "okay";
>>>>                    disable-over-current;
>>>>                    vbus-supply = <0x2d>;
>>>>                };
>>>>
>>>>                usbphy@20ca000 {
>>>>                    compatible = "fsl,imx6q-usbphy", "fsl,imx23-usbphy";
>>>>                    reg = <0x20ca000 0x1000>;
>>>>                    interrupts = <0x0 0x2d 0x4>;
>>>>                    clocks = <0x4 0xb7>;
>>>>                    fsl,anatop = <0x2>;
>>>>                    phandle = <0x2c>;
>>>>                };
>>>>
>>>> So, using deprecated? "fsl,usbphy" instead of "phys", in case that matters.
>>>>
>>> It is ok.
>>>
>>> Check two things:
>>> - ci->usb_phy is non-NULL, and ci->phy is NULL
>> That is correct
>>
>>> - phy_roothub is NULL at the functions of drivers/usb/core/phy.c
>> I put a trace at the beginning of each of the functions of that file but none of them is
>> ever called.
>>
> It is so strange. Please double confirm your git bisect is correct, if it is, try to find which
> line causes your regression.

Bisect confirmed.

Admittedly without really understanding everything that is going on, I put
back the deleted lines from this patch chunk and it started working again:


Without a value in hcd->usb_phy, the call to usb_phy_notify_disconnect()
in hub_port_connect() (usb/core/hub.c) never fires but that is only part of the
problem. Hope this helps.

BR // Mats

Comments

Peter Chen June 6, 2018, 2:10 a.m. UTC | #1

> 

> Admittedly without really understanding everything that is going on, I put back the

> deleted lines from this patch chunk and it started working again:

> 

> diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c index

> 19d60ed..af45aa32 100644

> --- a/drivers/usb/chipidea/host.c

> +++ b/drivers/usb/chipidea/host.c

> @@ -124,10 +124,8 @@ static int host_start(struct ci_hdrc *ci)

> 

>          hcd->power_budget = ci->platdata->power_budget;

>          hcd->tpl_support = ci->platdata->tpl_support;

> -       if (ci->phy)

> -               hcd->phy = ci->phy;

> -       else

> -               hcd->usb_phy = ci->usb_phy;

> +       if (ci->phy || ci->usb_phy)

> +               hcd->skip_phy_initialization = 1;

> 

>          ehci = hcd_to_ehci(hcd);

>          ehci->caps = ci->hw_bank.cap;

> 

> Without a value in hcd->usb_phy, the call to usb_phy_notify_disconnect() in

> hub_port_connect() (usb/core/hub.c) never fires but that is only part of the problem.

> Hope this helps.

> 


Thanks, Mat.
I posted a patch for this fix, help to test please.

Peter

Patch

diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
index 19d60ed..af45aa32 100644
--- a/drivers/usb/chipidea/host.c
+++ b/drivers/usb/chipidea/host.c
@@ -124,10 +124,8 @@  static int host_start(struct ci_hdrc *ci)

         hcd->power_budget = ci->platdata->power_budget;
         hcd->tpl_support = ci->platdata->tpl_support;
-       if (ci->phy)
-               hcd->phy = ci->phy;
-       else
-               hcd->usb_phy = ci->usb_phy;
+       if (ci->phy || ci->usb_phy)
+               hcd->skip_phy_initialization = 1;

         ehci = hcd_to_ehci(hcd);
         ehci->caps = ci->hw_bank.cap;