diff mbox

"asix: Don't reset PHY on if_up for ASIX 88772" breaks net onarndale platform

Message ID C89EFD3CD56F64468D3D206D683A8D22039FFC5F@ldam-msx2.fugro-nl.local (mailing list archive)
State New, archived
Headers show

Commit Message

Stam, Michel [FINT] Nov. 5, 2014, 12:04 p.m. UTC
Hello Charles,

After looking around I found the reset value for the 8772 chip, which
seems to be 0x1E1 (ANAR register).

This equates to (according to include/uapi/linux/mii.h)
ADVERTISE_ALL | ADVERTISE_CSMA.

The register only seems to become 0 if the software reset fails.

Unfortunately, this is exactly what I get when the patch is applied;
asix 1-2:1.0 eth1: Failed to send software reset: ffffffb5
asix 1-2:1.0 eth1: link reset failed (-75) usbnet usb-0000:00:1d.0-2, 
ASIX AX88772 USB 2.0 Ethernet
asix 1-2:1.0 eth1: Failed to send software reset: ffffffb5
asix 1-2:1.0 eth1: link reset failed (-75) usbnet usb-0000:00:1d.0-2, 
ASIX AX88772 USB 2.0 Ethernet

A little while after this its 'Failed to enable software MII access'.
The ethernet device fails to see any link or accept any ethtool -s
command.

My device has vid:devid 0b95:772a (ASIX Elec. Corp.).

Can you tell me what device is on the Andale platform, Charles? Same
vendor/device id?

Another thing that bothers me is that dev->mii.advertising seems to
contain the same value, so maybe that can be used instead of a call to
asix_mdio_read(). Can anyone comment on its purpose? Should it be a
shadow copy of the real register or something?

Riku, can you test Charles' patch as well?

Kind regards,

Michel Stam

-----Original Message-----
From: Charles Keepax [mailto:ckeepax@opensource.wolfsonmicro.com] 
Sent: Tuesday, November 04, 2014 21:09 PM
To: Stam, Michel [FINT]
Cc: Riku Voipio; davem@davemloft.net; linux-usb@vger.kernel.org;
netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
linux-samsung-soc@vger.kernel.org
Subject: Re: "asix: Don't reset PHY on if_up for ASIX 88772" breaks net
onarndale platform

On Tue, Nov 04, 2014 at 11:23:06AM +0100, Stam, Michel [FINT] wrote:
> Hello Riku,
> 
> >Fixing a bug (ethtool support) must not cause breakage elsewhere (in
> this case on arndale). This is now a regression of functionality from 
> 3.17.
> >
> >I think it would better to revert the change now and with less hurry
> introduce a ethtool fix that doesn't break arndale.
> 
> I don't fully agree here;
> I would like to point out that this commit is a revert itself. Fixing 
> the armdale will then cause breakage in other implementations, such as

> ours. Blankly reverting breaks other peoples' implementations.
> 
> The PHY reset is the thing that breaks ethtool support, so any fix 
> that appeases all would have to take existing PHY state into account.
> 
> I'm not an expert on the ASIX driver, nor the MII, but I think this is

> the cause;
> drivers/net/usb/asix_devices.c:
>    361      asix_mdio_write(dev->net, dev->mii.phy_id, MII_BMCR,
> BMCR_RESET);
>    362      asix_mdio_write(dev->net, dev->mii.phy_id, MII_ADVERTISE,
>    363              ADVERTISE_ALL | ADVERTISE_CSMA);
>    364      mii_nway_restart(&dev->mii);
> 
> I would think that the ADVERTISE_ALL is the cause here, as it will 
> reset the MII back to default, thus overriding ethtool settings.
> Would an:
> Int reg;
> reg = asix_mdio_read(dev->net,dev->mii.phy_id,MII_ADVERTISE);
> 
> prior to the offending lines, and then;
> 
>    362      asix_mdio_write(dev->net, dev->mii.phy_id, MII_ADVERTISE,
>    363             reg);
> 
> solve the problem for you guys?

If I revert the patch in question and add this in:

        msleep(150);

        asix_mdio_write(dev->net, dev->mii.phy_id, MII_BMCR,
BMCR_RESET);
-       asix_mdio_write(dev->net, dev->mii.phy_id, MII_ADVERTISE,
-                       ADVERTISE_ALL | ADVERTISE_CSMA);
+       reg = asix_mdio_read(dev->net, dev->mii.phy_id, MII_ADVERTISE);
+       if (!reg)
+               reg = ADVERTISE_ALL | ADVERTISE_CSMA;
+       asix_mdio_write(dev->net, dev->mii.phy_id, MII_ADVERTISE, reg);
        mii_nway_restart(&dev->mii);

        ret = asix_write_medium_mode(dev, AX88772_MEDIUM_DEFAULT);

Then things work on Arndale for me. Does that work for you?
Whether that is a sensible fix I don't know however.

> 
> Mind, maybe the read function should take into account the reset value

> of the MII, and set it to ADVERTISE_ALL | ADVERTISE_CSMA. I don't have

> any documention here at the moment.

Yeah I also have no documentation.

Thanks,
Charles

> 
> Is anyone able to confirm my suspicions?
> 
> Kind regards,
> 
> Michel Stam
> -----Original Message-----
> From: Riku Voipio [mailto:riku.voipio@iki.fi]
> Sent: Tuesday, November 04, 2014 10:44 AM
> To: Stam, Michel [FINT]
> Cc: Riku Voipio; davem@davemloft.net; linux-usb@vger.kernel.org; 
> netdev@vger.kernel.org; linux-kernel@vger.kernel.org; 
> linux-samsung-soc@vger.kernel.org; ckeepax@opensource.wolfsonmicro.com
> Subject: Re: "asix: Don't reset PHY on if_up for ASIX 88772" breaks 
> net on arndale platform
> 
> On Tue, Nov 04, 2014 at 09:19:26AM +0100, Stam, Michel [FINT] wrote:
> > Interesting, as the commit itself is a revert from a kernel back to
> > 2.6 somewhere. The problem I had is related to the PHY being reset 
> > on interface-up, can you confirm that you require this?
> 
> I can't confirm what exactly is needed on arndale. I'm neither expert 
> in USB or ethernet. However, I can confirm that without the PHY reset,

> networking doesn't work on arndale.
> 
> I now see someone else has the same problem, adding Charles to CC.
> 
> http://www.spinics.net/lists/linux-usb/msg116656.html
> 
> > Reverting this
> > breaks ethtool support in turn.
> 
> Fixing a bug (ethtool support) must not cause breakage elsewhere (in 
> this case on arndale). This is now a regression of functionality from 
> 3.17.
> 
> I think it would better to revert the change now and with less hurry 
> introduce a ethtool fix that doesn't break arndale.
>  
> > Kind regards,
> > 
> > Michel Stam
> > 
> > -----Original Message-----
> > From: Riku Voipio [mailto:riku.voipio@iki.fi]
> > Sent: Tuesday, November 04, 2014 8:23 AM
> > To: davem@davemloft.net; Stam, Michel [FINT]
> > Cc: linux-usb@vger.kernel.org; netdev@vger.kernel.org; 
> > linux-kernel@vger.kernel.org; linux-samsung-soc@vger.kernel.org
> > Subject: "asix: Don't reset PHY on if_up for ASIX 88772" breaks net 
> > on
> 
> > arndale platform
> > 
> > Hi,
> > 
> > With 3.18-rc3, asix on arndale (samsung exynos 5250 based board), 
> > fails to work. Interface is initialized but network traffic seem not

