[PATCH v4 2/2] clk: amlogic: add video-related clocks for S4 SoC

Chuan Liu via B4 Relay posted 2 patches 5 months ago
There is a newer version of this series
[PATCH v4 2/2] clk: amlogic: add video-related clocks for S4 SoC
Posted by Chuan Liu via B4 Relay 5 months ago
From: Chuan Liu <chuan.liu@amlogic.com>

Add video encoder, demodulator and CVBS clocks.

Signed-off-by: Chuan Liu <chuan.liu@amlogic.com>
---
 drivers/clk/meson/s4-peripherals.c | 203 +++++++++++++++++++++++++++++++++++++
 1 file changed, 203 insertions(+)

diff --git a/drivers/clk/meson/s4-peripherals.c b/drivers/clk/meson/s4-peripherals.c
index 6d69b132d1e1..b855e8f1fc04 100644
--- a/drivers/clk/meson/s4-peripherals.c
+++ b/drivers/clk/meson/s4-peripherals.c
@@ -44,6 +44,7 @@
 #define CLKCTRL_VDIN_MEAS_CLK_CTRL                 0x0f8
 #define CLKCTRL_VAPBCLK_CTRL                       0x0fc
 #define CLKCTRL_HDCP22_CTRL                        0x100
+#define CLKCTRL_CDAC_CLK_CTRL                      0x108
 #define CLKCTRL_VDEC_CLK_CTRL                      0x140
 #define CLKCTRL_VDEC2_CLK_CTRL                     0x144
 #define CLKCTRL_VDEC3_CLK_CTRL                     0x148
@@ -1126,6 +1127,22 @@ static struct clk_regmap s4_cts_encp_sel = {
 	},
 };
 
