All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5] clk: sunxi: Rework the clocks code to deal with orphans
@ 2016-02-02  8:47 ` Maxime Ripard
  0 siblings, 0 replies; 40+ messages in thread
From: Maxime Ripard @ 2016-02-02  8:47 UTC (permalink / raw)
  To: Mike Turquette, Stephen Boyd, Emilio Lopez, heiko, Chen-Yu Tsai
  Cc: linux-clk, linux-arm-kernel, Andre Przywara, Maxime Ripard

Hi everyone,

Here is an attempt at making the whole clk-sunxi code register the
clocks without creating any orphans by relying on CLK_OF_DECLARE.

This also has the net benefit of not having to add the SoC compatible
to the list of compatibles that trigger the clock registration, which
has started to be a pain.

I tested this on an A13 (R8 actually), everything seems to be working
just fine. I also tested it both with and without Heiko's patch to
defer clk_get if the clock is orphan, and previously broke everything.

Let me know what you think,
Maxime

Maxime Ripard (5):
  clk: sunxi: Make clocks setup functions return their clock
  clk: sunxi: Make clocks setup functions take const pointer
  clk: sunxi: convert current clocks registration to CLK_OF_DECLARE
  clk: sunxi: Remove old probe and protection code
  clk: sunxi: Remove clk_register_clkdev calls

 drivers/clk/sunxi/clk-a10-hosc.c         |   3 +-
 drivers/clk/sunxi/clk-a20-gmac.c         |   2 -
 drivers/clk/sunxi/clk-factors.c          |   7 -
 drivers/clk/sunxi/clk-factors.h          |   1 -
 drivers/clk/sunxi/clk-mod0.c             |   1 +
 drivers/clk/sunxi/clk-sun6i-apb0-gates.c |   2 -
 drivers/clk/sunxi/clk-sun9i-core.c       |   1 +
 drivers/clk/sunxi/clk-sunxi.c            | 263 ++++++++++++++++---------------
 8 files changed, 135 insertions(+), 145 deletions(-)

-- 
2.6.4

^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH 0/5] clk: sunxi: Rework the clocks code to deal with orphans
@ 2016-02-02  8:47 ` Maxime Ripard
  0 siblings, 0 replies; 40+ messages in thread
From: Maxime Ripard @ 2016-02-02  8:47 UTC (permalink / raw)
  To: linux-arm-kernel

Hi everyone,

Here is an attempt at making the whole clk-sunxi code register the
clocks without creating any orphans by relying on CLK_OF_DECLARE.

This also has the net benefit of not having to add the SoC compatible
to the list of compatibles that trigger the clock registration, which
has started to be a pain.

I tested this on an A13 (R8 actually), everything seems to be working
just fine. I also tested it both with and without Heiko's patch to
defer clk_get if the clock is orphan, and previously broke everything.

Let me know what you think,
Maxime

Maxime Ripard (5):
  clk: sunxi: Make clocks setup functions return their clock
  clk: sunxi: Make clocks setup functions take const pointer
  clk: sunxi: convert current clocks registration to CLK_OF_DECLARE
  clk: sunxi: Remove old probe and protection code
  clk: sunxi: Remove clk_register_clkdev calls

 drivers/clk/sunxi/clk-a10-hosc.c         |   3 +-
 drivers/clk/sunxi/clk-a20-gmac.c         |   2 -
 drivers/clk/sunxi/clk-factors.c          |   7 -
 drivers/clk/sunxi/clk-factors.h          |   1 -
 drivers/clk/sunxi/clk-mod0.c             |   1 +
 drivers/clk/sunxi/clk-sun6i-apb0-gates.c |   2 -
 drivers/clk/sunxi/clk-sun9i-core.c       |   1 +
 drivers/clk/sunxi/clk-sunxi.c            | 263 ++++++++++++++++---------------
 8 files changed, 135 insertions(+), 145 deletions(-)

-- 
2.6.4

^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH 1/5] clk: sunxi: Make clocks setup functions return their clock
  2016-02-02  8:47 ` Maxime Ripard
@ 2016-02-02  8:47   ` Maxime Ripard
  -1 siblings, 0 replies; 40+ messages in thread
From: Maxime Ripard @ 2016-02-02  8:47 UTC (permalink / raw)
  To: Mike Turquette, Stephen Boyd, Emilio Lopez, heiko, Chen-Yu Tsai
  Cc: linux-clk, linux-arm-kernel, Andre Przywara, Maxime Ripard

The clocks registration code in clk-sunxi was most of the time not
returning the struct clk (or struct clk array) that was registered,
preventing the users of such functions to manipulate it, for example to
protect it.

Make them return it so that we can start using it.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/clk/sunxi/clk-sunxi.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
index e460a6b0068c..67ef94948544 100644
--- a/drivers/clk/sunxi/clk-sunxi.c
+++ b/drivers/clk/sunxi/clk-sunxi.c
@@ -615,8 +615,8 @@ static const struct mux_data sun8i_h3_ahb2_mux_data __initconst = {
 	.shift = 0,
 };
 
-static void __init sunxi_mux_clk_setup(struct device_node *node,
-				       struct mux_data *data)
+static struct clk * __init sunxi_mux_clk_setup(struct device_node *node,
+					       struct mux_data *data)
 {
 	struct clk *clk;
 	const char *clk_name = node->name;
@@ -638,6 +638,8 @@ static void __init sunxi_mux_clk_setup(struct device_node *node,
 		of_clk_add_provider(node, of_clk_src_simple_get, clk);
 		clk_register_clkdev(clk, clk_name, NULL);
 	}
+
+	return clk;
 }
 
 
@@ -722,7 +724,6 @@ static void __init sunxi_divider_clk_setup(struct device_node *node,
 }
 
 
-
 /**
  * sunxi_gates_clk_setup() - Setup function for leaf gates on clocks
  */
