diff mbox series

clk: qcom: gcc: Add missing clocks in SM8150

Message ID 20190917091623.3453-1-vkoul@kernel.org
State Superseded
Headers show
Series clk: qcom: gcc: Add missing clocks in SM8150 | expand

Commit Message

Vinod Koul Sept. 17, 2019, 9:16 a.m. UTC 
The initial upstreaming of SM8150 GCC driver missed few clock so add
them up now.

Signed-off-by: Vinod Koul <vkoul@kernel.org>

---
 drivers/clk/qcom/gcc-sm8150.c | 172 ++++++++++++++++++++++++++++++++++
 1 file changed, 172 insertions(+)

-- 
2.20.1

Comments

Vinod Koul Oct. 16, 2019, 12:23 p.m. UTC | #1 
Hi Steve,

Looks like I missed replying to this one, apologies!

On 17-09-19, 09:09, Stephen Boyd wrote:
> Quoting Vinod Koul (2019-09-17 02:16:23)

> > The initial upstreaming of SM8150 GCC driver missed few clock so add

> > them up now.

> > 

> > Signed-off-by: Vinod Koul <vkoul@kernel.org>

> > ---

> 

> Should have some sort of fixes tag?


Not really, the drivers to use these clks are not upstream so we dont
miss it yet

> 

> >  drivers/clk/qcom/gcc-sm8150.c | 172 ++++++++++++++++++++++++++++++++++

> >  1 file changed, 172 insertions(+)

> > 

> > diff --git a/drivers/clk/qcom/gcc-sm8150.c b/drivers/clk/qcom/gcc-sm8150.c

> > index 12ca2d14797f..13d4d14a5744 100644

> > --- a/drivers/clk/qcom/gcc-sm8150.c

> > +++ b/drivers/clk/qcom/gcc-sm8150.c

> > @@ -1616,6 +1616,38 @@ static struct clk_branch gcc_gpu_cfg_ahb_clk = {

> >         },

> >  };

> >  

> > +static struct clk_branch gcc_gpu_gpll0_clk_src = {

> > +       .halt_check = BRANCH_HALT_SKIP,

> 

> Why skip?


I will explore and add comments for that

> > +       .clkr = {

> > +               .enable_reg = 0x52004,

> > +               .enable_mask = BIT(15),

> > +               .hw.init = &(struct clk_init_data){

> > +                       .name = "gcc_gpu_gpll0_clk_src",

> > +                       .parent_hws = (const struct clk_hw *[]){

> > +                               &gpll0.clkr.hw },

> > +                       .num_parents = 1,

> > +                       .flags = CLK_SET_RATE_PARENT,

> > +                       .ops = &clk_branch2_ops,

> > +               },

> > +       },

> > +};

> > +

> > +static struct clk_branch gcc_gpu_gpll0_div_clk_src = {

> > +       .halt_check = BRANCH_HALT_SKIP,

> 

> Why skip?

> 

> > +       .clkr = {

> > +               .enable_reg = 0x52004,

> > +               .enable_mask = BIT(16),

> > +               .hw.init = &(struct clk_init_data){

> > +                       .name = "gcc_gpu_gpll0_div_clk_src",

> > +                       .parent_hws = (const struct clk_hw *[]){

> > +                               &gcc_gpu_gpll0_clk_src.clkr.hw },

> > +                       .num_parents = 1,

> > +                       .flags = CLK_SET_RATE_PARENT,

> > +                       .ops = &clk_branch2_ops,

> > +               },

> > +       },

> > +};

> > +

> >  static struct clk_branch gcc_gpu_iref_clk = {

> >         .halt_reg = 0x8c010,

> >         .halt_check = BRANCH_HALT,

> > @@ -1698,6 +1730,38 @@ static struct clk_branch gcc_npu_cfg_ahb_clk = {

> >         },

> >  };

> >  

> > +static struct clk_branch gcc_npu_gpll0_clk_src = {

> > +       .halt_check = BRANCH_HALT_SKIP,

> > +       .clkr = {

> > +               .enable_reg = 0x52004,

> > +               .enable_mask = BIT(18),

> > +               .hw.init = &(struct clk_init_data){

> > +                       .name = "gcc_npu_gpll0_clk_src",

> > +                       .parent_hws = (const struct clk_hw *[]){

> > +                               &gpll0.clkr.hw },

> > +                       .num_parents = 1,

> > +                       .flags = CLK_SET_RATE_PARENT,

> > +                       .ops = &clk_branch2_ops,

> > +               },

> > +       },

> > +};

