Maybe this is also missing in uboot driver and so AN times out (but works after AN is done) and cannot be restarted?
It was really something I missed when changing the read link status code to use the AN mmd device. Setting autoneg_complete should be part of this code as it is gathered from the same register
But it could also be an AN problem in u-boot… I am not familiar enough with u-boot to know when it does link-up/down and/or restarts AN.
I’m working on adapting the driver to U-Boot and giving it various firmware loading options (from MMC, SPI-NOR and UBI/SPI-NAND).
From what I can tell the one issue which is still unresolved is having to go through the whole reset → load-firmware → … cycle every time the link is set down and the missing 2500M link-partner advertisement bit which we were promised to have in some custom register. So that didn’t happen…?
TX rate adaption for 1000M link partners is not working that great apparently 
iperf3 from 1G-connected host to R3-mini via 2.5G-capable switch (link speed on R3-mini is 2.5G)
Connecting to host 192.168.5.169, port 5201
[ 5] local 192.168.1.136 port 38688 connected to 192.168.5.169 port 5201
[ ID] Interval Transfer Bitrate Retr Cwnd
[ 5] 0.00-1.00 sec 115 MBytes 962 Mbits/sec 59 889 KBytes
[ 5] 1.00-2.00 sec 113 MBytes 945 Mbits/sec 0 981 KBytes
[ 5] 2.00-3.00 sec 112 MBytes 941 Mbits/sec 0 1.04 MBytes
[ 5] 3.00-4.00 sec 113 MBytes 946 Mbits/sec 9 806 KBytes
[ 5] 4.00-5.00 sec 112 MBytes 941 Mbits/sec 0 912 KBytes
[ 5] 5.00-6.00 sec 112 MBytes 937 Mbits/sec 0 994 KBytes
[ 5] 6.00-7.00 sec 112 MBytes 936 Mbits/sec 0 1.05 MBytes
[ 5] 7.00-8.00 sec 113 MBytes 951 Mbits/sec 9 822 KBytes
[ 5] 8.00-9.00 sec 111 MBytes 931 Mbits/sec 0 921 KBytes
[ 5] 9.00-10.00 sec 112 MBytes 942 Mbits/sec 0 1011 KBytes
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval Transfer Bitrate Retr
[ 5] 0.00-10.00 sec 1.10 GBytes 943 Mbits/sec 77 sender
[ 5] 0.00-10.01 sec 1.10 GBytes 941 Mbits/sec receiver
iperf Done.
iperf3 from host to R3-mini connect direct to the R3-mini with 1G
[root@box filogic]# iperf3 -R -c 192.168.1.1
Connecting to host 192.168.1.1, port 5201
Reverse mode, remote host 192.168.1.1 is sending
[ 5] local 192.168.1.254 port 53862 connected to 192.168.1.1 port 5201
[ ID] Interval Transfer Bitrate
[ 5] 0.00-1.00 sec 92.1 MBytes 772 Mbits/sec
[ 5] 1.00-2.00 sec 94.8 MBytes 795 Mbits/sec
[ 5] 2.00-3.00 sec 91.0 MBytes 763 Mbits/sec
[ 5] 3.00-4.00 sec 95.6 MBytes 802 Mbits/sec
[ 5] 4.00-5.00 sec 85.0 MBytes 713 Mbits/sec
[ 5] 5.00-6.00 sec 88.5 MBytes 742 Mbits/sec
[ 5] 6.00-7.00 sec 84.6 MBytes 710 Mbits/sec
[ 5] 7.00-8.00 sec 90.0 MBytes 755 Mbits/sec
[ 5] 8.00-9.00 sec 98.4 MBytes 825 Mbits/sec
[ 5] 9.00-10.00 sec 90.9 MBytes 762 Mbits/sec
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval Transfer Bitrate Retr
[ 5] 0.00-10.00 sec 911 MBytes 764 Mbits/sec 4537 sender
[ 5] 0.00-10.00 sec 911 MBytes 764 Mbits/sec receiver
iperf Done.
So 764 MBit/s instead of 942 MBit/s just because the EN8811H is sending a few too many pause frames…
This part it apparently is not possible to fix.
You do not use the uboot driver from immortalwrt/my tree as base? This has firmware builtin via header file, not good?
But we should discuss this in the uboot thread
I will have to look in to this, I was not aware of this until yesterday. So I need to check this with original driver.
I tracked this all the way back to my first version (almost same as original) and with older firmware, still this issue.
Edit: So I was too hasty with my last post and deleted it.
It may not be the most elegant solution, but it seems to work. Based on the load firmware routine, found that the ‘host’ can be restarted like this:
static int en8811h_restart_host(struct phy_device *phydev)
{
int ret;
ret = air_buckpbus_reg_write(phydev, EN8811H_FW_CTRL_1,
EN8811H_FW_CTRL_1_START);
if (ret < 0)
return ret;
return air_buckpbus_reg_write(phydev, EN8811H_FW_CTRL_1,
EN8811H_FW_CTRL_1_FINISH);
}
So added:
if (!priv->fw_loaded) {
ret = en8811h_load_firmware(phydev);
if (ret < 0)
phydev_err(phydev, "Load firmware failed: %d\n", ret);
else
priv->fw_loaded = true;
} else {
ret = en8811h_restart_host(phydev);
}
if (ret < 0)
return ret;
But then still need to do the entire setup, but it is a lot quicker without loading the firmware files.
Edit:
So I cleaned up a bit, so that the code in en8811h_config_init() is clean:
if (!priv->fw_loaded)
ret = en8811h_load_firmware(phydev);
else
ret = en8811h_restart_host(phydev);
if (ret < 0)
return ret;
A few lines moved to: en8811h_load_firmware().
1 Like
Maybe this is interesting:
Updated firmware was posted linux-firmware
https://lore.kernel.org/linux-firmware/[email protected]/T/#u
Ok, tested it (eh8811h firmware 24011202) and now indeed 2500M LPA gets set if the link partner advertises 2500M. However, it doesn’t get cleared if the link partner no longer advertises it or even the cable is unplugged and a different link-partner is plugged in, then we see
[ 56.198597] Airoha EN8811H mdio-bus:0f: Downshift occurred from negotiated speed 2.5Gbps to actual speed 1Gbps, check cabling!
[ 56.209986] mtk_soc_eth 15100000.ethernet eth1: Link is Up - 1Gbps/Full - flow control rx/tx
I guess that will need another firmware fix…
Thanks for the tip…
Or… we clear the bit:
if (pbus_value & EN8811H_2P5G_LPA_2P5G)
linkmode_set_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT,
phydev->lp_advertising);
else
linkmode_clear_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT,
phydev->lp_advertising);
Edit:
Better to use linkmode_mod_bit()
linkmode_mod_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT,
phydev->lp_advertising,
pbus_value & EN8811H_2P5G_LPA_2P5G);
@dangowrt @frank-w @castiel652
So I guess it is about ready for rfc patch v2, if there are no further comments here…
Well, did you actually test that?
I’ve tried that and added some debugging code to actually validate the content of the register.
The full output including my debugging printout which I added to the (pbus_value & EN8811H_2P5G_LPA_2P5G) condition:
[ 56.191775] Airoha EN8811H mdio-bus:0f: EN8811H_2P5G_LPA_2P5G bit was set
[ 56.198597] Airoha EN8811H mdio-bus:0f: Downshift occurred from negotiated speed 2.5Gbps to actual speed 1Gbps, check cabling!
[ 56.209986] mtk_soc_eth 15100000.ethernet eth1: Link is Up - 1Gbps/Full - flow control rx/tx
(link partner is NetXtreme BCM5762 Gigabit Ethernet PCIe which doesn’t use 2.5G/5G/10G advertisement bits at all. But before another link partner with RTL8221BV PHY which does support and did advertise 2.5G was plugged.)
One comment is that we can simplify things a bit by storing the firmware version as u32 instead of bool firmware_loaded. All memory in the priv struct gets zerod on allocation anyway, and we should only set firmware_version once we read it and it was non-zero. Then we can use the firmware_version member just like the firmware_loaded member before (ie. decide whether to call en8811h_load_firmware or en8811h_restart_host) and we could even use work-arounds for known firmware bugs (such as missing 2.5G LPA…) if the version u32 (which luckily can be compared as in unsigned int in a meaningful way as in 0x24011202 > 0x23112102).
Maybe diff is better than English to explain what I mean 
Yes it works fine here.
The original code only setting the bit had genphy_c45_read_lpa() but this was changed. There the bit was always cleared. So this is why my old code was only setting the bit.
I was also thinking about using the firmware version… before firmware load it always reads a different number, but always 0x8bxxxxxx. Always negative then it would always be smaller than loaded firmware version.
I will look into your suggestions this weekend.
Edit:
@dangowrt I was thinking to drop the dirty bug fix lpa2500 totally If by any chance someone is still using old firmware, there is no big deal, only missing the lpa2500 bit…
Actually, my first thought of using the firmware version register, to use it to check if firmware is loaded or not. But I am not sure if the register value reliably can be used before any firmware is loaded (to determine it is not loaded). It is then always 0x8bxxxxxx, but I am but sure this will always in the future be the case …
I’m testing with your latest driver now and it does not work for me when switching cables from 2500M-capable link partner to Gigabit Ethernet link partner without any 802.3bz capabilities. See email for details and pull-request for a working work-around.