[PATCH RFC v2 00/16] ARM: provide common arch init for DT clocks

[PATCH RFC v2 00/16] ARM: provide common arch init for DT clocks

[Sebastian Hesselbarth]

This RFC converts arch/arm to provide a common arch init for DT clock
providers. Currently, the call to of_clk_init(NULL) to initialize DT
clock providers is spread among several mach-dirs. Since most machs
require DT clocks initialized before timers, no initcall can be used.

By adding of_clk_init(NULL) to ARM time_init(), we can remove all
mach-specific .init_time hooks that basically called of_clk_init before
starting timers.

Based on the previous version, this one now includes patches provided by
Stephen Warren for mach-tegra and Soren Brinkmann for mach-zynq.

With mach-tegra now requiring clocks as late as the other machs, the
patch to protect of_clk_init() from being called twice, has been dropped.

The RFCv2 is based on next-20130827 and has been compile tested for
multi_v7_defconfig and mach-dove. All single patches have also been sent
to the respective maintainers and corresponding mailing lists.

Before going for a real patch set after v3.11 drops, I prefer to have
Tested-by or Acked-by for at least mach-imx, mach-mxs, mach-sti, and
mach-vexpress. Others are trivial, prepared by maintainers (mach-tegra
and mach-zynq), or have already been Acked-by (mach-highbank).

Also, I'd like to know if the patches should be grouped by mach- or
rather been split into mach- preparation and .init_time removal.

Sebastian Hesselbarth (14):
  ARM: call clk_of_init from time_init
  ARM: dove: remove custom .init_time hook
  ARM: exynos: remove custom .init_time hook
  ARM: highbank: remove custom .init_time hook
  ARM: imx: remove custom .init_time hook
  ARM: kirkwood: remove custom .init_time hook
  ARM: mvebu: remove custom .init_time hook
  ARM: mxs: remove custom .init_time hook
  ARM: nspire: remove custom .init_time hook
  ARM: rockchip: remove custom .init_time hook
  ARM: socfpga: remove call to of_clk_init
  ARM: sti: remove custom .init_time hook
  ARM: vexpress: remove custom .init_time hook
  clk: vt8500: remove call to of_clk_init

Soren Brinkmann (1):
  ARM: zynq: Don't call of_clk_init()

Stephen Warren (1):
  ARM: tegra: split tegra_pmc_init() in two

 arch/arm/kernel/time.c                 |   24 +++++++++++-------
 arch/arm/mach-dove/board-dt.c          |   17 +------------
 arch/arm/mach-exynos/common.c          |    7 -----
 arch/arm/mach-exynos/common.h          |    1 -
 arch/arm/mach-exynos/mach-exynos4-dt.c |    1 -
 arch/arm/mach-exynos/mach-exynos5-dt.c |    1 -
 arch/arm/mach-highbank/highbank.c      |   23 +++++------------
 arch/arm/mach-imx/clk-imx51-imx53.c    |   29 +++++++--------------
 arch/arm/mach-imx/common.h             |    4 ---
 arch/arm/mach-imx/imx51-dt.c           |    6 ----
 arch/arm/mach-imx/mach-imx53.c         |    6 ----
 arch/arm/mach-imx/mach-imx6q.c         |   14 ++--------
 arch/arm/mach-imx/mach-imx6sl.c        |    7 -----
 arch/arm/mach-imx/mach-vf610.c         |    9 -------
 arch/arm/mach-kirkwood/board-dt.c      |    8 ------
 arch/arm/mach-mvebu/armada-370-xp.c    |    2 -
 arch/arm/mach-mxs/mach-mxs.c           |   13 ----------
 arch/arm/mach-nspire/nspire.c          |    9 -------
 arch/arm/mach-rockchip/rockchip.c      |    9 -------
 arch/arm/mach-socfpga/socfpga.c        |    2 -
 arch/arm/mach-sti/board-dt.c           |   10 +++----
 arch/arm/mach-tegra/common.c           |    4 +--
 arch/arm/mach-tegra/pmc.c              |   41 +++++++++++++++++--------------
 arch/arm/mach-tegra/pmc.h              |    1 +
 arch/arm/mach-tegra/tegra.c            |    5 ++-
 arch/arm/mach-vexpress/v2m.c           |   14 +----------
 arch/arm/mach-zynq/common.c            |    9 +++----
 drivers/clk/clk-highbank.c             |   10 +++++--
 drivers/clk/clk-vt8500.c               |    2 -
 drivers/clk/mxs/clk-imx23.c            |   16 +++++-------
 drivers/clk/mxs/clk-imx28.c            |   16 +++++-------
 drivers/clk/zynq/clkc.c                |    4 ++-
 include/linux/clk/mxs.h                |    2 -
 33 files changed, 95 insertions(+), 231 deletions(-)