@@ -808,8 +809,8 @@ static const struct divs_data sun6i_a31_pll6_divs_data __initconst = {
  *           |________________________|
  */
 
-static void __init sunxi_divs_clk_setup(struct device_node *node,
-					struct divs_data *data)
+static struct clk ** __init sunxi_divs_clk_setup(struct device_node *node,
+						 struct divs_data *data)
 {
 	struct clk_onecell_data *clk_data;
 	const char *parent;
@@ -836,7 +837,7 @@ static void __init sunxi_divs_clk_setup(struct device_node *node,
 
 	clk_data = kmalloc(sizeof(struct clk_onecell_data), GFP_KERNEL);
 	if (!clk_data)
-		return;
+		return NULL;
 
 	clks = kcalloc(ndivs, sizeof(*clks), GFP_KERNEL);
 	if (!clks)
@@ -922,7 +923,7 @@ static void __init sunxi_divs_clk_setup(struct device_node *node,
 
 	of_clk_add_provider(node, of_clk_src_onecell_get, clk_data);
 
-	return;
+	return clks;
 
 free_gate:
 	kfree(gate);
@@ -930,6 +931,7 @@ free_clks:
 	kfree(clks);
 free_clkdata:
 	kfree(clk_data);
+	return NULL;
 }
 
 
-- 
2.6.4

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 1/5] clk: sunxi: Make clocks setup functions return their clock
@ 2016-02-02  8:47   ` Maxime Ripard
  0 siblings, 0 replies; 40+ messages in thread
From: Maxime Ripard @ 2016-02-02  8:47 UTC (permalink / raw)
  To: linux-arm-kernel

The clocks registration code in clk-sunxi was most of the time not
returning the struct clk (or struct clk array) that was registered,
preventing the users of such functions to manipulate it, for example to
protect it.

Make them return it so that we can start using it.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/clk/sunxi/clk-sunxi.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
index e460a6b0068c..67ef94948544 100644
--- a/drivers/clk/sunxi/clk-sunxi.c
+++ b/drivers/clk/sunxi/clk-sunxi.c
@@ -615,8 +615,8 @@ static const struct mux_data sun8i_h3_ahb2_mux_data __initconst = {
 	.shift = 0,
 };
 
-static void __init sunxi_mux_clk_setup(struct device_node *node,
-				       struct mux_data *data)
+static struct clk * __init sunxi_mux_clk_setup(struct device_node *node,
+					       struct mux_data *data)
 {
 	struct clk *clk;
 	const char *clk_name = node->name;
@@ -638,6 +638,8 @@ static void __init sunxi_mux_clk_setup(struct device_node *node,
 		of_clk_add_provider(node, of_clk_src_simple_get, clk);
 		clk_register_clkdev(clk, clk_name, NULL);
 	}
+
+	return clk;
 }
 
 
@@ -722,7 +724,6 @@ static void __init sunxi_divider_clk_setup(struct device_node *node,
 }
 
 
-
 /**
  * sunxi_gates_clk_setup() - Setup function for leaf gates on clocks
  */
@@ -808,8 +809,8 @@ static const struct divs_data sun6i_a31_pll6_divs_data __initconst = {
  *           |________________________|
  */
 
-static void __init sunxi_divs_clk_setup(struct device_node *node,
-					struct divs_data *data)
+static struct clk ** __init sunxi_divs_clk_setup(struct device_node *node,
+						 struct divs_data *data)
 {
 	struct clk_onecell_data *clk_data;
 	const char *parent;
@@ -836,7 +837,7 @@ static void __init sunxi_divs_clk_setup(struct device_node *node,
 
 	clk_data = kmalloc(sizeof(struct clk_onecell_data), GFP_KERNEL);
 	if (!clk_data)
-		return;
+		return NULL;
 
 	clks = kcalloc(ndivs, sizeof(*clks), GFP_KERNEL);
 	if (!clks)
@@ -922,7 +923,7 @@ static void __init sunxi_divs_clk_setup(struct device_node *node,
 
 	of_clk_add_provider(node, of_clk_src_onecell_get, clk_data);
 
-	return;
+	return clks;
 
 free_gate:
 	kfree(gate);
@@ -930,6 +931,7 @@ free_clks:
 	kfree(clks);
 free_clkdata:
 	kfree(clk_data);
+	return NULL;
 }
 
 
-- 
2.6.4

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 2/5] clk: sunxi: Make clocks setup functions take const pointer
  2016-02-02  8:47 ` Maxime Ripard
@ 2016-02-02  8:47   ` Maxime Ripard
  -1 siblings, 0 replies; 40+ messages in thread
From: Maxime Ripard @ 2016-02-02  8:47 UTC (permalink / raw)
  To: Mike Turquette, Stephen Boyd, Emilio Lopez, heiko, Chen-Yu Tsai
  Cc: linux-clk, linux-arm-kernel, Andre Przywara, Maxime Ripard

All the data structure that we pass to the clocks setup functions are
declared const, while our setup functions expects a regular pointer. This
was hidden by the fact that we cast a void * pointer back to these
structures, which made it go unnoticed.

Fix the functions prototype.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/clk/sunxi/clk-sunxi.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
index 67ef94948544..a3d706f5b21c 100644
--- a/drivers/clk/sunxi/clk-sunxi.c
+++ b/drivers/clk/sunxi/clk-sunxi.c
@@ -616,7 +616,7 @@ static const struct mux_data sun8i_h3_ahb2_mux_data __initconst = {
 };
 
 static struct clk * __init sunxi_mux_clk_setup(struct device_node *node,
-					       struct mux_data *data)
+					       const struct mux_data *data)
 {
 	struct clk *clk;
 	const char *clk_name = node->name;
@@ -700,7 +700,7 @@ static const struct div_data sun4i_apb0_data __initconst = {
 };
 
 static void __init sunxi_divider_clk_setup(struct device_node *node,
-					   struct div_data *data)
+					   const struct div_data *data)
 {
 	struct clk *clk;
 	const char *clk_name = node->name;
@@ -810,7 +810,7 @@ static const struct divs_data sun6i_a31_pll6_divs_data __initconst = {
  */
 
 static struct clk ** __init sunxi_divs_clk_setup(struct device_node *node,
-						 struct divs_data *data)
+						 const struct divs_data *data)
 {
 	struct clk_onecell_data *clk_data;
 	const char *parent;
-- 
2.6.4

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 2/5] clk: sunxi: Make clocks setup functions take const pointer
@ 2016-02-02  8:47   ` Maxime Ripard
  0 siblings, 0 replies; 40+ messages in thread
From: Maxime Ripard @ 2016-02-02  8:47 UTC (permalink / raw)
  To: linux-arm-kernel

All the data structure that we pass to the clocks setup functions are
declared const, while our setup functions expects a regular pointer. This
was hidden by the fact that we cast a void * pointer back to these
structures, which made it go unnoticed.

Fix the functions prototype.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/clk/sunxi/clk-sunxi.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
index 67ef94948544..a3d706f5b21c 100644
--- a/drivers/clk/sunxi/clk-sunxi.c
+++ b/drivers/clk/sunxi/clk-sunxi.c
@@ -616,7 +616,7 @@ static const struct mux_data sun8i_h3_ahb2_mux_data __initconst = {
 };
 
 static struct clk * __init sunxi_mux_clk_setup(struct device_node *node,
-					       struct mux_data *data)
+					       const struct mux_data *data)
 {
 	struct clk *clk;
 	const char *clk_name = node->name;
@@ -700,7 +700,7 @@ static const struct div_data sun4i_apb0_data __initconst = {
 };
 
 static void __init sunxi_divider_clk_setup(struct device_node *node,
-					   struct div_data *data)
+					   const struct div_data *data)
 {
 	struct clk *clk;
 	const char *clk_name = node->name;
@@ -810,7 +810,7 @@ static const struct divs_data sun6i_a31_pll6_divs_data __initconst = {
  */
 
 static struct clk ** __init sunxi_divs_clk_setup(struct device_node *node,
-						 struct divs_data *data)
+						 const struct divs_data *data)
 {
 	struct clk_onecell_data *clk_data;
 	const char *parent;
-- 
2.6.4

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 3/5] clk: sunxi: convert current clocks registration to CLK_OF_DECLARE
  2016-02-02  8:47 ` Maxime Ripard
@ 2016-02-02  8:47   ` Maxime Ripard
  -1 siblings, 0 replies; 40+ messages in thread
From: Maxime Ripard @ 2016-02-02  8:47 UTC (permalink / raw)
  To: Mike Turquette, Stephen Boyd, Emilio Lopez, heiko, Chen-Yu Tsai
  Cc: linux-clk, linux-arm-kernel, Andre Przywara, Maxime Ripard

The current clock registration and protection code has a few drawbacks, the
two main ones being that we create a lot of orphans clock in the
registration phase, which will be troublesome when we will start being less
relaxed about them.

The protection code also relies on clkdev, which we don't really use but
for this particular case.

Fix both at the same time by moving everyone to the CLK_OF_DECLARE that
will probe our clock tree in the right and thus avoid orphans, and by
protecting directly the clock returned by our registration function.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/clk/sunxi/clk-sunxi.c | 150 +++++++++++++++++++++++++++++++++++++-----
 1 file changed, 133 insertions(+), 17 deletions(-)

diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
index a3d706f5b21c..da6c1bc763d1 100644
--- a/drivers/clk/sunxi/clk-sunxi.c
+++ b/drivers/clk/sunxi/clk-sunxi.c
@@ -585,6 +585,41 @@ static struct clk * __init sunxi_factors_clk_setup(struct device_node *node,
 	return sunxi_factors_register(node, data, &clk_lock, reg);
 }
 
+static void __init sun4i_pll1_clk_setup(struct device_node *node)
+{
+	sunxi_factors_clk_setup(node, &sun4i_pll1_data);
+}
+CLK_OF_DECLARE(sun4i_pll1, "allwinner,sun4i-a10-pll1-clk",
+	       sun4i_pll1_clk_setup);
+
+static void __init sun6i_pll1_clk_setup(struct device_node *node)
+{
+	sunxi_factors_clk_setup(node, &sun6i_a31_pll1_data);
+}
+CLK_OF_DECLARE(sun6i_pll1, "allwinner,sun6i-a31-pll1-clk",
+	       sun6i_pll1_clk_setup);
+
+static void __init sun8i_pll1_clk_setup(struct device_node *node)
+{
+	sunxi_factors_clk_setup(node, &sun8i_a23_pll1_data);
+}
+CLK_OF_DECLARE(sun8i_pll1, "allwinner,sun8i-a23-pll1-clk",
+	       sun8i_pll1_clk_setup);
+
+static void __init sun7i_pll4_clk_setup(struct device_node *node)
+{
+	sunxi_factors_clk_setup(node, &sun7i_a20_pll4_data);
+}
+CLK_OF_DECLARE(sun7i_pll4, "allwinner,sun7i-a20-pll4-clk",
+	       sun7i_pll4_clk_setup);
+
+static void __init sun5i_ahb_clk_setup(struct device_node *node)
+{
+	sunxi_factors_clk_setup(node, &sun5i_a13_ahb_data);
+}
+CLK_OF_DECLARE(sun5i_ahb, "allwinner,sun5i-a13-ahb-clk",
+	       sun5i_ahb_clk_setup);
+
 static void __init sun6i_ahb1_clk_setup(struct device_node *node)
 {
 	sunxi_factors_clk_setup(node, &sun6i_ahb1_data);
@@ -592,6 +627,20 @@ static void __init sun6i_ahb1_clk_setup(struct device_node *node)
 CLK_OF_DECLARE(sun6i_a31_ahb1, "allwinner,sun6i-a31-ahb1-clk",
 	       sun6i_ahb1_clk_setup);
 
+static void __init sun4i_apb1_clk_setup(struct device_node *node)
+{
+	sunxi_factors_clk_setup(node, &sun4i_apb1_data);
+}
+CLK_OF_DECLARE(sun4i_apb1, "allwinner,sun4i-a10-apb1-clk",
+	       sun4i_apb1_clk_setup);
+
+static void __init sun7i_out_clk_setup(struct device_node *node)
+{
+	sunxi_factors_clk_setup(node, &sun7i_a20_out_data);
+}
+CLK_OF_DECLARE(sun7i_out, "allwinner,sun7i-a20-out-clk",
+	       sun7i_out_clk_setup);
+
 
 /**
  * sunxi_mux_clk_setup() - Setup function for muxes
@@ -642,6 +691,34 @@ static struct clk * __init sunxi_mux_clk_setup(struct device_node *node,
 	return clk;
 }
 
+static void __init sun4i_cpu_clk_setup(struct device_node *node)
+{
+	struct clk *clk;
+
+	clk = sunxi_mux_clk_setup(node, &sun4i_cpu_mux_data);
+	if (!clk)
+		return;
+
+	/* Protect CPU clock */
+	__clk_get(clk);
+	clk_prepare_enable(clk);
+}
+CLK_OF_DECLARE(sun4i_cpu, "allwinner,sun4i-a10-cpu-clk",
+	       sun4i_cpu_clk_setup);
+
+static void __init sun6i_ahb1_mux_clk_setup(struct device_node *node)
+{
+	sunxi_mux_clk_setup(node, &sun6i_a31_ahb1_mux_data);
+}
+CLK_OF_DECLARE(sun6i_ahb1_mux, "allwinner,sun6i-a31-ahb1-mux-clk",
+	       sun6i_ahb1_mux_clk_setup);
+
+static void __init sun8i_ahb2_clk_setup(struct device_node *node)
+{
+	sunxi_mux_clk_setup(node, &sun8i_h3_ahb2_mux_data);
+}
+CLK_OF_DECLARE(sun8i_ahb2, "allwinner,sun8i-a31-ahb2-clk",
+	       sun8i_ahb2_clk_setup);
 
 
 /**
@@ -723,6 +800,34 @@ static void __init sunxi_divider_clk_setup(struct device_node *node,
 	}
 }
 
+static void __init sun4i_ahb_clk_setup(struct device_node *node)
+{
+	sunxi_divider_clk_setup(node, &sun4i_ahb_data);
+}
+CLK_OF_DECLARE(sun4i_ahb, "allwinner,sun4i-a10-ahb-clk",
+	       sun4i_ahb_clk_setup);
+
+static void __init sun4i_apb0_clk_setup(struct device_node *node)
+{
+	sunxi_divider_clk_setup(node, &sun4i_apb0_data);
+}
+CLK_OF_DECLARE(sun4i_apb0, "allwinner,sun4i-a10-apb0-clk",
+	       sun4i_apb0_clk_setup);
+
+static void __init sun4i_axi_clk_setup(struct device_node *node)
+{
+	sunxi_divider_clk_setup(node, &sun4i_axi_data);
+}
+CLK_OF_DECLARE(sun4i_axi, "allwinner,sun4i-a10-axi-clk",
+	       sun4i_axi_clk_setup);
+
+static void __init sun8i_axi_clk_setup(struct device_node *node)
+{
+	sunxi_divider_clk_setup(node, &sun8i_a23_axi_data);
+}
+CLK_OF_DECLARE(sun8i_axi, "allwinner,sun8i-a23-axi-clk",
+	       sun8i_axi_clk_setup);
+
 
 /**
  * sunxi_gates_clk_setup() - Setup function for leaf gates on clocks
@@ -934,42 +1039,53 @@ free_clkdata:
 	return NULL;
 }
 
+static void __init sun4i_pll5_clk_setup(struct device_node *node)
+{
+	struct clk **clks;
+
+	clks = sunxi_divs_clk_setup(node, &pll5_divs_data);
+	if (!clks)
+		return;
+
+	/* Protect PLL5_DDR */
+	__clk_get(clks[0]);
+	clk_prepare_enable(clks[0]);
+}
+CLK_OF_DECLARE(sun4i_pll5, "allwinner,sun4i-a10-pll5-clk",
+	       sun4i_pll5_clk_setup);
+
+static void __init sun4i_pll6_clk_setup(struct device_node *node)
+{
+	sunxi_divs_clk_setup(node, &pll6_divs_data);
+}
+CLK_OF_DECLARE(sun4i_pll6, "allwinner,sun4i-a10-pll6-clk",
+	       sun4i_pll6_clk_setup);
+
+static void __init sun6i_pll6_clk_setup(struct device_node *node)
+{
+	sunxi_divs_clk_setup(node, &sun6i_a31_pll6_divs_data);
+}
+CLK_OF_DECLARE(sun6i_pll6, "allwinner,sun6i-a31-pll6-clk",
+	       sun6i_pll6_clk_setup);
 
 
 /* Matches for factors clocks */
 static const struct of_device_id clk_factors_match[] __initconst = {
-	{.compatible = "allwinner,sun4i-a10-pll1-clk", .data = &sun4i_pll1_data,},
-	{.compatible = "allwinner,sun6i-a31-pll1-clk", .data = &sun6i_a31_pll1_data,},
-	{.compatible = "allwinner,sun8i-a23-pll1-clk", .data = &sun8i_a23_pll1_data,},
-	{.compatible = "allwinner,sun7i-a20-pll4-clk", .data = &sun7i_a20_pll4_data,},
-	{.compatible = "allwinner,sun5i-a13-ahb-clk", .data = &sun5i_a13_ahb_data,},
-	{.compatible = "allwinner,sun4i-a10-apb1-clk", .data = &sun4i_apb1_data,},
-	{.compatible = "allwinner,sun7i-a20-out-clk", .data = &sun7i_a20_out_data,},
 	{}
 };
 
 /* Matches for divider clocks */
 static const struct of_device_id clk_div_match[] __initconst = {
-	{.compatible = "allwinner,sun4i-a10-axi-clk", .data = &sun4i_axi_data,},
-	{.compatible = "allwinner,sun8i-a23-axi-clk", .data = &sun8i_a23_axi_data,},
-	{.compatible = "allwinner,sun4i-a10-ahb-clk", .data = &sun4i_ahb_data,},
-	{.compatible = "allwinner,sun4i-a10-apb0-clk", .data = &sun4i_apb0_data,},
 	{}
 };
 
 /* Matches for divided outputs */
 static const struct of_device_id clk_divs_match[] __initconst = {
-	{.compatible = "allwinner,sun4i-a10-pll5-clk", .data = &pll5_divs_data,},
-	{.compatible = "allwinner,sun4i-a10-pll6-clk", .data = &pll6_divs_data,},
-	{.compatible = "allwinner,sun6i-a31-pll6-clk", .data = &sun6i_a31_pll6_divs_data,},
 	{}
 };
 
 /* Matches for mux clocks */
 static const struct of_device_id clk_mux_match[] __initconst = {
-	{.compatible = "allwinner,sun4i-a10-cpu-clk", .data = &sun4i_cpu_mux_data,},
-	{.compatible = "allwinner,sun6i-a31-ahb1-mux-clk", .data = &sun6i_a31_ahb1_mux_data,},
-	{.compatible = "allwinner,sun8i-h3-ahb2-clk", .data = &sun8i_h3_ahb2_mux_data,},
 	{}
 };
 
-- 
2.6.4

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 3/5] clk: sunxi: convert current clocks registration to CLK_OF_DECLARE
@ 2016-02-02  8:47   ` Maxime Ripard
  0 siblings, 0 replies; 40+ messages in thread
From: Maxime Ripard @ 2016-02-02  8:47 UTC (permalink / raw)
  To: linux-arm-kernel

The current clock registration and protection code has a few drawbacks, the
two main ones being that we create a lot of orphans clock in the
registration phase, which will be troublesome when we will start being less
relaxed about them.

The protection code also relies on clkdev, which we don't really use but
for this particular case.

Fix both at the same time by moving everyone to the CLK_OF_DECLARE that
will probe our clock tree in the right and thus avoid orphans, and by
protecting directly the clock returned by our registration function.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/clk/sunxi/clk-sunxi.c | 150 +++++++++++++++++++++++++++++++++++++-----
 1 file changed, 133 insertions(+), 17 deletions(-)

diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
index a3d706f5b21c..da6c1bc763d1 100644
--- a/drivers/clk/sunxi/clk-sunxi.c
+++ b/drivers/clk/sunxi/clk-sunxi.c
@@ -585,6 +585,41 @@ static struct clk * __init sunxi_factors_clk_setup(struct device_node *node,
 	return sunxi_factors_register(node, data, &clk_lock, reg);
 }
 
+static void __init sun4i_pll1_clk_setup(struct device_node *node)
+{
+	sunxi_factors_clk_setup(node, &sun4i_pll1_data);
+}
+CLK_OF_DECLARE(sun4i_pll1, "allwinner,sun4i-a10-pll1-clk",
+	       sun4i_pll1_clk_setup);
+
+static void __init sun6i_pll1_clk_setup(struct device_node *node)
+{
+	sunxi_factors_clk_setup(node, &sun6i_a31_pll1_data);
+}
+CLK_OF_DECLARE(sun6i_pll1, "allwinner,sun6i-a31-pll1-clk",
+	       sun6i_pll1_clk_setup);
+
+static void __init sun8i_pll1_clk_setup(struct device_node *node)
+{
+	sunxi_factors_clk_setup(node, &sun8i_a23_pll1_data);
+}
+CLK_OF_DECLARE(sun8i_pll1, "allwinner,sun8i-a23-pll1-clk",
+	       sun8i_pll1_clk_setup);
+
+static void __init sun7i_pll4_clk_setup(struct device_node *node)
+{
+	sunxi_factors_clk_setup(node, &sun7i_a20_pll4_data);
+}
+CLK_OF_DECLARE(sun7i_pll4, "allwinner,sun7i-a20-pll4-clk",
+	       sun7i_pll4_clk_setup);
+
+static void __init sun5i_ahb_clk_setup(struct device_node *node)
+{
+	sunxi_factors_clk_setup(node, &sun5i_a13_ahb_data);
+}
+CLK_OF_DECLARE(sun5i_ahb, "allwinner,sun5i-a13-ahb-clk",
+	       sun5i_ahb_clk_setup);
+
 static void __init sun6i_ahb1_clk_setup(struct device_node *node)
 {
 	sunxi_factors_clk_setup(node, &sun6i_ahb1_data);
@@ -592,6 +627,20 @@ static void __init sun6i_ahb1_clk_setup(struct device_node *node)
 CLK_OF_DECLARE(sun6i_a31_ahb1, "allwinner,sun6i-a31-ahb1-clk",
 	       sun6i_ahb1_clk_setup);
 
+static void __init sun4i_apb1_clk_setup(struct device_node *node)
+{
+	sunxi_factors_clk_setup(node, &sun4i_apb1_data);
+}
+CLK_OF_DECLARE(sun4i_apb1, "allwinner,sun4i-a10-apb1-clk",
+	       sun4i_apb1_clk_setup);
+
+static void __init sun7i_out_clk_setup(struct device_node *node)
+{
+	sunxi_factors_clk_setup(node, &sun7i_a20_out_data);
+}
+CLK_OF_DECLARE(sun7i_out, "allwinner,sun7i-a20-out-clk",
+	       sun7i_out_clk_setup);
+
 
 /**
  * sunxi_mux_clk_setup() - Setup function for muxes
@@ -642,6 +691,34 @@ static struct clk * __init sunxi_mux_clk_setup(struct device_node *node,
 	return clk;
 }
 
+static void __init sun4i_cpu_clk_setup(struct device_node *node)
+{
+	struct clk *clk;
+
+	clk = sunxi_mux_clk_setup(node, &sun4i_cpu_mux_data);
+	if (!clk)
+		return;
+
+	/* Protect CPU clock */
+	__clk_get(clk);
+	clk_prepare_enable(clk);
+}
+CLK_OF_DECLARE(sun4i_cpu, "allwinner,sun4i-a10-cpu-clk",
+	       sun4i_cpu_clk_setup);
+
+static void __init sun6i_ahb1_mux_clk_setup(struct device_node *node)
+{
+	sunxi_mux_clk_setup(node, &sun6i_a31_ahb1_mux_data);
+}
+CLK_OF_DECLARE(sun6i_ahb1_mux, "allwinner,sun6i-a31-ahb1-mux-clk",
+	       sun6i_ahb1_mux_clk_setup);
+
+static void __init sun8i_ahb2_clk_setup(struct device_node *node)
+{
+	sunxi_mux_clk_setup(node, &sun8i_h3_ahb2_mux_data);
+}
+CLK_OF_DECLARE(sun8i_ahb2, "allwinner,sun8i-a31-ahb2-clk",
+	       sun8i_ahb2_clk_setup);
 
 
 /**
@@ -723,6 +800,34 @@ static void __init sunxi_divider_clk_setup(struct device_node *node,
 	}
 }
 
+static void __init sun4i_ahb_clk_setup(struct device_node *node)
+{
+	sunxi_divider_clk_setup(node, &sun4i_ahb_data);
+}
+CLK_OF_DECLARE(sun4i_ahb, "allwinner,sun4i-a10-ahb-clk",
+	       sun4i_ahb_clk_setup);
+
+static void __init sun4i_apb0_clk_setup(struct device_node *node)
+{
+	sunxi_divider_clk_setup(node, &sun4i_apb0_data);
+}
+CLK_OF_DECLARE(sun4i_apb0, "allwinner,sun4i-a10-apb0-clk",
+	       sun4i_apb0_clk_setup);
+
+static void __init sun4i_axi_clk_setup(struct device_node *node)
+{
+	sunxi_divider_clk_setup(node, &sun4i_axi_data);
+}
+CLK_OF_DECLARE(sun4i_axi, "allwinner,sun4i-a10-axi-clk",
+	       sun4i_axi_clk_setup);
+
+static void __init sun8i_axi_clk_setup(struct device_node *node)
+{
+	sunxi_divider_clk_setup(node, &sun8i_a23_axi_data);
+}
+CLK_OF_DECLARE(sun8i_axi, "allwinner,sun8i-a23-axi-clk",
+	       sun8i_axi_clk_setup);
+
 
 /**
  * sunxi_gates_clk_setup() - Setup function for leaf gates on clocks
@@ -934,42 +1039,53 @@ free_clkdata:
 	return NULL;
 }
 
+static void __init sun4i_pll5_clk_setup(struct device_node *node)
+{
+	struct clk **clks;
+
+	clks = sunxi_divs_clk_setup(node, &pll5_divs_data);
+	if (!clks)
+		return;
+
+	/* Protect PLL5_DDR */
+	__clk_get(clks[0]);
+	clk_prepare_enable(clks[0]);
+}
+CLK_OF_DECLARE(sun4i_pll5, "allwinner,sun4i-a10-pll5-clk",
+	       sun4i_pll5_clk_setup);
+
+static void __init sun4i_pll6_clk_setup(struct device_node *node)
+{
+	sunxi_divs_clk_setup(node, &pll6_divs_data);
+}
+CLK_OF_DECLARE(sun4i_pll6, "allwinner,sun4i-a10-pll6-clk",
+	       sun4i_pll6_clk_setup);
+
+static void __init sun6i_pll6_clk_setup(struct device_node *node)
+{
+	sunxi_divs_clk_setup(node, &sun6i_a31_pll6_divs_data);
+}
+CLK_OF_DECLARE(sun6i_pll6, "allwinner,sun6i-a31-pll6-clk",
+	       sun6i_pll6_clk_setup);
 
 
 /* Matches for factors clocks */
 static const struct of_device_id clk_factors_match[] __initconst = {
-	{.compatible = "allwinner,sun4i-a10-pll1-clk", .data = &sun4i_pll1_data,},
-	{.compatible = "allwinner,sun6i-a31-pll1-clk", .data = &sun6i_a31_pll1_data,},
-	{.compatible = "allwinner,sun8i-a23-pll1-clk", .data = &sun8i_a23_pll1_data,},
-	{.compatible = "allwinner,sun7i-a20-pll4-clk", .data = &sun7i_a20_pll4_data,},
-	{.compatible = "allwinner,sun5i-a13-ahb-clk", .data = &sun5i_a13_ahb_data,},
-	{.compatible = "allwinner,sun4i-a10-apb1-clk", .data = &sun4i_apb1_data,},
-	{.compatible = "allwinner,sun7i-a20-out-clk", .data = &sun7i_a20_out_data,},
 	{}
 };
 
 /* Matches for divider clocks */
 static const struct of_device_id clk_div_match[] __initconst = {
-	{.compatible = "allwinner,sun4i-a10-axi-clk", .data = &sun4i_axi_data,},
-	{.compatible = "allwinner,sun8i-a23-axi-clk", .data = &sun8i_a23_axi_data,},
-	{.compatible = "allwinner,sun4i-a10-ahb-clk", .data = &sun4i_ahb_data,},
-	{.compatible = "allwinner,sun4i-a10-apb0-clk", .data = &sun4i_apb0_data,},
 	{}
 };
 
 /* Matches for divided outputs */
 static const struct of_device_id clk_divs_match[] __initconst = {
-	{.compatible = "allwinner,sun4i-a10-pll5-clk", .data = &pll5_divs_data,},
-	{.compatible = "allwinner,sun4i-a10-pll6-clk", .data = &pll6_divs_data,},
-	{.compatible = "allwinner,sun6i-a31-pll6-clk", .data = &sun6i_a31_pll6_divs_data,},
 	{}
 };
 
 /* Matches for mux clocks */
 static const struct of_device_id clk_mux_match[] __initconst = {
-	{.compatible = "allwinner,sun4i-a10-cpu-clk", .data = &sun4i_cpu_mux_data,},
-	{.compatible = "allwinner,sun6i-a31-ahb1-mux-clk", .data = &sun6i_a31_ahb1_mux_data,},
-	{.compatible = "allwinner,sun8i-h3-ahb2-clk", .data = &sun8i_h3_ahb2_mux_data,},
 	{}
 };
 
-- 
2.6.4

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 4/5] clk: sunxi: Remove old probe and protection code
  2016-02-02  8:47 ` Maxime Ripard
