[v2] clk/samsung: Use CLK_OF_DECLARE_DRIVER initialization method for CLKOUT

Message ID 1477462340-7867-1-git-send-email-m.szyprowski@samsung.com
State New
Headers show

Commit Message

Marek Szyprowski Oct. 26, 2016, 6:12 a.m.
The Exynos PMU node is an interrupt, clock and PMU (Power Management Unit)
controller, and these functionalities are supported by different drivers
that matches the same compatible strings.

Since commit 989eafd0b609 ("clk: core: Avoid double initialization of
clocks") the OF core flags clock controllers registered with the
CLK_OF_DECLARE() macro as OF_POPULATED, so platform devices with the same
compatible string will not be registered.

This prevents the PMU platform device to be created, so the Exynos PMU
driver is never probed. This breaks (among other things) Suspend-to-RAM.

Fix this by changing CLKOUT driver initialization method to
CLK_OF_DECLARE_DRIVER(), which doesn't clear the OF_POPULATED flag, so
later a platform device is created and the Exynos PMU platform driver
can be be probed properly.

Fixes: 989eafd0b609 ("clk: core: Avoid double initialization of clocks")
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>

---
Changelog:
v2:
- switched to CLK_OF_DECLARE_DRIVER instead of calling
  of_node_clear_flag(node, OF_POPULATED) as suggested by Stephen Boyd

v1: https://www.spinics.net/lists/linux-clk/msg12833.html
- initial version
---
 drivers/clk/samsung/clk-exynos-clkout.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Chanwoo Choi Oct. 26, 2016, 9:10 a.m. | #1
Hi Marek,

On 2016년 10월 26일 15:12, Marek Szyprowski wrote:
> The Exynos PMU node is an interrupt, clock and PMU (Power Management Unit)

> controller, and these functionalities are supported by different drivers

> that matches the same compatible strings.

> 

> Since commit 989eafd0b609 ("clk: core: Avoid double initialization of

> clocks") the OF core flags clock controllers registered with the

> CLK_OF_DECLARE() macro as OF_POPULATED, so platform devices with the same

> compatible string will not be registered.

> 

> This prevents the PMU platform device to be created, so the Exynos PMU

> driver is never probed. This breaks (among other things) Suspend-to-RAM.

> 

> Fix this by changing CLKOUT driver initialization method to

> CLK_OF_DECLARE_DRIVER(), which doesn't clear the OF_POPULATED flag, so

> later a platform device is created and the Exynos PMU platform driver

> can be be probed properly.

> 

> Fixes: 989eafd0b609 ("clk: core: Avoid double initialization of clocks")

> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>

> ---

> Changelog:

> v2:

> - switched to CLK_OF_DECLARE_DRIVER instead of calling

>   of_node_clear_flag(node, OF_POPULATED) as suggested by Stephen Boyd

> 

> v1: https://www.spinics.net/lists/linux-clk/msg12833.html

> - initial version

> ---

>  drivers/clk/samsung/clk-exynos-clkout.c | 22 ++++++++++++++--------

>  1 file changed, 14 insertions(+), 8 deletions(-)

> 

> diff --git a/drivers/clk/samsung/clk-exynos-clkout.c b/drivers/clk/samsung/clk-exynos-clkout.c

> index 96fab6cfb202..6c6afb87b4ce 100644

> --- a/drivers/clk/samsung/clk-exynos-clkout.c

> +++ b/drivers/clk/samsung/clk-exynos-clkout.c

> @@ -132,28 +132,34 @@ static void __init exynos_clkout_init(struct device_node *node, u32 mux_mask)

>  	pr_err("%s: failed to register clkout clock\n", __func__);

>  }

>  

> +/*

> + * We use CLK_OF_DECLARE_DRIVER initialization method to avoid setting

> + * the OF_POPULATED flag on the pmu device tree node, so later the

> + * Exynos PMU platform device can be properly probed with PMU driver.

> + */

> +

>  static void __init exynos4_clkout_init(struct device_node *node)

>  {

>  	exynos_clkout_init(node, EXYNOS4_CLKOUT_MUX_MASK);

>  }

> -CLK_OF_DECLARE(exynos4210_clkout, "samsung,exynos4210-pmu",

> +CLK_OF_DECLARE_DRIVER(exynos4210_clkout, "samsung,exynos4210-pmu",

>  		exynos4_clkout_init);

> -CLK_OF_DECLARE(exynos4212_clkout, "samsung,exynos4212-pmu",

> +CLK_OF_DECLARE_DRIVER(exynos4212_clkout, "samsung,exynos4212-pmu",

>  		exynos4_clkout_init);

> -CLK_OF_DECLARE(exynos4412_clkout, "samsung,exynos4412-pmu",

> +CLK_OF_DECLARE_DRIVER(exynos4412_clkout, "samsung,exynos4412-pmu",

>  		exynos4_clkout_init);

> -CLK_OF_DECLARE(exynos3250_clkout, "samsung,exynos3250-pmu",

> +CLK_OF_DECLARE_DRIVER(exynos3250_clkout, "samsung,exynos3250-pmu",

>  		exynos4_clkout_init);

>  

>  static void __init exynos5_clkout_init(struct device_node *node)

>  {

>  	exynos_clkout_init(node, EXYNOS5_CLKOUT_MUX_MASK);

>  }

> -CLK_OF_DECLARE(exynos5250_clkout, "samsung,exynos5250-pmu",

> +CLK_OF_DECLARE_DRIVER(exynos5250_clkout, "samsung,exynos5250-pmu",

>  		exynos5_clkout_init);

> -CLK_OF_DECLARE(exynos5410_clkout, "samsung,exynos5410-pmu",

> +CLK_OF_DECLARE_DRIVER(exynos5410_clkout, "samsung,exynos5410-pmu",

>  		exynos5_clkout_init);

> -CLK_OF_DECLARE(exynos5420_clkout, "samsung,exynos5420-pmu",

> +CLK_OF_DECLARE_DRIVER(exynos5420_clkout, "samsung,exynos5420-pmu",

>  		exynos5_clkout_init);

> -CLK_OF_DECLARE(exynos5433_clkout, "samsung,exynos5433-pmu",

> +CLK_OF_DECLARE_DRIVER(exynos5433_clkout, "samsung,exynos5433-pmu",

>  		exynos5_clkout_init);

> 


Looks good to me.
Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>


-- 
Best Regards,
Chanwoo Choi
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Javier Martinez Canillas Oct. 26, 2016, 10:12 a.m. | #2
Hello Marek,

On 10/26/2016 03:12 AM, Marek Szyprowski wrote:
> The Exynos PMU node is an interrupt, clock and PMU (Power Management Unit)

> controller, and these functionalities are supported by different drivers

> that matches the same compatible strings.

> 

> Since commit 989eafd0b609 ("clk: core: Avoid double initialization of

> clocks") the OF core flags clock controllers registered with the

> CLK_OF_DECLARE() macro as OF_POPULATED, so platform devices with the same

> compatible string will not be registered.

> 

> This prevents the PMU platform device to be created, so the Exynos PMU

> driver is never probed. This breaks (among other things) Suspend-to-RAM.

> 

> Fix this by changing CLKOUT driver initialization method to

> CLK_OF_DECLARE_DRIVER(), which doesn't clear the OF_POPULATED flag, so

> later a platform device is created and the Exynos PMU platform driver

> can be be probed properly.

> 

> Fixes: 989eafd0b609 ("clk: core: Avoid double initialization of clocks")

> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>

> ---

> Changelog:

> v2:

> - switched to CLK_OF_DECLARE_DRIVER instead of calling

>   of_node_clear_flag(node, OF_POPULATED) as suggested by Stephen Boyd

>


I was not aware about this new macro and certainly looks like a better
option. The patch looks good to me.

Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>


Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stephen Boyd Oct. 28, 2016, 12:27 a.m. | #3
On 10/26, Marek Szyprowski wrote:
> The Exynos PMU node is an interrupt, clock and PMU (Power Management Unit)

> controller, and these functionalities are supported by different drivers

> that matches the same compatible strings.

> 

> Since commit 989eafd0b609 ("clk: core: Avoid double initialization of

> clocks") the OF core flags clock controllers registered with the

> CLK_OF_DECLARE() macro as OF_POPULATED, so platform devices with the same

> compatible string will not be registered.

> 

> This prevents the PMU platform device to be created, so the Exynos PMU

> driver is never probed. This breaks (among other things) Suspend-to-RAM.

> 

> Fix this by changing CLKOUT driver initialization method to

> CLK_OF_DECLARE_DRIVER(), which doesn't clear the OF_POPULATED flag, so

> later a platform device is created and the Exynos PMU platform driver

> can be be probed properly.

> 

> Fixes: 989eafd0b609 ("clk: core: Avoid double initialization of clocks")

> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>

> ---


Applied to clk-fixes

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/clk/samsung/clk-exynos-clkout.c b/drivers/clk/samsung/clk-exynos-clkout.c
index 96fab6cfb202..6c6afb87b4ce 100644
--- a/drivers/clk/samsung/clk-exynos-clkout.c
+++ b/drivers/clk/samsung/clk-exynos-clkout.c
@@ -132,28 +132,34 @@  static void __init exynos_clkout_init(struct device_node *node, u32 mux_mask)
 	pr_err("%s: failed to register clkout clock\n", __func__);
 }
 
+/*
+ * We use CLK_OF_DECLARE_DRIVER initialization method to avoid setting
+ * the OF_POPULATED flag on the pmu device tree node, so later the
+ * Exynos PMU platform device can be properly probed with PMU driver.
+ */
+
 static void __init exynos4_clkout_init(struct device_node *node)
 {
 	exynos_clkout_init(node, EXYNOS4_CLKOUT_MUX_MASK);
 }
-CLK_OF_DECLARE(exynos4210_clkout, "samsung,exynos4210-pmu",
+CLK_OF_DECLARE_DRIVER(exynos4210_clkout, "samsung,exynos4210-pmu",
 		exynos4_clkout_init);
-CLK_OF_DECLARE(exynos4212_clkout, "samsung,exynos4212-pmu",
+CLK_OF_DECLARE_DRIVER(exynos4212_clkout, "samsung,exynos4212-pmu",
 		exynos4_clkout_init);
-CLK_OF_DECLARE(exynos4412_clkout, "samsung,exynos4412-pmu",
+CLK_OF_DECLARE_DRIVER(exynos4412_clkout, "samsung,exynos4412-pmu",
 		exynos4_clkout_init);
-CLK_OF_DECLARE(exynos3250_clkout, "samsung,exynos3250-pmu",
+CLK_OF_DECLARE_DRIVER(exynos3250_clkout, "samsung,exynos3250-pmu",
 		exynos4_clkout_init);
 
 static void __init exynos5_clkout_init(struct device_node *node)
 {
 	exynos_clkout_init(node, EXYNOS5_CLKOUT_MUX_MASK);
 }
-CLK_OF_DECLARE(exynos5250_clkout, "samsung,exynos5250-pmu",
+CLK_OF_DECLARE_DRIVER(exynos5250_clkout, "samsung,exynos5250-pmu",
 		exynos5_clkout_init);
-CLK_OF_DECLARE(exynos5410_clkout, "samsung,exynos5410-pmu",
+CLK_OF_DECLARE_DRIVER(exynos5410_clkout, "samsung,exynos5410-pmu",
 		exynos5_clkout_init);
-CLK_OF_DECLARE(exynos5420_clkout, "samsung,exynos5420-pmu",
+CLK_OF_DECLARE_DRIVER(exynos5420_clkout, "samsung,exynos5420-pmu",
 		exynos5_clkout_init);
-CLK_OF_DECLARE(exynos5433_clkout, "samsung,exynos5433-pmu",
+CLK_OF_DECLARE_DRIVER(exynos5433_clkout, "samsung,exynos5433-pmu",
 		exynos5_clkout_init);