Re: [PATCH] usb: phy: tegra: Fix phy suspend for UDC

[Dmitry Osipenko <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

On 29.09.2017 16:10, Jon Hunter wrote:
> Adding Dmitry ...
> 
> Felipe, Thierry, this is needed for v4.14-rc because suspend is currently
> broken for some Tegra devices.
> 
> Jon
> 
> On 29/09/17 14:06, Jon Hunter wrote:
>> Commit dfebb5f43a78 ("usb: chipidea: Add support for Tegra20/30/114/124")
>> added UDC support for Tegra but with UDC support enabled, is was found
>> that Tegra30, Tegra114 and Tegra124 would hang on entry to suspend.
>>
>> The hang occurred during the suspend of the USB PHY when the Tegra PHY
>> driver attempted to disable the PHY clock. The problem is that before
>> the Tegra PHY driver is suspended, the chipidea driver already disabled
>> the PHY clock and when the Tegra PHY driver suspended, it could not read
>> DEVLC register and caused the device to hang.
>>
>> The Tegra USB PHY driver is used by both the Tegra EHCI driver and now
>> the chipidea UDC driver and so simply removing the disabling of the PHY
>> clock from the USB PHY driver would not work for the Tegra EHCI driver.
>> Fortunately, the status of the USB PHY clock can be read from the
>> USB_SUSP_CTRL register and therefore, to workaround this issue, simply
>> poll the register prior to disabling the clock in USB PHY driver to see
>> if clock gating has already been initiated. Please note that it can take
>> a few uS for the clock to disable and so simply reading this status
>> register once on entry is not sufficient.
>>
>> Please note that no issues are seen with Tegra20 because it has a slightly
>> different PHY to Tegra30/114/124.
>>
> 
> I forgot the fixes tag ...
> 
> Fixes: dfebb5f43a78 ("usb: chipidea: Add support for Tegra20/30/114/124")
>  
>> Signed-off-by: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>> ---
>>  drivers/usb/phy/phy-tegra-usb.c | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/usb/phy/phy-tegra-usb.c b/drivers/usb/phy/phy-tegra-usb.c
>> index 5fe4a5704bde..c8fff99bd16e 100644
>> --- a/drivers/usb/phy/phy-tegra-usb.c
>> +++ b/drivers/usb/phy/phy-tegra-usb.c
>> @@ -329,6 +329,14 @@ static void utmi_phy_clk_disable(struct tegra_usb_phy *phy)
>>  	unsigned long val;
>>  	void __iomem *base = phy->regs;
>>  
>> +	/*
>> +	 * The USB driver may have already initiated the phy clock
>> +	 * disable so wait to see if the clock turns off and if not
>> +	 * then proceed with gating the clock.
>> +	 */
>> +	if (utmi_wait_register(base + USB_SUSP_CTRL, USB_PHY_CLK_VALID, 0) == 0)
>> +		return;
>> +
>>  	if (phy->is_legacy_phy) {
>>  		val = readl(base + USB_SUSP_CTRL);
>>  		val |= USB_SUSP_SET;
>>
> 

Adding analogical check to clk_enable fixes "utmi_phy_clk_enable: timeout
waiting for phy to stabilize" error message during of kernel boot-up on Tegra20.

What about to incorporate it to this patch?

@@ -354,16 +354,25 @@ static void utmi_phy_clk_disable(struct tegra_usb_phy *phy)
 		pr_err("%s: timeout waiting for phy to stabilize\n", __func__);
 }

 static void utmi_phy_clk_enable(struct tegra_usb_phy *phy)
 {
 	unsigned long val;
 	void __iomem *base = phy->regs;

+	/*
+	 * The USB driver may have already initiated the phy clock
+	 * enable so wait to see if the clock turns on and if not
+	 * then proceed with ungating the clock.
+	 */
+	if (utmi_wait_register(base + USB_SUSP_CTRL, USB_PHY_CLK_VALID,
+						     USB_PHY_CLK_VALID) == 0)
+		return;
+
 	if (phy->is_legacy_phy) {
 		val = readl(base + USB_SUSP_CTRL);
 		val |= USB_SUSP_CLR;
 		writel(val, base + USB_SUSP_CTRL);

 		udelay(10);

 		val = readl(base + USB_SUSP_CTRL);

-- 
Dmitry