Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932213AbcKIUyW (ORCPT ); Wed, 9 Nov 2016 15:54:22 -0500 Received: from mail-wm0-f50.google.com ([74.125.82.50]:35697 "EHLO mail-wm0-f50.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754224AbcKIUyU (ORCPT ); Wed, 9 Nov 2016 15:54:20 -0500 MIME-Version: 1.0 In-Reply-To: <1478523658-9400-1-git-send-email-wulf@rock-chips.com> References: <1478523658-9400-1-git-send-email-wulf@rock-chips.com> From: Doug Anderson Date: Wed, 9 Nov 2016 12:54:18 -0800 Message-ID: Subject: Re: [PATCH] phy: rockchip-inno-usb2: correct 480MHz output clock stable time To: William Wu , =?UTF-8?Q?Heiko_St=C3=BCbner?= Cc: Kishon Vijay Abraham I , "linux-kernel@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , "open list:ARM/Rockchip SoC..." , "devicetree@vger.kernel.org" , Rob Herring , Frank Wang , =?UTF-8?B?6buE5rab?= , Brian Norris , Guenter Roeck , Matthias Kaehlcke , linux-clk Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1618 Lines: 41 Hi, On Mon, Nov 7, 2016 at 5:00 AM, William Wu wrote: > We found that the system crashed due to 480MHz output clock of > USB2 PHY was unstable after clock had been enabled by gpu module. > > Theoretically, 1 millisecond is a critical value for 480MHz > output clock stable time, so we try to change the delay time > to 1.2 millisecond to avoid this issue. > > Signed-off-by: William Wu > --- > drivers/phy/phy-rockchip-inno-usb2.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/phy/phy-rockchip-inno-usb2.c b/drivers/phy/phy-rockchip-inno-usb2.c > index ecfd7d1..8f2d2b6 100644 > --- a/drivers/phy/phy-rockchip-inno-usb2.c > +++ b/drivers/phy/phy-rockchip-inno-usb2.c > @@ -267,7 +267,7 @@ static int rockchip_usb2phy_clk480m_enable(struct clk_hw *hw) > return ret; > > /* waitting for the clk become stable */ > - mdelay(1); > + udelay(1200); Several people who have seen this patch have expressed concern that a 1.2 ms delay is pretty long for something that's supposed to be "atomic" like a clk_enable(). Consider that someone might call clk_enable() while interrupts are disabled and that a 1.2 ms interrupt latency is not so great. It seems like this clock should be moved to be enabled in "prepare" and the "enable" should be a no-op. This is a functionality change, but I don't think there are any real users for this clock at the moment so it should be fine. (of course, the 1 ms latency that existed before this patch was still pretty bad, but ...) -Doug