Re: [RFC net-next 6/8] net: phylink: Configure MAC/PCS when link is up without PHY - Russell King

From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
To: Andrew Lunn <andrew@lunn.ch>
Cc: Jose Abreu <Jose.Abreu@synopsys.com>,
	Joao Pinto <Joao.Pinto@synopsys.com>,
	Alexandre Torgue <alexandre.torgue@st.com>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"David S. Miller" <davem@davemloft.net>,
	Florian Fainelli <f.fainelli@gmail.com>,
	Maxime Coquelin <mcoquelin.stm32@gmail.com>,
	Giuseppe Cavallaro <peppe.cavallaro@st.com>,
	Jakub Kicinski <kuba@kernel.org>,
	"linux-stm32@st-md-mailman.stormreply.com"
	<linux-stm32@st-md-mailman.stormreply.com>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	Heiner Kallweit <hkallweit1@gmail.com>
Subject: Re: [RFC net-next 6/8] net: phylink: Configure MAC/PCS when link is up without PHY
Date: Tue, 28 Jan 2020 18:08:03 +0000	[thread overview]
Message-ID: <20200128180802.GD25745@shell.armlinux.org.uk> (raw)
In-Reply-To: <20200127145107.GE13647@lunn.ch>

On Mon, Jan 27, 2020 at 03:51:07PM +0100, Andrew Lunn wrote:
> I've also had issues with the DSA links, also being configured to
> 10/Half. That seems to be related to having a phy-mode property in
> device tree. I need to add a fixed-link property to set the correct
> speed. Something is broken here, previously the fixed-link was only
> needed if the speed needed to be lower than the ports maximum. I think
> that is a separate issue i need to dig into, not part of the PCS to
> MAC transfer.

I think I understand what is happening on this one more fully.

When DSA initialises, the DSA and CPU ports are initially configured to
maximum speed via mv88e6xxx_setup_port(), called via mv88e6xxx_setup(),
the .setup method, dsa_switch_setup(), and dsa_tree_setup_switches().

dsa_tree_setup_switches() then moves on to calling dsa_port_setup().
dsa_port_setup() calls dsa_port_link_register_of() for the DSA and CPU
ports, which calls into dsa_port_phylink_register().

That calls phylink_create(), and then attempts to attach a PHY using
phylink_of_phy_connect() - which itself is rather weird - since when
has a DSA or CPU port been allowed to have a PHY in its DT node?

The upshot is, phylink_create() will (and always has) treated a node
without a fixed-link or in-band specification as a "phy" mode link.
Moving on, phylink_start() will be called.

phylink_start() attempts to set an initial configuration.  As there
is no PHY attached, phylink has no idea what parameters to set, but
it needs to set an initial configuration, so it does so.  The result
is, dsa_port_phylink_mac_config() gets called without the speed and
duplex being set as one would expect.

That hasn't changed in phylink yet - so it's a bug that dates back
to the phylink integration into the DSA core, and is a regression
resulting from that.

The reason my patch above appears to solve it is because I'm ignoring
calls to mac_config() with mode == MLO_AN_PHY in various circumstances,
which results in the initial configuration by mv88e6xxx_setup_port()
remaining.

I'm not yet sure what to do about that; and I'm out of time to think
about that anymore today - but I thought I'd post my analysis so far
in the hope that it helps.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel