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: <CAGXv+5GxJs03EcMt0jm-x_fDuy_RtCrnOmyJvVVgAP9O9R6E2Q@mail.gmail.com>
Date: Fri, 15 Aug 2025 12:42:06 +0900
From: Chen-Yu Tsai <wenst@...omium.org>
To: Laura Nao <laura.nao@...labora.com>
Cc: mturquette@...libre.com, sboyd@...nel.org, robh@...nel.org, 
	krzk+dt@...nel.org, conor+dt@...nel.org, matthias.bgg@...il.com, 
	angelogioacchino.delregno@...labora.com, p.zabel@...gutronix.de, 
	richardcochran@...il.com, guangjie.song@...iatek.com, 
	linux-clk@...r.kernel.org, devicetree@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org, 
	linux-mediatek@...ts.infradead.org, netdev@...r.kernel.org, 
	kernel@...labora.com, Nícolas F . R . A . Prado <nfraprado@...labora.com>
Subject: Re: [PATCH v4 06/27] clk: mediatek: clk-gate: Refactor
 mtk_clk_register_gate to use mtk_gate struct

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.

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
>