| Message ID | 4FFE7DB2.4040702@renesas.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Jul 12, 2012 at 08:33:06AM +0100, Shinya Kuribayashi wrote: > Hi Will, Stephen, Hi Shinya, > On 6/30/2012 2:33 AM, Will Deacon wrote: > > +void __init init_current_timer_delay(unsigned long freq) > > +{ > > + pr_info("Switching to timer-based delay loop\n"); > > + lpj_fine = freq / HZ; > > + arm_delay_ops.delay = __timer_delay; > > + arm_delay_ops.const_udelay = __timer_const_udelay; > > + arm_delay_ops.udelay = __timer_udelay; > > +} > > Once this function is processed, the udelay() behavior changes > _immediately_ from loop-based delay to timer-based one, without waiting > for 'loops_per_jiffy' itself being corrected in calibrate_delay(). > > As a result, actual udelay()s may be toooo long than expected, in > particular udelay()s used between init_current_timer_delay() and > calibrate_delay(). It's unlikely be short, as the frequency of a > counter for read_current_timer is typically slower than CPU frequency. Surely using udelay before calibrate_delay_loop has been called is a fundamental error? Will
On 7/12/2012 5:44 PM, Will Deacon wrote: >> On 6/30/2012 2:33 AM, Will Deacon wrote: >>> +void __init init_current_timer_delay(unsigned long freq) >>> +{ >>> + pr_info("Switching to timer-based delay loop\n"); >>> + lpj_fine = freq / HZ; >>> + arm_delay_ops.delay = __timer_delay; >>> + arm_delay_ops.const_udelay = __timer_const_udelay; >>> + arm_delay_ops.udelay = __timer_udelay; >>> +} >> >> Once this function is processed, the udelay() behavior changes >> _immediately_ from loop-based delay to timer-based one, without waiting >> for 'loops_per_jiffy' itself being corrected in calibrate_delay(). >> >> As a result, actual udelay()s may be toooo long than expected, in >> particular udelay()s used between init_current_timer_delay() and >> calibrate_delay(). It's unlikely be short, as the frequency of a >> counter for read_current_timer is typically slower than CPU frequency. > > Surely using udelay before calibrate_delay_loop has been called is a > fundamental error? Got it. I'm just not confident about disallowing early use of udelay(). Shinya
On 07/12/12 02:35, Shinya Kuribayashi wrote: > On 7/12/2012 5:44 PM, Will Deacon wrote: >>> On 6/30/2012 2:33 AM, Will Deacon wrote: >>>> +void __init init_current_timer_delay(unsigned long freq) >>>> +{ >>>> + pr_info("Switching to timer-based delay loop\n"); >>>> + lpj_fine = freq / HZ; >>>> + arm_delay_ops.delay = __timer_delay; >>>> + arm_delay_ops.const_udelay = __timer_const_udelay; >>>> + arm_delay_ops.udelay = __timer_udelay; >>>> +} >>> Once this function is processed, the udelay() behavior changes >>> _immediately_ from loop-based delay to timer-based one, without waiting >>> for 'loops_per_jiffy' itself being corrected in calibrate_delay(). >>> >>> As a result, actual udelay()s may be toooo long than expected, in >>> particular udelay()s used between init_current_timer_delay() and >>> calibrate_delay(). It's unlikely be short, as the frequency of a >>> counter for read_current_timer is typically slower than CPU frequency. >> Surely using udelay before calibrate_delay_loop has been called is a >> fundamental error? > Got it. I'm just not confident about disallowing early use of udelay(). > > I don't think it's an error. Instead you get a very large delay, similar to what would happen if you called udelay() before calibrate_delay() anyway (see the comment in init/main.c above loops_per_jiffy).
On 7/13/2012 1:40 AM, Stephen Boyd wrote: >>>> As a result, actual udelay()s may be toooo long than expected, in >>>> particular udelay()s used between init_current_timer_delay() and >>>> calibrate_delay(). It's unlikely be short, as the frequency of a >>>> counter for read_current_timer is typically slower than CPU frequency. >>> Surely using udelay before calibrate_delay_loop has been called is a >>> fundamental error? >> Got it. I'm just not confident about disallowing early use of udelay(). >> > > I don't think it's an error. Instead you get a very large delay, similar > to what would happen if you called udelay() before calibrate_delay() > anyway (see the comment in init/main.c above loops_per_jiffy). Thanks, so I'd set up loops_per_jiffy early, along with lpj_fine in init_current_timer_delay(). As a matter of fact, I do have early use of udelay()s in my tree :-) Will give it a try for some time.
On Fri, Jul 13, 2012 at 03:16:41AM +0100, Shinya Kuribayashi wrote: > On 7/13/2012 1:40 AM, Stephen Boyd wrote: > >>>> As a result, actual udelay()s may be toooo long than expected, in > >>>> particular udelay()s used between init_current_timer_delay() and > >>>> calibrate_delay(). It's unlikely be short, as the frequency of a > >>>> counter for read_current_timer is typically slower than CPU frequency. > >>> Surely using udelay before calibrate_delay_loop has been called is a > >>> fundamental error? > >> Got it. I'm just not confident about disallowing early use of udelay(). > >> > > > > I don't think it's an error. Instead you get a very large delay, similar > > to what would happen if you called udelay() before calibrate_delay() > > anyway (see the comment in init/main.c above loops_per_jiffy). Interesting, I didn't notice it was initialised to 4k, so yes I suppose you could make use of some sort of delay. I don't think it's necessarily `very large' though -- anything ticking at over ~400KHz with HZ=100 would give you a smaller delay. > Thanks, so I'd set up loops_per_jiffy early, along with lpj_fine in > init_current_timer_delay(). That should work, providing you can get a sensible initial estimate for loops_per_jiffy. Will
Wiil, On Fri, Jul 13, 2012 at 2:27 PM, Will Deacon <will.deacon@arm.com> wrote: > On Fri, Jul 13, 2012 at 03:16:41AM +0100, Shinya Kuribayashi wrote: >> On 7/13/2012 1:40 AM, Stephen Boyd wrote: >> >>>> As a result, actual udelay()s may be toooo long than expected, in >> >>>> particular udelay()s used between init_current_timer_delay() and >> >>>> calibrate_delay(). It's unlikely be short, as the frequency of a >> >>>> counter for read_current_timer is typically slower than CPU frequency. >> >>> Surely using udelay before calibrate_delay_loop has been called is a >> >>> fundamental error? >> >> Got it. I'm just not confident about disallowing early use of udelay(). >> >> >> > >> > I don't think it's an error. Instead you get a very large delay, similar >> > to what would happen if you called udelay() before calibrate_delay() >> > anyway (see the comment in init/main.c above loops_per_jiffy). > > Interesting, I didn't notice it was initialised to 4k, so yes I suppose you > could make use of some sort of delay. I don't think it's necessarily `very > large' though -- anything ticking at over ~400KHz with HZ=100 would give you > a smaller delay. > >> Thanks, so I'd set up loops_per_jiffy early, along with lpj_fine in >> init_current_timer_delay(). > > That should work, providing you can get a sensible initial estimate for > loops_per_jiffy. > Do you have an updated version of the patch ? Regards, Santosh
diff --git a/arch/arm/lib/delay.c b/arch/arm/lib/delay.c index cb881c3..9065d30 100644 --- a/arch/arm/lib/delay.c +++ b/arch/arm/lib/delay.c @@ -57,7 +57,7 @@ static void __timer_udelay(unsigned long usecs) void __init init_current_timer_delay(unsigned long freq) { - lpj_fine = (freq + HZ/2) / HZ; + loops_per_jiffy = lpj_fine = (freq + HZ/2) / HZ; arm_delay_ops.delay = __timer_delay; arm_delay_ops.const_udelay = __timer_const_udelay; arm_delay_ops.udelay = __timer_udelay;