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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1jbjq1d5xh.fsf@starbuckisacylon.baylibre.com>
Date: Thu, 03 Jul 2025 10:39:22 +0200
From: Jerome Brunet <jbrunet@...libre.com>
To: Chuan Liu <chuan.liu@...ogic.com>
Cc: 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>,  linux-amlogic@...ts.infradead.org,
  linux-clk@...r.kernel.org,  linux-kernel@...r.kernel.org
Subject: Re: [PATCH 23/26] clk: amlogic: use the common pclk definition

On Thu 03 Jul 2025 at 15:16, Chuan Liu <chuan.liu@...ogic.com> wrote:

> Hi Jerome:
>
>
> On 7/2/2025 11:26 PM, Jerome Brunet wrote:
>> [ EXTERNAL EMAIL ]
>>
>> Replace marcros defining pclks with the common one, reducing code
>> duplication.
>>
>> Signed-off-by: Jerome Brunet <jbrunet@...libre.com>
>> ---
>>   drivers/clk/meson/axg-aoclk.c      | 35 +++++++++-----------------
>>   drivers/clk/meson/c3-peripherals.c | 34 +++++++-------------------
>>   drivers/clk/meson/g12a-aoclk.c     | 50 +++++++++++++++-----------------------
>>   drivers/clk/meson/gxbb-aoclk.c     | 33 +++++++++----------------
>>   4 files changed, 51 insertions(+), 101 deletions(-)
>>
>> diff --git a/drivers/clk/meson/axg-aoclk.c b/drivers/clk/meson/axg-aoclk.c
>> index 74c2f51424f11cc04a80a3a4918e4de0a5d11d08..902fbd34039cc06d512f1237a1e5d9050fd00b4b 100644
>> --- a/drivers/clk/meson/axg-aoclk.c
>> +++ b/drivers/clk/meson/axg-aoclk.c
>> @@ -34,30 +34,19 @@
>>   #define AO_RTC_ALT_CLK_CNTL0   0x94
>>   #define AO_RTC_ALT_CLK_CNTL1   0x98
>>
>> -#define AXG_AO_GATE(_name, _bit, _flags)                               \
>> -static struct clk_regmap axg_ao_##_name = {                            \
>> -       .data = &(struct clk_regmap_gate_data) {                        \
>> -               .offset = (AO_RTI_GEN_CNTL_REG0),                       \
>> -               .bit_idx = (_bit),                                      \
>> -       },                                                              \
>> -       .hw.init = &(struct clk_init_data) {                            \
>> -               .name =  "axg_ao_" #_name,                              \
>> -               .ops = &clk_regmap_gate_ops,                            \
>> -               .parent_data = &(const struct clk_parent_data) {        \
>> -                       .fw_name = "mpeg-clk",                          \
>> -               },                                                      \
>> -               .num_parents = 1,                                       \
>> -               .flags = (_flags),                                      \
>> -       },                                                              \
>> -}
>> +static const struct clk_parent_data axg_ao_pclk_parents = { .fw_name = "mpeg-clk" };
>>
>> -AXG_AO_GATE(remote,    0, CLK_IGNORE_UNUSED);
>> -AXG_AO_GATE(i2c_master,        1, CLK_IGNORE_UNUSED);
>> -AXG_AO_GATE(i2c_slave, 2, CLK_IGNORE_UNUSED);
>> -AXG_AO_GATE(uart1,     3, CLK_IGNORE_UNUSED);
>> -AXG_AO_GATE(uart2,     5, CLK_IGNORE_UNUSED);
>> -AXG_AO_GATE(ir_blaster,        6, CLK_IGNORE_UNUSED);
>> -AXG_AO_GATE(saradc,    7, CLK_IGNORE_UNUSED);
>> +#define AXG_AO_GATE(_name, _bit, _flags)                      \
>> +       MESON_PCLK(axg_ao_##_name, AO_RTI_GEN_CNTL_REG0, _bit, \
>> +                  &axg_ao_pclk_parents, _flags)
>> +
>> +static AXG_AO_GATE(remote,     0, CLK_IGNORE_UNUSED);
>> +static AXG_AO_GATE(i2c_master, 1, CLK_IGNORE_UNUSED);
>> +static AXG_AO_GATE(i2c_slave,  2, CLK_IGNORE_UNUSED);
>> +static AXG_AO_GATE(uart1,      3, CLK_IGNORE_UNUSED);
>> +static AXG_AO_GATE(uart2,      5, CLK_IGNORE_UNUSED);
>> +static AXG_AO_GATE(ir_blaster, 6, CLK_IGNORE_UNUSED);
>> +static AXG_AO_GATE(saradc,     7, CLK_IGNORE_UNUSED);
>>
>>   static struct clk_regmap axg_ao_cts_oscin = {
>>          .data = &(struct clk_regmap_gate_data){
>> diff --git a/drivers/clk/meson/c3-peripherals.c b/drivers/clk/meson/c3-peripherals.c
>> index e9c1ef99be13d0542b8a972ceffe69c8a9977118..02c9820cd98655e57a290859b595cf09d39e5fe3 100644
>> --- a/drivers/clk/meson/c3-peripherals.c
>> +++ b/drivers/clk/meson/c3-peripherals.c
>> @@ -164,30 +164,13 @@ static struct clk_regmap c3_rtc_clk = {
>>          },
>>   };
>>
>> -#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 = "c3_" #_name,                                   \
>> -               .ops = _ops,                                            \
>> -               .parent_data = &(const struct clk_parent_data) {        \
>> -                       .fw_name = (_fw_name),                          \
>> -               },                                                      \
>> -               .num_parents = 1,                                       \
>> -               .flags = (_flags),                                      \
>> -       },                                                              \
>> -}
>> +static const struct clk_parent_data c3_sys_pclk_parents = { .fw_name = "sysclk" };
>>
>> -#define C3_SYS_PCLK(_name, _reg, _bit, _flags)                         \
>> -       C3_PCLK(_name, _reg, _bit, "sysclk",                            \
>> -               &clk_regmap_gate_ops, _flags)
>> +#define C3_SYS_PCLK(_name, _reg, _bit, _flags) \
>> +       MESON_PCLK(c3_##_name, _reg, _bit, &c3_sys_pclk_parents, _flags)
>>
>> -#define C3_SYS_PCLK_RO(_name, _reg, _bit)                              \
>> -       C3_PCLK(_name, _reg, _bit, "sysclk",                            \
>> -               &clk_regmap_gate_ro_ops, 0)
>> +#define C3_SYS_PCLK_RO(_name, _reg, _bit) \
>> +       MESON_PCLK_RO(c3_##_name, _reg, _bit, &c3_sys_pclk_parents, 0)
>
>
> Adding 'SoC' prefix to clock names appears redundant and inconsistent - only
> 'sys_clk' carries this prefix while all other clock names don't.
>

The prefix is not added here but on patch 5 and I've replied there.

>
>>
>>   static C3_SYS_PCLK(sys_reset_ctrl,     SYS_CLK_EN0_REG0, 1, 0);
>>   static C3_SYS_PCLK(sys_pwr_ctrl,       SYS_CLK_EN0_REG0, 3, 0);
>> @@ -290,9 +273,10 @@ static C3_SYS_PCLK(sys_vc9000e,            SYS_CLK_EN0_REG2, 2, 0);
>>   static C3_SYS_PCLK(sys_pwm_mn,         SYS_CLK_EN0_REG2, 3, 0);
>>   static C3_SYS_PCLK(sys_sd_emmc_b,      SYS_CLK_EN0_REG2, 4, 0);
>>
>> -#define C3_AXI_PCLK(_name, _reg, _bit, _flags)                         \
>> -       C3_PCLK(_name, _reg, _bit, "axiclk",                            \
>> -               &clk_regmap_gate_ops, _flags)
>> +static const struct clk_parent_data c3_axi_pclk_parents = { .fw_name = "axiclk" };
>> +
>> +#define C3_AXI_PCLK(_name, _reg, _bit, _flags) \
>> +       MESON_PCLK(c3_##_name, _reg, _bit, &c3_axi_pclk_parents, _flags)
>>
>>   /*
>>    * NOTE: axi_sys_nic provides the clock to the AXI bus of the system NIC. After
>> diff --git a/drivers/clk/meson/g12a-aoclk.c b/drivers/clk/meson/g12a-aoclk.c
>> index 45e4df393feb6f916b6e035ad71e379e6e30ee99..96981da271fa1453ebbe433e36cff4409661fa6a 100644
>> --- a/drivers/clk/meson/g12a-aoclk.c
>> +++ b/drivers/clk/meson/g12a-aoclk.c
>> @@ -37,22 +37,10 @@
>>   #define AO_RTC_ALT_CLK_CNTL0   0x94
>>   #define AO_RTC_ALT_CLK_CNTL1   0x98
>>
>> -#define G12A_AO_PCLK(_name, _reg, _bit, _flags)                                \
>> -static struct clk_regmap g12a_ao_##_name = {                           \
>> -       .data = &(struct clk_regmap_gate_data) {                        \
>> -               .offset = (_reg),                                       \
>> -               .bit_idx = (_bit),                                      \
>> -       },                                                              \
>> -       .hw.init = &(struct clk_init_data) {                            \
>> -               .name =  "g12a_ao_" #_name,                             \
>> -               .ops = &clk_regmap_gate_ops,                            \
>> -               .parent_data = &(const struct clk_parent_data) {        \
>> -                       .fw_name = "mpeg-clk",                          \
>> -               },                                                      \
>> -               .num_parents = 1,                                       \
>> -               .flags = (_flags),                                      \
>> -       },                                                              \
>> -}
>> +static const struct clk_parent_data g12a_ao_pclk_parents = { .fw_name = "mpeg-clk" };
>> +
>> +#define G12A_AO_PCLK(_name, _reg, _bit, _flags) \
>> +       MESON_PCLK(g12a_ao_##_name, _reg, _bit, &g12a_ao_pclk_parents, _flags)
>>
>>   /*
>>    * NOTE: The gates below are marked with CLK_IGNORE_UNUSED for historic reasons
>> @@ -63,22 +51,22 @@ static struct clk_regmap g12a_ao_##_name = {                                \
>>    *  - add a comment explaining why the use of CLK_IGNORE_UNUSED is desirable
>>    *    for a particular clock.
>>    */
>> -G12A_AO_PCLK(ahb,      AO_CLK_GATE0,    0, CLK_IGNORE_UNUSED);
>> -G12A_AO_PCLK(ir_in,    AO_CLK_GATE0,    1, CLK_IGNORE_UNUSED);
>> -G12A_AO_PCLK(i2c_m0,   AO_CLK_GATE0,    2, CLK_IGNORE_UNUSED);
>> -G12A_AO_PCLK(i2c_s0,   AO_CLK_GATE0,    3, CLK_IGNORE_UNUSED);
>> -G12A_AO_PCLK(uart,     AO_CLK_GATE0,    4, CLK_IGNORE_UNUSED);
>> -G12A_AO_PCLK(prod_i2c, AO_CLK_GATE0,    5, CLK_IGNORE_UNUSED);
>> -G12A_AO_PCLK(uart2,    AO_CLK_GATE0,    6, CLK_IGNORE_UNUSED);
>> -G12A_AO_PCLK(ir_out,   AO_CLK_GATE0,    7, CLK_IGNORE_UNUSED);
>> -G12A_AO_PCLK(saradc,   AO_CLK_GATE0,    8, CLK_IGNORE_UNUSED);
>> +static G12A_AO_PCLK(ahb,       AO_CLK_GATE0,    0, CLK_IGNORE_UNUSED);
>> +static G12A_AO_PCLK(ir_in,     AO_CLK_GATE0,    1, CLK_IGNORE_UNUSED);
>> +static G12A_AO_PCLK(i2c_m0,    AO_CLK_GATE0,    2, CLK_IGNORE_UNUSED);
>> +static G12A_AO_PCLK(i2c_s0,    AO_CLK_GATE0,    3, CLK_IGNORE_UNUSED);
>> +static G12A_AO_PCLK(uart,      AO_CLK_GATE0,    4, CLK_IGNORE_UNUSED);
>> +static G12A_AO_PCLK(prod_i2c,  AO_CLK_GATE0,    5, CLK_IGNORE_UNUSED);
>> +static G12A_AO_PCLK(uart2,     AO_CLK_GATE0,    6, CLK_IGNORE_UNUSED);
>> +static G12A_AO_PCLK(ir_out,    AO_CLK_GATE0,    7, CLK_IGNORE_UNUSED);
>> +static G12A_AO_PCLK(saradc,    AO_CLK_GATE0,    8, CLK_IGNORE_UNUSED);
>>
>> -G12A_AO_PCLK(mailbox,  AO_CLK_GATE0_SP, 0, CLK_IGNORE_UNUSED);
>> -G12A_AO_PCLK(m3,       AO_CLK_GATE0_SP, 1, CLK_IGNORE_UNUSED);
>> -G12A_AO_PCLK(ahb_sram, AO_CLK_GATE0_SP, 2, CLK_IGNORE_UNUSED);
>> -G12A_AO_PCLK(rti,      AO_CLK_GATE0_SP, 3, CLK_IGNORE_UNUSED);
>> -G12A_AO_PCLK(m4_fclk,  AO_CLK_GATE0_SP, 4, CLK_IGNORE_UNUSED);
>> -G12A_AO_PCLK(m4_hclk,  AO_CLK_GATE0_SP, 5, CLK_IGNORE_UNUSED);
>> +static G12A_AO_PCLK(mailbox,   AO_CLK_GATE0_SP, 0, CLK_IGNORE_UNUSED);
>> +static G12A_AO_PCLK(m3,                AO_CLK_GATE0_SP, 1, CLK_IGNORE_UNUSED);
>> +static G12A_AO_PCLK(ahb_sram,  AO_CLK_GATE0_SP, 2, CLK_IGNORE_UNUSED);
>> +static G12A_AO_PCLK(rti,       AO_CLK_GATE0_SP, 3, CLK_IGNORE_UNUSED);
>> +static G12A_AO_PCLK(m4_fclk,   AO_CLK_GATE0_SP, 4, CLK_IGNORE_UNUSED);
>> +static G12A_AO_PCLK(m4_hclk,   AO_CLK_GATE0_SP, 5, CLK_IGNORE_UNUSED);
>>
>>   static struct clk_regmap g12a_ao_cts_oscin = {
>>          .data = &(struct clk_regmap_gate_data){
>> diff --git a/drivers/clk/meson/gxbb-aoclk.c b/drivers/clk/meson/gxbb-aoclk.c
>> index 2bf45fd7fe4ba0783e736fbbb126209870985b22..c7dfb3a06cb5f70c98f65bb91b937e1b870b34fe 100644
>> --- a/drivers/clk/meson/gxbb-aoclk.c
>> +++ b/drivers/clk/meson/gxbb-aoclk.c
>> @@ -23,29 +23,18 @@
>>   #define AO_RTC_ALT_CLK_CNTL0   0x94
>>   #define AO_RTC_ALT_CLK_CNTL1   0x98
>>
>> -#define GXBB_AO_PCLK(_name, _bit, _flags)                                      \
>> -static struct clk_regmap gxbb_ao_##_name = {                           \
>> -       .data = &(struct clk_regmap_gate_data) {                        \
>> -               .offset = AO_RTI_GEN_CNTL_REG0,                         \
>> -               .bit_idx = (_bit),                                      \
>> -       },                                                              \
>> -       .hw.init = &(struct clk_init_data) {                            \
>> -               .name = "gxbb_ao_" #_name,                              \
>> -               .ops = &clk_regmap_gate_ops,                            \
>> -               .parent_data = &(const struct clk_parent_data) {        \
>> -                       .fw_name = "mpeg-clk",                          \
>> -               },                                                      \
>> -               .num_parents = 1,                                       \
>> -               .flags = (_flags),                                      \
>> -       },                                                              \
>> -}
>> +static const struct clk_parent_data gxbb_ao_pclk_parents = { .fw_name = "mpeg-clk" };
>>
>> -GXBB_AO_PCLK(remote,           0, CLK_IGNORE_UNUSED);
>> -GXBB_AO_PCLK(i2c_master,       1, CLK_IGNORE_UNUSED);
>> -GXBB_AO_PCLK(i2c_slave,                2, CLK_IGNORE_UNUSED);
>> -GXBB_AO_PCLK(uart1,            3, CLK_IGNORE_UNUSED);
>> -GXBB_AO_PCLK(uart2,            5, CLK_IGNORE_UNUSED);
>> -GXBB_AO_PCLK(ir_blaster,       6, CLK_IGNORE_UNUSED);
>> +#define GXBB_AO_PCLK(_name, _bit, _flags)                      \
>> +       MESON_PCLK(gxbb_ao_##_name, AO_RTI_GEN_CNTL_REG0, _bit, \
>> +                  &gxbb_ao_pclk_parents, _flags)
>> +
>> +static GXBB_AO_PCLK(remote,    0, CLK_IGNORE_UNUSED);
>> +static GXBB_AO_PCLK(i2c_master,        1, CLK_IGNORE_UNUSED);
>> +static GXBB_AO_PCLK(i2c_slave, 2, CLK_IGNORE_UNUSED);
>> +static GXBB_AO_PCLK(uart1,     3, CLK_IGNORE_UNUSED);
>> +static GXBB_AO_PCLK(uart2,     5, CLK_IGNORE_UNUSED);
>> +static GXBB_AO_PCLK(ir_blaster,        6, CLK_IGNORE_UNUSED);
>>
>>   static struct clk_regmap gxbb_ao_cts_oscin = {
>>          .data = &(struct clk_regmap_gate_data){
>>
>> --
>> 2.47.2
>>
>>
>> _______________________________________________
>> linux-amlogic mailing list
>> linux-amlogic@...ts.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-amlogic

-- 
Jerome

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