> > +

> > +static struct clk_branch gcc_npu_gpll0_div_clk_src = {

> > +       .halt_check = BRANCH_HALT_SKIP,

> > +       .clkr = {

> > +               .enable_reg = 0x52004,

> > +               .enable_mask = BIT(19),

> > +               .hw.init = &(struct clk_init_data){

> > +                       .name = "gcc_npu_gpll0_div_clk_src",

> > +                       .parent_hws = (const struct clk_hw *[]){

> > +                               &gcc_npu_gpll0_clk_src.clkr.hw },

> > +                       .num_parents = 1,

> > +                       .flags = CLK_SET_RATE_PARENT,

> > +                       .ops = &clk_branch2_ops,

> > +               },

> > +       },

> > +};

> > +

> >  static struct clk_branch gcc_npu_trig_clk = {

> >         .halt_reg = 0x4d00c,

> >         .halt_check = BRANCH_VOTED,

> > @@ -2812,6 +2876,42 @@ static struct clk_branch gcc_ufs_card_phy_aux_hw_ctl_clk = {

> >         },

> >  };

> >  

> > +static struct clk_branch gcc_ufs_card_rx_symbol_0_clk = {

> > +       .halt_check = BRANCH_HALT_SKIP,

> 

> Can't we fix the UFS driver to not require this anymore? This is the

> fourth or fifth time I've asked for this.


yeah Bjorn did tell me that and I think there was some other thread on
similar lines. So is this fine by you.

-- 
~Vinod
Stephen Boyd Oct. 27, 2019, 9:25 p.m. UTC | #2 
Quoting Vinod Koul (2019-10-21 03:54:35)
> On 17-10-19, 10:48, Stephen Boyd wrote:

> > > > > diff --git a/drivers/clk/qcom/gcc-sm8150.c b/drivers/clk/qcom/gcc-sm8150.c

> > > > > index 12ca2d14797f..13d4d14a5744 100644

> > > > > --- a/drivers/clk/qcom/gcc-sm8150.c

> > > > > +++ b/drivers/clk/qcom/gcc-sm8150.c

> > > > > @@ -1616,6 +1616,38 @@ static struct clk_branch gcc_gpu_cfg_ahb_clk = {

> > > > >         },

> > > > >  };

> > > > >  

> > > > > +static struct clk_branch gcc_gpu_gpll0_clk_src = {

> > > > > +       .halt_check = BRANCH_HALT_SKIP,

> > > > 

> > > > Why skip?

> > > 

> > > I will explore and add comments for that

> > > 

> > > > > +       .clkr = {

> > > > > +               .enable_reg = 0x52004,

> > > > > +               .enable_mask = BIT(15),

> > > > > +               .hw.init = &(struct clk_init_data){

> > > > > +                       .name = "gcc_gpu_gpll0_clk_src",

> > > > > +                       .parent_hws = (const struct clk_hw *[]){

> > > > > +                               &gpll0.clkr.hw },

> > > > > +                       .num_parents = 1,

> > > > > +                       .flags = CLK_SET_RATE_PARENT,

> > > > > +                       .ops = &clk_branch2_ops,

> > > > > +               },

> > > > > +       },

> > > > > +};

> > > > > +

> > > > > +static struct clk_branch gcc_gpu_gpll0_div_clk_src = {

> > > > > +       .halt_check = BRANCH_HALT_SKIP,

> > > > 

> > > > Why skip?

> > > > 

> > 

> > Any answer from the explorations?

> 

> Yeah so asking around the answer I got is that these are external

> clocks and we need cannot rely on CLK_OFF bit for these clocks

> 


The parents are from some other clk controller? Not external to the
chip, right? If so, I still don't get it. Please add some sort of
comment here in the code.
Vinod Koul Oct. 28, 2019, 7:23 a.m. UTC | #3 
On 27-10-19, 14:25, Stephen Boyd wrote:
> Quoting Vinod Koul (2019-10-21 03:54:35)

> > On 17-10-19, 10:48, Stephen Boyd wrote:

> > > > > > diff --git a/drivers/clk/qcom/gcc-sm8150.c b/drivers/clk/qcom/gcc-sm8150.c

> > > > > > index 12ca2d14797f..13d4d14a5744 100644

> > > > > > --- a/drivers/clk/qcom/gcc-sm8150.c

> > > > > > +++ b/drivers/clk/qcom/gcc-sm8150.c

> > > > > > @@ -1616,6 +1616,38 @@ static struct clk_branch gcc_gpu_cfg_ahb_clk = {

> > > > > >         },

> > > > > >  };

