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

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

[Riku Voipio]

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

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

[Stam, Michel [FINT]]

Hello Riku,

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? Reverting this
breaks ethtool support in turn.

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

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

[Riku Voipio]

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

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

[Stam, Michel [FINT]]

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?

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.

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

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

[Charles Keepax]

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

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

[Stam, Michel [FINT]]

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:

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

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

[Riku Voipio]

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

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

[Charles Keepax]

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

RE: "asix: Don't reset PHY on if_up for ASIX 88772" breaks netonarndale platform

[Stam, Michel [FINT]]

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

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

[Stam, Michel [FINT]]

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

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

[Riku Voipio]

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.

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

[Charles Keepax]

On Thu, Nov 06, 2014 at 11:06:51AM +0200, Riku Voipio wrote:
> 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:

Ok I think I have cracked this one. I am pretty sure you are
right that the USB comes to us in a strange state and needs
a full reset, but that only needs to happen once when the driver
is bound in. So there is some code in ax88772_bind that appears
to try to reset the device but does a lot less than ax88772_reset
and I think that must be the problem. Applying the following on
top of the patch we have been debating I think will make
everything work for all of us:

--- a/drivers/net/usb/asix_devices.c
+++ b/drivers/net/usb/asix_devices.c
@@ -465,19 +465,7 @@ static int ax88772_bind(struct usbnet *dev,
struct usb_interface *in
                return ret;
        }

-       ret = asix_sw_reset(dev, AX_SWRESET_IPPD | AX_SWRESET_PRL);
-       if (ret < 0)
-               return ret;
-
-       msleep(150);
-
-       ret = asix_sw_reset(dev, AX_SWRESET_CLEAR);
-       if (ret < 0)
-               return ret;
-
-       msleep(150);
-
-       ret = asix_sw_reset(dev, embd_phy ? AX_SWRESET_IPRL : AX_SWRESET_PRTE);
+       ax88772_reset(dev);

If you guys could test that and let me know how you get on I will
send in a proper patch if it looks good.

Thanks,
Charles

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

[Riku Voipio]

On Thu, Nov 06, 2014 at 10:01:04AM +0000, Charles Keepax wrote:
> On Thu, Nov 06, 2014 at 11:06:51AM +0200, Riku Voipio wrote:
> > 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:
 
> Ok I think I have cracked this one. I am pretty sure you are
> right that the USB comes to us in a strange state and needs
> a full reset, but that only needs to happen once when the driver
> is bound in. So there is some code in ax88772_bind that appears
> to try to reset the device but does a lot less than ax88772_reset
> and I think that must be the problem. Applying the following on
> top of the patch we have been debating I think will make
> everything work for all of us:

The patch below on top of 3.18-rc3 fixes arndale network for me.

Tested-by: Riku Voipio <riku.voipio@linaro.org>

> --- a/drivers/net/usb/asix_devices.c
> +++ b/drivers/net/usb/asix_devices.c
> @@ -465,19 +465,7 @@ static int ax88772_bind(struct usbnet *dev,
> struct usb_interface *in
>                 return ret;
>         }
> 
> -       ret = asix_sw_reset(dev, AX_SWRESET_IPPD | AX_SWRESET_PRL);
> -       if (ret < 0)
> -               return ret;
> -
> -       msleep(150);
> -
> -       ret = asix_sw_reset(dev, AX_SWRESET_CLEAR);
> -       if (ret < 0)
> -               return ret;
> -
> -       msleep(150);
> -
> -       ret = asix_sw_reset(dev, embd_phy ? AX_SWRESET_IPRL : AX_SWRESET_PRTE);
> +       ax88772_reset(dev);
> 
> If you guys could test that and let me know how you get on I will
> send in a proper patch if it looks good.
> 
> Thanks,
> Charles

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

[Stam, Michel [FINT]]

Hello Riku and Charles,

I tried this with my original patch and the suggested patch applied,
this seems to work for me too.

One thing that bothers me, is the suspend / resume situation; usbnet.c
seems to call the bind( ) on probe( ). Suspend / resume do not seem to
call bind( ) directly.

