linux-kernel - Re: [PATCH v3 1/6] phy: qcom-qmp: Enable pipe_clk before checking USB3 PHY

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 27 Mar 2018 10:37:08 +0530
From:   Manu Gautam <mgautam@...eaurora.org>
To:     Doug Anderson <dianders@...omium.org>
Cc:     Kishon Vijay Abraham I <kishon@...com>,
        LKML <linux-kernel@...r.kernel.org>, devicetree@...r.kernel.org,
        Rob Herring <robh+dt@...nel.org>,
        linux-arm-msm@...r.kernel.org,
        Vivek Gautam <vivek.gautam@...eaurora.org>,
        Varadarajan Narayanan <varada@...eaurora.org>,
        Viresh Kumar <viresh.kumar@...aro.org>,
        Wei Yongjun <weiyongjun1@...wei.com>,
        Fengguang Wu <fengguang.wu@...el.com>,
        Vivek Gautam <vivek.gautam@...eaurora.org>,
        anischal@...eaurora.org
Subject: Re: [PATCH v3 1/6] phy: qcom-qmp: Enable pipe_clk before checking
 USB3 PHY_STATUS

Hi Doug,


On 3/27/2018 9:56 AM, Doug Anderson wrote:
> Manu
>
> On Thu, Mar 22, 2018 at 11:11 PM, Manu Gautam <mgautam@...eaurora.org> wrote:
>> QMP PHY for USB mode requires pipe_clk for calibration and PLL lock
>> to take place. This clock is output from PHY to GCC clock_ctl and then
>> fed back to QMP PHY and is available from PHY only after PHY is reset
>> and initialized, hence it can't be enabled too early in initialization
>> sequence.
>>
>> Signed-off-by: Manu Gautam <mgautam@...eaurora.org>
>> ---
>>  drivers/phy/qualcomm/phy-qcom-qmp.c | 33 ++++++++++++++++++++++++++++++++-
>>  1 file changed, 32 insertions(+), 1 deletion(-)
> So it's now new with this patch, but it's more obvious with this
> patch.  It seems like "UFS/PCIE" is kinda broken w/ respect to how it
> controls its clock.  Specifically:
>
> * If you init the PHY but don't power it on, then you "exit" the PHY:
> you'll disable/unprepare "pipe_clk" even though you never
> prepare/enabled it.
>
> * If you init the PHY, power it on, power it off, power it on, and
> exit the PHY: you'll leave the clock prepared one extra time.
>
> Specifically I'd expect: for UFS/PCIE the disable/unprepare should be
> symmetric with the enable/prepare and should be in "power off", not in
> exit.
>
> ...or did I miss something?
>
>
> Interestingly, your patch fixes this problem for USB3 (where init/exit
> are now symmetric), but leaves the problem there for UFS/PCIE.
>

Thanks for review.
One of the reason why pipe_clk is disabled as part of phy_exit is that
halt_check from clk_disable reports error if called after PHY has been
powered down or phy_exit.
I believe that warning should be ignored in qcom gcc-clock driver
(for applicable platforms) by using BRANCH_HALT_DELAY as halt_check
for pipe_clk and performing clk_disable from power_off for UFS/PCIE.

I can implement that as separate patch once dependent gcc driver
patch(es) gets in. Would that be ok?

-Manu

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project