sunxi: clk: Various cleanup and rework

PATCH New Maxime Ripard 3 participants 18 messages May 10, 2014 – May 15, 2014

Subsystems (3)

Common clk Arm/Allwinner sunXi SoC support Arm/Allwinner SoC Clock Support

lore

Share & tools

lore

Maxime Ripard

author

2014-05-10 03:33 UTC #0 lore raw

cover sunxi: clk: Various cleanup and rework

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

Signature · 1 line

1.9.1

Maxime Ripard

author

2014-05-10 03:33 UTC #1 lore raw

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

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 <[email protected]>

Acked-by Mike Turquette <[email protected]>

---
 drivers/clk/sunxi/clk-sunxi.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

drivers/clk/sunxi/clk-sunxi.c +2 -6

index bd7dc733c1ca..b1fde0f89bc1 100644
@@ -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)

Signature · 1 line

1.9.1

Maxime Ripard

author

2014-05-10 03:33 UTC #2 lore raw

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

In reply to 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 <[email protected]>

---
 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

drivers/clk/sunxi/Makefile +1 -0

index b5bac917612c..2c3d2fed2560 100644
@@ -3,3 +3,4 @@
 #
 
 obj-y += clk-sunxi.o clk-factors.o
+obj-y += clk-a10-hosc.o
\ No newline at end of file

drivers/clk/sunxi/clk-a10-hosc.c new file +73 -0

73 lines not on this page · raw message

drivers/clk/sunxi/clk-sunxi.c +0 ~~-57~~

63 lines not on this page · raw message

Signature · 1 line

1.9.1

Emilio López

maintainer

2014-05-10 16:13 UTC #3 lore raw

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

Emilio López

maintainer

2014-05-10 16:46 UTC #4 lore raw

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

In reply to Maxime Ripard

Maxime Ripard

author

2014-05-11 04:35 UTC #5 lore raw

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

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 :)

Signature · 10 lines

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>

Emilio López

maintainer

2014-05-11 04:44 UTC #6 lore raw

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

El 11/05/14 01:35, Maxime Ripard escribi?:

Quoted 5 lines · Emilio L?pez

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

Maxime Ripard

author

2014-05-10 03:33 UTC #7 lore raw

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

In reply to 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 <[email protected]>

---
 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

drivers/clk/sunxi/Makefile +2 -1

index 2c3d2fed2560..839029e36144 100644
@@ -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

drivers/clk/sunxi/clk-a20-gmac.c new file +119 -0

119 lines not on this page · raw message

drivers/clk/sunxi/clk-sunxi.c +0 ~~-98~~

104 lines not on this page · raw message

Signature · 1 line

1.9.1

Emilio López

maintainer

2014-05-10 17:07 UTC #8 lore raw

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

Hi Maxime,

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

El 10/05/14 00:33, Maxime Ripard escribi?:

Quoted 8 lines

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 <[email protected]>
---

Looks good to me, but see below.

Quoted 6 lines

  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)

drivers/clk/sunxi/clk-a20-gmac.c line 107

Quoted 13 lines

drivers/clk/sunxi/clk-a20-gmac.c new file +13 -0

+
+	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

Maxime Ripard

author

2014-05-12 19:36 UTC #9 lore raw

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

Quoted 8 lines · Emilio L?pez

Quoted 8 lines

drivers/clk/sunxi/clk-a20-gmac.c new file +8 -0

+
+	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);
+

Quoted 8 lines

drivers/clk/sunxi/clk-a20-gmac.c line 107
Quoted 5 lines
drivers/clk/sunxi/clk-a20-gmac.c new file +5 -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

Signature · 10 lines

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>

Emilio López

maintainer

2014-05-12 19:45 UTC #10 lore raw

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

Hi,

El 12/05/14 16:36, Maxime Ripard escribi?:

Quoted 18 lines · Emilio L?pez

Quoted 8 lines
Quoted 8 lines
drivers/clk/sunxi/clk-a20-gmac.c new file +8 -0
+
+	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);
+
Quoted 8 lines
drivers/clk/sunxi/clk-a20-gmac.c line 107
Quoted 5 lines
drivers/clk/sunxi/clk-a20-gmac.c new file +5 -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.

Quoted 3 lines

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

Maxime Ripard

author

2014-05-12 20:54 UTC #11 lore raw

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

Quoted 16 lines · Emilio L?pez

Quoted 16 lines
Quoted 15 lines
drivers/clk/sunxi/clk-a20-gmac.c line 107
Quoted 12 lines
drivers/clk/sunxi/clk-a20-gmac.c new file +12 -0
+	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.

Quoted 6 lines

Quoted 3 lines
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

Signature · 10 lines

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>

Maxime Ripard

author

2014-05-10 03:33 UTC #12 lore raw

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

In reply to 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 <[email protected]>

---
 drivers/clk/sunxi/clk-sunxi.c | 72 ++++++++++++++++++++++++++-----------------
 1 file changed, 44 insertions(+), 28 deletions(-)

drivers/clk/sunxi/clk-sunxi.c +44 ~~-28~~

index 291a2605afa7..384a6763eccc 100644
@@ -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);

Signature · 1 line

1.9.1

Maxime Ripard

author

2014-05-10 03:33 UTC #13 lore raw

PATCH 5/6 clk: sun6i: Protect CPU clock

In reply to 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 <[email protected]>

---
 drivers/clk/sunxi/clk-sunxi.c | 1 +
 1 file changed, 1 insertion(+)

drivers/clk/sunxi/clk-sunxi.c +1 -0

index 384a6763eccc..fdb9cea60011 100644
@@ -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)

Signature · 1 line

1.9.1

Maxime Ripard

author

2014-05-10 03:33 UTC #14 lore raw

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

In reply to Maxime Ripard

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

Signed-off-by Maxime Ripard <[email protected]>

---
 drivers/clk/sunxi/clk-sunxi.c | 1 +
 1 file changed, 1 insertion(+)

drivers/clk/sunxi/clk-sunxi.c +1 -0

index fdb9cea60011..548f7113efa6 100644
@@ -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)

Signature · 1 line

1.9.1

Emilio López

maintainer

2014-05-10 17:22 UTC #15 lore raw

cover sunxi: clk: Various cleanup and rework

In reply to Maxime Ripard

Hi Maxime,

El 10/05/14 00:33, Maxime Ripard escribi?:

Quoted 12 lines

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.

Quoted 8 lines

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

Mike Turquette

2014-05-13 01:12 UTC #16 lore raw

cover sunxi: clk: Various cleanup and rework

Quoting Emilio L?pez (2014-05-10 10:22:15)

Quoted 24 lines

Hi Maxime,
El 10/05/14 00:33, Maxime Ripard escribi?:
Quoted 12 lines
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.
Quoted 8 lines
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

Quoted 6 lines

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

Quoted 4 lines

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

Emilio López

maintainer

2014-05-15 15:10 UTC #17 lore raw

cover sunxi: clk: Various cleanup and rework

El 12/05/14 22:12, Mike Turquette escribi?:

Quoted 28 lines

Quoting Emilio L?pez (2014-05-10 10:22:15)
Quoted 27 lines
Hi Maxime,
El 10/05/14 00:33, Maxime Ripard escribi?:
Quoted 12 lines
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.
Quoted 8 lines
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.

Quoted 7 lines

Quoted 3 lines
@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