[PATCH 1/3] clk: imx8mq: initialize clock tree earlier

[PATCH 1/3] clk: imx8mq: initialize clock tree earlier

[Anson Huang]

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,
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 <Anson.Huang@nxp.com>
---
 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 + 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 + 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);
-- 
2.7.4

[PATCH 2/3] clk: imx8mq: add GPIO clocks to clock tree

[Anson Huang]

i.MX8MQ has clock gate for each GPIO bank, add them
into clock tree for GPIO driver to manage.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
 drivers/clk/imx/clk-imx8mq.c             | 5 +++++
 include/dt-bindings/clock/imx8mq-clock.h | 8 +++++++-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/imx/clk-imx8mq.c b/drivers/clk/imx/clk-imx8mq.c
index 2df1575..5839e45 100644
--- a/drivers/clk/imx/clk-imx8mq.c
+++ b/drivers/clk/imx/clk-imx8mq.c
@@ -498,6 +498,11 @@ static void __init imx8mq_clocks_init(struct device_node *np)
 	clks[IMX8MQ_CLK_ECSPI2_ROOT] = imx_clk_gate4("ecspi2_root_clk", "ecspi2", base + 0x4080, 0);
 	clks[IMX8MQ_CLK_ECSPI3_ROOT] = imx_clk_gate4("ecspi3_root_clk", "ecspi3", base + 0x4090, 0);
 	clks[IMX8MQ_CLK_ENET1_ROOT] = imx_clk_gate4("enet1_root_clk", "enet_axi", base + 0x40a0, 0);
+	clks[IMX8MQ_CLK_GPIO1_ROOT] = imx_clk_gate4("gpio1_root_clk", "ipg_root", base + 0x40b0, 0);
+	clks[IMX8MQ_CLK_GPIO2_ROOT] = imx_clk_gate4("gpio2_root_clk", "ipg_root", base + 0x40c0, 0);
+	clks[IMX8MQ_CLK_GPIO3_ROOT] = imx_clk_gate4("gpio3_root_clk", "ipg_root", base + 0x40d0, 0);
+	clks[IMX8MQ_CLK_GPIO4_ROOT] = imx_clk_gate4("gpio4_root_clk", "ipg_root", base + 0x40e0, 0);
+	clks[IMX8MQ_CLK_GPIO5_ROOT] = imx_clk_gate4("gpio5_root_clk", "ipg_root", base + 0x40f0, 0);
 	clks[IMX8MQ_CLK_GPT1_ROOT] = imx_clk_gate4("gpt1_root_clk", "gpt1", base + 0x4100, 0);
 	clks[IMX8MQ_CLK_I2C1_ROOT] = imx_clk_gate4("i2c1_root_clk", "i2c1", base + 0x4170, 0);
 	clks[IMX8MQ_CLK_I2C2_ROOT] = imx_clk_gate4("i2c2_root_clk", "i2c2", base + 0x4180, 0);
diff --git a/include/dt-bindings/clock/imx8mq-clock.h b/include/dt-bindings/clock/imx8mq-clock.h
index 04f7ac3..aff3335 100644
--- a/include/dt-bindings/clock/imx8mq-clock.h
+++ b/include/dt-bindings/clock/imx8mq-clock.h
@@ -391,5 +391,11 @@
 
 #define IMX8MQ_CLK_NAND_USDHC_BUS_RAWNAND_CLK	267
 
-#define IMX8MQ_CLK_END				268
+#define IMX8MQ_CLK_GPIO1_ROOT			268
+#define IMX8MQ_CLK_GPIO2_ROOT			269
+#define IMX8MQ_CLK_GPIO3_ROOT			270
+#define IMX8MQ_CLK_GPIO4_ROOT			271
+#define IMX8MQ_CLK_GPIO5_ROOT			272
+
+#define IMX8MQ_CLK_END				273
 #endif /* __DT_BINDINGS_CLOCK_IMX8MQ_H */
-- 
2.7.4

[PATCH 3/3] arm64: dts: freescale: imx8mq: add clock for GPIO node

[Anson Huang]

i.MX8MQ has clock gate for each GPIO bank, add clock info
to GPIO node for clock management.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
 arch/arm64/boot/dts/freescale/imx8mq.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
