openwrt / mt76

mac80211 driver for MediaTek MT76x0e, MT76x2e, MT7603, MT7615, MT7628 and MT7688
747 stars 342 forks source link

MT7628 frequent disconnects #423

Closed mcondarelli closed 4 years ago

mcondarelli commented 4 years ago

It's very likely I'm not asking in the right place, but I found no better; please redirect as needed.

I am trying to "upgrade" a system based on VoCore2 (which comes with an ancient OpenWRT version and an archaic version of the proprietary driver (mt_wifi.ko). I do not need full OpenWRT as WiFi is used just for Internet connection (station mode).

I have a fully "modern" system (Buildroot build with up-to-date U-Boot and stock Linux kernel v5.7). This works as expected but for WiFi connection which seems to have extremely low sensitivity (I see A.P. only if it is at arm length) and with frequent disconnections.

I also tried to build this driver with my kernel (as it seems aggressively maintained), but compilation bombed:

/home/mcon/vocore/__V2__/Buildroot-2/recov/build/mt76-master/./agg-rx.c: In function ‘mt76_rx_aggr_stop’:
/home/mcon/vocore/__V2__/Buildroot-2/recov/build/mt76-master/./agg-rx.c:293:2: error: implicit declaration of function ‘rcu_swap_protected’; did you mean ‘mem_cgroup_protected’? [-Werror=implicit-function-declaration]
  rcu_swap_protected(wcid->aggr[tidno], tid,
  ^~~~~~~~~~~~~~~~~~
  mem_cgroup_protected
/home/mcon/vocore/__V2__/Buildroot-2/recov/build/mt76-master/./agg-rx.c:294:7: error: implicit declaration of function ‘lockdep_is_held’; did you mean ‘lockdep_rtnl_is_held’? [-Werror=implicit-function-declaration]
       lockdep_is_held(&dev->mutex));
       ^~~~~~~~~~~~~~~
       lockdep_rtnl_is_held

... so I'm not sure this driver is really compatible with stock kernel (Linus tree).

I'm fully prepared to give all details, but please confirm this is the right place to ask. I saw several threads reporting essentially the same problem I have and that's why I ventured asking.

Many Thanks in Advance.

Vonger commented 4 years ago

I guess that issue is caused by VoCore2 default factory setting is using 1T1R(one antenna), but driver default is 2T2R. So when mt76 driver start, it will make MCS > 7, cause the speed can not be stable. I am trying to fix this recently, is there anyone can give a hand where to start? such a big project and I am totally noob for this wifi driver part :)

ryderlee1110 commented 4 years ago

I guess that issue is caused by VoCore2 default factory setting is using 1T1R(one antenna), but driver default is 2T2R. So when mt76 driver start, it will make MCS > 7, cause the speed can not be stable. I am trying to fix this recently, is there anyone can give a hand where to start? such a big project and I am totally noob for this wifi driver part :)

If you think it is related to antenna then https://github.com/openwrt/mt76/blob/master/mt7603/init.c#L540

mcondarelli commented 4 years ago

Shouldn't antenna configuration be part of DT? MT7688 has only one, but nobody guarantees both MT7628 antennas are actually connected. In case of VoCore2 "normal case" is only internal (solid state) antenna is connected, but we might want to disable it IFF external antenna is plugged in. It is enough to have dev->mphy.antenna_mask = 1; or dev->mphy.antenna_mask = 2; respectively or we must touch something else?

ryderlee1110 commented 4 years ago

You can add a .set_antenna() callback. Or either way, read value from OTP to config antenna_mask.

mcondarelli commented 4 years ago

I can confirm situation is much better with the following tentative patch (tests are ongoing to ascertain problem is really solved and not just mitigated):

diff --git a/drivers/net/wireless/mediatek/mt76/mt7603/init.c b/drivers/net/wireless/mediatek/mt76/mt7603/init.c
index 94196599797e..b006261d00bd 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7603/init.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7603/init.c
@@ -537,9 +537,13 @@ int mt7603_register_device(struct mt7603_dev *dev)
             (unsigned long)dev);

    /* Check for 7688, which only has 1SS */
