From patchwork Tue Jul 25 11:41:32 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mason X-Patchwork-Id: 9861827 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 3FF08602B1 for ; Tue, 25 Jul 2017 11:42:21 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 3907E28650 for ; Tue, 25 Jul 2017 11:42:21 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 2DD1D28651; Tue, 25 Jul 2017 11:42:21 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-1.9 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, FREEMAIL_FROM, RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [65.50.211.133]) (using TLSv1.2 with cipher AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 6609828655 for ; Tue, 25 Jul 2017 11:42:20 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Date: Message-ID:References:To:From:Subject:Reply-To:Content-ID:Content-Description :Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=wrOWgfPmQQevLJsHCe7ku4po4J2V1pvBR2zYMnOIq5I=; b=KGvl7JBXgksN7N su/yRfTHz3CMUvoh2fayW9o7YNBKgyWFcRurW5QhpeSH9D6B6Tus4vMGG8Acl8KonXjnNdHkUyNGD sXQRLWbD/osW0gPs5/iVT3mPa5/jX+ZO+Qu7fd9+AHMJn8jr60MjV22qNaqsX2ULU/xPstGul3Waj D+Vo9AyWix5T2sJ4+aK3yAEZz3IJuQ9JoiVcIosK5L4ULRaF/cjHOEZj1efskPWvRiiM/mflIeeA8 rkOrUNE3CzX22vkvGa15vcAT7vQ/5c96/1Yc4ISKZt+N6S5/rHScyGq7PLezrt9L8nQgyHJbgNggP ctcNzWc9m7209jk2+Ehg==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux)) id 1dZyEB-0001q7-8v; Tue, 25 Jul 2017 11:42:15 +0000 Received: from smtp5-g21.free.fr ([2a01:e0c:1:1599::14]) by bombadil.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux)) id 1dZyE7-0001kw-FH for linux-arm-kernel@lists.infradead.org; Tue, 25 Jul 2017 11:42:13 +0000 Received: from [172.27.0.114] (unknown [92.154.11.170]) (Authenticated sender: slash.tmp) by smtp5-g21.free.fr (Postfix) with ESMTPSA id 2AEDF5FFBA; Tue, 25 Jul 2017 13:41:33 +0200 (CEST) Subject: Re: Problem with PHY state machine when using interrupts From: Mason To: Florian Fainelli , Andrew Lunn , Mans Rullgard References: <65b9f1f5-724c-cd03-1c39-59405e773efa@free.fr> <849513d9-c981-ec20-5a10-08c663d0aa37@free.fr> <02e9e171-ec22-e491-5bc8-ef4b3ad7fec2@gmail.com> <0f56083f-d43e-a141-1470-b76546af6d58@gmail.com> <62085f48-df18-f987-f521-dcdfbd7f574b@gmail.com> <1c00935b-3a3c-4268-db99-a0487ddece76@free.fr> Message-ID: Date: Tue, 25 Jul 2017 13:41:32 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Firefox/52.0 SeaMonkey/2.49.1 MIME-Version: 1.0 In-Reply-To: <1c00935b-3a3c-4268-db99-a0487ddece76@free.fr> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20170725_044211_798157_7979B363 X-CRM114-Status: GOOD ( 16.73 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: netdev , Linux ARM Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP On 25/07/2017 12:51, Mason wrote: > Moving the call to phy_stop() down after all the MAC tear down > avoids the hang. > > As far as I understand, when we are shutting everything down, > we don't need the phy_state_machine to run asynchronously. > We can run it synchronously one last time after the delayed > stuff has been disabled. Below is my current WIP diff. (It conflates the two issues I've been discussing. Splitting the diff required.) Tested in interrupt mode: # ip link set eth0 up [ 11.107547] Atheros 8035 ethernet 26000.nb8800-mii:04: PHY state change UP -> AN [ 14.530329] nb8800 26000.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx [ 14.538136] Atheros 8035 ethernet 26000.nb8800-mii:04: PHY state change AN -> RUNNING # ip link set eth0 down [ 23.801018] nb8800 26000.ethernet eth0: Link is Down # ip link set eth0 up [ 28.740870] Atheros 8035 ethernet 26000.nb8800-mii:04: PHY state change UP -> AN [ 31.431528] nb8800 26000.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx [ 31.439350] Atheros 8035 ethernet 26000.nb8800-mii:04: PHY state change AN -> RUNNING Works as expected. Tested in polling mode: # ip link set eth0 up [ 23.001199] Atheros 8035 ethernet 26000.nb8800-mii:04: PHY state change UP -> AN [ 24.024315] Atheros 8035 ethernet 26000.nb8800-mii:04: PHY state change AN -> NOLINK [ 27.064355] nb8800 26000.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx [ 27.072156] Atheros 8035 ethernet 26000.nb8800-mii:04: PHY state change NOLINK -> RUNNING # ip link set eth0 down [ 42.134617] nb8800 26000.ethernet eth0: Link is Down # ip link set eth0 up [ 48.381185] Atheros 8035 ethernet 26000.nb8800-mii:04: PHY state change UP -> AN [ 49.410976] Atheros 8035 ethernet 26000.nb8800-mii:04: PHY state change AN -> NOLINK [ 51.437686] nb8800 26000.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx [ 51.445486] Atheros 8035 ethernet 26000.nb8800-mii:04: PHY state change NOLINK -> RUNNING Works as expected. Also tested on my old board, no regression seen. Can you confirm that the changes to drivers/net/phy/phy.c look reasonable? Regards. diff --git a/drivers/net/ethernet/aurora/nb8800.c b/drivers/net/ethernet/aurora/nb8800.c index e94159507847..b5eba7958871 100644 --- a/drivers/net/ethernet/aurora/nb8800.c +++ b/drivers/net/ethernet/aurora/nb8800.c @@ -41,6 +41,7 @@ static void nb8800_tx_done(struct net_device *dev); static int nb8800_dma_stop(struct net_device *dev); +static int nb8800_init(struct net_device *dev); static inline u8 nb8800_readb(struct nb8800_priv *priv, int reg) { @@ -957,6 +958,9 @@ static int nb8800_open(struct net_device *dev) struct phy_device *phydev; int err; + nb8800_init(dev); + priv->speed = 0; + /* clear any pending interrupts */ nb8800_writel(priv, NB8800_RXC_SR, 0xf); nb8800_writel(priv, NB8800_TXC_SR, 0xf); @@ -1004,8 +1008,6 @@ static int nb8800_stop(struct net_device *dev) struct nb8800_priv *priv = netdev_priv(dev); struct phy_device *phydev = dev->phydev; - phy_stop(phydev); - netif_stop_queue(dev); napi_disable(&priv->napi); @@ -1013,6 +1015,7 @@ static int nb8800_stop(struct net_device *dev) nb8800_mac_rx(dev, false); nb8800_mac_tx(dev, false); + phy_stop(phydev); phy_disconnect(phydev); free_irq(dev->irq, dev); @@ -1350,6 +1353,21 @@ static int nb8800_tango4_init(struct net_device *dev) }; MODULE_DEVICE_TABLE(of, nb8800_dt_ids); +static int nb8800_init(struct net_device *dev) +{ +#ifndef RESET_IN_PROBE + struct nb8800_priv *priv = netdev_priv(dev); + const struct nb8800_ops *ops = priv->ops; + + ops->reset(dev); + nb8800_hw_init(dev); + ops->init(dev); + nb8800_update_mac_addr(dev); +#endif + + return 0; +} + static int nb8800_probe(struct platform_device *pdev) { const struct of_device_id *match; @@ -1389,6 +1407,7 @@ static int nb8800_probe(struct platform_device *pdev) priv = netdev_priv(dev); priv->base = base; + priv->ops = ops; priv->phy_mode = of_get_phy_mode(pdev->dev.of_node); if (priv->phy_mode < 0) @@ -1407,11 +1426,13 @@ static int nb8800_probe(struct platform_device *pdev) spin_lock_init(&priv->tx_lock); +#ifdef RESET_IN_PROBE if (ops && ops->reset) { ret = ops->reset(dev); if (ret) goto err_disable_clk; } +#endif bus = devm_mdiobus_alloc(&pdev->dev); if (!bus) { @@ -1454,6 +1475,7 @@ static int nb8800_probe(struct platform_device *pdev) priv->mii_bus = bus; +#ifdef RESET_IN_PROBE ret = nb8800_hw_init(dev); if (ret) goto err_deregister_fixed_link; @@ -1463,6 +1485,7 @@ static int nb8800_probe(struct platform_device *pdev) if (ret) goto err_deregister_fixed_link; } +#endif dev->netdev_ops = &nb8800_netdev_ops; dev->ethtool_ops = &nb8800_ethtool_ops; @@ -1476,7 +1499,9 @@ static int nb8800_probe(struct platform_device *pdev) if (!is_valid_ether_addr(dev->dev_addr)) eth_hw_addr_random(dev); +#ifdef RESET_IN_PROBE nb8800_update_mac_addr(dev); +#endif netif_carrier_off(dev); diff --git a/drivers/net/ethernet/aurora/nb8800.h b/drivers/net/ethernet/aurora/nb8800.h index 6ec4a956e1e5..d5f4481a2c7b 100644 --- a/drivers/net/ethernet/aurora/nb8800.h +++ b/drivers/net/ethernet/aurora/nb8800.h @@ -305,6 +305,7 @@ struct nb8800_priv { dma_addr_t tx_desc_dma; struct clk *clk; + const struct nb8800_ops *ops; }; struct nb8800_ops { diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c index d0626bf5c540..3bc505b56c71 100644 --- a/drivers/net/phy/phy.c +++ b/drivers/net/phy/phy.c @@ -14,6 +14,7 @@ */ #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt +#define DEBUG #include #include @@ -743,12 +744,16 @@ void phy_trigger_machine(struct phy_device *phydev, bool sync) */ void phy_stop_machine(struct phy_device *phydev) { + flush_delayed_work(&phydev->state_queue); cancel_delayed_work_sync(&phydev->state_queue); mutex_lock(&phydev->lock); if (phydev->state > PHY_UP && phydev->state != PHY_HALTED) phydev->state = PHY_UP; mutex_unlock(&phydev->lock); + + /* Now we can run the state machine synchronously */ + phy_state_machine(&phydev->state_queue.work); } /**