| Message ID | 20230623164619.11464-5-quic_jkona@quicinc.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | Add camera clock controller support for SM8550 | expand |
On 23.06.2023 18:46, Jagadeesh Kona wrote: > Add support for camera qdss, sleep and xo clocks. > > Co-developed-by: Taniya Das <quic_tdas@quicinc.com> > Signed-off-by: Taniya Das <quic_tdas@quicinc.com> > Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com> > --- > Changes since v4: > - No changes. > Changes since v3: > - No changes. > Changes since v2: > - No changes. > Changes since v1: > - Newly added. > > drivers/clk/qcom/camcc-sm8550.c | 180 ++++++++++++++++++++++++++++++++ > 1 file changed, 180 insertions(+) > > diff --git a/drivers/clk/qcom/camcc-sm8550.c b/drivers/clk/qcom/camcc-sm8550.c > index 075bea32087c..7b4882444d58 100644 > --- a/drivers/clk/qcom/camcc-sm8550.c > +++ b/drivers/clk/qcom/camcc-sm8550.c > @@ -22,6 +22,8 @@ > enum { > DT_IFACE, > DT_BI_TCXO, > + DT_BI_TCXO_AO, > + DT_SLEEP_CLK, > }; > > enum { > @@ -43,6 +45,7 @@ enum { > P_CAM_CC_PLL10_OUT_EVEN, > P_CAM_CC_PLL11_OUT_EVEN, > P_CAM_CC_PLL12_OUT_EVEN, > + P_SLEEP_CLK, > }; > > static const struct pll_vco lucid_ole_vco[] = { > @@ -881,6 +884,22 @@ static const struct clk_parent_data cam_cc_parent_data_11[] = { > { .hw = &cam_cc_pll7_out_even.clkr.hw }, > }; > > +static const struct parent_map cam_cc_parent_map_12[] = { > + { P_SLEEP_CLK, 0 }, > +}; > + > +static const struct clk_parent_data cam_cc_parent_data_12[] = { > + { .index = DT_SLEEP_CLK }, > +}; > + > +static const struct parent_map cam_cc_parent_map_13[] = { > + { P_BI_TCXO, 0 }, > +}; > + > +static const struct clk_parent_data cam_cc_parent_data_13_ao[] = { > + { .index = DT_BI_TCXO_AO }, > +}; > + > static const struct freq_tbl ftbl_cam_cc_bps_clk_src[] = { > F(19200000, P_BI_TCXO, 1, 0, 0), > F(200000000, P_CAM_CC_PLL8_OUT_EVEN, 1, 0, 0), > @@ -1565,6 +1584,29 @@ static struct clk_rcg2 cam_cc_mclk7_clk_src = { > }, > }; > > +static const struct freq_tbl ftbl_cam_cc_qdss_debug_clk_src[] = { > + F(19200000, P_BI_TCXO, 1, 0, 0), > + F(75000000, P_CAM_CC_PLL0_OUT_EVEN, 8, 0, 0), > + F(150000000, P_CAM_CC_PLL0_OUT_EVEN, 4, 0, 0), > + F(300000000, P_CAM_CC_PLL0_OUT_MAIN, 4, 0, 0), > + { } > +}; > + > +static struct clk_rcg2 cam_cc_qdss_debug_clk_src = { > + .cmd_rcgr = 0x13f24, > + .mnd_width = 0, > + .hid_width = 5, > + .parent_map = cam_cc_parent_map_0, > + .freq_tbl = ftbl_cam_cc_qdss_debug_clk_src, > + .clkr.hw.init = &(const struct clk_init_data) { > + .name = "cam_cc_qdss_debug_clk_src", > + .parent_data = cam_cc_parent_data_0, > + .num_parents = ARRAY_SIZE(cam_cc_parent_data_0), > + .flags = CLK_SET_RATE_PARENT, > + .ops = &clk_rcg2_shared_ops, > + }, > +}; > + > static const struct freq_tbl ftbl_cam_cc_sfe_0_clk_src[] = { > F(466000000, P_CAM_CC_PLL6_OUT_EVEN, 1, 0, 0), > F(594000000, P_CAM_CC_PLL6_OUT_EVEN, 1, 0, 0), > @@ -1611,6 +1653,26 @@ static struct clk_rcg2 cam_cc_sfe_1_clk_src = { > }, > }; > > +static const struct freq_tbl ftbl_cam_cc_sleep_clk_src[] = { > + F(32000, P_SLEEP_CLK, 1, 0, 0), > + { } > +}; > + > +static struct clk_rcg2 cam_cc_sleep_clk_src = { > + .cmd_rcgr = 0x141a0, > + .mnd_width = 0, > + .hid_width = 5, > + .parent_map = cam_cc_parent_map_12, > + .freq_tbl = ftbl_cam_cc_sleep_clk_src, > + .clkr.hw.init = &(const struct clk_init_data) { > + .name = "cam_cc_sleep_clk_src", > + .parent_data = cam_cc_parent_data_12, > + .num_parents = ARRAY_SIZE(cam_cc_parent_data_12), > + .flags = CLK_SET_RATE_PARENT, > + .ops = &clk_rcg2_shared_ops, > + }, > +}; > + > static const struct freq_tbl ftbl_cam_cc_slow_ahb_clk_src[] = { > F(19200000, P_BI_TCXO, 1, 0, 0), > F(80000000, P_CAM_CC_PLL0_OUT_EVEN, 7.5, 0, 0), > @@ -1632,6 +1694,26 @@ static struct clk_rcg2 cam_cc_slow_ahb_clk_src = { > }, > }; > > +static const struct freq_tbl ftbl_cam_cc_xo_clk_src[] = { > + F(19200000, P_BI_TCXO, 1, 0, 0), You're overloading P_BI_TCXO with a different parent clock (XO_A). The rest lgtm Konrad > + { } > +}; > + > +static struct clk_rcg2 cam_cc_xo_clk_src = { > + .cmd_rcgr = 0x14070, > + .mnd_width = 0, > + .hid_width = 5, > + .parent_map = cam_cc_parent_map_13, > + .freq_tbl = ftbl_cam_cc_xo_clk_src, > + .clkr.hw.init = &(const struct clk_init_data) { > + .name = "cam_cc_xo_clk_src", > + .parent_data = cam_cc_parent_data_13_ao, > + .num_parents = ARRAY_SIZE(cam_cc_parent_data_13_ao), > + .flags = CLK_SET_RATE_PARENT, > + .ops = &clk_rcg2_shared_ops, > + }, > +}; > + > static struct clk_branch cam_cc_bps_ahb_clk = { > .halt_reg = 0x10274, > .halt_check = BRANCH_HALT, > @@ -1704,6 +1786,42 @@ static struct clk_branch cam_cc_camnoc_axi_clk = { > }, > }; > > +static struct clk_branch cam_cc_camnoc_dcd_xo_clk = { > + .halt_reg = 0x13f18, > + .halt_check = BRANCH_HALT, > + .clkr = { > + .enable_reg = 0x13f18, > + .enable_mask = BIT(0), > + .hw.init = &(const struct clk_init_data) { > + .name = "cam_cc_camnoc_dcd_xo_clk", > + .parent_hws = (const struct clk_hw*[]) { > + &cam_cc_xo_clk_src.clkr.hw, > + }, > + .num_parents = 1, > + .flags = CLK_SET_RATE_PARENT, > + .ops = &clk_branch2_ops, > + }, > + }, > +}; > + > +static struct clk_branch cam_cc_camnoc_xo_clk = { > + .halt_reg = 0x13f1c, > + .halt_check = BRANCH_HALT, > + .clkr = { > + .enable_reg = 0x13f1c, > + .enable_mask = BIT(0), > + .hw.init = &(const struct clk_init_data) { > + .name = "cam_cc_camnoc_xo_clk", > + .parent_hws = (const struct clk_hw*[]) { > + &cam_cc_xo_clk_src.clkr.hw, > + }, > + .num_parents = 1, > + .flags = CLK_SET_RATE_PARENT, > + .ops = &clk_branch2_ops, > + }, > + }, > +}; > + > static struct clk_branch cam_cc_cci_0_clk = { > .halt_reg = 0x13a2c, > .halt_check = BRANCH_HALT, > @@ -2370,6 +2488,24 @@ static struct clk_branch cam_cc_drv_ahb_clk = { > }, > }; > > +static struct clk_branch cam_cc_drv_xo_clk = { > + .halt_reg = 0x142d4, > + .halt_check = BRANCH_HALT, > + .clkr = { > + .enable_reg = 0x142d4, > + .enable_mask = BIT(0), > + .hw.init = &(const struct clk_init_data) { > + .name = "cam_cc_drv_xo_clk", > + .parent_hws = (const struct clk_hw*[]) { > + &cam_cc_xo_clk_src.clkr.hw, > + }, > + .num_parents = 1, > + .flags = CLK_SET_RATE_PARENT, > + .ops = &clk_branch2_ops, > + }, > + }, > +}; > + > static struct clk_branch cam_cc_icp_ahb_clk = { > .halt_reg = 0x138fc, > .halt_check = BRANCH_HALT, > @@ -2910,6 +3046,42 @@ static struct clk_branch cam_cc_mclk7_clk = { > }, > }; > > +static struct clk_branch cam_cc_qdss_debug_clk = { > + .halt_reg = 0x14050, > + .halt_check = BRANCH_HALT, > + .clkr = { > + .enable_reg = 0x14050, > + .enable_mask = BIT(0), > + .hw.init = &(const struct clk_init_data) { > + .name = "cam_cc_qdss_debug_clk", > + .parent_hws = (const struct clk_hw*[]) { > + &cam_cc_qdss_debug_clk_src.clkr.hw, > + }, > + .num_parents = 1, > + .flags = CLK_SET_RATE_PARENT, > + .ops = &clk_branch2_ops, > + }, > + }, > +}; > + > +static struct clk_branch cam_cc_qdss_debug_xo_clk = { > + .halt_reg = 0x14054, > + .halt_check = BRANCH_HALT, > + .clkr = { > + .enable_reg = 0x14054, > + .enable_mask = BIT(0), > + .hw.init = &(const struct clk_init_data) { > + .name = "cam_cc_qdss_debug_xo_clk", > + .parent_hws = (const struct clk_hw*[]) { > + &cam_cc_xo_clk_src.clkr.hw, > + }, > + .num_parents = 1, > + .flags = CLK_SET_RATE_PARENT, > + .ops = &clk_branch2_ops, > + }, > + }, > +}; > + > static struct clk_branch cam_cc_sbi_clk = { > .halt_reg = 0x10540, > .halt_check = BRANCH_HALT, > @@ -3133,6 +3305,8 @@ static struct clk_regmap *cam_cc_sm8550_clocks[] = { > [CAM_CC_BPS_FAST_AHB_CLK] = &cam_cc_bps_fast_ahb_clk.clkr, > [CAM_CC_CAMNOC_AXI_CLK] = &cam_cc_camnoc_axi_clk.clkr, > [CAM_CC_CAMNOC_AXI_CLK_SRC] = &cam_cc_camnoc_axi_clk_src.clkr, > + [CAM_CC_CAMNOC_DCD_XO_CLK] = &cam_cc_camnoc_dcd_xo_clk.clkr, > + [CAM_CC_CAMNOC_XO_CLK] = &cam_cc_camnoc_xo_clk.clkr, > [CAM_CC_CCI_0_CLK] = &cam_cc_cci_0_clk.clkr, > [CAM_CC_CCI_0_CLK_SRC] = &cam_cc_cci_0_clk_src.clkr, > [CAM_CC_CCI_1_CLK] = &cam_cc_cci_1_clk.clkr, > @@ -3184,6 +3358,7 @@ static struct clk_regmap *cam_cc_sm8550_clocks[] = { > [CAM_CC_CSIPHY6_CLK] = &cam_cc_csiphy6_clk.clkr, > [CAM_CC_CSIPHY7_CLK] = &cam_cc_csiphy7_clk.clkr, > [CAM_CC_DRV_AHB_CLK] = &cam_cc_drv_ahb_clk.clkr, > + [CAM_CC_DRV_XO_CLK] = &cam_cc_drv_xo_clk.clkr, > [CAM_CC_FAST_AHB_CLK_SRC] = &cam_cc_fast_ahb_clk_src.clkr, > [CAM_CC_ICP_AHB_CLK] = &cam_cc_icp_ahb_clk.clkr, > [CAM_CC_ICP_CLK] = &cam_cc_icp_clk.clkr, > @@ -3260,6 +3435,9 @@ static struct clk_regmap *cam_cc_sm8550_clocks[] = { > [CAM_CC_PLL11_OUT_EVEN] = &cam_cc_pll11_out_even.clkr, > [CAM_CC_PLL12] = &cam_cc_pll12.clkr, > [CAM_CC_PLL12_OUT_EVEN] = &cam_cc_pll12_out_even.clkr, > + [CAM_CC_QDSS_DEBUG_CLK] = &cam_cc_qdss_debug_clk.clkr, > + [CAM_CC_QDSS_DEBUG_CLK_SRC] = &cam_cc_qdss_debug_clk_src.clkr, > + [CAM_CC_QDSS_DEBUG_XO_CLK] = &cam_cc_qdss_debug_xo_clk.clkr, > [CAM_CC_SBI_CLK] = &cam_cc_sbi_clk.clkr, > [CAM_CC_SBI_FAST_AHB_CLK] = &cam_cc_sbi_fast_ahb_clk.clkr, > [CAM_CC_SFE_0_CLK] = &cam_cc_sfe_0_clk.clkr, > @@ -3268,7 +3446,9 @@ static struct clk_regmap *cam_cc_sm8550_clocks[] = { > [CAM_CC_SFE_1_CLK] = &cam_cc_sfe_1_clk.clkr, > [CAM_CC_SFE_1_CLK_SRC] = &cam_cc_sfe_1_clk_src.clkr, > [CAM_CC_SFE_1_FAST_AHB_CLK] = &cam_cc_sfe_1_fast_ahb_clk.clkr, > + [CAM_CC_SLEEP_CLK_SRC] = &cam_cc_sleep_clk_src.clkr, > [CAM_CC_SLOW_AHB_CLK_SRC] = &cam_cc_slow_ahb_clk_src.clkr, > + [CAM_CC_XO_CLK_SRC] = &cam_cc_xo_clk_src.clkr, > }; > > static struct gdsc *cam_cc_sm8550_gdscs[] = {
On 6/24/2023 5:49 PM, Konrad Dybcio wrote: > On 23.06.2023 18:46, Jagadeesh Kona wrote: >> Add support for camera qdss, sleep and xo clocks. >> >> Co-developed-by: Taniya Das <quic_tdas@quicinc.com> >> Signed-off-by: Taniya Das <quic_tdas@quicinc.com> >> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com> >> --- >> Changes since v4: >> - No changes. >> Changes since v3: >> - No changes. >> Changes since v2: >> - No changes. >> Changes since v1: >> - Newly added. >> >> drivers/clk/qcom/camcc-sm8550.c | 180 ++++++++++++++++++++++++++++++++ >> 1 file changed, 180 insertions(+) >> >> diff --git a/drivers/clk/qcom/camcc-sm8550.c b/drivers/clk/qcom/camcc-sm8550.c >> index 075bea32087c..7b4882444d58 100644 >> --- a/drivers/clk/qcom/camcc-sm8550.c >> +++ b/drivers/clk/qcom/camcc-sm8550.c >> @@ -22,6 +22,8 @@ >> enum { >> DT_IFACE, >> DT_BI_TCXO, >> + DT_BI_TCXO_AO, >> + DT_SLEEP_CLK, >> }; >> >> enum { >> @@ -43,6 +45,7 @@ enum { >> P_CAM_CC_PLL10_OUT_EVEN, >> P_CAM_CC_PLL11_OUT_EVEN, >> P_CAM_CC_PLL12_OUT_EVEN, >> + P_SLEEP_CLK, >> }; >> >> static const struct pll_vco lucid_ole_vco[] = { >> @@ -881,6 +884,22 @@ static const struct clk_parent_data cam_cc_parent_data_11[] = { >> { .hw = &cam_cc_pll7_out_even.clkr.hw }, >> }; >> >> +static const struct parent_map cam_cc_parent_map_12[] = { >> + { P_SLEEP_CLK, 0 }, >> +}; >> + >> +static const struct clk_parent_data cam_cc_parent_data_12[] = { >> + { .index = DT_SLEEP_CLK }, >> +}; >> + >> +static const struct parent_map cam_cc_parent_map_13[] = { >> + { P_BI_TCXO, 0 }, >> +}; >> + >> +static const struct clk_parent_data cam_cc_parent_data_13_ao[] = { >> + { .index = DT_BI_TCXO_AO }, >> +}; >> + >> static const struct freq_tbl ftbl_cam_cc_bps_clk_src[] = { >> F(19200000, P_BI_TCXO, 1, 0, 0), >> F(200000000, P_CAM_CC_PLL8_OUT_EVEN, 1, 0, 0), >> @@ -1565,6 +1584,29 @@ static struct clk_rcg2 cam_cc_mclk7_clk_src = { >> }, >> }; >> >> +static const struct freq_tbl ftbl_cam_cc_qdss_debug_clk_src[] = { >> + F(19200000, P_BI_TCXO, 1, 0, 0), >> + F(75000000, P_CAM_CC_PLL0_OUT_EVEN, 8, 0, 0), >> + F(150000000, P_CAM_CC_PLL0_OUT_EVEN, 4, 0, 0), >> + F(300000000, P_CAM_CC_PLL0_OUT_MAIN, 4, 0, 0), >> + { } >> +}; >> + >> +static struct clk_rcg2 cam_cc_qdss_debug_clk_src = { >> + .cmd_rcgr = 0x13f24, >> + .mnd_width = 0, >> + .hid_width = 5, >> + .parent_map = cam_cc_parent_map_0, >> + .freq_tbl = ftbl_cam_cc_qdss_debug_clk_src, >> + .clkr.hw.init = &(const struct clk_init_data) { >> + .name = "cam_cc_qdss_debug_clk_src", >> + .parent_data = cam_cc_parent_data_0, >> + .num_parents = ARRAY_SIZE(cam_cc_parent_data_0), >> + .flags = CLK_SET_RATE_PARENT, >> + .ops = &clk_rcg2_shared_ops, >> + }, >> +}; >> + >> static const struct freq_tbl ftbl_cam_cc_sfe_0_clk_src[] = { >> F(466000000, P_CAM_CC_PLL6_OUT_EVEN, 1, 0, 0), >> F(594000000, P_CAM_CC_PLL6_OUT_EVEN, 1, 0, 0), >> @@ -1611,6 +1653,26 @@ static struct clk_rcg2 cam_cc_sfe_1_clk_src = { >> }, >> }; >> >> +static const struct freq_tbl ftbl_cam_cc_sleep_clk_src[] = { >> + F(32000, P_SLEEP_CLK, 1, 0, 0), >> + { } >> +}; >> + >> +static struct clk_rcg2 cam_cc_sleep_clk_src = { >> + .cmd_rcgr = 0x141a0, >> + .mnd_width = 0, >> + .hid_width = 5, >> + .parent_map = cam_cc_parent_map_12, >> + .freq_tbl = ftbl_cam_cc_sleep_clk_src, >> + .clkr.hw.init = &(const struct clk_init_data) { >> + .name = "cam_cc_sleep_clk_src", >> + .parent_data = cam_cc_parent_data_12, >> + .num_parents = ARRAY_SIZE(cam_cc_parent_data_12), >> + .flags = CLK_SET_RATE_PARENT, >> + .ops = &clk_rcg2_shared_ops, >> + }, >> +}; >> + >> static const struct freq_tbl ftbl_cam_cc_slow_ahb_clk_src[] = { >> F(19200000, P_BI_TCXO, 1, 0, 0), >> F(80000000, P_CAM_CC_PLL0_OUT_EVEN, 7.5, 0, 0), >> @@ -1632,6 +1694,26 @@ static struct clk_rcg2 cam_cc_slow_ahb_clk_src = { >> }, >> }; >> >> +static const struct freq_tbl ftbl_cam_cc_xo_clk_src[] = { >> + F(19200000, P_BI_TCXO, 1, 0, 0), > You're overloading P_BI_TCXO with a different parent clock (XO_A). > This RCG just requires active only voting, hence using XO_A as its parent. Both XO and XO_A are same clock in HW (BI_TCXO), hence we can reuse P_BI_TCXO in frequency table for XO_A parent as well. Thanks, Jagadeesh > The rest lgtm > > Konrad >> + { } >> +}; >> + >> +static struct clk_rcg2 cam_cc_xo_clk_src = { >> + .cmd_rcgr = 0x14070, >> + .mnd_width = 0, >> + .hid_width = 5, >> + .parent_map = cam_cc_parent_map_13, >> + .freq_tbl = ftbl_cam_cc_xo_clk_src, >> + .clkr.hw.init = &(const struct clk_init_data) { >> + .name = "cam_cc_xo_clk_src", >> + .parent_data = cam_cc_parent_data_13_ao, >> + .num_parents = ARRAY_SIZE(cam_cc_parent_data_13_ao), >> + .flags = CLK_SET_RATE_PARENT, >> + .ops = &clk_rcg2_shared_ops, >> + }, >> +}; >> + >> static struct clk_branch cam_cc_bps_ahb_clk = { >> .halt_reg = 0x10274, >> .halt_check = BRANCH_HALT, >> @@ -1704,6 +1786,42 @@ static struct clk_branch cam_cc_camnoc_axi_clk = { >> }, >> }; >> >> +static struct clk_branch cam_cc_camnoc_dcd_xo_clk = { >> + .halt_reg = 0x13f18, >> + .halt_check = BRANCH_HALT, >> + .clkr = { >> + .enable_reg = 0x13f18, >> + .enable_mask = BIT(0), >> + .hw.init = &(const struct clk_init_data) { >> + .name = "cam_cc_camnoc_dcd_xo_clk", >> + .parent_hws = (const struct clk_hw*[]) { >> + &cam_cc_xo_clk_src.clkr.hw, >> + }, >> + .num_parents = 1, >> + .flags = CLK_SET_RATE_PARENT, >> + .ops = &clk_branch2_ops, >> + }, >> + }, >> +}; >> + >> +static struct clk_branch cam_cc_camnoc_xo_clk = { >> + .halt_reg = 0x13f1c, >> + .halt_check = BRANCH_HALT, >> + .clkr = { >> + .enable_reg = 0x13f1c, >> + .enable_mask = BIT(0), >> + .hw.init = &(const struct clk_init_data) { >> + .name = "cam_cc_camnoc_xo_clk", >> + .parent_hws = (const struct clk_hw*[]) { >> + &cam_cc_xo_clk_src.clkr.hw, >> + }, >> + .num_parents = 1, >> + .flags = CLK_SET_RATE_PARENT, >> + .ops = &clk_branch2_ops, >> + }, >> + }, >> +}; >> + >> static struct clk_branch cam_cc_cci_0_clk = { >> .halt_reg = 0x13a2c, >> .halt_check = BRANCH_HALT, >> @@ -2370,6 +2488,24 @@ static struct clk_branch cam_cc_drv_ahb_clk = { >> }, >> }; >> >> +static struct clk_branch cam_cc_drv_xo_clk = { >> + .halt_reg = 0x142d4, >> + .halt_check = BRANCH_HALT, >> + .clkr = { >> + .enable_reg = 0x142d4, >> + .enable_mask = BIT(0), >> + .hw.init = &(const struct clk_init_data) { >> + .name = "cam_cc_drv_xo_clk", >> + .parent_hws = (const struct clk_hw*[]) { >> + &cam_cc_xo_clk_src.clkr.hw, >> + }, >> + .num_parents = 1, >> + .flags = CLK_SET_RATE_PARENT, >> + .ops = &clk_branch2_ops, >> + }, >> + }, >> +}; >> + >> static struct clk_branch cam_cc_icp_ahb_clk = { >> .halt_reg = 0x138fc, >> .halt_check = BRANCH_HALT, >> @@ -2910,6 +3046,42 @@ static struct clk_branch cam_cc_mclk7_clk = { >> }, >> }; >> >> +static struct clk_branch cam_cc_qdss_debug_clk = { >> + .halt_reg = 0x14050, >> + .halt_check = BRANCH_HALT, >> + .clkr = { >> + .enable_reg = 0x14050, >> + .enable_mask = BIT(0), >> + .hw.init = &(const struct clk_init_data) { >> + .name = "cam_cc_qdss_debug_clk", >> + .parent_hws = (const struct clk_hw*[]) { >> + &cam_cc_qdss_debug_clk_src.clkr.hw, >> + }, >> + .num_parents = 1, >> + .flags = CLK_SET_RATE_PARENT, >> + .ops = &clk_branch2_ops, >> + }, >> + }, >> +}; >> + >> +static struct clk_branch cam_cc_qdss_debug_xo_clk = { >> + .halt_reg = 0x14054, >> + .halt_check = BRANCH_HALT, >> + .clkr = { >> + .enable_reg = 0x14054, >> + .enable_mask = BIT(0), >> + .hw.init = &(const struct clk_init_data) { >> + .name = "cam_cc_qdss_debug_xo_clk", >> + .parent_hws = (const struct clk_hw*[]) { >> + &cam_cc_xo_clk_src.clkr.hw, >> + }, >> + .num_parents = 1, >> + .flags = CLK_SET_RATE_PARENT, >> + .ops = &clk_branch2_ops, >> + }, >> + }, >> +}; >> + >> static struct clk_branch cam_cc_sbi_clk = { >> .halt_reg = 0x10540, >> .halt_check = BRANCH_HALT, >> @@ -3133,6 +3305,8 @@ static struct clk_regmap *cam_cc_sm8550_clocks[] = { >> [CAM_CC_BPS_FAST_AHB_CLK] = &cam_cc_bps_fast_ahb_clk.clkr, >> [CAM_CC_CAMNOC_AXI_CLK] = &cam_cc_camnoc_axi_clk.clkr, >> [CAM_CC_CAMNOC_AXI_CLK_SRC] = &cam_cc_camnoc_axi_clk_src.clkr, >> + [CAM_CC_CAMNOC_DCD_XO_CLK] = &cam_cc_camnoc_dcd_xo_clk.clkr, >> + [CAM_CC_CAMNOC_XO_CLK] = &cam_cc_camnoc_xo_clk.clkr, >> [CAM_CC_CCI_0_CLK] = &cam_cc_cci_0_clk.clkr, >> [CAM_CC_CCI_0_CLK_SRC] = &cam_cc_cci_0_clk_src.clkr, >> [CAM_CC_CCI_1_CLK] = &cam_cc_cci_1_clk.clkr, >> @@ -3184,6 +3358,7 @@ static struct clk_regmap *cam_cc_sm8550_clocks[] = { >> [CAM_CC_CSIPHY6_CLK] = &cam_cc_csiphy6_clk.clkr, >> [CAM_CC_CSIPHY7_CLK] = &cam_cc_csiphy7_clk.clkr, >> [CAM_CC_DRV_AHB_CLK] = &cam_cc_drv_ahb_clk.clkr, >> + [CAM_CC_DRV_XO_CLK] = &cam_cc_drv_xo_clk.clkr, >> [CAM_CC_FAST_AHB_CLK_SRC] = &cam_cc_fast_ahb_clk_src.clkr, >> [CAM_CC_ICP_AHB_CLK] = &cam_cc_icp_ahb_clk.clkr, >> [CAM_CC_ICP_CLK] = &cam_cc_icp_clk.clkr, >> @@ -3260,6 +3435,9 @@ static struct clk_regmap *cam_cc_sm8550_clocks[] = { >> [CAM_CC_PLL11_OUT_EVEN] = &cam_cc_pll11_out_even.clkr, >> [CAM_CC_PLL12] = &cam_cc_pll12.clkr, >> [CAM_CC_PLL12_OUT_EVEN] = &cam_cc_pll12_out_even.clkr, >> + [CAM_CC_QDSS_DEBUG_CLK] = &cam_cc_qdss_debug_clk.clkr, >> + [CAM_CC_QDSS_DEBUG_CLK_SRC] = &cam_cc_qdss_debug_clk_src.clkr, >> + [CAM_CC_QDSS_DEBUG_XO_CLK] = &cam_cc_qdss_debug_xo_clk.clkr, >> [CAM_CC_SBI_CLK] = &cam_cc_sbi_clk.clkr, >> [CAM_CC_SBI_FAST_AHB_CLK] = &cam_cc_sbi_fast_ahb_clk.clkr, >> [CAM_CC_SFE_0_CLK] = &cam_cc_sfe_0_clk.clkr, >> @@ -3268,7 +3446,9 @@ static struct clk_regmap *cam_cc_sm8550_clocks[] = { >> [CAM_CC_SFE_1_CLK] = &cam_cc_sfe_1_clk.clkr, >> [CAM_CC_SFE_1_CLK_SRC] = &cam_cc_sfe_1_clk_src.clkr, >> [CAM_CC_SFE_1_FAST_AHB_CLK] = &cam_cc_sfe_1_fast_ahb_clk.clkr, >> + [CAM_CC_SLEEP_CLK_SRC] = &cam_cc_sleep_clk_src.clkr, >> [CAM_CC_SLOW_AHB_CLK_SRC] = &cam_cc_slow_ahb_clk_src.clkr, >> + [CAM_CC_XO_CLK_SRC] = &cam_cc_xo_clk_src.clkr, >> }; >> >> static struct gdsc *cam_cc_sm8550_gdscs[] = {
On 26/06/2023 14:59, Jagadeesh Kona wrote: > > > On 6/24/2023 5:49 PM, Konrad Dybcio wrote: >> On 23.06.2023 18:46, Jagadeesh Kona wrote: >>> Add support for camera qdss, sleep and xo clocks. >>> >>> Co-developed-by: Taniya Das <quic_tdas@quicinc.com> >>> Signed-off-by: Taniya Das <quic_tdas@quicinc.com> >>> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com> >>> --- >>> Changes since v4: >>> - No changes. >>> Changes since v3: >>> - No changes. >>> Changes since v2: >>> - No changes. >>> Changes since v1: >>> - Newly added. >>> >>> drivers/clk/qcom/camcc-sm8550.c | 180 ++++++++++++++++++++++++++++++++ >>> 1 file changed, 180 insertions(+) >>> >>> diff --git a/drivers/clk/qcom/camcc-sm8550.c >>> b/drivers/clk/qcom/camcc-sm8550.c >>> index 075bea32087c..7b4882444d58 100644 >>> --- a/drivers/clk/qcom/camcc-sm8550.c >>> +++ b/drivers/clk/qcom/camcc-sm8550.c >>> @@ -22,6 +22,8 @@ >>> enum { >>> DT_IFACE, >>> DT_BI_TCXO, >>> + DT_BI_TCXO_AO, >>> + DT_SLEEP_CLK, >>> }; >>> enum { >>> @@ -43,6 +45,7 @@ enum { >>> P_CAM_CC_PLL10_OUT_EVEN, >>> P_CAM_CC_PLL11_OUT_EVEN, >>> P_CAM_CC_PLL12_OUT_EVEN, >>> + P_SLEEP_CLK, >>> }; >>> static const struct pll_vco lucid_ole_vco[] = { >>> @@ -881,6 +884,22 @@ static const struct clk_parent_data >>> cam_cc_parent_data_11[] = { >>> { .hw = &cam_cc_pll7_out_even.clkr.hw }, >>> }; >>> +static const struct parent_map cam_cc_parent_map_12[] = { >>> + { P_SLEEP_CLK, 0 }, >>> +}; >>> + >>> +static const struct clk_parent_data cam_cc_parent_data_12[] = { >>> + { .index = DT_SLEEP_CLK }, >>> +}; >>> + >>> +static const struct parent_map cam_cc_parent_map_13[] = { >>> + { P_BI_TCXO, 0 }, >>> +}; >>> + >>> +static const struct clk_parent_data cam_cc_parent_data_13_ao[] = { >>> + { .index = DT_BI_TCXO_AO }, >>> +}; >>> + >>> static const struct freq_tbl ftbl_cam_cc_bps_clk_src[] = { >>> F(19200000, P_BI_TCXO, 1, 0, 0), >>> F(200000000, P_CAM_CC_PLL8_OUT_EVEN, 1, 0, 0), >>> @@ -1565,6 +1584,29 @@ static struct clk_rcg2 cam_cc_mclk7_clk_src = { >>> }, >>> }; >>> +static const struct freq_tbl ftbl_cam_cc_qdss_debug_clk_src[] = { >>> + F(19200000, P_BI_TCXO, 1, 0, 0), >>> + F(75000000, P_CAM_CC_PLL0_OUT_EVEN, 8, 0, 0), >>> + F(150000000, P_CAM_CC_PLL0_OUT_EVEN, 4, 0, 0), >>> + F(300000000, P_CAM_CC_PLL0_OUT_MAIN, 4, 0, 0), >>> + { } >>> +}; >>> + >>> +static struct clk_rcg2 cam_cc_qdss_debug_clk_src = { >>> + .cmd_rcgr = 0x13f24, >>> + .mnd_width = 0, >>> + .hid_width = 5, >>> + .parent_map = cam_cc_parent_map_0, >>> + .freq_tbl = ftbl_cam_cc_qdss_debug_clk_src, >>> + .clkr.hw.init = &(const struct clk_init_data) { >>> + .name = "cam_cc_qdss_debug_clk_src", >>> + .parent_data = cam_cc_parent_data_0, >>> + .num_parents = ARRAY_SIZE(cam_cc_parent_data_0), >>> + .flags = CLK_SET_RATE_PARENT, >>> + .ops = &clk_rcg2_shared_ops, >>> + }, >>> +}; >>> + >>> static const struct freq_tbl ftbl_cam_cc_sfe_0_clk_src[] = { >>> F(466000000, P_CAM_CC_PLL6_OUT_EVEN, 1, 0, 0), >>> F(594000000, P_CAM_CC_PLL6_OUT_EVEN, 1, 0, 0), >>> @@ -1611,6 +1653,26 @@ static struct clk_rcg2 cam_cc_sfe_1_clk_src = { >>> }, >>> }; >>> +static const struct freq_tbl ftbl_cam_cc_sleep_clk_src[] = { >>> + F(32000, P_SLEEP_CLK, 1, 0, 0), >>> + { } >>> +}; >>> + >>> +static struct clk_rcg2 cam_cc_sleep_clk_src = { >>> + .cmd_rcgr = 0x141a0, >>> + .mnd_width = 0, >>> + .hid_width = 5, >>> + .parent_map = cam_cc_parent_map_12, >>> + .freq_tbl = ftbl_cam_cc_sleep_clk_src, >>> + .clkr.hw.init = &(const struct clk_init_data) { >>> + .name = "cam_cc_sleep_clk_src", >>> + .parent_data = cam_cc_parent_data_12, >>> + .num_parents = ARRAY_SIZE(cam_cc_parent_data_12), >>> + .flags = CLK_SET_RATE_PARENT, >>> + .ops = &clk_rcg2_shared_ops, >>> + }, >>> +}; >>> + >>> static const struct freq_tbl ftbl_cam_cc_slow_ahb_clk_src[] = { >>> F(19200000, P_BI_TCXO, 1, 0, 0), >>> F(80000000, P_CAM_CC_PLL0_OUT_EVEN, 7.5, 0, 0), >>> @@ -1632,6 +1694,26 @@ static struct clk_rcg2 cam_cc_slow_ahb_clk_src >>> = { >>> }, >>> }; >>> +static const struct freq_tbl ftbl_cam_cc_xo_clk_src[] = { >>> + F(19200000, P_BI_TCXO, 1, 0, 0), >> You're overloading P_BI_TCXO with a different parent clock (XO_A). >> > > This RCG just requires active only voting, hence using XO_A as its parent. > > Both XO and XO_A are same clock in HW (BI_TCXO), hence we can reuse > P_BI_TCXO in frequency table for XO_A parent as well. Please don't do such things, it complicates understanding the driver. The reviewer could have thought that here the driver was really referencing to the BI_TCXO rather than BI_TCXO_AO. > > Thanks, > Jagadeesh > >> The rest lgtm >> >> Konrad [skipped the rest]
On 6/26/2023 7:10 PM, Dmitry Baryshkov wrote: > On 26/06/2023 14:59, Jagadeesh Kona wrote: >> >> >> On 6/24/2023 5:49 PM, Konrad Dybcio wrote: >>> On 23.06.2023 18:46, Jagadeesh Kona wrote: >>>> Add support for camera qdss, sleep and xo clocks. >>>> >>>> Co-developed-by: Taniya Das <quic_tdas@quicinc.com> >>>> Signed-off-by: Taniya Das <quic_tdas@quicinc.com> >>>> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com> >>>> --- >>>> Changes since v4: >>>> - No changes. >>>> Changes since v3: >>>> - No changes. >>>> Changes since v2: >>>> - No changes. >>>> Changes since v1: >>>> - Newly added. >>>> >>>> drivers/clk/qcom/camcc-sm8550.c | 180 >>>> ++++++++++++++++++++++++++++++++ >>>> 1 file changed, 180 insertions(+) >>>> >>>> diff --git a/drivers/clk/qcom/camcc-sm8550.c >>>> b/drivers/clk/qcom/camcc-sm8550.c >>>> index 075bea32087c..7b4882444d58 100644 >>>> --- a/drivers/clk/qcom/camcc-sm8550.c >>>> +++ b/drivers/clk/qcom/camcc-sm8550.c >>>> @@ -22,6 +22,8 @@ >>>> enum { >>>> DT_IFACE, >>>> DT_BI_TCXO, >>>> + DT_BI_TCXO_AO, >>>> + DT_SLEEP_CLK, >>>> }; >>>> enum { >>>> @@ -43,6 +45,7 @@ enum { >>>> P_CAM_CC_PLL10_OUT_EVEN, >>>> P_CAM_CC_PLL11_OUT_EVEN, >>>> P_CAM_CC_PLL12_OUT_EVEN, >>>> + P_SLEEP_CLK, >>>> }; >>>> static const struct pll_vco lucid_ole_vco[] = { >>>> @@ -881,6 +884,22 @@ static const struct clk_parent_data >>>> cam_cc_parent_data_11[] = { >>>> { .hw = &cam_cc_pll7_out_even.clkr.hw }, >>>> }; >>>> +static const struct parent_map cam_cc_parent_map_12[] = { >>>> + { P_SLEEP_CLK, 0 }, >>>> +}; >>>> + >>>> +static const struct clk_parent_data cam_cc_parent_data_12[] = { >>>> + { .index = DT_SLEEP_CLK }, >>>> +}; >>>> + >>>> +static const struct parent_map cam_cc_parent_map_13[] = { >>>> + { P_BI_TCXO, 0 }, >>>> +}; >>>> + >>>> +static const struct clk_parent_data cam_cc_parent_data_13_ao[] = { >>>> + { .index = DT_BI_TCXO_AO }, >>>> +}; >>>> + >>>> static const struct freq_tbl ftbl_cam_cc_bps_clk_src[] = { >>>> F(19200000, P_BI_TCXO, 1, 0, 0), >>>> F(200000000, P_CAM_CC_PLL8_OUT_EVEN, 1, 0, 0), >>>> @@ -1565,6 +1584,29 @@ static struct clk_rcg2 cam_cc_mclk7_clk_src = { >>>> }, >>>> }; >>>> +static const struct freq_tbl ftbl_cam_cc_qdss_debug_clk_src[] = { >>>> + F(19200000, P_BI_TCXO, 1, 0, 0), >>>> + F(75000000, P_CAM_CC_PLL0_OUT_EVEN, 8, 0, 0), >>>> + F(150000000, P_CAM_CC_PLL0_OUT_EVEN, 4, 0, 0), >>>> + F(300000000, P_CAM_CC_PLL0_OUT_MAIN, 4, 0, 0), >>>> + { } >>>> +}; >>>> + >>>> +static struct clk_rcg2 cam_cc_qdss_debug_clk_src = { >>>> + .cmd_rcgr = 0x13f24, >>>> + .mnd_width = 0, >>>> + .hid_width = 5, >>>> + .parent_map = cam_cc_parent_map_0, >>>> + .freq_tbl = ftbl_cam_cc_qdss_debug_clk_src, >>>> + .clkr.hw.init = &(const struct clk_init_data) { >>>> + .name = "cam_cc_qdss_debug_clk_src", >>>> + .parent_data = cam_cc_parent_data_0, >>>> + .num_parents = ARRAY_SIZE(cam_cc_parent_data_0), >>>> + .flags = CLK_SET_RATE_PARENT, >>>> + .ops = &clk_rcg2_shared_ops, >>>> + }, >>>> +}; >>>> + >>>> static const struct freq_tbl ftbl_cam_cc_sfe_0_clk_src[] = { >>>> F(466000000, P_CAM_CC_PLL6_OUT_EVEN, 1, 0, 0), >>>> F(594000000, P_CAM_CC_PLL6_OUT_EVEN, 1, 0, 0), >>>> @@ -1611,6 +1653,26 @@ static struct clk_rcg2 cam_cc_sfe_1_clk_src = { >>>> }, >>>> }; >>>> +static const struct freq_tbl ftbl_cam_cc_sleep_clk_src[] = { >>>> + F(32000, P_SLEEP_CLK, 1, 0, 0), >>>> + { } >>>> +}; >>>> + >>>> +static struct clk_rcg2 cam_cc_sleep_clk_src = { >>>> + .cmd_rcgr = 0x141a0, >>>> + .mnd_width = 0, >>>> + .hid_width = 5, >>>> + .parent_map = cam_cc_parent_map_12, >>>> + .freq_tbl = ftbl_cam_cc_sleep_clk_src, >>>> + .clkr.hw.init = &(const struct clk_init_data) { >>>> + .name = "cam_cc_sleep_clk_src", >>>> + .parent_data = cam_cc_parent_data_12, >>>> + .num_parents = ARRAY_SIZE(cam_cc_parent_data_12), >>>> + .flags = CLK_SET_RATE_PARENT, >>>> + .ops = &clk_rcg2_shared_ops, >>>> + }, >>>> +}; >>>> + >>>> static const struct freq_tbl ftbl_cam_cc_slow_ahb_clk_src[] = { >>>> F(19200000, P_BI_TCXO, 1, 0, 0), >>>> F(80000000, P_CAM_CC_PLL0_OUT_EVEN, 7.5, 0, 0), >>>> @@ -1632,6 +1694,26 @@ static struct clk_rcg2 >>>> cam_cc_slow_ahb_clk_src = { >>>> }, >>>> }; >>>> +static const struct freq_tbl ftbl_cam_cc_xo_clk_src[] = { >>>> + F(19200000, P_BI_TCXO, 1, 0, 0), >>> You're overloading P_BI_TCXO with a different parent clock (XO_A). >>> >> >> This RCG just requires active only voting, hence using XO_A as its >> parent. >> >> Both XO and XO_A are same clock in HW (BI_TCXO), hence we can reuse >> P_BI_TCXO in frequency table for XO_A parent as well. > > Please don't do such things, it complicates understanding the driver. > The reviewer could have thought that here the driver was really > referencing to the BI_TCXO rather than BI_TCXO_AO. > The enum in parent list indicates the actual HW clock, and since XO and XO_A are the same HW clock, parent enum needs to be a single one. Only parent_data needs to be updated with AO as we have been doing for all targets. Thanks, Jagadeesh >> >> Thanks, >> Jagadeesh >> >>> The rest lgtm >>> >>> Konrad > > [skipped the rest] >
On 30.06.2023 09:14, Jagadeesh Kona wrote: > > > On 6/26/2023 7:10 PM, Dmitry Baryshkov wrote: >> On 26/06/2023 14:59, Jagadeesh Kona wrote: >>> >>> >>> On 6/24/2023 5:49 PM, Konrad Dybcio wrote: >>>> On 23.06.2023 18:46, Jagadeesh Kona wrote: >>>>> Add support for camera qdss, sleep and xo clocks. >>>>> >>>>> Co-developed-by: Taniya Das <quic_tdas@quicinc.com> >>>>> Signed-off-by: Taniya Das <quic_tdas@quicinc.com> >>>>> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com> >>>>> --- >>>>> Changes since v4: >>>>> - No changes. >>>>> Changes since v3: >>>>> - No changes. >>>>> Changes since v2: >>>>> - No changes. >>>>> Changes since v1: >>>>> - Newly added. >>>>> >>>>> drivers/clk/qcom/camcc-sm8550.c | 180 ++++++++++++++++++++++++++++++++ >>>>> 1 file changed, 180 insertions(+) >>>>> >>>>> diff --git a/drivers/clk/qcom/camcc-sm8550.c b/drivers/clk/qcom/camcc-sm8550.c >>>>> index 075bea32087c..7b4882444d58 100644 >>>>> --- a/drivers/clk/qcom/camcc-sm8550.c >>>>> +++ b/drivers/clk/qcom/camcc-sm8550.c >>>>> @@ -22,6 +22,8 @@ >>>>> enum { >>>>> DT_IFACE, >>>>> DT_BI_TCXO, >>>>> + DT_BI_TCXO_AO, >>>>> + DT_SLEEP_CLK, >>>>> }; >>>>> enum { >>>>> @@ -43,6 +45,7 @@ enum { >>>>> P_CAM_CC_PLL10_OUT_EVEN, >>>>> P_CAM_CC_PLL11_OUT_EVEN, >>>>> P_CAM_CC_PLL12_OUT_EVEN, >>>>> + P_SLEEP_CLK, >>>>> }; >>>>> static const struct pll_vco lucid_ole_vco[] = { >>>>> @@ -881,6 +884,22 @@ static const struct clk_parent_data cam_cc_parent_data_11[] = { >>>>> { .hw = &cam_cc_pll7_out_even.clkr.hw }, >>>>> }; >>>>> +static const struct parent_map cam_cc_parent_map_12[] = { >>>>> + { P_SLEEP_CLK, 0 }, >>>>> +}; >>>>> + >>>>> +static const struct clk_parent_data cam_cc_parent_data_12[] = { >>>>> + { .index = DT_SLEEP_CLK }, >>>>> +}; >>>>> + >>>>> +static const struct parent_map cam_cc_parent_map_13[] = { >>>>> + { P_BI_TCXO, 0 }, >>>>> +}; >>>>> + >>>>> +static const struct clk_parent_data cam_cc_parent_data_13_ao[] = { >>>>> + { .index = DT_BI_TCXO_AO }, >>>>> +}; >>>>> + >>>>> static const struct freq_tbl ftbl_cam_cc_bps_clk_src[] = { >>>>> F(19200000, P_BI_TCXO, 1, 0, 0), >>>>> F(200000000, P_CAM_CC_PLL8_OUT_EVEN, 1, 0, 0), >>>>> @@ -1565,6 +1584,29 @@ static struct clk_rcg2 cam_cc_mclk7_clk_src = { >>>>> }, >>>>> }; >>>>> +static const struct freq_tbl ftbl_cam_cc_qdss_debug_clk_src[] = { >>>>> + F(19200000, P_BI_TCXO, 1, 0, 0), >>>>> + F(75000000, P_CAM_CC_PLL0_OUT_EVEN, 8, 0, 0), >>>>> + F(150000000, P_CAM_CC_PLL0_OUT_EVEN, 4, 0, 0), >>>>> + F(300000000, P_CAM_CC_PLL0_OUT_MAIN, 4, 0, 0), >>>>> + { } >>>>> +}; >>>>> + >>>>> +static struct clk_rcg2 cam_cc_qdss_debug_clk_src = { >>>>> + .cmd_rcgr = 0x13f24, >>>>> + .mnd_width = 0, >>>>> + .hid_width = 5, >>>>> + .parent_map = cam_cc_parent_map_0, >>>>> + .freq_tbl = ftbl_cam_cc_qdss_debug_clk_src, >>>>> + .clkr.hw.init = &(const struct clk_init_data) { >>>>> + .name = "cam_cc_qdss_debug_clk_src", >>>>> + .parent_data = cam_cc_parent_data_0, >>>>> + .num_parents = ARRAY_SIZE(cam_cc_parent_data_0), >>>>> + .flags = CLK_SET_RATE_PARENT, >>>>> + .ops = &clk_rcg2_shared_ops, >>>>> + }, >>>>> +}; >>>>> + >>>>> static const struct freq_tbl ftbl_cam_cc_sfe_0_clk_src[] = { >>>>> F(466000000, P_CAM_CC_PLL6_OUT_EVEN, 1, 0, 0), >>>>> F(594000000, P_CAM_CC_PLL6_OUT_EVEN, 1, 0, 0), >>>>> @@ -1611,6 +1653,26 @@ static struct clk_rcg2 cam_cc_sfe_1_clk_src = { >>>>> }, >>>>> }; >>>>> +static const struct freq_tbl ftbl_cam_cc_sleep_clk_src[] = { >>>>> + F(32000, P_SLEEP_CLK, 1, 0, 0), >>>>> + { } >>>>> +}; >>>>> + >>>>> +static struct clk_rcg2 cam_cc_sleep_clk_src = { >>>>> + .cmd_rcgr = 0x141a0, >>>>> + .mnd_width = 0, >>>>> + .hid_width = 5, >>>>> + .parent_map = cam_cc_parent_map_12, >>>>> + .freq_tbl = ftbl_cam_cc_sleep_clk_src, >>>>> + .clkr.hw.init = &(const struct clk_init_data) { >>>>> + .name = "cam_cc_sleep_clk_src", >>>>> + .parent_data = cam_cc_parent_data_12, >>>>> + .num_parents = ARRAY_SIZE(cam_cc_parent_data_12), >>>>> + .flags = CLK_SET_RATE_PARENT, >>>>> + .ops = &clk_rcg2_shared_ops, >>>>> + }, >>>>> +}; >>>>> + >>>>> static const struct freq_tbl ftbl_cam_cc_slow_ahb_clk_src[] = { >>>>> F(19200000, P_BI_TCXO, 1, 0, 0), >>>>> F(80000000, P_CAM_CC_PLL0_OUT_EVEN, 7.5, 0, 0), >>>>> @@ -1632,6 +1694,26 @@ static struct clk_rcg2 cam_cc_slow_ahb_clk_src = { >>>>> }, >>>>> }; >>>>> +static const struct freq_tbl ftbl_cam_cc_xo_clk_src[] = { >>>>> + F(19200000, P_BI_TCXO, 1, 0, 0), >>>> You're overloading P_BI_TCXO with a different parent clock (XO_A). >>>> >>> >>> This RCG just requires active only voting, hence using XO_A as its parent. >>> >>> Both XO and XO_A are same clock in HW (BI_TCXO), hence we can reuse P_BI_TCXO in frequency table for XO_A parent as well. >> >> Please don't do such things, it complicates understanding the driver. The reviewer could have thought that here the driver was really referencing to the BI_TCXO rather than BI_TCXO_AO. >> > > The enum in parent list indicates the actual HW clock, and since XO and XO_A are the same HW clock, parent enum needs to be a single one. Only parent_data needs to be updated with AO as we have been doing for all targets. I see your point, however to Linux, XO and XO_A are two separate clocks within the CCF. Konrad > > Thanks, > Jagadeesh > >>> >>> Thanks, >>> Jagadeesh >>> >>>> The rest lgtm >>>> >>>> Konrad >> >> [skipped the rest] >>
On 7/1/2023 4:51 AM, Konrad Dybcio wrote: > On 30.06.2023 09:14, Jagadeesh Kona wrote: >> >> >> On 6/26/2023 7:10 PM, Dmitry Baryshkov wrote: >>> On 26/06/2023 14:59, Jagadeesh Kona wrote: >>>> >>>> >>>> On 6/24/2023 5:49 PM, Konrad Dybcio wrote: >>>>> On 23.06.2023 18:46, Jagadeesh Kona wrote: >>>>>> Add support for camera qdss, sleep and xo clocks. >>>>>> >>>>>> Co-developed-by: Taniya Das <quic_tdas@quicinc.com> >>>>>> Signed-off-by: Taniya Das <quic_tdas@quicinc.com> >>>>>> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com> >>>>>> --- >>>>>> Changes since v4: >>>>>> - No changes. >>>>>> Changes since v3: >>>>>> - No changes. >>>>>> Changes since v2: >>>>>> - No changes. >>>>>> Changes since v1: >>>>>> - Newly added. >>>>>> >>>>>> drivers/clk/qcom/camcc-sm8550.c | 180 ++++++++++++++++++++++++++++++++ >>>>>> 1 file changed, 180 insertions(+) >>>>>> >>>>>> diff --git a/drivers/clk/qcom/camcc-sm8550.c b/drivers/clk/qcom/camcc-sm8550.c >>>>>> index 075bea32087c..7b4882444d58 100644 >>>>>> --- a/drivers/clk/qcom/camcc-sm8550.c >>>>>> +++ b/drivers/clk/qcom/camcc-sm8550.c >>>>>> @@ -22,6 +22,8 @@ >>>>>> enum { >>>>>> DT_IFACE, >>>>>> DT_BI_TCXO, >>>>>> + DT_BI_TCXO_AO, >>>>>> + DT_SLEEP_CLK, >>>>>> }; >>>>>> enum { >>>>>> @@ -43,6 +45,7 @@ enum { >>>>>> P_CAM_CC_PLL10_OUT_EVEN, >>>>>> P_CAM_CC_PLL11_OUT_EVEN, >>>>>> P_CAM_CC_PLL12_OUT_EVEN, >>>>>> + P_SLEEP_CLK, >>>>>> }; >>>>>> static const struct pll_vco lucid_ole_vco[] = { >>>>>> @@ -881,6 +884,22 @@ static const struct clk_parent_data cam_cc_parent_data_11[] = { >>>>>> { .hw = &cam_cc_pll7_out_even.clkr.hw }, >>>>>> }; >>>>>> +static const struct parent_map cam_cc_parent_map_12[] = { >>>>>> + { P_SLEEP_CLK, 0 }, >>>>>> +}; >>>>>> + >>>>>> +static const struct clk_parent_data cam_cc_parent_data_12[] = { >>>>>> + { .index = DT_SLEEP_CLK }, >>>>>> +}; >>>>>> + >>>>>> +static const struct parent_map cam_cc_parent_map_13[] = { >>>>>> + { P_BI_TCXO, 0 }, >>>>>> +}; >>>>>> + >>>>>> +static const struct clk_parent_data cam_cc_parent_data_13_ao[] = { >>>>>> + { .index = DT_BI_TCXO_AO }, >>>>>> +}; >>>>>> + >>>>>> static const struct freq_tbl ftbl_cam_cc_bps_clk_src[] = { >>>>>> F(19200000, P_BI_TCXO, 1, 0, 0), >>>>>> F(200000000, P_CAM_CC_PLL8_OUT_EVEN, 1, 0, 0), >>>>>> @@ -1565,6 +1584,29 @@ static struct clk_rcg2 cam_cc_mclk7_clk_src = { >>>>>> }, >>>>>> }; >>>>>> +static const struct freq_tbl ftbl_cam_cc_qdss_debug_clk_src[] = { >>>>>> + F(19200000, P_BI_TCXO, 1, 0, 0), >>>>>> + F(75000000, P_CAM_CC_PLL0_OUT_EVEN, 8, 0, 0), >>>>>> + F(150000000, P_CAM_CC_PLL0_OUT_EVEN, 4, 0, 0), >>>>>> + F(300000000, P_CAM_CC_PLL0_OUT_MAIN, 4, 0, 0), >>>>>> + { } >>>>>> +}; >>>>>> + >>>>>> +static struct clk_rcg2 cam_cc_qdss_debug_clk_src = { >>>>>> + .cmd_rcgr = 0x13f24, >>>>>> + .mnd_width = 0, >>>>>> + .hid_width = 5, >>>>>> + .parent_map = cam_cc_parent_map_0, >>>>>> + .freq_tbl = ftbl_cam_cc_qdss_debug_clk_src, >>>>>> + .clkr.hw.init = &(const struct clk_init_data) { >>>>>> + .name = "cam_cc_qdss_debug_clk_src", >>>>>> + .parent_data = cam_cc_parent_data_0, >>>>>> + .num_parents = ARRAY_SIZE(cam_cc_parent_data_0), >>>>>> + .flags = CLK_SET_RATE_PARENT, >>>>>> + .ops = &clk_rcg2_shared_ops, >>>>>> + }, >>>>>> +}; >>>>>> + >>>>>> static const struct freq_tbl ftbl_cam_cc_sfe_0_clk_src[] = { >>>>>> F(466000000, P_CAM_CC_PLL6_OUT_EVEN, 1, 0, 0), >>>>>> F(594000000, P_CAM_CC_PLL6_OUT_EVEN, 1, 0, 0), >>>>>> @@ -1611,6 +1653,26 @@ static struct clk_rcg2 cam_cc_sfe_1_clk_src = { >>>>>> }, >>>>>> }; >>>>>> +static const struct freq_tbl ftbl_cam_cc_sleep_clk_src[] = { >>>>>> + F(32000, P_SLEEP_CLK, 1, 0, 0), >>>>>> + { } >>>>>> +}; >>>>>> + >>>>>> +static struct clk_rcg2 cam_cc_sleep_clk_src = { >>>>>> + .cmd_rcgr = 0x141a0, >>>>>> + .mnd_width = 0, >>>>>> + .hid_width = 5, >>>>>> + .parent_map = cam_cc_parent_map_12, >>>>>> + .freq_tbl = ftbl_cam_cc_sleep_clk_src, >>>>>> + .clkr.hw.init = &(const struct clk_init_data) { >>>>>> + .name = "cam_cc_sleep_clk_src", >>>>>> + .parent_data = cam_cc_parent_data_12, >>>>>> + .num_parents = ARRAY_SIZE(cam_cc_parent_data_12), >>>>>> + .flags = CLK_SET_RATE_PARENT, >>>>>> + .ops = &clk_rcg2_shared_ops, >>>>>> + }, >>>>>> +}; >>>>>> + >>>>>> static const struct freq_tbl ftbl_cam_cc_slow_ahb_clk_src[] = { >>>>>> F(19200000, P_BI_TCXO, 1, 0, 0), >>>>>> F(80000000, P_CAM_CC_PLL0_OUT_EVEN, 7.5, 0, 0), >>>>>> @@ -1632,6 +1694,26 @@ static struct clk_rcg2 cam_cc_slow_ahb_clk_src = { >>>>>> }, >>>>>> }; >>>>>> +static const struct freq_tbl ftbl_cam_cc_xo_clk_src[] = { >>>>>> + F(19200000, P_BI_TCXO, 1, 0, 0), >>>>> You're overloading P_BI_TCXO with a different parent clock (XO_A). >>>>> >>>> >>>> This RCG just requires active only voting, hence using XO_A as its parent. >>>> >>>> Both XO and XO_A are same clock in HW (BI_TCXO), hence we can reuse P_BI_TCXO in frequency table for XO_A parent as well. >>> >>> Please don't do such things, it complicates understanding the driver. The reviewer could have thought that here the driver was really referencing to the BI_TCXO rather than BI_TCXO_AO. >>> >> >> The enum in parent list indicates the actual HW clock, and since XO and XO_A are the same HW clock, parent enum needs to be a single one. Only parent_data needs to be updated with AO as we have been doing for all targets. > I see your point, however to Linux, XO and XO_A are two separate clocks > within the CCF. > Sure, I will introduce P_BI_TCXO_AO and use the same in above frequency table in next series. I hope we will have to take care of similar changes in all other existing chipsets cc files as well in a separate cleanup patch. Thanks, Jagadeesh > Konrad >> >> Thanks, >> Jagadeesh >> >>>> >>>> Thanks, >>>> Jagadeesh >>>> >>>>> The rest lgtm >>>>> >>>>> Konrad >>> >>> [skipped the rest] >>>
diff --git a/drivers/clk/qcom/camcc-sm8550.c b/drivers/clk/qcom/camcc-sm8550.c index 075bea32087c..7b4882444d58 100644 --- a/drivers/clk/qcom/camcc-sm8550.c +++ b/drivers/clk/qcom/camcc-sm8550.c @@ -22,6 +22,8 @@ enum { DT_IFACE, DT_BI_TCXO, + DT_BI_TCXO_AO, + DT_SLEEP_CLK, }; enum { @@ -43,6 +45,7 @@ enum { P_CAM_CC_PLL10_OUT_EVEN, P_CAM_CC_PLL11_OUT_EVEN, P_CAM_CC_PLL12_OUT_EVEN, + P_SLEEP_CLK, }; static const struct pll_vco lucid_ole_vco[] = { @@ -881,6 +884,22 @@ static const struct clk_parent_data cam_cc_parent_data_11[] = { { .hw = &cam_cc_pll7_out_even.clkr.hw }, }; +static const struct parent_map cam_cc_parent_map_12[] = { + { P_SLEEP_CLK, 0 }, +}; + +static const struct clk_parent_data cam_cc_parent_data_12[] = { + { .index = DT_SLEEP_CLK }, +}; + +static const struct parent_map cam_cc_parent_map_13[] = { + { P_BI_TCXO, 0 }, +}; + +static const struct clk_parent_data cam_cc_parent_data_13_ao[] = { + { .index = DT_BI_TCXO_AO }, +}; + static const struct freq_tbl ftbl_cam_cc_bps_clk_src[] = { F(19200000, P_BI_TCXO, 1, 0, 0), F(200000000, P_CAM_CC_PLL8_OUT_EVEN, 1, 0, 0), @@ -1565,6 +1584,29 @@ static struct clk_rcg2 cam_cc_mclk7_clk_src = { }, }; +static const struct freq_tbl ftbl_cam_cc_qdss_debug_clk_src[] = { + F(19200000, P_BI_TCXO, 1, 0, 0), + F(75000000, P_CAM_CC_PLL0_OUT_EVEN, 8, 0, 0), + F(150000000, P_CAM_CC_PLL0_OUT_EVEN, 4, 0, 0), + F(300000000, P_CAM_CC_PLL0_OUT_MAIN, 4, 0, 0), + { } +}; + +static struct clk_rcg2 cam_cc_qdss_debug_clk_src = { + .cmd_rcgr = 0x13f24, + .mnd_width = 0, + .hid_width = 5, + .parent_map = cam_cc_parent_map_0, + .freq_tbl = ftbl_cam_cc_qdss_debug_clk_src, + .clkr.hw.init = &(const struct clk_init_data) { + .name = "cam_cc_qdss_debug_clk_src", + .parent_data = cam_cc_parent_data_0, + .num_parents = ARRAY_SIZE(cam_cc_parent_data_0), + .flags = CLK_SET_RATE_PARENT, + .ops = &clk_rcg2_shared_ops, + }, +}; + static const struct freq_tbl ftbl_cam_cc_sfe_0_clk_src[] = { F(466000000, P_CAM_CC_PLL6_OUT_EVEN, 1, 0, 0), F(594000000, P_CAM_CC_PLL6_OUT_EVEN, 1, 0, 0), @@ -1611,6 +1653,26 @@ static struct clk_rcg2 cam_cc_sfe_1_clk_src = { }, }; +static const struct freq_tbl ftbl_cam_cc_sleep_clk_src[] = { + F(32000, P_SLEEP_CLK, 1, 0, 0), + { } +}; + +static struct clk_rcg2 cam_cc_sleep_clk_src = { + .cmd_rcgr = 0x141a0, + .mnd_width = 0, + .hid_width = 5, + .parent_map = cam_cc_parent_map_12, + .freq_tbl = ftbl_cam_cc_sleep_clk_src, + .clkr.hw.init = &(const struct clk_init_data) { + .name = "cam_cc_sleep_clk_src", + .parent_data = cam_cc_parent_data_12, + .num_parents = ARRAY_SIZE(cam_cc_parent_data_12), + .flags = CLK_SET_RATE_PARENT, + .ops = &clk_rcg2_shared_ops, + }, +}; + static const struct freq_tbl ftbl_cam_cc_slow_ahb_clk_src[] = { F(19200000, P_BI_TCXO, 1, 0, 0), F(80000000, P_CAM_CC_PLL0_OUT_EVEN, 7.5, 0, 0), @@ -1632,6 +1694,26 @@ static struct clk_rcg2 cam_cc_slow_ahb_clk_src = { }, }; +static const struct freq_tbl ftbl_cam_cc_xo_clk_src[] = { + F(19200000, P_BI_TCXO, 1, 0, 0), + { } +}; + +static struct clk_rcg2 cam_cc_xo_clk_src = { + .cmd_rcgr = 0x14070, + .mnd_width = 0, + .hid_width = 5, + .parent_map = cam_cc_parent_map_13, + .freq_tbl = ftbl_cam_cc_xo_clk_src, + .clkr.hw.init = &(const struct clk_init_data) { + .name = "cam_cc_xo_clk_src", + .parent_data = cam_cc_parent_data_13_ao, + .num_parents = ARRAY_SIZE(cam_cc_parent_data_13_ao), + .flags = CLK_SET_RATE_PARENT, + .ops = &clk_rcg2_shared_ops, + }, +}; + static struct clk_branch cam_cc_bps_ahb_clk = { .halt_reg = 0x10274, .halt_check = BRANCH_HALT, @@ -1704,6 +1786,42 @@ static struct clk_branch cam_cc_camnoc_axi_clk = { }, }; +static struct clk_branch cam_cc_camnoc_dcd_xo_clk = { + .halt_reg = 0x13f18, + .halt_check = BRANCH_HALT, + .clkr = { + .enable_reg = 0x13f18, + .enable_mask = BIT(0), + .hw.init = &(const struct clk_init_data) { + .name = "cam_cc_camnoc_dcd_xo_clk", + .parent_hws = (const struct clk_hw*[]) { + &cam_cc_xo_clk_src.clkr.hw, + }, + .num_parents = 1, + .flags = CLK_SET_RATE_PARENT, + .ops = &clk_branch2_ops, + }, + }, +}; + +static struct clk_branch cam_cc_camnoc_xo_clk = { + .halt_reg = 0x13f1c, + .halt_check = BRANCH_HALT, + .clkr = { + .enable_reg = 0x13f1c, + .enable_mask = BIT(0), + .hw.init = &(const struct clk_init_data) { + .name = "cam_cc_camnoc_xo_clk", + .parent_hws = (const struct clk_hw*[]) { + &cam_cc_xo_clk_src.clkr.hw, + }, + .num_parents = 1, + .flags = CLK_SET_RATE_PARENT, + .ops = &clk_branch2_ops, + }, + }, +}; + static struct clk_branch cam_cc_cci_0_clk = { .halt_reg = 0x13a2c, .halt_check = BRANCH_HALT, @@ -2370,6 +2488,24 @@ static struct clk_branch cam_cc_drv_ahb_clk = { }, }; +static struct clk_branch cam_cc_drv_xo_clk = { + .halt_reg = 0x142d4, + .halt_check = BRANCH_HALT, + .clkr = { + .enable_reg = 0x142d4, + .enable_mask = BIT(0), + .hw.init = &(const struct clk_init_data) { + .name = "cam_cc_drv_xo_clk", + .parent_hws = (const struct clk_hw*[]) { + &cam_cc_xo_clk_src.clkr.hw, + }, + .num_parents = 1, + .flags = CLK_SET_RATE_PARENT, + .ops = &clk_branch2_ops, + }, + }, +}; + static struct clk_branch cam_cc_icp_ahb_clk = { .halt_reg = 0x138fc, .halt_check = BRANCH_HALT, @@ -2910,6 +3046,42 @@ static struct clk_branch cam_cc_mclk7_clk = { }, }; +static struct clk_branch cam_cc_qdss_debug_clk = { + .halt_reg = 0x14050, + .halt_check = BRANCH_HALT, + .clkr = { + .enable_reg = 0x14050, + .enable_mask = BIT(0), + .hw.init = &(const struct clk_init_data) { + .name = "cam_cc_qdss_debug_clk", + .parent_hws = (const struct clk_hw*[]) { + &cam_cc_qdss_debug_clk_src.clkr.hw, + }, + .num_parents = 1, + .flags = CLK_SET_RATE_PARENT, + .ops = &clk_branch2_ops, + }, + }, +}; + +static struct clk_branch cam_cc_qdss_debug_xo_clk = { + .halt_reg = 0x14054, + .halt_check = BRANCH_HALT, + .clkr = { + .enable_reg = 0x14054, + .enable_mask = BIT(0), + .hw.init = &(const struct clk_init_data) { + .name = "cam_cc_qdss_debug_xo_clk", + .parent_hws = (const struct clk_hw*[]) { + &cam_cc_xo_clk_src.clkr.hw, + }, + .num_parents = 1, + .flags = CLK_SET_RATE_PARENT, + .ops = &clk_branch2_ops, + }, + }, +}; + static struct clk_branch cam_cc_sbi_clk = { .halt_reg = 0x10540, .halt_check = BRANCH_HALT, @@ -3133,6 +3305,8 @@ static struct clk_regmap *cam_cc_sm8550_clocks[] = { [CAM_CC_BPS_FAST_AHB_CLK] = &cam_cc_bps_fast_ahb_clk.clkr, [CAM_CC_CAMNOC_AXI_CLK] = &cam_cc_camnoc_axi_clk.clkr, [CAM_CC_CAMNOC_AXI_CLK_SRC] = &cam_cc_camnoc_axi_clk_src.clkr, + [CAM_CC_CAMNOC_DCD_XO_CLK] = &cam_cc_camnoc_dcd_xo_clk.clkr, + [CAM_CC_CAMNOC_XO_CLK] = &cam_cc_camnoc_xo_clk.clkr, [CAM_CC_CCI_0_CLK] = &cam_cc_cci_0_clk.clkr, [CAM_CC_CCI_0_CLK_SRC] = &cam_cc_cci_0_clk_src.clkr, [CAM_CC_CCI_1_CLK] = &cam_cc_cci_1_clk.clkr, @@ -3184,6 +3358,7 @@ static struct clk_regmap *cam_cc_sm8550_clocks[] = { [CAM_CC_CSIPHY6_CLK] = &cam_cc_csiphy6_clk.clkr, [CAM_CC_CSIPHY7_CLK] = &cam_cc_csiphy7_clk.clkr, [CAM_CC_DRV_AHB_CLK] = &cam_cc_drv_ahb_clk.clkr, + [CAM_CC_DRV_XO_CLK] = &cam_cc_drv_xo_clk.clkr, [CAM_CC_FAST_AHB_CLK_SRC] = &cam_cc_fast_ahb_clk_src.clkr, [CAM_CC_ICP_AHB_CLK] = &cam_cc_icp_ahb_clk.clkr, [CAM_CC_ICP_CLK] = &cam_cc_icp_clk.clkr, @@ -3260,6 +3435,9 @@ static struct clk_regmap *cam_cc_sm8550_clocks[] = { [CAM_CC_PLL11_OUT_EVEN] = &cam_cc_pll11_out_even.clkr, [CAM_CC_PLL12] = &cam_cc_pll12.clkr, [CAM_CC_PLL12_OUT_EVEN] = &cam_cc_pll12_out_even.clkr, + [CAM_CC_QDSS_DEBUG_CLK] = &cam_cc_qdss_debug_clk.clkr, + [CAM_CC_QDSS_DEBUG_CLK_SRC] = &cam_cc_qdss_debug_clk_src.clkr, + [CAM_CC_QDSS_DEBUG_XO_CLK] = &cam_cc_qdss_debug_xo_clk.clkr, [CAM_CC_SBI_CLK] = &cam_cc_sbi_clk.clkr, [CAM_CC_SBI_FAST_AHB_CLK] = &cam_cc_sbi_fast_ahb_clk.clkr, [CAM_CC_SFE_0_CLK] = &cam_cc_sfe_0_clk.clkr, @@ -3268,7 +3446,9 @@ static struct clk_regmap *cam_cc_sm8550_clocks[] = { [CAM_CC_SFE_1_CLK] = &cam_cc_sfe_1_clk.clkr, [CAM_CC_SFE_1_CLK_SRC] = &cam_cc_sfe_1_clk_src.clkr, [CAM_CC_SFE_1_FAST_AHB_CLK] = &cam_cc_sfe_1_fast_ahb_clk.clkr, + [CAM_CC_SLEEP_CLK_SRC] = &cam_cc_sleep_clk_src.clkr, [CAM_CC_SLOW_AHB_CLK_SRC] = &cam_cc_slow_ahb_clk_src.clkr, + [CAM_CC_XO_CLK_SRC] = &cam_cc_xo_clk_src.clkr, }; static struct gdsc *cam_cc_sm8550_gdscs[] = {