[PATCH] clocksource/cadence_ttc: Reuse clocksource as sched_clock

[PATCH] clocksource/cadence_ttc: Reuse clocksource as sched_clock

[Soren Brinkmann]

Reuse the TTC clocksource timer as sched clock, too. Since only a single
sched clock is supported in Linux, this feature optional and can be
selected through Kconfig.

Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
---
 drivers/clocksource/Kconfig             |  7 +++++++
 drivers/clocksource/cadence_ttc_timer.c | 18 ++++++++++++++++++
 2 files changed, 25 insertions(+)

diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index 5871933..8dda767 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -36,6 +36,13 @@ config VT8500_TIMER
 config CADENCE_TTC_TIMER
 	bool
 
+config CADENCE_TTC_TIMER_SCHED_CLOCK
+	bool "Cadence TTC Sched Clock"
+	depends on CADENCE_TTC_TIMER
+	help
+	  Use the clocksource timer provided by the TTC as
+	  sched clock, also.
+
 config CLKSRC_NOMADIK_MTU
 	bool
 	depends on (ARCH_NOMADIK || ARCH_U8500)
diff --git a/drivers/clocksource/cadence_ttc_timer.c b/drivers/clocksource/cadence_ttc_timer.c
index 4cbe28c..ef0f52b 100644
--- a/drivers/clocksource/cadence_ttc_timer.c
+++ b/drivers/clocksource/cadence_ttc_timer.c
@@ -22,6 +22,7 @@
 #include <linux/of_irq.h>
 #include <linux/slab.h>
 #include <linux/clk-provider.h>