@ 2016-02-02  8:47   ` Maxime Ripard
  -1 siblings, 0 replies; 40+ messages in thread
From: Maxime Ripard @ 2016-02-02  8:47 UTC (permalink / raw)
  To: Mike Turquette, Stephen Boyd, Emilio Lopez, heiko, Chen-Yu Tsai
  Cc: linux-clk, linux-arm-kernel, Andre Przywara, Maxime Ripard

Now that we don't have any user left for the old registration code, we can
remove it.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/clk/sunxi/clk-sunxi.c | 114 ------------------------------------------
 1 file changed, 114 deletions(-)

diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
index da6c1bc763d1..5f2f3e0408bd 100644
--- a/drivers/clk/sunxi/clk-sunxi.c
+++ b/drivers/clk/sunxi/clk-sunxi.c
@@ -1067,117 +1067,3 @@ static void __init sun6i_pll6_clk_setup(struct device_node *node)
 }
 CLK_OF_DECLARE(sun6i_pll6, "allwinner,sun6i-a31-pll6-clk",
 	       sun6i_pll6_clk_setup);
-
-
-/* Matches for factors clocks */
-static const struct of_device_id clk_factors_match[] __initconst = {
-	{}
-};
-
-/* Matches for divider clocks */
-static const struct of_device_id clk_div_match[] __initconst = {
-	{}
-};
-
-/* Matches for divided outputs */
-static const struct of_device_id clk_divs_match[] __initconst = {
-	{}
-};
-
-/* Matches for mux clocks */
-static const struct of_device_id clk_mux_match[] __initconst = {
-	{}
-};
-
-
-static void __init of_sunxi_table_clock_setup(const struct of_device_id *clk_match,
-					      void *function)
-{
-	struct device_node *np;
-	const struct div_data *data;
-	const struct of_device_id *match;
-	void (*setup_function)(struct device_node *, const void *) = function;
-
-	for_each_matching_node_and_match(np, clk_match, &match) {
-		data = match->data;
-		setup_function(np, data);
-	}
-}
-
-static void __init sunxi_init_clocks(const char *clocks[], int nclocks)
-{
-	unsigned int i;
-
-	/* Register divided output clocks */
-	of_sunxi_table_clock_setup(clk_divs_match, sunxi_divs_clk_setup);
-
-	/* Register factor clocks */
-	of_sunxi_table_clock_setup(clk_factors_match, sunxi_factors_clk_setup);
-
-	/* Register divider clocks */
-	of_sunxi_table_clock_setup(clk_div_match, sunxi_divider_clk_setup);
-
-	/* Register mux clocks */
-	of_sunxi_table_clock_setup(clk_mux_match, sunxi_mux_clk_setup);
-
-	/* Protect the clocks that needs to stay on */
-	for (i = 0; i < nclocks; i++) {
-		struct clk *clk = clk_get(NULL, clocks[i]);
-
-		if (!IS_ERR(clk))
-			clk_prepare_enable(clk);
-	}
-}
-
-static const char *sun4i_a10_critical_clocks[] __initdata = {
-	"pll5_ddr",
-};
-
-static void __init sun4i_a10_init_clocks(struct device_node *node)
-{
-	sunxi_init_clocks(sun4i_a10_critical_clocks,
-			  ARRAY_SIZE(sun4i_a10_critical_clocks));
-}
-CLK_OF_DECLARE(sun4i_a10_clk_init, "allwinner,sun4i-a10", sun4i_a10_init_clocks);
-
-static const char *sun5i_critical_clocks[] __initdata = {
-	"cpu",
-	"pll5_ddr",
-};
-
-static void __init sun5i_init_clocks(struct device_node *node)
-{
-	sunxi_init_clocks(sun5i_critical_clocks,
-			  ARRAY_SIZE(sun5i_critical_clocks));
-}
-CLK_OF_DECLARE(sun5i_a10s_clk_init, "allwinner,sun5i-a10s", sun5i_init_clocks);
-CLK_OF_DECLARE(sun5i_a13_clk_init, "allwinner,sun5i-a13", sun5i_init_clocks);
-CLK_OF_DECLARE(sun5i_r8_clk_init, "allwinner,sun5i-r8", sun5i_init_clocks);
-CLK_OF_DECLARE(sun7i_a20_clk_init, "allwinner,sun7i-a20", sun5i_init_clocks);
-
-static const char *sun6i_critical_clocks[] __initdata = {
-	"cpu",
-};
-
-static void __init sun6i_init_clocks(struct device_node *node)
-{
-	sunxi_init_clocks(sun6i_critical_clocks,
-			  ARRAY_SIZE(sun6i_critical_clocks));
-}
-CLK_OF_DECLARE(sun6i_a31_clk_init, "allwinner,sun6i-a31", sun6i_init_clocks);
-CLK_OF_DECLARE(sun6i_a31s_clk_init, "allwinner,sun6i-a31s", sun6i_init_clocks);
-CLK_OF_DECLARE(sun8i_a23_clk_init, "allwinner,sun8i-a23", sun6i_init_clocks);
-CLK_OF_DECLARE(sun8i_a33_clk_init, "allwinner,sun8i-a33", sun6i_init_clocks);
-CLK_OF_DECLARE(sun8i_h3_clk_init, "allwinner,sun8i-h3", sun6i_init_clocks);
-
-static void __init sun8i_a83t_init_clocks(struct device_node *node)
-{
-	sunxi_init_clocks(NULL, 0);
-}
-CLK_OF_DECLARE(sun8i_a83t_clk_init, "allwinner,sun8i-a83t", sun8i_a83t_init_clocks);
-
-static void __init sun9i_init_clocks(struct device_node *node)
-{
-	sunxi_init_clocks(NULL, 0);
-}
-CLK_OF_DECLARE(sun9i_a80_clk_init, "allwinner,sun9i-a80", sun9i_init_clocks);
-- 
2.6.4

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 4/5] clk: sunxi: Remove old probe and protection code
@ 2016-02-02  8:47   ` Maxime Ripard
  0 siblings, 0 replies; 40+ messages in thread
From: Maxime Ripard @ 2016-02-02  8:47 UTC (permalink / raw)
  To: linux-arm-kernel

Now that we don't have any user left for the old registration code, we can
remove it.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/clk/sunxi/clk-sunxi.c | 114 ------------------------------------------
 1 file changed, 114 deletions(-)

diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
index da6c1bc763d1..5f2f3e0408bd 100644
--- a/drivers/clk/sunxi/clk-sunxi.c
+++ b/drivers/clk/sunxi/clk-sunxi.c
@@ -1067,117 +1067,3 @@ static void __init sun6i_pll6_clk_setup(struct device_node *node)
 }
 CLK_OF_DECLARE(sun6i_pll6, "allwinner,sun6i-a31-pll6-clk",
 	       sun6i_pll6_clk_setup);
-
-
-/* Matches for factors clocks */
-static const struct of_device_id clk_factors_match[] __initconst = {
-	{}
-};
-
-/* Matches for divider clocks */
-static const struct of_device_id clk_div_match[] __initconst = {
-	{}
-};
-
-/* Matches for divided outputs */
-static const struct of_device_id clk_divs_match[] __initconst = {
-	{}
-};
-
-/* Matches for mux clocks */
-static const struct of_device_id clk_mux_match[] __initconst = {
-	{}
-};
-
-
-static void __init of_sunxi_table_clock_setup(const struct of_device_id *clk_match,
-					      void *function)
-{
-	struct device_node *np;
-	const struct div_data *data;
-	const struct of_device_id *match;
-	void (*setup_function)(struct device_node *, const void *) = function;
-
-	for_each_matching_node_and_match(np, clk_match, &match) {
-		data = match->data;
-		setup_function(np, data);
-	}
-}
-
-static void __init sunxi_init_clocks(const char *clocks[], int nclocks)
-{
-	unsigned int i;
-
-	/* Register divided output clocks */
-	of_sunxi_table_clock_setup(clk_divs_match, sunxi_divs_clk_setup);
-
-	/* Register factor clocks */
-	of_sunxi_table_clock_setup(clk_factors_match, sunxi_factors_clk_setup);
-
-	/* Register divider clocks */
-	of_sunxi_table_clock_setup(clk_div_match, sunxi_divider_clk_setup);
-
-	/* Register mux clocks */
-	of_sunxi_table_clock_setup(clk_mux_match, sunxi_mux_clk_setup);
-
-	/* Protect the clocks that needs to stay on */
-	for (i = 0; i < nclocks; i++) {
-		struct clk *clk = clk_get(NULL, clocks[i]);
-
-		if (!IS_ERR(clk))
-			clk_prepare_enable(clk);
-	}
-}
-
-static const char *sun4i_a10_critical_clocks[] __initdata = {
-	"pll5_ddr",
-};
-
-static void __init sun4i_a10_init_clocks(struct device_node *node)
-{
-	sunxi_init_clocks(sun4i_a10_critical_clocks,
-			  ARRAY_SIZE(sun4i_a10_critical_clocks));
-}
-CLK_OF_DECLARE(sun4i_a10_clk_init, "allwinner,sun4i-a10", sun4i_a10_init_clocks);
-
-static const char *sun5i_critical_clocks[] __initdata = {
-	"cpu",
-	"pll5_ddr",
-};
-
-static void __init sun5i_init_clocks(struct device_node *node)
-{
-	sunxi_init_clocks(sun5i_critical_clocks,
-			  ARRAY_SIZE(sun5i_critical_clocks));
-}
-CLK_OF_DECLARE(sun5i_a10s_clk_init, "allwinner,sun5i-a10s", sun5i_init_clocks);
-CLK_OF_DECLARE(sun5i_a13_clk_init, "allwinner,sun5i-a13", sun5i_init_clocks);
-CLK_OF_DECLARE(sun5i_r8_clk_init, "allwinner,sun5i-r8", sun5i_init_clocks);
-CLK_OF_DECLARE(sun7i_a20_clk_init, "allwinner,sun7i-a20", sun5i_init_clocks);
-
-static const char *sun6i_critical_clocks[] __initdata = {
-	"cpu",
-};
-
-static void __init sun6i_init_clocks(struct device_node *node)
-{
-	sunxi_init_clocks(sun6i_critical_clocks,
-			  ARRAY_SIZE(sun6i_critical_clocks));
-}
-CLK_OF_DECLARE(sun6i_a31_clk_init, "allwinner,sun6i-a31", sun6i_init_clocks);
-CLK_OF_DECLARE(sun6i_a31s_clk_init, "allwinner,sun6i-a31s", sun6i_init_clocks);
-CLK_OF_DECLARE(sun8i_a23_clk_init, "allwinner,sun8i-a23", sun6i_init_clocks);
-CLK_OF_DECLARE(sun8i_a33_clk_init, "allwinner,sun8i-a33", sun6i_init_clocks);
-CLK_OF_DECLARE(sun8i_h3_clk_init, "allwinner,sun8i-h3", sun6i_init_clocks);
-
-static void __init sun8i_a83t_init_clocks(struct device_node *node)
-{
-	sunxi_init_clocks(NULL, 0);
-}
-CLK_OF_DECLARE(sun8i_a83t_clk_init, "allwinner,sun8i-a83t", sun8i_a83t_init_clocks);
-
-static void __init sun9i_init_clocks(struct device_node *node)
-{
-	sunxi_init_clocks(NULL, 0);
-}
-CLK_OF_DECLARE(sun9i_a80_clk_init, "allwinner,sun9i-a80", sun9i_init_clocks);
-- 
2.6.4

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 5/5] clk: sunxi: Remove clk_register_clkdev calls
  2016-02-02  8:47 ` Maxime Ripard
@ 2016-02-02  8:47   ` Maxime Ripard
  -1 siblings, 0 replies; 40+ messages in thread
From: Maxime Ripard @ 2016-02-02  8:47 UTC (permalink / raw)
  To: Mike Turquette, Stephen Boyd, Emilio Lopez, heiko, Chen-Yu Tsai
  Cc: linux-clk, linux-arm-kernel, Andre Przywara, Maxime Ripard

Now that our protection code doesn't use the global name lookup anymore, we
can remove the clkdev registrations.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/clk/sunxi/clk-a10-hosc.c         | 3 +--
 drivers/clk/sunxi/clk-a20-gmac.c         | 2 --
 drivers/clk/sunxi/clk-factors.c          | 7 -------
 drivers/clk/sunxi/clk-factors.h          | 1 -
 drivers/clk/sunxi/clk-mod0.c             | 1 +
 drivers/clk/sunxi/clk-sun6i-apb0-gates.c | 2 --
 drivers/clk/sunxi/clk-sun9i-core.c       | 1 +
 drivers/clk/sunxi/clk-sunxi.c            | 3 ---
 8 files changed, 3 insertions(+), 17 deletions(-)

diff --git a/drivers/clk/sunxi/clk-a10-hosc.c b/drivers/clk/sunxi/clk-a10-hosc.c
index 0481d5d673d6..6b598c6a0213 100644
--- a/drivers/clk/sunxi/clk-a10-hosc.c
+++ b/drivers/clk/sunxi/clk-a10-hosc.c
@@ -15,9 +15,9 @@
  */
 
 #include <linux/clk-provider.h>
-#include <linux/clkdev.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
+#include <linux/slab.h>
 
 #define SUNXI_OSC24M_GATE	0
 
@@ -61,7 +61,6 @@ static void __init sun4i_osc_clk_setup(struct device_node *node)
 		goto err_free_gate;
 
 	of_clk_add_provider(node, of_clk_src_simple_get, clk);
-	clk_register_clkdev(clk, clk_name, NULL);
 
 	return;
 
diff --git a/drivers/clk/sunxi/clk-a20-gmac.c b/drivers/clk/sunxi/clk-a20-gmac.c
index 1611b036421c..3437f734c9bf 100644
--- a/drivers/clk/sunxi/clk-a20-gmac.c
+++ b/drivers/clk/sunxi/clk-a20-gmac.c
@@ -17,7 +17,6 @@
  */
 
 #include <linux/clk-provider.h>
-#include <linux/clkdev.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
 #include <linux/slab.h>
