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: <748f1176-c73c-48af-a1af-0b63d088e834@collabora.com>
Date: Wed, 1 Oct 2025 13:49:54 +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>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, Matthias Brugger <matthias.bgg@...il.com>,
 Guangjie Song <guangjie.song@...iatek.com>,
 Laura Nao <laura.nao@...labora.com>,
 NĂ­colas F. R. A. Prado <nfraprado@...labora.com>,
 Yassine Oudjana <y.oudjana@...tonmail.com>
Cc: kernel@...labora.com, Krzysztof Kozlowski
 <krzysztof.kozlowski@...aro.org>, 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
Subject: Re: [PATCH 4/4] clk: mediatek: Add rpm clocks to clk-mt8196-mfg

Il 29/09/25 14:13, Nicolas Frattaroli ha scritto:
> The mfgpll clocks on mt8196 require that the MFG's EB clock is on if
> their control registers are touched in any way at all.

....so, the MFGPLL clocks are children of EB?

Why are you using such a convoluted way of adding a parent clock to the MFGPLL
instead of just
---->   `.parent_name = "mfg_eb"`  <-----

???????

Cheers,
Angelo

> Failing to ensure
> this results in a pleasant SError interrupt if the EB clock happens to
> be off.
> 
> To achieve this, leverage the CCF core's runtime power management
> support. Define the necessary suspend/resume callbacks, add the
> necessary code to get RPM clocks from the DT, and make sure RPM is
> enabled before clock registration happens.
> 
> For the RPM callbacks to really make much sense at all, we change the
> drvdata from clk_data to a new private struct, as is common in drivers
> across the Linux kernel.
> 
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@...labora.com>
> ---
>   drivers/clk/mediatek/clk-mt8196-mfg.c | 104 +++++++++++++++++++++++++++-------
>   drivers/clk/mediatek/clk-pll.h        |   2 +
>   2 files changed, 87 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/clk/mediatek/clk-mt8196-mfg.c b/drivers/clk/mediatek/clk-mt8196-mfg.c
> index 8e09c0f7b7548f8e286671cea2dac64530b8ce47..64cc0c037f62d7eab8d0e7fc00c05d122bf4130c 100644
> --- a/drivers/clk/mediatek/clk-mt8196-mfg.c
> +++ b/drivers/clk/mediatek/clk-mt8196-mfg.c
> @@ -13,6 +13,7 @@
>   #include <linux/of_address.h>
>   #include <linux/of_device.h>
>   #include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
>   
>   #include "clk-mtk.h"
>   #include "clk-pll.h"
> @@ -38,7 +39,7 @@
>   	    _flags, _rst_bar_mask,				\
>   	    _pd_reg, _pd_shift, _tuner_reg,			\
>   	    _tuner_en_reg, _tuner_en_bit,			\
> -	    _pcw_reg, _pcw_shift, _pcwbits) {			\
> +	    _pcw_reg, _pcw_shift, _pcwbits, _rpm_clks) {	\
>   		.id = _id,					\
>   		.name = _name,					\
>   		.reg = _reg,					\
> @@ -58,26 +59,60 @@
>   		.pcw_shift = _pcw_shift,			\
>   		.pcwbits = _pcwbits,				\
>   		.pcwibits = MT8196_INTEGER_BITS,		\
> +		.rpm_clk_names = _rpm_clks,			\
> +		.num_rpm_clks = ARRAY_SIZE(_rpm_clks),		\
>   	}
>   
> +static const char * const mfgpll_rpm_clk_names[] = {
> +	NULL
> +};
> +
>   static const struct mtk_pll_data mfg_ao_plls[] = {
>   	PLL(CLK_MFG_AO_MFGPLL, "mfgpll", MFGPLL_CON0, MFGPLL_CON0, 0, 0, 0,
> -	    BIT(0), MFGPLL_CON1, 24, 0, 0, 0,
> -	    MFGPLL_CON1, 0, 22),
> +	    BIT(0), MFGPLL_CON1, 24, 0, 0, 0, MFGPLL_CON1, 0, 22,
> +	    mfgpll_rpm_clk_names),
>   };
>   
>   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, 0, BIT(0), MFGPLL_SC0_CON1, 24, 0, 0, 0,
> -	    MFGPLL_SC0_CON1, 0, 22),
> +	    MFGPLL_SC0_CON1, 0, 22, mfgpll_rpm_clk_names),
>   };
>   
>   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, 0, BIT(0), MFGPLL_SC1_CON1, 24, 0, 0, 0,
> -	    MFGPLL_SC1_CON1, 0, 22),
> +	    MFGPLL_SC1_CON1, 0, 22, mfgpll_rpm_clk_names),
>   };
>   
> +struct clk_mt8196_mfg {
> +	struct clk_hw_onecell_data *clk_data;
> +	struct clk_bulk_data *rpm_clks;
> +	unsigned int num_rpm_clks;
> +};
> +
> +static int __maybe_unused clk_mt8196_mfg_resume(struct device *dev)
> +{
> +	struct clk_mt8196_mfg *clk_mfg = dev_get_drvdata(dev);
> +
> +	if (!clk_mfg || !clk_mfg->rpm_clks)
> +		return 0;
> +
> +	return clk_bulk_prepare_enable(clk_mfg->num_rpm_clks, clk_mfg->rpm_clks);
> +}
> +
> +static int __maybe_unused clk_mt8196_mfg_suspend(struct device *dev)
> +{
> +	struct clk_mt8196_mfg *clk_mfg = dev_get_drvdata(dev);
> +
> +	if (!clk_mfg || !clk_mfg->rpm_clks)
> +		return 0;
> +
> +	clk_bulk_disable_unprepare(clk_mfg->num_rpm_clks, clk_mfg->rpm_clks);
> +
> +	return 0;
> +}
> +
>   static const struct of_device_id of_match_clk_mt8196_mfg[] = {
>   	{ .compatible = "mediatek,mt8196-mfgpll-pll-ctrl",
>   	  .data = &mfg_ao_plls },
> @@ -92,35 +127,60 @@ MODULE_DEVICE_TABLE(of, of_match_clk_mt8196_mfg);
>   static int clk_mt8196_mfg_probe(struct platform_device *pdev)
>   {
>   	const struct mtk_pll_data *plls;
> -	struct clk_hw_onecell_data *clk_data;
> +	struct clk_mt8196_mfg *clk_mfg;
>   	struct device_node *node = pdev->dev.of_node;
> +	struct device *dev = &pdev->dev;
>   	const int num_plls = 1;
> -	int r;
> +	int r, i;
>   
> -	plls = of_device_get_match_data(&pdev->dev);
> +	plls = of_device_get_match_data(dev);
>   	if (!plls)
>   		return -EINVAL;
>   
> -	clk_data = mtk_alloc_clk_data(num_plls);
> -	if (!clk_data)
> +	clk_mfg = devm_kzalloc(dev, sizeof(*clk_mfg), GFP_KERNEL);
> +	if (!clk_mfg)
>   		return -ENOMEM;
>   
> -	r = mtk_clk_register_plls(&pdev->dev, plls, num_plls, clk_data);
> +	clk_mfg->num_rpm_clks = plls[0].num_rpm_clks;
> +
> +	if (clk_mfg->num_rpm_clks) {
> +		clk_mfg->rpm_clks = devm_kcalloc(dev, clk_mfg->num_rpm_clks,
> +						 sizeof(*clk_mfg->rpm_clks),
> +						 GFP_KERNEL);
> +		if (!clk_mfg->rpm_clks)
> +			return -ENOMEM;
> +
> +		for (i = 0; i < clk_mfg->num_rpm_clks; i++)
> +			clk_mfg->rpm_clks->id = plls[0].rpm_clk_names[i];
> +
> +		r = devm_clk_bulk_get(dev, clk_mfg->num_rpm_clks,
> +				      clk_mfg->rpm_clks);
> +		if (r)
> +			return r;
> +	}
> +
> +	clk_mfg->clk_data = mtk_alloc_clk_data(num_plls);
> +	if (!clk_mfg->clk_data)
> +		return -ENOMEM;
> +
> +	dev_set_drvdata(dev, clk_mfg);
> +	pm_runtime_enable(dev);
> +
> +	r = mtk_clk_register_plls(dev, plls, num_plls, clk_mfg->clk_data);
>   	if (r)
>   		goto free_clk_data;
>   
> -	r = of_clk_add_hw_provider(node, of_clk_hw_onecell_get, clk_data);
> +	r = of_clk_add_hw_provider(node, of_clk_hw_onecell_get,
> +				   clk_mfg->clk_data);
>   	if (r)
>   		goto unregister_plls;
>   
> -	platform_set_drvdata(pdev, clk_data);
> -
>   	return r;
>   
>   unregister_plls:
> -	mtk_clk_unregister_plls(plls, num_plls, clk_data);
> +	mtk_clk_unregister_plls(plls, num_plls, clk_mfg->clk_data);
>   free_clk_data:
> -	mtk_free_clk_data(clk_data);
> +	mtk_free_clk_data(clk_mfg->clk_data);
>   
>   	return r;
>   }
> @@ -128,20 +188,26 @@ static int clk_mt8196_mfg_probe(struct platform_device *pdev)
>   static void clk_mt8196_mfg_remove(struct platform_device *pdev)
>   {
>   	const struct mtk_pll_data *plls = of_device_get_match_data(&pdev->dev);
> -	struct clk_hw_onecell_data *clk_data = platform_get_drvdata(pdev);
> +	struct clk_mt8196_mfg *clk_mfg = dev_get_drvdata(&pdev->dev);
>   	struct device_node *node = pdev->dev.of_node;
>   
>   	of_clk_del_provider(node);
> -	mtk_clk_unregister_plls(plls, 1, clk_data);
> -	mtk_free_clk_data(clk_data);
> +	mtk_clk_unregister_plls(plls, 1, clk_mfg->clk_data);
> +	mtk_free_clk_data(clk_mfg->clk_data);
>   }
>   
> +static DEFINE_RUNTIME_DEV_PM_OPS(clk_mt8196_mfg_pm_ops,
> +				 clk_mt8196_mfg_suspend,
> +				 clk_mt8196_mfg_resume,
> +				 NULL);
> +
>   static struct platform_driver clk_mt8196_mfg_drv = {
>   	.probe = clk_mt8196_mfg_probe,
>   	.remove = clk_mt8196_mfg_remove,
>   	.driver = {
>   		.name = "clk-mt8196-mfg",
>   		.of_match_table = of_match_clk_mt8196_mfg,
> +		.pm = pm_ptr(&clk_mt8196_mfg_pm_ops),
>   	},
>   };
>   module_platform_driver(clk_mt8196_mfg_drv);
> diff --git a/drivers/clk/mediatek/clk-pll.h b/drivers/clk/mediatek/clk-pll.h
> index 0f2a1d19eea78b7390b221af47016eb9897f3596..82b86b849a67359d8f23d828f50422081c2747e3 100644
> --- a/drivers/clk/mediatek/clk-pll.h
> +++ b/drivers/clk/mediatek/clk-pll.h
> @@ -53,6 +53,8 @@ struct mtk_pll_data {
>   	u8 pll_en_bit; /* Assume 0, indicates BIT(0) by default */
>   	u8 pcw_chg_bit;
>   	u8 fenc_sta_bit;
> +	const char * const *rpm_clk_names;
> +	unsigned int num_rpm_clks;
>   };
>   
>   /*
> 



Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