As Riku pointed out, the original patch I reverted was because of
suspend/resume issues. I wonder if this will still work? 

Kind regards,

Michel Stam

-----Original Message-----
From: Riku Voipio [mailto:riku.voipio@iki.fi] 
Sent: Thursday, November 06, 2014 13:04 PM
To: Charles Keepax
Cc: Riku Voipio; Stam, Michel [FINT]; freddy@asix.com.tw;
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
on arndale platform

On Thu, Nov 06, 2014 at 10:01:04AM +0000, Charles Keepax wrote:
> On Thu, Nov 06, 2014 at 11:06:51AM +0200, Riku Voipio wrote:
> > 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:
 
> Ok I think I have cracked this one. I am pretty sure you are right 
> that the USB comes to us in a strange state and needs a full reset, 
> but that only needs to happen once when the driver is bound in. So 
> there is some code in ax88772_bind that appears to try to reset the 
> device but does a lot less than ax88772_reset and I think that must be

> the problem. Applying the following on top of the patch we have been 
> debating I think will make everything work for all of us:

The patch below on top of 3.18-rc3 fixes arndale network for me.

Tested-by: Riku Voipio <riku.voipio@linaro.org>

> --- a/drivers/net/usb/asix_devices.c
> +++ b/drivers/net/usb/asix_devices.c
> @@ -465,19 +465,7 @@ static int ax88772_bind(struct usbnet *dev, 
> struct usb_interface *in
>                 return ret;
>         }
> 
> -       ret = asix_sw_reset(dev, AX_SWRESET_IPPD | AX_SWRESET_PRL);
> -       if (ret < 0)
> -               return ret;
> -
> -       msleep(150);
> -
> -       ret = asix_sw_reset(dev, AX_SWRESET_CLEAR);
> -       if (ret < 0)
> -               return ret;
> -
> -       msleep(150);
> -
> -       ret = asix_sw_reset(dev, embd_phy ? AX_SWRESET_IPRL :
AX_SWRESET_PRTE);
> +       ax88772_reset(dev);
> 
> If you guys could test that and let me know how you get on I will send

> in a proper patch if it looks good.
> 
> Thanks,
> Charles

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

[Charles Keepax]

On Thu, Nov 06, 2014 at 01:39:07PM +0100, Stam, Michel [FINT] wrote:
> Hello Riku and Charles,
> 
> I tried this with my original patch and the suggested patch applied,
> this seems to work for me too.
> 
> One thing that bothers me, is the suspend / resume situation; usbnet.c
> seems to call the bind( ) on probe( ). Suspend / resume do not seem to
> call bind( ) directly.
> 
> As Riku pointed out, the original patch I reverted was because of
> suspend/resume issues. I wonder if this will still work? 

I seem to remember I had a few issues with Arndale suspend and
resume last time I tried it anyways, so not sure I will be able
to test for that. But I will have a go. But in either case I
guess a fix for that is probably orthogonal to my suggested fix
here, as it looks pretty clear we intended to fully reset the
device in bind and were appartently not doing that. So this
patch is probably a needed fix anyway. Even if there are
seperate issues relating to suspend and resume.

Thanks,
Charles

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

[Stam, Michel [FINT]]

Hello Charles and Riku,

I've quickly tested this on a 3.10 kernel i had around;
I enabled CONFIG_PM, CONFIG_PM_RUNTIME, CONFIG_PM_AUTOSLEEP,
CONFIG_SUSPEND, CONFIG_SUSPEND_FREEZER, CONFIG_FREEZER in the kernel (by
default they are disabled for our setup, I enabled anything regarded to
runtime powermanagement to be sure I would trigger suspend/resume).

Then:
cd /sys/bus/usb/devices/1-2/power
echo auto > control
echo 1 > autosuspend
echo 0 > autosuspend_delay_ms
echo enabled > wakeup

# make sure there's no processes routing traffic over the eth1 interface

ifconfig eth1 down
sleep 4 # sleep some arbitrary long time
ifconfig eth1 up