-   dev->mphy.antenna_mask = 3;
-   if (mt76_rr(dev, MT_EFUSE_BASE + 0x64) & BIT(4))
+   if (IS_ENABLED(CONFIG_DTB_VOCORE2)) {
        dev->mphy.antenna_mask = 1;
+   } else {
+       dev->mphy.antenna_mask = 3;
+       if (mt76_rr(dev, MT_EFUSE_BASE + 0x64) & BIT(4))
+           dev->mphy.antenna_mask = 1;
+   }

    dev->slottime = 9;
    dev->sensitivity_limit = 28;

@Vonger: can You contact me OOB so we can decide how to actually implement a proper fix? IMHO we should implement a .set_antenna() callback and read relevant info from EEPROM ("factory" partition).

@ryderlee1110: Is this the right place to send patch for this? (once it's rightly done and tested, of course)

ryderlee1110 commented 4 years ago

Is there any OTP field can be used to distinguish VOCORE2 from mt7628?

mcondarelli commented 4 years ago

If You are referring to "E-Fuse Configuration" register (at 0x10000008) that is, on my VoCore2, 00010000 (I doubt this is useful, but @Vonger should know better as he is from VoCore2 vendor firm).

I was considering either some custom value in DT or using EEPROM values (most likely `ANT_DIV_CTRL: Antenna Diversity control -> 10: Fix antenna at main antenna).

Any hint welcome (of course).

mcondarelli commented 4 years ago

@ryderlee1110: I am unsure if using OTP is the right way to handle this, at least in the general case.

VoCore2 board has one SMD antenna soldered directly on the board (connected to MT7628 "main" antenna port) and a connector for the second (auxiliary) antenna. While the vast majority of users will use the internal antenna we have two other cases we should handle:

IMHO we should handle choice using device driver optional argument options, something like: insmod mt7603e --antenna="main" (or ="aux" or ="both", possibly defaulting to "both").

What is Your opinion?

dangowrt commented 4 years ago

Afaik an optional device-tree property would be the way to go. That can also be changed at run-time using device-tree overlays.

ryderlee1110 commented 4 years ago

If you think DT is better then you can post a patch for public review.

Vonger commented 4 years ago

@ryderlee1110 I try to update the value to 1 and it works good now :) one antenna speed stable at 50Mbps. Thanks for the help. Currently from datasheet, this config is in flash factory setting partition, so if we move it to DTS, that config will not effective anymore. Does the mt76 driver still read the factory setting parameter to modify the wifi output? If so, could you let me know the file position? I can use that way first. DTS way maybe later.

Vonger commented 4 years ago

I find it out :) mt76 driver do not use any data from factory setting except the MAC address. and antenna select is at eeprom address MT_EE_NIC_CONF_2(0x42) 0x22 for two antennas 2T2R, 0x11 for one antenna 1T1R, it is ignored.

So we can just use that byte. And this mt76 driver is using cal free setting, it works, but not that good as one use mt7628 driver with cal, from my test around 5% speed difference, not perfect but much much better than before.

PS: eeprom 0x34 RX_PATH, TX_PATH and 0x36 WF1_AUX, WF0_AUX looks like the 1T1R/2T2R control bit too...

Vonger commented 4 years ago

Done, please review https://github.com/openwrt/mt76/pull/426

@@ -277,6 +277,9 @@ mt7603_init_hardware(struct mt7603_dev *dev)
    if (ret < 0)
        return ret;

+   if (((u8*)dev->mphy.eeprom.data)[MT_EE_NIC_CONF_0] == 0x11)
+       dev->mphy.antenna_mask = 1;
+
    ret = mt7603_dma_init(dev);
    if (ret)
        return ret;
ryderlee1110 commented 4 years ago

Post it publicly. Can we close this issue?

nbd168 commented 4 years ago

Should be fixed in the latest version

ajcollett commented 4 years ago

I am somewhat confused, the merge isn't closed yet, how is it fixed in the latest version?

namidairo commented 4 years ago

See https://github.com/openwrt/mt76/commit/97e65131440ccae916e76323251b80cafdfc9006 and https://github.com/openwrt/mt76/commit/b36d7ae096a3d8c7d6a8a246f2e8a471a467041e

ajcollett commented 4 years ago

Thank you. I have tried snapshot on my VoCore2 and it looks like wifi is now stable out the box. Thanks!