[PATCH 0/2] eth: asix88179: Fixes for ASIX AX88179A variant
Hello U-Boot Maintainers,
I am submitting two patches that address issues with the ASIX AX88179A Ethernet driver in U-Boot. These patches resolve problems related to PHY hang during auto-negotiation and stalling when handling large fragmented packets for the ASIX AX88179A variant.
---
Example of AX88179A hang issue:
=> dhcp Waiting for Ethernet connection... unable to connect. Reset Ethernet Device Waiting for Ethernet connection... unable to connect. =>
---
Thanks, Khoa
Khoa Hoang (2): eth: asix88179: Fix ASIX AX88179A PHY hang eth: asix88179: Fix drop of large fragmented packets
drivers/usb/eth/asix88179.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) --
In the original code, the MII_ADVERTISE register is modified while auto-negotiation is still in progress, which causes the PHY to lock up with the ASIX AX88179A.
This patch disables auto-negotiation and resets the PHY before modifying the MII_ADVERTISE register, then restarts auto-negotiation afterward. This prevents the PHY from locking up.
Additionally, both AX88179 and AX88179A variants do not appear to support the ADVERTISE_LPACK (BIT(14)) in the MII_ADVERTISE register, as setting this bit does not seem to have any effect. While I do not have access to the AX88179{,A} datasheet, in most Ethernet PHYs, this bit is reserved as 0, so we should avoid setting it.
Signed-off-by: Khoa Hoang admin@khoahoang.com --- drivers/usb/eth/asix88179.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/eth/asix88179.c b/drivers/usb/eth/asix88179.c index 4bd3b9d1..a856aa2d 100644 --- a/drivers/usb/eth/asix88179.c +++ b/drivers/usb/eth/asix88179.c @@ -344,14 +344,22 @@ static int asix_basic_reset(struct ueth_data *dev, AX_MEDIUM_GIGAMODE | AX_MEDIUM_JUMBO_EN; asix_write_cmd(dev, AX_ACCESS_MAC, AX_MEDIUM_STATUS_MODE, 2, 2, tmp16);
+ /* Disable auto-negotiation and reset the PHY */ + *tmp16 = BMCR_RESET; + asix_write_cmd(dev, AX_ACCESS_PHY, 0x03, MII_BMCR, 2, tmp16); + u16 adv = 0; - adv = ADVERTISE_ALL | ADVERTISE_CSMA | ADVERTISE_LPACK | + adv = ADVERTISE_ALL | ADVERTISE_CSMA | ADVERTISE_NPAGE | ADVERTISE_PAUSE_ASYM | ADVERTISE_PAUSE_CAP; asix_write_cmd(dev, AX_ACCESS_PHY, 0x03, MII_ADVERTISE, 2, &adv);
adv = ADVERTISE_1000FULL; asix_write_cmd(dev, AX_ACCESS_PHY, 0x03, MII_CTRL1000, 2, &adv);
+ /* Restart auto-negotiation */ + *tmp16 = BMCR_ANENABLE | BMCR_ANRESTART | BMCR_RESET; + asix_write_cmd(dev, AX_ACCESS_PHY, 0x03, MII_BMCR, 2, tmp16); + return 0; }
On 10/27/24 6:12 AM, Khoa Hoang wrote:
In the original code, the MII_ADVERTISE register is modified while auto-negotiation is still in progress, which causes the PHY to lock up with the ASIX AX88179A.
This patch disables auto-negotiation and resets the PHY before modifying the MII_ADVERTISE register, then restarts auto-negotiation afterward. This prevents the PHY from locking up.
Additionally, both AX88179 and AX88179A variants do not appear to support the ADVERTISE_LPACK (BIT(14)) in the MII_ADVERTISE register, as setting this bit does not seem to have any effect. While I do not have access to the AX88179{,A} datasheet, in most Ethernet PHYs, this bit is reserved as 0, so we should avoid setting it.
Signed-off-by: Khoa Hoang admin@khoahoang.com
drivers/usb/eth/asix88179.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/eth/asix88179.c b/drivers/usb/eth/asix88179.c index 4bd3b9d1..a856aa2d 100644 --- a/drivers/usb/eth/asix88179.c +++ b/drivers/usb/eth/asix88179.c @@ -344,14 +344,22 @@ static int asix_basic_reset(struct ueth_data *dev, AX_MEDIUM_GIGAMODE | AX_MEDIUM_JUMBO_EN; asix_write_cmd(dev, AX_ACCESS_MAC, AX_MEDIUM_STATUS_MODE, 2, 2, tmp16);
- /* Disable auto-negotiation and reset the PHY */
- *tmp16 = BMCR_RESET;
Why not use the same trick as u16 adv below here ?
u16 reset = BMCR_RESET; asix_write_cmd(dev, AX_ACCESS_PHY, 0x03, MII_BMCR, 2, &reset);
- asix_write_cmd(dev, AX_ACCESS_PHY, 0x03, MII_BMCR, 2, tmp16);
- u16 adv = 0;
---- On Sun, 27 Oct 2024 07:44:59 -0700 Marek Vasut wrote ---
On 10/27/24 6:12 AM, Khoa Hoang wrote:
In the original code, the MII_ADVERTISE register is modified while auto-negotiation is still in progress, which causes the PHY to lock up with the ASIX AX88179A.
This patch disables auto-negotiation and resets the PHY before modifying the MII_ADVERTISE register, then restarts auto-negotiation afterward. This prevents the PHY from locking up.
Additionally, both AX88179 and AX88179A variants do not appear to support the ADVERTISE_LPACK (BIT(14)) in the MII_ADVERTISE register, as setting this bit does not seem to have any effect. While I do not have access to the AX88179{,A} datasheet, in most Ethernet PHYs, this bit is reserved as 0, so we should avoid setting it.
Signed-off-by: Khoa Hoang admin@khoahoang.com>
drivers/usb/eth/asix88179.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/eth/asix88179.c b/drivers/usb/eth/asix88179.c index 4bd3b9d1..a856aa2d 100644 --- a/drivers/usb/eth/asix88179.c +++ b/drivers/usb/eth/asix88179.c @@ -344,14 +344,22 @@ static int asix_basic_reset(struct ueth_data *dev, AX_MEDIUM_GIGAMODE | AX_MEDIUM_JUMBO_EN; asix_write_cmd(dev, AX_ACCESS_MAC, AX_MEDIUM_STATUS_MODE, 2, 2, tmp16);
+ /* Disable auto-negotiation and reset the PHY */ + *tmp16 = BMCR_RESET;
Why not use the same trick as u16 adv below here ?
u16 reset = BMCR_RESET; asix_write_cmd(dev, AX_ACCESS_PHY, 0x03, MII_BMCR, 2, &reset);
+ asix_write_cmd(dev, AX_ACCESS_PHY, 0x03, MII_BMCR, 2, tmp16);
- u16 adv = 0;
Sure, I can apply this change. I don’t have a strong preference for one approach over the other.
I noticed a mistake in these lines:
+ /* Restart auto-negotiation */ + *tmp16 = BMCR_ANENABLE | BMCR_ANRESTART | BMCR_RESET; + asix_write_cmd(dev, AX_ACCESS_PHY, 0x03, MII_BMCR, 2, tmp16);
The issue is that I set BMCR_RESET, which would reset the PHY’s settings. This should be changed to:
u16 bmcr = 0; asix_read_cmd(dev, AX_ACCESS_PHY, 0x03, MII_BMCR, 2, &bmcr); bmcr |= BMCR_ANENABLE | BMCR_ANRESTART; asix_write_cmd(dev, AX_ACCESS_PHY, 0x03, MII_BMCR, 2, tmp16);
This avoids resetting the PHY settings while enabling and restarting auto-negotiation correctly. Will test this change and update in next version.
Thanks, Khoa
On 10/28/24 12:41 AM, Khoa Hoang wrote:
---- On Sun, 27 Oct 2024 07:44:59 -0700 Marek Vasut wrote ---
On 10/27/24 6:12 AM, Khoa Hoang wrote:
In the original code, the MII_ADVERTISE register is modified while auto-negotiation is still in progress, which causes the PHY to lock up with the ASIX AX88179A.
This patch disables auto-negotiation and resets the PHY before modifying the MII_ADVERTISE register, then restarts auto-negotiation afterward. This prevents the PHY from locking up.
Additionally, both AX88179 and AX88179A variants do not appear to support the ADVERTISE_LPACK (BIT(14)) in the MII_ADVERTISE register, as setting this bit does not seem to have any effect. While I do not have access to the AX88179{,A} datasheet, in most Ethernet PHYs, this bit is reserved as 0, so we should avoid setting it.
Signed-off-by: Khoa Hoang admin@khoahoang.com>
drivers/usb/eth/asix88179.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/eth/asix88179.c b/drivers/usb/eth/asix88179.c index 4bd3b9d1..a856aa2d 100644 --- a/drivers/usb/eth/asix88179.c +++ b/drivers/usb/eth/asix88179.c @@ -344,14 +344,22 @@ static int asix_basic_reset(struct ueth_data *dev, AX_MEDIUM_GIGAMODE | AX_MEDIUM_JUMBO_EN; asix_write_cmd(dev, AX_ACCESS_MAC, AX_MEDIUM_STATUS_MODE, 2, 2, tmp16);
+ /* Disable auto-negotiation and reset the PHY */ + *tmp16 = BMCR_RESET;
Why not use the same trick as u16 adv below here ?
u16 reset = BMCR_RESET; asix_write_cmd(dev, AX_ACCESS_PHY, 0x03, MII_BMCR, 2, &reset);
+ asix_write_cmd(dev, AX_ACCESS_PHY, 0x03, MII_BMCR, 2, tmp16);
- u16 adv = 0;
Sure, I can apply this change. I don’t have a strong preference for one approach over the other.
I noticed a mistake in these lines:
- /* Restart auto-negotiation */
- *tmp16 = BMCR_ANENABLE | BMCR_ANRESTART | BMCR_RESET;
- asix_write_cmd(dev, AX_ACCESS_PHY, 0x03, MII_BMCR, 2, tmp16);
The issue is that I set BMCR_RESET, which would reset the PHY’s settings. This should be changed to:
u16 bmcr = 0; asix_read_cmd(dev, AX_ACCESS_PHY, 0x03, MII_BMCR, 2, &bmcr); bmcr |= BMCR_ANENABLE | BMCR_ANRESTART; asix_write_cmd(dev, AX_ACCESS_PHY, 0x03, MII_BMCR, 2, tmp16);
This avoids resetting the PHY settings while enabling and restarting auto-negotiation correctly. Will test this change and update in next version.
Nice !
When receiving large fragmented packets, the ASIX 88179A can drop packets due to an insufficient URB buffer size. This change adjusts the URB buffer size to match the configuration used in the Linux kernel, resolving packet drop issues observed when performing TFTP with a 16KB block size.
Signed-off-by: Khoa Hoang admin@khoahoang.com --- drivers/usb/eth/asix88179.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/eth/asix88179.c b/drivers/usb/eth/asix88179.c index a856aa2d..647a85a5 100644 --- a/drivers/usb/eth/asix88179.c +++ b/drivers/usb/eth/asix88179.c @@ -192,10 +192,10 @@ static const struct { unsigned char ctrl, timer_l, timer_h, size, ifg; } AX88179_BULKIN_SIZE[] = { - {7, 0x4f, 0, 0x02, 0xff}, - {7, 0x20, 3, 0x03, 0xff}, - {7, 0xae, 7, 0x04, 0xff}, - {7, 0xcc, 0x4c, 0x04, 8}, + {7, 0x4f, 0, 0x12, 0xff}, + {7, 0x20, 3, 0x16, 0xff}, + {7, 0xae, 7, 0x18, 0xff}, + {7, 0xcc, 0x4c, 0x18, 8}, };
/* driver private */ @@ -311,7 +311,7 @@ static int asix_basic_reset(struct ueth_data *dev, memcpy(tmp, &AX88179_BULKIN_SIZE[0], 5); asix_write_cmd(dev, AX_ACCESS_MAC, AX_RX_BULKIN_QCTRL, 5, 5, tmp);
- dev_priv->rx_urb_size = 128 * 20; + dev_priv->rx_urb_size = 1024 * 20;
/* Water Level configuration */ *tmp = 0x34;
On 10/27/24 6:12 AM, Khoa Hoang wrote:
When receiving large fragmented packets, the ASIX 88179A can drop packets due to an insufficient URB buffer size. This change adjusts the URB buffer size to match the configuration used in the Linux kernel, resolving packet drop issues observed when performing TFTP with a 16KB block size.
Is there a test case ? Can you please include it in the commit message ?
Thank you
---- On Sun, 27 Oct 2024 07:43:26 -0700 Marek Vasut wrote ---
On 10/27/24 6:12 AM, Khoa Hoang wrote:
When receiving large fragmented packets, the ASIX 88179A can drop packets due to an insufficient URB buffer size. This change adjusts the URB buffer size to match the configuration used in the Linux kernel, resolving packet drop issues observed when performing TFTP with a 16KB block size.
Is there a test case ? Can you please include it in the commit message ?
Thank you
To reproduce this issue, set the following configurations in the config file:
CONFIG_IP_DEFRAG=y CONFIG_TFTP_BLOCKSIZE=16352 CONFIG_TFTP_WINDOWSIZE=1468
Then, run tftpboot, and it will fail to load with a timeout error: => tftpboot zImage Using axg0 device TFTP from server 192.168.128.130; our IP address is 192.168.128.244 Filename 'zImage'. Load address: 0x20000000 Loading: ##T T T T T T Retry count exceeded; starting again
I’ll include this information in the commit message in the next version.
On 10/28/24 12:14 AM, Khoa Hoang wrote:
---- On Sun, 27 Oct 2024 07:43:26 -0700 Marek Vasut wrote ---
On 10/27/24 6:12 AM, Khoa Hoang wrote:
When receiving large fragmented packets, the ASIX 88179A can drop packets due to an insufficient URB buffer size. This change adjusts the URB buffer size to match the configuration used in the Linux kernel, resolving packet drop issues observed when performing TFTP with a 16KB block size.
Is there a test case ? Can you please include it in the commit message ?
Thank you
To reproduce this issue, set the following configurations in the config file:
CONFIG_IP_DEFRAG=y CONFIG_TFTP_BLOCKSIZE=16352 CONFIG_TFTP_WINDOWSIZE=1468
Then, run tftpboot, and it will fail to load with a timeout error: => tftpboot zImage Using axg0 device TFTP from server 192.168.128.130; our IP address is 192.168.128.244 Filename 'zImage'. Load address: 0x20000000 Loading: ##T T T T T T Retry count exceeded; starting again
I’ll include this information in the commit message in the next version.
Thank you!
participants (2)
-
Khoa Hoang -
Marek Vasut