On 2025/5/14 16:00, Jerome Brunet wrote:

[ EXTERNAL EMAIL ]

On Fri 09 May 2025 at 07:48, Jian Hu [off-list ref] wrote:

quoted

Add Peripheral clock controller driver for the Amlogic T7 SoC family.

Signed-off-by: Jian Hu <redacted>
---
  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.

quoted

+
+#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.

quoted

+
+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.

quoted

......

--
Jerome