check dmesg; it will reset back to 100 Mbps/full duplex.

This confirms that the suspend / resume does not work well. So long as
the suspend is not triggered, it does seem to work, though. I cannot say
whether the original issue that triggered this is still around; the ASIX
chip setup we use is soldered to the PCB and hooked up to a fixed device
on-board. 
I also tried to ping the device on the other side of the ASIX chip after
the suspend/resume cycle, I could not ping it. I cannot conclusively say
that this is due to the ASIX driver, as the device on the other side
does not like switching PHY speeds (it may go into a non-responsive
state). It is for this reason that we run it at half duplex/ 10Mbps at
all times.

As said; we are not using this kind of power management, so it does not
raise any issues for us. I am merely pointing out that this may need
work (in the future?).

Kind regards,

Michel Stam
-----Original Message-----
From: Charles Keepax [mailto:ckeepax@opensource.wolfsonmicro.com] 
Sent: Thursday, November 06, 2014 13:47 PM
To: Stam, Michel [FINT]
Cc: Riku Voipio; freddy@asix.com.tw; 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 Thu, Nov 06, 2014 at 01:39:07PM +0100, Stam, Michel [FINT] wrote:
> Hello Riku and Charles,
> 
> I tried this with my original patch and the suggested patch applied, 
> this seems to work for me too.
> 
> One thing that bothers me, is the suspend / resume situation; usbnet.c

> seems to call the bind( ) on probe( ). Suspend / resume do not seem to

> call bind( ) directly.
> 
> As Riku pointed out, the original patch I reverted was because of 
> suspend/resume issues. I wonder if this will still work?

I seem to remember I had a few issues with Arndale suspend and resume
last time I tried it anyways, so not sure I will be able to test for
that. But I will have a go. But in either case I guess a fix for that is
probably orthogonal to my suggested fix here, as it looks pretty clear
we intended to fully reset the device in bind and were appartently not
doing that. So this patch is probably a needed fix anyway. Even if there
are seperate issues relating to suspend and resume.

Thanks,
Charles

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

[Charles Keepax]

On Thu, Nov 06, 2014 at 03:01:56PM +0100, Stam, Michel [FINT] wrote:
> Hello Charles and Riku,
> 
> I've quickly tested this on a 3.10 kernel i had around;
> I enabled CONFIG_PM, CONFIG_PM_RUNTIME, CONFIG_PM_AUTOSLEEP,
> CONFIG_SUSPEND, CONFIG_SUSPEND_FREEZER, CONFIG_FREEZER in the kernel (by
> default they are disabled for our setup, I enabled anything regarded to
> runtime powermanagement to be sure I would trigger suspend/resume).
> 
> Then:
> cd /sys/bus/usb/devices/1-2/power
> echo auto > control
> echo 1 > autosuspend
> echo 0 > autosuspend_delay_ms
> echo enabled > wakeup
> 
> # make sure there's no processes routing traffic over the eth1 interface
> 
> ifconfig eth1 down
> sleep 4 # sleep some arbitrary long time
> ifconfig eth1 up
> 
> check dmesg; it will reset back to 100 Mbps/full duplex.
> 
> This confirms that the suspend / resume does not work well. So long as
> the suspend is not triggered, it does seem to work, though. I cannot say
> whether the original issue that triggered this is still around; the ASIX
> chip setup we use is soldered to the PCB and hooked up to a fixed device
> on-board. 
> I also tried to ping the device on the other side of the ASIX chip after
> the suspend/resume cycle, I could not ping it. I cannot conclusively say
> that this is due to the ASIX driver, as the device on the other side
> does not like switching PHY speeds (it may go into a non-responsive
> state). It is for this reason that we run it at half duplex/ 10Mbps at
> all times.
> 
> As said; we are not using this kind of power management, so it does not
> raise any issues for us. I am merely pointing out that this may need
> work (in the future?).

Cool thanks for checking this I will make a note in the commit
message that suspend/resume might need some more work.

Thanks,
Charles

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

[Riku Voipio]

