Why call calibrate_delay() in smp.c: secondary_start_kernel()

[Santosh Shilimkar]

> -----Original Message-----
> From: linus.ml.walleij at gmail.com [mailto:linus.ml.walleij at gmail.com]
> On Behalf Of Linus Walleij
> Sent: Tuesday, January 18, 2011 8:48 PM
> To: Santosh Shilimkar
> Cc: Russell King - ARM Linux; Jonas Aaberg; linux-arm-
> kernel at lists.infradead.org; STEricsson_nomadik_linux
> Subject: Re: Why call calibrate_delay() in smp.c:
> secondary_start_kernel()
>
> 2011/1/18 Santosh Shilimkar <santosh.shilimkar at ti.com>:
>

[...]

>
> Why do you want to select that manually? Surely the kernel
> writer should know whether the cores are clocked together
> or not. Just:
>
> config ARCH_HAS_COMMON_CORES_CLOCK
>       bool
>       depends on SMP
>
> This will be default "n" so archs that want to flag to the
> build system that the core clock is common can e.g.:
>
> config ARCH_U8500
>         bool "ST-Ericsson U8500 Series"
>         select CPU_V7
> +       select ARCH_HAS_COMMON_CORES_CLOCK
>
> Looks reasonable? (And feel free to add that to U8500
> as part of the patch.)
>
This looks better. Thanks will add U8500

> >  /*
> > + * Skip the secondary calibration on architectures sharing clock
> > + * with primary cpu. Archs can use ARCH_SKIP_SECONDARY_CALIBRATE
> > + * for this.
> > + */
> > +static inline int skip_secondary_calibrate(void)
>
> bool? Returning an error code seem overdesigned for a function
> named like that.
>
> Also the name is misleading, if you look at how you
> use it, it should be named "do_not_skip_secondary_calibrate()".
>
> > +{
> > +#ifdef CONFIG_ARCH_SKIP_SECONDARY_CALIBRATE
> > +       return 0;
>
> return false;
>
> > +#else
> > +       return -ENXIO;
>
> return true;
>
> > +#endif
> > +}
>
>
> > +
> > +/*
> >  * This is the secondary CPU boot entry.  We're using this CPUs
> >  * idle thread stack, but a set of temporary page tables.
> >  */
> > @@ -312,7 +326,8 @@ asmlinkage void __cpuinit
> secondary_start_kernel(void)
> >         */
> >        percpu_timer_setup();
> >
> > -       calibrate_delay();
> > +       if (skip_secondary_calibrate())
> > +               calibrate_delay();
>
> Change sematics of that function and
> if (!skip_secondary_calibrate())
>   ...
>
> >        smp_store_cpu_info(cpu);
>
Fair enough.

> ...and thanks for driving this!

Np.

Regards,
Santosh