Re: [PATCH RFC net-next 1/5] net: dsa: mt7530: Convert to PHYLINK API

From: Andrew Lunn <andrew@lunn.ch>
To: Daniel Santos <daniel.santos@pobox.com>
Cc: "Russell King - ARM Linux admin" <linux@armlinux.org.uk>,
	"René van Dorst" <opensource@vdorst.com>,
	sean.wang@mediatek.com, f.fainelli@gmail.com,
	davem@davemloft.net, matthias.bgg@gmail.com,
	vivien.didelot@gmail.com, frank-w@public-files.de,
	netdev@vger.kernel.org, linux-mediatek@lists.infradead.org,
	linux-mips@vger.kernel.org
Subject: Re: [PATCH RFC net-next 1/5] net: dsa: mt7530: Convert to PHYLINK API
Date: Tue, 25 Jun 2019 22:41:48 +0200	[thread overview]
Message-ID: <20190625204148.GB27733@lunn.ch> (raw)
In-Reply-To: <4fc51dc4-0eec-30d7-86d1-3404819cf6fe@pobox.com>

On Tue, Jun 25, 2019 at 02:27:55PM -0500, Daniel Santos wrote:
> On 6/25/19 2:02 PM, Andrew Lunn wrote:
> >> But will there still be a mechanism to ignore link partner's advertising
> >> and force these parameters?
> > >From man 1 ethtool:
> >
> >        -a --show-pause
> >               Queries the specified Ethernet device for pause parameter information.
> >
> >        -A --pause
> >               Changes the pause parameters of the specified Ethernet device.
> >
> >            autoneg on|off
> >                   Specifies whether pause autonegotiation should be enabled.
> >
> >            rx on|off
> >                   Specifies whether RX pause should be enabled.
> >
> >            tx on|off
> >                   Specifies whether TX pause should be enabled.
> >
> > You need to check the driver to see if it actually implements this
> > ethtool call, but that is how it should be configured.
> >
> > 	Andrew
> >
> Thank you Andrew,
> 
> So in this context, my question is the difference between "enabling" and
> "forcing".  Here's that register for the mt7620 (which has an mt7530 on
> its die): https://imgur.com/a/pTk0668  I believe this is also what René
> is seeking clarity on?

Lets start with normal operation. If the MAC supports pause or asym
pause, it calls phy_support_sym_pause() or phy_support_asym_pause().
phylib will then configure the PHY to advertise pause as appropriate.
Once auto-neg has completed, the results of the negotiation are set in
phydev. So phdev->pause and phydev->asym_pause. The MAC callback is
then used to tell the MAC about the autoneg results. The MAC should be
programmed using the values in phdev->pause and phydev->asym_pause.

For ethtool, the MAC driver needs to implement .get_pauseparam and
.set_pauseparam. The set_pauseparam needs to validate the settings,
using phy_validate_pause(). If valid, phy_set_asym_pause() is used to
tell the PHY about the new configuration. This will trigger a new
auto-neg if auto-neg is enabled, and the results will be passed back
in the usual way. If auto-neg is disabled, or pause auto-neg is
disabled, the MAC should configure pause directly based on the
settings passed.

Looking at the data sheet page, you want FORCE_MODE_Pn set. You never
want the MAC directly talking to the PHY. Bad things will happen.
Then use FORCE_RX_FC_Pn and FORCE_TX_Pn to reflect phydev->pause and
phydev->asym_pause.

The same idea applies when using phylink.

    Andrew