lkml.org 
[lkml]   [2010]   [May]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRE: [PATCH 3/8] x86/apic: allow use of lapic timer early calibration result
    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?

    > > 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 :)
    >
    > > ---
    > > 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.
    >
    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)

    > > 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@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2010-05-12 10:17    [W:0.035 / U:267.288 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site