urtwn: add rate control support for RTL8188EU

[avos]

Test Plan

Tested with:

• RTL8188EU, STA and HOSTAP modes.
• RTL8188CUS, STA mode.

[kevlo]

Works for me. Tested on amd64 with a D-Link DWA-125 rev D1 (RTL8188ETV).
BTW, would you add copyright for you? Thanks. :)

[avos]

Fix possible LOR / recursion on non-recursive mutex.

[avos]

Add a missing newline character in a DPRINTF.

[adrian]

sys/dev/usb/wlan/if_urtwn.c

580 ↗

(On Diff #10941)

There's no need to conditionify this; it's cheap to just always create/destroy the lock.

660 ↗

(On Diff #10941)

are there other chips that support rate control?

normally I'd define this as a driver flag so we can set it for more chips; or a #define like URTWN_CHIP_HAS_RATECTL(). That way it's easy to use grep on that flag/define to find the places which are rate control specific.

2111 ↗

(On Diff #10941)

shouldn't the rate control code be setting up the initial rate?

2262 ↗

(On Diff #10941)

What's ridx an index into?

[avos]

sys/dev/usb/wlan/if_urtwn.c

660 ↗

(On Diff #10941)

are there other chips that support rate control?

Yes (but they use slightly different way to report TX stats).
+ firmware supports an alternative method for it (rates bitmap + rssi per MAC id) (used by OpenBSD).

2111 ↗

(On Diff #10941)

it seems to be already handled by ieee80211_ratectl_node_init()

2262 ↗

(On Diff #10941)

rate index (there is already ridx2rate array in the if_urtwnreg.h)

[avos]

• Create / destroy node table lock even when it is not needed.
• Add URTWN_CHIP_HAS_RATECTL() macro.

[adrian]

sys/dev/usb/wlan/if_urtwn.c

2110 ↗

(On Diff #10946)

Ah okay. That's likely why ni_txrate is initialised here; just to have some default rate show up in ifconfig output when ratectl_init() isn't called and the firmware is doing the rate control.

sys/dev/usb/wlan/if_urtwnvar.h

163 ↗

(On Diff #10946)

You can do !! (thing) to have it turn into true/false rather than the above!

[kevlo]

sys/dev/usb/wlan/if_urtwn.c

658 ↗

(On Diff #10946)

IIRC, rtl8188eu's firmware also use that method, no?

[adrian]

sys/dev/usb/wlan/if_urtwn.c

2331 ↗

(On Diff #10946)

ok, this following bit just hard-codes the rate to be 11mbit (CCK) or 54mbit (OFDM.)

Why can't we do rate control for other chips? Do we not get transmit completion notifications from other firmware?

2339 ↗

(On Diff #10946)

So this isn't entirely true - the rate control code may decide to select a CCK rate when it's OFDM - that's the nature of 11bg. (strictly speaking same as 11n - you can actually do CCK and 11g OFDM rates in 11n mode if the non-MCS rates are part of the legacy rate set.)

So, should the 'raid' assignment should be based on the rate being selected, /not/ the operating mode of the chip? I think this specifies which mode to transmit with, right?

2367 ↗

(On Diff #10946)

this is another confusing bit. For some of these chips (if_rsu too) the macid defaults to BSS because we only have one session. In hostap mode it will support multiple macids.

So, why don't we just always assign macid = un->id, and we always populate the URTWN_NODE() regardless of chipset type, and we assign the macid at node creation time appropriately? That'll make the code easier to understand.

(And, we could likely then break out the node create/destroy path and the macid assign path in said URTWN_NODE path as a separate, self-contained diff. I may even take a crack at that tonight.)

2379 ↗

(On Diff #10946)

hm, interesting. The CCX_RPT - do we know what the format of that report is? It'd be nice to get more debugging info from this NIC!

3889 ↗

(On Diff #10946)

This is for hostap operation, right?

4230 ↗

(On Diff #10946)

Ok, so not all the urtwn chips support some form of per-packet TX completion notification? Do they have their own rate control in firmware then?

[avos]

sys/dev/usb/wlan/if_urtwn.c

2331 ↗

(On Diff #10946)

Yes, other chips will not send this event (or I don't know how to enable it). If I correctly understand Realtek's driver - we need to poll REG_C2HEVT_CLEAR - and, if any info is available, read it from REG_C2HEVT_MSG_NORMAL.

P.S. this notification is not from firmware (i.e., you can safely drop urtwn_load_firmware() for RTL8188EU, because it's not used anywhere for it right now)

2367 ↗

(On Diff #10946)

urtwn_ra_init() doesn't initialize other id's - that's why it is separated here.
(however, I have not seen any effect from it - probably, something is missing / incomplete)

2379 ↗

(On Diff #10946)

Yes, it's documented:
RTL8188EU - include/rtl8188e_xmit.h (txrpt_ccx_88e structure)
other chips - include/rtl8192c_xmit.h (txrpt_ccx_8192c structure)

3889 ↗

(On Diff #10946)

For every (except monitor) mode

[avos]

Update URTWN_CHIP_HAS_RATECTL() macro

[adrian]

ok, I'm going to approve it.

Let's next look at the 'raid = ' bits to see if we need to set that based on the selected rate, rather than the operating mode. (because as I said, you can transmit CCK rates during OFDM operation.. :-)

[avos]

sys/dev/usb/wlan/if_urtwn.c

2333 ↗

(On Diff #11153)

Probably, the reason is in urtwn_init():

if (sc->chip & URTWN_CHIP_88E) {
                   ...
                usb_err = urtwn_write_1(sc, R92C_USB_SPECIAL_OPTION,
                    urtwn_read_1(sc, R92C_USB_SPECIAL_OPTION) |
                    R92C_USB_SPECIAL_OPTION_INT_BULK_SEL);
                    // documentation says that this bit forces to use bulk
                    // endpoint for interrupt packets
                    ...
        } else {
                     // and it doesn't work for RTL8188CUS (device hangs)
        }

There is an additional endpoint for 8188CUS (in headers it is marked as '4: interrupt in'); however, there is no useful information in messages from it right now.