All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1] clocksource: atmel-pit: register as a sched_clock
@ 2016-02-04 17:21 ` Romain Izard
  0 siblings, 0 replies; 6+ messages in thread
From: Romain Izard @ 2016-02-04 17:21 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel
  Cc: Nicolas Ferre, Boris Brezillon, Daniel Lezcano, Thomas Gleixner,
	Romain Izard

Register the counter of the Periodic Interval Timer as a possible source
for sched_clock. This provides a better precision than the jiffies-based
default.

Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>
---

To reduce overhead and cache consumption, sched_clock_register does not
take a void* argument, as it is the case when using interrupt handlers.
As a result, the signature of the function used to read the counter is
u64 (*) (void). It is thus necessary to use a static variable inside the
driver to keep a reference to the clocksource. It is a common pattern in
other existing clocksource drivers.

All existing Atmel SoCs that rely on the PIT only have a single instance
of the controller, and future SoCs should remain this way as additional
timers are provided in 'Timer Counter' controllers. Additional code
handling the case of multiple PIT instances would thus be dead code.

 drivers/clocksource/timer-atmel-pit.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/clocksource/timer-atmel-pit.c b/drivers/clocksource/timer-atmel-pit.c
index d911c5dca8f1..744e1afc4b69 100644
--- a/drivers/clocksource/timer-atmel-pit.c
+++ b/drivers/clocksource/timer-atmel-pit.c
@@ -21,6 +21,7 @@
 #include <linux/of_address.h>
 #include <linux/of_irq.h>
 #include <linux/slab.h>
+#include <linux/sched_clock.h>
 
 #define AT91_PIT_MR		0x00			/* Mode Register */
 #define AT91_PIT_PITIEN			BIT(25)			/* Timer Interrupt Enable */
@@ -174,6 +175,13 @@ static irqreturn_t at91sam926x_pit_interrupt(int irq, void *dev_id)
 	return IRQ_NONE;
 }
 
+static struct clocksource *pit_sched_clock;
+
+static u64 pit_sched_clock_read(void)
+{
+	return read_pit_clk(pit_sched_clock);
+}
+
 /*
  * Set up both clocksource and clockevent support.
  */
@@ -206,6 +214,9 @@ static void __init at91sam926x_pit_common_init(struct pit_data *data)
 	data->clksrc.flags = CLOCK_SOURCE_IS_CONTINUOUS;
 	clocksource_register_hz(&data->clksrc, pit_rate);
 
+	pit_sched_clock = &data->clksrc;
+	sched_clock_register(pit_sched_clock_read, bits, pit_rate);
+
 	/* Set up irq handler */
 	ret = request_irq(data->irq, at91sam926x_pit_interrupt,
 			  IRQF_SHARED | IRQF_TIMER | IRQF_IRQPOLL,
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v1] clocksource: atmel-pit: register as a sched_clock
@ 2016-02-04 17:21 ` Romain Izard
  0 siblings, 0 replies; 6+ messages in thread
From: Romain Izard @ 2016-02-04 17:21 UTC (permalink / raw)
  To: linux-arm-kernel

Register the counter of the Periodic Interval Timer as a possible source
for sched_clock. This provides a better precision than the jiffies-based
default.

Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>
---

To reduce overhead and cache consumption, sched_clock_register does not
take a void* argument, as it is the case when using interrupt handlers.
As a result, the signature of the function used to read the counter is
u64 (*) (void). It is thus necessary to use a static variable inside the
driver to keep a reference to the clocksource. It is a common pattern in
other existing clocksource drivers.

All existing Atmel SoCs that rely on the PIT only have a single instance
of the controller, and future SoCs should remain this way as additional
timers are provided in 'Timer Counter' controllers. Additional code
handling the case of multiple PIT instances would thus be dead code.

 drivers/clocksource/timer-atmel-pit.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/clocksource/timer-atmel-pit.c b/drivers/clocksource/timer-atmel-pit.c
index d911c5dca8f1..744e1afc4b69 100644
--- a/drivers/clocksource/timer-atmel-pit.c
+++ b/drivers/clocksource/timer-atmel-pit.c
@@ -21,6 +21,7 @@
 #include <linux/of_address.h>
 #include <linux/of_irq.h>
 #include <linux/slab.h>
+#include <linux/sched_clock.h>
 
 #define AT91_PIT_MR		0x00			/* Mode Register */
 #define AT91_PIT_PITIEN			BIT(25)			/* Timer Interrupt Enable */
@@ -174,6 +175,13 @@ static irqreturn_t at91sam926x_pit_interrupt(int irq, void *dev_id)
 	return IRQ_NONE;
 }
 
+static struct clocksource *pit_sched_clock;
+
+static u64 pit_sched_clock_read(void)
+{
+	return read_pit_clk(pit_sched_clock);
+}
+
 /*
  * Set up both clocksource and clockevent support.
  */
