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
> Fixes: 8d4025943e13 ("clk: qcom: camcc-sc7180: Use runtime PM ops instead of clk ones")
> Signed-off-by: Taniya Das <tdas@xxxxxxxxxxxxxx>
> ---
> 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.
>
