On 2025/1/14 2:18, Jerome Brunet wrote:
[ EXTERNAL EMAIL ]
On Wed 08 Jan 2025 at 17:40, Jian Hu <jian.hu@xxxxxxxxxxx> wrote:
Add Peripheral clock controller driver for the Amlogic T7 SoC family.
Signed-off-by: Jian Hu <jian.hu@xxxxxxxxxxx>
---
drivers/clk/meson/Kconfig | 13 +
drivers/clk/meson/Makefile | 1 +
drivers/clk/meson/t7-peripherals.c | 2323 ++++++++++++++++++++++++++++
3 files changed, 2337 insertions(+)
create mode 100644 drivers/clk/meson/t7-peripherals.c
......
+
+#define SPI_PWM_CLK_MUX(_name, _reg, _mask, _shift, _parent_data) { \
The same macros keeps getting defined again and again.
This has been going on for too long now.
I'm addressing the problem it will take a bit of time and I guess it
will delay t7 and a5 a bit.
Great, a common macro can be applied to c3/t7/a5. Wait for your good news.
+ .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, \
+ }, \
+}
+
+static const struct clk_parent_data 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 spicc0_sel =
+ SPI_PWM_CLK_MUX(spicc0, CLKCTRL_SPICC_CLK_CTRL, 0x7, 7, spicc_parents);
+static struct clk_regmap spicc0_div = SPI_PWM_CLK_DIV(spicc0, CLKCTRL_SPICC_CLK_CTRL, 0, 6, spicc0_sel);
+static struct clk_regmap spicc0 = SPI_PWM_CLK_GATE(spicc0, CLKCTRL_SPICC_CLK_CTRL, 6, spicc0_div);
+
......
+
+#define T7_CLK_GATE(_name, _reg, _bit, _fw_name, _flags) \
See, redefining the peripheral once again ... something all the SoCs
uses with minor variation.
A common macro is better for it.
there is common macro in clk/meson/clk-regmap.h, but CLK_IGNORE_UNUSED
flag is added.
it can not be used here. maybe we can rework the macro or new one for it.
+struct clk_regmap _name = { \
+ .data = &(struct clk_regmap_gate_data){ \
+ .offset = (_reg), \
+ .bit_idx = (_bit), \
+ }, \
+ .hw.init = &(struct clk_init_data) { \
+ .name = #_name, \
There is an exception in the naming convention for peripheral clocks.
The name is soc id prefixed in most SoC. It is these pointless minor
diff that makes factorisation difficult.
Yes. I think a common MESON_GATE or AMLOGIC_GATE is okay for it.
And how about this change for it? it remove the old defination and add
new one 'MESON_GATE' in clk-regmap.h :
--- a/drivers/clk/meson/clk-regmap.h
+++ b/drivers/clk/meson/clk-regmap.h
@@ -114,7 +114,7 @@ clk_get_regmap_mux_data(struct clk_regmap *clk)
extern const struct clk_ops clk_regmap_mux_ops;
extern const struct clk_ops clk_regmap_mux_ro_ops;
-#define __MESON_PCLK(_name, _reg, _bit, _ops, _pname) \
+#define __MESON_PCLK(_name, _reg, _bit, _ops, _pname, _flag) \
struct clk_regmap _name = { \
.data = &(struct clk_regmap_gate_data){ \
.offset = (_reg), \
@@ -125,13 +125,19 @@ struct clk_regmap _name =
{ \
.ops = _ops, \
.parent_hws = (const struct clk_hw *[]) { _pname }, \
.num_parents = 1, \
- .flags = (CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED), \
+ .flags = (CLK_SET_RATE_PARENT | flag), \
}, \
}
#define MESON_PCLK(_name, _reg, _bit, _pname) \
- __MESON_PCLK(_name, _reg, _bit, &clk_regmap_gate_ops, _pname)
+ __MESON_PCLK(_name, _reg, _bit, &clk_regmap_gate_ops, _pname,
CLK_IGNORE_UNUSED)
#define MESON_PCLK_RO(_name, _reg, _bit, _pname) \
- __MESON_PCLK(_name, _reg, _bit, &clk_regmap_gate_ro_ops, _pname)
+ __MESON_PCLK(_name, _reg, _bit, &clk_regmap_gate_ro_ops, _pname,
CLK_IGNORE_UNUSED)
+
+#define MESON_GATE(_name, _reg, _bit, _pname, _flag) \
+ __MESON_PCLK(_name, _reg, _bit, &clk_regmap_gate_ops, _pname, _flag)
+
+#define MESON_GATE_RO(_name, _reg, _bit, _pname, _flag) \
+ __MESON_PCLK(_name, _reg, _bit, &clk_regmap_gate_ro_ops, _pname,
_flag)
#endif /* __CLK_REGMAP_H */
diff --git a/drivers/clk/meson/t7-peripherals.c
b/drivers/clk/meson/t7-peripherals.c
index 362000fe4a7f..3a1aec703618 100644
--- a/drivers/clk/meson/t7-peripherals.c
+++ b/drivers/clk/meson/t7-peripherals.c
@@ -1750,25 +1750,10 @@ static struct clk_regmap t7_pwm_ao_h_div =
static struct clk_regmap t7_pwm_ao_h =
SPI_PWM_CLK_GATE(t7_pwm_ao_h, CLKCTRL_PWM_CLK_AO_GH_CTRL, 24,
t7_pwm_ao_h_div);
-#define T7_CLK_GATE(_name, _reg, _bit, _fw_name, _flags) \
-struct clk_regmap _name = { \
- .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_data = &(const struct clk_parent_data) { \
- .fw_name = #_fw_name, \
- }, \
- .num_parents = 1, \
- .flags = (_flags), \
- }, \
-}
+#define T7_SYS_GATE(_name, _reg, _bit, _flag) \
+ MESON_GATE(_name, _reg, _bit, &sys.hw, _flag)
-#define T7_SYS_GATE(_name, _reg, _bit, _flags) \
- T7_CLK_GATE(_name, _reg, _bit, sys, _flags)
+static T7_SYS_GATE(t7_sys_ddr, CLKCTRL_SYS_CLK_EN0_REG0, 0, 0);
or another patch based above. T7_SYS_GATE macro is not necessary, and
'&sys.hw' is needed for each sys clock.
and we can define the clocks like:
-#define T7_SYS_GATE(_name, _reg, _bit, _flag) \
- MESON_GATE(_name, _reg, _bit, &sys.hw, _flag)
+static MESON_GATE(t7_sys_ddr, CLKCTRL_SYS_CLK_EN0_REG0, 0, &sys.hw, 0);
+static MESON_GATE(t7_sys_dos, CLKCTRL_SYS_CLK_EN0_REG0, 0, &sys.hw, 1);
Please help to review it,
+ .ops = &clk_regmap_gate_ops, \
+ .parent_data = &(const struct clk_parent_data) { \
+ .fw_name = #_fw_name, \
+ }, \
+ .num_parents = 1, \
+ .flags = (_flags), \
+ }, \
+}
+
+#define T7_SYS_GATE(_name, _reg, _bit, _flags) \
+ T7_CLK_GATE(_name, _reg, _bit, sys, _flags)
+
+static T7_SYS_GATE(sys_ddr, CLKCTRL_SYS_CLK_EN0_REG0, 0, 0);
......
--
Jerome