> > to pass through. With kernel IP config the result looks like:
> > 
> > [    3.323275] usb 3-3.2.4: new high-speed USB device number 4 using
> > exynos-ehci
> > [    3.419151] usb 3-3.2.4: New USB device found, idVendor=0b95,
> > idProduct=772a
> > [    3.424735] usb 3-3.2.4: New USB device strings: Mfr=1,
Product=2,
> > SerialNumber=3
> > [    3.432196] usb 3-3.2.4: Product: AX88772 
> > [    3.436279] usb 3-3.2.4: Manufacturer: ASIX Elec. Corp.
> > [    3.441486] usb 3-3.2.4: SerialNumber: 000001
> > [    3.447530] asix 3-3.2.4:1.0 (unnamed net_device)
(uninitialized):
> > invalid hw address, using random
> > [    3.764352] asix 3-3.2.4:1.0 eth0: register 'asix' at
> > usb-12110000.usb-3.2.4, ASIX AX88772 USB 2.0 Ethernet,
> de:a2:66:bf:ca:4f
> > [    4.488773] asix 3-3.2.4:1.0 eth0: link down
> > [    5.690025] asix 3-3.2.4:1.0 eth0: link up, 100Mbps, full-duplex,
> lpa
> > 0xC5E1
> > [    5.712947] Sending DHCP requests ...... timed out!
> > [   83.165303] IP-Config: Retrying forever (NFS root)...
> > [   83.170397] asix 3-3.2.4:1.0 eth0: link up, 100Mbps, full-duplex,
> lpa
> > 0xC5E1
> > [   83.192944] Sending DHCP requests .....
> > 
> > Similar results also with dhclient. Git bisect identified the 
> > breaking
> 
> > commit as:
> > 
> > commit 3cc81d85ee01e5a0b7ea2f4190e2ed1165f53c31
> > Author: Michel Stam <m.stam@fugro.nl>
> > Date:   Thu Oct 2 10:22:02 2014 +0200
> > 
> >     asix: Don't reset PHY on if_up for ASIX 88772
> > 
> > Taking 3.18-rc3 and that commit reverted, network works again:
> > 
> > [    3.303500] usb 3-3.2.4: new high-speed USB device number 4 using
> > exynos-ehci
> > [    3.399375] usb 3-3.2.4: New USB device found, idVendor=0b95,
> > idProduct=772a
> > [    3.404963] usb 3-3.2.4: New USB device strings: Mfr=1,
Product=2,
> > SerialNumber=3
> > [    3.412424] usb 3-3.2.4: Product: AX88772 
> > [    3.416508] usb 3-3.2.4: Manufacturer: ASIX Elec. Corp.
> > [    3.421715] usb 3-3.2.4: SerialNumber: 000001
> > [    3.427755] asix 3-3.2.4:1.0 (unnamed net_device)
(uninitialized):
> > invalid hw address, using random
> > [    3.744837] asix 3-3.2.4:1.0 eth0: register 'asix' at
> > usb-12110000.usb-3.2.4, ASIX AX88772 USB 2.0 Ethernet,
> 12:59:f1:a8:43:90
> > [    7.098998] asix 3-3.2.4:1.0 eth0: link up, 100Mbps, full-duplex,
> lpa
> > 0xC5E1
> > [    7.118258] Sending DHCP requests ., OK
> > [    9.753259] IP-Config: Got DHCP answer from 192.168.1.1, my
address
> > is 192.168.1.111
> > 
> > There might something wrong on the samsung platform code (I 
> > understand
> 
> > the USB on arndale is "funny"), but this is still an regression from

> > 3.17.
> > 
> > Riku
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Riku Voipio Nov. 5, 2014, 12:39 p.m. UTC | #1
Hi Michel,

On Wed, Nov 05, 2014 at 01:04:37PM +0100, Stam, Michel [FINT] wrote:
> After looking around I found the reset value for the 8772 chip, which
> seems to be 0x1E1 (ANAR register).
> 
> This equates to (according to include/uapi/linux/mii.h)
> ADVERTISE_ALL | ADVERTISE_CSMA.
> 
> The register only seems to become 0 if the software reset fails.
> 
> Unfortunately, this is exactly what I get when the patch is applied;
> asix 1-2:1.0 eth1: Failed to send software reset: ffffffb5
> asix 1-2:1.0 eth1: link reset failed (-75) usbnet usb-0000:00:1d.0-2, 
> ASIX AX88772 USB 2.0 Ethernet
> asix 1-2:1.0 eth1: Failed to send software reset: ffffffb5
> asix 1-2:1.0 eth1: link reset failed (-75) usbnet usb-0000:00:1d.0-2, 
> ASIX AX88772 USB 2.0 Ethernet
> 
> A little while after this its 'Failed to enable software MII access'.
> The ethernet device fails to see any link or accept any ethtool -s
> command.

> My device has vid:devid 0b95:772a (ASIX Elec. Corp.).

> Can you tell me what device is on the Andale platform, Charles? Same
> vendor/device id?

Bus 003 Device 004: ID 0b95:772a ASIX Electronics Corp. AX88772A Fast Ethernet

> Another thing that bothers me is that dev->mii.advertising seems to
> contain the same value, so maybe that can be used instead of a call to
> asix_mdio_read(). Can anyone comment on its purpose? Should it be a
> shadow copy of the real register or something?
 
> Riku, can you test Charles' patch as well?

With that patch + revert to ax88772_reset network works. I'm unable to get
ethtool to work with that patch or with the original 3.17 state of asix.
Once i disable autoneg network doesn't just work. 

> > >I think it would better to revert the change now and with less hurry
> > introduce a ethtool fix that doesn't break arndale.

> > I don't fully agree here;
> > I would like to point out that this commit is a revert itself. Fixing 
> > the armdale will then cause breakage in other implementations, such as
> > ours. Blankly reverting breaks other peoples' implementations.

My concern is, that none of us with this problem is a linux network
drivers expert. And no such expert joined the thread to help us. Thus if
we hurry to have proper fix for 3.18, our fix might easily be really
wrong.

