LKML Archive on lore.kernel.org help / color / mirror / Atom feed
From: Manu Gautam <firstname.lastname@example.org> To: Doug Anderson <email@example.com> Cc: Kishon Vijay Abraham I <firstname.lastname@example.org>, Rob Herring <email@example.com>, Stephen Boyd <firstname.lastname@example.org>, LKML <email@example.com>, firstname.lastname@example.org, Rob Herring <email@example.com>, Vivek Gautam <firstname.lastname@example.org>, Evan Green <email@example.com>, firstname.lastname@example.org, Varadarajan Narayanan <email@example.com>, Wei Yongjun <firstname.lastname@example.org>, Fengguang Wu <email@example.com> Subject: Re: [PATCH v4 2/7] phy: qcom-qmp: Enable pipe_clk before PHY initialization Date: Tue, 10 Apr 2018 12:06:27 +0530 [thread overview] Message-ID: <firstname.lastname@example.org> (raw) In-Reply-To: <CAD=FV=WGEKy98De_zH9rp+g-tE2YuVK2vRxLJrjeeSx=ijBAww@mail.gmail.com> Hi, On 3/30/2018 2:24 AM, Doug Anderson wrote: > Hi, > > On Thu, Mar 29, 2018 at 11:44 AM, Doug Anderson <email@example.com> wrote: >> Hi, >> >> On Thu, Mar 29, 2018 at 4:04 AM, Manu Gautam <firstname.lastname@example.org> wrote: >>> QMP PHY for USB/PCIE requires pipe_clk for locking of >>> retime buffers at the pipe interface. Driver checks for >>> PHY_STATUS without enabling pipe_clk due to which >>> phy_init() fails with initialization timeout. >>> Though pipe_clk is output from PHY (after PLL is programmed >>> during initialization sequence) to GCC clock_ctl and then fed >>> back to PHY but for PHY_STATUS register to reflect successful >>> initialization pipe_clk from GCC must be present. >>> Since, clock driver now ignores status_check for pipe_clk on >>> clk_enable/disable, driver can safely enable/disable pipe_clk >>> from phy_init/exit. >>> >>> Signed-off-by: Manu Gautam <email@example.com> >>> --- >>> drivers/phy/qualcomm/phy-qcom-qmp.c | 22 ++++++++-------------- >>> 1 file changed, 8 insertions(+), 14 deletions(-) >> Overall this looks much better than the previous version. Thanks! :) >> >> I wonder one thing though. You describe the original problem as this: >> >> 1. If you don't turn the clock on in qcom_qmp_phy_init() then the PHY >> never sets the "ready" status. >> >> 2. If you don't have the PHY powered on / out of reset (which happens >> in qcom_qmp_phy_init()) then when you enable/disable the clock it >> doesn't properly update the status. That's why you needed patch #1 in >> this series. >> >> >> I wonder: could you solve the above _without_ needing to use >> BRANCH_HALT_DELAY in the clock driver? Specifically, can you tell me >> what happens if you put the clk_prepare_enable() after you've powered >> on the PHY and taken it out of reset but before you check the status? >> Said another way, put the "clk_prepare_enable(qphy->pipe_clk)" call >> right before the "readl_poll_timeout" of the ready status? >> >> >> If you do that, you'll turn everything on. Then you'll check that the >> clock's status is OK and then that the PHY's status is OK. > 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. -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
next prev parent reply other threads:[~2018-04-10 6:36 UTC|newest] Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-03-29 11:04 [PATCH v4 0/7] phy: qcom: Updates for USB PHYs on SDM845 Manu Gautam 2018-03-29 11:04 ` [PATCH v4 1/7] clk: msm8996-gcc: change halt check for USB/PCIE pipe_clk Manu Gautam 2018-03-29 20:55 ` Doug Anderson 2018-04-05 20:07 ` Stephen Boyd 2018-04-10 6:44 ` Manu Gautam 2018-03-29 11:04 ` [PATCH v4 2/7] phy: qcom-qmp: Enable pipe_clk before PHY initialization Manu Gautam 2018-03-29 17:28 ` Evan Green 2018-03-29 18:44 ` Doug Anderson 2018-03-29 20:54 ` Doug Anderson 2018-04-10 6:36 ` Manu Gautam [this message] 2018-04-10 15:05 ` Doug Anderson 2018-04-10 18:32 ` Stephen Boyd 2018-04-11 15:37 ` Manu Gautam 2018-04-12 20:38 ` Stephen Boyd 2018-04-13 7:00 ` Manu Gautam 2018-03-29 11:04 ` [PATCH v4 3/7] phy: qcom-qusb2: Fix crash if nvmem cell not specified Manu Gautam 2018-03-29 11:04 ` [PATCH v4 4/7] dt-bindings: phy-qcom-qmp: Update bindings for sdm845 Manu Gautam 2018-03-29 19:39 ` Doug Anderson 2018-03-29 11:04 ` [PATCH v4 5/7] phy: qcom-qmp: Add QMP V3 USB3 UNI PHY support " Manu Gautam 2018-03-29 11:04 ` [PATCH v4 6/7] dt-bindings: phy-qcom-usb2: Add support to override tuning values Manu Gautam 2018-03-29 20:38 ` Doug Anderson 2018-04-09 20:18 ` Rob Herring 2018-04-10 7:16 ` Manu Gautam 2018-04-12 20:47 ` Doug Anderson 2018-04-16 1:36 ` Manu Gautam 2018-03-29 11:04 ` [PATCH v4 7/7] phy: qcom-qusb2: Add QUSB2 PHYs support for sdm845 Manu Gautam 2018-03-29 20:38 ` Doug Anderson 2018-04-10 6:55 ` Manu Gautam
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).