[PATCH 0/6] sunxi: clk: Various cleanup and rework

[PATCH 0/6] sunxi: clk: Various cleanup and rework

[Maxime Ripard]

Hi everyone,

This patchset fixes a few things that have been pending for quite a
while in the clock driver.

First, it removes the clk_put calls in the clock protection
part. Since it's not really something that should be done, I guess
this patch is not very controversial.

Then, it starts splitting the huge clock driver file into separate,
smaller drivers when it makes sense.

Finally, it reworks the clock protection mechanism to handle
differences between SoC in a better way. This has been pretty
controversial because the first approach has been to move this to the
machine code. This is another attempt that leaves all the
modifications in the driver itself.

Thanks,
Maxime

Maxime Ripard (6):
  clk: sunxi: Remove calls to clk_put
  clk: sunxi: Move the 24M oscillator to a file of its own
  clk: sunxi: Move the GMAC clock to a file of its own
  clk: sunxi: Rework clock protection code
  clk: sun6i: Protect CPU clock
  clk: sun6i: Protect SDRAM gating bit

 drivers/clk/sunxi/Makefile       |   2 +
 drivers/clk/sunxi/clk-a10-hosc.c |  73 ++++++++++++
 drivers/clk/sunxi/clk-a20-gmac.c | 119 ++++++++++++++++++++
 drivers/clk/sunxi/clk-sunxi.c    | 233 ++++++++-------------------------------
 4 files changed, 240 insertions(+), 187 deletions(-)
 create mode 100644 drivers/clk/sunxi/clk-a10-hosc.c
 create mode 100644 drivers/clk/sunxi/clk-a20-gmac.c

-- 
1.9.1

[PATCH 1/6] clk: sunxi: Remove calls to clk_put

[Maxime Ripard]

Callers of clk_put must disable the clock first. This also means that as long
as the clock is enabled the driver should hold a reference to that clock.
Hence, the call to clk_put here are bogus and should be removed.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
Acked-by: Mike Turquette <mturquette@linaro.org>
---
 drivers/clk/sunxi/clk-sunxi.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
index bd7dc733c1ca..b1fde0f89bc1 100644
--- a/drivers/clk/sunxi/clk-sunxi.c
+++ b/drivers/clk/sunxi/clk-sunxi.c
@@ -1297,17 +1297,13 @@ static void __init sunxi_clock_protect(void)
 
 	/* memory bus clock - sun5i+ */
 	clk = clk_get(NULL, "mbus");
-	if (!IS_ERR(clk)) {
+	if (!IS_ERR(clk))
 		clk_prepare_enable(clk);
-		clk_put(clk);
-	}
 
 	/* DDR clock - sun4i+ */
 	clk = clk_get(NULL, "pll5_ddr");
-	if (!IS_ERR(clk)) {
+	if (!IS_ERR(clk))
 		clk_prepare_enable(clk);
-		clk_put(clk);
-	}
 }
 
 static void __init sunxi_init_clocks(void)
-- 
1.9.1

[PATCH 2/6] clk: sunxi: Move the 24M oscillator to a file of its own

[Maxime Ripard]

Since we have a folder of our own, we can actually make use of it by splitting
the huge clock file into several sub drivers.

The main oscillator is pretty easy to deal with, since it's pretty much
isolated.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/clk/sunxi/Makefile       |  1 +
 drivers/clk/sunxi/clk-a10-hosc.c | 73 ++++++++++++++++++++++++++++++++++++++++
 drivers/clk/sunxi/clk-sunxi.c    | 57 -------------------------------
 3 files changed, 74 insertions(+), 57 deletions(-)
 create mode 100644 drivers/clk/sunxi/clk-a10-hosc.c

diff --git a/drivers/clk/sunxi/Makefile b/drivers/clk/sunxi/Makefile
index b5bac917612c..2c3d2fed2560 100644
--- a/drivers/clk/sunxi/Makefile
+++ b/drivers/clk/sunxi/Makefile
@@ -3,3 +3,4 @@
 #
 
 obj-y += clk-sunxi.o clk-factors.o