On Thu, Nov 06, 2014 at 02:09:40PM +0000, Charles Keepax wrote:
> On Thu, Nov 06, 2014 at 03:01:56PM +0100, Stam, Michel [FINT] wrote:
> > Hello Charles and Riku,
> > 
> > I've quickly tested this on a 3.10 kernel i had around;
> > I enabled CONFIG_PM, CONFIG_PM_RUNTIME, CONFIG_PM_AUTOSLEEP,
> > CONFIG_SUSPEND, CONFIG_SUSPEND_FREEZER, CONFIG_FREEZER in the kernel (by
> > default they are disabled for our setup, I enabled anything regarded to
> > runtime powermanagement to be sure I would trigger suspend/resume).
> > 
> > Then:
> > cd /sys/bus/usb/devices/1-2/power
> > echo auto > control
> > echo 1 > autosuspend
> > echo 0 > autosuspend_delay_ms
> > echo enabled > wakeup
> > 
> > # make sure there's no processes routing traffic over the eth1 interface
> > 
> > ifconfig eth1 down
> > sleep 4 # sleep some arbitrary long time
> > ifconfig eth1 up
> > 
> > check dmesg; it will reset back to 100 Mbps/full duplex.
> > 
> > This confirms that the suspend / resume does not work well. So long as
> > the suspend is not triggered, it does seem to work, though. I cannot say
> > whether the original issue that triggered this is still around; the ASIX
> > chip setup we use is soldered to the PCB and hooked up to a fixed device
> > on-board. 
> > I also tried to ping the device on the other side of the ASIX chip after
> > the suspend/resume cycle, I could not ping it. I cannot conclusively say
> > that this is due to the ASIX driver, as the device on the other side
> > does not like switching PHY speeds (it may go into a non-responsive
> > state). It is for this reason that we run it at half duplex/ 10Mbps at
> > all times.
> > 
> > As said; we are not using this kind of power management, so it does not
> > raise any issues for us. I am merely pointing out that this may need
> > work (in the future?).
 
> Cool thanks for checking this I will make a note in the commit
> message that suspend/resume might need some more work.

Thanks for digging through,

Make sure the commit message is clear that your patch is a regression
fix - following just this thread it might be a bit unclear.

Riku

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

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 2044 bytes --]

On Tue, 2014-11-04 at 20:09 +0000, Charles Keepax wrote:
> 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. 
[...]
> --- 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);
[...]

Why is there no sleep after setting the RESET bit?  Doesn't that make
the following register writes unreliable?

Ben.

-- 
Ben Hutchings
Experience is directly proportional to the value of equipment destroyed.
                                                         - Carolyn Scheppner

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

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

[Stam, Michel [FINT]]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 3420 bytes --]

Hello Ben,

Regarding the code snippet;

Good question, The original code didn't do this either, which is why I left it as it is. It could cause undesirable behaviour, agreed.

After a quick driver examination: I do see that asix_set_sw_mii and asix_set_hw_mii are called prior to the actual write (asix_mdio_write), it may be that this takes care of ensuring data is written to the chip, as asix_write_cmd waits for usbnet_write_cmd to send (and acknowledge) a USB CONTROL MSG. A lock to the phy is held during this time.

If I recall my USB knowledge, control messages are acknowledged, which would ensure data is written to the chip. Whether the ASIX requires further delay I would not know. I would have to dive deeper into the timings of the ASIX chip and the driver behaviour to see if that would be the case.

Kind regards,

Michel Stam
-----Original Message-----
From: Ben Hutchings [mailto:ben@decadent.org.uk] 
Sent: Wednesday, November 12, 2014 1:23 AM
To: Charles Keepax
Cc: Stam, Michel [FINT]; 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 on arndale platform

On Tue, 2014-11-04 at 20:09 +0000, Charles Keepax wrote:
> 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.
[...]
> --- 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);
[...]

Why is there no sleep after setting the RESET bit?  Doesn't that make the following register writes unreliable?

Ben.

--
Ben Hutchings
Experience is directly proportional to the value of equipment destroyed.
                                                         - Carolyn Scheppner
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

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

[David Miller]

Please do not top-post.