@@ -107,7 +106,6 @@ static void __init sun7i_a20_gmac_clk_setup(struct device_node *node)
 		goto iounmap_reg;
 
 	of_clk_add_provider(node, of_clk_src_simple_get, clk);
-	clk_register_clkdev(clk, clk_name, NULL);
 
 	return;
 
diff --git a/drivers/clk/sunxi/clk-factors.c b/drivers/clk/sunxi/clk-factors.c
index 6e97f46c0c37..ddefe9668863 100644
--- a/drivers/clk/sunxi/clk-factors.c
+++ b/drivers/clk/sunxi/clk-factors.c
@@ -258,14 +258,8 @@ struct clk *sunxi_factors_register(struct device_node *node,
 	if (ret)
 		goto err_provider;
 
-	ret = clk_register_clkdev(clk, clk_name, NULL);
-	if (ret)
-		goto err_clkdev;
-
 	return clk;
 
-err_clkdev:
-	of_clk_del_provider(node);
 err_provider:
 	/* TODO: The composite clock stuff will leak a bit here. */
 	clk_unregister(clk);
@@ -291,7 +285,6 @@ void sunxi_factors_unregister(struct device_node *node, struct clk *clk)
 	factors = to_clk_factors(hw);
 	name = clk_hw_get_name(hw);
 
-	/* No unregister call for clkdev_* */
 	of_clk_del_provider(node);
 	/* TODO: The composite clock stuff will leak a bit here. */
 	clk_unregister(clk);
diff --git a/drivers/clk/sunxi/clk-factors.h b/drivers/clk/sunxi/clk-factors.h
index a44a865a6b9e..1e63c5b2d5f4 100644
--- a/drivers/clk/sunxi/clk-factors.h
+++ b/drivers/clk/sunxi/clk-factors.h
@@ -2,7 +2,6 @@
 #define __MACH_SUNXI_CLK_FACTORS_H
 
 #include <linux/clk-provider.h>
-#include <linux/clkdev.h>
 #include <linux/spinlock.h>
 
 #define SUNXI_FACTORS_NOT_APPLICABLE	(0)
diff --git a/drivers/clk/sunxi/clk-mod0.c b/drivers/clk/sunxi/clk-mod0.c
index 578bff0dd251..b38d71cec74c 100644
--- a/drivers/clk/sunxi/clk-mod0.c
+++ b/drivers/clk/sunxi/clk-mod0.c
@@ -15,6 +15,7 @@
  */
 
 #include <linux/clk.h>
+#include <linux/clkdev.h>
 #include <linux/clk-provider.h>
 #include <linux/of_address.h>
 #include <linux/platform_device.h>
diff --git a/drivers/clk/sunxi/clk-sun6i-apb0-gates.c b/drivers/clk/sunxi/clk-sun6i-apb0-gates.c
index 23d042aabb4f..68021fa5ecd9 100644
--- a/drivers/clk/sunxi/clk-sun6i-apb0-gates.c
+++ b/drivers/clk/sunxi/clk-sun6i-apb0-gates.c
@@ -9,7 +9,6 @@
  */
 
 #include <linux/clk-provider.h>
-#include <linux/clkdev.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
@@ -87,7 +86,6 @@ static int sun6i_a31_apb0_gates_clk_probe(struct platform_device *pdev)
 						      clk_parent, 0, reg, i,
 						      0, NULL);
 		WARN_ON(IS_ERR(clk_data->clks[i]));
-		clk_register_clkdev(clk_data->clks[i], clk_name, NULL);
 
 		j++;
 	}
diff --git a/drivers/clk/sunxi/clk-sun9i-core.c b/drivers/clk/sunxi/clk-sun9i-core.c
index a81211062ecb..43f014f85803 100644
--- a/drivers/clk/sunxi/clk-sun9i-core.c
+++ b/drivers/clk/sunxi/clk-sun9i-core.c
@@ -15,6 +15,7 @@
  */
 
 #include <linux/clk.h>
+#include <linux/clkdev.h>
 #include <linux/clk-provider.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
index 5f2f3e0408bd..91f6d8ef81df 100644
--- a/drivers/clk/sunxi/clk-sunxi.c
+++ b/drivers/clk/sunxi/clk-sunxi.c
@@ -685,7 +685,6 @@ static struct clk * __init sunxi_mux_clk_setup(struct device_node *node,
 
 	if (clk) {
 		of_clk_add_provider(node, of_clk_src_simple_get, clk);
-		clk_register_clkdev(clk, clk_name, NULL);
 	}
 
 	return clk;
@@ -796,7 +795,6 @@ static void __init sunxi_divider_clk_setup(struct device_node *node,
 					 data->table, &clk_lock);
 	if (clk) {
 		of_clk_add_provider(node, of_clk_src_simple_get, clk);
-		clk_register_clkdev(clk, clk_name, NULL);
 	}
 }
 
@@ -1020,7 +1018,6 @@ static struct clk ** __init sunxi_divs_clk_setup(struct device_node *node,
 						 clkflags);
 
 		WARN_ON(IS_ERR(clk_data->clks[i]));
-		clk_register_clkdev(clks[i], clk_name, NULL);
 	}
 
 	/* Adjust to the real max */
-- 
2.6.4

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 5/5] clk: sunxi: Remove clk_register_clkdev calls
@ 2016-02-02  8:47   ` Maxime Ripard
  0 siblings, 0 replies; 40+ messages in thread
From: Maxime Ripard @ 2016-02-02  8:47 UTC (permalink / raw)
  To: linux-arm-kernel

Now that our protection code doesn't use the global name lookup anymore, we
can remove the clkdev registrations.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/clk/sunxi/clk-a10-hosc.c         | 3 +--
 drivers/clk/sunxi/clk-a20-gmac.c         | 2 --
 drivers/clk/sunxi/clk-factors.c          | 7 -------
 drivers/clk/sunxi/clk-factors.h          | 1 -
 drivers/clk/sunxi/clk-mod0.c             | 1 +
 drivers/clk/sunxi/clk-sun6i-apb0-gates.c | 2 --
 drivers/clk/sunxi/clk-sun9i-core.c       | 1 +
 drivers/clk/sunxi/clk-sunxi.c            | 3 ---
 8 files changed, 3 insertions(+), 17 deletions(-)

diff --git a/drivers/clk/sunxi/clk-a10-hosc.c b/drivers/clk/sunxi/clk-a10-hosc.c
index 0481d5d673d6..6b598c6a0213 100644
--- a/drivers/clk/sunxi/clk-a10-hosc.c
+++ b/drivers/clk/sunxi/clk-a10-hosc.c
@@ -15,9 +15,9 @@
  */
 
 #include <linux/clk-provider.h>
-#include <linux/clkdev.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
+#include <linux/slab.h>
 
 #define SUNXI_OSC24M_GATE	0
 
@@ -61,7 +61,6 @@ static void __init sun4i_osc_clk_setup(struct device_node *node)
 		goto err_free_gate;
 
 	of_clk_add_provider(node, of_clk_src_simple_get, clk);
-	clk_register_clkdev(clk, clk_name, NULL);
 
 	return;
 
diff --git a/drivers/clk/sunxi/clk-a20-gmac.c b/drivers/clk/sunxi/clk-a20-gmac.c
index 1611b036421c..3437f734c9bf 100644
--- a/drivers/clk/sunxi/clk-a20-gmac.c
+++ b/drivers/clk/sunxi/clk-a20-gmac.c
@@ -17,7 +17,6 @@
  */
 
 #include <linux/clk-provider.h>
-#include <linux/clkdev.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
 #include <linux/slab.h>
@@ -107,7 +106,6 @@ static void __init sun7i_a20_gmac_clk_setup(struct device_node *node)
 		goto iounmap_reg;
 
 	of_clk_add_provider(node, of_clk_src_simple_get, clk);
-	clk_register_clkdev(clk, clk_name, NULL);
 
 	return;
 
diff --git a/drivers/clk/sunxi/clk-factors.c b/drivers/clk/sunxi/clk-factors.c
index 6e97f46c0c37..ddefe9668863 100644
--- a/drivers/clk/sunxi/clk-factors.c
+++ b/drivers/clk/sunxi/clk-factors.c
@@ -258,14 +258,8 @@ struct clk *sunxi_factors_register(struct device_node *node,
 	if (ret)
 		goto err_provider;
 
-	ret = clk_register_clkdev(clk, clk_name, NULL);
-	if (ret)
-		goto err_clkdev;
-
 	return clk;
 
-err_clkdev:
-	of_clk_del_provider(node);
 err_provider:
 	/* TODO: The composite clock stuff will leak a bit here. */
 	clk_unregister(clk);
@@ -291,7 +285,6 @@ void sunxi_factors_unregister(struct device_node *node, struct clk *clk)
 	factors = to_clk_factors(hw);
 	name = clk_hw_get_name(hw);
 
-	/* No unregister call for clkdev_* */
 	of_clk_del_provider(node);
 	/* TODO: The composite clock stuff will leak a bit here. */
 	clk_unregister(clk);
diff --git a/drivers/clk/sunxi/clk-factors.h b/drivers/clk/sunxi/clk-factors.h
index a44a865a6b9e..1e63c5b2d5f4 100644
--- a/drivers/clk/sunxi/clk-factors.h
+++ b/drivers/clk/sunxi/clk-factors.h
@@ -2,7 +2,6 @@
 #define __MACH_SUNXI_CLK_FACTORS_H
 
 #include <linux/clk-provider.h>
-#include <linux/clkdev.h>
 #include <linux/spinlock.h>
 
 #define SUNXI_FACTORS_NOT_APPLICABLE	(0)
diff --git a/drivers/clk/sunxi/clk-mod0.c b/drivers/clk/sunxi/clk-mod0.c
index 578bff0dd251..b38d71cec74c 100644
--- a/drivers/clk/sunxi/clk-mod0.c
+++ b/drivers/clk/sunxi/clk-mod0.c
@@ -15,6 +15,7 @@
  */
 
 #include <linux/clk.h>
+#include <linux/clkdev.h>
 #include <linux/clk-provider.h>
 #include <linux/of_address.h>
 #include <linux/platform_device.h>
diff --git a/drivers/clk/sunxi/clk-sun6i-apb0-gates.c b/drivers/clk/sunxi/clk-sun6i-apb0-gates.c
index 23d042aabb4f..68021fa5ecd9 100644
--- a/drivers/clk/sunxi/clk-sun6i-apb0-gates.c
+++ b/drivers/clk/sunxi/clk-sun6i-apb0-gates.c
@@ -9,7 +9,6 @@
  */
 
 #include <linux/clk-provider.h>
-#include <linux/clkdev.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
@@ -87,7 +86,6 @@ static int sun6i_a31_apb0_gates_clk_probe(struct platform_device *pdev)
 						      clk_parent, 0, reg, i,
 						      0, NULL);
 		WARN_ON(IS_ERR(clk_data->clks[i]));
-		clk_register_clkdev(clk_data->clks[i], clk_name, NULL);
 
 		j++;
 	}
diff --git a/drivers/clk/sunxi/clk-sun9i-core.c b/drivers/clk/sunxi/clk-sun9i-core.c
index a81211062ecb..43f014f85803 100644
--- a/drivers/clk/sunxi/clk-sun9i-core.c
+++ b/drivers/clk/sunxi/clk-sun9i-core.c
@@ -15,6 +15,7 @@
  */
 
 #include <linux/clk.h>
+#include <linux/clkdev.h>
 #include <linux/clk-provider.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
index 5f2f3e0408bd..91f6d8ef81df 100644
--- a/drivers/clk/sunxi/clk-sunxi.c
+++ b/drivers/clk/sunxi/clk-sunxi.c
@@ -685,7 +685,6 @@ static struct clk * __init sunxi_mux_clk_setup(struct device_node *node,
 
 	if (clk) {
 		of_clk_add_provider(node, of_clk_src_simple_get, clk);
-		clk_register_clkdev(clk, clk_name, NULL);
 	}
 
 	return clk;
@@ -796,7 +795,6 @@ static void __init sunxi_divider_clk_setup(struct device_node *node,
 					 data->table, &clk_lock);
 	if (clk) {
 		of_clk_add_provider(node, of_clk_src_simple_get, clk);
-		clk_register_clkdev(clk, clk_name, NULL);
 	}
 }
 
@@ -1020,7 +1018,6 @@ static struct clk ** __init sunxi_divs_clk_setup(struct device_node *node,
 						 clkflags);
 
 		WARN_ON(IS_ERR(clk_data->clks[i]));
-		clk_register_clkdev(clks[i], clk_name, NULL);
 	}
 
 	/* Adjust to the real max */
-- 
2.6.4

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* Re: [PATCH 5/5] clk: sunxi: Remove clk_register_clkdev calls
  2016-02-02  8:47   ` Maxime Ripard
@ 2016-02-02 10:26     ` Jean-Francois Moine
  -1 siblings, 0 replies; 40+ messages in thread
From: Jean-Francois Moine @ 2016-02-02 10:26 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Mike Turquette, Stephen Boyd, Emilio Lopez, heiko, Chen-Yu Tsai,
	Andre Przywara, linux-clk, linux-arm-kernel

On Tue,  2 Feb 2016 09:47:14 +0100
Maxime Ripard <maxime.ripard@free-electrons.com> wrote:

> diff --git a/drivers/clk/sunxi/clk-mod0.c b/drivers/clk/sunxi/clk-mod0.c
> index 578bff0dd251..b38d71cec74c 100644
> --- a/drivers/clk/sunxi/clk-mod0.c
> +++ b/drivers/clk/sunxi/clk-mod0.c
> @@ -15,6 +15,7 @@
>   */
> =20
>  #include <linux/clk.h>
> +#include <linux/clkdev.h>

Why?

>  #include <linux/clk-provider.h>
>  #include <linux/of_address.h>
>  #include <linux/platform_device.h>


--=20
Ken ar c'henta=F1	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/

^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH 5/5] clk: sunxi: Remove clk_register_clkdev calls
@ 2016-02-02 10:26     ` Jean-Francois Moine
  0 siblings, 0 replies; 40+ messages in thread
From: Jean-Francois Moine @ 2016-02-02 10:26 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue,  2 Feb 2016 09:47:14 +0100
Maxime Ripard <maxime.ripard@free-electrons.com> wrote:

> diff --git a/drivers/clk/sunxi/clk-mod0.c b/drivers/clk/sunxi/clk-mod0.c
> index 578bff0dd251..b38d71cec74c 100644
> --- a/drivers/clk/sunxi/clk-mod0.c
> +++ b/drivers/clk/sunxi/clk-mod0.c
> @@ -15,6 +15,7 @@
>   */
>  
>  #include <linux/clk.h>
> +#include <linux/clkdev.h>

Why?

>  #include <linux/clk-provider.h>
>  #include <linux/of_address.h>
>  #include <linux/platform_device.h>


-- 
Ken ar c'henta?	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/

^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH 1/5] clk: sunxi: Make clocks setup functions return their clock
  2016-02-02  8:47   ` Maxime Ripard
  (?)
@ 2016-02-02 10:35   ` Chen-Yu Tsai
  2016-02-02 11:32       ` Andre Przywara
  2016-02-02 13:27       ` Maxime Ripard
  -1 siblings, 2 replies; 40+ messages in thread
From: Chen-Yu Tsai @ 2016-02-02 10:35 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Feb 2, 2016 at 4:47 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> The clocks registration code in clk-sunxi was most of the time not
> returning the struct clk (or struct clk array) that was registered,
> preventing the users of such functions to manipulate it, for example to
> protect it.
>
> Make them return it so that we can start using it.

Should they return error codes as well? So the CLK_OF_DECLARE setup functions
can report errors. Or make these helper functions report errors directly?

ChenYu

> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  drivers/clk/sunxi/clk-sunxi.c | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
> index e460a6b0068c..67ef94948544 100644
> --- a/drivers/clk/sunxi/clk-sunxi.c
> +++ b/drivers/clk/sunxi/clk-sunxi.c
> @@ -615,8 +615,8 @@ static const struct mux_data sun8i_h3_ahb2_mux_data __initconst = {
>         .shift = 0,
>  };
>
> -static void __init sunxi_mux_clk_setup(struct device_node *node,
> -                                      struct mux_data *data)
> +static struct clk * __init sunxi_mux_clk_setup(struct device_node *node,
> +                                              struct mux_data *data)
>  {
>         struct clk *clk;
>         const char *clk_name = node->name;
> @@ -638,6 +638,8 @@ static void __init sunxi_mux_clk_setup(struct device_node *node,
>                 of_clk_add_provider(node, of_clk_src_simple_get, clk);
>                 clk_register_clkdev(clk, clk_name, NULL);
>         }
> +
> +       return clk;
>  }
>
>
> @@ -722,7 +724,6 @@ static void __init sunxi_divider_clk_setup(struct device_node *node,
>  }
>
>
> -
>  /**
>   * sunxi_gates_clk_setup() - Setup function for leaf gates on clocks
>   */
> @@ -808,8 +809,8 @@ static const struct divs_data sun6i_a31_pll6_divs_data __initconst = {
>   *           |________________________|
>   */
>
> -static void __init sunxi_divs_clk_setup(struct device_node *node,
> -                                       struct divs_data *data)
> +static struct clk ** __init sunxi_divs_clk_setup(struct device_node *node,
> +                                                struct divs_data *data)
>  {
>         struct clk_onecell_data *clk_data;
>         const char *parent;
> @@ -836,7 +837,7 @@ static void __init sunxi_divs_clk_setup(struct device_node *node,
>
>         clk_data = kmalloc(sizeof(struct clk_onecell_data), GFP_KERNEL);
>         if (!clk_data)
> -               return;
> +               return NULL;
>
>         clks = kcalloc(ndivs, sizeof(*clks), GFP_KERNEL);
>         if (!clks)
> @@ -922,7 +923,7 @@ static void __init sunxi_divs_clk_setup(struct device_node *node,
>
>         of_clk_add_provider(node, of_clk_src_onecell_get, clk_data);
>
> -       return;
> +       return clks;
>
>  free_gate:
>         kfree(gate);
> @@ -930,6 +931,7 @@ free_clks:
>         kfree(clks);
>  free_clkdata:
>         kfree(clk_data);
> +       return NULL;
>  }
>
>
> --
> 2.6.4
>

^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH 2/5] clk: sunxi: Make clocks setup functions take const pointer
  2016-02-02  8:47   ` Maxime Ripard
  (?)
@ 2016-02-02 10:38   ` Chen-Yu Tsai
  -1 siblings, 0 replies; 40+ messages in thread
From: Chen-Yu Tsai @ 2016-02-02 10:38 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Feb 2, 2016 at 4:47 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> All the data structure that we pass to the clocks setup functions are
> declared const, while our setup functions expects a regular pointer. This
> was hidden by the fact that we cast a void * pointer back to these
> structures, which made it go unnoticed.
>
> Fix the functions prototype.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Acked-by: Chen-Yu Tsai <wens@csie.org>

> ---
>  drivers/clk/sunxi/clk-sunxi.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
> index 67ef94948544..a3d706f5b21c 100644
> --- a/drivers/clk/sunxi/clk-sunxi.c
> +++ b/drivers/clk/sunxi/clk-sunxi.c
> @@ -616,7 +616,7 @@ static const struct mux_data sun8i_h3_ahb2_mux_data __initconst = {
>  };
>
>  static struct clk * __init sunxi_mux_clk_setup(struct device_node *node,
> -                                              struct mux_data *data)
> +                                              const struct mux_data *data)
>  {
>         struct clk *clk;
>         const char *clk_name = node->name;
> @@ -700,7 +700,7 @@ static const struct div_data sun4i_apb0_data __initconst = {
>  };
>
>  static void __init sunxi_divider_clk_setup(struct device_node *node,
> -                                          struct div_data *data)
> +                                          const struct div_data *data)
>  {
>         struct clk *clk;
>         const char *clk_name = node->name;
> @@ -810,7 +810,7 @@ static const struct divs_data sun6i_a31_pll6_divs_data __initconst = {
>   */
>
>  static struct clk ** __init sunxi_divs_clk_setup(struct device_node *node,
> -                                                struct divs_data *data)
> +                                                const struct divs_data *data)
>  {
>         struct clk_onecell_data *clk_data;
>         const char *parent;
> --
> 2.6.4
>

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 1/5] clk: sunxi: Make clocks setup functions return their clock
  2016-02-02 10:35   ` Chen-Yu Tsai
@ 2016-02-02 11:32       ` Andre Przywara
  2016-02-02 13:27       ` Maxime Ripard
  1 sibling, 0 replies; 40+ messages in thread
From: Andre Przywara @ 2016-02-02 11:32 UTC (permalink / raw)
  To: Chen-Yu Tsai, Maxime Ripard
  Cc: Mike Turquette, Stephen Boyd, Emilio Lopez, Heiko Stübner,
	linux-clk, linux-arm-kernel

Hi Maxime,

On 02/02/16 10:35, Chen-Yu Tsai wrote:
> On Tue, Feb 2, 2016 at 4:47 PM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
>> The clocks registration code in clk-sunxi was most of the time not
>> returning the struct clk (or struct clk array) that was registered,
>> preventing the users of such functions to manipulate it, for example to
>> protect it.
>>
>> Make them return it so that we can start using it.
> 
> Should they return error codes as well? So the CLK_OF_DECLARE setup functions
> can report errors. Or make these helper functions report errors directly?

Yeah, I was about the reply the same.
It looks like we can use the ERR_PTR facility to achieve this, as it's
already used in other parts of the clk framework.

For instance ...

> ChenYu
> 
>> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
>> ---
>>  drivers/clk/sunxi/clk-sunxi.c | 16 +++++++++-------
>>  1 file changed, 9 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
>> index e460a6b0068c..67ef94948544 100644
>> --- a/drivers/clk/sunxi/clk-sunxi.c
>> +++ b/drivers/clk/sunxi/clk-sunxi.c
>> @@ -615,8 +615,8 @@ static const struct mux_data sun8i_h3_ahb2_mux_data __initconst = {
>>         .shift = 0,
>>  };
>>
>> -static void __init sunxi_mux_clk_setup(struct device_node *node,
>> -                                      struct mux_data *data)
>> +static struct clk * __init sunxi_mux_clk_setup(struct device_node *node,
>> +                                              struct mux_data *data)
>>  {
>>         struct clk *clk;
>>         const char *clk_name = node->name;
>> @@ -638,6 +638,8 @@ static void __init sunxi_mux_clk_setup(struct device_node *node,
>>                 of_clk_add_provider(node, of_clk_src_simple_get, clk);
>>                 clk_register_clkdev(clk, clk_name, NULL);

... here we have two functions that could fail and actually return an
error code.
This whole clock tree is quite complex already, so having clock
initialisations fail without any error message is not really helping.
Doing: "if (ret) {pr_warn(...); return ERR_PTR(ret);}" should be doable,
I think. Or we do it in the upper layers, when we have the error reason.

Otherwise I like the approach!

Cheers,
Andre.

>>         }
>> +
>> +       return clk;
>>  }
>>
>>
>> @@ -722,7 +724,6 @@ static void __init sunxi_divider_clk_setup(struct device_node *node,
>>  }
>>
>>
>> -
>>  /**
>>   * sunxi_gates_clk_setup() - Setup function for leaf gates on clocks
>>   */
>> @@ -808,8 +809,8 @@ static const struct divs_data sun6i_a31_pll6_divs_data __initconst = {
>>   *           |________________________|
>>   */
>>
>> -static void __init sunxi_divs_clk_setup(struct device_node *node,
>> -                                       struct divs_data *data)
>> +static struct clk ** __init sunxi_divs_clk_setup(struct device_node *node,
>> +                                                struct divs_data *data)
>>  {
>>         struct clk_onecell_data *clk_data;
>>         const char *parent;
>> @@ -836,7 +837,7 @@ static void __init sunxi_divs_clk_setup(struct device_node *node,
>>
>>         clk_data = kmalloc(sizeof(struct clk_onecell_data), GFP_KERNEL);
>>         if (!clk_data)
>> -               return;
>> +               return NULL;
>>
>>         clks = kcalloc(ndivs, sizeof(*clks), GFP_KERNEL);
>>         if (!clks)
>> @@ -922,7 +923,7 @@ static void __init sunxi_divs_clk_setup(struct device_node *node,
>>
>>         of_clk_add_provider(node, of_clk_src_onecell_get, clk_data);
>>
>> -       return;
>> +       return clks;
>>
>>  free_gate:
>>         kfree(gate);
>> @@ -930,6 +931,7 @@ free_clks:
>>         kfree(clks);
>>  free_clkdata:
>>         kfree(clk_data);
>> +       return NULL;
>>  }
>>
>>
>> --
>> 2.6.4
>>
> 

^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH 1/5] clk: sunxi: Make clocks setup functions return their clock
@ 2016-02-02 11:32       ` Andre Przywara
  0 siblings, 0 replies; 40+ messages in thread
From: Andre Przywara @ 2016-02-02 11:32 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Maxime,

On 02/02/16 10:35, Chen-Yu Tsai wrote:
> On Tue, Feb 2, 2016 at 4:47 PM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
>> The clocks registration code in clk-sunxi was most of the time not
>> returning the struct clk (or struct clk array) that was registered,
>> preventing the users of such functions to manipulate it, for example to
>> protect it.
>>
>> Make them return it so that we can start using it.
> 
> Should they return error codes as well? So the CLK_OF_DECLARE setup functions
> can report errors. Or make these helper functions report errors directly?

Yeah, I was about the reply the same.
It looks like we can use the ERR_PTR facility to achieve this, as it's
already used in other parts of the clk framework.

For instance ...

> ChenYu
> 
>> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
>> ---
>>  drivers/clk/sunxi/clk-sunxi.c | 16 +++++++++-------
>>  1 file changed, 9 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
>> index e460a6b0068c..67ef94948544 100644
>> --- a/drivers/clk/sunxi/clk-sunxi.c
>> +++ b/drivers/clk/sunxi/clk-sunxi.c
>> @@ -615,8 +615,8 @@ static const struct mux_data sun8i_h3_ahb2_mux_data __initconst = {
>>         .shift = 0,
>>  };
>>
>> -static void __init sunxi_mux_clk_setup(struct device_node *node,
>> -                                      struct mux_data *data)
>> +static struct clk * __init sunxi_mux_clk_setup(struct device_node *node,
>> +                                              struct mux_data *data)
>>  {
>>         struct clk *clk;
>>         const char *clk_name = node->name;
>> @@ -638,6 +638,8 @@ static void __init sunxi_mux_clk_setup(struct device_node *node,
>>                 of_clk_add_provider(node, of_clk_src_simple_get, clk);
>>                 clk_register_clkdev(clk, clk_name, NULL);

... here we have two functions that could fail and actually return an
error code.
This whole clock tree is quite complex already, so having clock
initialisations fail without any error message is not really helping.
Doing: "if (ret) {pr_warn(...); return ERR_PTR(ret);}" should be doable,
I think. Or we do it in the upper layers, when we have the error reason.

Otherwise I like the approach!

Cheers,
Andre.

>>         }
>> +
>> +       return clk;
>>  }
>>
>>
>> @@ -722,7 +724,6 @@ static void __init sunxi_divider_clk_setup(struct device_node *node,
>>  }
>>
>>
>> -
>>  /**
>>   * sunxi_gates_clk_setup() - Setup function for leaf gates on clocks
>>   */
>> @@ -808,8 +809,8 @@ static const struct divs_data sun6i_a31_pll6_divs_data __initconst = {
>>   *           |________________________|
>>   */
>>
>> -static void __init sunxi_divs_clk_setup(struct device_node *node,
>> -                                       struct divs_data *data)
>> +static struct clk ** __init sunxi_divs_clk_setup(struct device_node *node,
>> +                                                struct divs_data *data)
>>  {
>>         struct clk_onecell_data *clk_data;
>>         const char *parent;
>> @@ -836,7 +837,7 @@ static void __init sunxi_divs_clk_setup(struct device_node *node,
>>
>>         clk_data = kmalloc(sizeof(struct clk_onecell_data), GFP_KERNEL);
>>         if (!clk_data)
>> -               return;
>> +               return NULL;
>>
>>         clks = kcalloc(ndivs, sizeof(*clks), GFP_KERNEL);
>>         if (!clks)
>> @@ -922,7 +923,7 @@ static void __init sunxi_divs_clk_setup(struct device_node *node,
>>
>>         of_clk_add_provider(node, of_clk_src_onecell_get, clk_data);
>>
>> -       return;
>> +       return clks;
>>
>>  free_gate:
>>         kfree(gate);
>> @@ -930,6 +931,7 @@ free_clks:
>>         kfree(clks);
>>  free_clkdata:
>>         kfree(clk_data);
>> +       return NULL;
>>  }
>>
>>
>> --
>> 2.6.4
>>
> 

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 2/5] clk: sunxi: Make clocks setup functions take const pointer
  2016-02-02  8:47   ` Maxime Ripard
@ 2016-02-02 12:11     ` Andre Przywara
  -1 siblings, 0 replies; 40+ messages in thread
From: Andre Przywara @ 2016-02-02 12:11 UTC (permalink / raw)
  To: Maxime Ripard, Mike Turquette, Stephen Boyd, Emilio Lopez, heiko,
	Chen-Yu Tsai
  Cc: linux-clk, linux-arm-kernel

On 02/02/16 08:47, Maxime Ripard wrote:
> All the data structure that we pass to the clocks setup functions are
> declared const, while our setup functions expects a regular pointer. This
> was hidden by the fact that we cast a void * pointer back to these
> structures, which made it go unnoticed.
> 
> Fix the functions prototype.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Cheers,
Andre.

> ---
>  drivers/clk/sunxi/clk-sunxi.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
> index 67ef94948544..a3d706f5b21c 100644
> --- a/drivers/clk/sunxi/clk-sunxi.c
> +++ b/drivers/clk/sunxi/clk-sunxi.c
> @@ -616,7 +616,7 @@ static const struct mux_data sun8i_h3_ahb2_mux_data __initconst = {
>  };
>  
>  static struct clk * __init sunxi_mux_clk_setup(struct device_node *node,
> -					       struct mux_data *data)
> +					       const struct mux_data *data)
>  {
>  	struct clk *clk;
>  	const char *clk_name = node->name;
> @@ -700,7 +700,7 @@ static const struct div_data sun4i_apb0_data __initconst = {
>  };
>  
>  static void __init sunxi_divider_clk_setup(struct device_node *node,
> -					   struct div_data *data)
> +					   const struct div_data *data)
>  {
>  	struct clk *clk;
>  	const char *clk_name = node->name;
> @@ -810,7 +810,7 @@ static const struct divs_data sun6i_a31_pll6_divs_data __initconst = {
>   */
>  
>  static struct clk ** __init sunxi_divs_clk_setup(struct device_node *node,
> -						 struct divs_data *data)
> +						 const struct divs_data *data)
>  {
>  	struct clk_onecell_data *clk_data;
>  	const char *parent;
> 

^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH 2/5] clk: sunxi: Make clocks setup functions take const pointer
@ 2016-02-02 12:11     ` Andre Przywara
  0 siblings, 0 replies; 40+ messages in thread
From: Andre Przywara @ 2016-02-02 12:11 UTC (permalink / raw)
  To: linux-arm-kernel

On 02/02/16 08:47, Maxime Ripard wrote:
> All the data structure that we pass to the clocks setup functions are
> declared const, while our setup functions expects a regular pointer. This
> was hidden by the fact that we cast a void * pointer back to these
> structures, which made it go unnoticed.
> 
> Fix the functions prototype.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Cheers,
Andre.

> ---
>  drivers/clk/sunxi/clk-sunxi.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
> index 67ef94948544..a3d706f5b21c 100644
> --- a/drivers/clk/sunxi/clk-sunxi.c
> +++ b/drivers/clk/sunxi/clk-sunxi.c
> @@ -616,7 +616,7 @@ static const struct mux_data sun8i_h3_ahb2_mux_data __initconst = {
>  };
>  
>  static struct clk * __init sunxi_mux_clk_setup(struct device_node *node,
> -					       struct mux_data *data)
> +					       const struct mux_data *data)
>  {
>  	struct clk *clk;
>  	const char *clk_name = node->name;
> @@ -700,7 +700,7 @@ static const struct div_data sun4i_apb0_data __initconst = {
>  };
>  
>  static void __init sunxi_divider_clk_setup(struct device_node *node,
> -					   struct div_data *data)
> +					   const struct div_data *data)
>  {
>  	struct clk *clk;
>  	const char *clk_name = node->name;
> @@ -810,7 +810,7 @@ static const struct divs_data sun6i_a31_pll6_divs_data __initconst = {
>   */
>  
>  static struct clk ** __init sunxi_divs_clk_setup(struct device_node *node,
> -						 struct divs_data *data)
> +						 const struct divs_data *data)
>  {
>  	struct clk_onecell_data *clk_data;
>  	const char *parent;
> 

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 1/5] clk: sunxi: Make clocks setup functions return their clock
  2016-02-02 10:35   ` Chen-Yu Tsai
@ 2016-02-02 13:27       ` Maxime Ripard
  2016-02-02 13:27       ` Maxime Ripard
  1 sibling, 0 replies; 40+ messages in thread
From: Maxime Ripard @ 2016-02-02 13:27 UTC (permalink / raw)
  To: Chen-Yu Tsai
  Cc: Mike Turquette, Stephen Boyd, Emilio Lopez, Heiko Stübner,
	linux-clk, linux-arm-kernel, Andre Przywara

[-- Attachment #1: Type: text/plain, Size: 1009 bytes --]

Hi,

On Tue, Feb 02, 2016 at 06:35:29PM +0800, Chen-Yu Tsai wrote:
> On Tue, Feb 2, 2016 at 4:47 PM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > The clocks registration code in clk-sunxi was most of the time not
> > returning the struct clk (or struct clk array) that was registered,
> > preventing the users of such functions to manipulate it, for example to
> > protect it.
> >
> > Make them return it so that we can start using it.
> 
> Should they return error codes as well? So the CLK_OF_DECLARE setup functions
> can report errors. Or make these helper functions report errors directly?

CLK_OF_DECLARE doesn't handle any kind of error, as the setup function
must return void, and the error printing should rather be in the
common functions in order to avoid duplicating it everywhere, so I'm
not sure. Or did you had something else in mind?

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH 1/5] clk: sunxi: Make clocks setup functions return their clock
@ 2016-02-02 13:27       ` Maxime Ripard
  0 siblings, 0 replies; 40+ messages in thread
From: Maxime Ripard @ 2016-02-02 13:27 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On Tue, Feb 02, 2016 at 06:35:29PM +0800, Chen-Yu Tsai wrote:
> On Tue, Feb 2, 2016 at 4:47 PM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > The clocks registration code in clk-sunxi was most of the time not
> > returning the struct clk (or struct clk array) that was registered,
> > preventing the users of such functions to manipulate it, for example to
> > protect it.
> >
> > Make them return it so that we can start using it.
> 
> Should they return error codes as well? So the CLK_OF_DECLARE setup functions
> can report errors. Or make these helper functions report errors directly?

CLK_OF_DECLARE doesn't handle any kind of error, as the setup function
must return void, and the error printing should rather be in the
common functions in order to avoid duplicating it everywhere, so I'm
not sure. Or did you had something else in mind?

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160202/9af172d6/attachment.sig>

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 3/5] clk: sunxi: convert current clocks registration to CLK_OF_DECLARE
  2016-02-02  8:47   ` Maxime Ripard
@ 2016-02-02 14:16     ` Andre Przywara
  -1 siblings, 0 replies; 40+ messages in thread
From: Andre Przywara @ 2016-02-02 14:16 UTC (permalink / raw)
  To: Maxime Ripard, Mike Turquette, Stephen Boyd, Emilio Lopez, heiko,
	Chen-Yu Tsai
  Cc: linux-clk, linux-arm-kernel

Hi Maxime,

On 02/02/16 08:47, Maxime Ripard wrote:
> The current clock registration and protection code has a few drawbacks, the
> two main ones being that we create a lot of orphans clock in the
> registration phase, which will be troublesome when we will start being less
> relaxed about them.
> 
> The protection code also relies on clkdev, which we don't really use but
> for this particular case.
> 
> Fix both at the same time by moving everyone to the CLK_OF_DECLARE that
> will probe our clock tree in the right and thus avoid orphans, and by
> protecting directly the clock returned by our registration function.

I very much appreciate this cleanup and like the idea. Any chance we can
have this rather quickly, so that I can rebase the A64 support series on it?

One issue below, though:

...

> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  drivers/clk/sunxi/clk-sunxi.c | 150 +++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 133 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
> index a3d706f5b21c..da6c1bc763d1 100644
> --- a/drivers/clk/sunxi/clk-sunxi.c
> +++ b/drivers/clk/sunxi/clk-sunxi.c
> @@ -585,6 +585,41 @@ static struct clk * __init sunxi_factors_clk_setup(struct device_node *node,
>  	return sunxi_factors_register(node, data, &clk_lock, reg);
>  }
>  
> +static void __init sun4i_pll1_clk_setup(struct device_node *node)
> +{
> +	sunxi_factors_clk_setup(node, &sun4i_pll1_data);
> +}
> +CLK_OF_DECLARE(sun4i_pll1, "allwinner,sun4i-a10-pll1-clk",
> +	       sun4i_pll1_clk_setup);
> +
> +static void __init sun6i_pll1_clk_setup(struct device_node *node)
> +{
> +	sunxi_factors_clk_setup(node, &sun6i_a31_pll1_data);
> +}
> +CLK_OF_DECLARE(sun6i_pll1, "allwinner,sun6i-a31-pll1-clk",
> +	       sun6i_pll1_clk_setup);
> +
> +static void __init sun8i_pll1_clk_setup(struct device_node *node)
> +{
> +	sunxi_factors_clk_setup(node, &sun8i_a23_pll1_data);
> +}
> +CLK_OF_DECLARE(sun8i_pll1, "allwinner,sun8i-a23-pll1-clk",
> +	       sun8i_pll1_clk_setup);
> +
> +static void __init sun7i_pll4_clk_setup(struct device_node *node)
> +{
> +	sunxi_factors_clk_setup(node, &sun7i_a20_pll4_data);
> +}
> +CLK_OF_DECLARE(sun7i_pll4, "allwinner,sun7i-a20-pll4-clk",
> +	       sun7i_pll4_clk_setup);
> +
> +static void __init sun5i_ahb_clk_setup(struct device_node *node)
> +{
> +	sunxi_factors_clk_setup(node, &sun5i_a13_ahb_data);
> +}
> +CLK_OF_DECLARE(sun5i_ahb, "allwinner,sun5i-a13-ahb-clk",
> +	       sun5i_ahb_clk_setup);
> +
>  static void __init sun6i_ahb1_clk_setup(struct device_node *node)
>  {
>  	sunxi_factors_clk_setup(node, &sun6i_ahb1_data);
> @@ -592,6 +627,20 @@ static void __init sun6i_ahb1_clk_setup(struct device_node *node)
>  CLK_OF_DECLARE(sun6i_a31_ahb1, "allwinner,sun6i-a31-ahb1-clk",
>  	       sun6i_ahb1_clk_setup);
>  
> +static void __init sun4i_apb1_clk_setup(struct device_node *node)
> +{
> +	sunxi_factors_clk_setup(node, &sun4i_apb1_data);
> +}
> +CLK_OF_DECLARE(sun4i_apb1, "allwinner,sun4i-a10-apb1-clk",
> +	       sun4i_apb1_clk_setup);
> +
> +static void __init sun7i_out_clk_setup(struct device_node *node)
> +{
> +	sunxi_factors_clk_setup(node, &sun7i_a20_out_data);
> +}
> +CLK_OF_DECLARE(sun7i_out, "allwinner,sun7i-a20-out-clk",
> +	       sun7i_out_clk_setup);
> +
>  
>  /**
>   * sunxi_mux_clk_setup() - Setup function for muxes
> @@ -642,6 +691,34 @@ static struct clk * __init sunxi_mux_clk_setup(struct device_node *node,
>  	return clk;
>  }
>  
> +static void __init sun4i_cpu_clk_setup(struct device_node *node)
> +{
> +	struct clk *clk;
> +
> +	clk = sunxi_mux_clk_setup(node, &sun4i_cpu_mux_data);
> +	if (!clk)
> +		return;
> +
> +	/* Protect CPU clock */
> +	__clk_get(clk);
> +	clk_prepare_enable(clk);
> +}
> +CLK_OF_DECLARE(sun4i_cpu, "allwinner,sun4i-a10-cpu-clk",
> +	       sun4i_cpu_clk_setup);
> +
> +static void __init sun6i_ahb1_mux_clk_setup(struct device_node *node)
> +{
> +	sunxi_mux_clk_setup(node, &sun6i_a31_ahb1_mux_data);
> +}
> +CLK_OF_DECLARE(sun6i_ahb1_mux, "allwinner,sun6i-a31-ahb1-mux-clk",
> +	       sun6i_ahb1_mux_clk_setup);
> +
> +static void __init sun8i_ahb2_clk_setup(struct device_node *node)
> +{
> +	sunxi_mux_clk_setup(node, &sun8i_h3_ahb2_mux_data);
> +}
> +CLK_OF_DECLARE(sun8i_ahb2, "allwinner,sun8i-a31-ahb2-clk",
> +	       sun8i_ahb2_clk_setup);

I don't find this clock in my tree (which is mripard/sunxi/for-next).
Instead I only have "allwinner,sun8i-h3-ahb2-clk", as mentioned below.
But as you remove this clock below from the old code and instead
instantiate this new clock here, this looks somehow wrong to me. Can you
confirm this or am I utterly confused?

Apart from that I checked each and every clock mentioned in this patch
and can confirm that the transformation is correct. So if you fix this,
I can send a Reviewed-by.

Cheers,
Andre.

>  
>  
>  /**
> @@ -723,6 +800,34 @@ static void __init sunxi_divider_clk_setup(struct device_node *node,
>  	}
>  }
>  
> +static void __init sun4i_ahb_clk_setup(struct device_node *node)
> +{
> +	sunxi_divider_clk_setup(node, &sun4i_ahb_data);
> +}
> +CLK_OF_DECLARE(sun4i_ahb, "allwinner,sun4i-a10-ahb-clk",
> +	       sun4i_ahb_clk_setup);
> +
> +static void __init sun4i_apb0_clk_setup(struct device_node *node)
> +{
> +	sunxi_divider_clk_setup(node, &sun4i_apb0_data);
> +}
> +CLK_OF_DECLARE(sun4i_apb0, "allwinner,sun4i-a10-apb0-clk",
> +	       sun4i_apb0_clk_setup);
> +
> +static void __init sun4i_axi_clk_setup(struct device_node *node)
> +{
> +	sunxi_divider_clk_setup(node, &sun4i_axi_data);
> +}
> +CLK_OF_DECLARE(sun4i_axi, "allwinner,sun4i-a10-axi-clk",
> +	       sun4i_axi_clk_setup);
> +
> +static void __init sun8i_axi_clk_setup(struct device_node *node)
> +{
> +	sunxi_divider_clk_setup(node, &sun8i_a23_axi_data);
> +}
> +CLK_OF_DECLARE(sun8i_axi, "allwinner,sun8i-a23-axi-clk",
> +	       sun8i_axi_clk_setup);
> +
>  
>  /**
>   * sunxi_gates_clk_setup() - Setup function for leaf gates on clocks
> @@ -934,42 +1039,53 @@ free_clkdata:
>  	return NULL;
>  }
>  
> +static void __init sun4i_pll5_clk_setup(struct device_node *node)
> +{
> +	struct clk **clks;
> +
> +	clks = sunxi_divs_clk_setup(node, &pll5_divs_data);
> +	if (!clks)
> +		return;
> +
> +	/* Protect PLL5_DDR */
> +	__clk_get(clks[0]);
> +	clk_prepare_enable(clks[0]);
> +}
> +CLK_OF_DECLARE(sun4i_pll5, "allwinner,sun4i-a10-pll5-clk",
> +	       sun4i_pll5_clk_setup);
> +
> +static void __init sun4i_pll6_clk_setup(struct device_node *node)
> +{
> +	sunxi_divs_clk_setup(node, &pll6_divs_data);
> +}
> +CLK_OF_DECLARE(sun4i_pll6, "allwinner,sun4i-a10-pll6-clk",
> +	       sun4i_pll6_clk_setup);
> +
> +static void __init sun6i_pll6_clk_setup(struct device_node *node)
> +{
> +	sunxi_divs_clk_setup(node, &sun6i_a31_pll6_divs_data);
> +}
> +CLK_OF_DECLARE(sun6i_pll6, "allwinner,sun6i-a31-pll6-clk",
> +	       sun6i_pll6_clk_setup);
>  
>  
>  /* Matches for factors clocks */
>  static const struct of_device_id clk_factors_match[] __initconst = {
> -	{.compatible = "allwinner,sun4i-a10-pll1-clk", .data = &sun4i_pll1_data,},
> -	{.compatible = "allwinner,sun6i-a31-pll1-clk", .data = &sun6i_a31_pll1_data,},
> -	{.compatible = "allwinner,sun8i-a23-pll1-clk", .data = &sun8i_a23_pll1_data,},
> -	{.compatible = "allwinner,sun7i-a20-pll4-clk", .data = &sun7i_a20_pll4_data,},
> -	{.compatible = "allwinner,sun5i-a13-ahb-clk", .data = &sun5i_a13_ahb_data,},
> -	{.compatible = "allwinner,sun4i-a10-apb1-clk", .data = &sun4i_apb1_data,},
> -	{.compatible = "allwinner,sun7i-a20-out-clk", .data = &sun7i_a20_out_data,},
>  	{}
>  };
>  
>  /* Matches for divider clocks */
>  static const struct of_device_id clk_div_match[] __initconst = {
> -	{.compatible = "allwinner,sun4i-a10-axi-clk", .data = &sun4i_axi_data,},
> -	{.compatible = "allwinner,sun8i-a23-axi-clk", .data = &sun8i_a23_axi_data,},
> -	{.compatible = "allwinner,sun4i-a10-ahb-clk", .data = &sun4i_ahb_data,},
> -	{.compatible = "allwinner,sun4i-a10-apb0-clk", .data = &sun4i_apb0_data,},
>  	{}
>  };
>  
>  /* Matches for divided outputs */
>  static const struct of_device_id clk_divs_match[] __initconst = {
> -	{.compatible = "allwinner,sun4i-a10-pll5-clk", .data = &pll5_divs_data,},
> -	{.compatible = "allwinner,sun4i-a10-pll6-clk", .data = &pll6_divs_data,},
> -	{.compatible = "allwinner,sun6i-a31-pll6-clk", .data = &sun6i_a31_pll6_divs_data,},
>  	{}
>  };
>  
>  /* Matches for mux clocks */
>  static const struct of_device_id clk_mux_match[] __initconst = {
> -	{.compatible = "allwinner,sun4i-a10-cpu-clk", .data = &sun4i_cpu_mux_data,},
> -	{.compatible = "allwinner,sun6i-a31-ahb1-mux-clk", .data = &sun6i_a31_ahb1_mux_data,},
> -	{.compatible = "allwinner,sun8i-h3-ahb2-clk", .data = &sun8i_h3_ahb2_mux_data,},
>  	{}
>  };
>  
> 

^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH 3/5] clk: sunxi: convert current clocks registration to CLK_OF_DECLARE
@ 2016-02-02 14:16     ` Andre Przywara
  0 siblings, 0 replies; 40+ messages in thread
From: Andre Przywara @ 2016-02-02 14:16 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Maxime,

On 02/02/16 08:47, Maxime Ripard wrote:
> The current clock registration and protection code has a few drawbacks, the
> two main ones being that we create a lot of orphans clock in the
> registration phase, which will be troublesome when we will start being less
> relaxed about them.
> 
> The protection code also relies on clkdev, which we don't really use but
> for this particular case.
> 
> Fix both at the same time by moving everyone to the CLK_OF_DECLARE that
> will probe our clock tree in the right and thus avoid orphans, and by
> protecting directly the clock returned by our registration function.

I very much appreciate this cleanup and like the idea. Any chance we can
have this rather quickly, so that I can rebase the A64 support series on it?

One issue below, though:

...

> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  drivers/clk/sunxi/clk-sunxi.c | 150 +++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 133 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
> index a3d706f5b21c..da6c1bc763d1 100644
> --- a/drivers/clk/sunxi/clk-sunxi.c
> +++ b/drivers/clk/sunxi/clk-sunxi.c
> @@ -585,6 +585,41 @@ static struct clk * __init sunxi_factors_clk_setup(struct device_node *node,
>  	return sunxi_factors_register(node, data, &clk_lock, reg);
>  }
>  
> +static void __init sun4i_pll1_clk_setup(struct device_node *node)
> +{
> +	sunxi_factors_clk_setup(node, &sun4i_pll1_data);
> +}
> +CLK_OF_DECLARE(sun4i_pll1, "allwinner,sun4i-a10-pll1-clk",
> +	       sun4i_pll1_clk_setup);
> +
> +static void __init sun6i_pll1_clk_setup(struct device_node *node)
> +{
> +	sunxi_factors_clk_setup(node, &sun6i_a31_pll1_data);
> +}
> +CLK_OF_DECLARE(sun6i_pll1, "allwinner,sun6i-a31-pll1-clk",
> +	       sun6i_pll1_clk_setup);
> +
> +static void __init sun8i_pll1_clk_setup(struct device_node *node)
> +{
> +	sunxi_factors_clk_setup(node, &sun8i_a23_pll1_data);
> +}
> +CLK_OF_DECLARE(sun8i_pll1, "allwinner,sun8i-a23-pll1-clk",
> +	       sun8i_pll1_clk_setup);
> +
> +static void __init sun7i_pll4_clk_setup(struct device_node *node)
> +{
> +	sunxi_factors_clk_setup(node, &sun7i_a20_pll4_data);
> +}
> +CLK_OF_DECLARE(sun7i_pll4, "allwinner,sun7i-a20-pll4-clk",
> +	       sun7i_pll4_clk_setup);
> +
> +static void __init sun5i_ahb_clk_setup(struct device_node *node)
> +{
> +	sunxi_factors_clk_setup(node, &sun5i_a13_ahb_data);
> +}
> +CLK_OF_DECLARE(sun5i_ahb, "allwinner,sun5i-a13-ahb-clk",
> +	       sun5i_ahb_clk_setup);
> +
>  static void __init sun6i_ahb1_clk_setup(struct device_node *node)
>  {
>  	sunxi_factors_clk_setup(node, &sun6i_ahb1_data);
> @@ -592,6 +627,20 @@ static void __init sun6i_ahb1_clk_setup(struct device_node *node)
>  CLK_OF_DECLARE(sun6i_a31_ahb1, "allwinner,sun6i-a31-ahb1-clk",
>  	       sun6i_ahb1_clk_setup);
>  
> +static void __init sun4i_apb1_clk_setup(struct device_node *node)
> +{
> +	sunxi_factors_clk_setup(node, &sun4i_apb1_data);
> +}
> +CLK_OF_DECLARE(sun4i_apb1, "allwinner,sun4i-a10-apb1-clk",
> +	       sun4i_apb1_clk_setup);
> +
> +static void __init sun7i_out_clk_setup(struct device_node *node)
> +{
> +	sunxi_factors_clk_setup(node, &sun7i_a20_out_data);
> +}
> +CLK_OF_DECLARE(sun7i_out, "allwinner,sun7i-a20-out-clk",
> +	       sun7i_out_clk_setup);
> +
>  
>  /**
>   * sunxi_mux_clk_setup() - Setup function for muxes
> @@ -642,6 +691,34 @@ static struct clk * __init sunxi_mux_clk_setup(struct device_node *node,
>  	return clk;
>  }
>  
> +static void __init sun4i_cpu_clk_setup(struct device_node *node)
> +{
> +	struct clk *clk;
> +
> +	clk = sunxi_mux_clk_setup(node, &sun4i_cpu_mux_data);
> +	if (!clk)
> +		return;
> +
> +	/* Protect CPU clock */
> +	__clk_get(clk);
> +	clk_prepare_enable(clk);
> +}
> +CLK_OF_DECLARE(sun4i_cpu, "allwinner,sun4i-a10-cpu-clk",
> +	       sun4i_cpu_clk_setup);
> +
> +static void __init sun6i_ahb1_mux_clk_setup(struct device_node *node)
> +{
> +	sunxi_mux_clk_setup(node, &sun6i_a31_ahb1_mux_data);
> +}
> +CLK_OF_DECLARE(sun6i_ahb1_mux, "allwinner,sun6i-a31-ahb1-mux-clk",
> +	       sun6i_ahb1_mux_clk_setup);
> +
> +static void __init sun8i_ahb2_clk_setup(struct device_node *node)
> +{
> +	sunxi_mux_clk_setup(node, &sun8i_h3_ahb2_mux_data);
> +}
> +CLK_OF_DECLARE(sun8i_ahb2, "allwinner,sun8i-a31-ahb2-clk",
> +	       sun8i_ahb2_clk_setup);

I don't find this clock in my tree (which is mripard/sunxi/for-next).
Instead I only have "allwinner,sun8i-h3-ahb2-clk", as mentioned below.
But as you remove this clock below from the old code and instead
instantiate this new clock here, this looks somehow wrong to me. Can you
confirm this or am I utterly confused?

Apart from that I checked each and every clock mentioned in this patch
and can confirm that the transformation is correct. So if you fix this,
I can send a Reviewed-by.

Cheers,
Andre.

>  
>  
>  /**
> @@ -723,6 +800,34 @@ static void __init sunxi_divider_clk_setup(struct device_node *node,
>  	}
>  }
>  
> +static void __init sun4i_ahb_clk_setup(struct device_node *node)
> +{
> +	sunxi_divider_clk_setup(node, &sun4i_ahb_data);
> +}
> +CLK_OF_DECLARE(sun4i_ahb, "allwinner,sun4i-a10-ahb-clk",
> +	       sun4i_ahb_clk_setup);
> +
> +static void __init sun4i_apb0_clk_setup(struct device_node *node)
> +{
> +	sunxi_divider_clk_setup(node, &sun4i_apb0_data);
> +}
> +CLK_OF_DECLARE(sun4i_apb0, "allwinner,sun4i-a10-apb0-clk",
> +	       sun4i_apb0_clk_setup);
> +
> +static void __init sun4i_axi_clk_setup(struct device_node *node)
> +{
> +	sunxi_divider_clk_setup(node, &sun4i_axi_data);
> +}
> +CLK_OF_DECLARE(sun4i_axi, "allwinner,sun4i-a10-axi-clk",
> +	       sun4i_axi_clk_setup);
> +
> +static void __init sun8i_axi_clk_setup(struct device_node *node)
> +{
> +	sunxi_divider_clk_setup(node, &sun8i_a23_axi_data);
> +}
> +CLK_OF_DECLARE(sun8i_axi, "allwinner,sun8i-a23-axi-clk",
> +	       sun8i_axi_clk_setup);
> +
>  
>  /**
>   * sunxi_gates_clk_setup() - Setup function for leaf gates on clocks
> @@ -934,42 +1039,53 @@ free_clkdata:
>  	return NULL;
>  }
>  
> +static void __init sun4i_pll5_clk_setup(struct device_node *node)
> +{
> +	struct clk **clks;
> +
> +	clks = sunxi_divs_clk_setup(node, &pll5_divs_data);
> +	if (!clks)
> +		return;
> +
> +	/* Protect PLL5_DDR */
> +	__clk_get(clks[0]);
> +	clk_prepare_enable(clks[0]);
> +}
> +CLK_OF_DECLARE(sun4i_pll5, "allwinner,sun4i-a10-pll5-clk",
> +	       sun4i_pll5_clk_setup);
> +
> +static void __init sun4i_pll6_clk_setup(struct device_node *node)
> +{
> +	sunxi_divs_clk_setup(node, &pll6_divs_data);
> +}
> +CLK_OF_DECLARE(sun4i_pll6, "allwinner,sun4i-a10-pll6-clk",
> +	       sun4i_pll6_clk_setup);
> +
> +static void __init sun6i_pll6_clk_setup(struct device_node *node)
> +{
> +	sunxi_divs_clk_setup(node, &sun6i_a31_pll6_divs_data);
> +}
> +CLK_OF_DECLARE(sun6i_pll6, "allwinner,sun6i-a31-pll6-clk",
> +	       sun6i_pll6_clk_setup);
>  
>  
>  /* Matches for factors clocks */
>  static const struct of_device_id clk_factors_match[] __initconst = {
> -	{.compatible = "allwinner,sun4i-a10-pll1-clk", .data = &sun4i_pll1_data,},
> -	{.compatible = "allwinner,sun6i-a31-pll1-clk", .data = &sun6i_a31_pll1_data,},
> -	{.compatible = "allwinner,sun8i-a23-pll1-clk", .data = &sun8i_a23_pll1_data,},
> -	{.compatible = "allwinner,sun7i-a20-pll4-clk", .data = &sun7i_a20_pll4_data,},
> -	{.compatible = "allwinner,sun5i-a13-ahb-clk", .data = &sun5i_a13_ahb_data,},
> -	{.compatible = "allwinner,sun4i-a10-apb1-clk", .data = &sun4i_apb1_data,},
> -	{.compatible = "allwinner,sun7i-a20-out-clk", .data = &sun7i_a20_out_data,},
>  	{}
>  };
>  
>  /* Matches for divider clocks */
>  static const struct of_device_id clk_div_match[] __initconst = {
> -	{.compatible = "allwinner,sun4i-a10-axi-clk", .data = &sun4i_axi_data,},
> -	{.compatible = "allwinner,sun8i-a23-axi-clk", .data = &sun8i_a23_axi_data,},
> -	{.compatible = "allwinner,sun4i-a10-ahb-clk", .data = &sun4i_ahb_data,},
> -	{.compatible = "allwinner,sun4i-a10-apb0-clk", .data = &sun4i_apb0_data,},
>  	{}
>  };
>  
>  /* Matches for divided outputs */
>  static const struct of_device_id clk_divs_match[] __initconst = {
> -	{.compatible = "allwinner,sun4i-a10-pll5-clk", .data = &pll5_divs_data,},
> -	{.compatible = "allwinner,sun4i-a10-pll6-clk", .data = &pll6_divs_data,},
> -	{.compatible = "allwinner,sun6i-a31-pll6-clk", .data = &sun6i_a31_pll6_divs_data,},
>  	{}
>  };
>  
>  /* Matches for mux clocks */
>  static const struct of_device_id clk_mux_match[] __initconst = {
> -	{.compatible = "allwinner,sun4i-a10-cpu-clk", .data = &sun4i_cpu_mux_data,},
> -	{.compatible = "allwinner,sun6i-a31-ahb1-mux-clk", .data = &sun6i_a31_ahb1_mux_data,},
> -	{.compatible = "allwinner,sun8i-h3-ahb2-clk", .data = &sun8i_h3_ahb2_mux_data,},
>  	{}
>  };
>  
> 

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 3/5] clk: sunxi: convert current clocks registration to CLK_OF_DECLARE
  2016-02-02 14:16     ` Andre Przywara
@ 2016-02-02 17:00       ` Maxime Ripard
  -1 siblings, 0 replies; 40+ messages in thread
From: Maxime Ripard @ 2016-02-02 17:00 UTC (permalink / raw)
  To: Andre Przywara
  Cc: Mike Turquette, Stephen Boyd, Emilio Lopez, heiko, Chen-Yu Tsai,
	linux-clk, linux-arm-kernel

[-- Attachment #1: Type: text/plain, Size: 2005 bytes --]

Hi,

On Tue, Feb 02, 2016 at 02:16:54PM +0000, Andre Przywara wrote:
> Hi Maxime,
> 
> On 02/02/16 08:47, Maxime Ripard wrote:
> > The current clock registration and protection code has a few drawbacks, the
> > two main ones being that we create a lot of orphans clock in the
> > registration phase, which will be troublesome when we will start being less
> > relaxed about them.
> > 
> > The protection code also relies on clkdev, which we don't really use but
> > for this particular case.
> > 
> > Fix both at the same time by moving everyone to the CLK_OF_DECLARE that
> > will probe our clock tree in the right and thus avoid orphans, and by
> > protecting directly the clock returned by our registration function.
> 
> I very much appreciate this cleanup and like the idea. Any chance we can
> have this rather quickly, so that I can rebase the A64 support series on it?

I actually count on that :)

I wasn't really happy about your allwinner,sunxi compatible, so I just
gave you an easier way out ;)

> > +static void __init sun8i_ahb2_clk_setup(struct device_node *node)
> > +{
> > +	sunxi_mux_clk_setup(node, &sun8i_h3_ahb2_mux_data);
> > +}
> > +CLK_OF_DECLARE(sun8i_ahb2, "allwinner,sun8i-a31-ahb2-clk",
> > +	       sun8i_ahb2_clk_setup);
> 
> I don't find this clock in my tree (which is mripard/sunxi/for-next).
> Instead I only have "allwinner,sun8i-h3-ahb2-clk", as mentioned below.
> But as you remove this clock below from the old code and instead
> instantiate this new clock here, this looks somehow wrong to me. Can you
> confirm this or am I utterly confused?

Damn, you're right, it's just a silly copy-paste issue, I'll fix it.

> Apart from that I checked each and every clock mentioned in this patch
> and can confirm that the transformation is correct. So if you fix this,
> I can send a Reviewed-by.

Thanks,
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH 3/5] clk: sunxi: convert current clocks registration to CLK_OF_DECLARE
@ 2016-02-02 17:00       ` Maxime Ripard
  0 siblings, 0 replies; 40+ messages in thread
From: Maxime Ripard @ 2016-02-02 17:00 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On Tue, Feb 02, 2016 at 02:16:54PM +0000, Andre Przywara wrote:
> Hi Maxime,
> 
> On 02/02/16 08:47, Maxime Ripard wrote:
> > The current clock registration and protection code has a few drawbacks, the
> > two main ones being that we create a lot of orphans clock in the
> > registration phase, which will be troublesome when we will start being less
> > relaxed about them.
> > 
> > The protection code also relies on clkdev, which we don't really use but
> > for this particular case.
> > 
> > Fix both at the same time by moving everyone to the CLK_OF_DECLARE that
> > will probe our clock tree in the right and thus avoid orphans, and by
> > protecting directly the clock returned by our registration function.
> 
> I very much appreciate this cleanup and like the idea. Any chance we can
> have this rather quickly, so that I can rebase the A64 support series on it?

I actually count on that :)

I wasn't really happy about your allwinner,sunxi compatible, so I just
gave you an easier way out ;)

> > +static void __init sun8i_ahb2_clk_setup(struct device_node *node)
> > +{
> > +	sunxi_mux_clk_setup(node, &sun8i_h3_ahb2_mux_data);
> > +}
> > +CLK_OF_DECLARE(sun8i_ahb2, "allwinner,sun8i-a31-ahb2-clk",
> > +	       sun8i_ahb2_clk_setup);
> 
> I don't find this clock in my tree (which is mripard/sunxi/for-next).
> Instead I only have "allwinner,sun8i-h3-ahb2-clk", as mentioned below.
> But as you remove this clock below from the old code and instead
> instantiate this new clock here, this looks somehow wrong to me. Can you
> confirm this or am I utterly confused?

Damn, you're right, it's just a silly copy-paste issue, I'll fix it.

> Apart from that I checked each and every clock mentioned in this patch
> and can confirm that the transformation is correct. So if you fix this,
> I can send a Reviewed-by.

Thanks,
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160202/b628781e/attachment.sig>

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 3/5] clk: sunxi: convert current clocks registration to CLK_OF_DECLARE
  2016-02-02 17:00       ` Maxime Ripard