index 9155bd4..4c1fca9 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
@@ -160,6 +160,7 @@
 				reg = <0x30200000 0x10000>;
 				interrupts = <GIC_SPI 64 IRQ_TYPE_LEVEL_HIGH>,
 				             <GIC_SPI 65 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&clk IMX8MQ_CLK_GPIO1_ROOT>;
 				gpio-controller;
 				#gpio-cells = <2>;
 				interrupt-controller;
@@ -171,6 +172,7 @@
 				reg = <0x30210000 0x10000>;
 				interrupts = <GIC_SPI 66 IRQ_TYPE_LEVEL_HIGH>,
 				             <GIC_SPI 67 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&clk IMX8MQ_CLK_GPIO2_ROOT>;
 				gpio-controller;
 				#gpio-cells = <2>;
 				interrupt-controller;
@@ -182,6 +184,7 @@
 				reg = <0x30220000 0x10000>;
 				interrupts = <GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>,
 				             <GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&clk IMX8MQ_CLK_GPIO3_ROOT>;
 				gpio-controller;
 				#gpio-cells = <2>;
 				interrupt-controller;
@@ -193,6 +196,7 @@
 				reg = <0x30230000 0x10000>;
 				interrupts = <GIC_SPI 70 IRQ_TYPE_LEVEL_HIGH>,
 				             <GIC_SPI 71 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&clk IMX8MQ_CLK_GPIO4_ROOT>;
 				gpio-controller;
 				#gpio-cells = <2>;
 				interrupt-controller;
@@ -204,6 +208,7 @@
 				reg = <0x30240000 0x10000>;
 				interrupts = <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>,
 				             <GIC_SPI 73 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&clk IMX8MQ_CLK_GPIO5_ROOT>;
 				gpio-controller;
 				#gpio-cells = <2>;
 				interrupt-controller;
-- 
2.7.4

RE: [PATCH 1/3] clk: imx8mq: initialize clock tree earlier

[Aisheng Dong]

Hi Anson,

> From: Anson Huang
> Sent: Friday, February 22, 2019 5:42 PM
> 
> 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, 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.

Please double check why GPIO driver can't handle PROBE_DEFER well.
Maintainers suggested us to use platform driver unless there is strong reason.

Regards
Dong Aisheng

> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---
>  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
> + 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
> + 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);
> --
> 2.7.4

Re: [PATCH 1/3] clk: imx8mq: initialize clock tree earlier

[Lucas Stach]

Am Freitag, den 22.02.2019, 09:42 +0000 schrieb Anson Huang:
> 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.

Regards,
Lucas

> 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 <Anson.Huang@nxp.com>
> ---
>  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 + 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 + 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);

RE: [PATCH 1/3] clk: imx8mq: initialize clock tree earlier

[Anson Huang]

Hi, Lucas

Best Regards!
Anson Huang

> -----Original Message-----
> From: Lucas Stach [mailto:l.stach@pengutronix.de]
> Sent: 2019年2月22日 18:55
> To: Anson Huang <anson.huang@nxp.com>; 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 <abel.vesa@nxp.com>; 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 <linux-imx@nxp.com>
> Subject: Re: [PATCH 1/3] clk: imx8mq: initialize clock tree earlier
> 
> Am Freitag, den 22.02.2019, 09:42 +0000 schrieb Anson Huang:
> > 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.

Thanks,
Anson.

> 
> Regards,
> Lucas
> 
> > 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 <Anson.Huang@nxp.com>
> > ---
> >  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 +
> > 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 +
> > 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);

RE: [PATCH 1/3] clk: imx8mq: initialize clock tree earlier

[Anson Huang]

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 <abel.vesa@nxp.com>; 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 <linux-imx@nxp.com>
> Subject: RE: [PATCH 1/3] clk: imx8mq: initialize clock tree earlier
> 
> Hi, Lucas
> 
> Best Regards!
> Anson Huang
> 
> > -----Original Message-----
> > From: Lucas Stach [mailto:l.stach@pengutronix.de]
> > Sent: 2019年2月22日 18:55
> > To: Anson Huang <anson.huang@nxp.com>; 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 <abel.vesa@nxp.com>;
> 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 <linux-imx@nxp.com>
> > Subject: Re: [PATCH 1/3] clk: imx8mq: initialize clock tree earlier
> >
> > Am Freitag, den 22.02.2019, 09:42 +0000 schrieb Anson Huang:
> > > 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.
> 
> >
> > Regards,
> > Lucas
> >
> > > 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 <Anson.Huang@nxp.com>
> > > ---
> > >  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 +
> > > 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 +
> > > 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);