+obj-y += clk-a10-hosc.o
\ No newline at end of file
diff --git a/drivers/clk/sunxi/clk-a10-hosc.c b/drivers/clk/sunxi/clk-a10-hosc.c
new file mode 100644
index 000000000000..0481d5d673d6
--- /dev/null
+++ b/drivers/clk/sunxi/clk-a10-hosc.c
@@ -0,0 +1,73 @@
+/*
+ * Copyright 2013 Emilio L??pez
+ *
+ * Emilio L??pez <emilio@elopez.com.ar>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/clkdev.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+
+#define SUNXI_OSC24M_GATE	0
+
+static DEFINE_SPINLOCK(hosc_lock);
+
+static void __init sun4i_osc_clk_setup(struct device_node *node)
+{
+	struct clk *clk;
+	struct clk_fixed_rate *fixed;
+	struct clk_gate *gate;
+	const char *clk_name = node->name;
+	u32 rate;
+
+	if (of_property_read_u32(node, "clock-frequency", &rate))
+		return;
+
+	/* allocate fixed-rate and gate clock structs */
+	fixed = kzalloc(sizeof(struct clk_fixed_rate), GFP_KERNEL);
+	if (!fixed)
+		return;
+	gate = kzalloc(sizeof(struct clk_gate), GFP_KERNEL);
+	if (!gate)
+		goto err_free_fixed;
+
+	of_property_read_string(node, "clock-output-names", &clk_name);
+
+	/* set up gate and fixed rate properties */
+	gate->reg = of_iomap(node, 0);
+	gate->bit_idx = SUNXI_OSC24M_GATE;
+	gate->lock = &hosc_lock;
+	fixed->fixed_rate = rate;
+
+	clk = clk_register_composite(NULL, clk_name,
+			NULL, 0,
+			NULL, NULL,
+			&fixed->hw, &clk_fixed_rate_ops,
+			&gate->hw, &clk_gate_ops,
+			CLK_IS_ROOT);
+
+	if (IS_ERR(clk))
+		goto err_free_gate;
+
+	of_clk_add_provider(node, of_clk_src_simple_get, clk);
+	clk_register_clkdev(clk, clk_name, NULL);
+
+	return;
+
+err_free_gate:
+	kfree(gate);
+err_free_fixed:
+	kfree(fixed);
+}
+CLK_OF_DECLARE(sun4i_osc, "allwinner,sun4i-a10-osc-clk", sun4i_osc_clk_setup);
diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
index b1fde0f89bc1..c719e40cfb31 100644
--- a/drivers/clk/sunxi/clk-sunxi.c
+++ b/drivers/clk/sunxi/clk-sunxi.c
@@ -28,63 +28,6 @@ static DEFINE_SPINLOCK(clk_lock);
 #define SUNXI_MAX_PARENTS	5
 
 /**
- * sun4i_osc_clk_setup() - Setup function for gatable oscillator
- */
-
-#define SUNXI_OSC24M_GATE	0
-
-static void __init sun4i_osc_clk_setup(struct device_node *node)
-{
-	struct clk *clk;
-	struct clk_fixed_rate *fixed;
-	struct clk_gate *gate;
-	const char *clk_name = node->name;
-	u32 rate;
-
-	if (of_property_read_u32(node, "clock-frequency", &rate))
-		return;
-
-	/* allocate fixed-rate and gate clock structs */
-	fixed = kzalloc(sizeof(struct clk_fixed_rate), GFP_KERNEL);
-	if (!fixed)
-		return;
-	gate = kzalloc(sizeof(struct clk_gate), GFP_KERNEL);
-	if (!gate)
-		goto err_free_fixed;
-
-	of_property_read_string(node, "clock-output-names", &clk_name);
-
-	/* set up gate and fixed rate properties */
-	gate->reg = of_iomap(node, 0);
-	gate->bit_idx = SUNXI_OSC24M_GATE;
-	gate->lock = &clk_lock;
-	fixed->fixed_rate = rate;
-
-	clk = clk_register_composite(NULL, clk_name,
-			NULL, 0,
-			NULL, NULL,
-			&fixed->hw, &clk_fixed_rate_ops,
-			&gate->hw, &clk_gate_ops,
-			CLK_IS_ROOT);
-
-	if (IS_ERR(clk))
-		goto err_free_gate;
-
-	of_clk_add_provider(node, of_clk_src_simple_get, clk);
-	clk_register_clkdev(clk, clk_name, NULL);
-
-	return;
-
-err_free_gate:
-	kfree(gate);
-err_free_fixed:
-	kfree(fixed);
-}
-CLK_OF_DECLARE(sun4i_osc, "allwinner,sun4i-a10-osc-clk", sun4i_osc_clk_setup);
-
-
-
-/**
  * sun4i_get_pll1_factors() - calculates n, k, m, p factors for PLL1
  * PLL1 rate is calculated as follows
  * rate = (parent_rate * n * (k + 1) >> p) / (m + 1);
-- 
1.9.1

[PATCH 2/6] clk: sunxi: Move the 24M oscillator to a file of its own

[Emilio López]

[PATCH 2/6] clk: sunxi: Move the 24M oscillator to a file of its own

[Emilio López]

[PATCH 2/6] clk: sunxi: Move the 24M oscillator to a file of its own

[Maxime Ripard]

On Sat, May 10, 2014 at 01:46:38PM -0300, Emilio L?pez wrote:

There was probably something interesting here, but both the messages
you sent are full of empty :)

-- 
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: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140511/dd6f78a8/attachment.sig>

[PATCH 2/6] clk: sunxi: Move the 24M oscillator to a file of its own

[Emilio López]

El 11/05/14 01:35, Maxime Ripard escribi?:
> On Sat, May 10, 2014 at 01:46:38PM -0300, Emilio L?pez wrote:
>
> There was probably something interesting here, but both the messages
> you sent are full of empty :)
>

My email client decided it was cool to drop my email body on my first 
email and my attempt to resend it. My reply to 3/6 didn't seem to be 
affected though, and the comment was basically the same so I didn't 
bother trying again :)

Cheers,

Emilio

[PATCH 3/6] clk: sunxi: Move the GMAC clock to a file of its own

[Maxime Ripard]

Since we have a folder of our own, we can actually make use of it by splitting
the huge clock file into several sub drivers.

The gmac clock is pretty easy to deal with, since it's pretty much isolated and
doesn't have any dependency on the other clocks.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/clk/sunxi/Makefile       |   3 +-
 drivers/clk/sunxi/clk-a20-gmac.c | 119 +++++++++++++++++++++++++++++++++++++++
 drivers/clk/sunxi/clk-sunxi.c    |  98 --------------------------------
 3 files changed, 121 insertions(+), 99 deletions(-)
 create mode 100644 drivers/clk/sunxi/clk-a20-gmac.c

diff --git a/drivers/clk/sunxi/Makefile b/drivers/clk/sunxi/Makefile
index 2c3d2fed2560..839029e36144 100644
--- a/drivers/clk/sunxi/Makefile
+++ b/drivers/clk/sunxi/Makefile
@@ -3,4 +3,5 @@
 #
 
 obj-y += clk-sunxi.o clk-factors.o
-obj-y += clk-a10-hosc.o
\ No newline at end of file
+obj-y += clk-a10-hosc.o
+obj-y += clk-a20-gmac.o
diff --git a/drivers/clk/sunxi/clk-a20-gmac.c b/drivers/clk/sunxi/clk-a20-gmac.c
new file mode 100644
index 000000000000..633ddc4389ef
--- /dev/null
+++ b/drivers/clk/sunxi/clk-a20-gmac.c
@@ -0,0 +1,119 @@
+/*
+ * Copyright 2013 Emilio L??pez
+ * Emilio L??pez <emilio@elopez.com.ar>
+ *
+ * Copyright 2013 Chen-Yu Tsai
+ * Chen-Yu Tsai <wens@csie.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/clkdev.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/slab.h>
+
+static DEFINE_SPINLOCK(gmac_lock);
+
+/**
+ * sun7i_a20_gmac_clk_setup - Setup function for A20/A31 GMAC clock module
+ *
+ * This clock looks something like this
+ *                               ________________________
+ *  MII TX clock from PHY >-----|___________    _________|----> to GMAC core
+ *  GMAC Int. RGMII TX clk >----|___________\__/__gate---|----> to PHY
+ *  Ext. 125MHz RGMII TX clk >--|__divider__/            |
+ *                              |________________________|
+ *
+ * The external 125 MHz reference is optional, i.e. GMAC can use its
+ * internal TX clock just fine. The A31 GMAC clock module does not have
+ * the divider controls for the external reference.
+ *
+ * To keep it simple, let the GMAC use either the MII TX clock for MII mode,
+ * and its internal TX clock for GMII and RGMII modes. The GMAC driver should
+ * select the appropriate source and gate/ungate the output to the PHY.
+ *
+ * Only the GMAC should use this clock. Altering the clock so that it doesn't
+ * match the GMAC's operation parameters will result in the GMAC not being
+ * able to send traffic out. The GMAC driver should set the clock rate and
+ * enable/disable this clock to configure the required state. The clock
+ * driver then responds by auto-reparenting the clock.
+ */
+
+#define SUN7I_A20_GMAC_GPIT	2
+#define SUN7I_A20_GMAC_MASK	0x3
+#define SUN7I_A20_GMAC_PARENTS	2
+
+static void __init sun7i_a20_gmac_clk_setup(struct device_node *node)
+{
+	struct clk *clk;
+	struct clk_mux *mux;
+	struct clk_gate *gate;
+	const char *clk_name = node->name;
+	const char *parents[SUN7I_A20_GMAC_PARENTS];
+	void *reg;
+
+	if (of_property_read_string(node, "clock-output-names", &clk_name))
+		return;
+
+	/* allocate mux and gate clock structs */
+	mux = kzalloc(sizeof(struct clk_mux), GFP_KERNEL);
+	if (!mux)
+		return;
+
+	gate = kzalloc(sizeof(struct clk_gate), GFP_KERNEL);
+	if (!gate)
+		goto free_mux;
+
+	/* gmac clock requires exactly 2 parents */
+	parents[0] = of_clk_get_parent_name(node, 0);
+	parents[1] = of_clk_get_parent_name(node, 1);
+	if (!parents[0] || !parents[1])
+		goto free_gate;
+
+	reg = of_iomap(node, 0);
+	if (!reg)
+		goto free_gate;
+
+	/* set up gate and fixed rate properties */
+	gate->reg = reg;
+	gate->bit_idx = SUN7I_A20_GMAC_GPIT;
+	gate->lock = &gmac_lock;
+	mux->reg = reg;
+	mux->mask = SUN7I_A20_GMAC_MASK;
+	mux->flags = CLK_MUX_INDEX_BIT;
+	mux->lock = &gmac_lock;
+
+	clk = clk_register_composite(NULL, clk_name,
+			parents, SUN7I_A20_GMAC_PARENTS,
+			&mux->hw, &clk_mux_ops,
+			NULL, NULL,
+			&gate->hw, &clk_gate_ops,
+			0);
+
+	if (IS_ERR(clk))
+		goto iounmap_reg;
+
+	of_clk_add_provider(node, of_clk_src_simple_get, clk);
+	clk_register_clkdev(clk, clk_name, NULL);
+
+	return;
+
+iounmap_reg:
+	iounmap(reg);
+free_gate:
+	kfree(gate);
+free_mux:
+	kfree(mux);
+}
+CLK_OF_DECLARE(sun7i_a20_gmac, "allwinner,sun7i-a20-gmac-clk",
+		sun7i_a20_gmac_clk_setup);
diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
index c719e40cfb31..291a2605afa7 100644
--- a/drivers/clk/sunxi/clk-sunxi.c
+++ b/drivers/clk/sunxi/clk-sunxi.c
@@ -351,104 +351,6 @@ static void sun7i_a20_get_out_factors(u32 *freq, u32 parent_rate,
 	*p = calcp;
 }
 
