Subject: Re: Please pull 'upstream' branch of wireless-2.6

From: Michael Buesch <[email-masked]>
Date: Tue, 27 Jun 2006 15:30:06 +0200

On Tuesday 27 June 2006 04:27, Larry Finger wrote:
> Jeff Garzik wrote:
> > John W. Linville wrote:
> >> +    assert(bcm->mac_suspended >= 0);
> >> +    if (bcm->mac_suspended == 0) {
> >> +        bcm43xx_power_saving_ctl_bits(bcm, -1, 1);
> >> +        bcm43xx_write32(bcm, BCM43xx_MMIO_STATUS_BITFIELD,
> >> +                        bcm43xx_read32(bcm, 
> >> BCM43xx_MMIO_STATUS_BITFIELD)
> >> +                & ~BCM43xx_SBF_MAC_ENABLED);
> >> +        bcm43xx_read32(bcm, BCM43xx_MMIO_GEN_IRQ_REASON); /* dummy 
> >> read */
> >> +        for (i = 100000; i; i--) {
> >> +            tmp = bcm43xx_read32(bcm, BCM43xx_MMIO_GEN_IRQ_REASON);
> >> +            if (tmp & BCM43xx_IRQ_READY)
> >> +                goto out;
> >> +            udelay(10);
> >> +        }
> >> +        printkl(KERN_ERR PFX "MAC suspend failed\n");
> >>      }
> > 
> > 
> > NAK this super-long delay...  should be done in a workqueue, looks like?
> > 
> > ACK everything else.
> > 
> 
> That delay was set to try to accommodate my interface when it refused to suspend the MAC, which 
> resulted in transmit errors. That problem has since been cured by reworking the periodic work 
> handlers - thus such a long delay should not be needed. The original spec from the clean-room group 
> was a delay loop of 1000. I'm currently testing that value now. If it passes the test, would a for 
> (i=1000; i; i--) be acceptable?

Short: Don't touch it. Fullstop.

Long: The delay will _never_ be exhausted. Actually the for-counter
is only there to not lock up the machine, if there is a Bug in the
driver. (__much__ easier debugging).
The loop will only iterate a few times, typically.
Actually, _if_ we want to change something, we should do this:

for (i = 1000000; i; i--) {
	...
	udelay(1);
}

(max loop multiplied by 10, delay value divided by 10).
This will shorten the whole delay by a few usecs (up to 10).
I will send a patch for this, if it is desired.

But lowering the loop counter value is NACKed by me,
because it simply does not make sense.

-- 
Greetings Michael.