Hey,
Good news. I figured out the DTIM thing and the multicast frame queue
stuff. For multicast frames, you simply send them down queue four and
they are dequeued and sent by the ucode after the DTIM beacon.
> But that's not all, also multicast frames (including ARP broadcast)
> are not buffered correctly. They are sent randomly instead of directly
> after a DTIM beacon. Also Multicast bit is not set and DTIM is always
> zero even though DTIM period is two.
Actually, they were sent right away because we enqueued them to queue
zero, not queue four.
Please try the attached patches. I haven't implemented the set_tim()
callback yet, but the device should now wake up for multicast traffic,
not for unicast yet. The patches work for me on a heavily patched kernel
('everything' plus http://johannes.sipsolutions.net/patches/kernel/all/)
I'll update the specs to indicate that
(1) the tim offset shm thing needs to be set as well as the dtim period
(2) queue four must be used for multicast frames [in client mode we
never send multicast frames so doing it unconditionally is fine]
(3) updating the beacon on every beacon interrupt isn't necessary, it
only needs to be updated if it has changed (it works fine even if
you update it, it's just not necessary)
(4) the TIM is software controlled and should be set by the driver with
help from the beacon interrupt.
(5) the 0xa8 (==0x54*2) shm offset needs to be described, it's the
"last multicast frame" for clearing the more-data bit and 0xffff is
a special value that causes the microcode to clear the more-data
bit on all multicast frames (odd, and I don't know why that would
be useful, probably something with IBSS when we're not beaconing)
Due to the last point, I'm not entirely sure these patches are correct.
And I haven't bothered about PIO, do we even have access to that queue
there?
As for the whole beacon handling... I suggest that we change mac80211 to
give the beacon head and beacon tail to the driver like it gets it from
hostapd. Thing is, we first go to all the effort to build a valid beacon
frame, but then the driver gets to parse it all out again. That's dumb.
Hardware that has beaconing offload and implements the set_tim()
callback needs to know the TIM position, so it's probably better off
knowing the beacon head and tail. Drivers that call get_beacon() don't
care about conf->beacon except that they need to free it. Not obvious,
and many are probably buggy.
johannes
---
drivers/net/wireless/b43/main.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
--- wireless-dev.orig/drivers/net/wireless/b43/main.c 2007-08-29 01:43:44.404618695 +0200
+++ wireless-dev/drivers/net/wireless/b43/main.c 2007-08-29 01:43:46.954618695 +0200
@@ -1180,6 +1180,7 @@ static void b43_write_beacon_template(st
len = min((size_t) dev->cached_beacon->len,
0x200 - sizeof(struct b43_plcp_hdr6));
data = (const u8 *)(dev->cached_beacon->data);
+ printk(KERN_DEBUG "writing beacon\n");
b43_write_template_common(dev, data,
len, ram_offset, shm_size_offset, rate);
}
@@ -1242,6 +1243,13 @@ static u8 *b43_generate_probe_resp(struc
memcpy(dest_data + dest_pos, src_data + src_pos,
elem_size);
dest_pos += elem_size;
+ } else {
+ printk(KERN_DEBUG "TIM is at 0x%x\n", dest_pos);
+ printk(KERN_DEBUG "DTIM period is %d\n", src_data[src_pos + 3]);
+ b43_shm_write16(dev, B43_SHM_SHARED,
+ B43_SHM_SH_TIMBPOS, dest_pos + 6 /* PLCP */);
+ b43_shm_write16(dev, B43_SHM_SHARED,
+ B43_SHM_SH_DTIMPER, src_data[src_pos + 3]);
}
}
*dest_size = dest_pos;
@@ -1371,12 +1379,12 @@ static void handle_irq_beacon(struct b43
return;
}
if (!(status & 0x1)) {
- b43_write_beacon_template(dev, 0x68, 0x18, B43_CCK_RATE_1MB);
+// b43_write_beacon_template(dev, 0x68, 0x18, B43_CCK_RATE_1MB);
status |= 0x1;
b43_write32(dev, B43_MMIO_STATUS2_BITFIELD, status);
}
if (!(status & 0x2)) {
- b43_write_beacon_template(dev, 0x468, 0x1A, B43_CCK_RATE_1MB);
+// b43_write_beacon_template(dev, 0x468, 0x1A, B43_CCK_RATE_1MB);
status |= 0x2;
b43_write32(dev, B43_MMIO_STATUS2_BITFIELD, status);
}
---
drivers/net/wireless/b43/dma.c | 19 ++++++++++++++++---
drivers/net/wireless/b43/xmit.c | 10 ++++++++++
2 files changed, 26 insertions(+), 3 deletions(-)
--- wireless-dev.orig/drivers/net/wireless/b43/dma.c 2007-08-29 02:22:29.084618695 +0200
+++ wireless-dev/drivers/net/wireless/b43/dma.c 2007-08-29 02:45:38.574618695 +0200
@@ -1038,6 +1038,8 @@ static u16 generate_cookie(struct b43_dm
* in the lower 12 bits.
* Note that the cookie must never be 0, as this
* is a special value used in RX path.
+ * It can also not be 0xFFFF because that is special
+ * for multicast frames.
*/
switch (ring->index) {
case 0:
@@ -1056,7 +1058,7 @@ static u16 generate_cookie(struct b43_dm
cookie = 0xE000;
break;
case 5:
- cookie = 0xF000;
+ cookie = 0x9000;
break;
}
B43_WARN_ON(slot & ~0x0FFF);
@@ -1088,7 +1090,7 @@ struct b43_dmaring *parse_cookie(struct
case 0xE000:
ring = dma->tx_ring4;
break;
- case 0xF000:
+ case 0x9000:
ring = dma->tx_ring5;
break;
default:
@@ -1205,7 +1207,18 @@ int b43_dma_tx(struct b43_wldev *dev,
int err = 0;
unsigned long flags;
- ring = priority_to_txring(dev, ctl->queue);
+ if (skb->len < 5) {
+ printk(KERN_ERR "really short frame\n");
+ return -EIO;
+ }
+
+ if (skb->data[4] & 1) {
+ ring = dev->dma.tx_ring4;
+ /* set more data bit, ucode will clear on the last frame */
+ skb->data[1] |= 0x20;
+ } else {
+ ring = priority_to_txring(dev, ctl->queue);
+ }
spin_lock_irqsave(&ring->lock, flags);
B43_WARN_ON(!ring->tx);
if (unlikely(free_slots(ring) < SLOTS_PER_PACKET)) {
--- wireless-dev.orig/drivers/net/wireless/b43/xmit.c 2007-08-29 02:39:53.884618695 +0200
+++ wireless-dev/drivers/net/wireless/b43/xmit.c 2007-08-29 02:47:34.304618695 +0200
@@ -195,6 +195,16 @@ static void generate_txhdr_fw4(struct b4
u16 phy_ctl = 0;
u8 extra_ft = 0;
+ /*
+ * give microcode the cookie of the last frame in the multicast
+ * queue so it can clear the more-data bit in it
+ */
+ /* FIXME: should really check the queue or something */
+ if ((cookie & 0xF000) == 0x9000) {
+ /* FIXME: locking */
+ b43_shm_write16(dev, B43_SHM_SHARED, 0x54*2, cookie);
+ }
+
memset(txhdr, 0, sizeof(*txhdr));
rate = txctl->tx_rate;
Attachment:
signature.asc
Description: This is a digitally signed message part
