Re: [PATCH 1/8] mac80211: A-MPDU Rx add low level driver API
From: Johannes Berg <johannes@sipsolutions.net>
Date: 2007-12-17 17:54:25
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.
quoted hunk ↗ jump to hunk
+ * 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
Attachments
- signature.asc [application/pgp-signature] 828 bytes