Re: [PATCH 1/1] New driver: rtl8xxxu (mac80211)

[Jes Sorensen <Jes.Sorensen@redhat.com>]

Johannes Berg <johannes@sipsolutions.net> writes:
> On Wed, 2015-09-02 at 21:59 -0400, Jes Sorensen wrote:
>> 
>> The code mimics what the vendor driver does - part of the bringup of the
>> chip sets a bunch of parameters from an register value array. Rather
>> than guessing what their default is, I feel it's safer to just shut 
>> it down here and stick with that.
>
> Ok, fair enough. Might be worth putting that into start() rather than
> add_interface() though? Anyway, it's not a big deal, I just wasn't sure
> if that was intentional.

OK, I'll probably just leave it there for now then.

>> > I'd be extremely surprised if any of this worked - perhaps you're 
>> > not advertising A-MPDU support (yet)?
>> > 
>> > You're not calling ieee80211_start_tx_ba_cb_irqsafe() or
>> > ieee80211_stop_tx_ba_cb_irqsafe(), so this can't really work. The
>> > session might be set up but will never actually start.
>> > 
>> > That said, the code also looks incomplete. Perhaps better to just
>> > remove it entirely for now?
>> 
>> I started working on this, but I guess I didn't finish it. One thing I
>> haven't figured out yet is why I see AMPDU_RX_START events, but I never
>> see AMPDU_TX_START events show in the log.
>
> Nothing in your system is calling ieee80211_start_tx_ba_session() then.

Gotcha, now you mention it, that rings a bell! I'll put that on my todo
list. One of the biggest things puzzling me is how much the firmware
does with AMPDU - there is a break bit I can set in the TX descriptor to
say 'don't AMPDU this packet' but do not actually know if it will do all
of this behind my back. I need to dig more into the vendor driver (and
their 80211 stack) to see if I can figure that out.

Cheers,
Jes