2015-04-30 10:44:47

by Sudeep Holla

[permalink] [raw]
Subject: [PATCH 0/2] ARM: move Dual-Timer SP804 driver to drivers/clocksource

Since there are ARM64 platforms(e.g. Juno) with SP804 timers, we need
to move the driver out of arch/arm so that the code can be shared.

Regards,
Sudeep

Sudeep Holla (2):
ARM: simplify timer initialisation and remove arm_timer.h inclusion
ARM: move Dual-Timer SP804 driver to drivers/clocksource

arch/arm/Kconfig | 5 -----
arch/arm/common/Makefile | 1 -
arch/arm/mach-integrator/integrator_ap.c | 1 -
arch/arm/mach-nspire/nspire.c | 2 --
arch/arm/mach-realview/core.c | 13 ++-----------
arch/arm/mach-versatile/core.c | 13 ++-----------
drivers/clocksource/Kconfig | 5 +++++
drivers/clocksource/Makefile | 1 +
drivers/clocksource/timer-integrator-ap.c | 3 ++-
.../hardware/arm_timer.h => drivers/clocksource/timer-sp.h | 5 -----
.../common/timer-sp.c => drivers/clocksource/timer-sp804.c | 7 ++++---
.../timer-sp.h => include/clocksource/timer-sp804.h | 4 ++++
12 files changed, 20 insertions(+), 40 deletions(-)
rename arch/arm/include/asm/hardware/arm_timer.h => drivers/clocksource/timer-sp.h (93%)
rename arch/arm/common/timer-sp.c => drivers/clocksource/timer-sp804.c (98%)
rename arch/arm/include/asm/hardware/timer-sp.h => include/clocksource/timer-sp804.h (90%)

--
1.9.1


2015-04-30 10:44:52

by Sudeep Holla

[permalink] [raw]
Subject: [PATCH 1/2] ARM: simplify timer initialisation and remove arm_timer.h inclusion

The header asm/hardware/arm_timer.h is included in various machine
specific files to access TIMER_CTRL and initialise to a known state.
However that's not required as the clock{source,event} driver timer-sp
initialises all the timer being used.

This patch removes the redundant code in timer_init and also the
inclusion of asm/hardware/arm_timer.h header.

Signed-off-by: Sudeep Holla <[email protected]>
Cc: Russell King <[email protected]>
Cc: Daniel Lezcano <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Olof Johansson <[email protected]>
---
arch/arm/mach-integrator/integrator_ap.c | 1 -
arch/arm/mach-realview/core.c | 9 ---------
arch/arm/mach-versatile/core.c | 10 ----------
3 files changed, 20 deletions(-)

diff --git a/arch/arm/mach-integrator/integrator_ap.c b/arch/arm/mach-integrator/integrator_ap.c
index 30003ba447a5..5b0e363fe5ba 100644
--- a/arch/arm/mach-integrator/integrator_ap.c
+++ b/arch/arm/mach-integrator/integrator_ap.c
@@ -37,7 +37,6 @@
#include <linux/stat.h>
#include <linux/termios.h>

-#include <asm/hardware/arm_timer.h>
#include <asm/setup.h>
#include <asm/param.h> /* HZ */
#include <asm/mach-types.h>
diff --git a/arch/arm/mach-realview/core.c b/arch/arm/mach-realview/core.c
index c309593abdb2..dd9c12d995db 100644
--- a/arch/arm/mach-realview/core.c
+++ b/arch/arm/mach-realview/core.c
@@ -38,7 +38,6 @@
#include <mach/hardware.h>
#include <asm/irq.h>
#include <asm/mach-types.h>
-#include <asm/hardware/arm_timer.h>
#include <asm/hardware/icst.h>

#include <asm/mach/arch.h>
@@ -378,14 +377,6 @@ void __init realview_timer_init(unsigned int timer_irq)
(REALVIEW_TIMCLK << REALVIEW_TIMER4_EnSel) | val,
__io_address(REALVIEW_SCTL_BASE));

- /*
- * Initialise to a known state (all timers off)
- */
- writel(0, timer0_va_base + TIMER_CTRL);
- writel(0, timer1_va_base + TIMER_CTRL);
- writel(0, timer2_va_base + TIMER_CTRL);
- writel(0, timer3_va_base + TIMER_CTRL);
-
sp804_clocksource_init(timer3_va_base, "timer3");
sp804_clockevents_init(timer0_va_base, timer_irq, "timer0");
}
diff --git a/arch/arm/mach-versatile/core.c b/arch/arm/mach-versatile/core.c
index 6ea09fe53426..a5a57e453698 100644
--- a/arch/arm/mach-versatile/core.c
+++ b/arch/arm/mach-versatile/core.c
@@ -42,7 +42,6 @@
#include <linux/reboot.h>

#include <asm/irq.h>
-#include <asm/hardware/arm_timer.h>
#include <asm/hardware/icst.h>
#include <asm/mach-types.h>

@@ -794,15 +793,6 @@ void __init versatile_init(void)
*/
void __init versatile_timer_init(void)
{
-
- /*
- * Initialise to a known state (all timers off)
- */
- writel(0, TIMER0_VA_BASE + TIMER_CTRL);
- writel(0, TIMER1_VA_BASE + TIMER_CTRL);
- writel(0, TIMER2_VA_BASE + TIMER_CTRL);
- writel(0, TIMER3_VA_BASE + TIMER_CTRL);
-
sp804_clocksource_init(TIMER3_VA_BASE, "timer3");
sp804_clockevents_init(TIMER0_VA_BASE, IRQ_TIMERINT0_1, "timer0");
}
--
1.9.1

2015-04-30 10:45:09

by Sudeep Holla

[permalink] [raw]
Subject: [PATCH 2/2] ARM: move Dual-Timer SP804 driver to drivers/clocksource