-
-
-/**
- * sun7i_a20_gmac_clk_setup - Setup function for A20/A31 GMAC clock module
- *
- * This clock looks something like this
- *                               ________________________
- *  MII TX clock from PHY >-----|___________    _________|----> to GMAC core
- *  GMAC Int. RGMII TX clk >----|___________\__/__gate---|----> to PHY
- *  Ext. 125MHz RGMII TX clk >--|__divider__/            |
- *                              |________________________|
- *
- * The external 125 MHz reference is optional, i.e. GMAC can use its
- * internal TX clock just fine. The A31 GMAC clock module does not have
- * the divider controls for the external reference.
- *
- * To keep it simple, let the GMAC use either the MII TX clock for MII mode,
- * and its internal TX clock for GMII and RGMII modes. The GMAC driver should
- * select the appropriate source and gate/ungate the output to the PHY.
- *
- * Only the GMAC should use this clock. Altering the clock so that it doesn't
- * match the GMAC's operation parameters will result in the GMAC not being
- * able to send traffic out. The GMAC driver should set the clock rate and
- * enable/disable this clock to configure the required state. The clock
- * driver then responds by auto-reparenting the clock.
- */
-
-#define SUN7I_A20_GMAC_GPIT	2
-#define SUN7I_A20_GMAC_MASK	0x3
-#define SUN7I_A20_GMAC_PARENTS	2
-
-static void __init sun7i_a20_gmac_clk_setup(struct device_node *node)
-{
-	struct clk *clk;
-	struct clk_mux *mux;
-	struct clk_gate *gate;
-	const char *clk_name = node->name;
-	const char *parents[SUN7I_A20_GMAC_PARENTS];
-	void *reg;
-
-	if (of_property_read_string(node, "clock-output-names", &clk_name))
-		return;
-
-	/* allocate mux and gate clock structs */
-	mux = kzalloc(sizeof(struct clk_mux), GFP_KERNEL);
-	if (!mux)
-		return;
-
-	gate = kzalloc(sizeof(struct clk_gate), GFP_KERNEL);
-	if (!gate)
-		goto free_mux;
-
-	/* gmac clock requires exactly 2 parents */
-	parents[0] = of_clk_get_parent_name(node, 0);
-	parents[1] = of_clk_get_parent_name(node, 1);
-	if (!parents[0] || !parents[1])
-		goto free_gate;
-
-	reg = of_iomap(node, 0);
-	if (!reg)
-		goto free_gate;
-
-	/* set up gate and fixed rate properties */
-	gate->reg = reg;
-	gate->bit_idx = SUN7I_A20_GMAC_GPIT;
-	gate->lock = &clk_lock;
-	mux->reg = reg;
-	mux->mask = SUN7I_A20_GMAC_MASK;
-	mux->flags = CLK_MUX_INDEX_BIT;
-	mux->lock = &clk_lock;
-
-	clk = clk_register_composite(NULL, clk_name,
-			parents, SUN7I_A20_GMAC_PARENTS,
-			&mux->hw, &clk_mux_ops,
-			NULL, NULL,
-			&gate->hw, &clk_gate_ops,
-			0);
-
-	if (IS_ERR(clk))
-		goto iounmap_reg;
-
-	of_clk_add_provider(node, of_clk_src_simple_get, clk);
-	clk_register_clkdev(clk, clk_name, NULL);
-
-	return;
-
-iounmap_reg:
-	iounmap(reg);
-free_gate:
-	kfree(gate);
-free_mux:
-	kfree(mux);
-}
-CLK_OF_DECLARE(sun7i_a20_gmac, "allwinner,sun7i-a20-gmac-clk",
-		sun7i_a20_gmac_clk_setup);
-
-
-
 /**
  * sunxi_factors_clk_setup() - Setup function for factor clocks
  */
