linux-kernel - Re: [PATCH v4 06/27] clk: mediatek: clk-gate: Refactor mtk_clk_register_gate to use mtk

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20250825124900.209515-1-laura.nao@collabora.com>
Date: Mon, 25 Aug 2025 14:49:00 +0200
From: Laura Nao <laura.nao@...labora.com>
To: wenst@...omium.org
Cc: angelogioacchino.delregno@...labora.com,
	conor+dt@...nel.org,
	devicetree@...r.kernel.org,
	guangjie.song@...iatek.com,
	kernel@...labora.com,
	krzk+dt@...nel.org,
	laura.nao@...labora.com,
	linux-arm-kernel@...ts.infradead.org,
	linux-clk@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	linux-mediatek@...ts.infradead.org,
	matthias.bgg@...il.com,
	mturquette@...libre.com,
	netdev@...r.kernel.org,
	nfraprado@...labora.com,
	p.zabel@...gutronix.de,
	richardcochran@...il.com,
	robh@...nel.org,
	sboyd@...nel.org
Subject: 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 <laura.nao@...labora.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@...labora.com>
>> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
>> Signed-off-by: Laura Nao <laura.nao@...labora.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
>>