Hi Peng,

On Tue, Dec 24, 2024 at 3:17 AM Peng Fan [off-list ref] wrote:

On Sun, Dec 22, 2024 at 06:04:25PM +0100, Dario Binacchi wrote:

quoted

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.

quoted

+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

quoted

+{
+#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.

quoted

+      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

www.amarulasolutions.com