-- 
1.9.1

[linux-sunxi] [PATCH 3/6] clk: sunxi: Move the GMAC clock to a file of its own

[Emilio López]

Hi Maxime,

[let's hope this email goes through as non-empty]

El 10/05/14 00:33, Maxime Ripard escribi?:
> Since we have a folder of our own, we can actually make use of it by splitting
> the huge clock file into several sub drivers.
>
> The gmac clock is pretty easy to deal with, since it's pretty much isolated and
> doesn't have any dependency on the other clocks.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---

Looks good to me, but see below.

>   drivers/clk/sunxi/Makefile       |   3 +-
>   drivers/clk/sunxi/clk-a20-gmac.c | 119 +++++++++++++++++++++++++++++++++++++++
>   drivers/clk/sunxi/clk-sunxi.c    |  98 --------------------------------
>   3 files changed, 121 insertions(+), 99 deletions(-)
>   create mode 100644 drivers/clk/sunxi/clk-a20-gmac.c
>
(snip)
> +
> +	clk = clk_register_composite(NULL, clk_name,
> +			parents, SUN7I_A20_GMAC_PARENTS,
> +			&mux->hw, &clk_mux_ops,
> +			NULL, NULL,
> +			&gate->hw, &clk_gate_ops,
> +			0);
> +
> +	if (IS_ERR(clk))
> +		goto iounmap_reg;
> +
> +	of_clk_add_provider(node, of_clk_src_simple_get, clk);
> +	clk_register_clkdev(clk, clk_name, NULL);

As I mentioned on the other email, I don't think we are using clkdev. 
Maybe we can drop it.

Thanks for working on this!

Emilio

[linux-sunxi] [PATCH 3/6] clk: sunxi: Move the GMAC clock to a file of its own

[Maxime Ripard]

On Sat, May 10, 2014 at 02:07:07PM -0300, Emilio L?pez wrote:
> >+
> >+	clk = clk_register_composite(NULL, clk_name,
> >+			parents, SUN7I_A20_GMAC_PARENTS,
> >+			&mux->hw, &clk_mux_ops,
> >+			NULL, NULL,
> >+			&gate->hw, &clk_gate_ops,
> >+			0);
> >+
> >+	if (IS_ERR(clk))
> >+		goto iounmap_reg;
> >+
> >+	of_clk_add_provider(node, of_clk_src_simple_get, clk);
> >+	clk_register_clkdev(clk, clk_name, NULL);
> 
> As I mentioned on the other email, I don't think we are using
> clkdev. Maybe we can drop it.

No we aren't, but we should not really modify code while moving it. I
can send another patch to do so, but that should not prevent this code
from going in.

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: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140512/47662594/attachment.sig>

[linux-sunxi] [PATCH 3/6] clk: sunxi: Move the GMAC clock to a file of its own

[Emilio López]

Hi,

El 12/05/14 16:36, Maxime Ripard escribi?:
> On Sat, May 10, 2014 at 02:07:07PM -0300, Emilio L?pez wrote:
>>> +
>>> +	clk = clk_register_composite(NULL, clk_name,
>>> +			parents, SUN7I_A20_GMAC_PARENTS,
>>> +			&mux->hw, &clk_mux_ops,
>>> +			NULL, NULL,
>>> +			&gate->hw, &clk_gate_ops,
>>> +			0);
>>> +
>>> +	if (IS_ERR(clk))
>>> +		goto iounmap_reg;
>>> +
>>> +	of_clk_add_provider(node, of_clk_src_simple_get, clk);
>>> +	clk_register_clkdev(clk, clk_name, NULL);
>>
>> As I mentioned on the other email, I don't think we are using
>> clkdev. Maybe we can drop it.
>
> No we aren't, but we should not really modify code while moving it. I
> can send another patch to do so, but that should not prevent this code
> from going in.

That would be great :) The patches themselves are okay, I just thought 
I'd mention it so it's not forgotten.

