Hi, On 3/27/2018 12:26 PM, Vivek Gautam wrote: > > > On 3/27/2018 10:37 AM, Manu Gautam wrote: >> 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@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxx> >>>> --- >>>> 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. > UFS doesn't use PIPE clock. Yes, UFS PHY doesn't use one. But similar to pipe_clk there are rx/tx symbol_clk output from PHY that is used by UFS controller. I will update code comments to not refer UFS for pipe_clk. > But considering for PCIe, if we disable pipe clock when phy is still running, then > it shouldn't be a problem. We should also not see the halt warning as the gcc > driver should be able to just turn the gate off. > The reason why it will throw that error is when the parent clock to that gate > is gated, i.e. the pipe clock is not flowing on that branch. I got the confirmation that pipe_clk is needed for PCIE as well for its initialization to happen successfully. So we do need clock driver change to fix this in PHY driver. > > Best regards > Vivek > >> >> 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 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