@@ -206,6 +214,9 @@ static void __init at91sam926x_pit_common_init(struct pit_data *data)
 	data->clksrc.flags = CLOCK_SOURCE_IS_CONTINUOUS;
 	clocksource_register_hz(&data->clksrc, pit_rate);
 
+	pit_sched_clock = &data->clksrc;
+	sched_clock_register(pit_sched_clock_read, bits, pit_rate);
+
 	/* Set up irq handler */
 	ret = request_irq(data->irq, at91sam926x_pit_interrupt,
 			  IRQF_SHARED | IRQF_TIMER | IRQF_IRQPOLL,
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v1] clocksource: atmel-pit: register as a sched_clock
  2016-02-04 17:21 ` Romain Izard
@ 2016-02-08 13:33   ` Daniel Lezcano
  -1 siblings, 0 replies; 6+ messages in thread
From: Daniel Lezcano @ 2016-02-08 13:33 UTC (permalink / raw)
  To: Nicolas Ferre, Boris Brezillon
  Cc: Romain Izard, linux-kernel, linux-arm-kernel, Thomas Gleixner

On 02/04/2016 06:21 PM, Romain Izard wrote:
> Register the counter of the Periodic Interval Timer as a possible source
> for sched_clock. This provides a better precision than the jiffies-based
> default.
>
> Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>

Nicolas, Boris,

Are you ok with this patch ?

[ ... ]

-- 
  <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v1] clocksource: atmel-pit: register as a sched_clock
@ 2016-02-08 13:33   ` Daniel Lezcano
  0 siblings, 0 replies; 6+ messages in thread
From: Daniel Lezcano @ 2016-02-08 13:33 UTC (permalink / raw)
  To: linux-arm-kernel

On 02/04/2016 06:21 PM, Romain Izard wrote:
> Register the counter of the Periodic Interval Timer as a possible source
> for sched_clock. This provides a better precision than the jiffies-based
> default.
>
> Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>

Nicolas, Boris,

Are you ok with this patch ?

[ ... ]

-- 
  <http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v1] clocksource: atmel-pit: register as a sched_clock
  2016-02-08 13:33   ` Daniel Lezcano
@ 2016-02-08 14:06     ` Boris Brezillon
  -1 siblings, 0 replies; 6+ messages in thread
From: Boris Brezillon @ 2016-02-08 14:06 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: Nicolas Ferre, Romain Izard, linux-kernel, linux-arm-kernel,
	Thomas Gleixner, Alexandre Belloni

+ Alexandre

Hi Daniel,

On Mon, 8 Feb 2016 14:33:04 +0100
Daniel Lezcano <daniel.lezcano@linaro.org> wrote:

> On 02/04/2016 06:21 PM, Romain Izard wrote:
> > Register the counter of the Periodic Interval Timer as a possible source
> > for sched_clock. This provides a better precision than the jiffies-based
> > default.
> >
> > Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>
> 
> Nicolas, Boris,
> 
> Are you ok with this patch ?

Not sure yet :). On most platforms we are also declaring a clocksource
based on the TCB IP, and this one provides a better precision than the
PIT clocksource.
What happens if the PIT is de-activated while it's still supposed to
provide the sched_clock(). Is there a way to flag the clocksource as
still needed so that the core does not shut it down by when the TCB
clocksource comes in.

Best Regards,

Boris

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v1] clocksource: atmel-pit: register as a sched_clock
@ 2016-02-08 14:06     ` Boris Brezillon
  0 siblings, 0 replies; 6+ messages in thread
From: Boris Brezillon @ 2016-02-08 14:06 UTC (permalink / raw)
  To: linux-arm-kernel

+ Alexandre

Hi Daniel,

On Mon, 8 Feb 2016 14:33:04 +0100
Daniel Lezcano <daniel.lezcano@linaro.org> wrote:

> On 02/04/2016 06:21 PM, Romain Izard wrote:
> > Register the counter of the Periodic Interval Timer as a possible source
> > for sched_clock. This provides a better precision than the jiffies-based
> > default.
> >
> > Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>
> 
> Nicolas, Boris,
> 
> Are you ok with this patch ?

Not sure yet :). On most platforms we are also declaring a clocksource
based on the TCB IP, and this one provides a better precision than the
PIT clocksource.
What happens if the PIT is de-activated while it's still supposed to
provide the sched_clock(). Is there a way to flag the clocksource as
still needed so that the core does not shut it down by when the TCB
clocksource comes in.

Best Regards,

Boris

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-02-08 14:06 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-04 17:21 [PATCH v1] clocksource: atmel-pit: register as a sched_clock Romain Izard
2016-02-04 17:21 ` Romain Izard
2016-02-08 13:33 ` Daniel Lezcano
2016-02-08 13:33   ` Daniel Lezcano
2016-02-08 14:06   ` Boris Brezillon
2016-02-08 14:06     ` Boris Brezillon

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.