[v2,2/2] ARM: delay: allow timer-based delay implementation to be selected

Commit Message

Will Deacon July 4, 2012, 3:36 p.m. UTC

On Tue, Jul 03, 2012 at 01:09:31PM +0100, Shinya Kuribayashi wrote:
> Hello,

Hi Shinya,

> Looking at init/calibrate.c, 'lpj_fine' can save the first loop
> calibration, but does not save the second and subsequent calls to
> calibrate_delay().
> 
> Magnus first noticed this, and changed to use 'preset_lpj' instead.
> See the commit 173bf69a7a (ARM / mach-shmobile: Use preset_lpj with
> calibrate_delay(), 2012-05-10).
> 
> If we use 'lpj_fine' for this, we need to skip secondary CPU calibration
> explicitly in another way, something like this:
> 
> http://lists.infradead.org/pipermail/linux-arm-kernel/2011-January/039506.html
> [PATCH 5/5] ARM: smp: Skip secondary cpu calibration to speed-up boot

How about keeping it simple like this:?




Will

Comments

Shinya Kuribayashi July 5, 2012, 12:12 p.m. UTC | #1

Hi Will,

On 7/5/2012 12:36 AM, Will Deacon wrote:
>> If we use 'lpj_fine' for this, we need to skip secondary CPU calibration
>> explicitly in another way, something like this:
>>
>> http://lists.infradead.org/pipermail/linux-arm-kernel/2011-January/039506.html
>> [PATCH 5/5] ARM: smp: Skip secondary cpu calibration to speed-up boot
> 
> How about keeping it simple like this:?
> 
> 
> diff --git a/arch/arm/lib/delay.c b/arch/arm/lib/delay.c
> index e1030e1..84bb5da 100644
> --- a/arch/arm/lib/delay.c
> +++ b/arch/arm/lib/delay.c
> @@ -63,4 +63,9 @@ void __init init_current_timer_delay(unsigned long freq)
>         arm_delay_ops.const_udelay      = __timer_const_udelay;
>         arm_delay_ops.udelay            = __timer_udelay;
>  }
> +
> +unsigned long __cpuinit calibrate_delay_is_known(void)
> +{
> +       return lpj_fine ?: 0;
> +}
>  #endif

Thanks for the patch, looks lika a missing piece of CPU calibration
optimization for SMP platforms in the face of core frequency scaling.

Ok, I gave your patch a try (including above), and confirmed that:

* It works fine with non-arch_timer counter.  I'm using SH/R-Mobile
  devices, with a memory mapped I/O, 32-bit free-run up-counter
  running at 13MHz.

* Secondary CPU calibration gets skipped as expected.

* Your new timer-based delay works as before (loop-based one).  I've
  verified 10..1999-microsecond busy-wait with a reasonable accuracy
  (and confirmed that 2000+ usec gets rejected as intended).

By the way,

> +       return lpj_fine ?: 0;

Is there any difference with just

        return lpj_fine;

?

Will Deacon July 5, 2012, 12:56 p.m. UTC | #2

On Thu, Jul 05, 2012 at 01:12:14PM +0100, Shinya Kuribayashi wrote:
> Ok, I gave your patch a try (including above), and confirmed that:
> 
> * It works fine with non-arch_timer counter.  I'm using SH/R-Mobile
>   devices, with a memory mapped I/O, 32-bit free-run up-counter
>   running at 13MHz.
> 
> * Secondary CPU calibration gets skipped as expected.
> 
> * Your new timer-based delay works as before (loop-based one).  I've
>   verified 10..1999-microsecond busy-wait with a reasonable accuracy
>   (and confirmed that 2000+ usec gets rejected as intended).

Great, thanks! Can I add your tested-by please?

> By the way,
> 
> > +       return lpj_fine ?: 0;
> 
> Is there any difference with just
> 
>         return lpj_fine;

Of course, I'll make that change, cheers.

Stephen -- can I keep your reviewed-by with the additional function please?

Will

Stephen Boyd July 5, 2012, 4:51 p.m. UTC | #3

On 07/05/12 05:56, Will Deacon wrote:
>
> Stephen -- can I keep your reviewed-by with the additional function please?
>
>

Yes, that's fine.

Patch

diff --git a/arch/arm/lib/delay.c b/arch/arm/lib/delay.c
index e1030e1..84bb5da 100644
--- a/arch/arm/lib/delay.c
+++ b/arch/arm/lib/delay.c
@@ -63,4 +63,9 @@  void __init init_current_timer_delay(unsigned long freq)
        arm_delay_ops.const_udelay      = __timer_const_udelay;
        arm_delay_ops.udelay            = __timer_udelay;
 }
+
+unsigned long __cpuinit calibrate_delay_is_known(void)
+{
+       return lpj_fine ?: 0;
+}
 #endif