On Fri, 7 May 2010, Jacob Pan wrote:

quoted text
> From: Jacob Pan <jacob.jun.pan@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.

quoted text
> Signed-off-by: Jacob Pan <jacob.jun.pan@intel.com>
> Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>

Hehe. So you handed the patch to yourself :)

quoted text
> ---
>  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.
  
quoted text
>  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.

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