@ 2016-02-02 17:04         ` Andre Przywara
  -1 siblings, 0 replies; 40+ messages in thread
From: Andre Przywara @ 2016-02-02 17:04 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Mike Turquette, Stephen Boyd, Emilio Lopez, heiko, Chen-Yu Tsai,
	linux-clk, linux-arm-kernel

Hi,

On 02/02/16 17:00, Maxime Ripard wrote:
> Hi,
> 
> On Tue, Feb 02, 2016 at 02:16:54PM +0000, Andre Przywara wrote:
>> Hi Maxime,
>>
>> On 02/02/16 08:47, Maxime Ripard wrote:
>>> The current clock registration and protection code has a few drawbacks, the
>>> two main ones being that we create a lot of orphans clock in the
>>> registration phase, which will be troublesome when we will start being less
>>> relaxed about them.
>>>
>>> The protection code also relies on clkdev, which we don't really use but
>>> for this particular case.
>>>
>>> Fix both at the same time by moving everyone to the CLK_OF_DECLARE that
>>> will probe our clock tree in the right and thus avoid orphans, and by
>>> protecting directly the clock returned by our registration function.
>>
>> I very much appreciate this cleanup and like the idea. Any chance we can
>> have this rather quickly, so that I can rebase the A64 support series on it?
> 
> I actually count on that :)

Great!

> I wasn't really happy about your allwinner,sunxi compatible,

Me neither, honestly, but I didn't dare to touch clk-sunxi.c even more ;-)

> so I just
> gave you an easier way out ;)

Appreciated!

> 
>>> +static void __init sun8i_ahb2_clk_setup(struct device_node *node)
>>> +{
>>> +	sunxi_mux_clk_setup(node, &sun8i_h3_ahb2_mux_data);
>>> +}
>>> +CLK_OF_DECLARE(sun8i_ahb2, "allwinner,sun8i-a31-ahb2-clk",
>>> +	       sun8i_ahb2_clk_setup);
>>
>> I don't find this clock in my tree (which is mripard/sunxi/for-next).
>> Instead I only have "allwinner,sun8i-h3-ahb2-clk", as mentioned below.
>> But as you remove this clock below from the old code and instead
>> instantiate this new clock here, this looks somehow wrong to me. Can you
>> confirm this or am I utterly confused?
> 
> Damn, you're right, it's just a silly copy-paste issue, I'll fix it.

I am sure you left it in to see if somebody actually checks it ;-)

Merci!
Andre

>> Apart from that I checked each and every clock mentioned in this patch
>> and can confirm that the transformation is correct. So if you fix this,
>> I can send a Reviewed-by.
> 
> Thanks,
> Maxime
> 

^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH 3/5] clk: sunxi: convert current clocks registration to CLK_OF_DECLARE
@ 2016-02-02 17:04         ` Andre Przywara
  0 siblings, 0 replies; 40+ messages in thread
