diff mbox series

[V5,4/5] clk: qcom: camcc-sm8550: Add support for qdss, sleep and xo clocks

Message ID 20230623164619.11464-5-quic_jkona@quicinc.com
State Superseded
Headers show
Series Add camera clock controller support for SM8550 | expand

Commit Message

Jagadeesh Kona June 23, 2023, 4:46 p.m. UTC
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(+)

Comments

Jagadeesh Kona June 26, 2023, 11:59 a.m. UTC | #1
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[] = {
Dmitry Baryshkov June 26, 2023, 1:40 p.m. UTC | #2
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]
Jagadeesh Kona June 30, 2023, 7:14 a.m. UTC | #3
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]
>
Konrad Dybcio June 30, 2023, 11:21 p.m. UTC | #4
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]
>>
Jagadeesh Kona July 6, 2023, 8:29 a.m. UTC | #5
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 mbox series

Patch

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[] = {