Re: [PATCH 01/18] brcmsmac: Rework tx code to avoid internal buffering of packets

["Arend van Spriel" <arend@broadcom.com>]

On 10/26/2012 04:23 PM, Seth Forshee wrote:
> The brcmsmac internal tx buffering is problematic for a number of
> reasons. The amount of buffering is excessive (228 packets in addition
> to the 256 slots in each DMA ring), frames may be dropped due to a lack
> of flow control, and the structure of the queues complicates the
> otherwise straightforward mapping of mac80211 queues to device fifos.
>
> This patch reworks the transmit code path to eliminate the internal
> buffering. Frames are immediately handed off to the DMA support rather
> than passing through an intermediate queue. ieee80211_(stop|wake)_queue()
> are used for flow control to stop receiving frames when a DMA tx ring is
> full.
>
> To handle aggregation the concept of AMPDU sessions is added to the
> driver. The AMPDU session allows MPDUs to be temporarily queued by the
> DMA code until either a full AMPDU has been collected or circumstances
> dictate that transmission should start with a partial AMPDU. The use of
> a queue ensures that the tx headers are fully initialized before the
> buffers are synced for DMA, and it allows non-aggregate frames to be
> immediately placed in the tx ring so that more frames can be collected
> for aggregation.
>
> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
> ---
>   drivers/net/wireless/brcm80211/brcmsmac/ampdu.c |  666 ++++++++++-------------
>   drivers/net/wireless/brcm80211/brcmsmac/ampdu.h |   29 +-
>   drivers/net/wireless/brcm80211/brcmsmac/dma.c   |  182 ++++++-
>   drivers/net/wireless/brcm80211/brcmsmac/dma.h   |    9 +-
>   drivers/net/wireless/brcm80211/brcmsmac/main.c  |  514 +++++------------
>   drivers/net/wireless/brcm80211/brcmsmac/main.h  |   39 +-
>   drivers/net/wireless/brcm80211/brcmsmac/types.h |    1 -
>   7 files changed, 628 insertions(+), 812 deletions(-)

Hi Seth,

The idea behind the change is a good move. However, this change is quite 
a overhaul especially for ampdu code. This makes it pretty hard to 
review. At least against the previous implementation. So we still need 
some time to chew on this one. We have been testing it and it looks good 
there.

Could you somehow split this change into smaller steps?

Gr. Avs