| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <a7738c67-25fc-4919-bee8-69a72abb4871@amlogic.com>
Date: Thu, 3 Jul 2025 11:18:04 +0800
From: Chuan Liu <chuan.liu@...ogic.com>
To: Jerome Brunet <jbrunet@...libre.com>,
Neil Armstrong <neil.armstrong@...aro.org>,
Michael Turquette <mturquette@...libre.com>, Stephen Boyd
<sboyd@...nel.org>, Kevin Hilman <khilman@...libre.com>,
Martin Blumenstingl <martin.blumenstingl@...glemail.com>
Cc: linux-amlogic@...ts.infradead.org, linux-clk@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 12/26] clk: amlogic: s4-peripherals: naming consistency
alignment
Hi Jerome:
On 7/2/2025 11:26 PM, Jerome Brunet wrote:
> [ EXTERNAL EMAIL ]
>
> Amlogic clock controller drivers are all doing the same thing, more or
> less. Yet, over the years, tiny (and often pointless) differences have
> emerged.
>
> This makes reviews more difficult, allows some errors to slip through and
> make it more difficult to exploit SoC commonalities, leading to code
> duplication.
>
> This change enforce, wherever possible, a consistent and predictable scheme
> when it comes to code organisation and naming, The scheme chosen is what
> was used the most already, to try and minimise the size of the ugly
> resulting diff. Here are some of the rules applied:
> - Aligning clock names, variable names and IDs.
> - ID cannot change (used in DT)
> - Variable names w/ SoC name prefixes
> - Clock names w/o SoC name prefixes, except pclks for historic reasons
> - Composite clock systematic naming : mux: X_sel, div:X_div, gate:X
> - Parent table systematically named with the same name as the clock and
> a '_parents' suffix
> - Group various tables next to the related clock
> - etc ...
>
> Doing so removes what would otherwise show up as unrelated diff in
> following changes. It will allow to introduce common definitions for
> peripheral clocks, probe helpers, composite clocks, etc ... making further
> review and maintenance easier.
>
> Signed-off-by: Jerome Brunet <jbrunet@...libre.com>
> ---
> drivers/clk/meson/s4-peripherals.c | 746 ++++++++++++++++++-------------------
> 1 file changed, 370 insertions(+), 376 deletions(-)
>
> diff --git a/drivers/clk/meson/s4-peripherals.c b/drivers/clk/meson/s4-peripherals.c
> index c9400cf54c84c3dc7c63d0636933951b0cac230c..9bcd35f12836de5e318fd1ad9c9ae15a2bfc3dd7 100644
> --- a/drivers/clk/meson/s4-peripherals.c
> +++ b/drivers/clk/meson/s4-peripherals.c
[...]
> @@ -1320,7 +1320,7 @@ static struct clk_regmap s4_ts_clk_gate = {
> * mux because it does top-to-bottom updates the each clock tree and
> * switches to the "inactive" one when CLK_SET_RATE_GATE is set.
> */
> -static const struct clk_parent_data s4_mali_0_1_parent_data[] = {
> +static const struct clk_parent_data s4_mali_parents[] = {
> { .fw_name = "xtal", },
> { .fw_name = "gp0_pll", },
> { .fw_name = "hifi_pll", },
> @@ -1340,8 +1340,8 @@ static struct clk_regmap s4_mali_0_sel = {
> .hw.init = &(struct clk_init_data){
> .name = "mali_0_sel",
> .ops = &clk_regmap_mux_ops,
> - .parent_data = s4_mali_0_1_parent_data,
> - .num_parents = ARRAY_SIZE(s4_mali_0_1_parent_data),
> + .parent_data = s4_mali_parents,
> + .num_parents = ARRAY_SIZE(s4_mali_parents),
> /*
> * Don't request the parent to change the rate because
> * all GPU frequencies can be derived from the fclk_*
> @@ -1394,8 +1394,8 @@ static struct clk_regmap s4_mali_1_sel = {
> .hw.init = &(struct clk_init_data){
> .name = "mali_1_sel",
> .ops = &clk_regmap_mux_ops,
> - .parent_data = s4_mali_0_1_parent_data,
> - .num_parents = ARRAY_SIZE(s4_mali_0_1_parent_data),
> + .parent_data = s4_mali_parents,
> + .num_parents = ARRAY_SIZE(s4_mali_parents),
> .flags = 0,
> },
> };
> @@ -1433,28 +1433,26 @@ static struct clk_regmap s4_mali_1 = {
> },
> };
>
> -static const struct clk_hw *s4_mali_parent_hws[] = {
> - &s4_mali_0.hw,
> - &s4_mali_1.hw
> -};
> -
> -static struct clk_regmap s4_mali_mux = {
> +static struct clk_regmap s4_mali_sel = {
> .data = &(struct clk_regmap_mux_data){
> .offset = CLKCTRL_MALI_CLK_CTRL,
> .mask = 1,
> .shift = 31,
> },
> .hw.init = &(struct clk_init_data){
> - .name = "mali",
> + .name = "mali_sel",
> .ops = &clk_regmap_mux_ops,
> - .parent_hws = s4_mali_parent_hws,
> + .parent_hws = (const struct clk_hw *[]) {
> + &s4_mali_0.hw,
> + &s4_mali_1.hw,
> + },
> .num_parents = 2,
> .flags = CLK_SET_RATE_PARENT,
> },
> };
>
> /* VDEC clocks */
> -static const struct clk_parent_data s4_dec_parent_data[] = {
> +static const struct clk_parent_data s4_dec_parents[] = {
> { .fw_name = "fclk_div2p5", },
> { .fw_name = "fclk_div3", },
> { .fw_name = "fclk_div4", },
> @@ -1465,7 +1463,7 @@ static const struct clk_parent_data s4_dec_parent_data[] = {
> { .fw_name = "xtal", }
> };
>
> -static struct clk_regmap s4_vdec_p0_mux = {
> +static struct clk_regmap s4_vdec_p0_sel = {
Since both vdec_clk and mali_clk are 'no glitch clock', should we also unify
the naming from 's4_vdec_p0'/'s4_vdec_p1' to 's4_vdec_0'/'s4_vdec_1'?
> .data = &(struct clk_regmap_mux_data){
> .offset = CLKCTRL_VDEC_CLK_CTRL,
> .mask = 0x7,
> @@ -1473,10 +1471,10 @@ static struct clk_regmap s4_vdec_p0_mux = {
> .flags = CLK_MUX_ROUND_CLOSEST,
> },
> .hw.init = &(struct clk_init_data) {
> - .name = "vdec_p0_mux",
> + .name = "vdec_p0_sel",
> .ops = &clk_regmap_mux_ops,
> - .parent_data = s4_dec_parent_data,
> - .num_parents = ARRAY_SIZE(s4_dec_parent_data),
> + .parent_data = s4_dec_parents,
> + .num_parents = ARRAY_SIZE(s4_dec_parents),
> .flags = 0,
> },
> };
> @@ -1492,7 +1490,7 @@ static struct clk_regmap s4_vdec_p0_div = {
> .name = "vdec_p0_div",
> .ops = &clk_regmap_divider_ops,
> .parent_hws = (const struct clk_hw *[]) {
> - &s4_vdec_p0_mux.hw
> + &s4_vdec_p0_sel.hw
> },
> .num_parents = 1,
> .flags = CLK_SET_RATE_PARENT,
> @@ -1515,7 +1513,7 @@ static struct clk_regmap s4_vdec_p0 = {
> },
> };
>
> -static struct clk_regmap s4_vdec_p1_mux = {
> +static struct clk_regmap s4_vdec_p1_sel = {
> .data = &(struct clk_regmap_mux_data){
> .offset = CLKCTRL_VDEC3_CLK_CTRL,
> .mask = 0x7,
> @@ -1523,10 +1521,10 @@ static struct clk_regmap s4_vdec_p1_mux = {
> .flags = CLK_MUX_ROUND_CLOSEST,
> },
> .hw.init = &(struct clk_init_data) {
> - .name = "vdec_p1_mux",
> + .name = "vdec_p1_sel",
> .ops = &clk_regmap_mux_ops,
> - .parent_data = s4_dec_parent_data,
> - .num_parents = ARRAY_SIZE(s4_dec_parent_data),
> + .parent_data = s4_dec_parents,
> + .num_parents = ARRAY_SIZE(s4_dec_parents),
> .flags = 0,
> },
> };
> @@ -1542,7 +1540,7 @@ static struct clk_regmap s4_vdec_p1_div = {
> .name = "vdec_p1_div",
> .ops = &clk_regmap_divider_ops,
> .parent_hws = (const struct clk_hw *[]) {
> - &s4_vdec_p1_mux.hw
> + &s4_vdec_p1_sel.hw
> },
> .num_parents = 1,
> .flags = CLK_SET_RATE_PARENT,
> @@ -1565,27 +1563,25 @@ static struct clk_regmap s4_vdec_p1 = {
> },
> };
>
> -static const struct clk_hw *s4_vdec_mux_parent_hws[] = {
> - &s4_vdec_p0.hw,
> - &s4_vdec_p1.hw
> -};
> -
> -static struct clk_regmap s4_vdec_mux = {
> +static struct clk_regmap s4_vdec_sel = {
> .data = &(struct clk_regmap_mux_data){
> .offset = CLKCTRL_VDEC3_CLK_CTRL,
> .mask = 0x1,
> .shift = 15,
> },
> .hw.init = &(struct clk_init_data) {
> - .name = "vdec_mux",
> + .name = "vdec_sel",
> .ops = &clk_regmap_mux_ops,
> - .parent_hws = s4_vdec_mux_parent_hws,
> - .num_parents = ARRAY_SIZE(s4_vdec_mux_parent_hws),
> + .parent_hws = (const struct clk_hw *[]) {
> + &s4_vdec_p0.hw,
> + &s4_vdec_p1.hw,
> + },
> + .num_parents = 2,
> .flags = CLK_SET_RATE_PARENT,
> },
> };
>
> -static struct clk_regmap s4_hevcf_p0_mux = {
> +static struct clk_regmap s4_hevcf_p0_sel = {
+static struct clk_regmap s4_hevcf_0_sel
+static struct clk_regmap s4_hevcf_0_div
.
.
.
> .data = &(struct clk_regmap_mux_data){
> .offset = CLKCTRL_VDEC2_CLK_CTRL,
> .mask = 0x7,
> @@ -1593,10 +1589,10 @@ static struct clk_regmap s4_hevcf_p0_mux = {
> .flags = CLK_MUX_ROUND_CLOSEST,
> },
> .hw.init = &(struct clk_init_data) {
> - .name = "hevcf_p0_mux",
> + .name = "hevcf_p0_sel",
> .ops = &clk_regmap_mux_ops,
> - .parent_data = s4_dec_parent_data,
> - .num_parents = ARRAY_SIZE(s4_dec_parent_data),
> + .parent_data = s4_dec_parents,
> + .num_parents = ARRAY_SIZE(s4_dec_parents),
> .flags = 0,
> },
> };
> @@ -1612,7 +1608,7 @@ static struct clk_regmap s4_hevcf_p0_div = {
> .name = "hevcf_p0_div",
> .ops = &clk_regmap_divider_ops,
> .parent_hws = (const struct clk_hw *[]) {
> - &s4_hevcf_p0_mux.hw
> + &s4_hevcf_p0_sel.hw
> },
> .num_parents = 1,
> .flags = CLK_SET_RATE_PARENT,
> @@ -1625,7 +1621,7 @@ static struct clk_regmap s4_hevcf_p0 = {
> .bit_idx = 8,
> },
> .hw.init = &(struct clk_init_data){
> - .name = "hevcf_p0_gate",
> + .name = "hevcf_p0",
> .ops = &clk_regmap_gate_ops,
> .parent_hws = (const struct clk_hw *[]) {
> &s4_hevcf_p0_div.hw
> @@ -1635,7 +1631,7 @@ static struct clk_regmap s4_hevcf_p0 = {
> },
> };
>
> -static struct clk_regmap s4_hevcf_p1_mux = {
> +static struct clk_regmap s4_hevcf_p1_sel = {
> .data = &(struct clk_regmap_mux_data){
> .offset = CLKCTRL_VDEC4_CLK_CTRL,
> .mask = 0x7,
> @@ -1643,10 +1639,10 @@ static struct clk_regmap s4_hevcf_p1_mux = {
> .flags = CLK_MUX_ROUND_CLOSEST,
> },
> .hw.init = &(struct clk_init_data) {
> - .name = "hevcf_p1_mux",
> + .name = "hevcf_p1_sel",
> .ops = &clk_regmap_mux_ops,
> - .parent_data = s4_dec_parent_data,
> - .num_parents = ARRAY_SIZE(s4_dec_parent_data),
> + .parent_data = s4_dec_parents,
> + .num_parents = ARRAY_SIZE(s4_dec_parents),
> .flags = 0,
> },
> };
> @@ -1662,7 +1658,7 @@ static struct clk_regmap s4_hevcf_p1_div = {
> .name = "hevcf_p1_div",
> .ops = &clk_regmap_divider_ops,
> .parent_hws = (const struct clk_hw *[]) {
> - &s4_hevcf_p1_mux.hw
> + &s4_hevcf_p1_sel.hw
> },
> .num_parents = 1,
> .flags = CLK_SET_RATE_PARENT,
> @@ -1685,28 +1681,26 @@ static struct clk_regmap s4_hevcf_p1 = {
> },
> };
>
> -static const struct clk_hw *s4_hevcf_mux_parent_hws[] = {
> - &s4_hevcf_p0.hw,
> - &s4_hevcf_p1.hw
> -};
> -
> -static struct clk_regmap s4_hevcf_mux = {
> +static struct clk_regmap s4_hevcf_sel = {
> .data = &(struct clk_regmap_mux_data){
> .offset = CLKCTRL_VDEC4_CLK_CTRL,
> .mask = 0x1,
> .shift = 15,
> },
> .hw.init = &(struct clk_init_data) {
> - .name = "hevcf",
> + .name = "hevcf_sel",
> .ops = &clk_regmap_mux_ops,
> - .parent_hws = s4_hevcf_mux_parent_hws,
> - .num_parents = ARRAY_SIZE(s4_hevcf_mux_parent_hws),
> + .parent_hws = (const struct clk_hw *[]) {
> + &s4_hevcf_p0.hw,
> + &s4_hevcf_p1.hw,
> + },
> + .num_parents = 2,
> .flags = CLK_SET_RATE_PARENT,
> },
> };
>
> /* VPU Clock */
> -static const struct clk_parent_data s4_vpu_parent_data[] = {
> +static const struct clk_parent_data s4_vpu_parents[] = {
> { .fw_name = "fclk_div3", },
> { .fw_name = "fclk_div4", },
> { .fw_name = "fclk_div5", },
> @@ -1726,8 +1720,8 @@ static struct clk_regmap s4_vpu_0_sel = {
> .hw.init = &(struct clk_init_data){
> .name = "vpu_0_sel",
> .ops = &clk_regmap_mux_ops,
> - .parent_data = s4_vpu_parent_data,
> - .num_parents = ARRAY_SIZE(s4_vpu_parent_data),
> + .parent_data = s4_vpu_parents,
> + .num_parents = ARRAY_SIZE(s4_vpu_parents),
> .flags = 0,
> },
> };
> @@ -1770,8 +1764,8 @@ static struct clk_regmap s4_vpu_1_sel = {
> .hw.init = &(struct clk_init_data){
> .name = "vpu_1_sel",
> .ops = &clk_regmap_mux_ops,
> - .parent_data = s4_vpu_parent_data,
> - .num_parents = ARRAY_SIZE(s4_vpu_parent_data),
> + .parent_data = s4_vpu_parents,
> + .num_parents = ARRAY_SIZE(s4_vpu_parents),
> .flags = 0,
> },
> };
> @@ -1823,24 +1817,24 @@ static struct clk_regmap s4_vpu = {
> },
> };
>
> -static const struct clk_parent_data vpu_clkb_tmp_parent_data[] = {
> +static const struct clk_parent_data vpu_clkb_tmp_parents[] = {
> { .hw = &s4_vpu.hw },
> { .fw_name = "fclk_div4", },
> { .fw_name = "fclk_div5", },
> { .fw_name = "fclk_div7", }
> };
>
> -static struct clk_regmap s4_vpu_clkb_tmp_mux = {
> +static struct clk_regmap s4_vpu_clkb_tmp_sel = {
> .data = &(struct clk_regmap_mux_data){
> .offset = CLKCTRL_VPU_CLKB_CTRL,
> .mask = 0x3,
> .shift = 20,
> },
> .hw.init = &(struct clk_init_data) {
> - .name = "vpu_clkb_tmp_mux",
> + .name = "vpu_clkb_tmp_sel",
> .ops = &clk_regmap_mux_ops,
> - .parent_data = vpu_clkb_tmp_parent_data,
> - .num_parents = ARRAY_SIZE(vpu_clkb_tmp_parent_data),
> + .parent_data = vpu_clkb_tmp_parents,
> + .num_parents = ARRAY_SIZE(vpu_clkb_tmp_parents),
> .flags = CLK_SET_RATE_PARENT,
> },
> };
> @@ -1855,7 +1849,7 @@ static struct clk_regmap s4_vpu_clkb_tmp_div = {
> .name = "vpu_clkb_tmp_div",
> .ops = &clk_regmap_divider_ops,
> .parent_hws = (const struct clk_hw *[]) {
> - &s4_vpu_clkb_tmp_mux.hw
> + &s4_vpu_clkb_tmp_sel.hw
> },
> .num_parents = 1,
> .flags = CLK_SET_RATE_PARENT,
> @@ -1911,7 +1905,7 @@ static struct clk_regmap s4_vpu_clkb = {
> },
> };
>
> -static const struct clk_parent_data s4_vpu_clkc_parent_data[] = {
> +static const struct clk_parent_data s4_vpu_clkc_parents[] = {
> { .fw_name = "fclk_div4", },
> { .fw_name = "fclk_div3", },
> { .fw_name = "fclk_div5", },
> @@ -1922,17 +1916,17 @@ static const struct clk_parent_data s4_vpu_clkc_parent_data[] = {
> { .fw_name = "gp0_pll", },
> };
>
> -static struct clk_regmap s4_vpu_clkc_p0_mux = {
> +static struct clk_regmap s4_vpu_clkc_p0_sel = {
+static struct clk_regmap s4_vpu_clkc_0_sel
+static struct clk_regmap s4_vpu_clkc_0_div
.
.
.
> .data = &(struct clk_regmap_mux_data){
> .offset = CLKCTRL_VPU_CLKC_CTRL,
> .mask = 0x7,
> .shift = 9,
> },
> .hw.init = &(struct clk_init_data) {
> - .name = "vpu_clkc_p0_mux",
> + .name = "vpu_clkc_p0_sel",
> .ops = &clk_regmap_mux_ops,
> - .parent_data = s4_vpu_clkc_parent_data,
> - .num_parents = ARRAY_SIZE(s4_vpu_clkc_parent_data),
> + .parent_data = s4_vpu_clkc_parents,
> + .num_parents = ARRAY_SIZE(s4_vpu_clkc_parents),
> .flags = 0,
> },
> };
> @@ -1947,7 +1941,7 @@ static struct clk_regmap s4_vpu_clkc_p0_div = {
> .name = "vpu_clkc_p0_div",
> .ops = &clk_regmap_divider_ops,
> .parent_hws = (const struct clk_hw *[]) {
> - &s4_vpu_clkc_p0_mux.hw
> + &s4_vpu_clkc_p0_sel.hw
> },
> .num_parents = 1,
> .flags = CLK_SET_RATE_PARENT,
> @@ -1970,17 +1964,17 @@ static struct clk_regmap s4_vpu_clkc_p0 = {
> },
> };
>
> -static struct clk_regmap s4_vpu_clkc_p1_mux = {
> +static struct clk_regmap s4_vpu_clkc_p1_sel = {
> .data = &(struct clk_regmap_mux_data){
> .offset = CLKCTRL_VPU_CLKC_CTRL,
> .mask = 0x7,
> .shift = 25,
> },
> .hw.init = &(struct clk_init_data) {
> - .name = "vpu_clkc_p1_mux",
> + .name = "vpu_clkc_p1_sel",
> .ops = &clk_regmap_mux_ops,
> - .parent_data = s4_vpu_clkc_parent_data,
> - .num_parents = ARRAY_SIZE(s4_vpu_clkc_parent_data),
> + .parent_data = s4_vpu_clkc_parents,
> + .num_parents = ARRAY_SIZE(s4_vpu_clkc_parents),
> .flags = 0,
> },
> };
> @@ -1995,7 +1989,7 @@ static struct clk_regmap s4_vpu_clkc_p1_div = {
> .name = "vpu_clkc_p1_div",
> .ops = &clk_regmap_divider_ops,
> .parent_hws = (const struct clk_hw *[]) {
> - &s4_vpu_clkc_p1_mux.hw
> + &s4_vpu_clkc_p1_sel.hw
> },
> .num_parents = 1,
> .flags = CLK_SET_RATE_PARENT,
> @@ -2018,28 +2012,26 @@ static struct clk_regmap s4_vpu_clkc_p1 = {
> },
> };
>
> -static const struct clk_hw *s4_vpu_mux_parent_hws[] = {
> - &s4_vpu_clkc_p0.hw,
> - &s4_vpu_clkc_p1.hw
> -};
> -
> -static struct clk_regmap s4_vpu_clkc_mux = {
> +static struct clk_regmap s4_vpu_clkc_sel = {
> .data = &(struct clk_regmap_mux_data){
> .offset = CLKCTRL_VPU_CLKC_CTRL,
> .mask = 0x1,
> .shift = 31,
> },
> .hw.init = &(struct clk_init_data) {
> - .name = "vpu_clkc_mux",
> + .name = "vpu_clkc_sel",
> .ops = &clk_regmap_mux_ops,
> - .parent_hws = s4_vpu_mux_parent_hws,
> - .num_parents = ARRAY_SIZE(s4_vpu_mux_parent_hws),
> + .parent_hws = (const struct clk_hw *[]) {
> + &s4_vpu_clkc_p0.hw,
> + &s4_vpu_clkc_p1.hw,
> + },
> + .num_parents = 2,
> .flags = CLK_SET_RATE_PARENT,
> },
> };
[...]
> MODULE_DESCRIPTION("Amlogic S4 Peripherals Clock Controller driver");
> MODULE_AUTHOR("Yu Tu <yu.tu@...ogic.com>");
>
> --
> 2.47.2
>
>
> _______________________________________________
> linux-amlogic mailing list
> linux-amlogic@...ts.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-amlogic
Powered by blists - more mailing lists