Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932093AbaDBKyp (ORCPT ); Wed, 2 Apr 2014 06:54:45 -0400 Received: from h1446028.stratoserver.net ([85.214.92.142]:44304 "EHLO mail.ahsoftware.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758161AbaDBKym (ORCPT ); Wed, 2 Apr 2014 06:54:42 -0400 Message-ID: <533BEC5B.1080607@ahsoftware.de> Date: Wed, 02 Apr 2014 12:54:19 +0200 From: Alexander Holler User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.4.0 MIME-Version: 1.0 To: Florian Fainelli , Sebastian Hesselbarth CC: "linux-kernel@vger.kernel.org" , netdev , Michal Simek , stable Subject: Re: [PATCH regression] net: phy: fix initialization (config_init) for Marvel 88E1116R PHYs References: <1396396559-6971-1-git-send-email-holler@ahsoftware.de> <533BD3CD.1010905@ahsoftware.de> In-Reply-To: <533BD3CD.1010905@ahsoftware.de> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Am 02.04.2014 11:09, schrieb Alexander Holler: > Am 02.04.2014 02:57, schrieb Florian Fainelli: >> 2014-04-01 16:55 GMT-07:00 Alexander Holler : >>> Commit 7cd1463664c2a15721ff4ccfb61d4d970815cb3d (introduced with 3.14) >>> changed the initialization of the mv643xx_eth driver to use >>> phy_init_hw() >>> to reset the PHY. Unfortunately the initialization for the 88E1116R PHY >>> was broken such, that it used mdelay() instead of really waiting for a >>> reset to finish. >> >> So the only big difference before >> 7cd1463664c2a15721ff4ccfb61d4d970815cb3d ("net: mv643xx_eth: use >> phy_init_hw to reset PHY") is that we waited potentially forever for >> the BMCR_RESET bit to get cleared, while now, we only wait for up to >> 500 milliseconds. >> >> Could you verify the following two things before your patch gets merged: >> >> - how long does it take for your PHY to clear the BMCR_RESET bit, is >> it more than the allowed time out in >> drivers/net/phy/phy_device.c::phy_poll_reset >> - is your PHY powered down (check BMCR_PWRDOWN), if that is the case, >> we might be hitting some corner case where toggling BMCR_RESET will >> power it on, but at the expense of waiting longer > > I've done two tests with pr_info before and after the two resets in > m88e1116r_config_init: > > with my patch: > ----------------------- > dmesg | grep -A1 -B1 AHO > [ 1.090099] mv643xx_eth: MV-643xx 10/100/1000 ethernet driver version > 1.4 > [ 1.175916] AHO: before first reset > [ 1.179613] AHO: after first reset > [ 1.183743] AHO: before second reset > [ 1.187530] AHO: after second reset > [ 1.191905] mv643xx_eth_port mv643xx_eth_port.0 eth0: port 0 with MAC > address xx > -- > [ 1.426487] netpoll: netconsole: device eth0 not up yet, forcing it > [ 1.505986] AHO: before first reset > [ 1.509725] AHO: after first reset > [ 1.513899] AHO: before second reset > [ 1.517754] AHO: after second reset > [ 1.521802] IPv6: ADDRCONF(NETDEV_UP): eth0: link is not ready > -- > [ 21.372591] Adding 2996116k swap on /dev/sda3. Priority:-1 extents:1 > across:2996116k > [ 28.305989] AHO: before first reset > [ 28.306200] AHO: after first reset > [ 28.306936] AHO: before second reset > [ 28.307153] AHO: after second reset > [ 31.509973] mv643xx_eth_port mv643xx_eth_port.0 eth0: link up, 1000 > Mb/s, full duplex, flow control disabled > ----------------------- > > > with mdelay (the value after reset is what contains MII_BMCR): > > ----------------------- > dmesg | grep -A1 -B1 AHO > [ 1.090072] mv643xx_eth: MV-643xx 10/100/1000 ethernet driver version > 1.4 > [ 1.175888] AHO: before first reset > [ 1.678806] AHO: after first reset 0x0 > [ 1.683281] AHO: before second reset > [ 2.186288] AHO: after second reset 0x0 > [ 2.191010] mv643xx_eth_port mv643xx_eth_port.0 eth0: port 0 with MAC > address xx > -- > [ 2.426349] netpoll: netconsole: device eth0 not up yet, forcing it > [ 2.505917] AHO: before first reset > [ 2.605824] usb 1-1: new high-speed USB device number 2 using orion-ehci > -- > [ 2.829987] hub 1-1:1.0: 4 ports detected > [ 3.044502] AHO: after first reset 0x0 > [ 3.049133] AHO: before second reset > [ 3.126110] usb 1-1.1: new high-speed USB device number 3 using > orion-ehci > -- > [ 3.526107] usb 1-1.3: device descriptor read/64, error -32 > [ 3.614264] AHO: after second reset 0x0 > [ 3.618708] IPv6: ADDRCONF(NETDEV_UP): eth0: link is not ready > -- > [ 21.335730] Adding 2996116k swap on /dev/sda3. Priority:-1 extents:1 > across:2996116k > [ 28.195942] AHO: before first reset > [ 28.696270] AHO: after first reset 0x800 > [ 28.696958] AHO: before second reset > [ 29.197354] AHO: after second reset 0x800 > [ 111.612263] RPC: Registered named UNIX socket transport module. > ----------------------- > > So we see, the first reset in the last call of m88e1116r_config_init() > fails to complete in 500ms and the phy seems to be stuck afterwards, > but, for whatever reason, it doesn't need 500ms if mdelay isn't used (if > we can trust the timestamps). > > (You can also see, I have netconsole enabled using netconsole=... in the > kernel cmdline). > > That behaviour is reproducible. The first reset in the last call to > m88e1116r_config_init() always fails if mdelay is used. I've forgotten to add that MII_BMCR is always zero when m88e1116r_config_init() is called. So to conclude, I've no idea why using mdelay seems to break the PHY. It looks like the forced delay of together one second in m88e1116r_config_init() somehow breaks something, but that's pure speculation. And as my patch fixes things here, I've no reason to dig further. Regards, Alexander Holler -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/