The ARM Dual-Timer SP804 module is peripheral found not only on ARM32
platforms but also on ARM64 platforms.

This patch moves the driver out of arch/arm to driver/clocksource
so that it can be used on ARM64 platforms also.

Signed-off-by: Sudeep Holla <[email protected]>
Cc: Russell King <[email protected]>
Cc: Daniel Lezcano <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: Olof Johansson <[email protected]>
---
arch/arm/Kconfig | 5 -----
arch/arm/common/Makefile | 1 -
arch/arm/mach-nspire/nspire.c | 2 --
arch/arm/mach-realview/core.c | 4 ++--
arch/arm/mach-versatile/core.c | 3 ++-
drivers/clocksource/Kconfig | 5 +++++
drivers/clocksource/Makefile | 1 +
drivers/clocksource/timer-integrator-ap.c | 3 ++-
.../asm/hardware/arm_timer.h => drivers/clocksource/timer-sp.h | 5 -----
arch/arm/common/timer-sp.c => drivers/clocksource/timer-sp804.c | 7 ++++---
.../asm/hardware/timer-sp.h => include/clocksource/timer-sp804.h | 4 ++++
11 files changed, 20 insertions(+), 20 deletions(-)
rename arch/arm/include/asm/hardware/arm_timer.h => drivers/clocksource/timer-sp.h (93%)
rename arch/arm/common/timer-sp.c => drivers/clocksource/timer-sp804.c (98%)
rename arch/arm/include/asm/hardware/timer-sp.h => include/clocksource/timer-sp804.h (90%)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 45df48ba0b12..3a1eb1bdac03 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -975,11 +975,6 @@ config PLAT_PXA
config PLAT_VERSATILE
bool

-config ARM_TIMER_SP804
- bool
- select CLKSRC_MMIO
- select CLKSRC_OF if OF
-
source "arch/arm/firmware/Kconfig"

source arch/arm/mm/Kconfig
diff --git a/arch/arm/common/Makefile b/arch/arm/common/Makefile
index 70b1eff477b3..6ee5959a813b 100644
--- a/arch/arm/common/Makefile
+++ b/arch/arm/common/Makefile
@@ -11,7 +11,6 @@ obj-$(CONFIG_SHARP_LOCOMO) += locomo.o
obj-$(CONFIG_SHARP_PARAM) += sharpsl_param.o
obj-$(CONFIG_SHARP_SCOOP) += scoop.o
obj-$(CONFIG_PCI_HOST_ITE8152) += it8152.o
-obj-$(CONFIG_ARM_TIMER_SP804) += timer-sp.o
obj-$(CONFIG_MCPM) += mcpm_head.o mcpm_entry.o mcpm_platsmp.o vlock.o
CFLAGS_REMOVE_mcpm_entry.o = -pg
AFLAGS_mcpm_head.o := -march=armv7-a
diff --git a/arch/arm/mach-nspire/nspire.c b/arch/arm/mach-nspire/nspire.c
index 3445a5686805..34c2a1b32e7d 100644
--- a/arch/arm/mach-nspire/nspire.c
+++ b/arch/arm/mach-nspire/nspire.c
@@ -22,8 +22,6 @@
#include <asm/mach-types.h>
#include <asm/mach/map.h>

-#include <asm/hardware/timer-sp.h>
-
#include "mmio.h"
#include "clcd.h"

diff --git a/arch/arm/mach-realview/core.c b/arch/arm/mach-realview/core.c
index dd9c12d995db..21e6088dce30 100644
--- a/arch/arm/mach-realview/core.c
+++ b/arch/arm/mach-realview/core.c
@@ -35,6 +35,8 @@
#include <linux/mtd/physmap.h>
#include <linux/memblock.h>

+#include <clocksource/timer-sp804.h>
+
#include <mach/hardware.h>
#include <asm/irq.h>
#include <asm/mach-types.h>
@@ -44,10 +46,8 @@
#include <asm/mach/irq.h>
#include <asm/mach/map.h>

-
#include <mach/platform.h>
#include <mach/irqs.h>
-#include <asm/hardware/timer-sp.h>

#include <plat/sched_clock.h>

diff --git a/arch/arm/mach-versatile/core.c b/arch/arm/mach-versatile/core.c
index a5a57e453698..79aaed6d8d8c 100644
--- a/arch/arm/mach-versatile/core.c
+++ b/arch/arm/mach-versatile/core.c
@@ -41,6 +41,8 @@
#include <linux/bitops.h>
#include <linux/reboot.h>

+#include <clocksource/timer-sp804.h>
+
#include <asm/irq.h>
#include <asm/hardware/icst.h>
#include <asm/mach-types.h>
@@ -51,7 +53,6 @@
#include <asm/mach/map.h>
#include <mach/hardware.h>
#include <mach/platform.h>
-#include <asm/hardware/timer-sp.h>

#include <plat/sched_clock.h>

diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index 51d7865fdddb..0f1c0e7f86da 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -132,6 +132,11 @@ config ARM_GLOBAL_TIMER
help
This options enables support for the ARM global timer unit

+config ARM_TIMER_SP804
+ bool "Support for Dual Timer SP804 module"
+ select CLKSRC_MMIO
+ select CLKSRC_OF if OF
+
config CLKSRC_ARM_GLOBAL_TIMER_SCHED_CLOCK
bool
depends on ARM_GLOBAL_TIMER
diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index 5b85f6adb258..5ca59f9b377f 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -45,6 +45,7 @@ obj-$(CONFIG_MTK_TIMER) += mtk_timer.o