+#include <asm/sched_clock.h>
 
 /*
  * This driver configures the 2 16-bit count-up timers as follows:
@@ -95,6 +96,10 @@ struct ttc_timer_clockevent {
 #define to_ttc_timer_clkevent(x) \
 		container_of(x, struct ttc_timer_clockevent, ce)
 
+#ifdef CONFIG_CADENCE_TTC_TIMER_SCHED_CLOCK
+static void __iomem *ttc_sched_clock_val_reg;
+#endif
+
 /**
  * ttc_set_interval - Set the timer interval value
  *
@@ -156,6 +161,13 @@ static cycle_t __ttc_clocksource_read(struct clocksource *cs)
 				TTC_COUNT_VAL_OFFSET);
 }
 
+#ifdef CONFIG_CADENCE_TTC_TIMER_SCHED_CLOCK
+static u32 notrace ttc_sched_clock_read(void)
+{
+	return __raw_readl(ttc_sched_clock_val_reg);
+}
+#endif
+
 /**
  * ttc_set_next_event - Sets the time interval for next event
  *
@@ -297,6 +309,12 @@ static void __init ttc_setup_clocksource(struct clk *clk, void __iomem *base)
 		kfree(ttccs);
 		return;
 	}
+
+#ifdef CONFIG_CADENCE_TTC_TIMER_SCHED_CLOCK
+	ttc_sched_clock_val_reg = base + TTC_COUNT_VAL_OFFSET;
+	setup_sched_clock(ttc_sched_clock_read, 16,
+			clk_get_rate(ttccs->ttc.clk) / PRESCALE);
+#endif
 }
 
 static int ttc_rate_change_clockevent_cb(struct notifier_block *nb,
-- 
1.8.3.2

Re: [PATCH] clocksource/cadence_ttc: Reuse clocksource as sched_clock

[Baruch Siach]

Hi Soren,

On Wed, Jul 03, 2013 at 02:50:16PM -0700, Soren Brinkmann wrote:
> Reuse the TTC clocksource timer as sched clock, too. Since only a single
> sched clock is supported in Linux, this feature optional and can be
> selected through Kconfig.
> 
> Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
> ---

[...]

> diff --git a/drivers/clocksource/cadence_ttc_timer.c 
> b/drivers/clocksource/cadence_ttc_timer.c
> index 4cbe28c..ef0f52b 100644
> --- a/drivers/clocksource/cadence_ttc_timer.c
> +++ b/drivers/clocksource/cadence_ttc_timer.c
> @@ -22,6 +22,7 @@
>  #include <linux/of_irq.h>
>  #include <linux/slab.h>
>  #include <linux/clk-provider.h>
> +#include <asm/sched_clock.h>

This has changed to linux/sched_clock.h in the tip tree. See 
https://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/commit/?h=timers/core&id=38ff87f77af0b5a93fc8581cff1d6e5692ab8970.

baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch@tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -

Re: [PATCH] clocksource/cadence_ttc: Reuse clocksource as sched_clock

[Thomas Gleixner]

On Wed, 3 Jul 2013, Soren Brinkmann wrote:

> Reuse the TTC clocksource timer as sched clock, too. Since only a single
> sched clock is supported in Linux, this feature optional and can be
> selected through Kconfig.

This changelog doesn't make sense.

There can be only one active sched_clock, but that does no mean, that
you cannot have different implementations compiled in.

So if you disable this config which sched_clock is your kernel using?
And if you enable it, how is guaranteed that you end up with the ttc
sched_clock as the active one? Just due to initcall ordering?

Thanks,

	tglx

Re: [PATCH] clocksource/cadence_ttc: Reuse clocksource as sched_clock

[Sören Brinkmann]

On Fri, Jul 05, 2013 at 08:30:47AM +0200, Thomas Gleixner wrote:
> On Wed, 3 Jul 2013, Soren Brinkmann wrote:
> 
> > Reuse the TTC clocksource timer as sched clock, too. Since only a single
> > sched clock is supported in Linux, this feature optional and can be
> > selected through Kconfig.
> 
> This changelog doesn't make sense.
> 
> There can be only one active sched_clock, but that does no mean, that
> you cannot have different implementations compiled in.
> 
> So if you disable this config which sched_clock is your kernel using?
Jiffies

> And if you enable it, how is guaranteed that you end up with the ttc
> sched_clock as the active one? Just due to initcall ordering?
I assumed so. Is there a different mechanism?

	Sören

Re: [PATCH] clocksource/cadence_ttc: Reuse clocksource as sched_clock

[Thomas Gleixner]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1127 bytes --]

On Fri, 5 Jul 2013, Sören Brinkmann wrote:
> On Fri, Jul 05, 2013 at 08:30:47AM +0200, Thomas Gleixner wrote:
> > On Wed, 3 Jul 2013, Soren Brinkmann wrote:
> > 
> > > Reuse the TTC clocksource timer as sched clock, too. Since only a single
> > > sched clock is supported in Linux, this feature optional and can be
> > > selected through Kconfig.
> > 
> > This changelog doesn't make sense.
> > 
> > There can be only one active sched_clock, but that does no mean, that
> > you cannot have different implementations compiled in.
> > 
> > So if you disable this config which sched_clock is your kernel using?
> Jiffies
> 
> > And if you enable it, how is guaranteed that you end up with the ttc
> > sched_clock as the active one? Just due to initcall ordering?
> I assumed so. Is there a different mechanism?

jiffies is the default one. If you setup an explicit sched clock then
this is used. initcall ordering only matters if you have two possible
sched clocks which might replace jiffies. The one which gets
registered last wins.

So the question is, why you want to disable your sched clock at
compile time.

Thanks,

	tglx

Re: [PATCH] clocksource/cadence_ttc: Reuse clocksource as sched_clock

[Sören Brinkmann]

On Fri, Jul 05, 2013 at 06:05:14PM +0200, Thomas Gleixner wrote:
> On Fri, 5 Jul 2013, Sören Brinkmann wrote:
> > On Fri, Jul 05, 2013 at 08:30:47AM +0200, Thomas Gleixner wrote:
> > > On Wed, 3 Jul 2013, Soren Brinkmann wrote:
> > > 
> > > > Reuse the TTC clocksource timer as sched clock, too. Since only a single
> > > > sched clock is supported in Linux, this feature optional and can be
> > > > selected through Kconfig.
> > > 
> > > This changelog doesn't make sense.
> > > 
> > > There can be only one active sched_clock, but that does no mean, that
> > > you cannot have different implementations compiled in.
> > > 
> > > So if you disable this config which sched_clock is your kernel using?
> > Jiffies
> > 
> > > And if you enable it, how is guaranteed that you end up with the ttc
> > > sched_clock as the active one? Just due to initcall ordering?
> > I assumed so. Is there a different mechanism?
> 
> jiffies is the default one. If you setup an explicit sched clock then
> this is used. initcall ordering only matters if you have two possible
> sched clocks which might replace jiffies. The one which gets
> registered last wins.
> 
> So the question is, why you want to disable your sched clock at
> compile time.
The timer drivers I have seen unconditionally register themselves as
sched_clock. There does not seem to be a runtime mechanism to choose the
best one - I might miss it though.
I was thinking about this due to the arm_global_timer driver which has
been dicussed on lkml recently, which seemed to do it this way too. And since
that timer would be an alternative sched_clock for Zynq too, I thought I
follow the same approach for the TTC.

	Sören

Re: [PATCH] clocksource/cadence_ttc: Reuse clocksource as sched_clock

[Thomas Gleixner]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2205 bytes --]

On Fri, 5 Jul 2013, Sören Brinkmann wrote:
> On Fri, Jul 05, 2013 at 06:05:14PM +0200, Thomas Gleixner wrote:
> > On Fri, 5 Jul 2013, Sören Brinkmann wrote:
> > > On Fri, Jul 05, 2013 at 08:30:47AM +0200, Thomas Gleixner wrote:
> > > > On Wed, 3 Jul 2013, Soren Brinkmann wrote:
> > > > 
> > > > > Reuse the TTC clocksource timer as sched clock, too. Since only a single
> > > > > sched clock is supported in Linux, this feature optional and can be
> > > > > selected through Kconfig.
> > > > 
> > > > This changelog doesn't make sense.
> > > > 
> > > > There can be only one active sched_clock, but that does no mean, that
> > > > you cannot have different implementations compiled in.
> > > > 
> > > > So if you disable this config which sched_clock is your kernel using?
> > > Jiffies
> > > 
> > > > And if you enable it, how is guaranteed that you end up with the ttc
> > > > sched_clock as the active one? Just due to initcall ordering?
> > > I assumed so. Is there a different mechanism?
> > 
> > jiffies is the default one. If you setup an explicit sched clock then
> > this is used. initcall ordering only matters if you have two possible
> > sched clocks which might replace jiffies. The one which gets
> > registered last wins.
> > 
> > So the question is, why you want to disable your sched clock at
> > compile time.
> The timer drivers I have seen unconditionally register themselves as
> sched_clock. There does not seem to be a runtime mechanism to choose the
> best one - I might miss it though.
> I was thinking about this due to the arm_global_timer driver which has
> been dicussed on lkml recently, which seemed to do it this way too. And since
> that timer would be an alternative sched_clock for Zynq too, I thought I
> follow the same approach for the TTC.

Hmm, ok. So there is a generic one as well. If we end up with multiple
choices for that sched_clock, then there should be a mechanism to
avoid that #ifdeffery and have it runtime selected.

Having a setup call with a rating argument would be a first step. So
the one with the highest rating wins. If people want to have it
selectable from the kernel commandline, it would need some string
matching as well.

Thanks,

	tglx

Re: [PATCH] clocksource/cadence_ttc: Reuse clocksource as sched_clock

[Sören Brinkmann]

On Thu, Jul 04, 2013 at 06:30:26AM +0300, Baruch Siach wrote:
> Hi Soren,
> 
> On Wed, Jul 03, 2013 at 02:50:16PM -0700, Soren Brinkmann wrote:
> > Reuse the TTC clocksource timer as sched clock, too. Since only a single
> > sched clock is supported in Linux, this feature optional and can be
> > selected through Kconfig.
> > 
> > Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
> > ---
> 
> [...]
> 
> > diff --git a/drivers/clocksource/cadence_ttc_timer.c 
> > b/drivers/clocksource/cadence_ttc_timer.c
> > index 4cbe28c..ef0f52b 100644
> > --- a/drivers/clocksource/cadence_ttc_timer.c
> > +++ b/drivers/clocksource/cadence_ttc_timer.c
> > @@ -22,6 +22,7 @@
> >  #include <linux/of_irq.h>
> >  #include <linux/slab.h>
> >  #include <linux/clk-provider.h>
> > +#include <asm/sched_clock.h>
> 
> This has changed to linux/sched_clock.h in the tip tree. See 
> https://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/commit/?h=timers/core&id=38ff87f77af0b5a93fc8581cff1d6e5692ab8970.
Is that change going into Linus' tree during the 3.11 merge window? The
tip tree is missing some changes to the TTC which went in through the
armsoc tree.

	Sören

Re: [PATCH] clocksource/cadence_ttc: Reuse clocksource as sched_clock

[Thomas Gleixner]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1762 bytes --]

On Fri, 5 Jul 2013, Sören Brinkmann wrote:
> On Thu, Jul 04, 2013 at 06:30:26AM +0300, Baruch Siach wrote:
> > Hi Soren,
> > 
> > On Wed, Jul 03, 2013 at 02:50:16PM -0700, Soren Brinkmann wrote:
> > > Reuse the TTC clocksource timer as sched clock, too. Since only a single
> > > sched clock is supported in Linux, this feature optional and can be
> > > selected through Kconfig.
> > > 
> > > Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
> > > ---
> > 
> > [...]
> > 
> > > diff --git a/drivers/clocksource/cadence_ttc_timer.c 
> > > b/drivers/clocksource/cadence_ttc_timer.c
> > > index 4cbe28c..ef0f52b 100644
> > > --- a/drivers/clocksource/cadence_ttc_timer.c
> > > +++ b/drivers/clocksource/cadence_ttc_timer.c
> > > @@ -22,6 +22,7 @@
> > >  #include <linux/of_irq.h>
> > >  #include <linux/slab.h>
> > >  #include <linux/clk-provider.h>
> > > +#include <asm/sched_clock.h>
> > 
> > This has changed to linux/sched_clock.h in the tip tree. See 
> > https://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/commit/?h=timers/core&id=38ff87f77af0b5a93fc8581cff1d6e5692ab8970.
> Is that change going into Linus' tree during the 3.11 merge window? The
> tip tree is missing some changes to the TTC which went in through the
> armsoc tree.

Yes, this is about to be sent Linuswards. Can we please coordinate
that stuff. drivers/clocksource depend on tip timers/* branches and
any modification in some other tree is going to be doomed.

We have a mechanism for that in place, if stuff goes cross trees. One
of the trees provides a set of commit for the other tree to pull, so
we do not end up with merge dependencies and conflicts.

Why is clocksource stuff going through armsoc unless it contains
related modifications in arch/arm/

Grmbl.

	tglx

Re: [PATCH] clocksource/cadence_ttc: Reuse clocksource as sched_clock

[Sören Brinkmann]

On Fri, Jul 05, 2013 at 10:42:03PM +0200, Thomas Gleixner wrote:
> On Fri, 5 Jul 2013, Sören Brinkmann wrote:
> > On Thu, Jul 04, 2013 at 06:30:26AM +0300, Baruch Siach wrote:
> > > Hi Soren,
> > > 
> > > On Wed, Jul 03, 2013 at 02:50:16PM -0700, Soren Brinkmann wrote:
> > > > Reuse the TTC clocksource timer as sched clock, too. Since only a single
> > > > sched clock is supported in Linux, this feature optional and can be
> > > > selected through Kconfig.
> > > > 
> > > > Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
> > > > ---
> > > 
> > > [...]
> > > 
> > > > diff --git a/drivers/clocksource/cadence_ttc_timer.c 
> > > > b/drivers/clocksource/cadence_ttc_timer.c
> > > > index 4cbe28c..ef0f52b 100644
> > > > --- a/drivers/clocksource/cadence_ttc_timer.c
> > > > +++ b/drivers/clocksource/cadence_ttc_timer.c
> > > > @@ -22,6 +22,7 @@
> > > >  #include <linux/of_irq.h>
> > > >  #include <linux/slab.h>
> > > >  #include <linux/clk-provider.h>
> > > > +#include <asm/sched_clock.h>
> > > 
> > > This has changed to linux/sched_clock.h in the tip tree. See 
> > > https://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/commit/?h=timers/core&id=38ff87f77af0b5a93fc8581cff1d6e5692ab8970.
> > Is that change going into Linus' tree during the 3.11 merge window? The
> > tip tree is missing some changes to the TTC which went in through the
> > armsoc tree.
> 
> Yes, this is about to be sent Linuswards. Can we please coordinate
> that stuff. drivers/clocksource depend on tip timers/* branches and
> any modification in some other tree is going to be doomed.
> 
> We have a mechanism for that in place, if stuff goes cross trees. One
> of the trees provides a set of commit for the other tree to pull, so
> we do not end up with merge dependencies and conflicts.
> 
> Why is clocksource stuff going through armsoc unless it contains
> related modifications in arch/arm/
We overhauled Zynq's common clock code and migrated all Zynq drivers to
it - the TTC being one. I'm pretty sure you were on the CC list for that
change.
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=30e1e28598c2674c133148d8aec6d431d7acd314

	Sören

Re: [PATCH] clocksource/cadence_ttc: Reuse clocksource as sched_clock

[Thomas Gleixner]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 812 bytes --]

On Fri, 5 Jul 2013, Sören Brinkmann wrote:
> On Fri, Jul 05, 2013 at 10:42:03PM +0200, Thomas Gleixner wrote:
> > We have a mechanism for that in place, if stuff goes cross trees. One
> > of the trees provides a set of commit for the other tree to pull, so
> > we do not end up with merge dependencies and conflicts.
> > 
> > Why is clocksource stuff going through armsoc unless it contains
> > related modifications in arch/arm/
> We overhauled Zynq's common clock code and migrated all Zynq drivers to
> it - the TTC being one. I'm pretty sure you were on the CC list for that
> change.
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=30e1e28598c2674c133148d8aec6d431d7acd314
> 

That's fine as it has no dependencies on any of the clocksource core
changes AFAICT.

Thanks,

	tglx

Re: [PATCH] clocksource/cadence_ttc: Reuse clocksource as sched_clock

[Sören Brinkmann]

On Fri, Jul 05, 2013 at 10:59:53PM +0200, Thomas Gleixner wrote:
> On Fri, 5 Jul 2013, Sören Brinkmann wrote:
> > On Fri, Jul 05, 2013 at 10:42:03PM +0200, Thomas Gleixner wrote:
> > > We have a mechanism for that in place, if stuff goes cross trees. One
> > > of the trees provides a set of commit for the other tree to pull, so
> > > we do not end up with merge dependencies and conflicts.
> > > 
> > > Why is clocksource stuff going through armsoc unless it contains
> > > related modifications in arch/arm/
> > We overhauled Zynq's common clock code and migrated all Zynq drivers to
> > it - the TTC being one. I'm pretty sure you were on the CC list for that
> > change.
> > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=30e1e28598c2674c133148d8aec6d431d7acd314
> > 
> 
> That's fine as it has no dependencies on any of the clocksource core
> changes AFAICT.
Right, shouldn't be a big deal. Just for this patch, I currently have the
choice of working with the old TTC but the new clocksource core or the
other way around. But since everything is going to meet in Linus' tree
soon, I just wait a bit until tip/timers is merged.

	Sören

Re: [PATCH] clocksource/cadence_ttc: Reuse clocksource as sched_clock

[Sören Brinkmann]

On Fri, Jul 05, 2013 at 06:23:19PM +0200, Thomas Gleixner wrote:
> On Fri, 5 Jul 2013, Sören Brinkmann wrote:
> > On Fri, Jul 05, 2013 at 06:05:14PM +0200, Thomas Gleixner wrote:
> > > On Fri, 5 Jul 2013, Sören Brinkmann wrote:
> > > > On Fri, Jul 05, 2013 at 08:30:47AM +0200, Thomas Gleixner wrote:
> > > > > On Wed, 3 Jul 2013, Soren Brinkmann wrote:
> > > > > 
> > > > > > Reuse the TTC clocksource timer as sched clock, too. Since only a single
> > > > > > sched clock is supported in Linux, this feature optional and can be
> > > > > > selected through Kconfig.
> > > > > 
> > > > > This changelog doesn't make sense.
> > > > > 
> > > > > There can be only one active sched_clock, but that does no mean, that
> > > > > you cannot have different implementations compiled in.
> > > > > 
> > > > > So if you disable this config which sched_clock is your kernel using?
> > > > Jiffies
> > > > 
> > > > > And if you enable it, how is guaranteed that you end up with the ttc
> > > > > sched_clock as the active one? Just due to initcall ordering?
> > > > I assumed so. Is there a different mechanism?
> > > 
> > > jiffies is the default one. If you setup an explicit sched clock then
> > > this is used. initcall ordering only matters if you have two possible
> > > sched clocks which might replace jiffies. The one which gets
> > > registered last wins.
> > > 
> > > So the question is, why you want to disable your sched clock at
> > > compile time.
> > The timer drivers I have seen unconditionally register themselves as
> > sched_clock. There does not seem to be a runtime mechanism to choose the
> > best one - I might miss it though.
> > I was thinking about this due to the arm_global_timer driver which has
> > been dicussed on lkml recently, which seemed to do it this way too. And since
> > that timer would be an alternative sched_clock for Zynq too, I thought I
> > follow the same approach for the TTC.
> 
> Hmm, ok. So there is a generic one as well. If we end up with multiple
> choices for that sched_clock, then there should be a mechanism to
> avoid that #ifdeffery and have it runtime selected.
> 
> Having a setup call with a rating argument would be a first step. So
> the one with the highest rating wins. If people want to have it
> selectable from the kernel commandline, it would need some string
> matching as well.
I revisited the code:
My original reason for adding the Kconfig option was that I hit this
warning when I had multiple sched_clock providers:
	arch/arm/kernel/sched_clock.c:setup_sched_clock():
		WARN_ON(read_sched_clock != jiffy_sched_clock_read);

That code is gone in the lates Linux tree.

Furthermore the now used setup_sched_clock routine, only updates the
sched clock provider, if the new one is faster than the old one:
	if (cd.rate > rate)                                                      
                return;

Which is good enough in terms of a rating mechanism, at least for this case.

So, I'll remove all the #ifdefs and Kconfig stuff and send a v2.

	Sören