Hello,

After I apply this patch on 4.14 (or receive it with 4.14.70) I see a 
regression with
the Qualcomm QUSB2 phy driver. I'm testing on a Dragonboard 820c.
In boot log I get:

[    4.525502] phy phy-7412000.phy.6: QUSB2PHY pll lock failed: status reg = 0
[    4.529232] phy phy-7412000.phy.6: phy init failed --> -16
[    4.536170] dwc3 7600000.dwc3: failed to initialize core
[    4.541743] dwc3: probe of 7600000.dwc3 failed with error -16
[    4.549979] phy phy-7411000.phy.5: QUSB2PHY pll lock failed: status reg = 0
[    4.552843] phy phy-7411000.phy.5: phy init failed --> -16
[    4.559606] dwc3 6a00000.dwc3: failed to initialize core
[    4.565181] dwc3: probe of 6a00000.dwc3 failed with error -16

I can provide a full log if needed.

Best regards,
Todor



On 12.02.2018 15:30, Roger Quadros wrote:
> In order for ULPI PHYs to work, dwc3_phy_setup() and dwc3_ulpi_init()
> must be doene before dwc3_core_get_phy().
> 
> commit 541768b08a40 ("usb: dwc3: core: Call dwc3_core_get_phy() before 
> initializing phys")
> broke this.
> 
> The other issue is that dwc3_core_get_phy() and dwc3_ulpi_init() should
> be called only once during the life cycle of the driver. However,
> as dwc3_core_init() is called during system suspend/resume it will
> result in multiple calls to dwc3_core_get_phy() and dwc3_ulpi_init()
> which is wrong.
> 
> Fix this by moving dwc3_ulpi_init() out of dwc3_phy_setup()
> into dwc3_core_ulpi_init(). Use a flag 'ulpi_ready' to ensure that
> dwc3_core_ulpi_init() is called only once from dwc3_core_init().
> 
> Use another flag 'phys_ready' to call dwc3_core_get_phy() only once from
> dwc3_core_init().
> 
> Fixes: 541768b08a40 ("usb: dwc3: core: Call dwc3_core_get_phy() before 
> initializing phys")
> Fixes: f54edb539c11 ("usb: dwc3: core: initialize ULPI before trying to get 
> the PHY")
> Cc: linux-stable <sta...@vger.kernel.org> # >= v4.13
> Signed-off-by: Roger Quadros <rog...@ti.com>
> ---
> Rebased on Felipe's testing/fixes branch.
> 
>  drivers/usb/dwc3/core.c | 47 ++++++++++++++++++++++++++++++++++++-----------
>  drivers/usb/dwc3/core.h |  5 +++++
>  2 files changed, 41 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 59511f2..f1d838a 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -486,6 +486,22 @@ static void dwc3_cache_hwparams(struct dwc3 *dwc)
>       parms->hwparams8 = dwc3_readl(dwc->regs, DWC3_GHWPARAMS8);
>  }
>  
> +static int dwc3_core_ulpi_init(struct dwc3 *dwc)
> +{
> +     int intf;
> +     int ret = 0;
> +
> +     intf = DWC3_GHWPARAMS3_HSPHY_IFC(dwc->hwparams.hwparams3);
> +
> +     if (intf == DWC3_GHWPARAMS3_HSPHY_IFC_ULPI ||
> +         (intf == DWC3_GHWPARAMS3_HSPHY_IFC_UTMI_ULPI &&
> +          dwc->hsphy_interface &&
> +          !strncmp(dwc->hsphy_interface, "ulpi", 4)))
> +             ret = dwc3_ulpi_init(dwc);
> +
> +     return ret;
> +}
> +
>  /**
>   * dwc3_phy_setup - Configure USB PHY Interface of DWC3 Core
>   * @dwc: Pointer to our controller context structure
> @@ -497,7 +513,6 @@ static void dwc3_cache_hwparams(struct dwc3 *dwc)
>  static int dwc3_phy_setup(struct dwc3 *dwc)
>  {
>       u32 reg;
> -     int ret;
>  
>       reg = dwc3_readl(dwc->regs, DWC3_GUSB3PIPECTL(0));
>  
> @@ -568,9 +583,6 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
>               }
>               /* FALLTHROUGH */
>       case DWC3_GHWPARAMS3_HSPHY_IFC_ULPI:
> -             ret = dwc3_ulpi_init(dwc);
> -             if (ret)
> -                     return ret;
>               /* FALLTHROUGH */
>       default:
>               break;
> @@ -727,6 +739,7 @@ static void dwc3_core_setup_global_control(struct dwc3 
> *dwc)
>  }
>  
>  static int dwc3_core_get_phy(struct dwc3 *dwc);
> +static int dwc3_core_ulpi_init(struct dwc3 *dwc);
>  
>  /**
>   * dwc3_core_init - Low-level initialization of DWC3 Core
> @@ -758,17 +771,27 @@ static int dwc3_core_init(struct dwc3 *dwc)
>                       dwc->maximum_speed = USB_SPEED_HIGH;
>       }
>  
> -     ret = dwc3_core_get_phy(dwc);
> +     ret = dwc3_phy_setup(dwc);
>       if (ret)
>               goto err0;
>  
> -     ret = dwc3_core_soft_reset(dwc);
> -     if (ret)
> -             goto err0;
> +     if (!dwc->ulpi_ready) {
> +             ret = dwc3_core_ulpi_init(dwc);
> +             if (ret)
> +                     goto err0;
> +             dwc->ulpi_ready = true;
> +     }
>  
> -     ret = dwc3_phy_setup(dwc);
> +     if (!dwc->phys_ready) {
> +             ret = dwc3_core_get_phy(dwc);
> +             if (ret)
> +                     goto err0a;
> +             dwc->phys_ready = true;
> +     }
> +
> +     ret = dwc3_core_soft_reset(dwc);
>       if (ret)
> -             goto err0;
> +             goto err0a;
>  
>       dwc3_core_setup_global_control(dwc);
>       dwc3_core_num_eps(dwc);
> @@ -841,6 +864,9 @@ static int dwc3_core_init(struct dwc3 *dwc)
>       phy_exit(dwc->usb2_generic_phy);
>       phy_exit(dwc->usb3_generic_phy);
>  
> +err0a:
> +     dwc3_ulpi_exit(dwc);
> +
>  err0:
>       return ret;
>  }
> @@ -1235,7 +1261,6 @@ static int dwc3_probe(struct platform_device *pdev)
>  
>  err3:
>       dwc3_free_event_buffers(dwc);
> -     dwc3_ulpi_exit(dwc);
>  
>  err2:
>       pm_runtime_allow(&pdev->dev);
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index 185b960..860d2bc 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -797,7 +797,9 @@ struct dwc3_scratchpad_array {
>   * @usb3_phy: pointer to USB3 PHY
>   * @usb2_generic_phy: pointer to USB2 PHY
>   * @usb3_generic_phy: pointer to USB3 PHY
> + * @phys_ready: flag to indicate that PHYs are ready
>   * @ulpi: pointer to ulpi interface
> + * @ulpi_ready: flag to indicate that ULPI is initialized
>   * @u2sel: parameter from Set SEL request.
>   * @u2pel: parameter from Set SEL request.
>   * @u1sel: parameter from Set SEL request.
> @@ -895,7 +897,10 @@ struct dwc3 {
>       struct phy              *usb2_generic_phy;
>       struct phy              *usb3_generic_phy;
>  
> +     bool                    phys_ready;
> +
>       struct ulpi             *ulpi;
> +     bool                    ulpi_ready;
>  
>       void __iomem            *regs;
>       size_t                  regs_size;
> 

Reply via email to