| 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: <eb233f20-1927-4944-9b57-f90c998db19a@amlogic.com>
Date: Thu, 3 Jul 2025 10:51:07 +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 05/26] clk: amlogic: c3-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/c3-peripherals.c | 1261 ++++++++++++++++++------------------
> 1 file changed, 630 insertions(+), 631 deletions(-)
>
> diff --git a/drivers/clk/meson/c3-peripherals.c b/drivers/clk/meson/c3-peripherals.c
> index a25e7d5dc6691d2d4a852e3d3da2b36f251cc141..a09cb1435ab108b2dcc209c6557bcd1988c4ba1a 100644
> --- a/drivers/clk/meson/c3-peripherals.c
> +++ b/drivers/clk/meson/c3-peripherals.c
> @@ -48,7 +48,7 @@
> #define SPIFC_CLK_CTRL 0x1a0
> #define NNA_CLK_CTRL 0x220
>
> -static struct clk_regmap rtc_xtal_clkin = {
> +static struct clk_regmap c3_rtc_xtal_clkin = {
> .data = &(struct clk_regmap_gate_data) {
> .offset = RTC_BY_OSCIN_CTRL0,
> .bit_idx = 31,
> @@ -63,12 +63,12 @@ static struct clk_regmap rtc_xtal_clkin = {
> },
> };
>
> -static const struct meson_clk_dualdiv_param rtc_32k_div_table[] = {
> +static const struct meson_clk_dualdiv_param c3_rtc_32k_div_table[] = {
> { 733, 732, 8, 11, 1 },
> { /* sentinel */ }
> };
>
> -static struct clk_regmap rtc_32k_div = {
> +static struct clk_regmap c3_rtc_32k_div = {
> .data = &(struct meson_clk_dualdiv_data) {
> .n1 = {
> .reg_off = RTC_BY_OSCIN_CTRL0,
> @@ -95,39 +95,39 @@ static struct clk_regmap rtc_32k_div = {
> .shift = 28,
> .width = 1,
> },
> - .table = rtc_32k_div_table,
> + .table = c3_rtc_32k_div_table,
> },
> .hw.init = &(struct clk_init_data) {
> .name = "rtc_32k_div",
> .ops = &meson_clk_dualdiv_ops,
> .parent_hws = (const struct clk_hw *[]) {
> - &rtc_xtal_clkin.hw
> + &c3_rtc_xtal_clkin.hw
> },
> .num_parents = 1,
> },
> };
>
> -static const struct clk_parent_data rtc_32k_mux_parent_data[] = {
> - { .hw = &rtc_32k_div.hw },
> - { .hw = &rtc_xtal_clkin.hw }
> +static const struct clk_parent_data c3_rtc_32k_parents[] = {
> + { .hw = &c3_rtc_32k_div.hw },
> + { .hw = &c3_rtc_xtal_clkin.hw }
> };
>
> -static struct clk_regmap rtc_32k_mux = {
> +static struct clk_regmap c3_rtc_32k_sel = {
> .data = &(struct clk_regmap_mux_data) {
> .offset = RTC_BY_OSCIN_CTRL1,
> .mask = 0x1,
> .shift = 24,
> },
> .hw.init = &(struct clk_init_data) {
> - .name = "rtc_32k_mux",
> + .name = "rtc_32k_sel",
> .ops = &clk_regmap_mux_ops,
> - .parent_data = rtc_32k_mux_parent_data,
> - .num_parents = ARRAY_SIZE(rtc_32k_mux_parent_data),
> + .parent_data = c3_rtc_32k_parents,
> + .num_parents = ARRAY_SIZE(c3_rtc_32k_parents),
> .flags = CLK_SET_RATE_PARENT,
> },
> };
>
> -static struct clk_regmap rtc_32k = {
> +static struct clk_regmap c3_rtc_32k = {
> .data = &(struct clk_regmap_gate_data) {
> .offset = RTC_BY_OSCIN_CTRL0,
> .bit_idx = 30,
> @@ -136,20 +136,20 @@ static struct clk_regmap rtc_32k = {
> .name = "rtc_32k",
> .ops = &clk_regmap_gate_ops,
> .parent_hws = (const struct clk_hw *[]) {
> - &rtc_32k_mux.hw
> + &c3_rtc_32k_sel.hw
> },
> .num_parents = 1,
> .flags = CLK_SET_RATE_PARENT,
> },
> };
>
> -static const struct clk_parent_data rtc_clk_mux_parent_data[] = {
> +static const struct clk_parent_data c3_rtc_clk_parents[] = {
> { .fw_name = "oscin" },
> - { .hw = &rtc_32k.hw },
> + { .hw = &c3_rtc_32k.hw },
> { .fw_name = "pad_osc" }
> };
>
> -static struct clk_regmap rtc_clk = {
> +static struct clk_regmap c3_rtc_clk = {
> .data = &(struct clk_regmap_mux_data) {
> .offset = RTC_CTRL,
> .mask = 0x3,
> @@ -158,62 +158,62 @@ static struct clk_regmap rtc_clk = {
> .hw.init = &(struct clk_init_data) {
> .name = "rtc_clk",
> .ops = &clk_regmap_mux_ops,
> - .parent_data = rtc_clk_mux_parent_data,
> - .num_parents = ARRAY_SIZE(rtc_clk_mux_parent_data),
> + .parent_data = c3_rtc_clk_parents,
> + .num_parents = ARRAY_SIZE(c3_rtc_clk_parents),
> .flags = CLK_SET_RATE_PARENT,
> },
> };
>
> -#define C3_CLK_GATE(_name, _reg, _bit, _fw_name, _ops, _flags) \
> -struct clk_regmap _name = { \
> +#define C3_PCLK(_name, _reg, _bit, _fw_name, _ops, _flags) \
> +struct clk_regmap c3_##_name = { \
> .data = &(struct clk_regmap_gate_data){ \
> .offset = (_reg), \
> .bit_idx = (_bit), \
> }, \
> .hw.init = &(struct clk_init_data) { \
> - .name = #_name, \
> + .name = "c3_" #_name, \
Prefixing variable names with 'SoC' is understandable (to avoid duplicate
definitions and facilitate variable searching), but is it necessary to add
'SoC' prefixes to clock names?
> .ops = _ops, \
> .parent_data = &(const struct clk_parent_data) { \
> - .fw_name = #_fw_name, \
> + .fw_name = (_fw_name), \
> }, \
> .num_parents = 1, \
> .flags = (_flags), \
> }, \
> }
[...]
Powered by blists - more mailing lists