On 8/15/25 05:42, Chen-Yu Tsai wrote:
> On Tue, Aug 5, 2025 at 10:55 PM Laura Nao <laura.nao@collabora.com> wrote:
>>
>> 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 <nfraprado@collabora.com>
>> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>> Signed-off-by: Laura Nao <laura.nao@collabora.com>
>> ---
>> 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
>
>> 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
>>