[PATCH]: tick-sched.c build fix

[PATCH]: tick-sched.c build fix

[David Miller]

While getting dynticks/hrtimers up on sparc64 I ran into
the following build problem.  If you use get_irq_regs() you
need to include asm/irq_regs.h

Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index 512a4a9..51556b9 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -21,6 +21,8 @@
 #include <linux/sched.h>
 #include <linux/tick.h>
 
+#include <asm/irq_regs.h>
+
 #include "tick-internal.h"
 
 /*

Re: [PATCH]: tick-sched.c build fix

[Thomas Gleixner]

On Sat, 2007-02-24 at 21:42 -0800, David Miller wrote:
> While getting dynticks/hrtimers up on sparc64 I ran into
> the following build problem.  If you use get_irq_regs() you
> need to include asm/irq_regs.h
> 
> Signed-off-by: David S. Miller <davem@davemloft.net>

Acked-by: Thomas Gleixner <tglx@linutronix.de>

> diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
> index 512a4a9..51556b9 100644
> --- a/kernel/time/tick-sched.c
> +++ b/kernel/time/tick-sched.c
> @@ -21,6 +21,8 @@
>  #include <linux/sched.h>
>  #include <linux/tick.h>
>  
> +#include <asm/irq_regs.h>
> +
>  #include "tick-internal.h"
>  
>  /*

Re: [PATCH]: tick-sched.c build fix

[David Miller]

From: Thomas Gleixner <tglx@linutronix.de>
Date: Sun, 25 Feb 2007 10:04:41 +0100

> On Sat, 2007-02-24 at 21:42 -0800, David Miller wrote:
> > While getting dynticks/hrtimers up on sparc64 I ran into
> > the following build problem.  If you use get_irq_regs() you
> > need to include asm/irq_regs.h
> > 
> > Signed-off-by: David S. Miller <davem@davemloft.net>
> 
> Acked-by: Thomas Gleixner <tglx@linutronix.de>

Thanks for reviewing Thomas, I'll push the two bug fixes I
posted, with your ACKs, to Linus.

Two things remaining:

1) The change of clocksource_done_booting() to an fs_initcall()
   really is necessary.  Could you please push that to Linus?

2) I got bit by shifting problems (again) on sparc64.

Let me elaborate about #2.  Nobody writing these timer drivers
should be concerned by all of these details, I think, they will
get it wrong just as I did.

In my case a shift of 32 was too high for TSC based clockevent
on Niagara.

What we really should do is provide a clockevent_init_multshift()
or whatever that takes ticks_per_sec as an argument and this
function figured out an appropriate shift and multiplier to use.
It could be as simple as:

void clockevent_set_multshift(struct clock_event_device *edev, unsigned long hz)
{
	u64 mult;
	int shift = 32;

	while (1) {
		mult = div_sc(hz, NSEC_PER_SEC, shift);
		if (mult && (mult >> 32UL) == 0UL)
			break;

		shift--;
	}

	edev->shift = shift;
	edev->mult = (u32) mult;
}

With this, nobody will get bit by problems again.  We should
provide something similar, if not identical, for clocksources
too.  Perhaps we could even start trying at initial shift values
larger than 32.

What do you think?

Another thing I noticed is that clock_event_device->mult is
declared as an unsigned long, but it is clearly limited to
being a u32 as it is used as the second argument to do_div().

BTW, that's how I hit the Niagara shift issue, divide by zero
in clockevent_delta2ns() :-)

Re: [PATCH]: tick-sched.c build fix

[David Miller]

From: David Miller <davem@davemloft.net>
Date: Mon, 26 Feb 2007 11:22:39 -0800 (PST)

> With this, nobody will get bit by problems again.  We should
> provide something similar, if not identical, for clocksources
> too.  Perhaps we could even start trying at initial shift values
> larger than 32.
> 
> What do you think?

Just for completeness, here is the suggested set of interfaces
as a patch.

diff --git a/include/linux/clockchips.h b/include/linux/clockchips.h
index 4ea7e7b..b35c06f 100644
--- a/include/linux/clockchips.h
+++ b/include/linux/clockchips.h
@@ -115,6 +115,8 @@ static inline unsigned long div_sc(unsigned long ticks, unsigned long nsec,
 /* Clock event layer functions */
 extern unsigned long clockevent_delta2ns(unsigned long latch,
 					 struct clock_event_device *evt);
+extern void clockevent_set_multshift(struct clock_event_device *edev,
+				     unsigned long hz);
 extern void clockevents_register_device(struct clock_event_device *dev);
 
 extern void clockevents_exchange_device(struct clock_event_device *old,
diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
index daa4940..cd0ed26 100644
--- a/include/linux/clocksource.h
+++ b/include/linux/clocksource.h
@@ -120,7 +120,7 @@ static inline u32 clocksource_khz2mult(u32 khz, u32 shift_constant)
  * frequency to a timsource multiplier, given the
  * clocksource shift value
  */
-static inline u32 clocksource_hz2mult(u32 hz, u32 shift_constant)
+static inline u64 clocksource_hz2mult(u32 hz, u32 shift_constant)
 {
 	/*  hz = cyc/(Billion ns)
 	 *  mult/2^shift  = ns/cyc
@@ -134,7 +134,7 @@ static inline u32 clocksource_hz2mult(u32 hz, u32 shift_constant)
 	tmp += hz/2; /* round for do_div */
 	do_div(tmp, hz);
 
-	return (u32)tmp;
+	return tmp;
 }
 
 /**
@@ -194,6 +194,8 @@ static inline void clocksource_calculate_interval(struct clocksource *c,
 }
 
 
+extern void clocksource_set_multshift(struct clocksource *cs, unsigned long hz);
+
 /* used to install a new clocksource */
 extern int clocksource_register(struct clocksource*);
 extern struct clocksource* clocksource_get_next(void);
diff --git a/kernel/time/clockevents.c b/kernel/time/clockevents.c
index 67932ea..9fd5aa3 100644
--- a/kernel/time/clockevents.c
+++ b/kernel/time/clockevents.c
@@ -50,6 +50,23 @@ unsigned long clockevent_delta2ns(unsigned long latch,
 	return (unsigned long) clc;
 }
 
+void clockevent_set_multshift(struct clock_event_device *edev, unsigned long hz)
+{
+	u64 mult;
+	int shift = 32;
+
+	while (1) {
+		mult = div_sc(hz, NSEC_PER_SEC, shift);
+		if (mult && (mult >> 32UL) == 0UL)
+			break;
+
+		shift--;
+	}
+
+	edev->shift = shift;
+	edev->mult = (u32) mult;
+}
+
 /**
  * clockevents_set_mode - set the operating mode of a clock event device
  * @dev:	device to modify
diff --git a/kernel/time/clocksource.c b/kernel/time/clocksource.c
index 193a079..1802dc8 100644
--- a/kernel/time/clocksource.c
+++ b/kernel/time/clocksource.c
@@ -181,6 +181,23 @@ static void clocksource_check_watchdog(struct clocksource *cs)
 }
 #endif
 
+void clocksource_set_multshift(struct clocksource *cs, unsigned long hz)
+{
+	u64 mult;
+	int shift = 16;
+
+	while (1) {
+		mult = clocksource_hz2mult(hz, shift);
+		if (mult && (mult >> 32UL) == 0UL)
+			break;
+
+		shift--;
+	}
+
+	cs->shift = shift;
+	cs->mult = (u32) mult;
+}
+
 /**
  * clocksource_get_next - Returns the selected clocksource
  *