Cheers,

Emilio

[linux-sunxi] [PATCH 3/6] clk: sunxi: Move the GMAC clock to a file of its own

[Maxime Ripard]

On Mon, May 12, 2014 at 04:45:08PM -0300, Emilio L?pez wrote:
> >>>+	clk = clk_register_composite(NULL, clk_name,
> >>>+			parents, SUN7I_A20_GMAC_PARENTS,
> >>>+			&mux->hw, &clk_mux_ops,
> >>>+			NULL, NULL,
> >>>+			&gate->hw, &clk_gate_ops,
> >>>+			0);
> >>>+
> >>>+	if (IS_ERR(clk))
> >>>+		goto iounmap_reg;
> >>>+
> >>>+	of_clk_add_provider(node, of_clk_src_simple_get, clk);
> >>>+	clk_register_clkdev(clk, clk_name, NULL);
> >>
> >>As I mentioned on the other email, I don't think we are using
> >>clkdev. Maybe we can drop it.
> >
> >No we aren't, but we should not really modify code while moving it. I
> >can send another patch to do so, but that should not prevent this code
> >from going in.
> 
> That would be great :) The patches themselves are okay, I just
> thought I'd mention it so it's not forgotten.

It's not, Boris reminded it to me a week ago :)

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: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140512/19e91e02/attachment.sig>

