Re: [PATCH 0/3] Fix EEE support for MT7531 and MT7988 SoC switch

From: Daniel Golle <daniel@makrotopia.org>
To: Andrew Lunn <andrew@lunn.ch>
Cc: "Arınç ÜNAL" <arinc.unal@arinc9.com>,
	"DENG Qingfang" <dqfext@gmail.com>,
	"Sean Wang" <sean.wang@mediatek.com>,
	"Florian Fainelli" <f.fainelli@gmail.com>,
	"Vladimir Oltean" <olteanv@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	"Eric Dumazet" <edumazet@google.com>,
	"Jakub Kicinski" <kuba@kernel.org>,
	"Paolo Abeni" <pabeni@redhat.com>,
	"Matthias Brugger" <matthias.bgg@gmail.com>,
	"AngeloGioacchino Del Regno"
	<angelogioacchino.delregno@collabora.com>,
	"René van Dorst" <opensource@vdorst.com>,
	"Russell King" <linux@armlinux.org.uk>,
	"SkyLake Huang" <SkyLake.Huang@mediatek.com>,
	"Heiner Kallweit" <hkallweit1@gmail.com>,
	"Bartel Eerdekens" <bartel.eerdekens@constell8.be>,
	mithat.guner@xeront.com, erkin.bozoglu@xeront.com,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-mediatek@lists.infradead.org
Subject: Re: [PATCH 0/3] Fix EEE support for MT7531 and MT7988 SoC switch
Date: Tue, 19 Mar 2024 20:26:55 +0000	[thread overview]
Message-ID: <Zfn1DxkEa3u-f7l2@makrotopia.org> (raw)
In-Reply-To: <6cb585f6-6da8-45a2-a28b-2fb556f95672@lunn.ch>

On Tue, Mar 19, 2024 at 08:38:03PM +0100, Andrew Lunn wrote:
> > I would argue that EEE advertisement on the PHY should be enabled by
> > default.
> 
> That is an open question at the moment. For some use cases, it can add
> extra delay and jitter which can cause problems. I've heard people
> doing PTP don't like EEE for example.

MediaTek consumer-grade hardware doesn't support PTP and hence that
quite certainly won't ever be an issue with all switch ICs supported
by the mt7530 driver.

I'd rather first change the (configuration) default in OpenWrt (which
is arguable the way most people are using this hardware), also because
that will be more visible/obvious for users. Or even just make EEE
configurable in the LuCI web-UI as a first step so users start playing
with it.

After all, I also have a hard time imagining that MediaTek disabled
EEE in their downstream driver for no reason:

https://git01.mediatek.com/plugins/gitiles/openwrt/feeds/mtk-openwrt-feeds/+/24091177a18ba7f2dd8d928a8f5b27b14df46b16

> 
> The current phylib core code leaves the PHY advertisement whatever its
> reset default is. So we leave it to the manufacture to decide if it
> should be enabled or disabled by default. It is policy, so it should
> really be down to user space to configure EEE how it wants it.

I very much agree with that policy, changing the default definitely
feels like something which could affect quite a lot of people and
should be done very carefully, if at all.