> > > > > >  

> > > > > > +static struct clk_branch gcc_gpu_gpll0_clk_src = {

> > > > > > +       .halt_check = BRANCH_HALT_SKIP,

> > > > > 

> > > > > Why skip?

> > > > 

> > > > I will explore and add comments for that

> > > > 

> > > > > > +       .clkr = {

> > > > > > +               .enable_reg = 0x52004,

> > > > > > +               .enable_mask = BIT(15),

> > > > > > +               .hw.init = &(struct clk_init_data){

> > > > > > +                       .name = "gcc_gpu_gpll0_clk_src",

> > > > > > +                       .parent_hws = (const struct clk_hw *[]){

> > > > > > +                               &gpll0.clkr.hw },

> > > > > > +                       .num_parents = 1,

> > > > > > +                       .flags = CLK_SET_RATE_PARENT,

> > > > > > +                       .ops = &clk_branch2_ops,

> > > > > > +               },

> > > > > > +       },

> > > > > > +};

> > > > > > +

> > > > > > +static struct clk_branch gcc_gpu_gpll0_div_clk_src = {

> > > > > > +       .halt_check = BRANCH_HALT_SKIP,

> > > > > 

> > > > > Why skip?

> > > > > 

> > > 

> > > Any answer from the explorations?

> > 

> > Yeah so asking around the answer I got is that these are external

> > clocks and we need cannot rely on CLK_OFF bit for these clocks

> > 

> 

> The parents are from some other clk controller? Not external to the

> chip, right? If so, I still don't get it. Please add some sort of

> comment here in the code.



Yeah I have added a comment and posted v2 few days back.

Thanks
-- 
~Vinod
diff mbox series

Patch

diff --git a/drivers/clk/qcom/gcc-sm8150.c b/drivers/clk/qcom/gcc-sm8150.c
index 12ca2d14797f..13d4d14a5744 100644
--- a/drivers/clk/qcom/gcc-sm8150.c
+++ b/drivers/clk/qcom/gcc-sm8150.c
@@ -1616,6 +1616,38 @@  static struct clk_branch gcc_gpu_cfg_ahb_clk = {
 	},
 };
 
+static struct clk_branch gcc_gpu_gpll0_clk_src = {
+	.halt_check = BRANCH_HALT_SKIP,
+	.clkr = {
+		.enable_reg = 0x52004,
+		.enable_mask = BIT(15),
+		.hw.init = &(struct clk_init_data){
+			.name = "gcc_gpu_gpll0_clk_src",
+			.parent_hws = (const struct clk_hw *[]){
+				&gpll0.clkr.hw },
+			.num_parents = 1,
+			.flags = CLK_SET_RATE_PARENT,
+			.ops = &clk_branch2_ops,
+		},
+	},
+};
+
+static struct clk_branch gcc_gpu_gpll0_div_clk_src = {
+	.halt_check = BRANCH_HALT_SKIP,
+	.clkr = {
+		.enable_reg = 0x52004,
+		.enable_mask = BIT(16),
+		.hw.init = &(struct clk_init_data){
+			.name = "gcc_gpu_gpll0_div_clk_src",
+			.parent_hws = (const struct clk_hw *[]){
+				&gcc_gpu_gpll0_clk_src.clkr.hw },
+			.num_parents = 1,
+			.flags = CLK_SET_RATE_PARENT,
+			.ops = &clk_branch2_ops,
+		},
+	},
+};
+
 static struct clk_branch gcc_gpu_iref_clk = {
 	.halt_reg = 0x8c010,
 	.halt_check = BRANCH_HALT,
@@ -1698,6 +1730,38 @@  static struct clk_branch gcc_npu_cfg_ahb_clk = {
 	},
 };
 