[PATCH 4/6] clk: sunxi: Rework clock protection code

[Maxime Ripard]

Since we start to have a lot of clocks to protect, some of them in a few SoCs
only, it becomes difficult to handle the clock protection without having to add
per machine exceptions.

Add per-SoC data to tell which clock to leave enabled.

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

diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
index 291a2605afa7..384a6763eccc 100644
--- a/drivers/clk/sunxi/clk-sunxi.c
+++ b/drivers/clk/sunxi/clk-sunxi.c
@@ -1130,29 +1130,10 @@ static void __init of_sunxi_table_clock_setup(const struct of_device_id *clk_mat
 	}
 }
 
-/**
- * System clock protection
- *
- * By enabling these critical clocks, we prevent their accidental gating
- * by the framework
- */
-static void __init sunxi_clock_protect(void)
+static void __init sunxi_init_clocks(const char *clocks[], int nclocks)
 {
-	struct clk *clk;
-
-	/* memory bus clock - sun5i+ */
-	clk = clk_get(NULL, "mbus");
-	if (!IS_ERR(clk))
-		clk_prepare_enable(clk);
-
-	/* DDR clock - sun4i+ */
-	clk = clk_get(NULL, "pll5_ddr");
-	if (!IS_ERR(clk))
-		clk_prepare_enable(clk);
-}
+	unsigned int i;
 
