On Mon, 2008-10-20 at 14:42 +0800, Zhu Yi wrote:
> > @@ -318,9 +348,33 @@ static void iwl3945_rx_reply_tx(struct i
> > }
> >
> > info = IEEE80211_SKB_CB(txq->txb[txq->q.read_ptr].skb[0]);
> > - memset(&info->status, 0, sizeof(info->status));
> > + ieee80211_tx_info_clear_status(info);
> > +
> > + /* Fill the MRR chain with some info about on-chip retransmissions */
> > + rate_idx = iwl3945_hwrate_to_plcp_idx(tx_resp->rate);
> > + if (info->band == IEEE80211_BAND_5GHZ)
> > + rate_idx -= IWL_FIRST_OFDM_RATE;
> > +
> > + info->status.rates[0].count = tx_resp->failure_frame + 1;
>
> This is useless. And it is even confusable with the later count++.
In what way useless? We've changed the semantics and made "count" be the
"transmit count" rather than the "# of retries", so it has to be one
more. Or was there a bug in the previous understanding? Wouldn't
surprise me, and we can fix the bug here.
> > + fail = tx_resp->failure_frame;
> > + for(i = 0; i < 4; i++) {
>
> Where does the '4' come from? Why not IEEE80211_TX_MAX_RATES?
>
> > + int next = iwl3945_rs_next_rate(priv, rate_idx);
> > +
> > + info->status.rates[i].idx = rate_idx;
> > +
> > + if ((rate_idx == next) || (i == 3)) {
>
> ditto
Probably should be MAX_RATES, but I'm not sure, can you explain how the
hardware will behave for retries?
> [...]
>
> Other iwlwifi changes looks fine to me.
Thanks for looking.
johannes
Attachment:
signature.asc
Description: This is a digitally signed message part
