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

 A new MESON_PCLK_V2 macro is introduced for the sys gate clock. Its parent
is an SCMI clock. It belongs to another clock controller, and the parent
configuration is different from  that of MESON_PCLK. This avoids new macro
definition in the peripheral clock driver.

Signed-off-by: Jian Hu <redacted>
---
 drivers/clk/meson/clk-regmap.h | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/clk/meson/clk-regmap.h b/drivers/clk/meson/clk-regmap.h
index e365312da54e..61b8fc2d875f 100644
--- a/drivers/clk/meson/clk-regmap.h
+++ b/drivers/clk/meson/clk-regmap.h

This file is not meant for amlogic specific stuff. I know some found
their way in regardless but that's being fixed

I found macro MESON_PCLK is used in G12A/MESON8B/GXBB/S4.

you mean the macro will be fixed? and use something else instead.

 
 @@ -134,4 +134,28 @@ struct clk_regmap _name = {                                              \

 #define MESON_PCLK_RO(_name, _reg, _bit, _pname)     \
      __MESON_PCLK(_name, _reg, _bit, &clk_regmap_gate_ro_ops, _pname)
+
+#define __MESON_PCLK_V2(_name, _reg, _bit, _ops, _pname)             \
+struct clk_regmap _name = {                                          \
+     .data = &(struct clk_regmap_gate_data){                         \
+             .offset = (_reg),                                       \
+             .bit_idx = (_bit),                                      \
+     },                                                              \
+     .hw.init = &(struct clk_init_data) {                            \
+             .name = #_name,                                         \
+             .ops = _ops,                                            \
+             .parent_data = &(const struct clk_parent_data) {        \
+                     .fw_name = #_pname,                             \
+             },                                                      \
+             .num_parents = 1,                                       \
+             .flags = (CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED),     \
+     },                                                              \
+}


The proliferation of those macros has been going on for far too long,
add using CLK_IGNORE_UNUSED inside is certainly a mistake I won't
repeat.

This will be part of more general clean-up that currently depends on a
this [1] patch to go further. You'll have to be patient.

[1]: https://lore.kernel.org/r/20250417-clk-hw-get-helpers-v1-0-7743e509612a@baylibre.com (local)

OK, CLK_IGNORE_UNUSED is added for all gate clocks. it is better to remove it. and wait for your patch merged.

And  quote your comments in v2.  the T7_CLK_GATE is newly added in T7 Peripheral driver in v2.

If I understand you correctly.  A common macro should be added for it and remove T7_CLK_GATE.

So I add a common macro in clk-regmap.h, and it can be used for different SoCs.

MESON_PCLK and MESON_PCLK_V2 difference is a clocks's parent defination.

the sys gate clock parent is in another clock controller. So parent_data member is need for it.

and parent_hw member is for previous SoCs which clock and parent clock are in one clock controller.

How should we deal with it. 

1.Add the macro in T7 Perpheral driver.

2.Or add a common macro for parent_data,   and where the macro should be placed?

---

quoted

+#define T7_CLK_GATE(_name, _reg, _bit, _fw_name, _flags)		\

See, redefining the peripheral once again ... something all the SoCs
uses with minor variation.

quoted

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

quoted

+		.ops = &clk_regmap_gate_ops,				\
+		.parent_data = &(const struct clk_parent_data) {	\
+			.fw_name = #_fw_name,				\
+		},							\
+		.num_parents = 1,					\
+		.flags = (_flags),					\
+	},								\

---

 
 +
+#define MESON_PCLK_V2(_name, _reg, _bit, _pname)     \
+     __MESON_PCLK_V2(_name, _reg, _bit, &clk_regmap_gate_ops, _pname)
+
+#define MESON_PCLK_RO_V2(_name, _reg, _bit, _pname)  \
+     __MESON_PCLK_V2(_name, _reg, _bit, &clk_regmap_gate_ro_ops, _pname)
+
 #endif /* __CLK_REGMAP_H */