From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-3.8 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 61680C43603 for ; Thu, 12 Dec 2019 19:06:45 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 435F221556 for ; Thu, 12 Dec 2019 19:06:45 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730284AbfLLTGo (ORCPT ); Thu, 12 Dec 2019 14:06:44 -0500 Received: from guitar.tcltek.co.il ([192.115.133.116]:36337 "EHLO mx.tkos.co.il" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1730096AbfLLTGo (ORCPT ); Thu, 12 Dec 2019 14:06:44 -0500 Received: from sapphire.tkos.co.il (unknown [192.168.100.188]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx.tkos.co.il (Postfix) with ESMTPS id C089D440406; Thu, 12 Dec 2019 21:06:23 +0200 (IST) Date: Thu, 12 Dec 2019 21:06:40 +0200 From: Baruch Siach To: Marek Behun Cc: Andrew Lunn , Vivien Didelot , netdev@vger.kernel.org, Denis Odintsov , Hubert Feurstein Subject: Re: [BUG] mv88e6xxx: tx regression in v5.3 Message-ID: <20191212190640.6vki2pjfacdnxihh@sapphire.tkos.co.il> References: <87tv67tcom.fsf@tarshish> <20191211131111.GK16369@lunn.ch> <87fthqu6y6.fsf@tarshish> <20191211174938.GB30053@lunn.ch> <20191212085045.nqhfldkbebqzzamv@sapphire.tkos.co.il> <20191212131448.GA9959@lunn.ch> <20191212150810.zx6o26jnk5croh4r@sapphire.tkos.co.il> <20191212151355.GE30053@lunn.ch> <20191212152355.iiepmi4cjriddeon@sapphire.tkos.co.il> <20191212193611.63111051@nic.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20191212193611.63111051@nic.cz> Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Hi Marek, On Thu, Dec 12, 2019 at 07:36:11PM +0100, Marek Behun wrote: > On Thu, 12 Dec 2019 17:23:55 +0200 > Baruch Siach wrote: > > > > On Thu, Dec 12, 2019 at 04:13:55PM +0100, Andrew Lunn wrote: > > > > I compared phylib to phylink calls to mv88e6xxx_port_setup_mac(). It turns out > > > > that the phylink adds mv88e6xxx_port_setup_mac() call for the cpu port (port 5 > > > > in my case) with these parameters and call stack: > > > > > > > > [ 4.219148] mv88e6xxx_port_setup_mac: port: 5 link: 0 speed: -1 duplex: 255 > > > > [ 4.226144] CPU: 2 PID: 21 Comm: kworker/2:0 Not tainted 5.3.15-00003-gb9bb09189d02-dirty #104 > > > > [ 4.234795] Hardware name: SolidRun ClearFog GT 8K (DT) > > > > [ 4.240044] Workqueue: events deferred_probe_work_func > > > > [ 4.245205] Call trace: > > > > [ 4.247661] dump_backtrace+0x0/0x128 > > > > [ 4.251339] show_stack+0x14/0x1c > > > > [ 4.254669] dump_stack+0xa4/0xd0 > > > > [ 4.257998] mv88e6xxx_port_setup_mac+0x78/0x2a0 > > > > [ 4.262635] mv88e6xxx_mac_config+0xd0/0x154 > > > > [ 4.266924] dsa_port_phylink_mac_config+0x2c/0x38 > > > > [ 4.271736] phylink_mac_config+0xe0/0x1cc > > > > [ 4.275849] phylink_start+0xc8/0x224 > > > > [ 4.279527] dsa_port_link_register_of+0xe8/0x1b0 > > > > [ 4.284251] dsa_register_switch+0x7fc/0x908 > > > > [ 4.288539] mv88e6xxx_probe+0x62c/0x66c > > > > [ 4.292478] mdio_probe+0x30/0x5c > > > > [ 4.295806] really_probe+0x1d0/0x280 > > > > [ 4.299483] driver_probe_device+0xd4/0xe4 > > > > [ 4.303596] __device_attach_driver+0x94/0xa0 > > > > [ 4.307971] bus_for_each_drv+0x94/0xb4 > > > > [ 4.311823] __device_attach+0xc0/0x12c > > > > [ 4.315674] device_initial_probe+0x10/0x18 > > > > [ 4.319875] bus_probe_device+0x2c/0x8c > > > > [ 4.323726] deferred_probe_work_func+0x84/0x98 > > > > [ 4.328276] process_one_work+0x19c/0x258 > > > > [ 4.332303] process_scheduled_works+0x3c/0x40 > > > > [ 4.336765] worker_thread+0x228/0x2f8 > > > > [ 4.340530] kthread+0x114/0x124 > > > > [ 4.343771] ret_from_fork+0x10/0x18 > > > > > > > > This hunk removed that mv88e6xxx_port_setup_mac() call, and fixed Tx for me on > > > > v5.3.15: > > > > > > > > diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c > > > > index d0a97eb73a37..f0457274b5a9 100644 > > > > --- a/drivers/net/dsa/mv88e6xxx/chip.c > > > > +++ b/drivers/net/dsa/mv88e6xxx/chip.c > > > > @@ -611,6 +611,9 @@ static void mv88e6xxx_mac_config(struct dsa_switch *ds, int port, > > > > if ((mode == MLO_AN_PHY) && mv88e6xxx_phy_is_internal(ds, port)) > > > > return; > > > > > > > > + if (dsa_is_cpu_port(ds, port)) > > > > + return; > > > > + > > > > if (mode == MLO_AN_FIXED) { > > > > link = LINK_FORCED_UP; > > > > speed = state->speed; > > > > > > > > Is that the right solution? > > > > > > What needs testing is: > > > > > > port@0 { > > > reg = <0>; > > > label = "cpu"; > > > ethernet = <&fec1>; > > > > > > fixed-link { > > > speed = <100>; > > > full-duplex; > > > }; > > > > > > At some point, there is a call to configure the CPU port to 100Mbps, > > > because the SoC Ethernet does not support 1G. We need to ensure this > > > does not break with your change. > > > > So maybe this: > > > > diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c > > index d0a97eb73a37..84ca4f36a778 100644 > > --- a/drivers/net/dsa/mv88e6xxx/chip.c > > +++ b/drivers/net/dsa/mv88e6xxx/chip.c > > @@ -611,6 +611,9 @@ static void mv88e6xxx_mac_config(struct dsa_switch *ds, int port, > > if ((mode == MLO_AN_PHY) && mv88e6xxx_phy_is_internal(ds, port)) > > return; > > > > + if (mode != MLO_AN_FIXED && dsa_is_cpu_port(ds, port)) > > + return; > > + > > if (mode == MLO_AN_FIXED) { > > link = LINK_FORCED_UP; > > speed = state->speed; > > No, if your switch is connected to the cpu via 100mbps on the cpu side, > you have to add fixed-link node as Andrew suggested. This is what we > were trying to do with the Topaz patches, so that dsa port nodes > support fixed-link via phylink. Thanks for the clarification. This also works without the hunk above: diff --git a/arch/arm64/boot/dts/marvell/armada-8040-clearfog-gt-8k.dts b/arch/arm64/boot/dts/marvell/armada-8040-clearfog-gt-8k.dts index bd881497b872..8f61cae9d3b0 100644 --- a/arch/arm64/boot/dts/marvell/armada-8040-clearfog-gt-8k.dts +++ b/arch/arm64/boot/dts/marvell/armada-8040-clearfog-gt-8k.dts @@ -408,6 +408,11 @@ port@5 { reg = <5>; label = "cpu"; ethernet = <&cp1_eth2>; + + fixed-link { + speed = <2500>; + full-duplex; + }; }; }; With this I get calls to mv88e6341_port_set_cmode() with PHY mode 0. So the other "fix" is still needed. Thanks, baruch -- http://baruch.siach.name/blog/ ~. .~ Tk Open Systems =}------------------------------------------------ooO--U--Ooo------------{= - baruch@tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -