Re: [PATCH v1] usb: phy: tegra: Increase PHY clock stabilization timeout

[Thierry Reding]

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 <digetx@xxxxxxxxx>
> ---
>  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 <linux/export.h>
>  #include <linux/module.h>
>  #include <linux/platform_device.h>
> -#include <linux/io.h>
> +#include <linux/iopoll.h>
>  #include <linux/gpio.h>
>  #include <linux/of.h>
>  #include <linux/of_device.h>
> @@ -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.

Thierry

Attachment: signature.asc
Description: PGP signature