Hi Ron, Thanks for the explaining comments in your 0/10 mail, that really helps. A few small comments. > +/** > + * ieee80211_start_tx_ba_session - Start a tx Block Ack session. > + * @hw: pointer as obtained from ieee80211_alloc_hw(). > + * @ra: destination address of the BA session recipient > + * @tid: the TID to BA on. > + * @return: success if addBA request was sent, failure otherwise > + * > + * Although mac80211/low level driver/user space application can estimate > + * the need to start aggregation on a certain RA/TID, the session level > + * will be managed by the mac80211. > + */ > +int ieee80211_start_tx_ba_session(struct ieee80211_hw *hw, u8 *ra, u16 tid); It seems to me that this should be per-virtual interface instead of per-hardware? I guess ultimately it won't make a difference because the RA will be unique but I think we should still keep it per-vif where it makes sense. > +/** > + * ieee80211_start_tx_ba_cb/_irqsafe - low level driver is ready to aggregate. > + * @hw: pointer as obtained from ieee80211_alloc_hw(). > + * @ra: destination address of the BA session recipient. > + * @tid: the TID to BA on. > + * > + * This function must be called by low level driver once it has > + * finished with preparations for the BA session. > + * Two versions of the function are provided for low-deriver's convinience, > + * a regular and irqsafe version. > + */ > +void ieee80211_start_tx_ba_cb(struct ieee80211_hw *hw, u8 *ra, u16 tid); > +void ieee80211_start_tx_ba_cb_irqsafe(struct ieee80211_hw *hw, u8 *ra, > + u16 tid); You actually need to write kerneldoc in two different comments so it can be embedded into output properly. I know, I should be writing the mac80211 book... I'll get around to it again, I promise :) Could you help with an 11n part of the book that explains how all this session stuff works? Just some text, possibly within the header file as /** * DOC: Aggregation * *... */ Also, should we just call it _irq instead of _irqsafe? I personally dislike the rx_irqsafe naming too. > +/** > + * ieee80211_stop_tx_ba_session - Stop a Block Ack session. > + * @hw: pointer as obtained from ieee80211_alloc_hw(). > + * @ra: destination address of the BA session recipient > + * @tid: the TID to stop BA. > + * @initiator: if indicates initiator DELBA frame will be sent. > + * @return: error if no sta with matching da found, success otherwise DA? The variable is called RA though. > + * Although mac80211/low level driver/user space application can estimate > + * the need to stop aggregation on a certain RA/TID, the session level > + * will be managed by the mac80211. > + */ > +int ieee80211_stop_tx_ba_session(struct ieee80211_hw *hw, u8 *ra, > + u16 tid, u8 initiator); Shouldn't the "initiator" argument get a proper enum type? > +/** > + * ieee80211_stop_BA_cb - low level driver is ready to stop aggregation. > + * @hw: pointer as obtained from ieee80211_alloc_hw(). > + * @ra: destination address of the BA session recipient. > + * @tid: the desired TID to BA on. > + * > + * This function must be called by low level driver once it has > + * finished with preparations for the BA session tear down. > + * Two versions of the function are provided for low-deriver's convinience, Typo, should be "convenience". Same as with the other one though, you need to copy the kerneldoc and explain them separately. > ret = local->ops->ampdu_action(hw, IEEE80211_AMPDU_RX_STOP, > - ra, tid, EINVAL); > + ra, tid, NULL); I think the kerneldoc should mention that ssn can be NULL for the RX_STOP notification. johannes
Attachment:
signature.asc
Description: This is a digitally signed message part
