[PATCH] minor cyclone cleanup

[PATCH] minor cyclone cleanup

[Bjorn Helgaas]

Trivial cleanup.  BTW, the ia64 cyclone code has no copyright or license.

There's also an arch/i386/kernel/timers/timer_cyclone.c that looks
suspiciously similar, and few if any actual architecture dependencies
in either the i386 or the ia64 files.  Why are they different?


Make cyclone_setup() take no args, rather than an ignored "char *"
and return void.  Fix typos and whitespace.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>

=== arch/ia64/Kconfig 1.76 vs edited ==--- 1.76/arch/ia64/Kconfig	2004-06-23 12:19:08 -06:00
+++ edited/arch/ia64/Kconfig	2004-07-29 10:48:05 -06:00
@@ -179,10 +179,10 @@
 	  See <file:Documentation/vm/numa> for more.
 
 config IA64_CYCLONE
-	bool "Support Cyclone(EXA) Time Source"
+	bool "Cyclone (EXA) Time Source support"
 	help
-		Say Y here to enable support for IBM EXA Cyclone time source.
-		If you're unsure, answer N.
+	  Say Y here to enable support for IBM EXA Cyclone time source.
+	  If you're unsure, answer N.
 
 config IOSAPIC
 	bool
=== arch/ia64/kernel/acpi.c 1.72 vs edited ==--- 1.72/arch/ia64/kernel/acpi.c	2004-07-15 15:41:29 -06:00
+++ edited/arch/ia64/kernel/acpi.c	2004-07-29 10:34:27 -06:00
@@ -293,20 +293,20 @@
 	return 0;
 }
 
-/* Hook from generic ACPI tables.c */
-void __init acpi_madt_oem_check(char *oem_id, char *oem_table_id)
+static void __init
+acpi_madt_oem_check (char *oem_id, char *oem_table_id)
 {
 	if (!strncmp(oem_id, "IBM", 3) &&
-	    (!strncmp(oem_table_id, "SERMOW", 6))){
+	    (!strncmp(oem_table_id, "SERMOW", 6))) {
 
-		/* Unfortunatly ITC_DRIFT is not yet part of the
+		/*
+		 * Unfortunately ITC_DRIFT is not yet part of the
 		 * official SAL spec, so the ITC_DRIFT bit is not
 		 * set by the BIOS on this hardware.
 		 */
 		sal_platform_features |= IA64_SAL_PLATFORM_FEATURE_ITC_DRIFT;
 
-		/*Start cyclone clock*/
-		cyclone_setup(0);
+		cyclone_setup();
 	}
 }
 
=== arch/ia64/kernel/cyclone.c 1.1 vs edited ==--- 1.1/arch/ia64/kernel/cyclone.c	2004-02-18 08:10:58 -07:00
+++ edited/arch/ia64/kernel/cyclone.c	2004-07-29 10:33:52 -06:00
@@ -10,10 +10,9 @@
 #define CYCLONE_TIMER_FREQ 100000000
 
 int use_cyclone;
-int __init cyclone_setup(char *str)
+void __init cyclone_setup(void)
 {
 	use_cyclone = 1;
-	return 1;
 }
 
 static u32* volatile cyclone_timer;	/* Cyclone MPMC0 register */
=== include/asm-ia64/cyclone.h 1.1 vs edited ==--- 1.1/include/asm-ia64/cyclone.h	2004-02-18 08:10:58 -07:00
+++ edited/include/asm-ia64/cyclone.h	2004-07-29 10:33:53 -06:00
@@ -3,10 +3,10 @@
 
 #ifdef	CONFIG_IA64_CYCLONE
 extern int use_cyclone;
-extern int __init cyclone_setup(char*);
+extern void __init cyclone_setup(void);
 #else	/* CONFIG_IA64_CYCLONE */
 #define use_cyclone 0
