Re: [PATCH v1] clk: qcom: Skip halt checks on gcc_pcie_0_pipe_clk for 8998

[Manu Gautam <mgautam@codeaurora.org>]

Hi Steve,

On 3/30/2019 4:02 AM, Stephen Boyd wrote:
> Quoting Marc Gonzalez (2019-03-28 09:26:59)
>> On 25/03/2019 14:49, Marc Gonzalez wrote:
>>
>>> See similar issue solved by commit 5f2420ed2189
>>> ("clk: qcom: Skip halt checks on gcc_usb3_phy_pipe_clk for 8998")
>>>
>>> Without this patch, PCIe PHY init fails:
>>>
>>> qcom-qmp-phy 1c06000.phy: pipe_clk enable failed err=-16
>>> phy phy-1c06000.phy.0: phy init failed --> -16
>>>
>>> Signed-off-by: Marc Gonzalez <marc.w.gonzalez@free.fr>
>>> ---
>>>  drivers/clk/qcom/gcc-msm8998.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/clk/qcom/gcc-msm8998.c b/drivers/clk/qcom/gcc-msm8998.c
>>> index c240fba794c7..033688264c7b 100644
>>> --- a/drivers/clk/qcom/gcc-msm8998.c
>>> +++ b/drivers/clk/qcom/gcc-msm8998.c
>>> @@ -2161,7 +2161,7 @@ static struct clk_branch gcc_pcie_0_mstr_axi_clk = {
>>>  
>>>  static struct clk_branch gcc_pcie_0_pipe_clk = {
>>>       .halt_reg = 0x6b018,
>>> -     .halt_check = BRANCH_HALT,
>>> +     .halt_check = BRANCH_HALT_SKIP,
>>>       .clkr = {
>>>               .enable_reg = 0x6b018,
>>>               .enable_mask = BIT(0),
>> Actually, 5f2420ed2189 is not the only similar instance.
>>
>> $ git log --oneline -G BRANCH_HALT_SKIP drivers/clk/qcom | grep -v controller
>> 924a86bf9793 clk: qcom: Skip halt checks on gcc_pcie_0_pipe_clk for 8998
>> 5f2420ed2189 clk: qcom: Skip halt checks on gcc_usb3_phy_pipe_clk for 8998
>> 2abf856202fd clk: qcom: gcc-msm8998: Disable halt check of UFS clocks
>> 5f75b78d3d67 clk: qcom: gcc-msm8996: Disable halt check on UFS tx clock
>> 12d807cd34b8 clk: qcom: gcc-msm8996: Disable halt check on UFS clocks
>> 096abdc296f1 clk: msm8996-gcc: Mark halt check as no-op for USB/PCIE pipe_clk
>> 7d99ced8f4c6 clk: qcom: Add support for BRANCH_HALT_SKIP flag for branch clocks
>>
> I keep asking Qualcomm engineers everytime this comes up why they can't
> fix their phy initialization sequence.
>
> Too bad they don't care anymore!


I have followed this up with QMP PHY hardware designers and they have
confirmed that QMP PHY must have pipe clock enabled at the beginning
of initialization sequence i.e. before bringing it out of reset and starting it.

Otherwise there is possibility of incorrect locking of pipe_interface/
retime buffers in PHY.
Hence, for both USB and PCIe we have to continue to use HALT_SKIP flag.

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