-static void __init sunxi_init_clocks(void)
-{
 	/* Register factor clocks */
 	of_sunxi_table_clock_setup(clk_factors_match, sunxi_factors_clk_setup);
 
@@ -1168,11 +1149,46 @@ static void __init sunxi_init_clocks(void)
 	/* Register gate clocks */
 	of_sunxi_table_clock_setup(clk_gates_match, sunxi_gates_clk_setup);
 
-	/* Enable core system clocks */
-	sunxi_clock_protect();
+	/* 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(void)
+{
+	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 = {
+	"mbus",
+	"pll5_ddr",
+};
+
+static void __init sun5i_init_clocks(void)
+{
+	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(sun7i_a20_clk_init, "allwinner,sun7i-a20", sun5i_init_clocks);
+
+static const char *sun6i_critical_clocks[] __initdata = {
+};
+
+static void __init sun6i_init_clocks(void)
+{
+	sunxi_init_clocks(sun6i_critical_clocks,
+			  ARRAY_SIZE(sun6i_critical_clocks));
 }
-CLK_OF_DECLARE(sun4i_a10_clk_init, "allwinner,sun4i-a10", sunxi_init_clocks);
-CLK_OF_DECLARE(sun5i_a10s_clk_init, "allwinner,sun5i-a10s", sunxi_init_clocks);
-CLK_OF_DECLARE(sun5i_a13_clk_init, "allwinner,sun5i-a13", sunxi_init_clocks);
-CLK_OF_DECLARE(sun6i_a31_clk_init, "allwinner,sun6i-a31", sunxi_init_clocks);
-CLK_OF_DECLARE(sun7i_a20_clk_init, "allwinner,sun7i-a20", sunxi_init_clocks);
+CLK_OF_DECLARE(sun6i_a31_clk_init, "allwinner,sun6i-a31", sun6i_init_clocks);
-- 
1.9.1

[PATCH 5/6] clk: sun6i: Protect CPU clock

[Maxime Ripard]

Right now, AHB is an indirect child clock of the CPU clock. If that happens to
change, since the CPU clock has no other consumers declared in Linux, it would
be shut down, which is not really a good idea.

Prevent this by forcing it enabled.

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

diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
index 384a6763eccc..fdb9cea60011 100644
--- a/drivers/clk/sunxi/clk-sunxi.c
+++ b/drivers/clk/sunxi/clk-sunxi.c
@@ -1184,6 +1184,7 @@ CLK_OF_DECLARE(sun5i_a13_clk_init, "allwinner,sun5i-a13", 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(void)
-- 
1.9.1

[PATCH 6/6] clk: sun6i: Protect SDRAM gating bit

[Maxime Ripard]

Prevent the SDRAM controller from being gated by force-enabling it in the
machine code.

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

diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
index fdb9cea60011..548f7113efa6 100644
--- a/drivers/clk/sunxi/clk-sunxi.c
+++ b/drivers/clk/sunxi/clk-sunxi.c
@@ -1185,6 +1185,7 @@ CLK_OF_DECLARE(sun7i_a20_clk_init, "allwinner,sun7i-a20", sun5i_init_clocks);
 
 static const char *sun6i_critical_clocks[] __initdata = {
 	"cpu",
+	"ahb1_sdram",
 };
 
 static void __init sun6i_init_clocks(void)
-- 
1.9.1

[linux-sunxi] [PATCH 0/6] sunxi: clk: Various cleanup and rework

[Emilio López]

Hi Maxime,

El 10/05/14 00:33, Maxime Ripard escribi?:
> Hi everyone,
>
> This patchset fixes a few things that have been pending for quite a
> while in the clock driver.
>
> First, it removes the clk_put calls in the clock protection
> part. Since it's not really something that should be done, I guess
> this patch is not very controversial.
>
> Then, it starts splitting the huge clock driver file into separate,
> smaller drivers when it makes sense.
>
> Finally, it reworks the clock protection mechanism to handle
> differences between SoC in a better way. This has been pretty
> controversial because the first approach has been to move this to the
> machine code. This is another attempt that leaves all the
> modifications in the driver itself.
>
> Thanks,
> Maxime

Overall the series looks good to me, other than the comment about clkdev 
on patches 2 and 3.

@Mike, do you want me to take them in a pull as usual after you have a 
look? Please let me know as we still haven't clarified the situation 
with the two patches Hans sent.

Cheers, and thanks Maxime for working on cleaning this :)

Emilio

[linux-sunxi] [PATCH 0/6] sunxi: clk: Various cleanup and rework

[Mike Turquette]

Quoting Emilio L?pez (2014-05-10 10:22:15)
> Hi Maxime,
> 
> El 10/05/14 00:33, Maxime Ripard escribi?:
> > Hi everyone,
> >
> > This patchset fixes a few things that have been pending for quite a
> > while in the clock driver.
> >
> > First, it removes the clk_put calls in the clock protection
> > part. Since it's not really something that should be done, I guess
> > this patch is not very controversial.
> >
> > Then, it starts splitting the huge clock driver file into separate,
> > smaller drivers when it makes sense.
> >
> > Finally, it reworks the clock protection mechanism to handle
> > differences between SoC in a better way. This has been pretty
> > controversial because the first approach has been to move this to the
> > machine code. This is another attempt that leaves all the
> > modifications in the driver itself.
> >
> > Thanks,
> > Maxime
> 
> Overall the series looks good to me, other than the comment about clkdev 
> on patches 2 and 3.
> 
> @Mike, do you want me to take them in a pull as usual after you have a 
> look? Please let me know as we still haven't clarified the situation 
> with the two patches Hans sent.

A pull request would be great. Feel free to add my Acked-by to all of
the patches here (#1 already has it) since I looked at all of them and
the changes seem good.

Regards,
Mike

> 
> Cheers, and thanks Maxime for working on cleaning this :)
> 
> Emilio

[linux-sunxi] [PATCH 0/6] sunxi: clk: Various cleanup and rework

[Emilio López]

El 12/05/14 22:12, Mike Turquette escribi?:
> Quoting Emilio L?pez (2014-05-10 10:22:15)
>> Hi Maxime,
>>
>> El 10/05/14 00:33, Maxime Ripard escribi?:
>>> Hi everyone,
>>>
>>> This patchset fixes a few things that have been pending for quite a
>>> while in the clock driver.
>>>
>>> First, it removes the clk_put calls in the clock protection
>>> part. Since it's not really something that should be done, I guess
>>> this patch is not very controversial.
>>>
>>> Then, it starts splitting the huge clock driver file into separate,
>>> smaller drivers when it makes sense.
>>>
>>> Finally, it reworks the clock protection mechanism to handle
>>> differences between SoC in a better way. This has been pretty
>>> controversial because the first approach has been to move this to the
>>> machine code. This is another attempt that leaves all the
>>> modifications in the driver itself.
>>>
>>> Thanks,
>>> Maxime
>>
>> Overall the series looks good to me, other than the comment about clkdev
>> on patches 2 and 3.
>>
>> @Mike, do you want me to take them in a pull as usual after you have a
>> look? Please let me know as we still haven't clarified the situation
>> with the two patches Hans sent.
>
> A pull request would be great. Feel free to add my Acked-by to all of
> the patches here (#1 already has it) since I looked at all of them and
> the changes seem good.

Ok, I've taken them in sunxi-clk-for-mike with Mike's ack. I'll be 
sending the pull request soon.

Cheers,

Emilio