This patch adds the DB8500 PRCMU Timer driver as a clocksource
and as sched_clock.
Signed-off-by: Mattias Wallin <[email protected]>
Acked-by: Linus Walleij <[email protected]>
---
drivers/clocksource/Kconfig | 15 +++++
drivers/clocksource/Makefile | 1 +
drivers/clocksource/clksrc-db8500-prcmu.c | 89 +++++++++++++++++++++++++++++
include/linux/clksrc-db8500-prcmu.h | 17 ++++++
4 files changed, 122 insertions(+), 0 deletions(-)
create mode 100644 drivers/clocksource/clksrc-db8500-prcmu.c
create mode 100644 include/linux/clksrc-db8500-prcmu.h
diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index 96c9219..d7ee415 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -3,3 +3,18 @@ config CLKSRC_I8253
config CLKSRC_MMIO
bool
+
+config CLKSRC_DB8500_PRCMU
+ bool "Clocksource PRCMU Timer"
+ depends on MFD_DB8500_PRCMU
+ default y
+ help
+ Use the always on PRCMU Timer as clocksource
+
+config CLKSRC_DB8500_PRCMU_SCHED_CLOCK
+ bool "Clocksource PRCMU Timer sched_clock"
+ depends on (CLKSRC_DB8500_PRCMU && !NOMADIK_MTU_SCHED_CLOCK)
+ select HAVE_SCHED_CLOCK
+ default y
+ help
+ Use the always on PRCMU Timer as sched_clock
diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index b995942..f2308ba 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -8,3 +8,4 @@ obj-$(CONFIG_SH_TIMER_MTU2) += sh_mtu2.o
obj-$(CONFIG_SH_TIMER_TMU) += sh_tmu.o
obj-$(CONFIG_CLKSRC_I8253) += i8253.o
obj-$(CONFIG_CLKSRC_MMIO) += mmio.o
+obj-$(CONFIG_CLKSRC_DB8500_PRCMU) += clksrc-db8500-prcmu.o
diff --git a/drivers/clocksource/clksrc-db8500-prcmu.c b/drivers/clocksource/clksrc-db8500-prcmu.c
new file mode 100644
index 0000000..07a4d1a
--- /dev/null
+++ b/drivers/clocksource/clksrc-db8500-prcmu.c
@@ -0,0 +1,89 @@
+/*
+ * Copyright (C) ST-Ericsson SA 2011
+ *
+ * License Terms: GNU General Public License v2
+ * Author: Mattias Wallin <[email protected]> for ST-Ericsson
+ * sched_clock implementation is based on:
+ * plat-nomadik/timer.c Linus Walleij <[email protected]>
+ *
+ * Clocksource DB8500 PRCMU Timer
+ * The PRCMU in the DB8500 chip has 5 timers which
+ * are available in an always-on power domain.
+ * Timer 4 is dedicated to our always-on clock source.
+ */
+#include <linux/clockchips.h>
+#include <linux/clksrc-db8500-prcmu.h>
+#include <asm/sched_clock.h>
+#include <mach/setup.h>
+#include <mach/db8500-regs.h>
+#include <mach/hardware.h>
+
+#define RATE_32K (32768)
+
+#define TIMER_MODE_CONTINOUS (0x1)
+#define TIMER_DOWNCOUNT_VAL (0xffffffff)
+
+/* PRCMU Timer 4 */
+#define PRCMU_TIMER_4_REF (prcmu_base + 0x450)
+#define PRCMU_TIMER_4_DOWNCOUNT (prcmu_base + 0x454)
+#define PRCMU_TIMER_4_MODE (prcmu_base + 0x458)
+
+static __iomem void *prcmu_base;
+
+#define SCHED_CLOCK_MIN_WRAP (131072) /* 2^32 / 32768 */
+
+static cycle_t clksrc_db8500_prcmu_read(struct clocksource *cs)
+{
+ u32 count, count2;
+
+ do {
+ count = readl(PRCMU_TIMER_4_DOWNCOUNT);
+ count2 = readl(PRCMU_TIMER_4_DOWNCOUNT);
+ } while (count2 != count);
+
+ /* Negate because the timer is a decrementing counter */
+ return ~count;
+}
+
+static struct clocksource clocksource_db8500_prcmu = {
+ .name = "clksrc-db8500-prcmu-timer4",
+ .rating = 300,
+ .read = clksrc_db8500_prcmu_read,
+ .shift = 10,
+ .mask = CLOCKSOURCE_MASK(32),
+ .flags = CLOCK_SOURCE_IS_CONTINUOUS,
+};
+
+#ifdef CONFIG_CS_DB8500_PRCMU_SCHED_CLOCK
+static DEFINE_CLOCK_DATA(cd);
+
+static void notrace clksrc_db8500_prcmu_update_sched_clock(void)
+{
+ u32 cyc = clksrc_db8500_prcmu_read(&clocksource_db8500_prcmu);
+ update_sched_clock(&cd, cyc, (u32)~0);
+}
+#endif
+
+void __init clksrc_db8500_prcmu_init(void)
+{
+ prcmu_base = __io_address(U8500_PRCMU_BASE);
+
+ /*
+ * The A9 sub system expects the timer to be configured as
+ * a continous looping timer.
+ * The PRCMU should configure it but if it for some reason
+ * don't we do it here.
+ */
+ if (readl(PRCMU_TIMER_4_MODE) != TIMER_MODE_CONTINOUS) {
+ writel(TIMER_MODE_CONTINOUS, PRCMU_TIMER_4_MODE);
+ writel(TIMER_DOWNCOUNT_VAL, PRCMU_TIMER_4_REF);
+ }
+#ifdef CONFIG_CS_DB8500_PRCMU_SCHED_CLOCK
+ init_sched_clock(&cd, clksrc_db8500_prcmu_update_sched_clock,
+ 32, RATE_32K);
+#endif
+ clocksource_calc_mult_shift(&clocksource_db8500_prcmu,
+ RATE_32K, SCHED_CLOCK_MIN_WRAP);
+ clocksource_register(&clocksource_db8500_prcmu);
+}
+
diff --git a/include/linux/clksrc-db8500-prcmu.h b/include/linux/clksrc-db8500-prcmu.h
new file mode 100644
index 0000000..42b8587
--- /dev/null
+++ b/include/linux/clksrc-db8500-prcmu.h
@@ -0,0 +1,17 @@
+/*
+ * Copyright (C) ST-Ericsson SA 2011
+ *
+ * License Terms: GNU General Public License v2
+ * Author: Mattias Wallin <[email protected]>
+ *
+ */
+#ifndef __CLKSRC_DB8500_PRCMU_H
+#define __CLKSRC_DB8500_PRCMU_H
+
+#ifdef CONFIG_CLKSRC_DB8500_PRCMU
+void __init clksrc_db8500_prcmu_init(void);
+#else
+void __init clksrc_db8500_prcmu_init(void) {}
+#endif
+
+#endif
--
1.7.4.3
On Tue, 31 May 2011, Mattias Wallin wrote:
> +static cycle_t clksrc_db8500_prcmu_read(struct clocksource *cs)
> +{
> + u32 count, count2;
> +
> + do {
> + count = readl(PRCMU_TIMER_4_DOWNCOUNT);
> + count2 = readl(PRCMU_TIMER_4_DOWNCOUNT);
> + } while (count2 != count);
What's the point of this exercise ?
> + clocksource_calc_mult_shift(&clocksource_db8500_prcmu,
> + RATE_32K, SCHED_CLOCK_MIN_WRAP);
> + clocksource_register(&clocksource_db8500_prcmu);
Please use clocksource_register_hz()
> diff --git a/include/linux/clksrc-db8500-prcmu.h b/include/linux/clksrc-db8500-prcmu.h
> new file mode 100644
> index 0000000..42b8587
> --- /dev/null
> +++ b/include/linux/clksrc-db8500-prcmu.h
Huch, why needs this to be a separate head in include/linux ?
> @@ -0,0 +1,17 @@
> +/*
> + * Copyright (C) ST-Ericsson SA 2011
> + *
> + * License Terms: GNU General Public License v2
> + * Author: Mattias Wallin <[email protected]>
> + *
> + */
> +#ifndef __CLKSRC_DB8500_PRCMU_H
> +#define __CLKSRC_DB8500_PRCMU_H
> +
> +#ifdef CONFIG_CLKSRC_DB8500_PRCMU
> +void __init clksrc_db8500_prcmu_init(void);
> +#else
> +void __init clksrc_db8500_prcmu_init(void) {}
> +#endif
> +
> +#endif
> --
> 1.7.4.3
>
>
On 05/31/2011 12:38 PM, Thomas Gleixner wrote:
> On Tue, 31 May 2011, Mattias Wallin wrote:
>> +static cycle_t clksrc_db8500_prcmu_read(struct clocksource *cs)
>> +{
>> + u32 count, count2;
>> +
>> + do {
>> + count = readl(PRCMU_TIMER_4_DOWNCOUNT);
>> + count2 = readl(PRCMU_TIMER_4_DOWNCOUNT);
>> + } while (count2 != count);
>
> What's the point of this exercise ?
It is a software workaround for a HW bug. If the timer is read exactly
when the logic is in the process of flipping over to a new value you
will get wrong value (large deviations). The problem was found running
LTP tests reporting "Timer going backwards". Reading it two times will
take long enough for the timer to stabilise according to our HW guys
measurements.
I agree with you that it is odd enough to deserve a comment in the code.
I will comment it in next version.
>
>> + clocksource_calc_mult_shift(&clocksource_db8500_prcmu,
>> + RATE_32K, SCHED_CLOCK_MIN_WRAP);
>> + clocksource_register(&clocksource_db8500_prcmu);
>
> Please use clocksource_register_hz()
Ok.
>
>> diff --git a/include/linux/clksrc-db8500-prcmu.h b/include/linux/clksrc-db8500-prcmu.h
>> new file mode 100644
>> index 0000000..42b8587
>> --- /dev/null
>> +++ b/include/linux/clksrc-db8500-prcmu.h
>
> Huch, why needs this to be a separate head in include/linux ?
I will move the header file to mach-ux500/include/mach/. Ok?
(I have long time ago been thought to avoid include mach headers from
the drivers. But if I understand it correct it is not true any more and
in this case it certainly makes no sense since I break that rule myself.)
>
>> @@ -0,0 +1,17 @@
>> +/*
>> + * Copyright (C) ST-Ericsson SA 2011
>> + *
>> + * License Terms: GNU General Public License v2
>> + * Author: Mattias Wallin<[email protected]>
>> + *
>> + */
>> +#ifndef __CLKSRC_DB8500_PRCMU_H
>> +#define __CLKSRC_DB8500_PRCMU_H
>> +
>> +#ifdef CONFIG_CLKSRC_DB8500_PRCMU
>> +void __init clksrc_db8500_prcmu_init(void);
>> +#else
>> +void __init clksrc_db8500_prcmu_init(void) {}
>> +#endif
>> +
>> +#endif
>> --
>> 1.7.4.3
>>
>>
On Tue, May 31, 2011 at 7:00 PM, Mattias Wallin
<[email protected]> wrote:
> On 05/31/2011 12:38 PM, Thomas Gleixner wrote:
>>> +++ b/include/linux/clksrc-db8500-prcmu.h
>> Huch, why needs this to be a separate head in include/linux ?
>
> I will move the header file to mach-ux500/include/mach/. Ok?
>
> (I have long time ago been thought to avoid include mach headers from the
> drivers. But if I understand it correct it is not true any more and in this
> case it certainly makes no sense since I break that rule myself.)
Keeping it under include/linux is probably wise since we want drivers
OUT of the ARM tree, but I don't know if there is much consensus on that?
I was more understanding the question as why it was needed at all.
Does it work to register the clocksource with say a core_initcall()
and avoid the header altogether?
Else a header is needed since we need to call out from the platform timer
init function to this clocksource to register it.
Thanks,
Linus Walleij
On 05/31/2011 11:18 PM, Linus Walleij wrote:
> Does it work to register the clocksource with say a core_initcall()
> and avoid the header altogether?
I wanted the init to be called from the struct sys_timer init call.
Mainly because it is really early and it seems to be what others do but
if we now want to move things out of the platform code it might make
sense to remove that dependency.
I'll experiment with it during the day and see how it goes.
On Tue, May 31, 2011 at 11:18:25PM +0200, Linus Walleij wrote:
> Keeping it under include/linux is probably wise since we want drivers
> OUT of the ARM tree, but I don't know if there is much consensus on that?
Header files private to a driver should be alongside the driver. Look
at drivers/net or drivers/scsi for plenty of examples.
Just because its a header file does not mean it must be in some directory
with 'include' in its path. The only reason to put it in include/linux
is if it contains stuff which needs to be shared outside of the driver
(eg, with arches).
2011/6/1 Russell King - ARM Linux <[email protected]>:
> On Tue, May 31, 2011 at 11:18:25PM +0200, Linus Walleij wrote:
>> Keeping it under include/linux is probably wise since we want drivers
>> OUT of the ARM tree, but I don't know if there is much consensus on that?
>
> Header files private to a driver should be alongside the driver. ?Look
> at drivers/net or drivers/scsi for plenty of examples.
>
> Just because its a header file does not mean it must be in some directory
> with 'include' in its path. ?The only reason to put it in include/linux
> is if it contains stuff which needs to be shared outside of the driver
> (eg, with arches).
I agree. Right now I have a creepy feeling that many drivers are just
putting headers in <mach/foo.h> out of habit, when plain "foo.h" in
working dir or <linux/subsystem/foo.h> is what is really apropriate.
Yours,
Linus Walleij
On 06/01/2011 09:57 AM, Mattias Wallin wrote:
> I'll experiment with it during the day and see how it goes.
It did not work to have the clocksource init as a core_initcall.
I got an null pointer oops in sched_clock_poll.
The lauterbach trace showed that the sched_clock_update_fn was null
since the poll function was called before the init.
I'll go back to having it called from the platform's struct system_time
(like in patch 3/3) and place the include file in include/linux.
Thomas, are you ok with having it directly under include/linux or do you
want me to create a include/linux/clocksource/ directory and place it there?
On Wed, Jun 01, 2011 at 04:22:06PM +0200, Mattias Wallin wrote:
> On 06/01/2011 09:57 AM, Mattias Wallin wrote:
>> I'll experiment with it during the day and see how it goes.
> It did not work to have the clocksource init as a core_initcall.
> I got an null pointer oops in sched_clock_poll.
> The lauterbach trace showed that the sched_clock_update_fn was null
> since the poll function was called before the init.
sched_clock is supposed to be setup and initialized really early. Why
are you doing trying to move it soo late?
On 06/01/2011 04:25 PM, Russell King - ARM Linux wrote:
> sched_clock is supposed to be setup and initialized really early. Why
> are you doing trying to move it soo late?
Linus idea was to remove the dependency from mach-ux500/cpu.c where I
originally called the init function by using core_initcall.
Should I use pure_initcall or solve it some other way?
On 06/01/2011 04:31 PM, Mattias Wallin wrote:
> On 06/01/2011 04:25 PM, Russell King - ARM Linux wrote:
>> sched_clock is supposed to be setup and initialized really early. Why
>> are you doing trying to move it soo late?
> Linus idea was to remove the dependency from mach-ux500/cpu.c where I
> originally called the init function by using core_initcall.
>
> Should I use pure_initcall or solve it some other way?
pure_initcall does not work either.
2011/6/1 Mattias Wallin <[email protected]>:
> On 06/01/2011 04:25 PM, Russell King - ARM Linux wrote:
>>
>> sched_clock is supposed to be setup and initialized really early. ?Why
>> are you doing trying to move it soo late?
>
> Linus idea was to remove the dependency from mach-ux500/cpu.c where I
> originally called the init function by using core_initcall.
OK we conclude the header is needed for now methinks.
Linus Walleij