Best Regards!
Anson Huang

-----Original Message-----
From: Anson Huang
Sent: 2019年2月23日 11:04
To: Lucas Stach <l.stach@pengutronix.de>; robh+dt@kernel.org;
mark.rutland@arm.com; shawnguo@kernel.org; s.hauer@pengutronix.de;
kernel@pengutronix.de; festevam@gmail.com; mturquette@baylibre.com;
sboyd@kernel.org; Abel Vesa [off-list ref]; ccaione@baylibre.com;
agx@sigxcpu.org; devicetree@vger.kernel.org; linux-arm-
kernel@lists.infradead.org; linux-kernel@vger.kernel.org; linux-
clk@vger.kernel.org
Cc: dl-linux-imx <redacted>
Subject: RE: [PATCH 1/3] clk: imx8mq: initialize clock tree earlier

Hi, Lucas

Best Regards!
Anson Huang

quoted

-----Original Message-----
From: Lucas Stach [mailto:l.stach@pengutronix.de]
Sent: 2019年2月22日 18:55
To: Anson Huang <redacted>; robh+dt@kernel.org;
mark.rutland@arm.com; shawnguo@kernel.org; s.hauer@pengutronix.de;
kernel@pengutronix.de; festevam@gmail.com; mturquette@baylibre.com;
sboyd@kernel.org; Abel Vesa [off-list ref];

ccaione@baylibre.com;

quoted

agx@sigxcpu.org; devicetree@vger.kernel.org; linux-arm-
kernel@lists.infradead.org; linux-kernel@vger.kernel.org; linux-
clk@vger.kernel.org
Cc: dl-linux-imx <redacted>
Subject: Re: [PATCH 1/3] clk: imx8mq: initialize clock tree earlier

Am Freitag, den 22.02.2019, 09:42 +0000 schrieb Anson Huang:

quoted

Currently on i.MX8MQ platform, clock driver is probed later than
GPIO driver, and GPIO driver does NOT have defer probe mechanism
since the GPIO clock is optional,

So this is what should be fixed. If there is a clock reference in the
DT, having the clock driver ready is not optional. Optional to the
GPIO driver just means it won't fail if there is no clock reference at all.
If there is and the clock provider is not yet there, it needs to do a
PROBE_DEFER.

So that's a NACK on this patch.

OK, I see. Then I will do a patch for GPIO driver, and I just check other drivers,
looks like imx2 watchdog driver also has similar issue, I will do a patch too.

Just double check the watch dog driver, its clock is NOT optional, and just return
the error value, so it should be OK, sorry for the noise.

Anson.

Thanks,
Anson.

quoted

Regards,
Lucas

quoted

some platforms have GPIO clocks and some are NOT. So it is an issue
that on i.MX8MQ platform, there are GPIO clocks defined, but due to
clock tree is NOT ready during GPIO driver probe, the GPIO clock
management will fail and cause system hang if GPIO clocks are OFF by
default.

This patch changes the i.MX8MQ clock tree initialization using
CLK_OF_DECLARE instead of platform driver model to make clock tree
ready earlier than GPIO driver.

Signed-off-by: Anson Huang <redacted>
---
 drivers/clk/imx/clk-imx8mq.c | 33 +++++++--------------------------
 1 file changed, 7 insertions(+), 26 deletions(-)

diff --git a/drivers/clk/imx/clk-imx8mq.c b/drivers/clk/imx/clk-

imx8mq.c index 26b57f4..2df1575 100644

--- a/drivers/clk/imx/clk-imx8mq.c
+++ b/drivers/clk/imx/clk-imx8mq.c

@@ -269,10 +269,9 @@ static const char *imx8mq_clko2_sels[] =

{"osc_25m", "sys2_pll_200m", "sys1_pll_

 static struct clk_onecell_data clk_data;

-static int imx8mq_clocks_probe(struct platform_device *pdev)
+static void __init imx8mq_clocks_init(struct device_node *np)
 {
-	struct device *dev = &pdev->dev;
-	struct device_node *np = dev->of_node;
+	struct device_node *anatop_np;
 	void __iomem *base;
 	int err;
 	int i;

@@ -286,10 +285,10 @@ static int imx8mq_clocks_probe(struct

platform_device *pdev)
 	clks[IMX8MQ_CLK_EXT3] = of_clk_get_by_name(np, "clk_ext3");
 	clks[IMX8MQ_CLK_EXT4] = of_clk_get_by_name(np, "clk_ext4");

-	np = of_find_compatible_node(NULL, NULL, "fsl,imx8mq-anatop");
-	base = of_iomap(np, 0);
+	anatop_np = of_find_compatible_node(NULL, NULL, "fsl,imx8mq-
anatop");
+	base = of_iomap(anatop_np, 0);
 	if (WARN_ON(!base))
-		return -ENOMEM;
+		return;

 	clks[IMX8MQ_ARM_PLL_REF_SEL] = imx_clk_mux("arm_pll_ref_sel",

base +

quoted

0x28, 16, 2, pll_ref_sels, ARRAY_SIZE(pll_ref_sels));
 	clks[IMX8MQ_GPU_PLL_REF_SEL] = imx_clk_mux("gpu_pll_ref_sel",

base +

quoted

0x18, 16, 2, pll_ref_sels, ARRAY_SIZE(pll_ref_sels)); @@ -389,10
+388,9 @@ static int imx8mq_clocks_probe(struct platform_device
+*pdev)
 	clks[IMX8MQ_SYS2_PLL_500M] =
imx_clk_fixed_factor("sys2_pll_500m", "sys2_pll_out", 1, 2);
 	clks[IMX8MQ_SYS2_PLL_1000M] =
imx_clk_fixed_factor("sys2_pll_1000m", "sys2_pll_out", 1, 1);

-	np = dev->of_node;
 	base = of_iomap(np, 0);
 	if (WARN_ON(!base))
-		return -ENOMEM;
+		return;

 	/* CORE */
 	clks[IMX8MQ_CLK_A53_SRC] = imx_clk_mux2("arm_a53_src", base +
0x8000, 24, 3, imx8mq_a53_sels, ARRAY_SIZE(imx8mq_a53_sels)); @@
-568,22 +566,5 @@ static int imx8mq_clocks_probe(struct
platform_device *pdev)

 	err = of_clk_add_provider(np, of_clk_src_onecell_get, &clk_data);
 	WARN_ON(err);
-
-	return err;
 }
-
-static const struct of_device_id imx8mq_clk_of_match[] = {
-	{ .compatible = "fsl,imx8mq-ccm" },
-	{ /* Sentinel */ },
-};
-MODULE_DEVICE_TABLE(of, imx8mq_clk_of_match);
-
-
-static struct platform_driver imx8mq_clk_driver = {
-	.probe = imx8mq_clocks_probe,
-	.driver = {
-		.name = "imx8mq-ccm",
-		.of_match_table = of_match_ptr(imx8mq_clk_of_match),
-	},
-};
-module_platform_driver(imx8mq_clk_driver);
+CLK_OF_DECLARE(imx8mq, "fsl,imx8mq-ccm", imx8mq_clocks_init);

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel