Importing if_mtw from OpenBSD
Details
Details
Diff Detail
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Skipped - Unit
Tests Skipped
Event Timeline
Comment Actions
Also add mtw to /usr/src/sbin/devd/devd.conf it escaped my diff.. The firmware port is at https://people.freebsd.org/~jsm/mt7601u-firmware-kmod.tar.gz (It should have the license file set just for ref for now)
Comment Actions
oh, interesting! does 11n work on this driver in openbsd? I see some 11n stuff is commented out here.
Comment Actions
I'll add the back reference to the PR: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=247545
And leave my comment here: based on the comments there this driver was written based on GPL-only sources (at least I could not find anything else). For whatever that means I do not know.
| share/man/man4/mtw.4 | ||
|---|---|---|
| 4 | Are we sure that is correct? | |
| 40 | There's a lot of blank lines everywhere... | |
| 80 | "based on"? | |
| sys/modules/usb/mtw/Makefile | ||
| 2 | Normally we do not add this to Makefiles but if you do can you add the SPDX tag in the first line so it matches the teemplate in /usr/share/examples/etc/bsd-copyright (typed from the top of my head)? | |
Comment Actions
I haven't had time to look at the code, but tested on my Xiaomi mini USB WiFi:
mtw0 on uhub1
mtw0: <MediaTek MI WLAN Adapter, class 0/0, rev 2.01/0.00, addr 3> on usbus0
mtw0: version:0x7601
mtw0: loaded firmware ver 30272.256
mtw0: timeout waiting for MCU to initialize
mtw0: /usr/src/sys/dev/usb/wlan/if_mtw.c:3000 USB_ERR_CANCELLED
device_attach: mtw0 attach returned 6
Unfortunately it doesn't work.
My /boot/loader.conf:
mt7601u_fw_load="YES"
if_mtw_load="YES"
Comment Actions
Thanks for testing. I did not manage to get hold of any other devices than a noname one where I never got usb err canceled loading firmware.
. . Is it consistent when unplugging/replugging the device. Can you perhaps post a usbconfig dump and a usbdump. Thanks.
| sys/dev/usb/wlan/if_mtw.c | ||
|---|---|---|
| 141 | This list is too short. Here's one to add: ugen0.2: <MediaTek Edimax Wi-Fi> at usbus0, cfg=0 md=HOST spd=HIGH (480Mbps) pwr=ON (160mA) bLength = 0x0012 bDescriptorType = 0x0001 bcdUSB = 0x0201 bDeviceClass = 0x0000 <Probed by interface class> bDeviceSubClass = 0x0000 bDeviceProtocol = 0x0000 bMaxPacketSize0 = 0x0040 idVendor = 0x7392 idProduct = 0x7710 bcdDevice = 0x0000 iManufacturer = 0x0001 <MediaTek> iProduct = 0x0002 <Edimax Wi-Fi> iSerialNumber = 0x0003 <1.0> bNumConfigurations = 0x0001 | |
| 147 | If you need to do "eject" handling; we added a USB Quirk for that a while ago (I think for a Zyxel) which also has firmware "on board" and shows up as cd0 first and only when that was ejected the wifi device would show up. Found it 9d2d04462d1080f4a4b84f674de90a895ce1bcc1 . Hope that helps. | |
| sys/modules/usb/Makefile | ||
| 48 | There's a blank at the end of the line (now). | |
| sys/dev/usb/usbdevs | ||
|---|---|---|
| 4049 | This is already present a couple of lines above... as "RT7601" | |
Comment Actions
some ht 20 support. fix ratectl and no need for the short transfer usb hack to avoid device timeout.
Comment Actions
Had it successfully running on
Dec 29 21:48:12 bcm kernel: ugen0.2: <MediaTek 802.11 n WLAN> at usbus0 Dec 29 21:48:12 bcm kernel: mtw0 on uhub0 Dec 29 21:48:12 bcm kernel: mtw0: <MediaTek 802.11 n WLAN, class 0/0, rev 2.01/0.00, addr 4> on usbus0 Dec 29 21:48:12 bcm kernel: mtw0: version:0x7601 Dec 29 21:48:12 bcm kernel: mtw0: firmware size:45412 Dec 29 21:48:12 bcm kernel: mtw0: loaded firmware ver 30272.256 Dec 29 21:48:12 bcm kernel: mtw0: mcu reaady 1 Dec 29 21:48:12 bcm kernel: mtw0: EEPROM RF rev=0x7601 chains=1T1R Dec 29 21:48:12 bcm kernel: mtw0: EEPROM rev=0, FAE=13 Dec 29 21:48:12 bcm kernel: mtw0: EEPROM CFG 0x1004 Dec 29 21:48:12 bcm kernel: mtw0: frequency offset 0x5f
for a while, though "ifconfig wlan0 destroy" whilst associated was enough to panic the host. I've not yet investigated this.
| share/man/man4/mtw.4 | ||
|---|---|---|
| 67 | ||
| sys/dev/usb/wlan/if_mtw.c | ||
| 22 | No longer needed | |
| 65–67 | I don't think you need this | |
| 136 | Add a blank line before this one | |
| 305–310 | It looks like you have the same defines (named MTW_RIDX_CCK1 etc) in if_mtwreg.h which is arguably the correct place as it's with the rt2860_rates array that they need to stay in sync with. However the RT2860_RIDX_* and MTW_RIDX_* are used interchangeably. Move the comment to that file, delete these RT2860_RIDX_* defines from here, and use MTW_RIDX_* everywhere. | |
| 313–322 | This #define already exists (same name, better formatting) in if_mtwreg.h, delete it from here | |
| 327–329 | Leftover comment from run(4)? | |
| 541 | Remove this blank line | |
| 626 | This looks wrong, Linux gets the ASIC version from MT_ASIC_VER register not MTW_MAC_VER_ID (and bases quite a few later tests on the ASIC version rather than the MAC version). I suspect this may not matter for the MT7601 but might for the MT7610/MT7612. | |
| 663 | The manpage says only station mode is supported? From a quick test monitor mode doesn't seem to work for me | |
| 670 | I don't think the hardware supports IEEE80211_C_FF | |
| 671 | Bad intentation | |
| 675 | mtw_ampdu_enable() is marked TODO so probably not good to announce support yet | |
| 679–681 | It doesn't look like there's code anywhere to support these yet either? | |
| 682 | _S is a shift value and shouldn't be OR'd in like this. | |
| 683 | I don't think you support SMPS either? | |
| 1201 | You have the MTW_LOCK held at this point (from mtw_attach() line 597) so get a lock order reversal here. firmware_get(4) says it must not be called with any locks held. | |
| 1763 | Linux drivers/net/wireless/mediatek/mt7601u/mcu.h has a table of command names, it would be good to use something similar rather than magic numbers (e.g. 16 = CMD_LED_MODE) | |
| 1887 | Bad intentation | |
| 3102–3103 | run(4) leftovers? | |
| 3135 | I suspect this should be MTW_PHY_HT unless GF is needed? | |
| 3249–3252 | Bad indentation, excess blank lines | |
| 3257 | Bad indentation | |
| 3721 | MTW_UNLOCK needed here? | |
| 3977 | This is different from OpenBSD behaviour (which has the for() as an empty loop) and I suspect it's wrong here. OpenBSD matches Linux. | |
| 4189–4190 | Leftover from run(4)? | |
| 4775 | ||
| sys/modules/usb/run/Makefile | ||
| 34 ↗ | (On Diff #148527) | Looks like this is unrelated |
Comment Actions
I'd like to urge a little caution with the formatting changes. If they are from the porting, do it. If they are leftovers from run, but still in OpenBSD, I'd suggest fixing those with a round trip through upstream. If upstream is unresponsive, I'd be tempted to do them as a followup commit so future imports would be easier. We have some good docs in the git history about why there appear to be gratuitous diffs with upstream. These would be hard to capture in the initial commit message.
Comment Actions
It's looking better, can you delete some of the if_run function definitions that aren't in the openbsd rtw driver?
| sys/dev/usb/wlan/if_mtw.c | ||
|---|---|---|
| 233 | are these all holdovers from if_run or something? can you just toss em? | |
Comment Actions
I think I got most of Gavins, comments, also I got monitor mode working, and put that into the man page. I set force short xfer to 0 again in the TX_BE but I am not sure it relates to device timeouts, I got one under non heavy load. Before ht mode it solved consistent device timeouts.
Comment Actions
mostly style. removed some comments, and device_printf, tries to reset on detach for better firmware reloading, but not consistently solving the reinitialization problem.