| Message ID | 20241222170534.3621453-11-dario.binacchi@amarulasolutions.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
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@amarulasolutions.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)? >+{ >+#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 To unsubscribe from this group and stop receiving emails from it, send an email to linux-amarula+unsubscribe@amarulasolutions.com.
Hi Peng, On Tue, Dec 24, 2024 at 3:17 AM Peng Fan <peng.fan@oss.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@amarulasolutions.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@30380000 { compatible = "fsl,imx8mn-ccm"; ... fsl,anatop = <&anatop> } 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
> 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@oss.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@amarulasolutions.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@30380000 { > 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@amarulasolutions.com > > __________________________________ > > > Amarula Solutions SRL > > Via Le Canevare 30, 31100 Treviso, Veneto, IT > > T. +39 042 243 5310 > info@amarulasolutions.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 To unsubscribe from this group and stop receiving emails from it, send an email to linux-amarula+unsubscribe@amarulasolutions.com.
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) +struct clk_hw *imx8m_anatop_get_clk_hw(int id) +{ +#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); + args.np = np; + args.args_count = 1; + args.args[0] = id; + of_node_put(np); + + hw = __clk_get_hw(of_clk_get_from_provider(&args)); + pr_debug("%s: got clk: %s\n", __func__, clk_hw_get_name(hw)); + return hw; +} +EXPORT_SYMBOL_GPL(imx8m_anatop_get_clk_hw); +#endif + /* * This fixups the register CCM_CSCMR1 write value. * The write/read/divider values of the aclk_podf field diff --git a/drivers/clk/imx/clk.h b/drivers/clk/imx/clk.h index aa5202f284f3..52055fda3058 100644 --- a/drivers/clk/imx/clk.h +++ b/drivers/clk/imx/clk.h @@ -487,4 +487,11 @@ struct clk_hw *imx_clk_gpr_mux(const char *name, const char *compatible, u32 reg, const char **parent_names, u8 num_parents, const u32 *mux_table, u32 mask); +#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) +struct clk_hw *imx8m_anatop_get_clk_hw(int id); +#endif + #endif
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@amarulasolutions.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(+)