| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 12 Dec 2019 21:06:40 +0200
From: Baruch Siach <baruch@...s.co.il>
To: Marek Behun <marek.behun@....cz>
Cc: Andrew Lunn <andrew@...n.ch>,
Vivien Didelot <vivien.didelot@...il.com>,
netdev@...r.kernel.org,
Denis Odintsov <d.odintsov@...viangames.com>,
Hubert Feurstein <h.feurstein@...il.com>
Subject: Re: [BUG] mv88e6xxx: tx regression in v5.3
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 <baruch@...s.co.il> 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@...s.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -
Powered by blists - more mailing lists