On Tue, May 26, 2009 at 6:44 PM, Jon Smirl jonsmirl@gmail.com wrote:
On Tue, May 26, 2009 at 8:38 PM, Timur Tabi timur@freescale.com wrote:
On Tue, May 26, 2009 at 7:25 PM, Jon Smirl jonsmirl@gmail.com wrote:
- spin_event_timeout(0, 10, 0, rc);
- spin_event_timeout(0, 10, 0);
out_8(®s->op0, MPC52xx_PSC_OP_RES);
- spin_event_timeout(0, 50, 0, rc);
- spin_event_timeout(0, 50, 0);
Jon, I'm still hoping you'll explain why you're not using udelay() here.
Because Grant didn't want me doing udelay(50) just to delay things in order to give the AC97 controller time to initialize. Your function lets me loop on cpu_relax() for 50us.
No, you misunderstand. My issue is that udelay is far too coarse grained for what you want. ie. If your are using udelay(1) in your loop, and your event changes state in 1.01us, then you're still going to burn a full 2us of time. If you can loop over cpu_relax() instead, then less time will get burned.
For a hard 50us delay, with no early exit condition, udelay() is the correct function.
HOWEVER; I'm distrustful of *any* spins, and would rather see sleeps instead of spins wherever possible. There are two reasons for this:
First, udelay just burns time, and if the delay is too large, then the it is wasting time that could be used for something else. That being said, it needs to be balanced with the context switch overhead. If the udelay() is less than double the context switch time, then the overhead is greater than the time spent spinning.
Second, udelay() with IRQs disabled cause unbounded latencies. Even if it is less efficient, I'd rather see mutexes+msleep() than spin_lock_irqsave()+udelay(). Of course, this only works in non-atomic contexts. If the code is atomic, then you have no choice.
There is no blanket rule here; but as the time delay requirement gets larger, the greater the benefit to overall system responsiveness by using sleep instead of delay. I'm not going to make a hard statement about how it should be done right now. It's a fiddly area and it can be tweaked after the driver is merged.
However, Timur is absolutely right. A spin without an early exit condition should simply be a udelay().
I have to delay 50us because ALSA tries to access the hardware immediately after the function returns.
Does the code run in atomic context?
g.