Re: [PATCH 1/8] mac80211: A-MPDU Rx add low level driver API

[Johannes Berg <johannes@sipsolutions.net>]

[-- Attachment #1: Type: text/plain, Size: 1849 bytes --]

Hi,

Cool stuff. Minor (really) comments below.

> +/**
> + * enum ieee80211_ampdu_mlme_flags - A-MPDU action flags
> + *
> + * These flags are used with the ampdu_action() callback in
> + * &struct ieee80211_ops to indicate which action is needed.
> + * @IEEE80211_AMPDU_RX_START: start Rx aggregation
> + * @IEEE80211_AMPDU_RX_STOP: stop Rx aggregation
> + */
> +enum ieee80211_ampdu_mlme_flags {
> +	IEEE80211_AMPDU_RX_START = 1<<0,
> +	IEEE80211_AMPDU_RX_STOP  = 1<<1,
> +};

Can both really happen at the same time? Or why are they flags rather
than just an action code?
 
> + * @ampdu_action: Ask low-level driver to perform a certain A-MPDU action

I think we usually just say "Perform a certain A-MPDU action" because
these comments are more for driver writers than mac80211 people and they
like to be told what to do ;)

> + * 	The RA/TID combination determines the destination and TID we want
> + * 	the ampdu action to be perfoemed for. The action is defined through

small typo: "performed"

> + * 	ieee80211_ampdu_mlme_flags. starting sequence number (ssn)

small typo: "Starting"

@ssn will highlight the variable name afaik. I need to get my docbook
stuff up to be mergeable.

> + * 	is the first frame we expect to perform the action on.
>   */
>  struct ieee80211_ops {
>  	int (*tx)(struct ieee80211_hw *hw, struct sk_buff *skb,
> @@ -1089,6 +1107,8 @@ struct ieee80211_ops {
>  			     struct ieee80211_tx_control *control);
>  	int (*tx_last_beacon)(struct ieee80211_hw *hw);
>  	int (*conf_ht)(struct ieee80211_hw *hw, struct ieee80211_conf *conf);
> +	int (*ampdu_action)(struct ieee80211_hw *hw, u8 action, const u8 *ra,
> +				u16 tid, u16 ssn);

I personally prefer indentation to right after the opening parenthesis
and that's the style over the whole struct.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]