obj-$(CONFIG_ARM_ARCH_TIMER) += arm_arch_timer.o
obj-$(CONFIG_ARM_GLOBAL_TIMER) += arm_global_timer.o
+obj-$(CONFIG_ARM_TIMER_SP804) += timer-sp804.o
obj-$(CONFIG_CLKSRC_METAG_GENERIC) += metag_generic.o
obj-$(CONFIG_ARCH_HAS_TICK_BROADCAST) += dummy_timer.o
obj-$(CONFIG_ARCH_KEYSTONE) += timer-keystone.o
diff --git a/drivers/clocksource/timer-integrator-ap.c b/drivers/clocksource/timer-integrator-ap.c
index b9efd30513d5..d7d21e4dcef0 100644
--- a/drivers/clocksource/timer-integrator-ap.c
+++ b/drivers/clocksource/timer-integrator-ap.c
@@ -26,7 +26,8 @@
#include <linux/clockchips.h>
#include <linux/interrupt.h>
#include <linux/sched_clock.h>
-#include <asm/hardware/arm_timer.h>
+
+#include "timer-sp.h"

static void __iomem * sched_clk_base;

diff --git a/arch/arm/include/asm/hardware/arm_timer.h b/drivers/clocksource/timer-sp.h
similarity index 93%
rename from arch/arm/include/asm/hardware/arm_timer.h
rename to drivers/clocksource/timer-sp.h
index d6030ff599db..050d88561e9c 100644
--- a/arch/arm/include/asm/hardware/arm_timer.h
+++ b/drivers/clocksource/timer-sp.h
@@ -1,6 +1,3 @@
-#ifndef __ASM_ARM_HARDWARE_ARM_TIMER_H
-#define __ASM_ARM_HARDWARE_ARM_TIMER_H
-
/*
* ARM timer implementation, found in Integrator, Versatile and Realview
* platforms. Not all platforms support all registers and bits in these
@@ -31,5 +28,3 @@
#define TIMER_RIS 0x10 /* CVR ro */
#define TIMER_MIS 0x14 /* CVR ro */
#define TIMER_BGLOAD 0x18 /* CVR rw */
-
-#endif
diff --git a/arch/arm/common/timer-sp.c b/drivers/clocksource/timer-sp804.c
similarity index 98%
rename from arch/arm/common/timer-sp.c
rename to drivers/clocksource/timer-sp804.c
index 19211324772f..f1141c227a61 100644
--- a/arch/arm/common/timer-sp.c
+++ b/drivers/clocksource/timer-sp804.c
@@ -1,5 +1,5 @@
/*
- * linux/arch/arm/common/timer-sp.c
+ * linux/drivers/clocksource/timer-sp.c
*
* Copyright (C) 1999 - 2003 ARM Limited
* Copyright (C) 2000 Deep Blue Solutions Ltd
@@ -30,8 +30,9 @@
#include <linux/of_irq.h>
#include <linux/sched_clock.h>

-#include <asm/hardware/arm_timer.h>
-#include <asm/hardware/timer-sp.h>
+#include <clocksource/timer-sp804.h>
+
+#include "timer-sp.h"

static long __init sp804_get_clock_rate(struct clk *clk)
{
diff --git a/arch/arm/include/asm/hardware/timer-sp.h b/include/clocksource/timer-sp804.h
similarity index 90%
rename from arch/arm/include/asm/hardware/timer-sp.h
rename to include/clocksource/timer-sp804.h
index bb28af7c32de..63bb88e183ca 100644
--- a/arch/arm/include/asm/hardware/timer-sp.h
+++ b/include/clocksource/timer-sp804.h
@@ -1,3 +1,6 @@
+#ifndef __CLKSOURCE_TIMER_SP_H
+#define __CLKSOURCE_TIMER_SP_H
+
struct clk;

void __sp804_clocksource_and_sched_clock_init(void __iomem *,
@@ -21,3 +24,4 @@ static inline void sp804_clockevents_init(void __iomem *base, unsigned int irq,
__sp804_clockevents_init(base, irq, NULL, name);

}
+#endif
--
1.9.1

2015-04-30 14:10:11

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH 1/2] ARM: simplify timer initialisation and remove arm_timer.h inclusion

On Thu, Apr 30, 2015 at 5:44 AM, Sudeep Holla <[email protected]> wrote:
> The header asm/hardware/arm_timer.h is included in various machine
> specific files to access TIMER_CTRL and initialise to a known state.
> However that's not required as the clock{source,event} driver timer-sp
> initialises all the timer being used.

I believe the idea is not to initialize the timers being used, but the
ones not being used and perhaps left running by the bootloader. Cases
where the interrupt is shared could cause a problem.

Rob

2015-04-30 14:19:07

by Sudeep Holla

[permalink] [raw]
Subject: Re: [PATCH 1/2] ARM: simplify timer initialisation and remove arm_timer.h inclusion



On 30/04/15 15:09, Rob Herring wrote:
> On Thu, Apr 30, 2015 at 5:44 AM, Sudeep Holla <[email protected]> wrote:
>> The header asm/hardware/arm_timer.h is included in various machine
>> specific files to access TIMER_CTRL and initialise to a known state.
>> However that's not required as the clock{source,event} driver timer-sp
>> initialises all the timer being used.
>
> I believe the idea is not to initialize the timers being used, but the
> ones not being used and perhaps left running by the bootloader. Cases
> where the interrupt is shared could cause a problem.
>

Ah OK, makes sense. I will wait for Russell to confirm. The main idea
was to keep the header file having offsets local to driver/clocksource
and avoid sharing it in include/linux but looks like that's not possible.

Regards,
Sudeep

2015-05-15 18:03:41

by Sudeep Holla

[permalink] [raw]
Subject: Re: [PATCH 1/2] ARM: simplify timer initialisation and remove arm_timer.h inclusion

Hi Russell,

On 30/04/15 15:19, Sudeep Holla wrote:
>
>
> On 30/04/15 15:09, Rob Herring wrote:
>> On Thu, Apr 30, 2015 at 5:44 AM, Sudeep Holla <[email protected]> wrote:
>>> The header asm/hardware/arm_timer.h is included in various machine
>>> specific files to access TIMER_CTRL and initialise to a known state.
>>> However that's not required as the clock{source,event} driver timer-sp
>>> initialises all the timer being used.
>>
>> I believe the idea is not to initialize the timers being used, but the
>> ones not being used and perhaps left running by the bootloader. Cases
>> where the interrupt is shared could cause a problem.
>>

Russell, can you confirm if that's the case ?

>
> Ah OK, makes sense. I will wait for Russell to confirm. The main idea
> was to keep the header file having offsets local to driver/clocksource
> and avoid sharing it in include/linux but looks like that's not possible.
>

Since we need this driver on ARM64, we might have to end up sharing the
header file with offsets if required for ARM platforms(though it would
be good to avoid it if there's any better alternative solution than that)

Regards,
Sudeep

2015-05-18 10:33:17

by Catalin Marinas

[permalink] [raw]
Subject: Re: [PATCH 1/2] ARM: simplify timer initialisation and remove arm_timer.h inclusion

On Fri, May 15, 2015 at 07:03:34PM +0100, Sudeep Holla wrote:
> On 30/04/15 15:19, Sudeep Holla wrote:
> >On 30/04/15 15:09, Rob Herring wrote:
> >>On Thu, Apr 30, 2015 at 5:44 AM, Sudeep Holla <[email protected]> wrote:
> >>>The header asm/hardware/arm_timer.h is included in various machine
> >>>specific files to access TIMER_CTRL and initialise to a known state.
> >>>However that's not required as the clock{source,event} driver timer-sp
> >>>initialises all the timer being used.
> >>
> >>I believe the idea is not to initialize the timers being used, but the
> >>ones not being used and perhaps left running by the bootloader. Cases
> >>where the interrupt is shared could cause a problem.
>
> Russell, can you confirm if that's the case ?

Unless you want to test all these platforms, I would suggest we assume
this is the case. The comments even state "Initialise to a known state
(all timers off)".

> >Ah OK, makes sense. I will wait for Russell to confirm. The main idea
> >was to keep the header file having offsets local to driver/clocksource
> >and avoid sharing it in include/linux but looks like that's not possible.
>
> Since we need this driver on ARM64, we might have to end up sharing the
> header file with offsets if required for ARM platforms(though it would
> be good to avoid it if there's any better alternative solution than that)

Can you not just move the definitions to
include/clocksource/timer-sp804.h and add some SP804_ prefix to avoid
name collisions?

--
Catalin

2015-05-18 10:39:01

by Sudeep Holla

[permalink] [raw]
Subject: Re: [PATCH 1/2] ARM: simplify timer initialisation and remove arm_timer.h inclusion



On 18/05/15 11:33, Catalin Marinas wrote:
> On Fri, May 15, 2015 at 07:03:34PM +0100, Sudeep Holla wrote:
>> On 30/04/15 15:19, Sudeep Holla wrote:
>>> On 30/04/15 15:09, Rob Herring wrote:
>>>> On Thu, Apr 30, 2015 at 5:44 AM, Sudeep Holla <[email protected]> wrote:
>>>>> The header asm/hardware/arm_timer.h is included in various machine
>>>>> specific files to access TIMER_CTRL and initialise to a known state.
>>>>> However that's not required as the clock{source,event} driver timer-sp
>>>>> initialises all the timer being used.
>>>>
>>>> I believe the idea is not to initialize the timers being used, but the
>>>> ones not being used and perhaps left running by the bootloader. Cases
>>>> where the interrupt is shared could cause a problem.
>>
>> Russell, can you confirm if that's the case ?
>
> Unless you want to test all these platforms, I would suggest we assume
> this is the case. The comments even state "Initialise to a known state
> (all timers off)".
>

OK makes sense.

>>> Ah OK, makes sense. I will wait for Russell to confirm. The main idea
>>> was to keep the header file having offsets local to driver/clocksource
>>> and avoid sharing it in include/linux but looks like that's not possible.
>>
>> Since we need this driver on ARM64, we might have to end up sharing the
>> header file with offsets if required for ARM platforms(though it would
>> be good to avoid it if there's any better alternative solution than that)
>
> Can you not just move the definitions to
> include/clocksource/timer-sp804.h and add some SP804_ prefix to avoid
> name collisions?
>

Yes I can do that and that's the alternate plan, wanted to avoid having
SP804 timer internal register offsets in that header file hoping to
contain those in the driver files if possible.

Regards,
Sudeep

2015-05-18 10:41:05

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [PATCH 1/2] ARM: simplify timer initialisation and remove arm_timer.h inclusion

On Thu, Apr 30, 2015 at 09:09:47AM -0500, Rob Herring wrote:
> On Thu, Apr 30, 2015 at 5:44 AM, Sudeep Holla <[email protected]> wrote:
> > The header asm/hardware/arm_timer.h is included in various machine
> > specific files to access TIMER_CTRL and initialise to a known state.
> > However that's not required as the clock{source,event} driver timer-sp
> > initialises all the timer being used.
>
> I believe the idea is not to initialize the timers being used, but the
> ones not being used and perhaps left running by the bootloader. Cases
> where the interrupt is shared could cause a problem.

Exactly. IMHO this code needs to stay.

--
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

2015-05-18 10:42:35

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [PATCH 1/2] ARM: simplify timer initialisation and remove arm_timer.h inclusion

On Thu, Apr 30, 2015 at 03:19:01PM +0100, Sudeep Holla wrote:
>
>
> On 30/04/15 15:09, Rob Herring wrote:
> >On Thu, Apr 30, 2015 at 5:44 AM, Sudeep Holla <[email protected]> wrote:
> >>The header asm/hardware/arm_timer.h is included in various machine
> >>specific files to access TIMER_CTRL and initialise to a known state.
> >>However that's not required as the clock{source,event} driver timer-sp
> >>initialises all the timer being used.
> >
> >I believe the idea is not to initialize the timers being used, but the
> >ones not being used and perhaps left running by the bootloader. Cases
> >where the interrupt is shared could cause a problem.
> >
>
> Ah OK, makes sense. I will wait for Russell to confirm. The main idea
> was to keep the header file having offsets local to driver/clocksource
> and avoid sharing it in include/linux but looks like that's not possible.

An alternative would be to have a new function, something like
sp804_disable() which takes the virtual address of the timer.
That'd still allow the platforms to disable all timers, but
without exposing the register stuff to them.

--
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

2015-05-18 10:43:35

by Sudeep Holla

[permalink] [raw]
Subject: Re: [PATCH 1/2] ARM: simplify timer initialisation and remove arm_timer.h inclusion

Hi Russell,

On 18/05/15 11:40, Russell King - ARM Linux wrote:
> On Thu, Apr 30, 2015 at 09:09:47AM -0500, Rob Herring wrote:
>> On Thu, Apr 30, 2015 at 5:44 AM, Sudeep Holla <[email protected]> wrote:
>>> The header asm/hardware/arm_timer.h is included in various machine
>>> specific files to access TIMER_CTRL and initialise to a known state.
>>> However that's not required as the clock{source,event} driver timer-sp
>>> initialises all the timer being used.
>>
>> I believe the idea is not to initialize the timers being used, but the
>> ones not being used and perhaps left running by the bootloader. Cases
>> where the interrupt is shared could cause a problem.
>
> Exactly. IMHO this code needs to stay.
>

Thanks for confirming.

Regards,
Sudeep

2015-05-18 10:44:36

by Sudeep Holla

[permalink] [raw]
Subject: Re: [PATCH 1/2] ARM: simplify timer initialisation and remove arm_timer.h inclusion


On 18/05/15 11:42, Russell King - ARM Linux wrote:
> On Thu, Apr 30, 2015 at 03:19:01PM +0100, Sudeep Holla wrote:
>>
>>
>> On 30/04/15 15:09, Rob Herring wrote:
>>> On Thu, Apr 30, 2015 at 5:44 AM, Sudeep Holla <[email protected]> wrote:
>>>> The header asm/hardware/arm_timer.h is included in various machine
>>>> specific files to access TIMER_CTRL and initialise to a known state.
>>>> However that's not required as the clock{source,event} driver timer-sp
>>>> initialises all the timer being used.
>>>
>>> I believe the idea is not to initialize the timers being used, but the
>>> ones not being used and perhaps left running by the bootloader. Cases
>>> where the interrupt is shared could cause a problem.
>>>
>>
>> Ah OK, makes sense. I will wait for Russell to confirm. The main idea
>> was to keep the header file having offsets local to driver/clocksource
>> and avoid sharing it in include/linux but looks like that's not possible.
>
> An alternative would be to have a new function, something like
> sp804_disable() which takes the virtual address of the timer.
> That'd still allow the platforms to disable all timers, but
> without exposing the register stuff to them.
>

Yes that's much better, will re-spin the patch accordingly.

Regards,
Sudeep

2015-05-18 12:45:00

by Sudeep Holla

[permalink] [raw]
Subject: [PATCH v2 0/2] ARM: move Dual-Timer SP804 driver to drivers/clocksource

Since there are ARM64 platforms(e.g. Juno) with SP804 timers, we need
to move the driver out of arch/arm so that the code can be shared.

Changes v1->v2:
- introduces sp804_timer_disable and retained the integrator
and realview timer initialisation code as suggested by Russell

v1: https://lkml.org/lkml/2015/4/30/225

Regards,
Sudeep

Sudeep Holla (2):
ARM: introduce sp804_timer_disable and remove arm_timer.h inclusion
ARM: move Dual-Timer SP804 driver to drivers/clocksource

arch/arm/Kconfig | 5 -----
arch/arm/common/Makefile | 1 -
arch/arm/mach-integrator/integrator_ap.c | 1 -
arch/arm/mach-nspire/nspire.c | 2 --
arch/arm/mach-realview/core.c | 13 ++++++-------
arch/arm/mach-versatile/core.c | 12 ++++++------
drivers/clocksource/Kconfig | 5 +++++
drivers/clocksource/Makefile | 1 +
drivers/clocksource/timer-integrator-ap.c | 3 ++-
.../hardware/arm_timer.h => drivers/clocksource/timer-sp.h | 5 -----
.../common/timer-sp.c => drivers/clocksource/timer-sp804.c | 12 +++++++++---
.../timer-sp.h => include/clocksource/timer-sp804.h | 5 +++++
12 files changed, 34 insertions(+), 31 deletions(-)
rename arch/arm/include/asm/hardware/arm_timer.h => drivers/clocksource/timer-sp.h (93%)
rename arch/arm/common/timer-sp.c => drivers/clocksource/timer-sp804.c (97%)
rename arch/arm/include/asm/hardware/timer-sp.h => include/clocksource/timer-sp804.h (85%)

--
1.9.1

2015-05-18 12:45:10

by Sudeep Holla

[permalink] [raw]
Subject: [PATCH v2 1/2] ARM: introduce sp804_timer_disable and remove arm_timer.h inclusion

The header asm/hardware/arm_timer.h is included in various machine
specific files to access TIMER_CTRL and initialise to a known state.

This patch introduces a new function sp804_timer_disable to disable
the SP804 timers and uses the same for initialising the timers to
known(off) state, thereby removing the dependency on the header
asm/hardware/arm_timer.h

This change is in prepartion to move sp804 timer support out of arch/arm
so that it can be used on ARM64 platforms.

Signed-off-by: Sudeep Holla <[email protected]>
Cc: Russell King <[email protected]>
Cc: Daniel Lezcano <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Olof Johansson <[email protected]>
---
arch/arm/common/timer-sp.c | 5 +++++
arch/arm/include/asm/hardware/timer-sp.h | 1 +
arch/arm/mach-integrator/integrator_ap.c | 1 -
arch/arm/mach-realview/core.c | 9 ++++-----
arch/arm/mach-versatile/core.c | 9 ++++-----
5 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/arch/arm/common/timer-sp.c b/arch/arm/common/timer-sp.c
index 19211324772f..000aea3722bc 100644
--- a/arch/arm/common/timer-sp.c
+++ b/arch/arm/common/timer-sp.c
@@ -71,6 +71,11 @@ static u64 notrace sp804_read(void)
return ~readl_relaxed(sched_clock_base + TIMER_VALUE);
}

+void __init sp804_timer_disable(void __iomem *base)
+{
+ writel(0, base + TIMER_CTRL);
+}
+
void __init __sp804_clocksource_and_sched_clock_init(void __iomem *base,
const char *name,
struct clk *clk,
diff --git a/arch/arm/include/asm/hardware/timer-sp.h b/arch/arm/include/asm/hardware/timer-sp.h
index bb28af7c32de..05eaefa46742 100644
--- a/arch/arm/include/asm/hardware/timer-sp.h
+++ b/arch/arm/include/asm/hardware/timer-sp.h
@@ -4,6 +4,7 @@ void __sp804_clocksource_and_sched_clock_init(void __iomem *,
const char *, struct clk *, int);
void __sp804_clockevents_init(void __iomem *, unsigned int,
struct clk *, const char *);
+void sp804_timer_disable(void __iomem *);

static inline void sp804_clocksource_init(void __iomem *base, const char *name)
{
diff --git a/arch/arm/mach-integrator/integrator_ap.c b/arch/arm/mach-integrator/integrator_ap.c
index 30003ba447a5..5b0e363fe5ba 100644
--- a/arch/arm/mach-integrator/integrator_ap.c
+++ b/arch/arm/mach-integrator/integrator_ap.c
@@ -37,7 +37,6 @@
#include <linux/stat.h>
#include <linux/termios.h>

-#include <asm/hardware/arm_timer.h>
#include <asm/setup.h>
#include <asm/param.h> /* HZ */
#include <asm/mach-types.h>
diff --git a/arch/arm/mach-realview/core.c b/arch/arm/mach-realview/core.c
index c309593abdb2..c611f489bdd2 100644
--- a/arch/arm/mach-realview/core.c
+++ b/arch/arm/mach-realview/core.c
@@ -38,7 +38,6 @@
#include <mach/hardware.h>
#include <asm/irq.h>
#include <asm/mach-types.h>
-#include <asm/hardware/arm_timer.h>
#include <asm/hardware/icst.h>

