From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Boyd Subject: Re: [PATCH v4 2/7] phy: qcom-qmp: Enable pipe_clk before PHY initialization Date: Tue, 10 Apr 2018 11:32:35 -0700 Message-ID: <152338515540.180276.4273344163431724770@swboyd.mtv.corp.google.com> References: <1522321466-21755-1-git-send-email-mgautam@codeaurora.org> <1522321466-21755-3-git-send-email-mgautam@codeaurora.org> <19c66b02-4f8e-902a-9397-21e7690db7b8@codeaurora.org> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8BIT Return-path: In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org To: Doug Anderson , Manu Gautam Cc: Kishon Vijay Abraham I , Rob Herring , Stephen Boyd , LKML , devicetree@vger.kernel.org, Rob Herring , Vivek Gautam , Evan Green , linux-arm-msm@vger.kernel.org, Varadarajan Narayanan , Wei Yongjun , Fengguang Wu List-Id: linux-arm-msm@vger.kernel.org Quoting Doug Anderson (2018-04-10 08:05:27) > On Mon, Apr 9, 2018 at 11:36 PM, Manu Gautam wrote: > > On 3/30/2018 2:24 AM, Doug Anderson wrote: > >> Oh! This is what you did in the previous version of the patch, then you said: > >> > >> "That is still needed as PHY might take some time to generate pipe_clk > >> after its PLL is locked". > >> > >> It's really going to take more than the 200 us that the clock driver > >> is giving it? If so, I'd prefer to increase the amount of time waited > >> in the clock driver, or adding a fixed delay _before_ the clk_enable() > >> so that the 200 us that the clock driver gives it would be enough. > >> > >> I'm just not a fan of ignoring status bits if it can be helped. > > > > I too would want to do that but it is not just about the delay. > > As per QMP PHY hardware designers, pipe_clk should be enabled in GCC > > as first thing in the PHY initialization sequence. Same sequence also has > > been used in downstream phy driver always. > > Changing the sequence might work but I would like to stick to the HPG > > recommendation and avoid any deviation as PHY issues are very hard to > > debug. > > So hardware guys tell you that you're _supposed to_ ignore the clock > ready bit for that clock and just hope it turns on and settles in time > once power comes on for the clock? That doesn't seem ideal. My guess > is that it's a bug in the specification that the QMP PHY hardware > designers gave you. > > Stephen can feel free to override me if he disagrees since he's in > charge of the clock part of this, but IMHO we should get the > specification fixed and turn things on in the order that lets us check > the status bits. > Presumably there's a PLL "enable" bit in the QMP phy init sequence that we can use to enable the PLL output at a bypass rate and then enable the clk in GCC and then resume the PLL enable sequence. That would allow us to make sure the clk is working. Are the branches in GCC ever turned on or off at runtime though? Or do we just turn them on because they're defaulted off out of reset and then leave them on? I ask because it may be easier to never expose these clks in Linux, hit the enable bits in the branches during clk driver probe, and then act like they never exist because we don't really use them.