[PATCH 1/2] eth: asix88179: Fix ASIX AX88179A PHY hang

[Khoa Hoang]

---- 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 at 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