| Message ID | 1611128871-5898-1-git-send-email-tdas@codeaurora.org |
|---|---|
| State | Accepted |
| Commit | d79dfa19ca4235c28be62952bda6091babdcf8f3 |
| Headers | show |
| Series | [V1] clk: qcom: gcc-sc7180: Mark the MM XO clocks to be always ON | expand |
Hi Bjorn, On 1/21/2021 4:10 AM, Bjorn Andersson wrote: > On Wed 20 Jan 01:47 CST 2021, Taniya Das wrote: > >> There are intermittent GDSC power-up failures observed for titan top >> gdsc, which requires the XO clock. Thus mark all the MM XO clocks always >> enabled from probe. >> > > But if this is the reason for keeping all these {ahb,xo}_clks critical > (or upstream just a bunch of hard coded regmap_update_bits()) why don't > we properly describe them as dependencies for the clock controller/gdsc? > I.e. by the use of pm_clk_add()? > > Regards, > Bjorn They are already defined in the camcc driver, but they are not working as expected, thus I am forced to mark them always ON. > >> Fixes: 8d4025943e13 ("clk: qcom: camcc-sc7180: Use runtime PM ops instead of clk ones") >> Signed-off-by: Taniya Das <tdas@codeaurora.org> >> --- >> drivers/clk/qcom/gcc-sc7180.c | 47 ++++--------------------------------------- >> 1 file changed, 4 insertions(+), 43 deletions(-) >> >> diff --git a/drivers/clk/qcom/gcc-sc7180.c b/drivers/clk/qcom/gcc-sc7180.c >> index b05901b..88e896a 100644 >> --- a/drivers/clk/qcom/gcc-sc7180.c >> +++ b/drivers/clk/qcom/gcc-sc7180.c >> @@ -1,6 +1,6 @@ >> // SPDX-License-Identifier: GPL-2.0-only >> /* >> - * Copyright (c) 2019-2020, The Linux Foundation. All rights reserved. >> + * Copyright (c) 2019-2021, The Linux Foundation. All rights reserved. >> */ >> >> #include <linux/clk-provider.h> >> @@ -919,19 +919,6 @@ static struct clk_branch gcc_camera_throttle_hf_axi_clk = { >> }, >> }; >> >> -static struct clk_branch gcc_camera_xo_clk = { >> - .halt_reg = 0xb02c, >> - .halt_check = BRANCH_HALT, >> - .clkr = { >> - .enable_reg = 0xb02c, >> - .enable_mask = BIT(0), >> - .hw.init = &(struct clk_init_data){ >> - .name = "gcc_camera_xo_clk", >> - .ops = &clk_branch2_ops, >> - }, >> - }, >> -}; >> - >> static struct clk_branch gcc_ce1_ahb_clk = { >> .halt_reg = 0x4100c, >> .halt_check = BRANCH_HALT_VOTED, >> @@ -1096,19 +1083,6 @@ static struct clk_branch gcc_disp_throttle_hf_axi_clk = { >> }, >> }; >> >> -static struct clk_branch gcc_disp_xo_clk = { >> - .halt_reg = 0xb030, >> - .halt_check = BRANCH_HALT, >> - .clkr = { >> - .enable_reg = 0xb030, >> - .enable_mask = BIT(0), >> - .hw.init = &(struct clk_init_data){ >> - .name = "gcc_disp_xo_clk", >> - .ops = &clk_branch2_ops, >> - }, >> - }, >> -}; >> - >> static struct clk_branch gcc_gp1_clk = { >> .halt_reg = 0x64000, >> .halt_check = BRANCH_HALT, >> @@ -2159,19 +2133,6 @@ static struct clk_branch gcc_video_throttle_axi_clk = { >> }, >> }; >> >> -static struct clk_branch gcc_video_xo_clk = { >> - .halt_reg = 0xb028, >> - .halt_check = BRANCH_HALT, >> - .clkr = { >> - .enable_reg = 0xb028, >> - .enable_mask = BIT(0), >> - .hw.init = &(struct clk_init_data){ >> - .name = "gcc_video_xo_clk", >> - .ops = &clk_branch2_ops, >> - }, >> - }, >> -}; >> - >> static struct clk_branch gcc_mss_cfg_ahb_clk = { >> .halt_reg = 0x8a000, >> .halt_check = BRANCH_HALT, >> @@ -2304,7 +2265,6 @@ static struct clk_regmap *gcc_sc7180_clocks[] = { >> [GCC_BOOT_ROM_AHB_CLK] = &gcc_boot_rom_ahb_clk.clkr, >> [GCC_CAMERA_HF_AXI_CLK] = &gcc_camera_hf_axi_clk.clkr, >> [GCC_CAMERA_THROTTLE_HF_AXI_CLK] = &gcc_camera_throttle_hf_axi_clk.clkr, >> - [GCC_CAMERA_XO_CLK] = &gcc_camera_xo_clk.clkr, >> [GCC_CE1_AHB_CLK] = &gcc_ce1_ahb_clk.clkr, >> [GCC_CE1_AXI_CLK] = &gcc_ce1_axi_clk.clkr, >> [GCC_CE1_CLK] = &gcc_ce1_clk.clkr, >> @@ -2317,7 +2277,6 @@ static struct clk_regmap *gcc_sc7180_clocks[] = { >> [GCC_DISP_GPLL0_DIV_CLK_SRC] = &gcc_disp_gpll0_div_clk_src.clkr, >> [GCC_DISP_HF_AXI_CLK] = &gcc_disp_hf_axi_clk.clkr, >> [GCC_DISP_THROTTLE_HF_AXI_CLK] = &gcc_disp_throttle_hf_axi_clk.clkr, >> - [GCC_DISP_XO_CLK] = &gcc_disp_xo_clk.clkr, >> [GCC_GP1_CLK] = &gcc_gp1_clk.clkr, >> [GCC_GP1_CLK_SRC] = &gcc_gp1_clk_src.clkr, >> [GCC_GP2_CLK] = &gcc_gp2_clk.clkr, >> @@ -2413,7 +2372,6 @@ static struct clk_regmap *gcc_sc7180_clocks[] = { >> [GCC_VIDEO_AXI_CLK] = &gcc_video_axi_clk.clkr, >> [GCC_VIDEO_GPLL0_DIV_CLK_SRC] = &gcc_video_gpll0_div_clk_src.clkr, >> [GCC_VIDEO_THROTTLE_AXI_CLK] = &gcc_video_throttle_axi_clk.clkr, >> - [GCC_VIDEO_XO_CLK] = &gcc_video_xo_clk.clkr, >> [GPLL0] = &gpll0.clkr, >> [GPLL0_OUT_EVEN] = &gpll0_out_even.clkr, >> [GPLL6] = &gpll6.clkr, >> @@ -2510,6 +2468,9 @@ static int gcc_sc7180_probe(struct platform_device *pdev) >> regmap_update_bits(regmap, 0x0b004, BIT(0), BIT(0)); >> regmap_update_bits(regmap, 0x0b008, BIT(0), BIT(0)); >> regmap_update_bits(regmap, 0x0b00c, BIT(0), BIT(0)); >> + regmap_update_bits(regmap, 0x0b02c, BIT(0), BIT(0)); >> + regmap_update_bits(regmap, 0x0b028, BIT(0), BIT(0)); >> + regmap_update_bits(regmap, 0x0b030, BIT(0), BIT(0)); >> regmap_update_bits(regmap, 0x71004, BIT(0), BIT(0)); >> >> ret = qcom_cc_register_rcg_dfs(regmap, gcc_dfs_clocks, >> -- >> Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member >> of the Code Aurora Forum, hosted by the Linux Foundation. >>
Quoting Taniya Das (2021-01-19 23:47:51) > There are intermittent GDSC power-up failures observed for titan top > gdsc, which requires the XO clock. Thus mark all the MM XO clocks always > enabled from probe. > > Fixes: 8d4025943e13 ("clk: qcom: camcc-sc7180: Use runtime PM ops instead of clk ones") > Signed-off-by: Taniya Das <tdas@codeaurora.org> > --- Applied to clk-next
diff --git a/drivers/clk/qcom/gcc-sc7180.c b/drivers/clk/qcom/gcc-sc7180.c index b05901b..88e896a 100644 --- a/drivers/clk/qcom/gcc-sc7180.c +++ b/drivers/clk/qcom/gcc-sc7180.c @@ -1,6 +1,6 @@ // SPDX-License-Identifier: GPL-2.0-only /* - * Copyright (c) 2019-2020, The Linux Foundation. All rights reserved. + * Copyright (c) 2019-2021, The Linux Foundation. All rights reserved. */ #include <linux/clk-provider.h> @@ -919,19 +919,6 @@ static struct clk_branch gcc_camera_throttle_hf_axi_clk = { }, }; -static struct clk_branch gcc_camera_xo_clk = { - .halt_reg = 0xb02c, - .halt_check = BRANCH_HALT, - .clkr = { - .enable_reg = 0xb02c, - .enable_mask = BIT(0), - .hw.init = &(struct clk_init_data){ - .name = "gcc_camera_xo_clk", - .ops = &clk_branch2_ops, - }, - }, -}; - static struct clk_branch gcc_ce1_ahb_clk = { .halt_reg = 0x4100c, .halt_check = BRANCH_HALT_VOTED, @@ -1096,19 +1083,6 @@ static struct clk_branch gcc_disp_throttle_hf_axi_clk = { }, }; -static struct clk_branch gcc_disp_xo_clk = { - .halt_reg = 0xb030, - .halt_check = BRANCH_HALT, - .clkr = { - .enable_reg = 0xb030, - .enable_mask = BIT(0), - .hw.init = &(struct clk_init_data){ - .name = "gcc_disp_xo_clk", - .ops = &clk_branch2_ops, - }, - }, -}; - static struct clk_branch gcc_gp1_clk = { .halt_reg = 0x64000, .halt_check = BRANCH_HALT, @@ -2159,19 +2133,6 @@ static struct clk_branch gcc_video_throttle_axi_clk = { }, }; -static struct clk_branch gcc_video_xo_clk = { - .halt_reg = 0xb028, - .halt_check = BRANCH_HALT, - .clkr = { - .enable_reg = 0xb028, - .enable_mask = BIT(0), - .hw.init = &(struct clk_init_data){ - .name = "gcc_video_xo_clk", - .ops = &clk_branch2_ops, - }, - }, -}; - static struct clk_branch gcc_mss_cfg_ahb_clk = { .halt_reg = 0x8a000, .halt_check = BRANCH_HALT, @@ -2304,7 +2265,6 @@ static struct clk_regmap *gcc_sc7180_clocks[] = { [GCC_BOOT_ROM_AHB_CLK] = &gcc_boot_rom_ahb_clk.clkr, [GCC_CAMERA_HF_AXI_CLK] = &gcc_camera_hf_axi_clk.clkr, [GCC_CAMERA_THROTTLE_HF_AXI_CLK] = &gcc_camera_throttle_hf_axi_clk.clkr, - [GCC_CAMERA_XO_CLK] = &gcc_camera_xo_clk.clkr, [GCC_CE1_AHB_CLK] = &gcc_ce1_ahb_clk.clkr, [GCC_CE1_AXI_CLK] = &gcc_ce1_axi_clk.clkr, [GCC_CE1_CLK] = &gcc_ce1_clk.clkr, @@ -2317,7 +2277,6 @@ static struct clk_regmap *gcc_sc7180_clocks[] = { [GCC_DISP_GPLL0_DIV_CLK_SRC] = &gcc_disp_gpll0_div_clk_src.clkr, [GCC_DISP_HF_AXI_CLK] = &gcc_disp_hf_axi_clk.clkr, [GCC_DISP_THROTTLE_HF_AXI_CLK] = &gcc_disp_throttle_hf_axi_clk.clkr, - [GCC_DISP_XO_CLK] = &gcc_disp_xo_clk.clkr, [GCC_GP1_CLK] = &gcc_gp1_clk.clkr, [GCC_GP1_CLK_SRC] = &gcc_gp1_clk_src.clkr, [GCC_GP2_CLK] = &gcc_gp2_clk.clkr, @@ -2413,7 +2372,6 @@ static struct clk_regmap *gcc_sc7180_clocks[] = { [GCC_VIDEO_AXI_CLK] = &gcc_video_axi_clk.clkr, [GCC_VIDEO_GPLL0_DIV_CLK_SRC] = &gcc_video_gpll0_div_clk_src.clkr, [GCC_VIDEO_THROTTLE_AXI_CLK] = &gcc_video_throttle_axi_clk.clkr, - [GCC_VIDEO_XO_CLK] = &gcc_video_xo_clk.clkr, [GPLL0] = &gpll0.clkr, [GPLL0_OUT_EVEN] = &gpll0_out_even.clkr, [GPLL6] = &gpll6.clkr, @@ -2510,6 +2468,9 @@ static int gcc_sc7180_probe(struct platform_device *pdev) regmap_update_bits(regmap, 0x0b004, BIT(0), BIT(0)); regmap_update_bits(regmap, 0x0b008, BIT(0), BIT(0)); regmap_update_bits(regmap, 0x0b00c, BIT(0), BIT(0)); + regmap_update_bits(regmap, 0x0b02c, BIT(0), BIT(0)); + regmap_update_bits(regmap, 0x0b028, BIT(0), BIT(0)); + regmap_update_bits(regmap, 0x0b030, BIT(0), BIT(0)); regmap_update_bits(regmap, 0x71004, BIT(0), BIT(0)); ret = qcom_cc_register_rcg_dfs(regmap, gcc_dfs_clocks,
There are intermittent GDSC power-up failures observed for titan top gdsc, which requires the XO clock. Thus mark all the MM XO clocks always enabled from probe. Fixes: 8d4025943e13 ("clk: qcom: camcc-sc7180: Use runtime PM ops instead of clk ones") Signed-off-by: Taniya Das <tdas@codeaurora.org> --- drivers/clk/qcom/gcc-sc7180.c | 47 ++++--------------------------------------- 1 file changed, 4 insertions(+), 43 deletions(-) -- Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member of the Code Aurora Forum, hosted by the Linux Foundation.