#include <asm/mach/arch.h>
@@ -381,10 +380,10 @@ void __init realview_timer_init(unsigned int timer_irq)
/*
* Initialise to a known state (all timers off)
*/
- writel(0, timer0_va_base + TIMER_CTRL);
- writel(0, timer1_va_base + TIMER_CTRL);
- writel(0, timer2_va_base + TIMER_CTRL);
- writel(0, timer3_va_base + TIMER_CTRL);
+ sp804_timer_disable(timer0_va_base);
+ sp804_timer_disable(timer1_va_base);
+ sp804_timer_disable(timer2_va_base);
+ sp804_timer_disable(timer3_va_base);

sp804_clocksource_init(timer3_va_base, "timer3");
sp804_clockevents_init(timer0_va_base, timer_irq, "timer0");
diff --git a/arch/arm/mach-versatile/core.c b/arch/arm/mach-versatile/core.c
index 6ea09fe53426..f98c1961be6a 100644
--- a/arch/arm/mach-versatile/core.c
+++ b/arch/arm/mach-versatile/core.c
@@ -42,7 +42,6 @@
#include <linux/reboot.h>

#include <asm/irq.h>
-#include <asm/hardware/arm_timer.h>
#include <asm/hardware/icst.h>
#include <asm/mach-types.h>

