On 12/22/09 06:25, Pavel Roskin wrote: > Quoting Gertjan van Wingerde <gwingerde@xxxxxxxxx>: > >>> Perhaps non-zero headroom is not handled correctly? >>> >> >> Hmmm, perhaps the problem is that the headroom is not a multiple of 4. >> Can you check what happens when you set the extra_tx_headroom fixed >> to e.g. 20? > > I tried 64 and 2, and neither worked. rt2x00dev->ops->extra_tx_headroom > is 0 for me. When the rt2x00dev->hw->extra_tx_headroom is 2, I observed > invalid packets being emitted (using another interface). The packets > have two bytes inserted in the beginning of the frame: > > 0000 00 00 1c 00 6f 48 00 00 fc de ee 13 00 00 00 00 ....oH.. ........ > 0010 10 02 85 09 a0 00 b1 b7 00 00 00 00 00 00 40 00 ........ ......@. > radiotap, 28 bytes ----------------------> > 2 extra bytes <---> > correct 802.11 header <---- > 0020 00 00 ff ff ff ff ff ff 00 d0 41 af ad 2c ff ff ........ ..A..,.. > 0030 ff ff cf 02 c0 02 00 05 77 62 65 6e 64 01 08 02 ........ wbend... > 0040 04 0b 16 0c 12 18 24 32 04 30 48 e7 f8 07 9d ......$2 .0H.... > > Here's a patch that is working for me, but I would not vouch for its > correctness. Signing off just in case it happens to be correct ;-) > > > rt2x00: use correct headroom for transmission > > Use rt2x00dev->ops->extra_tx_headroom, not > rt2x00dev->hw->extra_tx_headroom in the tx code, as the later includes > headroom only meant for the monitor mode. > > Signed-off-by: Pavel Roskin <proski@xxxxxxx> Doh, that's it. Here we should indeed only take the real extra TX headroom required by the driver into account. Seems like I goofed up in a preparatory patch that centralized setting of rt2x00dev->hw->extra_tx_headroom. Can you confirm that everything works okay with the original code and this patch applied? In any way: Acked-by: Gertjan van Wingerde <gwingerde@xxxxxxxxx> John, with this you can reinstate the original patch as well. Let me know if you want me to resubmit that one with this one included in it. > --- > drivers/net/wireless/rt2x00/rt2x00queue.c | 6 +++--- > 1 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/wireless/rt2x00/rt2x00queue.c b/drivers/net/wireless/rt2x00/rt2x00queue.c > index 3d8fb68..0b4801a 100644 > --- a/drivers/net/wireless/rt2x00/rt2x00queue.c > +++ b/drivers/net/wireless/rt2x00/rt2x00queue.c > @@ -104,7 +104,7 @@ void rt2x00queue_map_txskb(struct rt2x00_dev *rt2x00dev, struct sk_buff *skb) > * is also mapped to the DMA so it can be used for transfering > * additional descriptor information to the hardware. > */ > - skb_push(skb, rt2x00dev->hw->extra_tx_headroom); > + skb_push(skb, rt2x00dev->ops->extra_tx_headroom); > > skbdesc->skb_dma = > dma_map_single(rt2x00dev->dev, skb->data, skb->len, DMA_TO_DEVICE); > @@ -112,7 +112,7 @@ void rt2x00queue_map_txskb(struct rt2x00_dev *rt2x00dev, struct sk_buff *skb) > /* > * Restore data pointer to original location again. > */ > - skb_pull(skb, rt2x00dev->hw->extra_tx_headroom); > + skb_pull(skb, rt2x00dev->ops->extra_tx_headroom); > > skbdesc->flags |= SKBDESC_DMA_MAPPED_TX; > } > @@ -134,7 +134,7 @@ void rt2x00queue_unmap_skb(struct rt2x00_dev *rt2x00dev, struct sk_buff *skb) > * by the driver, but it was actually mapped to DMA. > */ > dma_unmap_single(rt2x00dev->dev, skbdesc->skb_dma, > - skb->len + rt2x00dev->hw->extra_tx_headroom, > + skb->len + rt2x00dev->ops->extra_tx_headroom, > DMA_TO_DEVICE); > skbdesc->flags &= ~SKBDESC_DMA_MAPPED_TX; > } > -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
