[<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