Re: [PATCH 1/3] ia64: convert to use clocksource code
On Wed, 2007-05-02 at 10:58 -0700, john stultz wrote: > On Fri, 2007-04-27 at 12:11 -0400, Peter Keilty wrote: > > Daniel Walker wrote: > > >On Fri, 2007-04-27 at 11:42 -0400, Peter Keilty wrote: > > >>>There is a read(), and a vread() did you modify the slow syscall path to > > >>>use the vread()? > > >>> > > >John mentioned that he thought fsys_mmio_ptr could be held in the vread > > >pointer. vread() is used in x86 for vsyscalls. It looks like you've used > > >the update_vsyscall() which is also used for vsyscalls. So vread could > > >also be used .. Have you considered that at all? > > > > > > > > No, but yes it can be done, overloading the meaning. > > Yea. I'm not really psyched about overloading the vread pointer's use. I > mentioned it could be done if the #ifdef was objected to, but it seems a > bit abusive. The #ifdef isn't great, but I think its something I can > live with for now. At least its explicit. Use of config options like that is a bad precedence I think, which is why I commented on it .. Since we have a vread pointer that exists already, and it's used a very similar purpose it seems like bloat to just add another pointer.. We could change the vread to be a plain void pointer, then let each architecture use it however they want. Daniel - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] ia64: convert to use clocksource code
On Fri, 2007-04-27 at 12:11 -0400, Peter Keilty wrote: > Daniel Walker wrote: > >On Fri, 2007-04-27 at 11:42 -0400, Peter Keilty wrote: > >>>There is a read(), and a vread() did you modify the slow syscall path to > >>>use the vread()? > >>> > >John mentioned that he thought fsys_mmio_ptr could be held in the vread > >pointer. vread() is used in x86 for vsyscalls. It looks like you've used > >the update_vsyscall() which is also used for vsyscalls. So vread could > >also be used .. Have you considered that at all? > > > > > No, but yes it can be done, overloading the meaning. Yea. I'm not really psyched about overloading the vread pointer's use. I mentioned it could be done if the #ifdef was objected to, but it seems a bit abusive. The #ifdef isn't great, but I think its something I can live with for now. At least its explicit. > It would need to change in the future if vread was needed. > I have no strong argument against using it. Yea. I'd hold off on that for now. thanks -john - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] ia64: convert to use clocksource code
On Fri, 2007-04-27 at 08:54 -0400, Peter Keilty wrote: > john stultz wrote: > >On Thu, 2007-04-26 at 16:26 -0400, Peter Keilty wrote: > >>From: Peter Keilty <[EMAIL PROTECTED]> > >> > >>Initial ia64 conversion to the generic timekeeping/clocksource code. > >> > >>Signed-off-by: Peter Keilty <[EMAIL PROTECTED]> > >>Signed-off-by: John Stultz <[EMAIL PROTECTED]> > >> > > Thanks so much for pushing this on! I suspect this patch needs to be > >updated a touch, as I'm not sure if it still compiles in light of recent > >changes... > > > You are correct, you need patch 3/3 for it to compile and run. > I did a patch to the orginal patch, thought that was correct thing to do. > But I can make a new patch 1 from the orginal of ours and my #3. > I would also make an update to the #2 patch from #3 for ntp correct change. > That would be just 2 patches then, the enable_ia64 and remove_interpolater. > What do you think? Yep. That sounds like the right path to me. Its a good idea to make sure your patch set compiles each step of the way, so later folks can properly bisect through it looking for other issues. thanks -john - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] ia64: convert to use clocksource code
On Fri, 2007-04-27 at 12:11 -0400, Peter Keilty wrote: > > > No, but yes it can be done, overloading the meaning. > It would need to change in the future if vread was needed. > I have no strong argument against using it. > Although we may still need the IA64 define, I removed 32bit read mmio and > if that is brought back the fast syscall patch call will need to have a > field in the > clocksource struct that would indicated that. Waiting on comments about > that... > John and discuss this awhile back felt it was not needed, may prove wrong. The vread field is just a pointer, which should be 64bits on ia64. Why do you mention 32bits above? The two variables similar enough .. Daniel - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] ia64: convert to use clocksource code
Daniel Walker wrote: On Fri, 2007-04-27 at 11:42 -0400, Peter Keilty wrote: There is a read(), and a vread() did you modify the slow syscall path to use the vread()? I miss type, read(). John mentioned that he thought fsys_mmio_ptr could be held in the vread pointer. vread() is used in x86 for vsyscalls. It looks like you've used the update_vsyscall() which is also used for vsyscalls. So vread could also be used .. Have you considered that at all? No, but yes it can be done, overloading the meaning. It would need to change in the future if vread was needed. I have no strong argument against using it. Although we may still need the IA64 define, I removed 32bit read mmio and if that is brought back the fast syscall patch call will need to have a field in the clocksource struct that would indicated that. Waiting on comments about that... John and discuss this awhile back felt it was not needed, may prove wrong. Daniel - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] ia64: convert to use clocksource code
On Fri, 2007-04-27 at 11:42 -0400, Peter Keilty wrote: > > > >There is a read(), and a vread() did you modify the slow syscall path to > >use the vread()? > > > > > I miss type, read(). John mentioned that he thought fsys_mmio_ptr could be held in the vread pointer. vread() is used in x86 for vsyscalls. It looks like you've used the update_vsyscall() which is also used for vsyscalls. So vread could also be used .. Have you considered that at all? Daniel - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] ia64: convert to use clocksource code
Daniel Walker wrote:
On Fri, 2007-04-27 at 10:38 -0400, Peter Keilty wrote:
Daniel Walker wrote:
On Thu, 2007-04-26 at 16:26 -0400, Peter Keilty wrote:
+.mask = (1LL << 40) - 1,
+.mult = 0, /*to be caluclated*/
+.shift = 16,
+.is_continuous = 1,
};
You should use CLOCKSOURCE_MASK() here ..
It is correct in patch 3, I believe.
There's another spot that should use CLOCKSOURE_MASK() in the hpet I
think .
Correct that was change in patch 3 also.
diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
index daa4940..a20b4d6 100644
--- a/include/linux/clocksource.h
+++ b/include/linux/clocksource.h
@@ -61,6 +61,9 @@ struct clocksource {
u32 shift;
unsigned long flags;
cycle_t (*vread)(void);
+#ifdef CONFIG_IA64
+ void *fsys_mmio_ptr;/* used by fsyscall asm code */
+#endif
Could you explain in detail why this is needed?
This ptr is needed to hold the mmio address to read the cycle value.
The fast ia64 path utilizies a special gate page which can allow user
code to execute small amount of kernel code, normal calling a function
not done and so the address is need. The fast syscall path executes on
user stack, between user/kernel state. And if the the fast path has to
fallback
to the slow syscall code the vread will be needed and used.
There is a read(), and a vread() did you modify the slow syscall path to
use the vread()?
I miss type, read().
Daniel
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] ia64: convert to use clocksource code
On Fri, 2007-04-27 at 10:38 -0400, Peter Keilty wrote:
> Daniel Walker wrote:
>
> >On Thu, 2007-04-26 at 16:26 -0400, Peter Keilty wrote:
> >
> >
> >
> >>+.mask = (1LL << 40) - 1,
> >>+.mult = 0, /*to be caluclated*/
> >>+.shift = 16,
> >>+.is_continuous = 1,
> >> };
> >>
> >>
> >
> >You should use CLOCKSOURCE_MASK() here ..
> >
> >
> >
> It is correct in patch 3, I believe.
There's another spot that should use CLOCKSOURE_MASK() in the hpet I
think .
> >
> >
> >>
> >>diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
> >>index daa4940..a20b4d6 100644
> >>--- a/include/linux/clocksource.h
> >>+++ b/include/linux/clocksource.h
> >>@@ -61,6 +61,9 @@ struct clocksource {
> >>u32 shift;
> >>unsigned long flags;
> >>cycle_t (*vread)(void);
> >>+#ifdef CONFIG_IA64
> >>+ void *fsys_mmio_ptr;/* used by fsyscall asm code */
> >>+#endif
> >>
> >>
> >
> >Could you explain in detail why this is needed?
> >
> >
> This ptr is needed to hold the mmio address to read the cycle value.
> The fast ia64 path utilizies a special gate page which can allow user
> code to execute small amount of kernel code, normal calling a function
> not done and so the address is need. The fast syscall path executes on
> user stack, between user/kernel state. And if the the fast path has to
> fallback
> to the slow syscall code the vread will be needed and used.
There is a read(), and a vread() did you modify the slow syscall path to
use the vread()?
Daniel
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] ia64: convert to use clocksource code
Daniel Walker wrote:
On Thu, 2007-04-26 at 16:26 -0400, Peter Keilty wrote:
+.mask = (1LL << 40) - 1,
+.mult = 0, /*to be caluclated*/
+.shift = 16,
+.is_continuous = 1,
};
You should use CLOCKSOURCE_MASK() here ..
It is correct in patch 3, I believe.
diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
index daa4940..a20b4d6 100644
--- a/include/linux/clocksource.h
+++ b/include/linux/clocksource.h
@@ -61,6 +61,9 @@ struct clocksource {
u32 shift;
unsigned long flags;
cycle_t (*vread)(void);
+#ifdef CONFIG_IA64
+ void *fsys_mmio_ptr;/* used by fsyscall asm code */
+#endif
Could you explain in detail why this is needed?
This ptr is needed to hold the mmio address to read the cycle value.
The fast ia64 path utilizies a special gate page which can allow user
code to execute small amount of kernel code, normal calling a function
not done and so the address is need. The fast syscall path executes on
user stack, between user/kernel state. And if the the fast path has to
fallback
to the slow syscall code the vread will be needed and used.
Hope this helps.
Daniel
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] ia64: convert to use clocksource code
Chris Wright wrote:
* Peter Keilty ([EMAIL PROTECTED]) wrote:
diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
index 6077300..35ad71f 100644
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -480,10 +480,12 @@ #endif
/* Get end time (ticks) */
t2 = inl(acpi_gbl_FADT.xpm_timer_block.address);
+#ifndef CONFIG_IA64
#ifdef CONFIG_GENERIC_TIME
/* TSC halts in C2, so notify users */
mark_tsc_unstable();
#endif
+#endif
Is this a better description of the dependency?
#if defined (CONFIG_GENERIC_TIME) && defined (CONFIG_X86_TSC)
yes, ok.
/* Re-enable interrupts */
local_irq_enable();
current_thread_info()->status |= TS_POLLING;
@@ -522,10 +524,12 @@ #endif
acpi_set_register(ACPI_BITREG_ARB_DISABLE, 0);
}
+#ifndef CONFIG_IA64
#ifdef CONFIG_GENERIC_TIME
/* TSC halts in C3, so notify users */
mark_tsc_unstable();
#endif
+#endif
ditto
/* Re-enable interrupts */
local_irq_enable();
current_thread_info()->status |= TS_POLLING;
diff --git a/drivers/char/hpet.c b/drivers/char/hpet.c
index 0be700f..5ea7d3e 100644
--- a/drivers/char/hpet.c
+++ b/drivers/char/hpet.c
@@ -29,6 +29,7 @@ #include
#include
#include
#include
+#include
#include
#include
@@ -51,8 +52,34 @@ #define HPET_DRIFT (500)
#define HPET_RANGE_SIZE 1024/* from HPET spec */
+#if BITS_PER_LONG == 64
+#definewrite_counter(V, MC)writeq(V, MC)
+#defineread_counter(MC)readq(MC)
+#else
+#definewrite_counter(V, MC)writel(V, MC)
+#defineread_counter(MC)readl(MC)
+#endif
+
static u32 hpet_nhpet, hpet_max_freq = HPET_USER_FREQ;
+static void __iomem *hpet_mc_ptr;
CodingStyle nit: we don't need all this _ptr...
+static cycle_t read_hpet(void)
+{
+ return (cycle_t)read_counter((void __iomem *)hpet_mc_ptr);
+}
+
+static struct clocksource clocksource_hpet = {
+.name = "hpet",
+.rating = 300,
+.read = read_hpet,
+.mask = 0xLL,
+.mult = 0, /*to be caluclated*/
+.shift = 10,
+.is_continuous = 1,
+};
+static struct clocksource *hpet_clocksource_p;
and _p naming.
+
/* A lock for concurrent access by app and isr hpet activity. */
static DEFINE_SPINLOCK(hpet_lock);
/* A lock for concurrent intermodule access to hpet and isr hpet activity. */
@@ -79,7 +106,7 @@ struct hpets {
struct hpets *hp_next;
struct hpet __iomem *hp_hpet;
unsigned long hp_hpet_phys;
- struct time_interpolator *hp_interpolator;
+ struct clocksource *hp_clocksource;
unsigned long long hp_tick_freq;
unsigned long hp_delta;
unsigned int hp_ntimer;
@@ -94,13 +121,6 @@ #define HPET_IE 0x0002 /* interrupt en
#define HPET_PERIODIC 0x0004
#define HPET_SHARED_IRQ 0x0008
-#if BITS_PER_LONG == 64
-#definewrite_counter(V, MC)writeq(V, MC)
-#defineread_counter(MC)readq(MC)
-#else
-#definewrite_counter(V, MC)writel(V, MC)
-#defineread_counter(MC)readl(MC)
-#endif
#ifndef readq
static inline unsigned long long readq(void __iomem *addr)
@@ -737,27 +757,6 @@ static ctl_table dev_root[] = {
static struct ctl_table_header *sysctl_header;
-static void hpet_register_interpolator(struct hpets *hpetp)
-{
-#ifdef CONFIG_TIME_INTERPOLATION
- struct time_interpolator *ti;
-
- ti = kzalloc(sizeof(*ti), GFP_KERNEL);
- if (!ti)
- return;
-
- ti->source = TIME_SOURCE_MMIO64;
- ti->shift = 10;
- ti->addr = &hpetp->hp_hpet->hpet_mc;
- ti->frequency = hpetp->hp_tick_freq;
- ti->drift = HPET_DRIFT;
- ti->mask = -1;
-
- hpetp->hp_interpolator = ti;
- register_time_interpolator(ti);
-#endif
-}
-
/*
* Adjustment for when arming the timer with
* initial conditions. That is, main counter
@@ -909,7 +908,14 @@ int hpet_alloc(struct hpet_data *hdp)
}
hpetp->hp_delta = hpet_calibrate(hpetp);
- hpet_register_interpolator(hpetp);
+
+ if (!hpet_clocksource_p) {
+ clocksource_hpet.fsys_mmio_ptr = hpet_mc_ptr =
&hpetp->hp_hpet->hpet_mc;
+ clocksource_hpet.mult = clocksource_hz2mult(hpetp->hp_tick_freq,
+ clocksource_hpet.shift);
+ clocksource_register(&clocksource_hpet);
+ hpet_clocksource_p = hpetp->hp_clocksource = &clocksource_hpet;
+ }
This looks like a change in behaviour for non-ia64. Now i386 and x86_64
will get this clocksource twice (and this one has a higher rating).
Doesn't look like this will even compile s
Re: [PATCH 1/3] ia64: convert to use clocksource code
john stultz wrote:
On Thu, 2007-04-26 at 16:26 -0400, Peter Keilty wrote:
From: Peter Keilty <[EMAIL PROTECTED]>
Initial ia64 conversion to the generic timekeeping/clocksource code.
Signed-off-by: Peter Keilty <[EMAIL PROTECTED]>
Signed-off-by: John Stultz <[EMAIL PROTECTED]>
Peter,
Thanks so much for pushing this on! I suspect this patch needs to be
updated a touch, as I'm not sure if it still compiles in light of recent
changes...
John,
You are correct, you need patch 3/3 for it to compile and run.
I did a patch to the orginal patch, thought that was correct thing to do.
But I can make a new patch 1 from the orginal of ours and my #3.
I would also make an update to the #2 patch from #3 for ntp correct change.
That would be just 2 patches then, the enable_ia64 and remove_interpolater.
What do you think?
diff --git a/arch/ia64/kernel/cyclone.c b/arch/ia64/kernel/cyclone.c
index e00b215..280383b 100644
--- a/arch/ia64/kernel/cyclone.c
+++ b/arch/ia64/kernel/cyclone.c
@@ -3,6 +3,7 @@ #include
#include
#include
#include
+#include
#include
/* IBM Summit (EXA) Cyclone counter code*/
@@ -18,13 +19,21 @@ void __init cyclone_setup(void)
use_cyclone = 1;
}
+static void __iomem *cyclone_mc_ptr;
-struct time_interpolator cyclone_interpolator = {
- .source = TIME_SOURCE_MMIO64,
- .shift =16,
- .frequency =CYCLONE_TIMER_FREQ,
- .drift =-100,
- .mask = (1LL << 40) - 1
+static cycle_t read_cyclone(void)
+{
+ return (cycle_t)readq((void __iomem *)cyclone_mc_ptr);
+}
+
+static struct clocksource clocksource_cyclone = {
+.name = "cyclone",
+.rating = 300,
+.read = read_cyclone,
+.mask = (1LL << 40) - 1,
Daniel Walker pointed out to me on IRC that CLOCKSOURCE_MASK() should
probably be used here.
+.mult = 0, /*to be caluclated*/
+.shift = 16,
+.is_continuous = 1,
.is_continuous no longer exists.
You want to use:
.flags = CLOCK_SOURCE_IS_CONTINUOUS
This holds for all the clocksources introduced.
thanks
-john
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] ia64: convert to use clocksource code
* Peter Keilty ([EMAIL PROTECTED]) wrote:
> diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
> index 6077300..35ad71f 100644
> --- a/drivers/acpi/processor_idle.c
> +++ b/drivers/acpi/processor_idle.c
> @@ -480,10 +480,12 @@ #endif
> /* Get end time (ticks) */
> t2 = inl(acpi_gbl_FADT.xpm_timer_block.address);
>
> +#ifndef CONFIG_IA64
> #ifdef CONFIG_GENERIC_TIME
> /* TSC halts in C2, so notify users */
> mark_tsc_unstable();
> #endif
> +#endif
Is this a better description of the dependency?
#if defined (CONFIG_GENERIC_TIME) && defined (CONFIG_X86_TSC)
> /* Re-enable interrupts */
> local_irq_enable();
> current_thread_info()->status |= TS_POLLING;
> @@ -522,10 +524,12 @@ #endif
> acpi_set_register(ACPI_BITREG_ARB_DISABLE, 0);
> }
>
> +#ifndef CONFIG_IA64
> #ifdef CONFIG_GENERIC_TIME
> /* TSC halts in C3, so notify users */
> mark_tsc_unstable();
> #endif
> +#endif
ditto
> /* Re-enable interrupts */
> local_irq_enable();
> current_thread_info()->status |= TS_POLLING;
> diff --git a/drivers/char/hpet.c b/drivers/char/hpet.c
> index 0be700f..5ea7d3e 100644
> --- a/drivers/char/hpet.c
> +++ b/drivers/char/hpet.c
> @@ -29,6 +29,7 @@ #include
> #include
> #include
> #include
> +#include
>
> #include
> #include
> @@ -51,8 +52,34 @@ #defineHPET_DRIFT (500)
>
> #define HPET_RANGE_SIZE 1024/* from HPET spec */
>
> +#if BITS_PER_LONG == 64
> +#define write_counter(V, MC)writeq(V, MC)
> +#define read_counter(MC)readq(MC)
> +#else
> +#define write_counter(V, MC)writel(V, MC)
> +#define read_counter(MC)readl(MC)
> +#endif
> +
> static u32 hpet_nhpet, hpet_max_freq = HPET_USER_FREQ;
>
> +static void __iomem *hpet_mc_ptr;
CodingStyle nit: we don't need all this _ptr...
> +static cycle_t read_hpet(void)
> +{
> + return (cycle_t)read_counter((void __iomem *)hpet_mc_ptr);
> +}
> +
> +static struct clocksource clocksource_hpet = {
> +.name = "hpet",
> +.rating = 300,
> +.read = read_hpet,
> +.mask = 0xLL,
> +.mult = 0, /*to be caluclated*/
> +.shift = 10,
> +.is_continuous = 1,
> +};
> +static struct clocksource *hpet_clocksource_p;
and _p naming.
> +
> /* A lock for concurrent access by app and isr hpet activity. */
> static DEFINE_SPINLOCK(hpet_lock);
> /* A lock for concurrent intermodule access to hpet and isr hpet activity. */
> @@ -79,7 +106,7 @@ struct hpets {
> struct hpets *hp_next;
> struct hpet __iomem *hp_hpet;
> unsigned long hp_hpet_phys;
> - struct time_interpolator *hp_interpolator;
> + struct clocksource *hp_clocksource;
> unsigned long long hp_tick_freq;
> unsigned long hp_delta;
> unsigned int hp_ntimer;
> @@ -94,13 +121,6 @@ #define HPET_IE 0x0002 /* interrupt en
> #define HPET_PERIODIC 0x0004
> #define HPET_SHARED_IRQ 0x0008
>
> -#if BITS_PER_LONG == 64
> -#define write_counter(V, MC)writeq(V, MC)
> -#define read_counter(MC)readq(MC)
> -#else
> -#define write_counter(V, MC)writel(V, MC)
> -#define read_counter(MC)readl(MC)
> -#endif
>
> #ifndef readq
> static inline unsigned long long readq(void __iomem *addr)
> @@ -737,27 +757,6 @@ static ctl_table dev_root[] = {
>
> static struct ctl_table_header *sysctl_header;
>
> -static void hpet_register_interpolator(struct hpets *hpetp)
> -{
> -#ifdef CONFIG_TIME_INTERPOLATION
> - struct time_interpolator *ti;
> -
> - ti = kzalloc(sizeof(*ti), GFP_KERNEL);
> - if (!ti)
> - return;
> -
> - ti->source = TIME_SOURCE_MMIO64;
> - ti->shift = 10;
> - ti->addr = &hpetp->hp_hpet->hpet_mc;
> - ti->frequency = hpetp->hp_tick_freq;
> - ti->drift = HPET_DRIFT;
> - ti->mask = -1;
> -
> - hpetp->hp_interpolator = ti;
> - register_time_interpolator(ti);
> -#endif
> -}
> -
> /*
> * Adjustment for when arming the timer with
> * initial conditions. That is, main counter
> @@ -909,7 +908,14 @@ int hpet_alloc(struct hpet_data *hdp)
> }
>
> hpetp->hp_delta = hpet_calibrate(hpetp);
> - hpet_register_interpolator(hpetp);
> +
> + if (!hpet_clocksource_p) {
> + clocksource_hpet.fsys_mmio_ptr = hpet_mc_ptr =
> &hpetp->hp_hpet->hpet_mc;
> + clocksource_hpet.mult = clocksource_hz2mult(hpetp->hp_tick_freq,
> + clocksource_hpet.shift);
> + clocksource_register(&clocksource_hpet);
> + hpet_clocksource_p = hpetp->hp_clocksource = &clocksource_hpet;
> + }
This looks like a change in behaviour for
Re: [PATCH 1/3] ia64: convert to use clocksource code
On Thu, 2007-04-26 at 16:26 -0400, Peter Keilty wrote:
> From: Peter Keilty <[EMAIL PROTECTED]>
>
> Initial ia64 conversion to the generic timekeeping/clocksource code.
>
> Signed-off-by: Peter Keilty <[EMAIL PROTECTED]>
> Signed-off-by: John Stultz <[EMAIL PROTECTED]>
Peter,
Thanks so much for pushing this on! I suspect this patch needs to be
updated a touch, as I'm not sure if it still compiles in light of recent
changes...
> diff --git a/arch/ia64/kernel/cyclone.c b/arch/ia64/kernel/cyclone.c
> index e00b215..280383b 100644
> --- a/arch/ia64/kernel/cyclone.c
> +++ b/arch/ia64/kernel/cyclone.c
> @@ -3,6 +3,7 @@ #include
> #include
> #include
> #include
> +#include
> #include
>
> /* IBM Summit (EXA) Cyclone counter code*/
> @@ -18,13 +19,21 @@ void __init cyclone_setup(void)
> use_cyclone = 1;
> }
>
> +static void __iomem *cyclone_mc_ptr;
>
> -struct time_interpolator cyclone_interpolator = {
> - .source = TIME_SOURCE_MMIO64,
> - .shift =16,
> - .frequency =CYCLONE_TIMER_FREQ,
> - .drift =-100,
> - .mask = (1LL << 40) - 1
> +static cycle_t read_cyclone(void)
> +{
> + return (cycle_t)readq((void __iomem *)cyclone_mc_ptr);
> +}
> +
> +static struct clocksource clocksource_cyclone = {
> +.name = "cyclone",
> +.rating = 300,
> +.read = read_cyclone,
> +.mask = (1LL << 40) - 1,
Daniel Walker pointed out to me on IRC that CLOCKSOURCE_MASK() should
probably be used here.
> +.mult = 0, /*to be caluclated*/
> +.shift = 16,
> +.is_continuous = 1,
.is_continuous no longer exists.
You want to use:
.flags = CLOCK_SOURCE_IS_CONTINUOUS
This holds for all the clocksources introduced.
thanks
-john
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] ia64: convert to use clocksource code
On Thu, 2007-04-26 at 16:26 -0400, Peter Keilty wrote:
> +.mask = (1LL << 40) - 1,
> +.mult = 0, /*to be caluclated*/
> +.shift = 16,
> +.is_continuous = 1,
> };
You should use CLOCKSOURCE_MASK() here ..
>
> diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
> index daa4940..a20b4d6 100644
> --- a/include/linux/clocksource.h
> +++ b/include/linux/clocksource.h
> @@ -61,6 +61,9 @@ struct clocksource {
> u32 shift;
> unsigned long flags;
> cycle_t (*vread)(void);
> +#ifdef CONFIG_IA64
> + void *fsys_mmio_ptr;/* used by fsyscall asm code */
> +#endif
Could you explain in detail why this is needed?
Daniel
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] ia64: convert to use clocksource code
On Thu, 2007-04-26 at 22:41 +0200, Sam Ravnborg wrote: > On Thu, Apr 26, 2007 at 04:26:32PM -0400, Peter Keilty wrote: > > From: Peter Keilty <[EMAIL PROTECTED]> > > > > Initial ia64 conversion to the generic timekeeping/clocksource code. > > > > Signed-off-by: Peter Keilty <[EMAIL PROTECTED]> > > Signed-off-by: John Stultz <[EMAIL PROTECTED]> > > The "Signed-off-by:" should reflect the order in which a path is processed > with the last submitter at the bottom of the list. > So if this patch came from John then he should be first in the list > and since the patch passes you and you submit it you should be in the bottom > of the list. > > And "Signed-off-by:" tell the path that the patch takes. It is not to be used > to let others say "I have seen it and I think the patch is ok". > For the latter we have "Acked-by:". Yea. Its a little odd in this case, because Peter sent me the signed off code, then I've made tweaks to it and signed it off, then peter picked that up and has made further improvements. So I don't think it is inaccurate, but I see how it could be confusing. Maybe Peter should add an extra signed-off so its more clear? thanks -john - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] ia64: convert to use clocksource code
On Thu, Apr 26, 2007 at 04:26:32PM -0400, Peter Keilty wrote: > From: Peter Keilty <[EMAIL PROTECTED]> > > Initial ia64 conversion to the generic timekeeping/clocksource code. > > Signed-off-by: Peter Keilty <[EMAIL PROTECTED]> > Signed-off-by: John Stultz <[EMAIL PROTECTED]> The "Signed-off-by:" should reflect the order in which a path is processed with the last submitter at the bottom of the list. So if this patch came from John then he should be first in the list and since the patch passes you and you submit it you should be in the bottom of the list. And "Signed-off-by:" tell the path that the patch takes. It is not to be used to let others say "I have seen it and I think the patch is ok". For the latter we have "Acked-by:". I did not look at the code so ne feedback there. Sam - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
