From: Thomas Gleixner on
On Fri, 7 May 2010, Jacob Pan wrote:

> From: Jacob Pan <jacob.jun.pan(a)intel.com>
>
> lapic timer calibration can be combined with tsc in platform specific
> calibration functions. if such calibration result is obtained early,
> we can skip the redundent calibration loops.

I'd rather move lapic calibration into TSC calibration in general as
we do the same thing twice for no good reason.

That needs some code restructuring, but that's worth it.

> Signed-off-by: Jacob Pan <jacob.jun.pan(a)intel.com>
> Signed-off-by: Jacob Pan <jacob.jun.pan(a)linux.intel.com>

Hehe. So you handed the patch to yourself :)

> ---
> arch/x86/kernel/apic/apic.c | 21 ++++++++++++++++++++-
> 1 files changed, 20 insertions(+), 1 deletions(-)
>
> diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
> index e5a4a1e..8ef56ac 100644
> --- a/arch/x86/kernel/apic/apic.c
> +++ b/arch/x86/kernel/apic/apic.c
> @@ -175,7 +175,7 @@ static struct resource lapic_resource = {
> .flags = IORESOURCE_MEM | IORESOURCE_BUSY,
> };
>
> -static unsigned int calibration_result;
> +unsigned int calibration_result;

Aside of my general objection it'd be not a good idea to make this
global w/o renaming it to something sensible like
lapic_timer_frequency.

> static int lapic_next_event(unsigned long delta,
> struct clock_event_device *evt);
> @@ -597,6 +597,25 @@ static int __init calibrate_APIC_clock(void)
> long delta, deltatsc;
> int pm_referenced = 0;
>
> + /**
> + * check if lapic timer has already been calibrated by platform
> + * specific routine, such as tsc calibration code. if so, we just fill
> + * in the clockevent structure and return.
> + */
> + if (calibration_result) {
> + apic_printk(APIC_VERBOSE, "lapic timer already calibrated %d\n",
> + calibration_result);
> + lapic_clockevent.mult = div_sc(calibration_result/APIC_DIVISOR,
> + TICK_NSEC, lapic_clockevent.shift);
> + lapic_clockevent.max_delta_ns =
> + clockevent_delta2ns(0x7FFFFF, &lapic_clockevent);
> + lapic_clockevent.min_delta_ns =
> + clockevent_delta2ns(0xF, &lapic_clockevent);
> + lapic_clockevent.features &= ~CLOCK_EVT_FEAT_DUMMY;
> + return 0;

Grr. I hate duplicated code.

> + }
> +
> local_irq_disable();
>
> /* Replace the global interrupt handler */
> --
> 1.6.3.3
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Thomas Gleixner on
Jacob,

On Tue, 11 May 2010, Pan, Jacob jun wrote:

> Thanks for the review.
>
> > >
> > > lapic timer calibration can be combined with tsc in platform specific
> > > calibration functions. if such calibration result is obtained early,
> > > we can skip the redundent calibration loops.
> >
> > I'd rather move lapic calibration into TSC calibration in general as
> > we do the same thing twice for no good reason.
> >
> > That needs some code restructuring, but that's worth it.
> >
> I am trying to avoid the risks of completely remove the current lapic
> calibration code since there are so many platforms with different timer
> options. And I don't understand things like why pm timer is preferred.
> Why not use the rating in clocksource?

We do not have access to the clocksource at that point. But we do a
calibration loop for TSC and for lapic timer. There is no reason why
we can't do that in one go.

> > Aside of my general objection it'd be not a good idea to make this
> > global w/o renaming it to something sensible like
> > lapic_timer_frequency.
> >
> perhaps, the calibration data can directly be assigned to lapic timer
> clock_event_device.mult? There is no need for the device specific result
> scale (e.g. bus clocks per tick)

No. That needs an accessor function.

Thanks,

tglx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: H. Peter Anvin on
On 05/11/2010 01:46 PM, Pan, Jacob jun wrote:
>
> The question I have is the reference clock used for calibrating those local timers,
> PIT, HPET, PM timer how should they be ranked. Can we make those known freq
> clocksource devices available at this point so that we can use the clocksource
> abstraction and its ranking automatically?
>

Personally I'd rank the PMTMR first, then HPET, then PIT, just based on
the relative complexity and relative known bugginess of the various
implementations.

The PMTMRs main defect is that it can't generate an interrupt; it's just
a dumb counter.

-hpa

--
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel. I don't speak on their behalf.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/