Re: [PATCH net-next 5/8] net: dpaa2-mac: use resolved link config in mac_link_up() - Russell King

From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
To: Ioana Ciornei <ioana.ciornei@nxp.com>
Cc: Andrew Lunn <andrew@lunn.ch>,
	"linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>,
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
	"linux-stm32@st-md-mailman.stormreply.com"
	<linux-stm32@st-md-mailman.stormreply.com>,
	Florian Fainelli <f.fainelli@gmail.com>,
	Jonathan Corbet <corbet@lwn.net>,
	Michal Simek <michal.simek@xilinx.com>,
	Jose Abreu <joabreu@synopsys.com>,
	Jakub Kicinski <kuba@kernel.org>,
	Mark Lee <Mark-MC.Lee@mediatek.com>,
	Sean Wang <sean.wang@mediatek.com>,
	Alexandre Torgue <alexandre.torgue@st.com>,
	Hauke Mehrtens <hauke@hauke-m.de>,
	Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>,
	"linux-mediatek@lists.infradead.org"
	<linux-mediatek@lists.infradead.org>,
	John Crispin <john@phrozen.org>,
	Matthias Brugger <matthias.bgg@gmail.com>,
	Giuseppe Cavallaro <peppe.cavallaro@st.com>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	Vladimir Oltean <olteanv@gmail.com>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	Vivien Didelot <vivien.didelot@gmail.com>,
	Maxime Coquelin <mcoquelin.stm32@gmail.com>,
	Felix Fietkau <nbd@nbd.name>,
	"David S. Miller" <davem@davemloft.net>,
	Heiner Kallweit <hkallweit1@gmail.com>
Subject: Re: [PATCH net-next 5/8] net: dpaa2-mac: use resolved link config in mac_link_up()
Date: Wed, 26 Feb 2020 10:12:04 +0000	[thread overview]
Message-ID: <20200226101204.GW25745@shell.armlinux.org.uk> (raw)
In-Reply-To: <DB8PR04MB68282F710FB598B977C36F99E0ED0@DB8PR04MB6828.eurprd04.prod.outlook.com>

On Tue, Feb 25, 2020 at 04:36:32PM +0000, Ioana Ciornei wrote:
> > Subject: [PATCH net-next 5/8] net: dpaa2-mac: use resolved link config in
> > mac_link_up()
> > 
> > Convert the DPAA2 ethernet driver to use the finalised link parameters in
> > mac_link_up() rather than the parameters in mac_config(), which are more
> > suited to the needs of the DPAA2 MC firmware than those available via
> > mac_config().
> > 
> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> 
> Tested-by: Ioana Ciornei <ioana.ciornei@nxp.com>

Thanks.

> > +
> > +		/* This is lossy; the firmware really should take the pause
> > +		 * enablement status rather than pause/asym pause status.
> > +		 */
> 
> In what sense it's lossy? I cannot see how information can be lost by translating the rx/tx pause state to pause/asym.
> If it's just about the unnecessary double translation, then I agree.. this could have been done in an easier manner.

If you're just translating rx/tx to pause/asym and then doing the
reverse, then it isn't lossy, but if the firmware is resolving
pause/asym according to the table in IEEE 802.3, then it will be
lossy.

If the firmware doesn't interpret the bits, then why not do the
sensible thing and just pass the enablement status rather than
trying to confusingly encode it back to pause/asym?

-- 
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