linux-kernel - Re: [PATCH v2 5/5] clk: mediatek: Add mfg

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 for Android: free password hash cracker in your pocket

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

Message-ID: <6d3e4b81-b8fc-45b3-a12f-b8492d8dc892@collabora.com>
Date: Thu, 9 Oct 2025 10:22:28 +0200
From: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
To: Nicolas Frattaroli <nicolas.frattaroli@...labora.com>,
 Michael Turquette <mturquette@...libre.com>, Stephen Boyd
 <sboyd@...nel.org>, Dong Aisheng <aisheng.dong@....com>,
 Matthias Brugger <matthias.bgg@...il.com>,
 Yassine Oudjana <y.oudjana@...tonmail.com>,
 Laura Nao <laura.nao@...labora.com>,
 Nícolas F. R. A. Prado <nfraprado@...labora.com>,
 Chia-I Wu <olvaffe@...il.com>, Chen-Yu Tsai <wenst@...omium.org>
Cc: kernel@...labora.com, linux-clk@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 linux-mediatek@...ts.infradead.org, Stephen Boyd <sboyd@...eaurora.org>
Subject: Re: [PATCH v2 5/5] clk: mediatek: Add mfg_eb as parent to mt8196
 mfgpll clocks

Il 08/10/25 18:05, Nicolas Frattaroli ha scritto:
> All the MFGPLL require MFG_EB to be on for any operation on them, and
> they only tick when MFG_EB is on as well, therefore making this a
> parent-child relationship.
> 
> This dependency wasn't clear during the initial upstreaming of these
> clock controllers, as it only made itself known when I could observe
> the effects of the clock by bringing up a different piece of hardware.
> 
> Add a new PLL_PARENT_EN flag to mediatek's clk-pll.h, and check for it
> when initialising the pll to then translate it into the actual
> CLK_OPS_PARENT_ENABLE flag.
> 
> Then add the mfg_eb parent to the mfgpll clocks, and set the new
> PLL_PARENT_EN flag.
> 
> Fixes: 03dc02f8c7dc ("clk: mediatek: Add MT8196 mfg clock support")
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@...labora.com>
> ---
>   drivers/clk/mediatek/clk-mt8196-mfg.c | 3 ++-
>   drivers/clk/mediatek/clk-pll.c        | 3 +++
>   drivers/clk/mediatek/clk-pll.h        | 1 +
>   3 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/mediatek/clk-mt8196-mfg.c b/drivers/clk/mediatek/clk-mt8196-mfg.c
> index 8e09c0f7b7548f8e286671cea2dac64530b8ce47..de6f426f148184e1bb95b5cfca590b1763fc0573 100644
> --- a/drivers/clk/mediatek/clk-mt8196-mfg.c
> +++ b/drivers/clk/mediatek/clk-mt8196-mfg.c
> @@ -45,7 +45,7 @@
>   		.en_reg = _en_reg,				\
>   		.en_mask = _en_mask,				\
>   		.pll_en_bit = _pll_en_bit,			\
> -		.flags = _flags,				\
> +		.flags = (_flags) | PLL_PARENT_EN,		\

In the event that we may want, one day, maybe commonize the PLL macro to all of the
PLL drivers, can you please add the PLL_PARENT_EN to the three clocks instead of
doing it in the macro itself?

static const struct mtk_pll_data mfg_ao_plls[] = {
	PLL(CLK_MFG_AO_MFGPLL, "mfgpll", MFGPLL_CON0, MFGPLL_CON0, 0, 0,
	    PLL_PARENT_EN, BIT(0), MFGPLL_CON1, 24, 0, 0, 0,
	    MFGPLL_CON1, 0, 22),
};

static const struct mtk_pll_data mfgsc0_ao_plls[] = {
	PLL(CLK_MFGSC0_AO_MFGPLL_SC0, "mfgpll-sc0", MFGPLL_SC0_CON0,
	    MFGPLL_SC0_CON0, 0, 0, PLL_PARENT_EN, BIT(0),
	    MFGPLL_SC0_CON1, 24, 0, 0, 0, MFGPLL_SC0_CON1, 0, 22),
};

static const struct mtk_pll_data mfgsc1_ao_plls[] = {
	PLL(CLK_MFGSC1_AO_MFGPLL_SC1, "mfgpll-sc1", MFGPLL_SC1_CON0,
	    MFGPLL_SC1_CON0, 0, 0, PLL_PARENT_EN, BIT(0),
	    MFGPLL_SC1_CON1, 24, 0, 0, 0, MFGPLL_SC1_CON1, 0, 22),
};


After which,

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>

>   		.rst_bar_mask = _rst_bar_mask,			\
>   		.fmax = MT8196_PLL_FMAX,			\
>   		.fmin = MT8196_PLL_FMIN,			\
> @@ -58,6 +58,7 @@
>   		.pcw_shift = _pcw_shift,			\
>   		.pcwbits = _pcwbits,				\
>   		.pcwibits = MT8196_INTEGER_BITS,		\
> +		.parent_name = "mfg_eb",			\
>   	}
>   
>   static const struct mtk_pll_data mfg_ao_plls[] = {
> diff --git a/drivers/clk/mediatek/clk-pll.c b/drivers/clk/mediatek/clk-pll.c
> index c4f9c06e5133dbc5902f261353c197fbde95e54d..0f3759fcd9d0228c23f4916d041d17b731a6c838 100644
> --- a/drivers/clk/mediatek/clk-pll.c
> +++ b/drivers/clk/mediatek/clk-pll.c
> @@ -359,6 +359,9 @@ struct clk_hw *mtk_clk_register_pll_ops(struct mtk_clk_pll *pll,
>   
>   	init.name = data->name;
>   	init.flags = (data->flags & PLL_AO) ? CLK_IS_CRITICAL : 0;
> +	if (data->flags & PLL_PARENT_EN)
> +		init.flags |= CLK_OPS_PARENT_ENABLE;
> +
>   	init.ops = pll_ops;
>   	if (data->parent_name)
>   		init.parent_names = &data->parent_name;
> diff --git a/drivers/clk/mediatek/clk-pll.h b/drivers/clk/mediatek/clk-pll.h
> index 0f2a1d19eea78b7390b221af47016eb9897f3596..492cad8ff80ba31a78a96085285cb938e9b978e9 100644
> --- a/drivers/clk/mediatek/clk-pll.h
> +++ b/drivers/clk/mediatek/clk-pll.h
> @@ -21,6 +21,7 @@ struct mtk_pll_div_table {
>   
>   #define HAVE_RST_BAR	BIT(0)
>   #define PLL_AO		BIT(1)
> +#define PLL_PARENT_EN	BIT(2)
>   #define POSTDIV_MASK	GENMASK(2, 0)
>   
>   struct mtk_pll_data {
>