+static struct clk_branch gcc_npu_gpll0_clk_src = {
+	.halt_check = BRANCH_HALT_SKIP,
+	.clkr = {
+		.enable_reg = 0x52004,
+		.enable_mask = BIT(18),
+		.hw.init = &(struct clk_init_data){
+			.name = "gcc_npu_gpll0_clk_src",
+			.parent_hws = (const struct clk_hw *[]){
+				&gpll0.clkr.hw },
+			.num_parents = 1,
+			.flags = CLK_SET_RATE_PARENT,
+			.ops = &clk_branch2_ops,
+		},
+	},
+};
+
+static struct clk_branch gcc_npu_gpll0_div_clk_src = {
+	.halt_check = BRANCH_HALT_SKIP,
+	.clkr = {
+		.enable_reg = 0x52004,
+		.enable_mask = BIT(19),
+		.hw.init = &(struct clk_init_data){
+			.name = "gcc_npu_gpll0_div_clk_src",
+			.parent_hws = (const struct clk_hw *[]){
+				&gcc_npu_gpll0_clk_src.clkr.hw },
+			.num_parents = 1,
+			.flags = CLK_SET_RATE_PARENT,
+			.ops = &clk_branch2_ops,
+		},
+	},
+};
+
 static struct clk_branch gcc_npu_trig_clk = {
 	.halt_reg = 0x4d00c,
 	.halt_check = BRANCH_VOTED,
@@ -2812,6 +2876,42 @@  static struct clk_branch gcc_ufs_card_phy_aux_hw_ctl_clk = {
 	},
 };
 
+static struct clk_branch gcc_ufs_card_rx_symbol_0_clk = {
+	.halt_check = BRANCH_HALT_SKIP,
+	.clkr = {
+		.enable_reg = 0x7501c,
+		.enable_mask = BIT(0),
+		.hw.init = &(struct clk_init_data){
+			.name = "gcc_ufs_card_rx_symbol_0_clk",
+			.ops = &clk_branch2_ops,
+		},
+	},
+};
+
+static struct clk_branch gcc_ufs_card_rx_symbol_1_clk = {
+	.halt_check = BRANCH_HALT_SKIP,
+	.clkr = {
+		.enable_reg = 0x750ac,
+		.enable_mask = BIT(0),
+		.hw.init = &(struct clk_init_data){
+			.name = "gcc_ufs_card_rx_symbol_1_clk",
+			.ops = &clk_branch2_ops,
+		},
+	},
+};
+
+static struct clk_branch gcc_ufs_card_tx_symbol_0_clk = {
+	.halt_check = BRANCH_HALT_SKIP,
+	.clkr = {
+		.enable_reg = 0x75018,
+		.enable_mask = BIT(0),
+		.hw.init = &(struct clk_init_data){
+			.name = "gcc_ufs_card_tx_symbol_0_clk",
+			.ops = &clk_branch2_ops,
+		},
+	},
+};
+
 static struct clk_branch gcc_ufs_card_unipro_core_clk = {
 	.halt_reg = 0x75058,
 	.halt_check = BRANCH_HALT,
@@ -2992,6 +3092,42 @@  static struct clk_branch gcc_ufs_phy_phy_aux_hw_ctl_clk = {
 	},
 };
 
+static struct clk_branch gcc_ufs_phy_rx_symbol_0_clk = {
+	.halt_check = BRANCH_HALT_SKIP,
+	.clkr = {
+		.enable_reg = 0x7701c,
+		.enable_mask = BIT(0),
+		.hw.init = &(struct clk_init_data){
+			.name = "gcc_ufs_phy_rx_symbol_0_clk",
+			.ops = &clk_branch2_ops,
+		},
+	},
+};
+
+static struct clk_branch gcc_ufs_phy_rx_symbol_1_clk = {
+	.halt_check = BRANCH_HALT_SKIP,
+	.clkr = {
+		.enable_reg = 0x770ac,
+		.enable_mask = BIT(0),
+		.hw.init = &(struct clk_init_data){
+			.name = "gcc_ufs_phy_rx_symbol_1_clk",
+			.ops = &clk_branch2_ops,
+		},
+	},
+};
+
+static struct clk_branch gcc_ufs_phy_tx_symbol_0_clk = {
+	.halt_check = BRANCH_HALT_SKIP,
+	.clkr = {
+		.enable_reg = 0x77018,
+		.enable_mask = BIT(0),
+		.hw.init = &(struct clk_init_data){
+			.name = "gcc_ufs_phy_tx_symbol_0_clk",
+			.ops = &clk_branch2_ops,
+		},
+	},
+};
+
 static struct clk_branch gcc_ufs_phy_unipro_core_clk = {
 	.halt_reg = 0x77058,
 	.halt_check = BRANCH_HALT,
@@ -3171,6 +3307,18 @@  static struct clk_branch gcc_usb3_prim_phy_com_aux_clk = {
 	},
 };
 
