Re: [PATCH v4 06/27] clk: mediatek: clk-gate: Refactor mtk_clk_register_gate to use mtk_gate struct

On 8/15/25 05:42, Chen-Yu Tsai wrote:

On Tue, Aug 5, 2025 at 10:55 PM Laura Nao [off-list ref] wrote:

quoted

MT8196 uses a HW voter for gate enable/disable control, with
set/clr/sta registers located in a separate regmap. Refactor
mtk_clk_register_gate() to take a struct mtk_gate instead of individual
parameters, avoiding the need to add three extra arguments to support
HW voter register offsets.

Reviewed-by: Nícolas F. R. A. Prado <redacted>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Signed-off-by: Laura Nao <redacted>
---
 drivers/clk/mediatek/clk-gate.c | 35 ++++++++++++---------------------
 1 file changed, 13 insertions(+), 22 deletions(-)

diff --git a/drivers/clk/mediatek/clk-gate.c b/drivers/clk/mediatek/clk-gate.c
index 67d9e741c5e7..0375ccad4be3 100644
--- a/drivers/clk/mediatek/clk-gate.c
+++ b/drivers/clk/mediatek/clk-gate.c

@@ -152,12 +152,9 @@ const struct clk_ops mtk_clk_gate_ops_no_setclr_inv = {
 };
 EXPORT_SYMBOL_GPL(mtk_clk_gate_ops_no_setclr_inv);

-static struct clk_hw *mtk_clk_register_gate(struct device *dev, const char *name,
-                                        const char *parent_name,
-                                        struct regmap *regmap, int set_ofs,
-                                        int clr_ofs, int sta_ofs, u8 bit,
-                                        const struct clk_ops *ops,
-                                        unsigned long flags)
+static struct clk_hw *mtk_clk_register_gate(struct device *dev,
+                                               const struct mtk_gate *gate,
+                                               struct regmap *regmap)
 {
        struct mtk_clk_gate *cg;
        int ret;

@@ -167,17 +164,17 @@ static struct clk_hw *mtk_clk_register_gate(struct device *dev, const char *name
        if (!cg)
                return ERR_PTR(-ENOMEM);

-       init.name = name;
-       init.flags = flags | CLK_SET_RATE_PARENT;
-       init.parent_names = parent_name ? &parent_name : NULL;
-       init.num_parents = parent_name ? 1 : 0;
-       init.ops = ops;
+       init.name = gate->name;
+       init.flags = gate->flags | CLK_SET_RATE_PARENT;
+       init.parent_names = gate->parent_name ? &gate->parent_name : NULL;
+       init.num_parents = gate->parent_name ? 1 : 0;
+       init.ops = gate->ops;

        cg->regmap = regmap;
-       cg->set_ofs = set_ofs;
-       cg->clr_ofs = clr_ofs;
-       cg->sta_ofs = sta_ofs;
-       cg->bit = bit;
+       cg->set_ofs = gate->regs->set_ofs;
+       cg->clr_ofs = gate->regs->clr_ofs;
+       cg->sta_ofs = gate->regs->sta_ofs;
+       cg->bit = gate->shift;

I'd rather see |struct mtk_clk_gate| (the runtime data) gain a pointer
to the static data |struct mtk_gate| instead of doing all the copying.
This is just needless duplication.

Ack - I'll refactor this in the next revision.

Thanks,

Laura

ChenYu

quoted

        cg->hw.init = &init;

@@ -228,13 +225,7 @@ int mtk_clk_register_gates(struct device *dev, struct device_node *node,
                        continue;
                }

-               hw = mtk_clk_register_gate(dev, gate->name, gate->parent_name,
-                                           regmap,
-                                           gate->regs->set_ofs,
-                                           gate->regs->clr_ofs,
-                                           gate->regs->sta_ofs,
-                                           gate->shift, gate->ops,
-                                           gate->flags);
+               hw = mtk_clk_register_gate(dev, gate, regmap);

                if (IS_ERR(hw)) {
                        pr_err("Failed to register clk %s: %pe\n", gate->name,
--

2.39.5