[03/12] clocksource: sti: Provide 'use timer as sched clock' capability

Message ID 1431005924-21777-4-git-send-email-lee.jones@linaro.org
State New
Headers show

Commit Message

Lee Jones May 7, 2015, 1:38 p.m.
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/clocksource/Kconfig         |  8 ++++++++
 drivers/clocksource/clksrc_st_lpc.c | 11 +++++++++++
 2 files changed, 19 insertions(+)

Comments

Lee Jones May 8, 2015, 9:36 a.m. | #1
On Fri, 08 May 2015, Paul Bolle wrote:
> On Thu, 2015-05-07 at 14:38 +0100, Lee Jones wrote:
> > --- a/drivers/clocksource/Kconfig
> > +++ b/drivers/clocksource/Kconfig
> 
> > +config CLKSRC_ST_LPC_TIMER_SCHED_CLOCK
> > +	bool
> > +	depends on CLKSRC_ST_LPC
> > +	default y
> > +	help
> > +	  Use Low Power controller timer clock source as sched_clock
> 
> This adds, as far as I can see, a Kconfig symbol that will always be in
> lockstep with CLKSRC_ST_LPC. Ie, CLKSRC_ST_LPC and
> CLKSRC_ST_LPC_TIMER_SCHED_CLOCK will always both be 'n' or both be 'y'.
> 
> Are there plans to break that lockstep in the future?

What's stopping CONFIG_CLKSRC_ST_LPC_TIMER_SCHED_CLOCK=n?
Lee Jones May 8, 2015, 10:29 a.m. | #2
On Fri, 08 May 2015, Paul Bolle wrote:

> On Fri, 2015-05-08 at 10:36 +0100, Lee Jones wrote:
> > On Fri, 08 May 2015, Paul Bolle wrote:
> > > On Thu, 2015-05-07 at 14:38 +0100, Lee Jones wrote:
> > > > --- a/drivers/clocksource/Kconfig
> > > > +++ b/drivers/clocksource/Kconfig
> > > 
> > > > +config CLKSRC_ST_LPC_TIMER_SCHED_CLOCK
> > > > +	bool
> > > > +	depends on CLKSRC_ST_LPC
> > > > +	default y
> > > > +	help
> > > > +	  Use Low Power controller timer clock source as sched_clock
> > > 
> > > This adds, as far as I can see, a Kconfig symbol that will always be in
> > > lockstep with CLKSRC_ST_LPC. Ie, CLKSRC_ST_LPC and
> > > CLKSRC_ST_LPC_TIMER_SCHED_CLOCK will always both be 'n' or both be 'y'.
> > > 
> > > Are there plans to break that lockstep in the future?
> > 
> > What's stopping CONFIG_CLKSRC_ST_LPC_TIMER_SCHED_CLOCK=n?
> 
> If CLKSRC_ST_LPC=y you mean? The lack of a prompt, I'd say. Ie, you
> would need
> 	bool "A very short description"
> 
> to be able to set these independently.
> 
> But now I notice that I missed that there appears to be a problem with
> CLKSRC_ST_LPC too. I'll reply to 2/12 shortly.

Ah, you mean from menuconfig.  I almost forgot what that was. ;)

I can add a menu option, no problem.
Daniel Lezcano May 11, 2015, 8:47 a.m. | #3
On 05/08/2015 01:47 PM, Paul Bolle wrote:
> On Fri, 2015-05-08 at 11:29 +0100, Lee Jones wrote:
>> Ah, you mean from menuconfig.  I almost forgot what that was. ;)
>>
>> I can add a menu option, no problem.
>
> It's not just menuconfig:
>      $ grep -e CONFIG_ARCH_STI -e CONFIG_CLKSRC_ST_LPC .config
>      CONFIG_ARCH_STI=y
>      CONFIG_CLKSRC_ST_LPC=y
>      CONFIG_CLKSRC_ST_LPC_TIMER_SCHED_CLOCK=y
>      $ vim .config
>      $ grep -e CONFIG_ARCH_STI -e CONFIG_CLKSRC_ST_LPC .config
>      CONFIG_ARCH_STI=y
>      # CONFIG_CLKSRC_ST_LPC is not set
>      # CONFIG_CLKSRC_ST_LPC_TIMER_SCHED_CLOCK is not set
>      $ make ARCH=arm silentoldconfig
>      scripts/kconfig/conf  --silentoldconfig Kconfig
>      #
>      # configuration written to .config
>      #
>      $ grep -e CONFIG_ARCH_STI -e CONFIG_CLKSRC_ST_LPC .config
>      CONFIG_ARCH_STI=y
>      CONFIG_CLKSRC_ST_LPC=y
>      CONFIG_CLKSRC_ST_LPC_TIMER_SCHED_CLOCK=y
>
> So I think that, if you want to be able to set
> CLKSRC_ST_LPC_TIMER_SCHED_CLOCK to 'n' even though CLKSRC_ST_LPC is 'y',
> you need to add a prompt. Or have you found a way around this?

The general policy is to have non-prompted options in the clocksource's 
Kconfig and let the platform's config to select the right components.

Patch

diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index ac424cf..218daf8 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -258,4 +258,12 @@  config CLKSRC_ST_LPC
 	help
 	  Enable this option to use the Low Power controller timer
 	  as clocksource.
+
+config CLKSRC_ST_LPC_TIMER_SCHED_CLOCK
+	bool
+	depends on CLKSRC_ST_LPC
+	default y
+	help
+	  Use Low Power controller timer clock source as sched_clock
+
 endmenu
diff --git a/drivers/clocksource/clksrc_st_lpc.c b/drivers/clocksource/clksrc_st_lpc.c
index 18a7dcd0..3d92b26 100644
--- a/drivers/clocksource/clksrc_st_lpc.c
+++ b/drivers/clocksource/clksrc_st_lpc.c
@@ -16,6 +16,7 @@ 
 #include <linux/clocksource.h>
 #include <linux/init.h>
 #include <linux/of_address.h>
+#include <linux/sched_clock.h>
 #include <linux/slab.h>
 
 #include <dt-bindings/mfd/st-lpc.h>
@@ -38,6 +39,13 @@  static void st_clksrc_reset(void)
 	writel_relaxed(1, ddata.base + LPC_LPT_START_OFF);
 }
 
+#ifdef CONFIG_CLKSRC_ST_LPC_TIMER_SCHED_CLOCK
+static u64 notrace st_clksrc_sched_clock_read(void)
+{
+	return (u64)readl_relaxed(ddata.base + LPC_LPT_LSB_OFF);
+}
+#endif
+
 static int __init st_clksrc_init(void)
 {
 	unsigned long rate;
@@ -47,6 +55,9 @@  static int __init st_clksrc_init(void)
 
 	rate = clk_get_rate(ddata.clk);
 
+#ifdef CONFIG_CLKSRC_ST_LPC_TIMER_SCHED_CLOCK
+	sched_clock_register(st_clksrc_sched_clock_read, 32, rate);
+#endif
 	ret = clocksource_mmio_init(ddata.base + LPC_LPT_LSB_OFF,
 				    "clksrc-st-lpc", rate, 300, 32,
 				    clocksource_mmio_readl_up);