| Message ID | 1522321466-21755-2-git-send-email-mgautam@codeaurora.org (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
Hi, On Thu, Mar 29, 2018 at 4:04 AM, Manu Gautam <mgautam@codeaurora.org> wrote: > The USB and PCIE pipe clocks are sourced from external clocks > inside the QMP USB/PCIE PHYs. Enabling or disabling of PIPE RCG > clocks is dependent on PHY initialization sequence hence > update halt_check to BRANCH_HALT_DELAY for these clocks so > that clock status bit is not polled when enabling or disabling > the clocks. It allows to simplify PHY client driver code which > is both user and source of the pipe_clk and avoid error logging > related status check on clk_disable/enable. > > Signed-off-by: Manu Gautam <mgautam@codeaurora.org> > --- > drivers/clk/qcom/gcc-msm8996.c | 4 ++++ > 1 file changed, 4 insertions(+) As per my feedback on <https://patchwork.kernel.org/patch/10314937/>, I'm not a fan of this. Hopefully we can adjust the PHY driver so it's not needed. -Doug -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoting Doug Anderson (2018-03-29 13:55:55) > Hi, > > On Thu, Mar 29, 2018 at 4:04 AM, Manu Gautam <mgautam@codeaurora.org> wrote: > > The USB and PCIE pipe clocks are sourced from external clocks > > inside the QMP USB/PCIE PHYs. Enabling or disabling of PIPE RCG > > clocks is dependent on PHY initialization sequence hence > > update halt_check to BRANCH_HALT_DELAY for these clocks so > > that clock status bit is not polled when enabling or disabling > > the clocks. It allows to simplify PHY client driver code which > > is both user and source of the pipe_clk and avoid error logging > > related status check on clk_disable/enable. > > > > Signed-off-by: Manu Gautam <mgautam@codeaurora.org> > > --- > > drivers/clk/qcom/gcc-msm8996.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > As per my feedback on <https://patchwork.kernel.org/patch/10314937/>, > I'm not a fan of this. Hopefully we can adjust the PHY driver so it's > not needed. > Agreed. We should be able to enable the clks at the right time and halt bits should work. From what I can recall we had that working before on db820c, so has something changed? -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On 4/6/2018 1:37 AM, Stephen Boyd wrote: > Quoting Doug Anderson (2018-03-29 13:55:55) >> Hi, >> >> On Thu, Mar 29, 2018 at 4:04 AM, Manu Gautam <mgautam@codeaurora.org> wrote: >>> The USB and PCIE pipe clocks are sourced from external clocks >>> inside the QMP USB/PCIE PHYs. Enabling or disabling of PIPE RCG >>> clocks is dependent on PHY initialization sequence hence >>> update halt_check to BRANCH_HALT_DELAY for these clocks so >>> that clock status bit is not polled when enabling or disabling >>> the clocks. It allows to simplify PHY client driver code which >>> is both user and source of the pipe_clk and avoid error logging >>> related status check on clk_disable/enable. >>> >>> Signed-off-by: Manu Gautam <mgautam@codeaurora.org> >>> --- >>> drivers/clk/qcom/gcc-msm8996.c | 4 ++++ >>> 1 file changed, 4 insertions(+) >> As per my feedback on <https://patchwork.kernel.org/patch/10314937/>, >> I'm not a fan of this. Hopefully we can adjust the PHY driver so it's >> not needed. >> > Agreed. We should be able to enable the clks at the right time and halt > bits should work. From what I can recall we had that working before on > db820c, so has something changed? As replied in other patch IMO it is better to stick to the recommended sequence and have this change as it would allow to cleanup PHY driver and align with HPG. One reason I can think of why it works on db820c is that there is some code in bootloader that enables all USB clocks (including pipe_clk). Same is the case with SDM845. And I start seeing errors if bootloader is changed or skip pipe_clk enable.
diff --git a/drivers/clk/qcom/gcc-msm8996.c b/drivers/clk/qcom/gcc-msm8996.c index 5d74512..336d12d 100644 --- a/drivers/clk/qcom/gcc-msm8996.c +++ b/drivers/clk/qcom/gcc-msm8996.c @@ -1418,6 +1418,7 @@ enum { static struct clk_branch gcc_usb3_phy_pipe_clk = { .halt_reg = 0x50004, + .halt_check = BRANCH_HALT_DELAY, .clkr = { .enable_reg = 0x50004, .enable_mask = BIT(0), @@ -2472,6 +2473,7 @@ enum { static struct clk_branch gcc_pcie_0_pipe_clk = { .halt_reg = 0x6b018, + .halt_check = BRANCH_HALT_DELAY, .clkr = { .enable_reg = 0x6b018, .enable_mask = BIT(0), @@ -2547,6 +2549,7 @@ enum { static struct clk_branch gcc_pcie_1_pipe_clk = { .halt_reg = 0x6d018, + .halt_check = BRANCH_HALT_DELAY, .clkr = { .enable_reg = 0x6d018, .enable_mask = BIT(0), @@ -2622,6 +2625,7 @@ enum { static struct clk_branch gcc_pcie_2_pipe_clk = { .halt_reg = 0x6e018, + .halt_check = BRANCH_HALT_DELAY, .clkr = { .enable_reg = 0x6e018, .enable_mask = BIT(0),
The USB and PCIE pipe clocks are sourced from external clocks inside the QMP USB/PCIE PHYs. Enabling or disabling of PIPE RCG clocks is dependent on PHY initialization sequence hence update halt_check to BRANCH_HALT_DELAY for these clocks so that clock status bit is not polled when enabling or disabling the clocks. It allows to simplify PHY client driver code which is both user and source of the pipe_clk and avoid error logging related status check on clk_disable/enable. Signed-off-by: Manu Gautam <mgautam@codeaurora.org> --- drivers/clk/qcom/gcc-msm8996.c | 4 ++++ 1 file changed, 4 insertions(+)