Hence, it would seem safer to revert to 3.17 state before 3.18, so we
can propose a proper fix for 3.19. At least from our myopic view, having
no functioning net on arndale is worse than having non-functioning
ethtool (which doesn't seem to have bothered people for years).

Riku

> > The PHY reset is the thing that breaks ethtool support, so any fix 
> > that appeases all would have to take existing PHY state into account.
> > 
> > I'm not an expert on the ASIX driver, nor the MII, but I think this is
> 
> > the cause;
> > drivers/net/usb/asix_devices.c:
> >    361      asix_mdio_write(dev->net, dev->mii.phy_id, MII_BMCR,
> > BMCR_RESET);
> >    362      asix_mdio_write(dev->net, dev->mii.phy_id, MII_ADVERTISE,
> >    363              ADVERTISE_ALL | ADVERTISE_CSMA);
> >    364      mii_nway_restart(&dev->mii);
> > 
> > I would think that the ADVERTISE_ALL is the cause here, as it will 
> > reset the MII back to default, thus overriding ethtool settings.
> > Would an:
> > Int reg;
> > reg = asix_mdio_read(dev->net,dev->mii.phy_id,MII_ADVERTISE);
> > 
> > prior to the offending lines, and then;
> > 
> >    362      asix_mdio_write(dev->net, dev->mii.phy_id, MII_ADVERTISE,
> >    363             reg);
> > 
> > solve the problem for you guys?
> 
> If I revert the patch in question and add this in:
> 
> --- a/drivers/net/usb/asix_devices.c
> +++ b/drivers/net/usb/asix_devices.c
> @@ -299,6 +299,7 @@ static int ax88772_reset(struct usbnet *dev)  {
>         struct asix_data *data = (struct asix_data *)&dev->data;
>         int ret, embd_phy;
> +       int reg;
>         u16 rx_ctl;
> 
>         ret = asix_write_gpio(dev,
> @@ -359,8 +360,10 @@ static int ax88772_reset(struct usbnet *dev)
>         msleep(150);
> 
>         asix_mdio_write(dev->net, dev->mii.phy_id, MII_BMCR,
> BMCR_RESET);
> -       asix_mdio_write(dev->net, dev->mii.phy_id, MII_ADVERTISE,
> -                       ADVERTISE_ALL | ADVERTISE_CSMA);
> +       reg = asix_mdio_read(dev->net, dev->mii.phy_id, MII_ADVERTISE);
> +       if (!reg)
> +               reg = ADVERTISE_ALL | ADVERTISE_CSMA;
> +       asix_mdio_write(dev->net, dev->mii.phy_id, MII_ADVERTISE, reg);
>         mii_nway_restart(&dev->mii);
> 
>         ret = asix_write_medium_mode(dev, AX88772_MEDIUM_DEFAULT);
> 
> Then things work on Arndale for me. Does that work for you?
> Whether that is a sensible fix I don't know however.
> 
> > 
> > Mind, maybe the read function should take into account the reset value
> 
> > of the MII, and set it to ADVERTISE_ALL | ADVERTISE_CSMA. I don't have
> 
> > any documention here at the moment.
> 
> Yeah I also have no documentation.
> 
> Thanks,
> Charles
> 
> > 
> > Is anyone able to confirm my suspicions?
> > 
> > Kind regards,
> > 
> > Michel Stam
> > -----Original Message-----
> > From: Riku Voipio [mailto:riku.voipio@iki.fi]
> > Sent: Tuesday, November 04, 2014 10:44 AM
> > To: Stam, Michel [FINT]
> > Cc: Riku Voipio; davem@davemloft.net; linux-usb@vger.kernel.org; 
> > netdev@vger.kernel.org; linux-kernel@vger.kernel.org; 
> > linux-samsung-soc@vger.kernel.org; ckeepax@opensource.wolfsonmicro.com
> > Subject: Re: "asix: Don't reset PHY on if_up for ASIX 88772" breaks 
> > net on arndale platform
> > 
> > On Tue, Nov 04, 2014 at 09:19:26AM +0100, Stam, Michel [FINT] wrote:
> > > Interesting, as the commit itself is a revert from a kernel back to
> > > 2.6 somewhere. The problem I had is related to the PHY being reset 
> > > on interface-up, can you confirm that you require this?
> > 
> > I can't confirm what exactly is needed on arndale. I'm neither expert 
> > in USB or ethernet. However, I can confirm that without the PHY reset,
> 
> > networking doesn't work on arndale.
> > 
> > I now see someone else has the same problem, adding Charles to CC.
> > 
> > http://www.spinics.net/lists/linux-usb/msg116656.html
> > 
> > > Reverting this
> > > breaks ethtool support in turn.
> > 
> > Fixing a bug (ethtool support) must not cause breakage elsewhere (in 
> > this case on arndale). This is now a regression of functionality from 
> > 3.17.
> > 
> > I think it would better to revert the change now and with less hurry 
> > introduce a ethtool fix that doesn't break arndale.
> >  
> > > Kind regards,
> > > 
> > > Michel Stam
> > > 
> > > -----Original Message-----
> > > From: Riku Voipio [mailto:riku.voipio@iki.fi]
> > > Sent: Tuesday, November 04, 2014 8:23 AM
> > > To: davem@davemloft.net; Stam, Michel [FINT]
> > > Cc: linux-usb@vger.kernel.org; netdev@vger.kernel.org; 
> > > linux-kernel@vger.kernel.org; linux-samsung-soc@vger.kernel.org
> > > Subject: "asix: Don't reset PHY on if_up for ASIX 88772" breaks net 
> > > on
> > 
> > > arndale platform
> > > 
> > > Hi,
> > > 
> > > With 3.18-rc3, asix on arndale (samsung exynos 5250 based board), 
> > > fails to work. Interface is initialized but network traffic seem not
> 
> > > to pass through. With kernel IP config the result looks like:
> > > 
> > > [    3.323275] usb 3-3.2.4: new high-speed USB device number 4 using
> > > exynos-ehci
> > > [    3.419151] usb 3-3.2.4: New USB device found, idVendor=0b95,
> > > idProduct=772a
> > > [    3.424735] usb 3-3.2.4: New USB device strings: Mfr=1,
> Product=2,
> > > SerialNumber=3
> > > [    3.432196] usb 3-3.2.4: Product: AX88772 
> > > [    3.436279] usb 3-3.2.4: Manufacturer: ASIX Elec. Corp.
> > > [    3.441486] usb 3-3.2.4: SerialNumber: 000001
> > > [    3.447530] asix 3-3.2.4:1.0 (unnamed net_device)
> (uninitialized):
> > > invalid hw address, using random
> > > [    3.764352] asix 3-3.2.4:1.0 eth0: register 'asix' at
> > > usb-12110000.usb-3.2.4, ASIX AX88772 USB 2.0 Ethernet,
> > de:a2:66:bf:ca:4f
> > > [    4.488773] asix 3-3.2.4:1.0 eth0: link down
> > > [    5.690025] asix 3-3.2.4:1.0 eth0: link up, 100Mbps, full-duplex,
> > lpa
> > > 0xC5E1
> > > [    5.712947] Sending DHCP requests ...... timed out!
> > > [   83.165303] IP-Config: Retrying forever (NFS root)...
> > > [   83.170397] asix 3-3.2.4:1.0 eth0: link up, 100Mbps, full-duplex,
> > lpa
> > > 0xC5E1
> > > [   83.192944] Sending DHCP requests .....
> > > 
> > > Similar results also with dhclient. Git bisect identified the 
> > > breaking
> > 
> > > commit as:
> > > 
> > > commit 3cc81d85ee01e5a0b7ea2f4190e2ed1165f53c31
> > > Author: Michel Stam <m.stam@fugro.nl>
> > > Date:   Thu Oct 2 10:22:02 2014 +0200
> > > 
> > >     asix: Don't reset PHY on if_up for ASIX 88772
> > > 
> > > Taking 3.18-rc3 and that commit reverted, network works again:
> > > 
> > > [    3.303500] usb 3-3.2.4: new high-speed USB device number 4 using
> > > exynos-ehci
> > > [    3.399375] usb 3-3.2.4: New USB device found, idVendor=0b95,
> > > idProduct=772a
> > > [    3.404963] usb 3-3.2.4: New USB device strings: Mfr=1,
> Product=2,
> > > SerialNumber=3
> > > [    3.412424] usb 3-3.2.4: Product: AX88772 
> > > [    3.416508] usb 3-3.2.4: Manufacturer: ASIX Elec. Corp.
> > > [    3.421715] usb 3-3.2.4: SerialNumber: 000001
> > > [    3.427755] asix 3-3.2.4:1.0 (unnamed net_device)
> (uninitialized):
> > > invalid hw address, using random
> > > [    3.744837] asix 3-3.2.4:1.0 eth0: register 'asix' at
> > > usb-12110000.usb-3.2.4, ASIX AX88772 USB 2.0 Ethernet,
> > 12:59:f1:a8:43:90
> > > [    7.098998] asix 3-3.2.4:1.0 eth0: link up, 100Mbps, full-duplex,
> > lpa
> > > 0xC5E1
> > > [    7.118258] Sending DHCP requests ., OK
> > > [    9.753259] IP-Config: Got DHCP answer from 192.168.1.1, my
> address
> > > is 192.168.1.111
> > > 
> > > There might something wrong on the samsung platform code (I 
> > > understand
> > 
> > > the USB on arndale is "funny"), but this is still an regression from
> 
> > > 3.17.
> > > 
> > > Riku
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Charles Keepax Nov. 5, 2014, 3:02 p.m. UTC | #2
On Wed, Nov 05, 2014 at 01:04:37PM +0100, Stam, Michel [FINT] wrote:
> Hello Charles,
> 
> After looking around I found the reset value for the 8772 chip, which
> seems to be 0x1E1 (ANAR register).
> 
> This equates to (according to include/uapi/linux/mii.h)
> ADVERTISE_ALL | ADVERTISE_CSMA.
> 
> The register only seems to become 0 if the software reset fails.

Odd it definitely reads back as zero on Arndale. I am guessing
that the root of the problem here is that for some reason Arndale
POR of the ethernet is pants and it needs a full software reset
before it will work and the patch removes the full reset
callback.

> 
> Unfortunately, this is exactly what I get when the patch is applied;
> asix 1-2:1.0 eth1: Failed to send software reset: ffffffb5
> asix 1-2:1.0 eth1: link reset failed (-75) usbnet usb-0000:00:1d.0-2, 
> ASIX AX88772 USB 2.0 Ethernet
> asix 1-2:1.0 eth1: Failed to send software reset: ffffffb5
> asix 1-2:1.0 eth1: link reset failed (-75) usbnet usb-0000:00:1d.0-2, 
> ASIX AX88772 USB 2.0 Ethernet

Ok so I am guessing you have a value in the register which is
neither the reset value or 0 and this causing problems later in
the reset/on the next reset. I do find the naming confusing in
the error message there as it says link reset failed but the
link_reset callback can't fail in the driver and I modified the
reset callback. But I guess that is just oddities of the network
stack I am not familiar with.

The other thing that feels odd is (and again apologies as I know
next to nothing about the networking stack) how come it is
unexpected that the reset callback destroys the state of the
device. Naively I would have expected that a reset callback would
reset the device back to its default state. Here we seem to be
trying to avoid that happening.

> 
> A little while after this its 'Failed to enable software MII access'.
> The ethernet device fails to see any link or accept any ethtool -s
> command.
> 
> My device has vid:devid 0b95:772a (ASIX Elec. Corp.).
> 
> Can you tell me what device is on the Andale platform, Charles? Same
> vendor/device id?

Yeah mine also idVendor=0b95, idProduct=772a

Thanks,
Charles

> 
> Another thing that bothers me is that dev->mii.advertising seems to
> contain the same value, so maybe that can be used instead of a call to
> asix_mdio_read(). Can anyone comment on its purpose? Should it be a
> shadow copy of the real register or something?
> 
> Riku, can you test Charles' patch as well?
> 
> Kind regards,
> 
> Michel Stam
> 
> -----Original Message-----
> From: Charles Keepax [mailto:ckeepax@opensource.wolfsonmicro.com] 
> Sent: Tuesday, November 04, 2014 21:09 PM
> To: Stam, Michel [FINT]
> Cc: Riku Voipio; davem@davemloft.net; linux-usb@vger.kernel.org;
> netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> linux-samsung-soc@vger.kernel.org
> Subject: Re: "asix: Don't reset PHY on if_up for ASIX 88772" breaks net
> onarndale platform
> 
> On Tue, Nov 04, 2014 at 11:23:06AM +0100, Stam, Michel [FINT] wrote:
> > Hello Riku,
> > 
> > >Fixing a bug (ethtool support) must not cause breakage elsewhere (in
> > this case on arndale). This is now a regression of functionality from 
> > 3.17.
> > >
> > >I think it would better to revert the change now and with less hurry
> > introduce a ethtool fix that doesn't break arndale.
> > 
> > I don't fully agree here;
> > I would like to point out that this commit is a revert itself. Fixing 
> > the armdale will then cause breakage in other implementations, such as
> 
> > ours. Blankly reverting breaks other peoples' implementations.
> > 
> > The PHY reset is the thing that breaks ethtool support, so any fix 
> > that appeases all would have to take existing PHY state into account.
> > 
> > I'm not an expert on the ASIX driver, nor the MII, but I think this is
> 
> > the cause;
> > drivers/net/usb/asix_devices.c:
> >    361      asix_mdio_write(dev->net, dev->mii.phy_id, MII_BMCR,
> > BMCR_RESET);
> >    362      asix_mdio_write(dev->net, dev->mii.phy_id, MII_ADVERTISE,
> >    363              ADVERTISE_ALL | ADVERTISE_CSMA);
> >    364      mii_nway_restart(&dev->mii);
> > 
> > I would think that the ADVERTISE_ALL is the cause here, as it will 
> > reset the MII back to default, thus overriding ethtool settings.
> > Would an:
> > Int reg;
> > reg = asix_mdio_read(dev->net,dev->mii.phy_id,MII_ADVERTISE);
> > 
> > prior to the offending lines, and then;
> > 
> >    362      asix_mdio_write(dev->net, dev->mii.phy_id, MII_ADVERTISE,
> >    363             reg);
> > 
> > solve the problem for you guys?
> 
> If I revert the patch in question and add this in:
> 
> --- a/drivers/net/usb/asix_devices.c
> +++ b/drivers/net/usb/asix_devices.c
> @@ -299,6 +299,7 @@ static int ax88772_reset(struct usbnet *dev)  {
>         struct asix_data *data = (struct asix_data *)&dev->data;
>         int ret, embd_phy;
> +       int reg;
>         u16 rx_ctl;
> 
>         ret = asix_write_gpio(dev,
> @@ -359,8 +360,10 @@ static int ax88772_reset(struct usbnet *dev)
>         msleep(150);
> 
>         asix_mdio_write(dev->net, dev->mii.phy_id, MII_BMCR,
> BMCR_RESET);
> -       asix_mdio_write(dev->net, dev->mii.phy_id, MII_ADVERTISE,
> -                       ADVERTISE_ALL | ADVERTISE_CSMA);
> +       reg = asix_mdio_read(dev->net, dev->mii.phy_id, MII_ADVERTISE);
> +       if (!reg)
> +               reg = ADVERTISE_ALL | ADVERTISE_CSMA;
> +       asix_mdio_write(dev->net, dev->mii.phy_id, MII_ADVERTISE, reg);
>         mii_nway_restart(&dev->mii);
> 
>         ret = asix_write_medium_mode(dev, AX88772_MEDIUM_DEFAULT);
> 
> Then things work on Arndale for me. Does that work for you?
> Whether that is a sensible fix I don't know however.
> 
> > 
> > Mind, maybe the read function should take into account the reset value
> 
> > of the MII, and set it to ADVERTISE_ALL | ADVERTISE_CSMA. I don't have
> 
> > any documention here at the moment.
> 
> Yeah I also have no documentation.
> 
> Thanks,
> Charles
> 
> > 
> > Is anyone able to confirm my suspicions?
> > 
> > Kind regards,
> > 
> > Michel Stam
> > -----Original Message-----
> > From: Riku Voipio [mailto:riku.voipio@iki.fi]
> > Sent: Tuesday, November 04, 2014 10:44 AM
> > To: Stam, Michel [FINT]
> > Cc: Riku Voipio; davem@davemloft.net; linux-usb@vger.kernel.org; 
> > netdev@vger.kernel.org; linux-kernel@vger.kernel.org; 
> > linux-samsung-soc@vger.kernel.org; ckeepax@opensource.wolfsonmicro.com
> > Subject: Re: "asix: Don't reset PHY on if_up for ASIX 88772" breaks 
> > net on arndale platform
> > 
> > On Tue, Nov 04, 2014 at 09:19:26AM +0100, Stam, Michel [FINT] wrote:
> > > Interesting, as the commit itself is a revert from a kernel back to
> > > 2.6 somewhere. The problem I had is related to the PHY being reset 
> > > on interface-up, can you confirm that you require this?
> > 
> > I can't confirm what exactly is needed on arndale. I'm neither expert 
> > in USB or ethernet. However, I can confirm that without the PHY reset,
> 
> > networking doesn't work on arndale.
> > 
> > I now see someone else has the same problem, adding Charles to CC.
> > 
> > http://www.spinics.net/lists/linux-usb/msg116656.html
> > 
> > > Reverting this
> > > breaks ethtool support in turn.
> > 
> > Fixing a bug (ethtool support) must not cause breakage elsewhere (in 
> > this case on arndale). This is now a regression of functionality from 
> > 3.17.
> > 
> > I think it would better to revert the change now and with less hurry 
> > introduce a ethtool fix that doesn't break arndale.
> >  
> > > Kind regards,
> > > 
> > > Michel Stam
> > > 
> > > -----Original Message-----
> > > From: Riku Voipio [mailto:riku.voipio@iki.fi]
> > > Sent: Tuesday, November 04, 2014 8:23 AM
> > > To: davem@davemloft.net; Stam, Michel [FINT]
> > > Cc: linux-usb@vger.kernel.org; netdev@vger.kernel.org; 
> > > linux-kernel@vger.kernel.org; linux-samsung-soc@vger.kernel.org
> > > Subject: "asix: Don't reset PHY on if_up for ASIX 88772" breaks net 
> > > on
> > 
> > > arndale platform
> > > 
> > > Hi,
> > > 
> > > With 3.18-rc3, asix on arndale (samsung exynos 5250 based board), 
> > > fails to work. Interface is initialized but network traffic seem not
> 
> > > to pass through. With kernel IP config the result looks like:
> > > 
> > > [    3.323275] usb 3-3.2.4: new high-speed USB device number 4 using
> > > exynos-ehci
> > > [    3.419151] usb 3-3.2.4: New USB device found, idVendor=0b95,
> > > idProduct=772a
> > > [    3.424735] usb 3-3.2.4: New USB device strings: Mfr=1,
> Product=2,
> > > SerialNumber=3
> > > [    3.432196] usb 3-3.2.4: Product: AX88772 
> > > [    3.436279] usb 3-3.2.4: Manufacturer: ASIX Elec. Corp.
> > > [    3.441486] usb 3-3.2.4: SerialNumber: 000001
> > > [    3.447530] asix 3-3.2.4:1.0 (unnamed net_device)
> (uninitialized):
> > > invalid hw address, using random
> > > [    3.764352] asix 3-3.2.4:1.0 eth0: register 'asix' at
> > > usb-12110000.usb-3.2.4, ASIX AX88772 USB 2.0 Ethernet,
> > de:a2:66:bf:ca:4f
> > > [    4.488773] asix 3-3.2.4:1.0 eth0: link down
> > > [    5.690025] asix 3-3.2.4:1.0 eth0: link up, 100Mbps, full-duplex,
> > lpa
> > > 0xC5E1
> > > [    5.712947] Sending DHCP requests ...... timed out!
> > > [   83.165303] IP-Config: Retrying forever (NFS root)...
> > > [   83.170397] asix 3-3.2.4:1.0 eth0: link up, 100Mbps, full-duplex,
> > lpa
> > > 0xC5E1
> > > [   83.192944] Sending DHCP requests .....
> > > 
> > > Similar results also with dhclient. Git bisect identified the 
> > > breaking
> > 
> > > commit as:
> > > 
> > > commit 3cc81d85ee01e5a0b7ea2f4190e2ed1165f53c31
> > > Author: Michel Stam <m.stam@fugro.nl>
> > > Date:   Thu Oct 2 10:22:02 2014 +0200
> > > 
> > >     asix: Don't reset PHY on if_up for ASIX 88772
> > > 
> > > Taking 3.18-rc3 and that commit reverted, network works again:
> > > 
> > > [    3.303500] usb 3-3.2.4: new high-speed USB device number 4 using
> > > exynos-ehci
> > > [    3.399375] usb 3-3.2.4: New USB device found, idVendor=0b95,
> > > idProduct=772a
> > > [    3.404963] usb 3-3.2.4: New USB device strings: Mfr=1,
> Product=2,
> > > SerialNumber=3
> > > [    3.412424] usb 3-3.2.4: Product: AX88772 
> > > [    3.416508] usb 3-3.2.4: Manufacturer: ASIX Elec. Corp.
> > > [    3.421715] usb 3-3.2.4: SerialNumber: 000001
> > > [    3.427755] asix 3-3.2.4:1.0 (unnamed net_device)
> (uninitialized):
> > > invalid hw address, using random
> > > [    3.744837] asix 3-3.2.4:1.0 eth0: register 'asix' at
> > > usb-12110000.usb-3.2.4, ASIX AX88772 USB 2.0 Ethernet,
> > 12:59:f1:a8:43:90
> > > [    7.098998] asix 3-3.2.4:1.0 eth0: link up, 100Mbps, full-duplex,
> > lpa
> > > 0xC5E1
> > > [    7.118258] Sending DHCP requests ., OK
> > > [    9.753259] IP-Config: Got DHCP answer from 192.168.1.1, my
> address
> > > is 192.168.1.111
> > > 
> > > There might something wrong on the samsung platform code (I 
> > > understand
> > 
> > > the USB on arndale is "funny"), but this is still an regression from
> 
> > > 3.17.
> > > 
> > > Riku
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stam, Michel [FINT] Nov. 5, 2014, 4:17 p.m. UTC | #3
Hello Charles,

First of all, my apologies. I manually applied your patch and made a
mistake; I swapped ax88772_link_reset with ax88772_reset for struct
driver_info_ax88772_info structure, which caused the software reset
failures. Blame it on a lack of coffee... Please disregard my previous
mail.

After (correctly) applying the patch I still don't get the desired
results; see below.

Test situation:
ifconfig eth1 down
ethtool -s advertise 1
ifconfig eth1 up

Check dmesg, It will report a link down, followed by the new speed,
which when using advertise 1, should be 10 Mbps/half duplex.
To make it more interesting, ethtool eth1 reports 10 Mbps/half duplex
even though the kernel reports 100 Mbps/full duplex.

What does work (but did before this whole situation as well):
ifconfig eth1 up
ethtool -s eth1 advertise 1

The interface will now be in 10 Mbps/half duplex, that is, until the
reset function is called (interface down or otherwise). 

What I do notice, is that dev->mii.advertising follows whatever I set
with ethtool. I tried writing the ADVERTISE register with the value of
the dev->mii.advertising variable, but that did not give me the desired
result either.

Kind regards,

Michel Stam

-----Original Message-----
From: Charles Keepax [mailto:ckeepax@opensource.wolfsonmicro.com] 
Sent: Wednesday, November 05, 2014 16:03 PM
To: Stam, Michel [FINT]
Cc: Riku Voipio; davem@davemloft.net; linux-usb@vger.kernel.org;
netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
linux-samsung-soc@vger.kernel.org
Subject: Re: "asix: Don't reset PHY on if_up for ASIX 88772" breaks
netonarndale platform

On Wed, Nov 05, 2014 at 01:04:37PM +0100, Stam, Michel [FINT] wrote:
> Hello Charles,
> 
> After looking around I found the reset value for the 8772 chip, which 
> seems to be 0x1E1 (ANAR register).
> 
> This equates to (according to include/uapi/linux/mii.h) ADVERTISE_ALL 
> | ADVERTISE_CSMA.
> 
> The register only seems to become 0 if the software reset fails.

Odd it definitely reads back as zero on Arndale. I am guessing that the
root of the problem here is that for some reason Arndale POR of the
ethernet is pants and it needs a full software reset before it will work
and the patch removes the full reset callback.

> 
> Unfortunately, this is exactly what I get when the patch is applied; 
> asix 1-2:1.0 eth1: Failed to send software reset: ffffffb5 asix 
> 1-2:1.0 eth1: link reset failed (-75) usbnet usb-0000:00:1d.0-2, ASIX 
> AX88772 USB 2.0 Ethernet asix 1-2:1.0 eth1: Failed to send software 
> reset: ffffffb5 asix 1-2:1.0 eth1: link reset failed (-75) usbnet 
> usb-0000:00:1d.0-2, ASIX AX88772 USB 2.0 Ethernet

Ok so I am guessing you have a value in the register which is neither
the reset value or 0 and this causing problems later in the reset/on the
next reset. I do find the naming confusing in the error message there as
it says link reset failed but the link_reset callback can't fail in the
driver and I modified the reset callback. But I guess that is just
oddities of the network stack I am not familiar with.

The other thing that feels odd is (and again apologies as I know next to
nothing about the networking stack) how come it is unexpected that the
reset callback destroys the state of the device. Naively I would have
expected that a reset callback would reset the device back to its
default state. Here we seem to be trying to avoid that happening.

> 
> A little while after this its 'Failed to enable software MII access'.
> The ethernet device fails to see any link or accept any ethtool -s 
> command.
> 
> My device has vid:devid 0b95:772a (ASIX Elec. Corp.).
> 
> Can you tell me what device is on the Andale platform, Charles? Same 
> vendor/device id?

Yeah mine also idVendor=0b95, idProduct=772a

Thanks,
Charles

> 
> Another thing that bothers me is that dev->mii.advertising seems to 
> contain the same value, so maybe that can be used instead of a call to

> asix_mdio_read(). Can anyone comment on its purpose? Should it be a 
> shadow copy of the real register or something?
> 
> Riku, can you test Charles' patch as well?
> 
> Kind regards,
> 
> Michel Stam
> 
> -----Original Message-----
> From: Charles Keepax [mailto:ckeepax@opensource.wolfsonmicro.com]
> Sent: Tuesday, November 04, 2014 21:09 PM
> To: Stam, Michel [FINT]
> Cc: Riku Voipio; davem@davemloft.net; linux-usb@vger.kernel.org; 
> netdev@vger.kernel.org; linux-kernel@vger.kernel.org; 
> linux-samsung-soc@vger.kernel.org
> Subject: Re: "asix: Don't reset PHY on if_up for ASIX 88772" breaks 
> net onarndale platform
> 
> On Tue, Nov 04, 2014 at 11:23:06AM +0100, Stam, Michel [FINT] wrote:
> > Hello Riku,
> > 
> > >Fixing a bug (ethtool support) must not cause breakage elsewhere 
> > >(in
> > this case on arndale). This is now a regression of functionality 
> > from 3.17.
> > >
> > >I think it would better to revert the change now and with less 
> > >hurry
> > introduce a ethtool fix that doesn't break arndale.
> > 
> > I don't fully agree here;
> > I would like to point out that this commit is a revert itself. 
> > Fixing the armdale will then cause breakage in other 
> > implementations, such as
> 
> > ours. Blankly reverting breaks other peoples' implementations.
> > 
> > The PHY reset is the thing that breaks ethtool support, so any fix 
> > that appeases all would have to take existing PHY state into
account.
> > 
> > I'm not an expert on the ASIX driver, nor the MII, but I think this 
> > is
> 
> > the cause;
> > drivers/net/usb/asix_devices.c:
> >    361      asix_mdio_write(dev->net, dev->mii.phy_id, MII_BMCR,
> > BMCR_RESET);
> >    362      asix_mdio_write(dev->net, dev->mii.phy_id,
MII_ADVERTISE,
> >    363              ADVERTISE_ALL | ADVERTISE_CSMA);
> >    364      mii_nway_restart(&dev->mii);
> > 
> > I would think that the ADVERTISE_ALL is the cause here, as it will 
> > reset the MII back to default, thus overriding ethtool settings.
> > Would an:
> > Int reg;
> > reg = asix_mdio_read(dev->net,dev->mii.phy_id,MII_ADVERTISE);
> > 
> > prior to the offending lines, and then;
> > 
> >    362      asix_mdio_write(dev->net, dev->mii.phy_id,
MII_ADVERTISE,
> >    363             reg);
> > 
> > solve the problem for you guys?
> 
> If I revert the patch in question and add this in:
> 
> --- a/drivers/net/usb/asix_devices.c
> +++ b/drivers/net/usb/asix_devices.c
> @@ -299,6 +299,7 @@ static int ax88772_reset(struct usbnet *dev)  {
>         struct asix_data *data = (struct asix_data *)&dev->data;
>         int ret, embd_phy;
> +       int reg;
>         u16 rx_ctl;
> 
>         ret = asix_write_gpio(dev,
> @@ -359,8 +360,10 @@ static int ax88772_reset(struct usbnet *dev)
>         msleep(150);
> 
>         asix_mdio_write(dev->net, dev->mii.phy_id, MII_BMCR, 
> BMCR_RESET);
> -       asix_mdio_write(dev->net, dev->mii.phy_id, MII_ADVERTISE,
> -                       ADVERTISE_ALL | ADVERTISE_CSMA);
> +       reg = asix_mdio_read(dev->net, dev->mii.phy_id,
MII_ADVERTISE);
> +       if (!reg)
> +               reg = ADVERTISE_ALL | ADVERTISE_CSMA;
> +       asix_mdio_write(dev->net, dev->mii.phy_id, MII_ADVERTISE, 
> + reg);
>         mii_nway_restart(&dev->mii);
> 
>         ret = asix_write_medium_mode(dev, AX88772_MEDIUM_DEFAULT);
> 
> Then things work on Arndale for me. Does that work for you?
> Whether that is a sensible fix I don't know however.
> 
> > 
> > Mind, maybe the read function should take into account the reset 
> > value
> 
> > of the MII, and set it to ADVERTISE_ALL | ADVERTISE_CSMA. I don't 
> > have
> 
> > any documention here at the moment.
> 
> Yeah I also have no documentation.
> 
> Thanks,
> Charles
> 
> > 
> > Is anyone able to confirm my suspicions?
> > 
> > Kind regards,
> > 
> > Michel Stam
> > -----Original Message-----
> > From: Riku Voipio [mailto:riku.voipio@iki.fi]
> > Sent: Tuesday, November 04, 2014 10:44 AM
> > To: Stam, Michel [FINT]
> > Cc: Riku Voipio; davem@davemloft.net; linux-usb@vger.kernel.org; 
> > netdev@vger.kernel.org; linux-kernel@vger.kernel.org; 
> > linux-samsung-soc@vger.kernel.org; 
> > ckeepax@opensource.wolfsonmicro.com
> > Subject: Re: "asix: Don't reset PHY on if_up for ASIX 88772" breaks 
> > net on arndale platform
> > 
> > On Tue, Nov 04, 2014 at 09:19:26AM +0100, Stam, Michel [FINT] wrote:
> > > Interesting, as the commit itself is a revert from a kernel back 
> > > to
> > > 2.6 somewhere. The problem I had is related to the PHY being reset

> > > on interface-up, can you confirm that you require this?
> > 
> > I can't confirm what exactly is needed on arndale. I'm neither 
> > expert in USB or ethernet. However, I can confirm that without the 
> > PHY reset,
> 
> > networking doesn't work on arndale.
> > 
> > I now see someone else has the same problem, adding Charles to CC.
> > 
> > http://www.spinics.net/lists/linux-usb/msg116656.html
> > 
> > > Reverting this
> > > breaks ethtool support in turn.
> > 
> > Fixing a bug (ethtool support) must not cause breakage elsewhere (in

> > this case on arndale). This is now a regression of functionality 
> > from 3.17.
> > 
> > I think it would better to revert the change now and with less hurry

> > introduce a ethtool fix that doesn't break arndale.
> >  
> > > Kind regards,
> > > 
> > > Michel Stam
> > > 
> > > -----Original Message-----
> > > From: Riku Voipio [mailto:riku.voipio@iki.fi]
> > > Sent: Tuesday, November 04, 2014 8:23 AM
> > > To: davem@davemloft.net; Stam, Michel [FINT]
> > > Cc: linux-usb@vger.kernel.org; netdev@vger.kernel.org; 
> > > linux-kernel@vger.kernel.org; linux-samsung-soc@vger.kernel.org
> > > Subject: "asix: Don't reset PHY on if_up for ASIX 88772" breaks 
> > > net on
> > 
> > > arndale platform
> > > 
> > > Hi,
> > > 
> > > With 3.18-rc3, asix on arndale (samsung exynos 5250 based board), 
> > > fails to work. Interface is initialized but network traffic seem 
> > > not
> 
> > > to pass through. With kernel IP config the result looks like:
> > > 
> > > [    3.323275] usb 3-3.2.4: new high-speed USB device number 4
using
> > > exynos-ehci
> > > [    3.419151] usb 3-3.2.4: New USB device found, idVendor=0b95,
> > > idProduct=772a
> > > [    3.424735] usb 3-3.2.4: New USB device strings: Mfr=1,
> Product=2,
> > > SerialNumber=3
> > > [    3.432196] usb 3-3.2.4: Product: AX88772 
> > > [    3.436279] usb 3-3.2.4: Manufacturer: ASIX Elec. Corp.
> > > [    3.441486] usb 3-3.2.4: SerialNumber: 000001
> > > [    3.447530] asix 3-3.2.4:1.0 (unnamed net_device)
> (uninitialized):
> > > invalid hw address, using random
> > > [    3.764352] asix 3-3.2.4:1.0 eth0: register 'asix' at
> > > usb-12110000.usb-3.2.4, ASIX AX88772 USB 2.0 Ethernet,
> > de:a2:66:bf:ca:4f
> > > [    4.488773] asix 3-3.2.4:1.0 eth0: link down
> > > [    5.690025] asix 3-3.2.4:1.0 eth0: link up, 100Mbps,
full-duplex,
> > lpa
> > > 0xC5E1
> > > [    5.712947] Sending DHCP requests ...... timed out!
> > > [   83.165303] IP-Config: Retrying forever (NFS root)...
> > > [   83.170397] asix 3-3.2.4:1.0 eth0: link up, 100Mbps,
full-duplex,
> > lpa
> > > 0xC5E1
> > > [   83.192944] Sending DHCP requests .....
> > > 
> > > Similar results also with dhclient. Git bisect identified the 
> > > breaking
> > 
> > > commit as:
> > > 
> > > commit 3cc81d85ee01e5a0b7ea2f4190e2ed1165f53c31
> > > Author: Michel Stam <m.stam@fugro.nl>
> > > Date:   Thu Oct 2 10:22:02 2014 +0200
> > > 
> > >     asix: Don't reset PHY on if_up for ASIX 88772
> > > 
> > > Taking 3.18-rc3 and that commit reverted, network works again:
> > > 
> > > [    3.303500] usb 3-3.2.4: new high-speed USB device number 4
using
> > > exynos-ehci
> > > [    3.399375] usb 3-3.2.4: New USB device found, idVendor=0b95,
> > > idProduct=772a
> > > [    3.404963] usb 3-3.2.4: New USB device strings: Mfr=1,
> Product=2,
> > > SerialNumber=3
> > > [    3.412424] usb 3-3.2.4: Product: AX88772 
> > > [    3.416508] usb 3-3.2.4: Manufacturer: ASIX Elec. Corp.
> > > [    3.421715] usb 3-3.2.4: SerialNumber: 000001
> > > [    3.427755] asix 3-3.2.4:1.0 (unnamed net_device)
> (uninitialized):
> > > invalid hw address, using random
> > > [    3.744837] asix 3-3.2.4:1.0 eth0: register 'asix' at
> > > usb-12110000.usb-3.2.4, ASIX AX88772 USB 2.0 Ethernet,
> > 12:59:f1:a8:43:90
> > > [    7.098998] asix 3-3.2.4:1.0 eth0: link up, 100Mbps,
full-duplex,
> > lpa
> > > 0xC5E1
> > > [    7.118258] Sending DHCP requests ., OK
> > > [    9.753259] IP-Config: Got DHCP answer from 192.168.1.1, my
> address
> > > is 192.168.1.111
> > > 
> > > There might something wrong on the samsung platform code (I 
> > > understand
> > 
> > > the USB on arndale is "funny"), but this is still an regression 
> > > from
> 
> > > 3.17.
> > > 
> > > Riku

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stam, Michel [FINT] Nov. 5, 2014, 4:21 p.m. UTC | #4
Hello Riku,

I will quickly reply to your message, and reserve any further comments
for the other thread.

>My concern is, that none of us with this problem is a linux network
drivers expert. And no such expert joined the thread to help us. Thus if
we hurry to have proper fix for 3.18, our fix might easily be really
wrong.
>
>Hence, it would seem safer to revert to 3.17 state before 3.18, so we
can propose a proper fix for 3.19. At least from our myopic view, having
no functioning net on arndale is worse than having non-functioning
ethtool (which doesn't >seem to have bothered people for years).

Lets agree to disagree here. Any further comment on this would not be
productive.

Kind regards,

Michel Stam

-----Original Message-----
From: Riku Voipio [mailto:riku.voipio@iki.fi] 
Sent: Wednesday, November 05, 2014 13:40 PM
To: Stam, Michel [FINT]
Cc: Charles Keepax; davem@davemloft.net; linux-usb@vger.kernel.org;
netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
linux-samsung-soc@vger.kernel.org
Subject: Re: "asix: Don't reset PHY on if_up for ASIX 88772" breaks net
onarndale platform

Hi Michel,

On Wed, Nov 05, 2014 at 01:04:37PM +0100, Stam, Michel [FINT] wrote:
> After looking around I found the reset value for the 8772 chip, which 
> seems to be 0x1E1 (ANAR register).
> 
> This equates to (according to include/uapi/linux/mii.h) ADVERTISE_ALL 
> | ADVERTISE_CSMA.
> 
> The register only seems to become 0 if the software reset fails.
> 
> Unfortunately, this is exactly what I get when the patch is applied; 
> asix 1-2:1.0 eth1: Failed to send software reset: ffffffb5 asix 
> 1-2:1.0 eth1: link reset failed (-75) usbnet usb-0000:00:1d.0-2, ASIX 
> AX88772 USB 2.0 Ethernet asix 1-2:1.0 eth1: Failed to send software 
> reset: ffffffb5 asix 1-2:1.0 eth1: link reset failed (-75) usbnet 
> usb-0000:00:1d.0-2, ASIX AX88772 USB 2.0 Ethernet
> 
> A little while after this its 'Failed to enable software MII access'.
> The ethernet device fails to see any link or accept any ethtool -s 
> command.

> My device has vid:devid 0b95:772a (ASIX Elec. Corp.).

> Can you tell me what device is on the Andale platform, Charles? Same 
> vendor/device id?

Bus 003 Device 004: ID 0b95:772a ASIX Electronics Corp. AX88772A Fast
Ethernet

> Another thing that bothers me is that dev->mii.advertising seems to 
> contain the same value, so maybe that can be used instead of a call to

> asix_mdio_read(). Can anyone comment on its purpose? Should it be a 
> shadow copy of the real register or something?
 
> Riku, can you test Charles' patch as well?

With that patch + revert to ax88772_reset network works. I'm unable to
get ethtool to work with that patch or with the original 3.17 state of
asix.
Once i disable autoneg network doesn't just work. 

> > >I think it would better to revert the change now and with less 
> > >hurry
> > introduce a ethtool fix that doesn't break arndale.

> > I don't fully agree here;
> > I would like to point out that this commit is a revert itself. 
> > Fixing the armdale will then cause breakage in other 
> > implementations, such as ours. Blankly reverting breaks other
peoples' implementations.

My concern is, that none of us with this problem is a linux network
drivers expert. And no such expert joined the thread to help us. Thus if
we hurry to have proper fix for 3.18, our fix might easily be really
wrong.

Hence, it would seem safer to revert to 3.17 state before 3.18, so we
can propose a proper fix for 3.19. At least from our myopic view, having
no functioning net on arndale is worse than having non-functioning
ethtool (which doesn't seem to have bothered people for years).

Riku

> > The PHY reset is the thing that breaks ethtool support, so any fix 
> > that appeases all would have to take existing PHY state into
account.
> > 
> > I'm not an expert on the ASIX driver, nor the MII, but I think this 
> > is
> 
> > the cause;
> > drivers/net/usb/asix_devices.c:
> >    361      asix_mdio_write(dev->net, dev->mii.phy_id, MII_BMCR,
> > BMCR_RESET);
> >    362      asix_mdio_write(dev->net, dev->mii.phy_id,
MII_ADVERTISE,
> >    363              ADVERTISE_ALL | ADVERTISE_CSMA);
> >    364      mii_nway_restart(&dev->mii);
> > 
> > I would think that the ADVERTISE_ALL is the cause here, as it will 
> > reset the MII back to default, thus overriding ethtool settings.
> > Would an:
> > Int reg;
> > reg = asix_mdio_read(dev->net,dev->mii.phy_id,MII_ADVERTISE);
> > 
> > prior to the offending lines, and then;
> > 
> >    362      asix_mdio_write(dev->net, dev->mii.phy_id,
MII_ADVERTISE,
> >    363             reg);
> > 
> > solve the problem for you guys?
> 
> If I revert the patch in question and add this in:
> 
> --- a/drivers/net/usb/asix_devices.c
> +++ b/drivers/net/usb/asix_devices.c
> @@ -299,6 +299,7 @@ static int ax88772_reset(struct usbnet *dev)  {
>         struct asix_data *data = (struct asix_data *)&dev->data;
>         int ret, embd_phy;
> +       int reg;
>         u16 rx_ctl;
> 
>         ret = asix_write_gpio(dev,
> @@ -359,8 +360,10 @@ static int ax88772_reset(struct usbnet *dev)
>         msleep(150);
> 
>         asix_mdio_write(dev->net, dev->mii.phy_id, MII_BMCR, 
> BMCR_RESET);
> -       asix_mdio_write(dev->net, dev->mii.phy_id, MII_ADVERTISE,
> -                       ADVERTISE_ALL | ADVERTISE_CSMA);
> +       reg = asix_mdio_read(dev->net, dev->mii.phy_id,
MII_ADVERTISE);
> +       if (!reg)
> +               reg = ADVERTISE_ALL | ADVERTISE_CSMA;
> +       asix_mdio_write(dev->net, dev->mii.phy_id, MII_ADVERTISE, 
> + reg);
>         mii_nway_restart(&dev->mii);
> 
>         ret = asix_write_medium_mode(dev, AX88772_MEDIUM_DEFAULT);
> 
> Then things work on Arndale for me. Does that work for you?
> Whether that is a sensible fix I don't know however.
> 
> > 
> > Mind, maybe the read function should take into account the reset 
> > value
> 
> > of the MII, and set it to ADVERTISE_ALL | ADVERTISE_CSMA. I don't 
> > have
> 
> > any documention here at the moment.
> 
> Yeah I also have no documentation.
> 
> Thanks,
> Charles
> 
> > 
> > Is anyone able to confirm my suspicions?
> > 
> > Kind regards,
> > 
> > Michel Stam
> > -----Original Message-----
> > From: Riku Voipio [mailto:riku.voipio@iki.fi]
> > Sent: Tuesday, November 04, 2014 10:44 AM
> > To: Stam, Michel [FINT]
> > Cc: Riku Voipio; davem@davemloft.net; linux-usb@vger.kernel.org; 
> > netdev@vger.kernel.org; linux-kernel@vger.kernel.org; 
> > linux-samsung-soc@vger.kernel.org; 
> > ckeepax@opensource.wolfsonmicro.com
> > Subject: Re: "asix: Don't reset PHY on if_up for ASIX 88772" breaks 
> > net on arndale platform
> > 
> > On Tue, Nov 04, 2014 at 09:19:26AM +0100, Stam, Michel [FINT] wrote:
> > > Interesting, as the commit itself is a revert from a kernel back 
> > > to
> > > 2.6 somewhere. The problem I had is related to the PHY being reset

> > > on interface-up, can you confirm that you require this?
> > 
> > I can't confirm what exactly is needed on arndale. I'm neither 
> > expert in USB or ethernet. However, I can confirm that without the 
> > PHY reset,
> 
> > networking doesn't work on arndale.
> > 
> > I now see someone else has the same problem, adding Charles to CC.
> > 
> > http://www.spinics.net/lists/linux-usb/msg116656.html
> > 
> > > Reverting this
> > > breaks ethtool support in turn.
> > 
> > Fixing a bug (ethtool support) must not cause breakage elsewhere (in

> > this case on arndale). This is now a regression of functionality 
> > from 3.17.
> > 
> > I think it would better to revert the change now and with less hurry

> > introduce a ethtool fix that doesn't break arndale.
> >  
> > > Kind regards,
> > > 
> > > Michel Stam
> > > 
> > > -----Original Message-----
> > > From: Riku Voipio [mailto:riku.voipio@iki.fi]
> > > Sent: Tuesday, November 04, 2014 8:23 AM
> > > To: davem@davemloft.net; Stam, Michel [FINT]
> > > Cc: linux-usb@vger.kernel.org; netdev@vger.kernel.org; 
> > > linux-kernel@vger.kernel.org; linux-samsung-soc@vger.kernel.org
> > > Subject: "asix: Don't reset PHY on if_up for ASIX 88772" breaks 
> > > net on
> > 
> > > arndale platform
> > > 
> > > Hi,
> > > 
> > > With 3.18-rc3, asix on arndale (samsung exynos 5250 based board), 
> > > fails to work. Interface is initialized but network traffic seem 
> > > not
> 
> > > to pass through. With kernel IP config the result looks like:
> > > 
> > > [    3.323275] usb 3-3.2.4: new high-speed USB device number 4
using
> > > exynos-ehci
> > > [    3.419151] usb 3-3.2.4: New USB device found, idVendor=0b95,
> > > idProduct=772a
> > > [    3.424735] usb 3-3.2.4: New USB device strings: Mfr=1,
> Product=2,
> > > SerialNumber=3
> > > [    3.432196] usb 3-3.2.4: Product: AX88772 
> > > [    3.436279] usb 3-3.2.4: Manufacturer: ASIX Elec. Corp.
> > > [    3.441486] usb 3-3.2.4: SerialNumber: 000001
> > > [    3.447530] asix 3-3.2.4:1.0 (unnamed net_device)
> (uninitialized):
> > > invalid hw address, using random
> > > [    3.764352] asix 3-3.2.4:1.0 eth0: register 'asix' at
> > > usb-12110000.usb-3.2.4, ASIX AX88772 USB 2.0 Ethernet,
> > de:a2:66:bf:ca:4f
> > > [    4.488773] asix 3-3.2.4:1.0 eth0: link down
> > > [    5.690025] asix 3-3.2.4:1.0 eth0: link up, 100Mbps,
full-duplex,
> > lpa
> > > 0xC5E1
> > > [    5.712947] Sending DHCP requests ...... timed out!
> > > [   83.165303] IP-Config: Retrying forever (NFS root)...
> > > [   83.170397] asix 3-3.2.4:1.0 eth0: link up, 100Mbps,
full-duplex,
> > lpa
> > > 0xC5E1
> > > [   83.192944] Sending DHCP requests .....
> > > 
> > > Similar results also with dhclient. Git bisect identified the 
> > > breaking
> > 
> > > commit as:
> > > 
> > > commit 3cc81d85ee01e5a0b7ea2f4190e2ed1165f53c31
> > > Author: Michel Stam <m.stam@fugro.nl>
> > > Date:   Thu Oct 2 10:22:02 2014 +0200
> > > 
> > >     asix: Don't reset PHY on if_up for ASIX 88772
> > > 
> > > Taking 3.18-rc3 and that commit reverted, network works again:
> > > 
> > > [    3.303500] usb 3-3.2.4: new high-speed USB device number 4
using
> > > exynos-ehci
> > > [    3.399375] usb 3-3.2.4: New USB device found, idVendor=0b95,
> > > idProduct=772a
> > > [    3.404963] usb 3-3.2.4: New USB device strings: Mfr=1,
> Product=2,
> > > SerialNumber=3
> > > [    3.412424] usb 3-3.2.4: Product: AX88772 
> > > [    3.416508] usb 3-3.2.4: Manufacturer: ASIX Elec. Corp.
> > > [    3.421715] usb 3-3.2.4: SerialNumber: 000001
> > > [    3.427755] asix 3-3.2.4:1.0 (unnamed net_device)
> (uninitialized):
> > > invalid hw address, using random
> > > [    3.744837] asix 3-3.2.4:1.0 eth0: register 'asix' at
> > > usb-12110000.usb-3.2.4, ASIX AX88772 USB 2.0 Ethernet,
> > 12:59:f1:a8:43:90
> > > [    7.098998] asix 3-3.2.4:1.0 eth0: link up, 100Mbps,
full-duplex,
> > lpa
> > > 0xC5E1
> > > [    7.118258] Sending DHCP requests ., OK
> > > [    9.753259] IP-Config: Got DHCP answer from 192.168.1.1, my
> address
> > > is 192.168.1.111
> > > 
> > > There might something wrong on the samsung platform code (I 
> > > understand
> > 
> > > the USB on arndale is "funny"), but this is still an regression 
> > > from
> 
> > > 3.17.
> > > 
> > > Riku
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Riku Voipio Nov. 6, 2014, 9:06 a.m. UTC | #5
On Wed, Nov 05, 2014 at 03:02:58PM +0000, Charles Keepax wrote:
> On Wed, Nov 05, 2014 at 01:04:37PM +0100, Stam, Michel [FINT] wrote:
> > Hello Charles,
> > 
> > After looking around I found the reset value for the 8772 chip, which
> > seems to be 0x1E1 (ANAR register).
> > 
> > This equates to (according to include/uapi/linux/mii.h)
> > ADVERTISE_ALL | ADVERTISE_CSMA.
> > 
> > The register only seems to become 0 if the software reset fails.
 
> Odd it definitely reads back as zero on Arndale. I am guessing
> that the root of the problem here is that for some reason Arndale
> POR of the ethernet is pants and it needs a full software reset
> before it will work and the patch removes the full reset
> callback.

The asix on arndale comes semi-configured from u-boot, which I guess is
not the state kernel expects it to come in. At least in my case where
I use tftp from u-boot to load my kernel.

So probably the full reset is needed here to make the asix chip come
to a truly pristine state.

The commit that Michel partially reverted (by returning to use
ax88772_link_reset instead of ax88772_reset), indicates that a strong reset
is needed for suspend/resume as well:

commit 4ad1438f025ed8d1e4e95a796ca7f0ad5a22c378
Author: Grant Grundler <grundler@chromium.org>
Date:   Tue Oct 4 09:55:16 2011 +0000

    NET: fix phy init for AX88772 USB ethernet
        
    Fix phy initialization for AX88772 (USB 2.0 100BT). Failure
    was occasionally DHCP wouldn't work after reboot or
    suspend/resume cycle.
 
> > Unfortunately, this is exactly what I get when the patch is applied;
> > asix 1-2:1.0 eth1: Failed to send software reset: ffffffb5
> > asix 1-2:1.0 eth1: link reset failed (-75) usbnet usb-0000:00:1d.0-2, 
> > ASIX AX88772 USB 2.0 Ethernet
> > asix 1-2:1.0 eth1: Failed to send software reset: ffffffb5
> > asix 1-2:1.0 eth1: link reset failed (-75) usbnet usb-0000:00:1d.0-2, 
> > ASIX AX88772 USB 2.0 Ethernet
> 
> Ok so I am guessing you have a value in the register which is
> neither the reset value or 0 and this causing problems later in
> the reset/on the next reset. I do find the naming confusing in
> the error message there as it says link reset failed but the
> link_reset callback can't fail in the driver and I modified the
> reset callback. But I guess that is just oddities of the network
> stack I am not familiar with.
> 
> The other thing that feels odd is (and again apologies as I know
> next to nothing about the networking stack) how come it is
> unexpected that the reset callback destroys the state of the
> device. Naively I would have expected that a reset callback would
> reset the device back to its default state. Here we seem to be
> trying to avoid that happening.

Indeed, it would seems some tracing would be neede to figure out in
which order the .reset and .link_reset callbacks are being called.

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

--- a/drivers/net/usb/asix_devices.c
+++ b/drivers/net/usb/asix_devices.c
@@ -299,6 +299,7 @@  static int ax88772_reset(struct usbnet *dev)  {
        struct asix_data *data = (struct asix_data *)&dev->data;
        int ret, embd_phy;
+       int reg;
        u16 rx_ctl;

        ret = asix_write_gpio(dev,
@@ -359,8 +360,10 @@  static int ax88772_reset(struct usbnet *dev)