@@ -798,10 +797,10 @@ void __init versatile_timer_init(void)
/*
* Initialise to a known state (all timers off)
*/
- writel(0, TIMER0_VA_BASE + TIMER_CTRL);
- writel(0, TIMER1_VA_BASE + TIMER_CTRL);
- writel(0, TIMER2_VA_BASE + TIMER_CTRL);
- writel(0, TIMER3_VA_BASE + TIMER_CTRL);
+ sp804_timer_disable(TIMER0_VA_BASE);
+ sp804_timer_disable(TIMER1_VA_BASE);
+ sp804_timer_disable(TIMER2_VA_BASE);
+ sp804_timer_disable(TIMER3_VA_BASE);

sp804_clocksource_init(TIMER3_VA_BASE, "timer3");
sp804_clockevents_init(TIMER0_VA_BASE, IRQ_TIMERINT0_1, "timer0");
--
1.9.1

2015-05-18 12:45:29

by Sudeep Holla

[permalink] [raw]
Subject: [PATCH v2 2/2] ARM: move Dual-Timer SP804 driver to drivers/clocksource

The ARM Dual-Timer SP804 module is peripheral found not only on ARM32
platforms but also on ARM64 platforms.

This patch moves the driver out of arch/arm to driver/clocksource
so that it can be used on ARM64 platforms also.

