[PATCH 0/1] How to choose system timer?

[PATCH 0/1] How to choose system timer?

[Michal Simek]

Hi,

I have one design question about specifying system timer in DTS.
Our proposed solution is to specify system-timer in chosen
node and then Linux kernel probe it.

For example:
	chosen {
		bootargs = " console=ttyS0,115200 ";
		linux,stdout-path = "/plb@0/serial@83e00000";
		system-timer = <&xps_timer_0>;
	} ;

Is it correct solution? We would like to use 
standard solution and not to reinvent the wheel.
How others archs handle it? AFAIK you have internal cpu
timer which is used as system timer.

I have also attached the patch for microblaze.

Thanks,
Michal



*** BLURB HERE ***

Peter A. G. Crosthwaite (1):
  microblaze: Added fdt chosen capability for timer

 arch/microblaze/kernel/timer.c |   12 ++++++++----
 1 files changed, 8 insertions(+), 4 deletions(-)

-- 
1.7.5.4

[PATCH 0/1] How to choose system timer?

[Michal Simek]

Hi,

I have one design question about specifying system timer in DTS.
Our proposed solution is to specify system-timer in chosen
node and then Linux kernel probe it.

For example:
	chosen {
		bootargs = " console=ttyS0,115200 ";
		linux,stdout-path = "/plb@0/serial@83e00000";
		system-timer = <&xps_timer_0>;
	} ;

Is it correct solution? We would like to use 
standard solution and not to reinvent the wheel.
How others archs handle it? AFAIK you have internal cpu
timer which is used as system timer.

I have also attached the patch for microblaze to show probing.

Thanks,
Michal



*** BLURB HERE ***

Peter A. G. Crosthwaite (1):
  microblaze: Added fdt chosen capability for timer

 arch/microblaze/kernel/timer.c |   12 ++++++++----
 1 files changed, 8 insertions(+), 4 deletions(-)

-- 
1.7.5.4

[PATCH 1/1] microblaze: Added fdt chosen capability for timer

[Michal Simek]

From: "Peter A. G. Crosthwaite" <peter.crosthwaite-g5w7nrANp4BDPfheJLI6IQ@public.gmane.org>

This lets a dts author flag a particular timer in the system as the system timer.
If the chosen node contains a "system-timer=<&foo>" entry than that handle will
be used to determine the system timer. In no such entry exists then the first
found timer will be used (current behaviour).

Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite-g5w7nrANp4BDPfheJLI6IQ@public.gmane.org>
Signed-off-by: Michal Simek <monstr-pSz03upnqPeHXe+LvDLADg@public.gmane.org>
---
 arch/microblaze/kernel/timer.c |   12 ++++++++----
 1 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/arch/microblaze/kernel/timer.c b/arch/microblaze/kernel/timer.c
index 91804cd..c289412 100644
--- a/arch/microblaze/kernel/timer.c
+++ b/arch/microblaze/kernel/timer.c
@@ -267,11 +267,15 @@ void __init time_init(void)
 		NULL
 	};
 
-	for (i = 0; timer_list[i] != NULL; i++) {
+	prop = of_get_property(of_chosen, "system-timer", NULL);
+	if (prop)
+		timer = of_find_node_by_phandle(be32_to_cpup(prop));
+	else
+		printk(KERN_INFO "No chosen timer found, using default\n");
+
+	for (i = 0; timer_list[i] != NULL && !timer; i++)
 		timer = of_find_compatible_node(NULL, NULL, timer_list[i]);
-		if (timer)
-			break;
-	}
+
 	BUG_ON(!timer);
 
 	timer_baseaddr = be32_to_cpup(of_get_property(timer, "reg", NULL));
-- 
1.7.5.4

Re: [PATCH 1/1] microblaze: Added fdt chosen capability for timer

[John Williams]

On Thu, Nov 10, 2011 at 11:10 PM, Michal Simek <monstr-pSz03upnqPeHXe+LvDLADg@public.gmane.org> wrote:
> From: "Peter A. G. Crosthwaite" <peter.crosthwaite-g5w7nrANp4BDPfheJLI6IQ@public.gmane.org>
>
> This lets a dts author flag a particular timer in the system as the system timer.
> If the chosen node contains a "system-timer=<&foo>" entry than that handle will
> be used to determine the system timer. In no such entry exists then the first
> found timer will be used (current behaviour).
>
> Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite-g5w7nrANp4BDPfheJLI6IQ@public.gmane.org>
> Signed-off-by: Michal Simek <monstr-pSz03upnqPeHXe+LvDLADg@public.gmane.org>
> ---
>  arch/microblaze/kernel/timer.c |   12 ++++++++----
>  1 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/arch/microblaze/kernel/timer.c b/arch/microblaze/kernel/timer.c
> index 91804cd..c289412 100644
> --- a/arch/microblaze/kernel/timer.c
> +++ b/arch/microblaze/kernel/timer.c
> @@ -267,11 +267,15 @@ void __init time_init(void)
>                NULL
>        };
>
> -       for (i = 0; timer_list[i] != NULL; i++) {
> +       prop = of_get_property(of_chosen, "system-timer", NULL);
> +       if (prop)
> +               timer = of_find_node_by_phandle(be32_to_cpup(prop));

Suggest expanding this code to check that the chosen timer meets the
kernel requirements - IRQ connected, dual channel etc.

If it fails, then perhaps fall through to finding the first suitable
one, or panic()?

> +       else
> +               printk(KERN_INFO "No chosen timer found, using default\n");
> +
> +       for (i = 0; timer_list[i] != NULL && !timer; i++)
>                timer = of_find_compatible_node(NULL, NULL, timer_list[i]);
> -               if (timer)
> -                       break;

There is another issue I think - can we somehow mark the system timer
instance as 'taken' so it can't be bound by another driver that might
be compatible (esp. generic-uio for example).

Currently if you mark the timer as generic-uio then a UIO app can mess
with it - not good.

John
--
PetaLogix - Linux Solutions for a Reconfigurable World
w: www.petalogix.com  p: +61-7-30090663  f: +61-7-30090663