Skip to contentSkip to conversation

b43: Add fastpath to b43_mac_suspend()

PATCH In review Michael Buesch 2 participants 3 messages Apr 15, 2008 – Apr 16, 2008 Networking drivers (wireless)

lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 b43: Add fastpath to b43_mac_suspend()

This adds a fastpath for the common workloads to the MAC suspend flushing.
In common workloads the FIFO flush will take between 100 and 200 microseconds. So we want to avoid calling msleep() in the common case, as it will waste over 800 microseconds + scheduler overhead.

This fastpath will hit in workloads where only small chunks of data are transmitted (downloading a file) or when a TX rate bigger or equal to 24MBit/s is used when transmitting lots of stuff (iperf). So in the commonly used workloads it will basically always hit.

In case the fastpath is not hit, there's no real performance or latency disadvantage from that.

And yes, I measured this. So this is not one of these bad Programmer Likeliness Assumptions that are always wrong. ;)

---

Patch version without useless {}

John, please apply to 2.6.26
Stefano, you may port this, if you like to.

drivers/net/wireless/b43/main.c +8 -0
Index: wireless-testing/drivers/net/wireless/b43/main.c
===================================================================
@@ -2340,12 +2340,20 @@ static void b43_mac_suspend(struct b43_w
 		b43_power_saving_ctl_bits(dev, B43_PS_AWAKE);
 		b43_write32(dev, B43_MMIO_MACCTL,
 			    b43_read32(dev, B43_MMIO_MACCTL)
 			    & ~B43_MACCTL_ENABLED);
 		/* force pci to flush the write */
 		b43_read32(dev, B43_MMIO_MACCTL);
+		/* Finally wait for the microcode to flush the fifos. */
+		for (i = 35; i; i--) {
+			tmp = b43_read32(dev, B43_MMIO_GEN_IRQ_REASON);
+			if (tmp & B43_IRQ_MAC_SUSPENDED)
+				goto out;
+			udelay(10);
+		}
+		/* Hm, it seems this will take some time. Use msleep(). */
 		for (i = 40; i; i--) {
 			tmp = b43_read32(dev, B43_MMIO_GEN_IRQ_REASON);
 			if (tmp & B43_IRQ_MAC_SUSPENDED)
 				goto out;
 			msleep(1);
 		}
maintainer
#1 lore raw

drivers/net/wireless/b43/main.c static void b43_mac_suspend(struct b43_w · line 2346

Quoted 12 lines · Michael Buesch

This adds a fastpath for the common workloads to the MAC suspend flushing.

drivers/net/wireless/b43/main.c +1 -0
@@ -2340,12 +2340,20 @@ static void b43_mac_suspend(struct b43_w
 		b43_power_saving_ctl_bits(dev, B43_PS_AWAKE);
 		b43_write32(dev, B43_MMIO_MACCTL,
 			    b43_read32(dev, B43_MMIO_MACCTL)
 			    & ~B43_MACCTL_ENABLED);
 		/* force pci to flush the write */
 		b43_read32(dev, B43_MMIO_MACCTL);
+		/* Finally wait for the microcode to flush the fifos. */

That comment is wrong, the ucode won't flush the FIFOs, it'll just go to sleep. If you want it to flush FIFOs you have to set a flush request bit somewhere (mac command I think), but you don't actually, you just don't want to have it processing stuff.

johannes

#2 lore raw
Quoted 5 lines · On Wednesday 16 April 2008 20:36:39 Johannes Berg wrote:
Quoted 2 lines · Michael Buesch

This adds a fastpath for the common workloads to the MAC suspend flushing.

Quoted 13 lines

drivers/net/wireless/b43/main.c static void b43_mac_suspend(struct b43_w · line 2346

Quoted 8 lines
drivers/net/wireless/b43/main.c +1 -0
@@ -2340,12 +2340,20 @@ static void b43_mac_suspend(struct b43_w
 		b43_power_saving_ctl_bits(dev, B43_PS_AWAKE);
 		b43_write32(dev, B43_MMIO_MACCTL,
 			    b43_read32(dev, B43_MMIO_MACCTL)
 			    & ~B43_MACCTL_ENABLED);
 		/* force pci to flush the write */
 		b43_read32(dev, B43_MMIO_MACCTL);
+		/* Finally wait for the microcode to flush the fifos. */

That comment is wrong, the ucode won't flush the FIFOs, it'll just go to sleep. If you want it to flush FIFOs you have to set a flush request bit somewhere (mac command I think), but you don't actually, you just don't want to have it processing stuff.

Ah yeah. Whatever. What was meant it it flushes everything from the host point of view and goes to sleep. Feel free to remove that comment, John. (simply remove the line from the patch before applying it).

Signature · 1 line
Greetings Michael.