-static inline void cyclone_setup(char* s)
+static inline void cyclone_setup(void)
 {
 	printk(KERN_ERR "Cyclone Counter: System not configured"
 					" w/ CONFIG_IA64_CYCLONE.\n");

Re: [PATCH] minor cyclone cleanup

[john stultz]

On Thu, 2004-07-29 at 09:54, Bjorn Helgaas wrote:
> Trivial cleanup.  BTW, the ia64 cyclone code has no copyright or license.

Looks good to me! If a copyright/license message is necessary, I can
create a patch for you. 

> There's also an arch/i386/kernel/timers/timer_cyclone.c that looks
> suspiciously similar, and few if any actual architecture dependencies
> in either the i386 or the ia64 files.  Why are they different?

Indeed, they are very similar, but because the timeofday subsystems are
so different in each arch I implemented it separately. 

I hope to fix this in 2.7/2.6 (whatever) by merging all the arch
specific timeofday code into one arch independent implementation. Then
just a single time-source driver will be needed for both arches (ACPI PM
and HPET will additionally be able to be shared). 

Stay tuned! As soon as I finish reworking the NTP code bI'll be mailing
out code. 

> Make cyclone_setup() take no args, rather than an ignored "char *"
> and return void.  Fix typos and whitespace.

Apologies for the mess, thanks for cleaning that up!

thanks again, 
-john

Re: [PATCH] minor cyclone cleanup

[Bjorn Helgaas]

On Thursday 29 July 2004 11:02 am, john stultz wrote:
> Stay tuned! As soon as I finish reworking the NTP code bI'll be mailing
> out code. 

Ah, an NTP expert :-)  What do you think about adding an RTC driver
to NTP?  On our boxes (and I suspect many others), the system clock
is driven by  the interval timer.  But the RTC is much more accurate,
so it'd be nice if NTP knew how to use it.

The RTC is terribly slow and has low resolution, but in the absence of
anything better, it'd be nice if we could use it.

Is this possible?  Feasible?  I know the NTP guys would probably
laugh us out of the room, but not everybody has a fancy clock
attached to their box.

Re: [PATCH] minor cyclone cleanup

[john stultz]

On Thu, 2004-07-29 at 10:13, Bjorn Helgaas wrote:
> On Thursday 29 July 2004 11:02 am, john stultz wrote:
> > Stay tuned! As soon as I finish reworking the NTP code bI'll be mailing
> > out code. 
> 
> Ah, an NTP expert :-)  

Ha! Hardly. Although after looking at the kernel code, I'm starting to
wonder if anyone was ever. And even if I learn a thing or two about the
topic, I'm sure to disavow all knowledge for fear of being pigeon-holed
for the rest of my life ;)

> What do you think about adding an RTC driver
> to NTP?  On our boxes (and I suspect many others), the system clock
> is driven by  the interval timer.  But the RTC is much more accurate,
> so it'd be nice if NTP knew how to use it.

With my re-work, the system clock is driven from a time-source, not the
interval timer. That time-source is the only semi-arch-specific bit in
the code, so it can be the ITC, the cyclone counter, HPET, or even the
RTC. Its not all that different then ia64's time_interpolator
infrastructure, just cleaned up and simplified a bit. 

> The RTC is terribly slow and has low resolution, but in the absence of
> anything better, it'd be nice if we could use it.

Indeed, using the RTC for gettimeofday and friends would be quite
painful, and almost so low res as to be worthless, so I doubt you'd
really want to use it. 

> Is this possible?  Feasible?  I know the NTP guys would probably
> laugh us out of the room, but not everybody has a fancy clock
> attached to their box.

Another idea would be to make a userspace daemon who would read the RTC
and use the kernel's adjtimex() syscall to manipulate time. This would
be just like the userspace NTP daemon, only using the RTC instead of a
timeserver. Not having sub-second resolution would mean it would have to
be conservative and take quite some time to sync up, but it would be
possible. 

Whats wrong with using regular NTP, btw?

thanks
-john

Re: [PATCH] minor cyclone cleanup

[Bjorn Helgaas]

On Thursday 29 July 2004 12:37 pm, john stultz wrote:
> With my re-work, the system clock is driven from a time-source, not the
> interval timer. That time-source is the only semi-arch-specific bit in
> the code, so it can be the ITC, the cyclone counter, HPET, or even the
> RTC. Its not all that different then ia64's time_interpolator
> infrastructure, just cleaned up and simplified a bit. 

This might have possibilities.  Is there a way to compare the accuracy
of the ITC, HPET, and RTC, so we can pick the best one?  I know that
SAL_FREQ_BASE can tell us about the ITC and RTC, but I dunno about
the HPET.

> Another idea would be to make a userspace daemon who would read the RTC
> and use the kernel's adjtimex() syscall to manipulate time. This would
> be just like the userspace NTP daemon, only using the RTC instead of a
> timeserver. Not having sub-second resolution would mean it would have to
> be conservative and take quite some time to sync up, but it would be
> possible. 

This is what I had in mind, except that I was thinking about adding
RTC support to NTP rather than writing something from scratch.  NTP
has a lot of scary code that I don't want to replicate.