From patchwork Wed Nov 15 10:53:23 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Marc Gonzalez X-Patchwork-Id: 10059147 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 6BAE26023A for ; Wed, 15 Nov 2017 10:53:56 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 54E3129EAC for ; Wed, 15 Nov 2017 10:53:56 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 4905F29EB0; Wed, 15 Nov 2017 10:53:56 +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=-4.2 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,RCVD_IN_DNSWL_MED 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 5490229EAC for ; Wed, 15 Nov 2017 10:53:55 +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:From:References:To:Subject:Reply-To:Content-ID:Content-Description :Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=uEJrzd3mIxk3FawNrGS1aRpEUlkQ3OGpMkag2czLQOk=; b=Qs5DcX2IL+Rzjh dZ8KikZZP+nziDTuJJuEQH7okLIZiPxdnP6/4p3FSuGvxWBu4+eemnnmc6ZYGp03XJntvGVxRLBrP zGA4d+HMj4jX9/+rZ5G1SgJVHAE/Y5yl9Z13mBq7GS/1xxyO/7vqhk2nU4azceiYzswnCxG9B+DNJ dzSxFq7WFK3M/F41IfY5s59LXPVZq3yiGDuG7R53GFtPlx2xd0Ng3ILgvZpkeFpngYjTNVNMtOU45 xkyAHTY2oxxm2mHZ3JZwcLlSDEbbsn7puEeLrD/eIhE+FBqcJaI0WYOjRqXYIuAAdOEMw4+1BWGvp WirpRyEz0AsketMwbwZw==; 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 1eEvKN-0006Yd-0o; Wed, 15 Nov 2017 10:53:55 +0000 Received: from us-smtp-delivery-107.mimecast.com ([216.205.24.107]) by bombadil.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux)) id 1eEvKK-0006X9-C4 for linux-arm-kernel@lists.infradead.org; Wed, 15 Nov 2017 10:53:54 +0000 Received: from CPH-EX1.SDESIGNS.COM (195-215-56-170-static.dk.customer.tdc.net [195.215.56.170]) (Using TLS) by us-smtp-1.mimecast.com with ESMTP id us-mta-163-1-TPlfdeNWih7OldKXwYWw-1; Wed, 15 Nov 2017 05:53:27 -0500 X-MC-Unique: 1-TPlfdeNWih7OldKXwYWw-1 Received: from [172.27.0.114] (172.27.0.114) by CPH-EX1.sdesigns.com (192.168.10.36) with Microsoft SMTP Server (TLS) id 14.3.294.0; Wed, 15 Nov 2017 11:53:24 +0100 Subject: Re: [PATCH v3 2/4] net: nb8800: Simplify nb8800_pause_config() To: Mans Rullgard References: <230165aa-eaf1-6e2b-7ff3-45b3ee4ffc62@sigmadesigns.com> <569542b1-8e56-d7da-f7a0-affd89bfed62@sigmadesigns.com> From: Marc Gonzalez Message-ID: <6f534b2b-f69a-bda5-dc5b-0281bf0df129@sigmadesigns.com> Date: Wed, 15 Nov 2017 11:53:23 +0100 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: X-Originating-IP: [172.27.0.114] X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20171115_025352_492391_BF5521E6 X-CRM114-Status: GOOD ( 15.31 ) 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: Florian Fainelli , Mason , netdev , Thibaud Cornic , David Miller , 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 14/11/2017 14:22, Måns Rullgård wrote: > Marc Gonzalez wrote: > >> On 14/11/2017 13:38, Måns Rullgård wrote: >> >>> Marc Gonzalez writes: >>> >>>> The "flow control enable" bit can be tweaked, even if DMA is enabled. >>> >>> No, it can't. Maybe on some of your newer chips it can, but not on the >>> older ones. >> >> Again, I suppose you are referring to your SMP8642-based board. >> >> Are you saying you tested this patch, and it doesn't work? >> What are the symptoms? > > I didn't test that patch per se. I did initially try simply changing > that bit, and this didn't work. Either it had no effect, or it locked > up the controller, I forget which. The manual explicitly states that > writes are forbidden while the DMA enabled bit is set. > > If shutting down the DMA really isn't possible, I would rather just > allow changing the flow control setting only when the interface is > stopped. The normal case of getting the initial setting from the > auto-negotiation will still work properly. It just won't be possible to > change it while the link is active. Hello Mans, With the default setup, priv->pause_aneg = true; priv->pause_rx = true; priv->pause_tx = true; even the very first call to nb8800_pause_config() occurs with netif_running=1 as well as the RX DMA bit enabled. buildroot login: root # ip addr add 172.27.64.23/18 brd 172.27.127.255 dev eth0 # ip link set eth0 up [ 25.771000] ENTER nb8800_pause_config: netif_running=1 [ 25.776195] CPU: 0 PID: 193 Comm: kworker/0:1 Not tainted 4.9.54-1-rc6 #18 [ 25.783102] Hardware name: Sigma Tango DT [ 25.787138] Workqueue: events_power_efficient phy_state_machine [ 25.793107] [] (unwind_backtrace) from [] (show_stack+0x10/0x14) [ 25.800896] [] (show_stack) from [] (dump_stack+0x88/0x9c) [ 25.808160] [] (dump_stack) from [] (nb8800_pause_config+0x28/0xf0) [ 25.816208] [] (nb8800_pause_config) from [] (nb8800_link_reconfigure+0x134/0x148) [ 25.825565] [] (nb8800_link_reconfigure) from [] (phy_state_machine+0x348/0x468) [ 25.834750] [] (phy_state_machine) from [] (process_one_work+0x1d8/0x3f0) [ 25.843323] [] (process_one_work) from [] (worker_thread+0x4c/0x560) [ 25.851459] [] (worker_thread) from [] (kthread+0xec/0xf4) [ 25.858721] [] (kthread) from [] (ret_from_fork+0x14/0x3c) [ 25.874597] nb8800 26000.ethernet eth0: Link is Up - 100Mbps/Full - flow control rx/tx This makes sense, since nb8800_open() calls nb8800_start_rx() before phy_start(). Moving nb8800_start_rx() to after nb8800_pause_config() does appear to work, but I'm not sure it is the correct action? FWIW, this is the patch I'm testing: diff --git a/drivers/net/ethernet/aurora/nb8800.c b/drivers/net/ethernet/aurora/nb8800.c index da6e8bdbb77a..86081632346e 100644 --- a/drivers/net/ethernet/aurora/nb8800.c +++ b/drivers/net/ethernet/aurora/nb8800.c @@ -667,6 +667,7 @@ static void nb8800_link_reconfigure(struct net_device *dev) nb8800_mac_config(dev); nb8800_pause_config(dev); + nb8800_start_rx(dev); } if (phydev->link != priv->link) { @@ -918,7 +919,6 @@ static int nb8800_open(struct net_device *dev) napi_enable(&priv->napi); netif_start_queue(dev); - nb8800_start_rx(dev); phy_start(phydev); return 0;