linux-kernel - RE: [PATCH v6 10/18] clk: imx: add hw API imx8m_anatop_get_clk

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] [day] [month] [year] [list]

Message-ID:
 <PAXPR04MB8459F7FF003D8D43CB1BE94F880C2@PAXPR04MB8459.eurprd04.prod.outlook.com>
Date: Wed, 25 Dec 2024 00:59:40 +0000
From: Peng Fan <peng.fan@....com>
To: Dario Binacchi <dario.binacchi@...rulasolutions.com>, "Peng Fan (OSS)"
	<peng.fan@....nxp.com>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-amarula@...rulasolutions.com" <linux-amarula@...rulasolutions.com>,
	Abel Vesa <abelvesa@...nel.org>, Fabio Estevam <festevam@...il.com>, Michael
 Turquette <mturquette@...libre.com>, Pengutronix Kernel Team
	<kernel@...gutronix.de>, Sascha Hauer <s.hauer@...gutronix.de>, Shawn Guo
	<shawnguo@...nel.org>, Stephen Boyd <sboyd@...nel.org>, "imx@...ts.linux.dev"
	<imx@...ts.linux.dev>, "linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "linux-clk@...r.kernel.org"
	<linux-clk@...r.kernel.org>
Subject: RE: [PATCH v6 10/18] clk: imx: add hw API imx8m_anatop_get_clk_hw

> Subject: Re: [PATCH v6 10/18] clk: imx: add hw API
> imx8m_anatop_get_clk_hw
> 
> Hi Peng,
> 
> On Tue, Dec 24, 2024 at 3:17 AM Peng Fan <peng.fan@....nxp.com>
> wrote:
> >
> > On Sun, Dec 22, 2024 at 06:04:25PM +0100, Dario Binacchi wrote:
> > >Get the hw of a clock registered by the anatop module. This function
> > >is preparatory for future developments.
> > >
> > >Signed-off-by: Dario Binacchi
> <dario.binacchi@...rulasolutions.com>
> > >
> > >---
> > >
> > >(no changes since v5)
> > >
> > >Changes in v5:
> > >- Consider CONFIG_CLK_IMX8M{M,N,P,Q}_MODULE to fix
> compilation errors
> > >
> > >Changes in v4:
> > >- New
> > >
> > > drivers/clk/imx/clk.c | 28 ++++++++++++++++++++++++++++
> > > drivers/clk/imx/clk.h |  7 +++++++
> > > 2 files changed, 35 insertions(+)
> > >
> > >diff --git a/drivers/clk/imx/clk.c b/drivers/clk/imx/clk.c index
> > >df83bd939492..9a21f233e105 100644
> > >--- a/drivers/clk/imx/clk.c
> > >+++ b/drivers/clk/imx/clk.c
> > >@@ -128,6 +128,34 @@ struct clk_hw
> *imx_get_clk_hw_by_name(struct
> > >device_node *np, const char *name)  }
> > >EXPORT_SYMBOL_GPL(imx_get_clk_hw_by_name);
> > >
> > >+#if defined(CONFIG_CLK_IMX8MM) ||
> defined(CONFIG_CLK_IMX8MM_MODULE) || \
> > >+      defined(CONFIG_CLK_IMX8MN) ||
> defined(CONFIG_CLK_IMX8MN_MODULE) || \
> > >+      defined(CONFIG_CLK_IMX8MP) ||
> defined(CONFIG_CLK_IMX8MP_MODULE) || \
> > >+      defined(CONFIG_CLK_IMX8MQ) ||
> > >+defined(CONFIG_CLK_IMX8MQ_MODULE)
> >
> > drop the guarding macros, then this could be reused by i.MX9.
> >
> > >+struct clk_hw *imx8m_anatop_get_clk_hw(int id)
> >
> > how about change to
> > struct clk_hw *imx_anatop_get_clk_hw(int id, struct device_node
> *np)?
> 
> I designed this function so that the caller, i.e., the CCM driver, would no
> longer need to reference the anatop compatible in any way, but I agree
> with you that it would be better  to add the np parameter. Do you
> think it would then make sense to add a phandle to the  anatop node
> in the clk node?
> 
> clk: clock-controller@...80000 {
>     compatible = "fsl,imx8mn-ccm";
>     ...
>     fsl,anatop = <&anatop>
> }

This needs binding update, so needs DT maintainers to approve.
I am fine with it.

Regards,
Peng

> 
> So that we can call
> anatop_np = of_parse_phandle(np, "fsl,anatop", 0); instead of
> anatop_np = of_find_compatible_node(NULL, NULL, "fsl,imx8mn-
> anatop"); This would require an additional patch to
> Documentation/devicetree/bindings/clock/imx8m-clock.yaml,
> but it would make the CCM driver code more generic.
> 
> What do you think? I’m waiting for your response before sending
> version 7.
> 
> Thanks and regards,
> Dario
> 
> 
> 
> 
> 
> >
> > >+{
> > >+#if defined(CONFIG_CLK_IMX8MQ) ||
> defined(CONFIG_CLK_IMX8MQ_MODULE)
> > >+      const char *compatible = "fsl,imx8mq-anatop"; #else
> > >+      const char *compatible = "fsl,imx8mm-anatop"; #endif
> > >+      struct device_node *np;
> > >+      struct of_phandle_args args;
> > >+      struct clk_hw *hw;
> > >+
> > >+      np = of_find_compatible_node(NULL, NULL, compatible);
> >
> > Then no need to find the compatible for every function call.
> >
> > >+      args.np = np;
> > >+      args.args_count = 1;
> > >+      args.args[0] = id;
> > >+      of_node_put(np);
> > >+
> >
> > Thanks,
> > Peng
> 
> 
> 
> --
> 
> Dario Binacchi
> 
> Senior Embedded Linux Developer
> 
> dario.binacchi@...rulasolutions.com
> 
> __________________________________
> 
> 
> Amarula Solutions SRL
> 
> Via Le Canevare 30, 31100 Treviso, Veneto, IT
> 
> T. +39 042 243 5310
> info@...rulasolutions.com
> 
> https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2F
> www.amarulasolutions.com%2F&data=05%7C02%7Cpeng.fan%40nxp.
> com%7C4e8243a5b9f94a532c8d08dd24152922%7C686ea1d3bc2b4c
> 6fa92cd99c5c301635%7C0%7C0%7C638706395491848166%7CUnkn
> own%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMD
> AwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7
> C%7C%7C&sdata=0hMc9UwgFpa8VhOJvUVl3I3krgeFSzcB149zLRmHTx
> g%3D&reserved=0