---
Cc: Mike Turquette <mturquette@linaro.org>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: linux-tegra@vger.kernel.org
Cc: kernel@stlinux.com
Cc: linux-samsung-soc@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
-- 
1.7.2.5

[PATCH RFC v2 01/16] ARM: call clk_of_init from time_init

[Sebastian Hesselbarth]

Most DT ARM machs require common clock providers initialized before timers.
Currently, arch/arm machs use .init_time to call clk_of_init right before
clocksource_of_init. This prevents to remove that hook and use the default
hook instead. clk_of_init is safe to call for non-DT platforms, so add
the call to ARM arch time_init by default. While at it, also reorder includes
alphabetically.

Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
Changelog:
v1->v2:
- reorder includes alphabetically

Cc: Russell King <linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
Cc: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: kernel-F5mvAk5X5gdBDgjK7y7TUQ@public.gmane.org
Cc: linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
---
 arch/arm/kernel/time.c |   24 ++++++++++++++----------
 1 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/arch/arm/kernel/time.c b/arch/arm/kernel/time.c
index 98aee32..dd1028e 100644
--- a/arch/arm/kernel/time.c
+++ b/arch/arm/kernel/time.c
@@ -11,25 +11,26 @@
  *  This file contains the ARM-specific time handling details:
  *  reading the RTC at bootup, etc...
  */
+#include <linux/clk-provider.h>
+#include <linux/clocksource.h>
+#include <linux/errno.h>
 #include <linux/export.h>
-#include <linux/kernel.h>
-#include <linux/interrupt.h>
-#include <linux/time.h>
 #include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/kernel.h>
+#include <linux/profile.h>
 #include <linux/sched.h>
+#include <linux/sched_clock.h>
 #include <linux/smp.h>
+#include <linux/time.h>
 #include <linux/timex.h>
-#include <linux/errno.h>
-#include <linux/profile.h>
 #include <linux/timer.h>
-#include <linux/clocksource.h>
-#include <linux/irq.h>
-#include <linux/sched_clock.h>
 
-#include <asm/thread_info.h>
-#include <asm/stacktrace.h>
 #include <asm/mach/arch.h>
 #include <asm/mach/time.h>
+#include <asm/stacktrace.h>
+#include <asm/thread_info.h>
 
 #if defined(CONFIG_RTC_DRV_CMOS) || defined(CONFIG_RTC_DRV_CMOS_MODULE) || \
     defined(CONFIG_NVRAM) || defined(CONFIG_NVRAM_MODULE)
