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] [day] [month] [year] [list]
Message-ID: <faf7d7bd-489a-4cc2-bb00-7df5980161cf@amlogic.com>
Date: Fri, 13 Jun 2025 17:39:03 +0800
From: Jian Hu <jian.hu@...ogic.com>
To: Jerome Brunet <jbrunet@...libre.com>
Cc: Xianwei Zhao <xianwei.zhao@...ogic.com>, Chuan Liu
 <chuan.liu@...ogic.com>, Neil Armstrong <neil.armstrong@...aro.org>,
 Kevin Hilman <khilman@...libre.com>, Stephen Boyd <sboyd@...nel.org>,
 Michael Turquette <mturquette@...libre.com>,
 Dmitry Rokosov <ddrokosov@...rdevices.ru>, robh+dt <robh+dt@...nel.org>,
 Rob Herring <robh@...nel.org>, devicetree <devicetree@...r.kernel.org>,
 linux-clk <linux-clk@...r.kernel.org>,
 linux-amlogic <linux-amlogic@...ts.infradead.org>,
 linux-kernel <linux-kernel@...r.kernel.org>,
 linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v3 6/6] clk: meson: t7: add t7 clock peripherals
 controller driver


On 2025/5/14 16:00, Jerome Brunet wrote:
> [ EXTERNAL EMAIL ]
>
> On Fri 09 May 2025 at 07:48, Jian Hu <jian.hu@...ogic.com> wrote:
>
>> Add Peripheral clock controller driver for the Amlogic T7 SoC family.
>>
>> Signed-off-by: Jian Hu <jian.hu@...ogic.com>
>> ---
>>   drivers/clk/meson/Kconfig          |   13 +
>>   drivers/clk/meson/Makefile         |    1 +
>>   drivers/clk/meson/t7-peripherals.c | 2359 ++++++++++++++++++++++++++++
>>   3 files changed, 2373 insertions(+)
>>   create mode 100644 drivers/clk/meson/t7-peripherals.c
>>
>> ......
>>
>> +
>> +static u32 t7_eth_rmii_table[] = { 0, 7 };
>> +
>> +static const struct clk_parent_data t7_eth_rmii_parents[] = {
>> +     { .fw_name = "fdiv2", },
>> +     { .fw_name = "rmii_pad", },
> Are you sure about that ? check the bindings


Ok, rmii_pad is optional parent, I will update it in DT bindings.

>> +
>> +#define SPI_PWM_CLK_MUX(_name, _reg, _mask, _shift, _parent_data) {  \
>> +     .data = &(struct clk_regmap_mux_data) {                 \
>> +             .offset = _reg,                                 \
>> +             .mask = _mask,                                  \
>> +             .shift = _shift,                                \
>> +     },                                                      \
>> +     .hw.init = &(struct clk_init_data) {                    \
>> +             .name = #_name "_sel",                          \
>> +             .ops = &clk_regmap_mux_ops,                     \
>> +             .parent_data = _parent_data,                    \
>> +             .num_parents = ARRAY_SIZE(_parent_data),        \
>> +     },                                                      \
>> +}
>> +
>> +#define SPI_PWM_CLK_DIV(_name, _reg, _shift, _width, _parent) {      \
>> +     .data = &(struct clk_regmap_div_data) {                 \
>> +             .offset = _reg,                                 \
>> +             .shift = _shift,                                \
>> +             .width = _width,                                \
>> +     },                                                      \
>> +     .hw.init = &(struct clk_init_data) {                    \
>> +             .name = #_name "_div",                          \
>> +             .ops = &clk_regmap_divider_ops,                 \
>> +             .parent_hws = (const struct clk_hw *[]) {       \
>> +                     &_parent.hw                             \
>> +             },                                              \
>> +             .num_parents = 1,                               \
>> +             .flags = CLK_SET_RATE_PARENT,                   \
>> +     },                                                      \
>> +}
>> +
>> +#define SPI_PWM_CLK_GATE(_name, _reg, _bit, _parent) {               \
>> +     .data = &(struct clk_regmap_gate_data) {                \
>> +             .offset = _reg,                                 \
>> +             .bit_idx = _bit,                                \
>> +     },                                                      \
>> +     .hw.init = &(struct clk_init_data) {                    \
>> +             .name = #_name,                                 \
>> +             .ops = &clk_regmap_gate_ops,                    \
>> +             .parent_hws = (const struct clk_hw *[]) {       \
>> +                     &_parent.hw                             \
>> +             },                                              \
>> +             .num_parents = 1,                               \
>> +             .flags = CLK_SET_RATE_PARENT,                   \
>> +     },                                                      \
>> +}
>
> Again that something that has been repeated for way too long.
> You'll wait for the clean-up to be done. If you want to help, you can
> review and test the patch being sent. It may speed things up.


I want to confirm here .

you said the clean-up patch is not related to spi and pwm clocks, Right ?


I can see the regmap drop table patch [0], is it the sending patch?


[0]: 
https://patchwork.kernel.org/project/linux-amlogic/patch/20250120-amlogic-clk-drop-clk-regmap-tables-v3-0-126244146947@baylibre.com/


I will apply the regmap clean-up patch serial and verified it on T7.


If not, Please correct me.

>> +
>> +static const struct clk_parent_data t7_spicc_parents[] = {
>> +     { .fw_name = "xtal", },
>> +     { .fw_name = "sys", },
>> +     { .fw_name = "fdiv4", },
>> +     { .fw_name = "fdiv3", },
>> +     { .fw_name = "fdiv2", },
>> +     { .fw_name = "fdiv5", },
>> +     { .fw_name = "fdiv7", },
>> +     { .fw_name = "gp1", },
>> +};
>> +
>> ......
>> +static struct clk_regmap t7_sys_gic = {
>> +     .data = &(struct clk_regmap_gate_data) {
>> +             .offset = CLKCTRL_SYS_CLK_EN0_REG2,
>> +             .bit_idx = 30,
>> +     },
>> +     .hw.init = &(struct clk_init_data){
>> +             .name = "t7_sys_gic",
>> +             .ops = &clk_regmap_gate_ops,
>> +             .parent_data = &(const struct clk_parent_data) {
>> +                     .fw_name = "sys",
>> +             },
>> +             .num_parents = 1,
>> +             .flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
> Do you really intend to for the rate of the sys pll to be set through
> this clock ?


Ok, CLK_SET_RATE_PARENT is not necessary here, I will remove it.

>> ......
> --
> Jerome

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