+static struct clk_regmap s4_cts_encl_sel = {
+	.data = &(struct clk_regmap_mux_data){
+		.offset = CLKCTRL_VIID_CLK_DIV,
+		.mask = 0xf,
+		.shift = 12,
+		.table = s4_cts_parents_val_table,
+	},
+	.hw.init = &(struct clk_init_data){
+		.name = "cts_encl_sel",
+		.ops = &clk_regmap_mux_ops,
+		.parent_hws = s4_cts_parents,
+		.num_parents = ARRAY_SIZE(s4_cts_parents),
+		.flags = CLK_SET_RATE_PARENT,
+	},
+};
+
 static struct clk_regmap s4_cts_vdac_sel = {
 	.data = &(struct clk_regmap_mux_data){
 		.offset = CLKCTRL_VIID_CLK_DIV,
@@ -1205,6 +1222,22 @@ static struct clk_regmap s4_cts_encp = {
 	},
 };
 
+static struct clk_regmap s4_cts_encl = {
+	.data = &(struct clk_regmap_gate_data){
+		.offset = CLKCTRL_VID_CLK_CTRL2,
+		.bit_idx = 3,
+	},
+	.hw.init = &(struct clk_init_data) {
+		.name = "cts_encl",
+		.ops = &clk_regmap_gate_ops,
+		.parent_hws = (const struct clk_hw *[]) {
+			&s4_cts_encl_sel.hw
+		},
+		.num_parents = 1,
+		.flags = CLK_SET_RATE_PARENT,
+	},
+};
+
 static struct clk_regmap s4_cts_vdac = {
 	.data = &(struct clk_regmap_gate_data){
 		.offset = CLKCTRL_VID_CLK_CTRL2,
@@ -2735,6 +2768,165 @@ static struct clk_regmap s4_gen_clk = {
 	},
 };
 
+/* CVBS DAC */
+static struct clk_regmap s4_cdac_sel = {
+	.data = &(struct clk_regmap_mux_data) {
+		.offset = CLKCTRL_CDAC_CLK_CTRL,
+		.mask = 0x3,
+		.shift = 16,
+	},
+	.hw.init = &(struct clk_init_data){
+		.name = "cdac_sel",
+		.ops = &clk_regmap_mux_ops,
+		.parent_data = (const struct clk_parent_data []) {
+			{ .fw_name = "xtal", },
+			{ .fw_name = "fclk_div5" },
+		},
+		.num_parents = 2,
+	},
+};
+
+static struct clk_regmap s4_cdac_div = {
+	.data = &(struct clk_regmap_div_data) {
+		.offset = CLKCTRL_CDAC_CLK_CTRL,
+		.shift = 0,
+		.width = 16,
+	},
+	.hw.init = &(struct clk_init_data){
+		.name = "cdac_div",
+		.ops = &clk_regmap_divider_ops,
+		.parent_hws = (const struct clk_hw *[]) {
+			&s4_cdac_sel.hw
+		},
+		.num_parents = 1,
+		.flags = CLK_SET_RATE_PARENT,
+	},
+};
+
+static struct clk_regmap s4_cdac = {
+	.data = &(struct clk_regmap_gate_data) {
+		.offset = CLKCTRL_CDAC_CLK_CTRL,
+		.bit_idx = 20,
+	},
+	.hw.init = &(struct clk_init_data){
+		.name = "cdac",
+		.ops = &clk_regmap_gate_ops,
+		.parent_hws = (const struct clk_hw *[]) {
+			&s4_cdac_div.hw
+		},
+		.num_parents = 1,
+		.flags = CLK_SET_RATE_PARENT,
+	},
+};
+
+static struct clk_regmap s4_demod_core_sel = {
+	.data = &(struct clk_regmap_mux_data) {
+		.offset = CLKCTRL_DEMOD_CLK_CTRL,
+		.mask = 0x3,
+		.shift = 9,
+	},
+	.hw.init = &(struct clk_init_data){
+		.name = "demod_core_sel",
+		.ops = &clk_regmap_mux_ops,
+		.parent_data = (const struct clk_parent_data []) {
+			{ .fw_name = "xtal" },
+			{ .fw_name = "fclk_div7" },
+			{ .fw_name = "fclk_div4" }
+		},
+		.num_parents = 3,
+	},
+};
+
+static struct clk_regmap s4_demod_core_div = {
+	.data = &(struct clk_regmap_div_data) {
+		.offset = CLKCTRL_DEMOD_CLK_CTRL,
+		.shift = 0,
+		.width = 7,
+	},
+	.hw.init = &(struct clk_init_data){
+		.name = "demod_core_div",
+		.ops = &clk_regmap_divider_ops,
+		.parent_hws = (const struct clk_hw *[]) {
+			&s4_demod_core_sel.hw
+		},
+		.num_parents = 1,
+		.flags = CLK_SET_RATE_PARENT,
+	},
+};
+
+static struct clk_regmap s4_demod_core = {
+	.data = &(struct clk_regmap_gate_data) {
+		.offset = CLKCTRL_DEMOD_CLK_CTRL,
+		.bit_idx = 8
+	},
+	.hw.init = &(struct clk_init_data){
+		.name = "demod_core",
+		.ops = &clk_regmap_gate_ops,
+		.parent_hws = (const struct clk_hw *[]) {
+			&s4_demod_core_div.hw
+		},
+		.num_parents = 1,
+		.flags = CLK_SET_RATE_PARENT,
+	},
+};
+
+/* CVBS ADC */
+static struct clk_regmap s4_adc_extclk_in_sel = {
+	.data = &(struct clk_regmap_mux_data) {
+		.offset = CLKCTRL_DEMOD_CLK_CTRL,
+		.mask = 0x7,
+		.shift = 25,
+	},
+	.hw.init = &(struct clk_init_data){
+		.name = "adc_extclk_in_sel",
+		.ops = &clk_regmap_mux_ops,
+		.parent_data = (const struct clk_parent_data []) {
+			{ .fw_name = "xtal" },
+			{ .fw_name = "fclk_div4" },
+			{ .fw_name = "fclk_div3" },
+			{ .fw_name = "fclk_div5" },
+			{ .fw_name = "fclk_div7" },
+			{ .fw_name = "mpll2" },
+			{ .fw_name = "gp0_pll" },
+			{ .fw_name = "hifi_pll" }
+		},
+		.num_parents = 8,
+	},
+};
+
+static struct clk_regmap s4_adc_extclk_in_div = {
+	.data = &(struct clk_regmap_div_data) {
+		.offset = CLKCTRL_DEMOD_CLK_CTRL,
+		.shift = 16,
+		.width = 7,
+	},
+	.hw.init = &(struct clk_init_data){
+		.name = "adc_extclk_in_div",
+		.ops = &clk_regmap_divider_ops,
+		.parent_hws = (const struct clk_hw *[]) {
+			&s4_adc_extclk_in_sel.hw
+		},
+		.num_parents = 1,
+		.flags = CLK_SET_RATE_PARENT,
+	},
+};
+
+static struct clk_regmap s4_adc_extclk_in = {
+	.data = &(struct clk_regmap_gate_data) {
+		.offset = CLKCTRL_DEMOD_CLK_CTRL,
+		.bit_idx = 24
+	},
+	.hw.init = &(struct clk_init_data){
+		.name = "adc_extclk_in",
+		.ops = &clk_regmap_gate_ops,
+		.parent_hws = (const struct clk_hw *[]) {
+			&s4_adc_extclk_in_div.hw
+		},
+		.num_parents = 1,
+		.flags = CLK_SET_RATE_PARENT,
+	},
+};
+
 static const struct clk_parent_data s4_pclk_parents = { .hw = &s4_sys_clk.hw };
 
 #define S4_PCLK(_name, _reg, _bit, _flags) \
@@ -3028,6 +3220,17 @@ static struct clk_hw *s4_peripherals_hw_clks[] = {
 	[CLKID_HDCP22_SKPCLK_SEL]	= &s4_hdcp22_skpclk_sel.hw,
 	[CLKID_HDCP22_SKPCLK_DIV]	= &s4_hdcp22_skpclk_div.hw,
 	[CLKID_HDCP22_SKPCLK]		= &s4_hdcp22_skpclk.hw,
+	[CLKID_CTS_ENCL_SEL]		= &s4_cts_encl_sel.hw,
+	[CLKID_CTS_ENCL]		= &s4_cts_encl.hw,
+	[CLKID_CDAC_SEL]		= &s4_cdac_sel.hw,
+	[CLKID_CDAC_DIV]		= &s4_cdac_div.hw,
+	[CLKID_CDAC]			= &s4_cdac.hw,
+	[CLKID_DEMOD_CORE_SEL]		= &s4_demod_core_sel.hw,
+	[CLKID_DEMOD_CORE_DIV]		= &s4_demod_core_div.hw,
+	[CLKID_DEMOD_CORE]		= &s4_demod_core.hw,
+	[CLKID_ADC_EXTCLK_IN_SEL]	= &s4_adc_extclk_in_sel.hw,
+	[CLKID_ADC_EXTCLK_IN_DIV]	= &s4_adc_extclk_in_div.hw,
+	[CLKID_ADC_EXTCLK_IN]		= &s4_adc_extclk_in.hw,
 };
 
 static const struct meson_clkc_data s4_peripherals_clkc_data = {

-- 
2.42.0
Re: [PATCH v4 2/2] clk: amlogic: add video-related clocks for S4 SoC
Posted by Jerome Brunet 5 months ago
On Tue 09 Sep 2025 at 15:29, Chuan Liu via B4 Relay <devnull+chuan.liu.amlogic.com@kernel.org> wrote:

> From: Chuan Liu <chuan.liu@amlogic.com>
>
> Add video encoder, demodulator and CVBS clocks.
>
> Signed-off-by: Chuan Liu <chuan.liu@amlogic.com>
> ---
>  drivers/clk/meson/s4-peripherals.c | 203 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 203 insertions(+)
>
> diff --git a/drivers/clk/meson/s4-peripherals.c b/drivers/clk/meson/s4-peripherals.c
> index 6d69b132d1e1..b855e8f1fc04 100644
> --- a/drivers/clk/meson/s4-peripherals.c
> +++ b/drivers/clk/meson/s4-peripherals.c
> @@ -44,6 +44,7 @@
>  #define CLKCTRL_VDIN_MEAS_CLK_CTRL                 0x0f8
>  #define CLKCTRL_VAPBCLK_CTRL                       0x0fc
>  #define CLKCTRL_HDCP22_CTRL                        0x100
> +#define CLKCTRL_CDAC_CLK_CTRL                      0x108
>  #define CLKCTRL_VDEC_CLK_CTRL                      0x140
>  #define CLKCTRL_VDEC2_CLK_CTRL                     0x144
>  #define CLKCTRL_VDEC3_CLK_CTRL                     0x148
> @@ -1126,6 +1127,22 @@ static struct clk_regmap s4_cts_encp_sel = {
>  	},
>  };
>  
> +static struct clk_regmap s4_cts_encl_sel = {
> +	.data = &(struct clk_regmap_mux_data){
> +		.offset = CLKCTRL_VIID_CLK_DIV,
> +		.mask = 0xf,
> +		.shift = 12,
> +		.table = s4_cts_parents_val_table,
> +	},
> +	.hw.init = &(struct clk_init_data){
> +		.name = "cts_encl_sel",
> +		.ops = &clk_regmap_mux_ops,
> +		.parent_hws = s4_cts_parents,
> +		.num_parents = ARRAY_SIZE(s4_cts_parents),
> +		.flags = CLK_SET_RATE_PARENT,

Do you really expect the rate of the parents to be adjusted when calling
set_rate() on this clock ?

It all trickle down to vclks which are shared with enci encp and vdac
clocks, so maybe not such a good idea, don't you think ?

> +	},
> +};
> +
>  static struct clk_regmap s4_cts_vdac_sel = {
>  	.data = &(struct clk_regmap_mux_data){
>  		.offset = CLKCTRL_VIID_CLK_DIV,
> @@ -1205,6 +1222,22 @@ static struct clk_regmap s4_cts_encp = {
>  	},
>  };
>  
> +static struct clk_regmap s4_cts_encl = {
> +	.data = &(struct clk_regmap_gate_data){
> +		.offset = CLKCTRL_VID_CLK_CTRL2,
> +		.bit_idx = 3,
> +	},
> +	.hw.init = &(struct clk_init_data) {
> +		.name = "cts_encl",
> +		.ops = &clk_regmap_gate_ops,
> +		.parent_hws = (const struct clk_hw *[]) {
> +			&s4_cts_encl_sel.hw
> +		},
> +		.num_parents = 1,
> +		.flags = CLK_SET_RATE_PARENT,
> +	},
> +};
> +
>  static struct clk_regmap s4_cts_vdac = {
>  	.data = &(struct clk_regmap_gate_data){
>  		.offset = CLKCTRL_VID_CLK_CTRL2,
> @@ -2735,6 +2768,165 @@ static struct clk_regmap s4_gen_clk = {
>  	},
>  };
>  
> +/* CVBS DAC */
> +static struct clk_regmap s4_cdac_sel = {
> +	.data = &(struct clk_regmap_mux_data) {
> +		.offset = CLKCTRL_CDAC_CLK_CTRL,
> +		.mask = 0x3,
> +		.shift = 16,
> +	},
> +	.hw.init = &(struct clk_init_data){
> +		.name = "cdac_sel",
> +		.ops = &clk_regmap_mux_ops,
> +		.parent_data = (const struct clk_parent_data []) {
> +			{ .fw_name = "xtal", },
> +			{ .fw_name = "fclk_div5" },
> +		},
> +		.num_parents = 2,
> +	},
> +};
> +
> +static struct clk_regmap s4_cdac_div = {
> +	.data = &(struct clk_regmap_div_data) {
> +		.offset = CLKCTRL_CDAC_CLK_CTRL,
> +		.shift = 0,
> +		.width = 16,
> +	},
> +	.hw.init = &(struct clk_init_data){
> +		.name = "cdac_div",
> +		.ops = &clk_regmap_divider_ops,
> +		.parent_hws = (const struct clk_hw *[]) {
> +			&s4_cdac_sel.hw
> +		},
> +		.num_parents = 1,
> +		.flags = CLK_SET_RATE_PARENT,
> +	},
> +};
> +
> +static struct clk_regmap s4_cdac = {
> +	.data = &(struct clk_regmap_gate_data) {
> +		.offset = CLKCTRL_CDAC_CLK_CTRL,
> +		.bit_idx = 20,
> +	},
> +	.hw.init = &(struct clk_init_data){
> +		.name = "cdac",
> +		.ops = &clk_regmap_gate_ops,
> +		.parent_hws = (const struct clk_hw *[]) {
> +			&s4_cdac_div.hw
> +		},
> +		.num_parents = 1,
> +		.flags = CLK_SET_RATE_PARENT,
> +	},
> +};
> +
> +static struct clk_regmap s4_demod_core_sel = {
> +	.data = &(struct clk_regmap_mux_data) {
> +		.offset = CLKCTRL_DEMOD_CLK_CTRL,
> +		.mask = 0x3,
> +		.shift = 9,
> +	},
> +	.hw.init = &(struct clk_init_data){
> +		.name = "demod_core_sel",
> +		.ops = &clk_regmap_mux_ops,
> +		.parent_data = (const struct clk_parent_data []) {
> +			{ .fw_name = "xtal" },
> +			{ .fw_name = "fclk_div7" },
> +			{ .fw_name = "fclk_div4" }
> +		},
> +		.num_parents = 3,
> +	},
> +};
> +
> +static struct clk_regmap s4_demod_core_div = {
> +	.data = &(struct clk_regmap_div_data) {
> +		.offset = CLKCTRL_DEMOD_CLK_CTRL,
> +		.shift = 0,
> +		.width = 7,
> +	},
> +	.hw.init = &(struct clk_init_data){
> +		.name = "demod_core_div",
> +		.ops = &clk_regmap_divider_ops,
> +		.parent_hws = (const struct clk_hw *[]) {
> +			&s4_demod_core_sel.hw
> +		},
> +		.num_parents = 1,
> +		.flags = CLK_SET_RATE_PARENT,
> +	},
> +};
> +
> +static struct clk_regmap s4_demod_core = {
> +	.data = &(struct clk_regmap_gate_data) {
> +		.offset = CLKCTRL_DEMOD_CLK_CTRL,
> +		.bit_idx = 8
> +	},
> +	.hw.init = &(struct clk_init_data){
> +		.name = "demod_core",
> +		.ops = &clk_regmap_gate_ops,
> +		.parent_hws = (const struct clk_hw *[]) {
> +			&s4_demod_core_div.hw
> +		},
> +		.num_parents = 1,
> +		.flags = CLK_SET_RATE_PARENT,
> +	},
> +};
> +
> +/* CVBS ADC */
> +static struct clk_regmap s4_adc_extclk_in_sel = {
> +	.data = &(struct clk_regmap_mux_data) {
> +		.offset = CLKCTRL_DEMOD_CLK_CTRL,
> +		.mask = 0x7,
> +		.shift = 25,
> +	},
> +	.hw.init = &(struct clk_init_data){
> +		.name = "adc_extclk_in_sel",
> +		.ops = &clk_regmap_mux_ops,
> +		.parent_data = (const struct clk_parent_data []) {
> +			{ .fw_name = "xtal" },
> +			{ .fw_name = "fclk_div4" },
> +			{ .fw_name = "fclk_div3" },
> +			{ .fw_name = "fclk_div5" },
> +			{ .fw_name = "fclk_div7" },
> +			{ .fw_name = "mpll2" },
> +			{ .fw_name = "gp0_pll" },
> +			{ .fw_name = "hifi_pll" }
> +		},
> +		.num_parents = 8,
> +	},
> +};
> +
> +static struct clk_regmap s4_adc_extclk_in_div = {
> +	.data = &(struct clk_regmap_div_data) {
> +		.offset = CLKCTRL_DEMOD_CLK_CTRL,
> +		.shift = 16,
> +		.width = 7,
> +	},
> +	.hw.init = &(struct clk_init_data){
> +		.name = "adc_extclk_in_div",
> +		.ops = &clk_regmap_divider_ops,
> +		.parent_hws = (const struct clk_hw *[]) {
> +			&s4_adc_extclk_in_sel.hw
> +		},
> +		.num_parents = 1,
> +		.flags = CLK_SET_RATE_PARENT,
> +	},
> +};
> +
> +static struct clk_regmap s4_adc_extclk_in = {
> +	.data = &(struct clk_regmap_gate_data) {
> +		.offset = CLKCTRL_DEMOD_CLK_CTRL,
> +		.bit_idx = 24
> +	},
> +	.hw.init = &(struct clk_init_data){
> +		.name = "adc_extclk_in",
> +		.ops = &clk_regmap_gate_ops,
> +		.parent_hws = (const struct clk_hw *[]) {
> +			&s4_adc_extclk_in_div.hw
> +		},
> +		.num_parents = 1,
> +		.flags = CLK_SET_RATE_PARENT,
> +	},
> +};
> +
>  static const struct clk_parent_data s4_pclk_parents = { .hw = &s4_sys_clk.hw };
>  
>  #define S4_PCLK(_name, _reg, _bit, _flags) \
> @@ -3028,6 +3220,17 @@ static struct clk_hw *s4_peripherals_hw_clks[] = {
>  	[CLKID_HDCP22_SKPCLK_SEL]	= &s4_hdcp22_skpclk_sel.hw,
>  	[CLKID_HDCP22_SKPCLK_DIV]	= &s4_hdcp22_skpclk_div.hw,
>  	[CLKID_HDCP22_SKPCLK]		= &s4_hdcp22_skpclk.hw,
> +	[CLKID_CTS_ENCL_SEL]		= &s4_cts_encl_sel.hw,
> +	[CLKID_CTS_ENCL]		= &s4_cts_encl.hw,
> +	[CLKID_CDAC_SEL]		= &s4_cdac_sel.hw,
> +	[CLKID_CDAC_DIV]		= &s4_cdac_div.hw,
> +	[CLKID_CDAC]			= &s4_cdac.hw,
> +	[CLKID_DEMOD_CORE_SEL]		= &s4_demod_core_sel.hw,
> +	[CLKID_DEMOD_CORE_DIV]		= &s4_demod_core_div.hw,
> +	[CLKID_DEMOD_CORE]		= &s4_demod_core.hw,
> +	[CLKID_ADC_EXTCLK_IN_SEL]	= &s4_adc_extclk_in_sel.hw,
> +	[CLKID_ADC_EXTCLK_IN_DIV]	= &s4_adc_extclk_in_div.hw,
> +	[CLKID_ADC_EXTCLK_IN]		= &s4_adc_extclk_in.hw,
>  };
>  
>  static const struct meson_clkc_data s4_peripherals_clkc_data = {

-- 
Jerome
Re: [PATCH v4 2/2] clk: amlogic: add video-related clocks for S4 SoC
Posted by Chuan Liu 5 months ago
Hi Jerome:


On 9/10/2025 5:41 PM, Jerome Brunet wrote:
> [ EXTERNAL EMAIL ]
>
> On Tue 09 Sep 2025 at 15:29, Chuan Liu via B4 Relay <devnull+chuan.liu.amlogic.com@kernel.org> wrote:
>
>> From: Chuan Liu <chuan.liu@amlogic.com>
>>
>> Add video encoder, demodulator and CVBS clocks.
>>
>> Signed-off-by: Chuan Liu <chuan.liu@amlogic.com>
>> ---
>>   drivers/clk/meson/s4-peripherals.c | 203 +++++++++++++++++++++++++++++++++++++
>>   1 file changed, 203 insertions(+)
>>
>> diff --git a/drivers/clk/meson/s4-peripherals.c b/drivers/clk/meson/s4-peripherals.c
>> index 6d69b132d1e1..b855e8f1fc04 100644
>> --- a/drivers/clk/meson/s4-peripherals.c
>> +++ b/drivers/clk/meson/s4-peripherals.c
>> @@ -44,6 +44,7 @@
>>   #define CLKCTRL_VDIN_MEAS_CLK_CTRL                 0x0f8
>>   #define CLKCTRL_VAPBCLK_CTRL                       0x0fc
>>   #define CLKCTRL_HDCP22_CTRL                        0x100
>> +#define CLKCTRL_CDAC_CLK_CTRL                      0x108
>>   #define CLKCTRL_VDEC_CLK_CTRL                      0x140
>>   #define CLKCTRL_VDEC2_CLK_CTRL                     0x144
>>   #define CLKCTRL_VDEC3_CLK_CTRL                     0x148
>> @@ -1126,6 +1127,22 @@ static struct clk_regmap s4_cts_encp_sel = {
>>        },
>>   };
>>
>> +static struct clk_regmap s4_cts_encl_sel = {
>> +     .data = &(struct clk_regmap_mux_data){
>> +             .offset = CLKCTRL_VIID_CLK_DIV,
>> +             .mask = 0xf,
>> +             .shift = 12,
>> +             .table = s4_cts_parents_val_table,
>> +     },
>> +     .hw.init = &(struct clk_init_data){
>> +             .name = "cts_encl_sel",
>> +             .ops = &clk_regmap_mux_ops,
>> +             .parent_hws = s4_cts_parents,
>> +             .num_parents = ARRAY_SIZE(s4_cts_parents),
>> +             .flags = CLK_SET_RATE_PARENT,
> Do you really expect the rate of the parents to be adjusted when calling
> set_rate() on this clock ?
>
> It all trickle down to vclks which are shared with enci encp and vdac
> clocks, so maybe not such a good idea, don't you think ?


Thanks for pointing this out. You're right, this flag doesn't belong
here.

I'll drop it in the next revision. If there are no further objections
on other aspects, I'll prepare a v5 series that also includes Conor's
Acked-by that I missed...


>> +     },
>> +};
>> +
>>   static struct clk_regmap s4_cts_vdac_sel = {
>>        .data = &(struct clk_regmap_mux_data){
>>                .offset = CLKCTRL_VIID_CLK_DIV,
>> @@ -1205,6 +1222,22 @@ static struct clk_regmap s4_cts_encp = {
>>        },
>>   };
>>
>> +static struct clk_regmap s4_cts_encl = {
>> +     .data = &(struct clk_regmap_gate_data){
>> +             .offset = CLKCTRL_VID_CLK_CTRL2,
>> +             .bit_idx = 3,
>> +     },
>> +     .hw.init = &(struct clk_init_data) {
>> +             .name = "cts_encl",
>> +             .ops = &clk_regmap_gate_ops,
>> +             .parent_hws = (const struct clk_hw *[]) {
>> +                     &s4_cts_encl_sel.hw
>> +             },
>> +             .num_parents = 1,
>> +             .flags = CLK_SET_RATE_PARENT,
>> +     },
>> +};
>> +
>>   static struct clk_regmap s4_cts_vdac = {
>>        .data = &(struct clk_regmap_gate_data){
>>                .offset = CLKCTRL_VID_CLK_CTRL2,
>> @@ -2735,6 +2768,165 @@ static struct clk_regmap s4_gen_clk = {
>>        },
>>   };
>>
>> +/* CVBS DAC */
>> +static struct clk_regmap s4_cdac_sel = {
>> +     .data = &(struct clk_regmap_mux_data) {
>> +             .offset = CLKCTRL_CDAC_CLK_CTRL,
>> +             .mask = 0x3,
>> +             .shift = 16,
>> +     },
>> +     .hw.init = &(struct clk_init_data){
>> +             .name = "cdac_sel",
>> +             .ops = &clk_regmap_mux_ops,
>> +             .parent_data = (const struct clk_parent_data []) {
>> +                     { .fw_name = "xtal", },
>> +                     { .fw_name = "fclk_div5" },
>> +             },
>> +             .num_parents = 2,
>> +     },
>> +};
>> +
>> +static struct clk_regmap s4_cdac_div = {
>> +     .data = &(struct clk_regmap_div_data) {
>> +             .offset = CLKCTRL_CDAC_CLK_CTRL,
>> +             .shift = 0,
>> +             .width = 16,
>> +     },
>> +     .hw.init = &(struct clk_init_data){
>> +             .name = "cdac_div",
>> +             .ops = &clk_regmap_divider_ops,
>> +             .parent_hws = (const struct clk_hw *[]) {
>> +                     &s4_cdac_sel.hw
>> +             },
>> +             .num_parents = 1,
>> +             .flags = CLK_SET_RATE_PARENT,
>> +     },
>> +};
>> +
>> +static struct clk_regmap s4_cdac = {
>> +     .data = &(struct clk_regmap_gate_data) {
>> +             .offset = CLKCTRL_CDAC_CLK_CTRL,
>> +             .bit_idx = 20,
>> +     },
>> +     .hw.init = &(struct clk_init_data){
>> +             .name = "cdac",
>> +             .ops = &clk_regmap_gate_ops,
>> +             .parent_hws = (const struct clk_hw *[]) {
>> +                     &s4_cdac_div.hw
>> +             },
>> +             .num_parents = 1,
>> +             .flags = CLK_SET_RATE_PARENT,
>> +     },
>> +};
>> +
>> +static struct clk_regmap s4_demod_core_sel = {
>> +     .data = &(struct clk_regmap_mux_data) {
>> +             .offset = CLKCTRL_DEMOD_CLK_CTRL,
>> +             .mask = 0x3,
>> +             .shift = 9,
>> +     },
>> +     .hw.init = &(struct clk_init_data){
>> +             .name = "demod_core_sel",
>> +             .ops = &clk_regmap_mux_ops,
>> +             .parent_data = (const struct clk_parent_data []) {
>> +                     { .fw_name = "xtal" },
>> +                     { .fw_name = "fclk_div7" },
>> +                     { .fw_name = "fclk_div4" }
>> +             },
>> +             .num_parents = 3,
>> +     },
>> +};
>> +
>> +static struct clk_regmap s4_demod_core_div = {
>> +     .data = &(struct clk_regmap_div_data) {
>> +             .offset = CLKCTRL_DEMOD_CLK_CTRL,
>> +             .shift = 0,
>> +             .width = 7,
>> +     },
>> +     .hw.init = &(struct clk_init_data){
>> +             .name = "demod_core_div",
>> +             .ops = &clk_regmap_divider_ops,
>> +             .parent_hws = (const struct clk_hw *[]) {
>> +                     &s4_demod_core_sel.hw
>> +             },
>> +             .num_parents = 1,
>> +             .flags = CLK_SET_RATE_PARENT,
>> +     },
>> +};
>> +
>> +static struct clk_regmap s4_demod_core = {
>> +     .data = &(struct clk_regmap_gate_data) {
>> +             .offset = CLKCTRL_DEMOD_CLK_CTRL,
>> +             .bit_idx = 8
>> +     },
>> +     .hw.init = &(struct clk_init_data){
>> +             .name = "demod_core",
>> +             .ops = &clk_regmap_gate_ops,
>> +             .parent_hws = (const struct clk_hw *[]) {
>> +                     &s4_demod_core_div.hw
>> +             },
>> +             .num_parents = 1,
>> +             .flags = CLK_SET_RATE_PARENT,
>> +     },
>> +};
>> +
>> +/* CVBS ADC */
>> +static struct clk_regmap s4_adc_extclk_in_sel = {
>> +     .data = &(struct clk_regmap_mux_data) {
>> +             .offset = CLKCTRL_DEMOD_CLK_CTRL,
>> +             .mask = 0x7,
>> +             .shift = 25,
>> +     },
>> +     .hw.init = &(struct clk_init_data){
>> +             .name = "adc_extclk_in_sel",
>> +             .ops = &clk_regmap_mux_ops,
>> +             .parent_data = (const struct clk_parent_data []) {
>> +                     { .fw_name = "xtal" },
>> +                     { .fw_name = "fclk_div4" },
>> +                     { .fw_name = "fclk_div3" },
>> +                     { .fw_name = "fclk_div5" },
>> +                     { .fw_name = "fclk_div7" },
>> +                     { .fw_name = "mpll2" },
>> +                     { .fw_name = "gp0_pll" },
>> +                     { .fw_name = "hifi_pll" }
>> +             },
>> +             .num_parents = 8,
>> +     },
>> +};
>> +
>> +static struct clk_regmap s4_adc_extclk_in_div = {
>> +     .data = &(struct clk_regmap_div_data) {
>> +             .offset = CLKCTRL_DEMOD_CLK_CTRL,
>> +             .shift = 16,
>> +             .width = 7,
>> +     },
>> +     .hw.init = &(struct clk_init_data){
>> +             .name = "adc_extclk_in_div",
>> +             .ops = &clk_regmap_divider_ops,
>> +             .parent_hws = (const struct clk_hw *[]) {
>> +                     &s4_adc_extclk_in_sel.hw
>> +             },
>> +             .num_parents = 1,
>> +             .flags = CLK_SET_RATE_PARENT,
>> +     },
>> +};
>> +
>> +static struct clk_regmap s4_adc_extclk_in = {
>> +     .data = &(struct clk_regmap_gate_data) {
>> +             .offset = CLKCTRL_DEMOD_CLK_CTRL,
>> +             .bit_idx = 24
>> +     },
>> +     .hw.init = &(struct clk_init_data){
>> +             .name = "adc_extclk_in",
>> +             .ops = &clk_regmap_gate_ops,
>> +             .parent_hws = (const struct clk_hw *[]) {
>> +                     &s4_adc_extclk_in_div.hw
>> +             },
>> +             .num_parents = 1,
>> +             .flags = CLK_SET_RATE_PARENT,
>> +     },
>> +};
>> +
>>   static const struct clk_parent_data s4_pclk_parents = { .hw = &s4_sys_clk.hw };
>>
>>   #define S4_PCLK(_name, _reg, _bit, _flags) \
>> @@ -3028,6 +3220,17 @@ static struct clk_hw *s4_peripherals_hw_clks[] = {
>>        [CLKID_HDCP22_SKPCLK_SEL]       = &s4_hdcp22_skpclk_sel.hw,
>>        [CLKID_HDCP22_SKPCLK_DIV]       = &s4_hdcp22_skpclk_div.hw,
>>        [CLKID_HDCP22_SKPCLK]           = &s4_hdcp22_skpclk.hw,
>> +     [CLKID_CTS_ENCL_SEL]            = &s4_cts_encl_sel.hw,
>> +     [CLKID_CTS_ENCL]                = &s4_cts_encl.hw,
>> +     [CLKID_CDAC_SEL]                = &s4_cdac_sel.hw,
>> +     [CLKID_CDAC_DIV]                = &s4_cdac_div.hw,
>> +     [CLKID_CDAC]                    = &s4_cdac.hw,
>> +     [CLKID_DEMOD_CORE_SEL]          = &s4_demod_core_sel.hw,
>> +     [CLKID_DEMOD_CORE_DIV]          = &s4_demod_core_div.hw,
>> +     [CLKID_DEMOD_CORE]              = &s4_demod_core.hw,
>> +     [CLKID_ADC_EXTCLK_IN_SEL]       = &s4_adc_extclk_in_sel.hw,
>> +     [CLKID_ADC_EXTCLK_IN_DIV]       = &s4_adc_extclk_in_div.hw,
>> +     [CLKID_ADC_EXTCLK_IN]           = &s4_adc_extclk_in.hw,
>>   };
>>
>>   static const struct meson_clkc_data s4_peripherals_clkc_data = {
> --
> Jerome
Re: [PATCH v4 2/2] clk: amlogic: add video-related clocks for S4 SoC
Posted by Jerome Brunet 5 months ago
On Thu 11 Sep 2025 at 10:01, Chuan Liu <chuan.liu@amlogic.com> wrote:

> Hi Jerome:
>
>
> On 9/10/2025 5:41 PM, Jerome Brunet wrote:
>> [ EXTERNAL EMAIL ]
>>
>> On Tue 09 Sep 2025 at 15:29, Chuan Liu via B4 Relay <devnull+chuan.liu.amlogic.com@kernel.org> wrote:
>>
>>> From: Chuan Liu <chuan.liu@amlogic.com>
>>>
>>> Add video encoder, demodulator and CVBS clocks.
>>>
>>> Signed-off-by: Chuan Liu <chuan.liu@amlogic.com>
>>> ---
>>>   drivers/clk/meson/s4-peripherals.c | 203 +++++++++++++++++++++++++++++++++++++
>>>   1 file changed, 203 insertions(+)
>>>
>>> diff --git a/drivers/clk/meson/s4-peripherals.c b/drivers/clk/meson/s4-peripherals.c
>>> index 6d69b132d1e1..b855e8f1fc04 100644
>>> --- a/drivers/clk/meson/s4-peripherals.c
>>> +++ b/drivers/clk/meson/s4-peripherals.c
>>> @@ -44,6 +44,7 @@
>>>   #define CLKCTRL_VDIN_MEAS_CLK_CTRL                 0x0f8
>>>   #define CLKCTRL_VAPBCLK_CTRL                       0x0fc
>>>   #define CLKCTRL_HDCP22_CTRL                        0x100
>>> +#define CLKCTRL_CDAC_CLK_CTRL                      0x108
>>>   #define CLKCTRL_VDEC_CLK_CTRL                      0x140
>>>   #define CLKCTRL_VDEC2_CLK_CTRL                     0x144
>>>   #define CLKCTRL_VDEC3_CLK_CTRL                     0x148
>>> @@ -1126,6 +1127,22 @@ static struct clk_regmap s4_cts_encp_sel = {
>>>        },
>>>   };
>>>
>>> +static struct clk_regmap s4_cts_encl_sel = {
>>> +     .data = &(struct clk_regmap_mux_data){
>>> +             .offset = CLKCTRL_VIID_CLK_DIV,
>>> +             .mask = 0xf,
>>> +             .shift = 12,
>>> +             .table = s4_cts_parents_val_table,
>>> +     },
>>> +     .hw.init = &(struct clk_init_data){
>>> +             .name = "cts_encl_sel",
>>> +             .ops = &clk_regmap_mux_ops,
>>> +             .parent_hws = s4_cts_parents,
>>> +             .num_parents = ARRAY_SIZE(s4_cts_parents),
>>> +             .flags = CLK_SET_RATE_PARENT,
>> Do you really expect the rate of the parents to be adjusted when calling
>> set_rate() on this clock ?
>>
>> It all trickle down to vclks which are shared with enci encp and vdac
>> clocks, so maybe not such a good idea, don't you think ?
>
>
> Thanks for pointing this out. You're right, this flag doesn't belong
> here.

Ok, let's be consistent then. Please add another change to drop the flag
from the other video clocks, such as enci, encp, etc . Thx.

>
> I'll drop it in the next revision. If there are no further objections
> on other aspects, I'll prepare a v5 series that also includes Conor's
> Acked-by that I missed...
>
>
>>> +     },
>>> +};
>>> +
>>>   static struct clk_regmap s4_cts_vdac_sel = {
>>>        .data = &(struct clk_regmap_mux_data){
>>>                .offset = CLKCTRL_VIID_CLK_DIV,
>>> @@ -1205,6 +1222,22 @@ static struct clk_regmap s4_cts_encp = {
>>>        },
>>>   };
>>>
>>> +static struct clk_regmap s4_cts_encl = {
>>> +     .data = &(struct clk_regmap_gate_data){
>>> +             .offset = CLKCTRL_VID_CLK_CTRL2,
>>> +             .bit_idx = 3,
>>> +     },
>>> +     .hw.init = &(struct clk_init_data) {
>>> +             .name = "cts_encl",
>>> +             .ops = &clk_regmap_gate_ops,
>>> +             .parent_hws = (const struct clk_hw *[]) {
>>> +                     &s4_cts_encl_sel.hw
>>> +             },
>>> +             .num_parents = 1,
>>> +             .flags = CLK_SET_RATE_PARENT,
>>> +     },
>>> +};
>>> +
>>>   static struct clk_regmap s4_cts_vdac = {
>>>        .data = &(struct clk_regmap_gate_data){
>>>                .offset = CLKCTRL_VID_CLK_CTRL2,
>>> @@ -2735,6 +2768,165 @@ static struct clk_regmap s4_gen_clk = {
>>>        },
>>>   };
>>>
>>> +/* CVBS DAC */
>>> +static struct clk_regmap s4_cdac_sel = {
>>> +     .data = &(struct clk_regmap_mux_data) {
>>> +             .offset = CLKCTRL_CDAC_CLK_CTRL,
>>> +             .mask = 0x3,
>>> +             .shift = 16,
>>> +     },
>>> +     .hw.init = &(struct clk_init_data){
>>> +             .name = "cdac_sel",
>>> +             .ops = &clk_regmap_mux_ops,
>>> +             .parent_data = (const struct clk_parent_data []) {
>>> +                     { .fw_name = "xtal", },
>>> +                     { .fw_name = "fclk_div5" },
>>> +             },
>>> +             .num_parents = 2,
>>> +     },
>>> +};
>>> +
>>> +static struct clk_regmap s4_cdac_div = {
>>> +     .data = &(struct clk_regmap_div_data) {
>>> +             .offset = CLKCTRL_CDAC_CLK_CTRL,
>>> +             .shift = 0,
>>> +             .width = 16,
>>> +     },
>>> +     .hw.init = &(struct clk_init_data){
>>> +             .name = "cdac_div",
>>> +             .ops = &clk_regmap_divider_ops,
>>> +             .parent_hws = (const struct clk_hw *[]) {
>>> +                     &s4_cdac_sel.hw
>>> +             },
>>> +             .num_parents = 1,
>>> +             .flags = CLK_SET_RATE_PARENT,
>>> +     },
>>> +};
>>> +
>>> +static struct clk_regmap s4_cdac = {
>>> +     .data = &(struct clk_regmap_gate_data) {
>>> +             .offset = CLKCTRL_CDAC_CLK_CTRL,
>>> +             .bit_idx = 20,
>>> +     },
>>> +     .hw.init = &(struct clk_init_data){
>>> +             .name = "cdac",
>>> +             .ops = &clk_regmap_gate_ops,
>>> +             .parent_hws = (const struct clk_hw *[]) {
>>> +                     &s4_cdac_div.hw
>>> +             },
>>> +             .num_parents = 1,
>>> +             .flags = CLK_SET_RATE_PARENT,
>>> +     },
>>> +};
>>> +
>>> +static struct clk_regmap s4_demod_core_sel = {
>>> +     .data = &(struct clk_regmap_mux_data) {
>>> +             .offset = CLKCTRL_DEMOD_CLK_CTRL,
>>> +             .mask = 0x3,
>>> +             .shift = 9,
>>> +     },
>>> +     .hw.init = &(struct clk_init_data){
>>> +             .name = "demod_core_sel",
>>> +             .ops = &clk_regmap_mux_ops,
>>> +             .parent_data = (const struct clk_parent_data []) {
>>> +                     { .fw_name = "xtal" },
>>> +                     { .fw_name = "fclk_div7" },
>>> +                     { .fw_name = "fclk_div4" }
>>> +             },
>>> +             .num_parents = 3,
>>> +     },
>>> +};
>>> +
>>> +static struct clk_regmap s4_demod_core_div = {
>>> +     .data = &(struct clk_regmap_div_data) {
>>> +             .offset = CLKCTRL_DEMOD_CLK_CTRL,
>>> +             .shift = 0,
>>> +             .width = 7,
>>> +     },
>>> +     .hw.init = &(struct clk_init_data){
>>> +             .name = "demod_core_div",
>>> +             .ops = &clk_regmap_divider_ops,
>>> +             .parent_hws = (const struct clk_hw *[]) {
>>> +                     &s4_demod_core_sel.hw
>>> +             },
>>> +             .num_parents = 1,
>>> +             .flags = CLK_SET_RATE_PARENT,
>>> +     },
>>> +};
>>> +
>>> +static struct clk_regmap s4_demod_core = {
>>> +     .data = &(struct clk_regmap_gate_data) {
>>> +             .offset = CLKCTRL_DEMOD_CLK_CTRL,
>>> +             .bit_idx = 8
>>> +     },
>>> +     .hw.init = &(struct clk_init_data){
>>> +             .name = "demod_core",
>>> +             .ops = &clk_regmap_gate_ops,
>>> +             .parent_hws = (const struct clk_hw *[]) {
>>> +                     &s4_demod_core_div.hw
>>> +             },
>>> +             .num_parents = 1,
>>> +             .flags = CLK_SET_RATE_PARENT,
>>> +     },
>>> +};
>>> +
>>> +/* CVBS ADC */
>>> +static struct clk_regmap s4_adc_extclk_in_sel = {
>>> +     .data = &(struct clk_regmap_mux_data) {
>>> +             .offset = CLKCTRL_DEMOD_CLK_CTRL,
>>> +             .mask = 0x7,
>>> +             .shift = 25,
>>> +     },
>>> +     .hw.init = &(struct clk_init_data){
>>> +             .name = "adc_extclk_in_sel",
>>> +             .ops = &clk_regmap_mux_ops,
>>> +             .parent_data = (const struct clk_parent_data []) {
>>> +                     { .fw_name = "xtal" },
>>> +                     { .fw_name = "fclk_div4" },
>>> +                     { .fw_name = "fclk_div3" },
>>> +                     { .fw_name = "fclk_div5" },
>>> +                     { .fw_name = "fclk_div7" },
>>> +                     { .fw_name = "mpll2" },
>>> +                     { .fw_name = "gp0_pll" },
>>> +                     { .fw_name = "hifi_pll" }
>>> +             },
>>> +             .num_parents = 8,
>>> +     },
>>> +};
>>> +
>>> +static struct clk_regmap s4_adc_extclk_in_div = {
>>> +     .data = &(struct clk_regmap_div_data) {
>>> +             .offset = CLKCTRL_DEMOD_CLK_CTRL,
>>> +             .shift = 16,
>>> +             .width = 7,
>>> +     },
>>> +     .hw.init = &(struct clk_init_data){
>>> +             .name = "adc_extclk_in_div",
>>> +             .ops = &clk_regmap_divider_ops,
>>> +             .parent_hws = (const struct clk_hw *[]) {
>>> +                     &s4_adc_extclk_in_sel.hw
>>> +             },
>>> +             .num_parents = 1,
>>> +             .flags = CLK_SET_RATE_PARENT,
>>> +     },
>>> +};
>>> +
>>> +static struct clk_regmap s4_adc_extclk_in = {
>>> +     .data = &(struct clk_regmap_gate_data) {
>>> +             .offset = CLKCTRL_DEMOD_CLK_CTRL,
>>> +             .bit_idx = 24
>>> +     },
>>> +     .hw.init = &(struct clk_init_data){
>>> +             .name = "adc_extclk_in",
>>> +             .ops = &clk_regmap_gate_ops,
>>> +             .parent_hws = (const struct clk_hw *[]) {
>>> +                     &s4_adc_extclk_in_div.hw
>>> +             },
>>> +             .num_parents = 1,
>>> +             .flags = CLK_SET_RATE_PARENT,
>>> +     },
>>> +};
>>> +
>>>   static const struct clk_parent_data s4_pclk_parents = { .hw = &s4_sys_clk.hw };
>>>
>>>   #define S4_PCLK(_name, _reg, _bit, _flags) \
>>> @@ -3028,6 +3220,17 @@ static struct clk_hw *s4_peripherals_hw_clks[] = {
>>>        [CLKID_HDCP22_SKPCLK_SEL]       = &s4_hdcp22_skpclk_sel.hw,
>>>        [CLKID_HDCP22_SKPCLK_DIV]       = &s4_hdcp22_skpclk_div.hw,
>>>        [CLKID_HDCP22_SKPCLK]           = &s4_hdcp22_skpclk.hw,
>>> +     [CLKID_CTS_ENCL_SEL]            = &s4_cts_encl_sel.hw,
>>> +     [CLKID_CTS_ENCL]                = &s4_cts_encl.hw,
>>> +     [CLKID_CDAC_SEL]                = &s4_cdac_sel.hw,
>>> +     [CLKID_CDAC_DIV]                = &s4_cdac_div.hw,
>>> +     [CLKID_CDAC]                    = &s4_cdac.hw,
>>> +     [CLKID_DEMOD_CORE_SEL]          = &s4_demod_core_sel.hw,
>>> +     [CLKID_DEMOD_CORE_DIV]          = &s4_demod_core_div.hw,
>>> +     [CLKID_DEMOD_CORE]              = &s4_demod_core.hw,
>>> +     [CLKID_ADC_EXTCLK_IN_SEL]       = &s4_adc_extclk_in_sel.hw,
>>> +     [CLKID_ADC_EXTCLK_IN_DIV]       = &s4_adc_extclk_in_div.hw,
>>> +     [CLKID_ADC_EXTCLK_IN]           = &s4_adc_extclk_in.hw,
>>>   };
>>>
>>>   static const struct meson_clkc_data s4_peripherals_clkc_data = {
>> --
>> Jerome

-- 
Jerome