@@ -116,6 +117,9 @@ int __init register_persistent_clock(clock_access_fn read_boot,
 
 void __init time_init(void)
 {
+	/* initalize common clocks before timers */
+	of_clk_init(NULL);
+
 	if (machine_desc->init_time)
 		machine_desc->init_time();
 	else
-- 
1.7.2.5

Re: [PATCH RFC v2 01/16] ARM: call clk_of_init from time_init

[Sören Brinkmann]

On Tue, Aug 27, 2013 at 11:27:55PM +0200, Sebastian Hesselbarth wrote:
> Most DT ARM machs require common clock providers initialized before timers.
> Currently, arch/arm machs use .init_time to call clk_of_init right before
> clocksource_of_init. This prevents to remove that hook and use the default
> hook instead. clk_of_init is safe to call for non-DT platforms, so add
> the call to ARM arch time_init by default. While at it, also reorder includes
> alphabetically.
> 
> Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
> ---
> Changelog:
> v1->v2:
> - reorder includes alphabetically
> 
> Cc: Russell King <linux@arm.linux.org.uk>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: linux-tegra@vger.kernel.org
> Cc: kernel@stlinux.com
> Cc: linux-samsung-soc@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> ---
>  arch/arm/kernel/time.c |   24 ++++++++++++++----------
>  1 files changed, 14 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/arm/kernel/time.c b/arch/arm/kernel/time.c
> index 98aee32..dd1028e 100644
> --- a/arch/arm/kernel/time.c
> +++ b/arch/arm/kernel/time.c
> @@ -11,25 +11,26 @@
[ ... ]
>  void __init time_init(void)
>  {
> +	/* initalize common clocks before timers */
> +	of_clk_init(NULL);
> +
>  	if (machine_desc->init_time)
>  		machine_desc->init_time();
>  	else

This forces zynq to move some initialization our clock code relies on to
init_irq(). Also, the current code already takes an approach of
doing either common init or machine specific init.
I think it might be better to move the call to of_clk_init() down into
the else branch of the if-else.
Though, this probably contradicts the purpose of the whole series.

	Sören

Re: [PATCH RFC v2 01/16] ARM: call clk_of_init from time_init

[Sebastian Hesselbarth]

On 08/28/13 00:19, Sören Brinkmann wrote:
> On Tue, Aug 27, 2013 at 11:27:55PM +0200, Sebastian Hesselbarth wrote:
>> Most DT ARM machs require common clock providers initialized before timers.
>> Currently, arch/arm machs use .init_time to call clk_of_init right before
>> clocksource_of_init. This prevents to remove that hook and use the default
>> hook instead. clk_of_init is safe to call for non-DT platforms, so add
>> the call to ARM arch time_init by default. While at it, also reorder includes
>> alphabetically.
>>
>> Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
>> ---
>> Changelog:
>> v1->v2:
>> - reorder includes alphabetically
>>
>> Cc: Russell King <linux@arm.linux.org.uk>
>> Cc: Arnd Bergmann <arnd@arndb.de>
>> Cc: linux-tegra@vger.kernel.org
>> Cc: kernel@stlinux.com
>> Cc: linux-samsung-soc@vger.kernel.org
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: linux-kernel@vger.kernel.org
>> ---
>>   arch/arm/kernel/time.c |   24 ++++++++++++++----------
>>   1 files changed, 14 insertions(+), 10 deletions(-)
>>
>> diff --git a/arch/arm/kernel/time.c b/arch/arm/kernel/time.c
>> index 98aee32..dd1028e 100644
>> --- a/arch/arm/kernel/time.c
>> +++ b/arch/arm/kernel/time.c
>> @@ -11,25 +11,26 @@
> [ ... ]
>>   void __init time_init(void)
>>   {
>> +	/* initalize common clocks before timers */
>> +	of_clk_init(NULL);
>> +
>>   	if (machine_desc->init_time)
>>   		machine_desc->init_time();
>>   	else
>
> This forces zynq to move some initialization our clock code relies on to
> init_irq(). Also, the current code already takes an approach of
> doing either common init or machine specific init.

Soeren,

you know that patch 16/16 takes care of zynq's clock init?

It's your own patch you provided from the last RFC. Looking at it, it
moves  zynq_sclr_init() to .init_irq and removes the call to
of_clk_init() from zynq_clock_init() which is called by
zynq_sclr_init().

Isn't that solving the above issues for mach-zynq?

> I think it might be better to move the call to of_clk_init() down into
> the else branch of the if-else.

Possibly, yes. But we could also unconditionally call of_clk_init() at
the beginning of time_init() and call clocksource_of_init() at the end.
That will make .init_time() to some fixup hook between initialization of 
clocks and timers.

> Though, this probably contradicts the purpose of the whole series.

Hmm, the purpose was to allow most platforms to remove a custom
.init_time hook only calling of_clk_init() and clocksource_of_init().

If we move of_clk_init() above also in the else-branch the purpose
is still met.

Sebastian

Re: [PATCH RFC v2 01/16] ARM: call clk_of_init from time_init

[Sören Brinkmann]

On Wed, Aug 28, 2013 at 12:58:39AM +0200, Sebastian Hesselbarth wrote:
> On 08/28/13 00:19, Sören Brinkmann wrote:
> >On Tue, Aug 27, 2013 at 11:27:55PM +0200, Sebastian Hesselbarth wrote:
> >>Most DT ARM machs require common clock providers initialized before timers.
> >>Currently, arch/arm machs use .init_time to call clk_of_init right before
> >>clocksource_of_init. This prevents to remove that hook and use the default
> >>hook instead. clk_of_init is safe to call for non-DT platforms, so add
> >>the call to ARM arch time_init by default. While at it, also reorder includes
> >>alphabetically.
> >>
> >>Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
> >>---
> >>Changelog:
> >>v1->v2:
> >>- reorder includes alphabetically
> >>
> >>Cc: Russell King <linux@arm.linux.org.uk>
> >>Cc: Arnd Bergmann <arnd@arndb.de>
> >>Cc: linux-tegra@vger.kernel.org
> >>Cc: kernel@stlinux.com
> >>Cc: linux-samsung-soc@vger.kernel.org
> >>Cc: linux-arm-kernel@lists.infradead.org
> >>Cc: linux-kernel@vger.kernel.org
> >>---
> >>  arch/arm/kernel/time.c |   24 ++++++++++++++----------
> >>  1 files changed, 14 insertions(+), 10 deletions(-)
> >>
> >>diff --git a/arch/arm/kernel/time.c b/arch/arm/kernel/time.c
> >>index 98aee32..dd1028e 100644
> >>--- a/arch/arm/kernel/time.c
> >>+++ b/arch/arm/kernel/time.c
> >>@@ -11,25 +11,26 @@
> >[ ... ]
> >>  void __init time_init(void)
> >>  {
> >>+	/* initalize common clocks before timers */
> >>+	of_clk_init(NULL);
> >>+
> >>  	if (machine_desc->init_time)
> >>  		machine_desc->init_time();
> >>  	else
> >
> >This forces zynq to move some initialization our clock code relies on to
> >init_irq(). Also, the current code already takes an approach of
> >doing either common init or machine specific init.
> 
> Soeren,
> 
> you know that patch 16/16 takes care of zynq's clock init?
> 
> It's your own patch you provided from the last RFC. Looking at it, it
> moves  zynq_sclr_init() to .init_irq and removes the call to
> of_clk_init() from zynq_clock_init() which is called by
> zynq_sclr_init().
> 
> Isn't that solving the above issues for mach-zynq?

Yes, I know. This alternative approach came to me after I sent my patch
and took a closer look at init_irq(). As you said, we move our problem
just to an earlier boot stage. Which wouldn't be true if a strict
if-else would allow us to explicitly call everything in the right order
- which init_irq by the way does.
I mentioned it in an email in the original thread yesterday. But since
there is a v2 now, I just thought to bring it up here now.

> 
> >I think it might be better to move the call to of_clk_init() down into
> >the else branch of the if-else.
> 
> Possibly, yes. But we could also unconditionally call of_clk_init() at
> the beginning of time_init() and call clocksource_of_init() at the end.
> That will make .init_time() to some fixup hook between
> initialization of clocks and timers.

Right, the question is what is the common case? What do SOCs use the
custom init_time() hook for?

If SOCs use it for things additionally to clock init where 
execution order doesn't matter, your patch works perfectly well and is
the preferred solution.

If they use it just to call of_clk_init() and of_clocksource_init(),
those hooks can go away and it would work in the else branch or outside,
equally well.

If they use it like Zynq, to do some required SOC specific init which must
be done before clock init, it would make more sense to have it in the
else branch. That way the SOC intentionally replaces init_time()
with its own implementation and has to take care of calling everything in the
right order.

Another approach might be to move the custom init_time() hook before the
call to of_clk_init().

As I said, it depends on how this hook is used.

	Sören

Re: [PATCH RFC v2 01/16] ARM: call clk_of_init from time_init

[Arnd Bergmann]

On Wednesday 28 August 2013, Sebastian Hesselbarth wrote:
> >
> > This forces zynq to move some initialization our clock code relies on to
> > init_irq(). Also, the current code already takes an approach of
> > doing either common init or machine specific init.
> 
> Soeren,
> 
> you know that patch 16/16 takes care of zynq's clock init?
> 
> It's your own patch you provided from the last RFC. Looking at it, it
> moves  zynq_sclr_init() to .init_irq and removes the call to
> of_clk_init() from zynq_clock_init() which is called by
> zynq_sclr_init().
> 
> Isn't that solving the above issues for mach-zynq?

Please be careful with the patch ordering here. The patch series should
be bisectable, i.e. no patch should ever knowingly break any of the
platforms, with the fix getting added in a later patch.

You should be able to do that by cleaning up all platforms to not
rely on ordering first, then add this patch, and finally remove
the other calls.

	Arnd

[PATCH RFC v2 03/16] ARM: exynos: remove custom .init_time hook

[Sebastian Hesselbarth]

With arch/arm calling of_clk_init(NULL) from time_init(), we can now
remove custom .init_time hooks.

Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
---
Cc: Kukjin Kim <kgene.kim@samsung.com>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: linux-samsung-soc@vger.kernel.org 
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---
 arch/arm/mach-exynos/common.c          |    7 -------
 arch/arm/mach-exynos/common.h          |    1 -
 arch/arm/mach-exynos/mach-exynos4-dt.c |    1 -
 arch/arm/mach-exynos/mach-exynos5-dt.c |    1 -
 4 files changed, 0 insertions(+), 10 deletions(-)

diff --git a/arch/arm/mach-exynos/common.c b/arch/arm/mach-exynos/common.c
index ba95e5d..587625b 100644
--- a/arch/arm/mach-exynos/common.c
+++ b/arch/arm/mach-exynos/common.c
@@ -27,7 +27,6 @@
 #include <linux/irqdomain.h>
 #include <linux/of_address.h>
 #include <linux/clocksource.h>
-#include <linux/clk-provider.h>
 #include <linux/irqchip/arm-gic.h>
 #include <linux/irqchip/chained_irq.h>
 
@@ -367,12 +366,6 @@ static void __init exynos5_map_io(void)
 		iotable_init(exynos5250_iodesc, ARRAY_SIZE(exynos5250_iodesc));
 }
 
-void __init exynos_init_time(void)
-{
-	of_clk_init(NULL);
-	clocksource_of_init();
-}
-
 struct bus_type exynos_subsys = {
 	.name		= "exynos-core",
 	.dev_name	= "exynos-core",
diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
index 8646a14..f0fa205 100644
--- a/arch/arm/mach-exynos/common.h
+++ b/arch/arm/mach-exynos/common.h
@@ -16,7 +16,6 @@
 #include <linux/of.h>
 
 void mct_init(void __iomem *base, int irq_g0, int irq_l0, int irq_l1);
-void exynos_init_time(void);
 
 struct map_desc;
 void exynos_init_io(void);
diff --git a/arch/arm/mach-exynos/mach-exynos4-dt.c b/arch/arm/mach-exynos/mach-exynos4-dt.c
index 0099c6c..7458dd4 100644
--- a/arch/arm/mach-exynos/mach-exynos4-dt.c
+++ b/arch/arm/mach-exynos/mach-exynos4-dt.c
@@ -54,7 +54,6 @@ DT_MACHINE_START(EXYNOS4210_DT, "Samsung Exynos4 (Flattened Device Tree)")
 	.init_early	= exynos_firmware_init,
 	.init_machine	= exynos4_dt_machine_init,
 	.init_late	= exynos_init_late,
-	.init_time	= exynos_init_time,
 	.dt_compat	= exynos4_dt_compat,
 	.restart        = exynos4_restart,
 	.reserve	= exynos4_reserve,
diff --git a/arch/arm/mach-exynos/mach-exynos5-dt.c b/arch/arm/mach-exynos/mach-exynos5-dt.c
index f874b77..8829da2 100644
--- a/arch/arm/mach-exynos/mach-exynos5-dt.c
+++ b/arch/arm/mach-exynos/mach-exynos5-dt.c
@@ -76,7 +76,6 @@ DT_MACHINE_START(EXYNOS5_DT, "SAMSUNG EXYNOS5 (Flattened Device Tree)")
 	.map_io		= exynos_init_io,
 	.init_machine	= exynos5_dt_machine_init,
 	.init_late	= exynos_init_late,
-	.init_time	= exynos_init_time,
 	.dt_compat	= exynos5_dt_compat,
 	.restart        = exynos5_restart,
 	.reserve	= exynos5_reserve,
-- 
1.7.2.5