Signed-off-by: Sudeep Holla <[email protected]>
Cc: Russell King <[email protected]>
Cc: Daniel Lezcano <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: Olof Johansson <[email protected]>
---
arch/arm/Kconfig | 5 -----
arch/arm/common/Makefile | 1 -
arch/arm/mach-nspire/nspire.c | 2 --
arch/arm/mach-realview/core.c | 4 ++--
arch/arm/mach-versatile/core.c | 3 ++-
drivers/clocksource/Kconfig | 5 +++++
drivers/clocksource/Makefile | 1 +
drivers/clocksource/timer-integrator-ap.c | 3 ++-
.../asm/hardware/arm_timer.h => drivers/clocksource/timer-sp.h | 5 -----
arch/arm/common/timer-sp.c => drivers/clocksource/timer-sp804.c | 7 ++++---
.../asm/hardware/timer-sp.h => include/clocksource/timer-sp804.h | 4 ++++
11 files changed, 20 insertions(+), 20 deletions(-)
rename arch/arm/include/asm/hardware/arm_timer.h => drivers/clocksource/timer-sp.h (93%)
rename arch/arm/common/timer-sp.c => drivers/clocksource/timer-sp804.c (98%)
rename arch/arm/include/asm/hardware/timer-sp.h => include/clocksource/timer-sp804.h (90%)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 45df48ba0b12..3a1eb1bdac03 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -975,11 +975,6 @@ config PLAT_PXA
config PLAT_VERSATILE
bool

