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

2014-11-12 Thread Stam, Michel [FINT] 
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:b...@decadent.org.uk] 
Sent: Wednesday, November 12, 2014 1:23 AM
To: Charles Keepax
Cc: Stam, Michel [FINT]; Riku Voipio; da...@davemloft.net; 
linux-usb@vger.kernel.org; net...@vger.kernel.org; 
linux-ker...@vger.kernel.org; linux-samsung-...@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 +, 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

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

2014-11-12 Thread David Miller 

Please do not top-post.
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2014-11-11 Thread Ben Hutchings 
On Tue, 2014-11-04 at 20:09 +, 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


signature.asc
Description: This is a digitally signed message part

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

2014-11-06 Thread Riku Voipio 
On Wed, Nov 05, 2014 at 03:02:58PM +, 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 grund...@chromium.org
Date:   Tue Oct 4 09:55:16 2011 +

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: ffb5
  asix 1-2:1.0 eth1: link reset failed (-75) usbnet usb-:00:1d.0-2, 
  ASIX AX88772 USB 2.0 Ethernet
  asix 1-2:1.0 eth1: Failed to send software reset: ffb5
  asix 1-2:1.0 eth1: link reset failed (-75) usbnet usb-:00:1d.0-2, 
  ASIX AX88772 USB 2.0 Ethernet
 
 Ok so I am guessing you have a value in the register which is
 neither the reset value or 0 and this causing problems later in
 the reset/on the next reset. I do find the naming confusing in
 the error message there as it says link reset failed but the
 link_reset callback can't fail in the driver and I modified the
 reset callback. But I guess that is just oddities of the network
 stack I am not familiar with.
 
 The other thing that feels odd is (and again apologies as I know
 next to nothing about the networking stack) how come it is
 unexpected that the reset callback destroys the state of the
 device. Naively I would have expected that a reset callback would
 reset the device back to its default state. Here we seem to be
 trying to avoid that happening.

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

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

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

2014-11-06 Thread Charles Keepax 
On Thu, Nov 06, 2014 at 11:06:51AM +0200, Riku Voipio wrote:
 On Wed, Nov 05, 2014 at 03:02:58PM +, 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
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2014-11-06 Thread Riku Voipio 
On Thu, Nov 06, 2014 at 10:01:04AM +, 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.voi...@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
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2014-11-06 Thread 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.voi...@iki.fi] 
Sent: Thursday, November 06, 2014 13:04 PM
To: Charles Keepax
Cc: Riku Voipio; Stam, Michel [FINT]; fre...@asix.com.tw;
da...@davemloft.net; linux-usb@vger.kernel.org; net...@vger.kernel.org;
linux-ker...@vger.kernel.org; linux-samsung-...@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 +, 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.voi...@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
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2014-11-06 Thread 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
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2014-11-04 Thread 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.voi...@iki.fi] 
Sent: Tuesday, November 04, 2014 8:23 AM
To: da...@davemloft.net; Stam, Michel [FINT]
Cc: linux-usb@vger.kernel.org; net...@vger.kernel.org;
linux-ker...@vger.kernel.org; linux-samsung-...@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: 01
[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-1211.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.s...@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: 01
[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-1211.usb-3.2.4, ASIX AX88772 USB 2.0 Ethernet, 12:59:f1:a8:43:90
[7.098998] asix 3-3.2.4:1.0 eth0: link up, 100Mbps, full-duplex, lpa
0xC5E1
[7.118258] Sending DHCP requests ., OK
[9.753259] IP-Config: Got DHCP answer from 192.168.1.1, my address
is 192.168.1.111

There might something wrong on the samsung platform code (I understand
the USB on arndale is funny), but this is still an regression from
3.17.

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

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

2014-11-04 Thread 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.voi...@iki.fi] 
 Sent: Tuesday, November 04, 2014 8:23 AM
 To: da...@davemloft.net; Stam, Michel [FINT]
 Cc: linux-usb@vger.kernel.org; net...@vger.kernel.org;
 linux-ker...@vger.kernel.org; linux-samsung-...@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: 01
 [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-1211.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.s...@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: 01
 [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-1211.usb-3.2.4, ASIX AX88772 USB 2.0 Ethernet, 12:59:f1:a8:43:90
 [7.098998] asix 3-3.2.4:1.0 eth0: link up, 100Mbps, full-duplex, lpa
 0xC5E1
 [7.118258] Sending DHCP requests ., OK
 [9.753259] IP-Config: Got DHCP answer from 192.168.1.1, my address
 is 192.168.1.111
 
 There might something wrong on the samsung platform code (I understand
 the USB on arndale is funny), but this is still an regression from
 3.17.
 
 Riku
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2014-11-04 Thread 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.voi...@iki.fi] 
Sent: Tuesday, November 04, 2014 10:44 AM
To: Stam, Michel [FINT]
Cc: Riku Voipio; da...@davemloft.net; linux-usb@vger.kernel.org;
net...@vger.kernel.org; linux-ker...@vger.kernel.org;
linux-samsung-...@vger.kernel.org; ckee...@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.voi...@iki.fi]
 Sent: Tuesday, November 04, 2014 8:23 AM
 To: da...@davemloft.net; Stam, Michel [FINT]
 Cc: linux-usb@vger.kernel.org; net...@vger.kernel.org; 
 linux-ker...@vger.kernel.org; linux-samsung-...@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: 01
 [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-1211.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.s...@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

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

2014-11-04 Thread 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.voi...@iki.fi] 
 Sent: Tuesday, November 04, 2014 10:44 AM
 To: Stam, Michel [FINT]
 Cc: Riku Voipio; da...@davemloft.net; linux-usb@vger.kernel.org;
 net...@vger.kernel.org; linux-ker...@vger.kernel.org;
 linux-samsung-...@vger.kernel.org; ckee...@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.voi...@iki.fi]
  Sent: Tuesday, November 04, 2014 8:23 AM
  To: da...@davemloft.net; Stam, Michel [FINT]
  Cc: linux-usb@vger.kernel.org; net...@vger.kernel.org; 
  linux-ker...@vger.kernel.org; linux-samsung-...@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

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

2014-11-03 Thread 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: 01
[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-1211.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.s...@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: 01
[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-1211.usb-3.2.4, ASIX AX88772 USB 2.0 Ethernet, 12:59:f1:a8:43:90
[7.098998] asix 3-3.2.4:1.0 eth0: link up, 100Mbps, full-duplex, lpa 0xC5E1
[7.118258] Sending DHCP requests ., OK
[9.753259] IP-Config: Got DHCP answer from 192.168.1.1, my address is 
192.168.1.111

There might something wrong on the samsung platform code (I understand the
USB on arndale is funny), but this is still an regression from 3.17.

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