On 11/23/09 19:33, Ivo van Doorn wrote: > On Monday 23 November 2009, Gertjan van Wingerde wrote: >> Current rt2x00 drivers may result in a "ieee80211_tx_status: headroom too >> small" error message when a frame needs to be properly aligned before >> transmitting it. >> This is because the space needed to ensure proper alignment isn't >> requested from mac80211. >> Fix this by adding sufficient amount of alignment space to the amount >> of headroom requested for TX frames. >> >> Reported-by: David Ellingsworth <david@xxxxxxxxxxxxxxxxx> >> Signed-off-by: Gertjan van Wingerde <gwingerde@xxxxxxxxx> >> Tested-by: David Ellingsworth <david@xxxxxxxxxxxxxxxxx> >> --- >> drivers/net/wireless/rt2x00/rt2400pci.c | 30 +++++++++++++++------------- >> drivers/net/wireless/rt2x00/rt2500pci.c | 30 +++++++++++++++------------- >> drivers/net/wireless/rt2x00/rt2500usb.c | 29 ++++++++++++++------------- >> drivers/net/wireless/rt2x00/rt2800lib.c | 7 +---- >> drivers/net/wireless/rt2x00/rt2800pci.c | 31 +++++++++++++++-------------- >> drivers/net/wireless/rt2x00/rt2800usb.c | 25 ++++++++++++----------- >> drivers/net/wireless/rt2x00/rt2x00.h | 7 ++++++ >> drivers/net/wireless/rt2x00/rt2x00queue.c | 6 ++-- >> drivers/net/wireless/rt2x00/rt61pci.c | 28 ++++++++++++++------------ >> drivers/net/wireless/rt2x00/rt73usb.c | 27 +++++++++++++------------ >> 10 files changed, 117 insertions(+), 103 deletions(-) >> >> diff --git a/drivers/net/wireless/rt2x00/rt2400pci.c b/drivers/net/wireless/rt2x00/rt2400pci.c >> index 6e68bc7..f534d70 100644 >> --- a/drivers/net/wireless/rt2x00/rt2400pci.c >> +++ b/drivers/net/wireless/rt2x00/rt2400pci.c >> @@ -1432,7 +1432,8 @@ static int rt2400pci_probe_hw_mode(struct rt2x00_dev *rt2x00dev) >> IEEE80211_HW_SIGNAL_DBM | >> IEEE80211_HW_SUPPORTS_PS | >> IEEE80211_HW_PS_NULLFUNC_STACK; >> - rt2x00dev->hw->extra_tx_headroom = 0; >> + rt2x00dev->hw->extra_tx_headroom = rt2x00dev->ops->extra_tx_headroom + >> + RT2X00_ALIGN_SIZE; > > Can't rt2x00lib initialize rt2x00dev->hw->extra_tx_headroom? > The drivers aren't performing the aligning, so it is logical that rt2x00lib > adds the required bytes rather than the drivers. The driver already sets > the value in ops->extra_tx_headroom, so it can rely on rt2x00lib to do the rest. > I guess that that can be done as an extra cleanup. Will do in a revision. >> diff --git a/drivers/net/wireless/rt2x00/rt2800lib.c b/drivers/net/wireless/rt2x00/rt2800lib.c >> index b809c49..de358b5 100644 >> --- a/drivers/net/wireless/rt2x00/rt2800lib.c >> +++ b/drivers/net/wireless/rt2x00/rt2800lib.c >> @@ -2030,11 +2030,8 @@ int rt2800_probe_hw_mode(struct rt2x00_dev *rt2x00dev) >> IEEE80211_HW_SUPPORTS_PS | >> IEEE80211_HW_PS_NULLFUNC_STACK; >> >> - if (rt2x00_intf_is_usb(rt2x00dev)) >> - rt2x00dev->hw->extra_tx_headroom = >> - TXINFO_DESC_SIZE + TXWI_DESC_SIZE; >> - else if (rt2x00_intf_is_pci(rt2x00dev)) >> - rt2x00dev->hw->extra_tx_headroom = TXWI_DESC_SIZE; >> + rt2x00dev->hw->extra_tx_headroom = rt2x00dev->ops->extra_tx_headroom + >> + RT2X00_L2PAD_SIZE; > > In this case we don't use RT2X00_ALIGN_SIZE, but the driver does set a flag to tell > rt2x00lib that L2padding should be used rather then aligning. So this case would still > hold when it is moved to rt2x00lib. > Yep, will do in a revision. >> /* >> + * Constants for extra TX headroom for alignment purposes. >> + */ >> +#define RT2X00_ALIGN_SIZE 4 >> +#define RT2X00_L2PAD_SIZE 8 > > #define RT2X00_L2PAD_SIZE RT2X00_ALIGN_SIZE + 4 > > Would probably better indicate why you are using 8. > I'll add some comments on what these values represent. Don't think that defining RT2X00_L2PAD_SIZE in your proposed way would help explaining, but some proper comments will. --- Gertjan. -- 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
