From patchwork Thu Apr 9 11:44:44 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mason X-Patchwork-Id: 459708 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 9D4B3140157 for ; Thu, 9 Apr 2015 21:44:59 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755474AbbDILoz (ORCPT ); Thu, 9 Apr 2015 07:44:55 -0400 Received: from smtp2-g21.free.fr ([212.27.42.2]:27492 "EHLO smtp2-g21.free.fr" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755219AbbDILox (ORCPT ); Thu, 9 Apr 2015 07:44:53 -0400 Received: from [172.27.0.114] (unknown [83.142.147.193]) (Authenticated sender: shill) by smtp2-g21.free.fr (Postfix) with ESMTPSA id E337B4B0150; Thu, 9 Apr 2015 13:42:26 +0200 (CEST) Message-ID: <5526662C.8010509@free.fr> Date: Thu, 09 Apr 2015 13:44:44 +0200 From: Mason User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:35.0) Gecko/20100101 Firefox/35.0 SeaMonkey/2.32.1 MIME-Version: 1.0 To: Florian Fainelli , netdev@vger.kernel.org CC: Mugunthan , "David S. Miller" , Matus Ujhelyi , Daniel Mack Subject: Re: Atheros 8035 PHY only works when at803x_config_init() is commented out References: <5525571D.7060909@free.fr> <5525658D.7000709@gmail.com> In-Reply-To: <5525658D.7000709@gmail.com> Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org [ Problem with Atheros AR8035 PHY reset ] Florian Fainelli wrote: > Mason wrote: > >> I have a weird problem, that I've only started investigating, and I'd be >> grateful if anyone can help me along. >> >> I have an ARM-based SoC that provides (AFAIU) an Atheros 8035 PHY. >> >> If I enable the driver in my kernel (CONFIG_AT803X_PHY) then something >> goes wrong, and the system cannot load the nfsroot. Bizarrely, DHCP >> seems to work (so there is limited connectivity) but then the NFS >> server just times out. >> >> However, if I just comment this line: >> //.config_init = at803x_config_init, >> in at803x_driver[0] >> then everything seems to work. (At least, the system sets the PHY >> to Gbit, and manages to download the nfsroot.) >> >> If I'm reading the code right (big "if"), when config_init is NULL, >> then phy_init_hw() turns into a NOP? >> >> So either at803x_config_init() or something called from phy_init_hw() >> seems to be hosing this system's networking? >> >> Sprinkling printk over phy_init_hw shows no errors (no premature exit) >> and phydev->drv->config_init(phydev) returns 0. >> >> Maybe I didn't set something up correctly, and phy_write() is scribbling >> over the wrong register? >> >> Does anyone see something I missed? > > So one possibility could be that the bootloader initializes the PHY in a > certain way, typically by applying workarounds, and your config_init() > callback is not restoring any of theses, which is why, after > phy_init_hw(), which does a software reset of the PHY, all of these > workarounds are wiped out, and your PHY behaves funky. > > The reason why config_init() needs to put the PHY back into a fully > functional state is because the PHY library should be able to software > reset the PHY when it needs to, but also be able to deal with deep sleep > modes etc.. where the PHY could loose its settings, yet the kernel > should be able to bring you back in a good state. > > An easy way to bypass that is to provide a soft_reset callback which does > nothing, and see if calling either at803x_config_init(), or > genphy_config_init() is sufficient to preserve the PHY settings. > Although the real solution is to look at what the bootloader does on > that front and replicate it in the config_init() callback. Here is the data sheet for the AR8035: http://www.redeszone.net/app/uploads/2014/04/AR8035.pdf It seems the problem comes from the fact that the PHY treats HW reset and SW reset differently: HW reset: registers are set to specific values SW reset: some bits are retained across reset Case in point: the control register (BMCR) HW reset: BMCR = 0x3100 SW reset: retain bits[6,8,12,13] / other bits = 0 (0x3100 means bit_6=0, bit_8=bit_12=bit_13=1) When we execute this line from genphy_soft_reset() phy_write(phydev, MII_BMCR, BMCR_RESET); we reset the bits in BMCR, and SW reset does not restore them. It seems to me (please tell me if I am wrong) that it should be safe for all PHYs to write old_val | BMCR_RESET, instead of just BMCR_RESET. Thus... On chips that REALLY reset, the old_val bits will be discarded; while on chips that retain some bits, we do want to keep them. So the patch would go something like below. What do you think? Regards --- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c index bdfe51f..4bcbde3 100644 --- a/drivers/net/phy/phy_device.c +++ b/drivers/net/phy/phy_device.c @@ -1087,9 +1087,10 @@ static int gen10g_read_status(struct phy_device *phydev) */ int genphy_soft_reset(struct phy_device *phydev) { - int ret; + int ret, val; - ret = phy_write(phydev, MII_BMCR, BMCR_RESET); + val = phy_read (phydev, MII_BMCR); + ret = phy_write(phydev, MII_BMCR, val | BMCR_RESET); if (ret < 0) return ret;