From patchwork Tue Sep 22 11:20:10 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Russell King - ARM Linux X-Patchwork-Id: 7239241 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id D2201BEEC1 for ; Tue, 22 Sep 2015 13:52:43 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id BEF88206F3 for ; Tue, 22 Sep 2015 13:52:42 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 49435206F1 for ; Tue, 22 Sep 2015 13:52:41 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1ZeNws-00087f-6p; Tue, 22 Sep 2015 13:49:34 +0000 Received: from pandora.arm.linux.org.uk ([2001:4d48:ad52:3201:214:fdff:fe10:1be6]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1ZeNwm-0006uK-R5 for linux-arm-kernel@lists.infradead.org; Tue, 22 Sep 2015 13:49:32 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=arm.linux.org.uk; s=pandora-2014; h=Sender:In-Reply-To:Content-Type:MIME-Version:References:Message-ID:Subject:Cc:To:From:Date; bh=KolfAUSHceVICL9m6vYUElIfKZIR8lR60DNKo0KCYkM=; b=jCJi4OAm3JX7UUny0dBjxg+npz8mzbX7cjjbLgiwi5vn8Zhk5bp7iZyc3/dJryBrvMin+T5eHnsRYGExnwSnBbmkEkTHkzas2m4rBL4MZNgcfNi89jllhBm1YRip2ZUsucZok7cxSxgn98onsdAQLzk5od+UuPaeX8CdhFfJ2zI=; Received: from n2100.arm.linux.org.uk ([fd8f:7570:feb6:1:214:fdff:fe10:4f86]:45733) by pandora.arm.linux.org.uk with esmtpsa (TLSv1:DHE-RSA-AES256-SHA:256) (Exim 4.82_1-5b7a7c0-XX) (envelope-from ) id 1ZeLcL-0006rL-CP; Tue, 22 Sep 2015 12:20:13 +0100 Received: from linux by n2100.arm.linux.org.uk with local (Exim 4.76) (envelope-from ) id 1ZeLcI-0006Cz-H7; Tue, 22 Sep 2015 12:20:10 +0100 Date: Tue, 22 Sep 2015 12:20:10 +0100 From: Russell King - ARM Linux To: Andrew Lunn Subject: Re: DSA: phy polling Message-ID: <20150922112009.GX21084@n2100.arm.linux.org.uk> References: <20150914104254.GI21084@n2100.arm.linux.org.uk> <20150922021426.GE20288@lunn.ch> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20150922021426.GE20288@lunn.ch> User-Agent: Mutt/1.5.23 (2014-03-12) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20150922_064930_134520_071E0BE3 X-CRM114-Status: GOOD ( 28.24 ) X-Spam-Score: -2.7 (--) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: netdev@vger.kernel.org, linux-arm-kernel@lists.infradead.org Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-4.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED,RP_MATCHES_RCVD,T_DKIM_INVALID,UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Tue, Sep 22, 2015 at 04:14:26AM +0200, Andrew Lunn wrote: > On Mon, Sep 14, 2015 at 11:42:54AM +0100, Russell King - ARM Linux wrote: > > Andrew, > > > > I think you're the current maintainer of the Marvell DSA code, as being > > the most recent author of changes to it. :) > > Hi Russell > > Sorry for the slow reply, i've been on vacation. > > Humm, i suppose i might be the defacto Maintainer for Marvell parts, > but i've no NDA with Marvell, so no access to the data sheets etc. Like the rest of us :( > > I've noticed in my testing that the Marvell DSA code seems to poll the > > internal phy link status in mv88e6xxx_poll_link(), and set the network > > device carrier status according to the results. > > Peter Korsgaard comment might be correct, the switch needs to know > what the PHY has negotiated. Hence the use of the PPU. There are also > comments in the code that the PPU is needed for indirect access to the > PHY. I'm not changing the PPU at all - all I'm doing is to remove the extra layer of software polling which partially duplicates what the phy layer is already doing with its own polling. The poll_link code is reading the port PCS control register and the port status register, and using that to (a) control the netdev carrier state, and (b) printing the link state. Meanwhile, the PHY layer is polling the PHY that we created for the port, decoding its registers, and controlling the same netdev carrier state, parsing the standard MII register set to get the speed, duplex and other parameters, and dsa_slave_adjust_link() is the printing the new link state. This then goes on to program the switch port via mv88e6xxx_adjust_link() if necessary. So, it's the PHY layer which is doing all the real work, the poll_link code is just a duplication. My current patch for this looks like this, and it's what I'm running against the 88E6176 switch. Link state is detected via the phy layer and everything appears to work fine. I think mv88e6060_poll_link() in drivers/net/dsa/mv88e6060.c can go the same way too (it seems to be mostly the same as the code below, except it doesn't support gigabit). That would mean there's no users of the .poll_link method in drivers/net/dsa - and that probably means the link polling code in net/dsa can also be removed. drivers/net/dsa/mv88e6123_61_65.c | 1 - drivers/net/dsa/mv88e6131.c | 1 - drivers/net/dsa/mv88e6171.c | 1 - drivers/net/dsa/mv88e6352.c | 1 - drivers/net/dsa/mv88e6xxx.c | 67 --------------------------------------- drivers/net/dsa/mv88e6xxx.h | 1 - 6 files changed, 72 deletions(-) diff --git a/drivers/net/dsa/mv88e6123_61_65.c b/drivers/net/dsa/mv88e6123_61_65.c index 3de2a6d73fdc..4bcfd683bbea 100644 --- a/drivers/net/dsa/mv88e6123_61_65.c +++ b/drivers/net/dsa/mv88e6123_61_65.c @@ -125,7 +125,6 @@ struct dsa_switch_driver mv88e6123_61_65_switch_driver = { .set_addr = mv88e6xxx_set_addr_indirect, .phy_read = mv88e6xxx_phy_read, .phy_write = mv88e6xxx_phy_write, - .poll_link = mv88e6xxx_poll_link, .get_strings = mv88e6xxx_get_strings, .get_ethtool_stats = mv88e6xxx_get_ethtool_stats, .get_sset_count = mv88e6xxx_get_sset_count, diff --git a/drivers/net/dsa/mv88e6131.c b/drivers/net/dsa/mv88e6131.c index 3e8386529965..c73121c8f155 100644 --- a/drivers/net/dsa/mv88e6131.c +++ b/drivers/net/dsa/mv88e6131.c @@ -178,7 +178,6 @@ struct dsa_switch_driver mv88e6131_switch_driver = { .set_addr = mv88e6xxx_set_addr_direct, .phy_read = mv88e6131_phy_read, .phy_write = mv88e6131_phy_write, - .poll_link = mv88e6xxx_poll_link, .get_strings = mv88e6xxx_get_strings, .get_ethtool_stats = mv88e6xxx_get_ethtool_stats, .get_sset_count = mv88e6xxx_get_sset_count, diff --git a/drivers/net/dsa/mv88e6171.c b/drivers/net/dsa/mv88e6171.c index c2daaf087761..c95cfab56a4f 100644 --- a/drivers/net/dsa/mv88e6171.c +++ b/drivers/net/dsa/mv88e6171.c @@ -104,7 +104,6 @@ struct dsa_switch_driver mv88e6171_switch_driver = { .set_addr = mv88e6xxx_set_addr_indirect, .phy_read = mv88e6xxx_phy_read_indirect, .phy_write = mv88e6xxx_phy_write_indirect, - .poll_link = mv88e6xxx_poll_link, .get_strings = mv88e6xxx_get_strings, .get_ethtool_stats = mv88e6xxx_get_ethtool_stats, .get_sset_count = mv88e6xxx_get_sset_count, diff --git a/drivers/net/dsa/mv88e6352.c b/drivers/net/dsa/mv88e6352.c index 1f5129c105fb..37367060676f 100644 --- a/drivers/net/dsa/mv88e6352.c +++ b/drivers/net/dsa/mv88e6352.c @@ -324,7 +324,6 @@ struct dsa_switch_driver mv88e6352_switch_driver = { .set_addr = mv88e6xxx_set_addr_indirect, .phy_read = mv88e6xxx_phy_read_indirect, .phy_write = mv88e6xxx_phy_write_indirect, - .poll_link = mv88e6xxx_poll_link, .get_strings = mv88e6xxx_get_strings, .get_ethtool_stats = mv88e6xxx_get_ethtool_stats, .get_sset_count = mv88e6xxx_get_sset_count, diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c index f8baa897d1a0..c6683f65b125 100644 --- a/drivers/net/dsa/mv88e6xxx.c +++ b/drivers/net/dsa/mv88e6xxx.c @@ -388,73 +388,6 @@ int mv88e6xxx_phy_write_ppu(struct dsa_switch *ds, int addr, } #endif -void mv88e6xxx_poll_link(struct dsa_switch *ds) -{ - int i; - - for (i = 0; i < DSA_MAX_PORTS; i++) { - struct net_device *dev; - int uninitialized_var(port_status); - int pcs_ctrl; - int link; - int speed; - int duplex; - int fc; - - dev = ds->ports[i]; - if (dev == NULL) - continue; - - pcs_ctrl = mv88e6xxx_reg_read(ds, REG_PORT(i), PORT_PCS_CTRL); - if (pcs_ctrl < 0 || pcs_ctrl & PORT_PCS_CTRL_FORCE_LINK) - continue; - - link = 0; - if (dev->flags & IFF_UP) { - port_status = mv88e6xxx_reg_read(ds, REG_PORT(i), - PORT_STATUS); - if (port_status < 0) - continue; - - link = !!(port_status & PORT_STATUS_LINK); - } - - if (!link) { - if (netif_carrier_ok(dev)) { - netdev_info(dev, "link down\n"); - netif_carrier_off(dev); - } - continue; - } - - switch (port_status & PORT_STATUS_SPEED_MASK) { - case PORT_STATUS_SPEED_10: - speed = 10; - break; - case PORT_STATUS_SPEED_100: - speed = 100; - break; - case PORT_STATUS_SPEED_1000: - speed = 1000; - break; - default: - speed = -1; - break; - } - duplex = (port_status & PORT_STATUS_DUPLEX) ? 1 : 0; - fc = (port_status & PORT_STATUS_PAUSE_EN) ? 1 : 0; - - if (!netif_carrier_ok(dev)) { - netdev_info(dev, - "link up, %d Mb/s, %s duplex, flow control %sabled\n", - speed, - duplex ? "full" : "half", - fc ? "en" : "dis"); - netif_carrier_on(dev); - } - } -} - static bool mv88e6xxx_6065_family(struct dsa_switch *ds) { struct mv88e6xxx_priv_state *ps = ds_to_priv(ds); diff --git a/drivers/net/dsa/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx.h index 9b6f3d9d5ae1..d94a4e134ee2 100644 --- a/drivers/net/dsa/mv88e6xxx.h +++ b/drivers/net/dsa/mv88e6xxx.h @@ -442,7 +442,6 @@ void mv88e6xxx_ppu_state_init(struct dsa_switch *ds); int mv88e6xxx_phy_read_ppu(struct dsa_switch *ds, int addr, int regnum); int mv88e6xxx_phy_write_ppu(struct dsa_switch *ds, int addr, int regnum, u16 val); -void mv88e6xxx_poll_link(struct dsa_switch *ds); void mv88e6xxx_get_strings(struct dsa_switch *ds, int port, uint8_t *data); void mv88e6xxx_get_ethtool_stats(struct dsa_switch *ds, int port, uint64_t *data);