-config ARM_TIMER_SP804
- bool
- select CLKSRC_MMIO
- select CLKSRC_OF if OF
-
source "arch/arm/firmware/Kconfig"

source arch/arm/mm/Kconfig
diff --git a/arch/arm/common/Makefile b/arch/arm/common/Makefile
index 70b1eff477b3..6ee5959a813b 100644
--- a/arch/arm/common/Makefile
+++ b/arch/arm/common/Makefile
@@ -11,7 +11,6 @@ obj-$(CONFIG_SHARP_LOCOMO) += locomo.o
obj-$(CONFIG_SHARP_PARAM) += sharpsl_param.o
obj-$(CONFIG_SHARP_SCOOP) += scoop.o
obj-$(CONFIG_PCI_HOST_ITE8152) += it8152.o
-obj-$(CONFIG_ARM_TIMER_SP804) += timer-sp.o
obj-$(CONFIG_MCPM) += mcpm_head.o mcpm_entry.o mcpm_platsmp.o vlock.o
CFLAGS_REMOVE_mcpm_entry.o = -pg
AFLAGS_mcpm_head.o := -march=armv7-a
diff --git a/arch/arm/mach-nspire/nspire.c b/arch/arm/mach-nspire/nspire.c
index 3445a5686805..34c2a1b32e7d 100644
--- a/arch/arm/mach-nspire/nspire.c
+++ b/arch/arm/mach-nspire/nspire.c
@@ -22,8 +22,6 @@
#include <asm/mach-types.h>
#include <asm/mach/map.h>

-#include <asm/hardware/timer-sp.h>
-
#include "mmio.h"
#include "clcd.h"

diff --git a/arch/arm/mach-realview/core.c b/arch/arm/mach-realview/core.c
index c611f489bdd2..44575edc44b1 100644
--- a/arch/arm/mach-realview/core.c
+++ b/arch/arm/mach-realview/core.c
@@ -35,6 +35,8 @@
#include <linux/mtd/physmap.h>
#include <linux/memblock.h>

+#include <clocksource/timer-sp804.h>
+
#include <mach/hardware.h>
#include <asm/irq.h>
#include <asm/mach-types.h>
@@ -44,10 +46,8 @@
#include <asm/mach/irq.h>
#include <asm/mach/map.h>

-
#include <mach/platform.h>
#include <mach/irqs.h>
-#include <asm/hardware/timer-sp.h>

#include <plat/sched_clock.h>

diff --git a/arch/arm/mach-versatile/core.c b/arch/arm/mach-versatile/core.c
index f98c1961be6a..23a04fe5d2ad 100644
--- a/arch/arm/mach-versatile/core.c
+++ b/arch/arm/mach-versatile/core.c
@@ -41,6 +41,8 @@
#include <linux/bitops.h>
#include <linux/reboot.h>

+#include <clocksource/timer-sp804.h>
+
#include <asm/irq.h>
#include <asm/hardware/icst.h>
#include <asm/mach-types.h>
@@ -51,7 +53,6 @@
#include <asm/mach/map.h>
#include <mach/hardware.h>
#include <mach/platform.h>
-#include <asm/hardware/timer-sp.h>

#include <plat/sched_clock.h>

diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index 51d7865fdddb..0f1c0e7f86da 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -132,6 +132,11 @@ config ARM_GLOBAL_TIMER
help
This options enables support for the ARM global timer unit

+config ARM_TIMER_SP804
+ bool "Support for Dual Timer SP804 module"
+ select CLKSRC_MMIO
+ select CLKSRC_OF if OF
+
config CLKSRC_ARM_GLOBAL_TIMER_SCHED_CLOCK
bool
depends on ARM_GLOBAL_TIMER
diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index 5b85f6adb258..5ca59f9b377f 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -45,6 +45,7 @@ obj-$(CONFIG_MTK_TIMER) += mtk_timer.o

obj-$(CONFIG_ARM_ARCH_TIMER) += arm_arch_timer.o
obj-$(CONFIG_ARM_GLOBAL_TIMER) += arm_global_timer.o
+obj-$(CONFIG_ARM_TIMER_SP804) += timer-sp804.o
obj-$(CONFIG_CLKSRC_METAG_GENERIC) += metag_generic.o
obj-$(CONFIG_ARCH_HAS_TICK_BROADCAST) += dummy_timer.o
obj-$(CONFIG_ARCH_KEYSTONE) += timer-keystone.o
diff --git a/drivers/clocksource/timer-integrator-ap.c b/drivers/clocksource/timer-integrator-ap.c
index b9efd30513d5..d7d21e4dcef0 100644
--- a/drivers/clocksource/timer-integrator-ap.c
+++ b/drivers/clocksource/timer-integrator-ap.c
@@ -26,7 +26,8 @@
#include <linux/clockchips.h>
#include <linux/interrupt.h>
#include <linux/sched_clock.h>
-#include <asm/hardware/arm_timer.h>
+
+#include "timer-sp.h"

static void __iomem * sched_clk_base;