From: Andre Przywara @ 2016-02-02 17:04 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On 02/02/16 17:00, Maxime Ripard wrote:
> Hi,
> 
> On Tue, Feb 02, 2016 at 02:16:54PM +0000, Andre Przywara wrote:
>> Hi Maxime,
>>
>> On 02/02/16 08:47, Maxime Ripard wrote:
>>> The current clock registration and protection code has a few drawbacks, the
>>> two main ones being that we create a lot of orphans clock in the
>>> registration phase, which will be troublesome when we will start being less
>>> relaxed about them.
>>>
>>> The protection code also relies on clkdev, which we don't really use but
>>> for this particular case.
>>>
>>> Fix both at the same time by moving everyone to the CLK_OF_DECLARE that
>>> will probe our clock tree in the right and thus avoid orphans, and by
>>> protecting directly the clock returned by our registration function.
>>
>> I very much appreciate this cleanup and like the idea. Any chance we can
>> have this rather quickly, so that I can rebase the A64 support series on it?
> 
> I actually count on that :)

Great!

> I wasn't really happy about your allwinner,sunxi compatible,

Me neither, honestly, but I didn't dare to touch clk-sunxi.c even more ;-)

> so I just
> gave you an easier way out ;)

Appreciated!

> 
>>> +static void __init sun8i_ahb2_clk_setup(struct device_node *node)
>>> +{
>>> +	sunxi_mux_clk_setup(node, &sun8i_h3_ahb2_mux_data);
>>> +}
>>> +CLK_OF_DECLARE(sun8i_ahb2, "allwinner,sun8i-a31-ahb2-clk",
>>> +	       sun8i_ahb2_clk_setup);
>>
>> I don't find this clock in my tree (which is mripard/sunxi/for-next).
>> Instead I only have "allwinner,sun8i-h3-ahb2-clk", as mentioned below.
>> But as you remove this clock below from the old code and instead
>> instantiate this new clock here, this looks somehow wrong to me. Can you
>> confirm this or am I utterly confused?
> 
> Damn, you're right, it's just a silly copy-paste issue, I'll fix it.