+static struct clk_branch gcc_usb3_prim_phy_pipe_clk = {
+	.halt_check = BRANCH_HALT_SKIP,
+	.clkr = {
+		.enable_reg = 0xf058,
+		.enable_mask = BIT(0),
+		.hw.init = &(struct clk_init_data){
+			.name = "gcc_usb3_prim_phy_pipe_clk",
+			.ops = &clk_branch2_ops,
+		},
+	},
+};
+
 static struct clk_branch gcc_usb3_sec_clkref_clk = {
 	.halt_reg = 0x8c028,
 	.halt_check = BRANCH_HALT,
@@ -3201,6 +3349,18 @@  static struct clk_branch gcc_usb3_sec_phy_aux_clk = {
 	},
 };
 
+static struct clk_branch gcc_usb3_sec_phy_pipe_clk = {
+	.halt_check = BRANCH_HALT_SKIP,
+	.clkr = {
+		.enable_reg = 0x10058,
+		.enable_mask = BIT(0),
+		.hw.init = &(struct clk_init_data){
+			.name = "gcc_usb3_sec_phy_pipe_clk",
+			.ops = &clk_branch2_ops,
+		},
+	},
+};
+
 static struct clk_branch gcc_usb3_sec_phy_com_aux_clk = {
 	.halt_reg = 0x10054,
 	.halt_check = BRANCH_HALT,
@@ -3332,12 +3492,16 @@  static struct clk_regmap *gcc_sm8150_clocks[] = {
 	[GCC_GP3_CLK] = &gcc_gp3_clk.clkr,
 	[GCC_GP3_CLK_SRC] = &gcc_gp3_clk_src.clkr,
 	[GCC_GPU_CFG_AHB_CLK] = &gcc_gpu_cfg_ahb_clk.clkr,
+	[GCC_GPU_GPLL0_CLK_SRC] = &gcc_gpu_gpll0_clk_src.clkr,
+	[GCC_GPU_GPLL0_DIV_CLK_SRC] = &gcc_gpu_gpll0_div_clk_src.clkr,
 	[GCC_GPU_IREF_CLK] = &gcc_gpu_iref_clk.clkr,
 	[GCC_GPU_MEMNOC_GFX_CLK] = &gcc_gpu_memnoc_gfx_clk.clkr,
 	[GCC_GPU_SNOC_DVM_GFX_CLK] = &gcc_gpu_snoc_dvm_gfx_clk.clkr,
 	[GCC_NPU_AT_CLK] = &gcc_npu_at_clk.clkr,
 	[GCC_NPU_AXI_CLK] = &gcc_npu_axi_clk.clkr,
 	[GCC_NPU_CFG_AHB_CLK] = &gcc_npu_cfg_ahb_clk.clkr,
+	[GCC_NPU_GPLL0_CLK_SRC] = &gcc_npu_gpll0_clk_src.clkr,
+	[GCC_NPU_GPLL0_DIV_CLK_SRC] = &gcc_npu_gpll0_div_clk_src.clkr,
 	[GCC_NPU_TRIG_CLK] = &gcc_npu_trig_clk.clkr,
 	[GCC_PCIE0_PHY_REFGEN_CLK] = &gcc_pcie0_phy_refgen_clk.clkr,
 	[GCC_PCIE1_PHY_REFGEN_CLK] = &gcc_pcie1_phy_refgen_clk.clkr,
@@ -3442,6 +3606,9 @@  static struct clk_regmap *gcc_sm8150_clocks[] = {
 	[GCC_UFS_CARD_PHY_AUX_CLK_SRC] = &gcc_ufs_card_phy_aux_clk_src.clkr,
 	[GCC_UFS_CARD_PHY_AUX_HW_CTL_CLK] =
 		&gcc_ufs_card_phy_aux_hw_ctl_clk.clkr,
+	[GCC_UFS_CARD_RX_SYMBOL_0_CLK] = &gcc_ufs_card_rx_symbol_0_clk.clkr,
+	[GCC_UFS_CARD_RX_SYMBOL_1_CLK] = &gcc_ufs_card_rx_symbol_1_clk.clkr,
+	[GCC_UFS_CARD_TX_SYMBOL_0_CLK] = &gcc_ufs_card_tx_symbol_0_clk.clkr,
 	[GCC_UFS_CARD_UNIPRO_CORE_CLK] = &gcc_ufs_card_unipro_core_clk.clkr,
 	[GCC_UFS_CARD_UNIPRO_CORE_CLK_SRC] =
 		&gcc_ufs_card_unipro_core_clk_src.clkr,
@@ -3459,6 +3626,9 @@  static struct clk_regmap *gcc_sm8150_clocks[] = {
 	[GCC_UFS_PHY_PHY_AUX_CLK] = &gcc_ufs_phy_phy_aux_clk.clkr,
 	[GCC_UFS_PHY_PHY_AUX_CLK_SRC] = &gcc_ufs_phy_phy_aux_clk_src.clkr,
 	[GCC_UFS_PHY_PHY_AUX_HW_CTL_CLK] = &gcc_ufs_phy_phy_aux_hw_ctl_clk.clkr,
+	[GCC_UFS_PHY_RX_SYMBOL_0_CLK] = &gcc_ufs_phy_rx_symbol_0_clk.clkr,
+	[GCC_UFS_PHY_RX_SYMBOL_1_CLK] = &gcc_ufs_phy_rx_symbol_1_clk.clkr,
+	[GCC_UFS_PHY_TX_SYMBOL_0_CLK] = &gcc_ufs_phy_tx_symbol_0_clk.clkr,
 	[GCC_UFS_PHY_UNIPRO_CORE_CLK] = &gcc_ufs_phy_unipro_core_clk.clkr,
 	[GCC_UFS_PHY_UNIPRO_CORE_CLK_SRC] =
 		&gcc_ufs_phy_unipro_core_clk_src.clkr,
@@ -3480,10 +3650,12 @@  static struct clk_regmap *gcc_sm8150_clocks[] = {
 	[GCC_USB3_PRIM_PHY_AUX_CLK] = &gcc_usb3_prim_phy_aux_clk.clkr,
 	[GCC_USB3_PRIM_PHY_AUX_CLK_SRC] = &gcc_usb3_prim_phy_aux_clk_src.clkr,
 	[GCC_USB3_PRIM_PHY_COM_AUX_CLK] = &gcc_usb3_prim_phy_com_aux_clk.clkr,
+	[GCC_USB3_PRIM_PHY_PIPE_CLK] = &gcc_usb3_prim_phy_pipe_clk.clkr,
 	[GCC_USB3_SEC_CLKREF_CLK] = &gcc_usb3_sec_clkref_clk.clkr,
 	[GCC_USB3_SEC_PHY_AUX_CLK] = &gcc_usb3_sec_phy_aux_clk.clkr,
 	[GCC_USB3_SEC_PHY_AUX_CLK_SRC] = &gcc_usb3_sec_phy_aux_clk_src.clkr,
 	[GCC_USB3_SEC_PHY_COM_AUX_CLK] = &gcc_usb3_sec_phy_com_aux_clk.clkr,
+	[GCC_USB3_SEC_PHY_PIPE_CLK] = &gcc_usb3_sec_phy_pipe_clk.clkr,
 	[GCC_VIDEO_AHB_CLK] = &gcc_video_ahb_clk.clkr,
 	[GCC_VIDEO_AXI0_CLK] = &gcc_video_axi0_clk.clkr,
 	[GCC_VIDEO_AXI1_CLK] = &gcc_video_axi1_clk.clkr,