diff --git a/arch/arm/include/asm/hardware/arm_timer.h b/drivers/clocksource/timer-sp.h
similarity index 93%
rename from arch/arm/include/asm/hardware/arm_timer.h
rename to drivers/clocksource/timer-sp.h
index d6030ff599db..050d88561e9c 100644
--- a/arch/arm/include/asm/hardware/arm_timer.h
+++ b/drivers/clocksource/timer-sp.h
@@ -1,6 +1,3 @@
-#ifndef __ASM_ARM_HARDWARE_ARM_TIMER_H
-#define __ASM_ARM_HARDWARE_ARM_TIMER_H
-
/*
* ARM timer implementation, found in Integrator, Versatile and Realview
* platforms. Not all platforms support all registers and bits in these
@@ -31,5 +28,3 @@
#define TIMER_RIS 0x10 /* CVR ro */
#define TIMER_MIS 0x14 /* CVR ro */
#define TIMER_BGLOAD 0x18 /* CVR rw */
-
-#endif
diff --git a/arch/arm/common/timer-sp.c b/drivers/clocksource/timer-sp804.c
similarity index 98%
rename from arch/arm/common/timer-sp.c
rename to drivers/clocksource/timer-sp804.c
index 000aea3722bc..ca02503f17d1 100644
--- a/arch/arm/common/timer-sp.c
+++ b/drivers/clocksource/timer-sp804.c
@@ -1,5 +1,5 @@
/*
- * linux/arch/arm/common/timer-sp.c
+ * linux/drivers/clocksource/timer-sp.c
*
* Copyright (C) 1999 - 2003 ARM Limited
* Copyright (C) 2000 Deep Blue Solutions Ltd
@@ -30,8 +30,9 @@
#include <linux/of_irq.h>
#include <linux/sched_clock.h>

-#include <asm/hardware/arm_timer.h>
-#include <asm/hardware/timer-sp.h>
+#include <clocksource/timer-sp804.h>
+
+#include "timer-sp.h"

static long __init sp804_get_clock_rate(struct clk *clk)
{
diff --git a/arch/arm/include/asm/hardware/timer-sp.h b/include/clocksource/timer-sp804.h
similarity index 90%
rename from arch/arm/include/asm/hardware/timer-sp.h
rename to include/clocksource/timer-sp804.h
index 05eaefa46742..1f8a1caa7cb4 100644
--- a/arch/arm/include/asm/hardware/timer-sp.h
+++ b/include/clocksource/timer-sp804.h
@@ -1,3 +1,6 @@
+#ifndef __CLKSOURCE_TIMER_SP804_H
+#define __CLKSOURCE_TIMER_SP804_H
+
struct clk;

void __sp804_clocksource_and_sched_clock_init(void __iomem *,
@@ -22,3 +25,4 @@ static inline void sp804_clockevents_init(void __iomem *base, unsigned int irq,
__sp804_clockevents_init(base, irq, NULL, name);

}
+#endif
--
1.9.1

2015-05-18 13:47:22

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [PATCH v2 0/2] ARM: move Dual-Timer SP804 driver to drivers/clocksource

On Mon, May 18, 2015 at 01:44:42PM +0100, Sudeep Holla wrote:
> Since there are ARM64 platforms(e.g. Juno) with SP804 timers, we need
> to move the driver out of arch/arm so that the code can be shared.
>
> Changes v1->v2:
> - introduces sp804_timer_disable and retained the integrator
> and realview timer initialisation code as suggested by Russell
>
> v1: https://lkml.org/lkml/2015/4/30/225

This looks fine to me, thanks.

Acked-by: Russell King <[email protected]>

--
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

2015-05-18 14:06:05

by Sudeep Holla

[permalink] [raw]
Subject: Re: [PATCH v2 0/2] ARM: move Dual-Timer SP804 driver to drivers/clocksource



On 18/05/15 14:47, Russell King - ARM Linux wrote:
> On Mon, May 18, 2015 at 01:44:42PM +0100, Sudeep Holla wrote:
>> Since there are ARM64 platforms(e.g. Juno) with SP804 timers, we need
>> to move the driver out of arch/arm so that the code can be shared.
>>
>> Changes v1->v2:
>> - introduces sp804_timer_disable and retained the integrator
>> and realview timer initialisation code as suggested by Russell
>>
>> v1: https://lkml.org/lkml/2015/4/30/225
>
> This looks fine to me, thanks.
>
> Acked-by: Russell King <[email protected]>
>

Thanks Russell, I assume you are fine if it goes via clocksource(i.e.
Daniel) tree.

Regards,
Sudeep

2015-05-18 14:34:35

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [PATCH v2 0/2] ARM: move Dual-Timer SP804 driver to drivers/clocksource

On Mon, 18 May 2015, Sudeep Holla wrote:

> Since there are ARM64 platforms(e.g. Juno) with SP804 timers, we need
> to move the driver out of arch/arm so that the code can be shared.
>
> Changes v1->v2:
> - introduces sp804_timer_disable and retained the integrator
> and realview timer initialisation code as suggested by Russell

Please route this through ARM with my ACK.

Thanks,

tglx

2015-05-18 15:32:58

by Sudeep Holla

[permalink] [raw]
Subject: Re: [PATCH v2 0/2] ARM: move Dual-Timer SP804 driver to drivers/clocksource

Hi Russell,

On 18/05/15 15:34, Thomas Gleixner wrote:
> On Mon, 18 May 2015, Sudeep Holla wrote:
>
>> Since there are ARM64 platforms(e.g. Juno) with SP804 timers, we need
>> to move the driver out of arch/arm so that the code can be shared.
>>
>> Changes v1->v2:
>> - introduces sp804_timer_disable and retained the integrator
>> and realview timer initialisation code as suggested by Russell
>
> Please route this through ARM with my ACK.
>

I have put the patches in your patch system with tglx ack
(8365/1 & 8366/1)

Regards,
Sudeep