I am sure you left it in to see if somebody actually checks it ;-)

Merci!
Andre

>> Apart from that I checked each and every clock mentioned in this patch
>> and can confirm that the transformation is correct. So if you fix this,
>> I can send a Reviewed-by.
> 
> Thanks,
> Maxime
> 

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 5/5] clk: sunxi: Remove clk_register_clkdev calls
  2016-02-02 10:26     ` Jean-Francois Moine
@ 2016-02-03 19:29       ` Maxime Ripard
  -1 siblings, 0 replies; 40+ messages in thread
From: Maxime Ripard @ 2016-02-03 19:29 UTC (permalink / raw)
  To: Jean-Francois Moine
  Cc: Mike Turquette, Stephen Boyd, Emilio Lopez, heiko, Chen-Yu Tsai,
	Andre Przywara, linux-clk, linux-arm-kernel

[-- Attachment #1: Type: text/plain, Size: 768 bytes --]

On Tue, Feb 02, 2016 at 11:26:04AM +0100, Jean-Francois Moine wrote:
> On Tue,  2 Feb 2016 09:47:14 +0100
> Maxime Ripard <maxime.ripard@free-electrons.com> wrote:
> 
> > diff --git a/drivers/clk/sunxi/clk-mod0.c b/drivers/clk/sunxi/clk-mod0.c
> > index 578bff0dd251..b38d71cec74c 100644
> > --- a/drivers/clk/sunxi/clk-mod0.c
> > +++ b/drivers/clk/sunxi/clk-mod0.c
> > @@ -15,6 +15,7 @@
> >   */
> >  
> >  #include <linux/clk.h>
> > +#include <linux/clkdev.h>
> 
> Why?

Because this driver uses __clk_get, that it was previously included in
clk-factors.h and is not anymore, and that otherwise it won't compile,
obviously.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH 5/5] clk: sunxi: Remove clk_register_clkdev calls
@ 2016-02-03 19:29       ` Maxime Ripard
  0 siblings, 0 replies; 40+ messages in thread
From: Maxime Ripard @ 2016-02-03 19:29 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Feb 02, 2016 at 11:26:04AM +0100, Jean-Francois Moine wrote:
> On Tue,  2 Feb 2016 09:47:14 +0100
> Maxime Ripard <maxime.ripard@free-electrons.com> wrote:
> 
> > diff --git a/drivers/clk/sunxi/clk-mod0.c b/drivers/clk/sunxi/clk-mod0.c
> > index 578bff0dd251..b38d71cec74c 100644
> > --- a/drivers/clk/sunxi/clk-mod0.c
> > +++ b/drivers/clk/sunxi/clk-mod0.c
> > @@ -15,6 +15,7 @@
> >   */
> >  
> >  #include <linux/clk.h>
> > +#include <linux/clkdev.h>
> 
> Why?

Because this driver uses __clk_get, that it was previously included in
clk-factors.h and is not anymore, and that otherwise it won't compile,
obviously.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160203/bfdb411e/attachment.sig>

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 1/5] clk: sunxi: Make clocks setup functions return their clock
  2016-02-02  8:47   ` Maxime Ripard
@ 2016-02-04 12:07     ` Maxime Ripard
  -1 siblings, 0 replies; 40+ messages in thread
From: Maxime Ripard @ 2016-02-04 12:07 UTC (permalink / raw)
  To: Mike Turquette, Stephen Boyd, Emilio Lopez, heiko, Chen-Yu Tsai
  Cc: linux-clk, linux-arm-kernel, Andre Przywara

[-- Attachment #1: Type: text/plain, Size: 658 bytes --]

On Tue, Feb 02, 2016 at 09:47:10AM +0100, Maxime Ripard wrote:
> The clocks registration code in clk-sunxi was most of the time not
> returning the struct clk (or struct clk array) that was registered,
> preventing the users of such functions to manipulate it, for example to
> protect it.
> 
> Make them return it so that we can start using it.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>

After discussing it with Chen-Yu on IRC, the error logging will be
done as a subsequent patch.

Applied.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH 1/5] clk: sunxi: Make clocks setup functions return their clock
@ 2016-02-04 12:07     ` Maxime Ripard
  0 siblings, 0 replies; 40+ messages in thread
From: Maxime Ripard @ 2016-02-04 12:07 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Feb 02, 2016 at 09:47:10AM +0100, Maxime Ripard wrote:
> The clocks registration code in clk-sunxi was most of the time not
> returning the struct clk (or struct clk array) that was registered,
> preventing the users of such functions to manipulate it, for example to
> protect it.
> 
> Make them return it so that we can start using it.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>

After discussing it with Chen-Yu on IRC, the error logging will be
done as a subsequent patch.

Applied.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160204/72a9f1ac/attachment.sig>

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 2/5] clk: sunxi: Make clocks setup functions take const pointer
  2016-02-02  8:47   ` Maxime Ripard
@ 2016-02-04 12:08     ` Maxime Ripard
  -1 siblings, 0 replies; 40+ messages in thread
From: Maxime Ripard @ 2016-02-04 12:08 UTC (permalink / raw)
  To: Mike Turquette, Stephen Boyd, Emilio Lopez, heiko, Chen-Yu Tsai
  Cc: linux-clk, linux-arm-kernel, Andre Przywara

[-- Attachment #1: Type: text/plain, Size: 602 bytes --]

On Tue, Feb 02, 2016 at 09:47:11AM +0100, Maxime Ripard wrote:
> All the data structure that we pass to the clocks setup functions are
> declared const, while our setup functions expects a regular pointer. This
> was hidden by the fact that we cast a void * pointer back to these
> structures, which made it go unnoticed.
> 
> Fix the functions prototype.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Applied with Andre and Chen-Yu Acked-by's

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH 2/5] clk: sunxi: Make clocks setup functions take const pointer
@ 2016-02-04 12:08     ` Maxime Ripard
  0 siblings, 0 replies; 40+ messages in thread
From: Maxime Ripard @ 2016-02-04 12:08 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Feb 02, 2016 at 09:47:11AM +0100, Maxime Ripard wrote:
> All the data structure that we pass to the clocks setup functions are
> declared const, while our setup functions expects a regular pointer. This
> was hidden by the fact that we cast a void * pointer back to these
> structures, which made it go unnoticed.
> 
> Fix the functions prototype.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Applied with Andre and Chen-Yu Acked-by's

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160204/b27f5495/attachment-0001.sig>

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 3/5] clk: sunxi: convert current clocks registration to CLK_OF_DECLARE
  2016-02-02  8:47   ` Maxime Ripard
@ 2016-02-04 12:13     ` Maxime Ripard
  -1 siblings, 0 replies; 40+ messages in thread
From: Maxime Ripard @ 2016-02-04 12:13 UTC (permalink / raw)
  To: Mike Turquette, Stephen Boyd, Emilio Lopez, heiko, Chen-Yu Tsai
  Cc: linux-clk, linux-arm-kernel, Andre Przywara

[-- Attachment #1: Type: text/plain, Size: 891 bytes --]

On Tue, Feb 02, 2016 at 09:47:12AM +0100, Maxime Ripard wrote:
> The current clock registration and protection code has a few drawbacks, the
> two main ones being that we create a lot of orphans clock in the
> registration phase, which will be troublesome when we will start being less
> relaxed about them.
> 
> The protection code also relies on clkdev, which we don't really use but
> for this particular case.
> 
> Fix both at the same time by moving everyone to the CLK_OF_DECLARE that
> will probe our clock tree in the right and thus avoid orphans, and by
> protecting directly the clock returned by our registration function.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Fixed the issue reported by Andre, and applied.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH 3/5] clk: sunxi: convert current clocks registration to CLK_OF_DECLARE
@ 2016-02-04 12:13     ` Maxime Ripard
  0 siblings, 0 replies; 40+ messages in thread
From: Maxime Ripard @ 2016-02-04 12:13 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Feb 02, 2016 at 09:47:12AM +0100, Maxime Ripard wrote:
> The current clock registration and protection code has a few drawbacks, the
> two main ones being that we create a lot of orphans clock in the
> registration phase, which will be troublesome when we will start being less
> relaxed about them.
> 
> The protection code also relies on clkdev, which we don't really use but
> for this particular case.
> 
> Fix both at the same time by moving everyone to the CLK_OF_DECLARE that
> will probe our clock tree in the right and thus avoid orphans, and by
> protecting directly the clock returned by our registration function.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Fixed the issue reported by Andre, and applied.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160204/d458471c/attachment.sig>

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 4/5] clk: sunxi: Remove old probe and protection code
  2016-02-02  8:47   ` Maxime Ripard
@ 2016-02-04 12:15     ` Maxime Ripard
  -1 siblings, 0 replies; 40+ messages in thread
From: Maxime Ripard @ 2016-02-04 12:15 UTC (permalink / raw)
  To: Mike Turquette, Stephen Boyd, Emilio Lopez, heiko, Chen-Yu Tsai
  Cc: linux-clk, linux-arm-kernel, Andre Przywara

[-- Attachment #1: Type: text/plain, Size: 362 bytes --]

On Tue, Feb 02, 2016 at 09:47:13AM +0100, Maxime Ripard wrote:
> Now that we don't have any user left for the old registration code, we can
> remove it.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Applied.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH 4/5] clk: sunxi: Remove old probe and protection code
@ 2016-02-04 12:15     ` Maxime Ripard
  0 siblings, 0 replies; 40+ messages in thread
From: Maxime Ripard @ 2016-02-04 12:15 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Feb 02, 2016 at 09:47:13AM +0100, Maxime Ripard wrote:
> Now that we don't have any user left for the old registration code, we can
> remove it.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Applied.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160204/dbb69dad/attachment.sig>

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 5/5] clk: sunxi: Remove clk_register_clkdev calls
  2016-02-02  8:47   ` Maxime Ripard
@ 2016-02-04 12:17     ` Maxime Ripard
  -1 siblings, 0 replies; 40+ messages in thread
From: Maxime Ripard @ 2016-02-04 12:17 UTC (permalink / raw)
  To: Mike Turquette, Stephen Boyd, Emilio Lopez, heiko, Chen-Yu Tsai
  Cc: linux-clk, linux-arm-kernel, Andre Przywara

[-- Attachment #1: Type: text/plain, Size: 389 bytes --]

On Tue, Feb 02, 2016 at 09:47:14AM +0100, Maxime Ripard wrote:
> Now that our protection code doesn't use the global name lookup anymore, we
> can remove the clkdev registrations.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Applied.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH 5/5] clk: sunxi: Remove clk_register_clkdev calls
@ 2016-02-04 12:17     ` Maxime Ripard
  0 siblings, 0 replies; 40+ messages in thread
From: Maxime Ripard @ 2016-02-04 12:17 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Feb 02, 2016 at 09:47:14AM +0100, Maxime Ripard wrote:
> Now that our protection code doesn't use the global name lookup anymore, we
> can remove the clkdev registrations.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Applied.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160204/0ddf838d/attachment.sig>

^ permalink raw reply	[flat|nested] 40+ messages in thread

end of thread, other threads:[~2016-02-04 12:17 UTC | newest]

Thread overview: 40+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-02  8:47 [PATCH 0/5] clk: sunxi: Rework the clocks code to deal with orphans Maxime Ripard
2016-02-02  8:47 ` Maxime Ripard
2016-02-02  8:47 ` [PATCH 1/5] clk: sunxi: Make clocks setup functions return their clock Maxime Ripard
2016-02-02  8:47   ` Maxime Ripard
2016-02-02 10:35   ` Chen-Yu Tsai
2016-02-02 11:32     ` Andre Przywara
2016-02-02 11:32       ` Andre Przywara
2016-02-02 13:27     ` Maxime Ripard
2016-02-02 13:27       ` Maxime Ripard
2016-02-04 12:07   ` Maxime Ripard
2016-02-04 12:07     ` Maxime Ripard
2016-02-02  8:47 ` [PATCH 2/5] clk: sunxi: Make clocks setup functions take const pointer Maxime Ripard
2016-02-02  8:47   ` Maxime Ripard
2016-02-02 10:38   ` Chen-Yu Tsai
2016-02-02 12:11   ` Andre Przywara
2016-02-02 12:11     ` Andre Przywara
2016-02-04 12:08   ` Maxime Ripard
2016-02-04 12:08     ` Maxime Ripard
2016-02-02  8:47 ` [PATCH 3/5] clk: sunxi: convert current clocks registration to CLK_OF_DECLARE Maxime Ripard
2016-02-02  8:47   ` Maxime Ripard
2016-02-02 14:16   ` Andre Przywara
2016-02-02 14:16     ` Andre Przywara
2016-02-02 17:00     ` Maxime Ripard
2016-02-02 17:00       ` Maxime Ripard
2016-02-02 17:04       ` Andre Przywara
2016-02-02 17:04         ` Andre Przywara
2016-02-04 12:13   ` Maxime Ripard
2016-02-04 12:13     ` Maxime Ripard
2016-02-02  8:47 ` [PATCH 4/5] clk: sunxi: Remove old probe and protection code Maxime Ripard
2016-02-02  8:47   ` Maxime Ripard
2016-02-04 12:15   ` Maxime Ripard
2016-02-04 12:15     ` Maxime Ripard
2016-02-02  8:47 ` [PATCH 5/5] clk: sunxi: Remove clk_register_clkdev calls Maxime Ripard
2016-02-02  8:47   ` Maxime Ripard
2016-02-02 10:26   ` Jean-Francois Moine
2016-02-02 10:26     ` Jean-Francois Moine
2016-02-03 19:29     ` Maxime Ripard
2016-02-03 19:29       ` Maxime Ripard
2016-02-04 12:17   ` Maxime Ripard
2016-02-04 12:17     ` Maxime Ripard

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.