From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753067AbdLKOUm (ORCPT ); Mon, 11 Dec 2017 09:20:42 -0500 Received: from mail-lf0-f65.google.com ([209.85.215.65]:37092 "EHLO mail-lf0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752806AbdLKOUk (ORCPT ); Mon, 11 Dec 2017 09:20:40 -0500 X-Google-Smtp-Source: ACJfBotQRMj/VmdF5bPEeUJblM11meU3JFcLPpBeZwmScINeA8tTR0XN7OpXBAN/FTLF3j630ddMTA== Subject: Re: [PATCH v1] usb: phy: tegra: Increase PHY clock stabilization timeout To: Thierry Reding Cc: Felipe Balbi , Jonathan Hunter , linux-usb@vger.kernel.org, linux-tegra@vger.kernel.org, linux-kernel@vger.kernel.org References: <20171210225535.8532-1-digetx@gmail.com> <20171211095322.GC10671@ulmo> From: Dmitry Osipenko Message-ID: <3690dece-c1a2-e7d5-1634-dfdf17c303c4@gmail.com> Date: Mon, 11 Dec 2017 17:20:36 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 MIME-Version: 1.0 In-Reply-To: <20171211095322.GC10671@ulmo> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 11.12.2017 12:53, Thierry Reding wrote: > On Mon, Dec 11, 2017 at 01:55:35AM +0300, Dmitry Osipenko wrote: >> This fixes "utmi_phy_clk_enable: timeout waiting for phy to stabilize" >> error message. >> >> Signed-off-by: Dmitry Osipenko >> --- >> drivers/usb/phy/phy-tegra-usb.c | 13 ++++--------- >> 1 file changed, 4 insertions(+), 9 deletions(-) >> >> diff --git a/drivers/usb/phy/phy-tegra-usb.c b/drivers/usb/phy/phy-tegra-usb.c >> index f668bfb708d3..7d5db625f800 100644 >> --- a/drivers/usb/phy/phy-tegra-usb.c >> +++ b/drivers/usb/phy/phy-tegra-usb.c >> @@ -16,7 +16,7 @@ >> #include >> #include >> #include >> -#include >> +#include >> #include >> #include >> #include >> @@ -305,14 +305,9 @@ static int utmip_pad_power_off(struct tegra_usb_phy *phy) >> >> static int utmi_wait_register(void __iomem *reg, u32 mask, u32 result) >> { >> - unsigned long timeout = 2000; >> - do { >> - if ((readl(reg) & mask) == result) >> - return 0; >> - udelay(1); >> - timeout--; >> - } while (timeout); >> - return -1; >> + u32 tmp; >> + >> + return readl_poll_timeout(reg, tmp, (tmp & mask) == result, 1, 5000); > > Technically I think this should be readl_poll_timeout_atomic() because > the above used to use udelay() instead of usleep_range(). But since the > function is never used inside atomic context, this looks fine. > > You may want to bump the sleep time between reads to something like 10 > or 20. usleep_range() doesn't always work well with very short values, > and given that you already bump the timeout from 2 ms to 5 ms indicates > to me that we're actually spending a lot of time in this loop, and > iterating somewhere between 2000 and 5000 times isn't any good. We only > use this function to wait for the USB_PHY_CLK_VALID bit which happens > during clock enable and disable, which isn't going to be very often, so > even in the best case (where the clock is immediately valid) there's no > need to return within a microsecond. Thank you very much for the suggestion. Given that 2ms isn't enough, it should be fine increase sleep time even to 1-2ms.