Messages in this thread | | | Date | Wed, 10 Apr 2024 19:52:24 +0100 | From | Daniel Golle <> | Subject | Re: [PATCH net v3] net: dsa: mt7530: fix enabling EEE on MT7531 switch on all boards |
| |
On Tue, Apr 09, 2024 at 01:58:56PM +0200, Paolo Abeni wrote: > On Mon, 2024-04-08 at 10:08 +0300, Arınç ÜNAL via B4 Relay wrote: > > From: Arınç ÜNAL <arinc.unal@arinc9.com> > > > > The commit 40b5d2f15c09 ("net: dsa: mt7530: Add support for EEE features") > > brought EEE support but did not enable EEE on MT7531 MACs. EEE is > > enabled on MT7531 switch MACs by pulling the LAN2LED0 pin low on the board > > (bootstrapping), unsetting the EEE_DIS bit on the trap register, or setting > > the internal EEE switch bit on the CORE_PLL_GROUP4 register. Thanks to > > SkyLake Huang (黃啟澤) from MediaTek for providing information on the > > internal EEE switch bit. > > > > There are existing boards that were not designed to pull the pin low. > > Because of that, the EEE status currently depends on the board design. > > > > The EEE_DIS bit on the trap pertains to the LAN2LED0 pin which is usually > > used to control an LED. Once the bit is unset, the pin will be low. That > > will make the active low LED turn on. The pin is controlled by the switch > > PHY. It seems that the PHY controls the pin in the way that it inverts the > > pin state. That means depending on the wiring of the LED connected to > > LAN2LED0 on the board, the LED may be on without an active link. > > > > To not cause this unwanted behaviour whilst enabling EEE on all boards, set > > the internal EEE switch bit on the CORE_PLL_GROUP4 register. > > > > My testing on MT7531 shows a certain amount of traffic loss when EEE is > > enabled. That said, I haven't come across a board that enables EEE. So > > enable EEE on the switch MACs but disable EEE advertisement on the switch > > PHYs. This way, we don't change the behaviour of the majority of the boards > > that have this switch. The mediatek-ge PHY driver already disables EEE > > advertisement on the switch PHYs but my testing shows that it is somehow > > enabled afterwards. Disabling EEE advertisement before the PHY driver > > initialises keeps it off. > > > > With this change, EEE can now be enabled using ethtool. > > > > Fixes: 40b5d2f15c09 ("net: dsa: mt7530: Add support for EEE features") > > Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com> > > Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com> > > --- > > Here's some information for the record. EEE could not be enabled on MT7531 > > on most boards using ethtool before this. On MT7988 SoC switch, EEE is > > disabled by default but can be turned on normally using ethtool. EEE is > > enabled by default on MT7530 and there's no need to make changes on the DSA > > subdriver for it. > > --- > > Changes in v3: > > - Remove patch 2, it was revealed that it doesn't fix a bug. > > - Patch 1 > > - Use the internal EEE switch bit provided by SkyLake Huang (黃啟澤). It > > is a better method compared to unsetting the EEE_DIS bit of the trap as > > the latter method causes unwanted behaviour on the LED connected to the > > pin that pertains to the EEE_DIS bit. > > Since this leverages something relatively obscure, it would be great if > someone in the CC list could independently test it. Let's wait a bit > more.
I've excessively tested this patch on MT7531 today, and reviewed it today and yesterday. I've also picked it as downstream patch[1] to OpenWrt, so an even wider audience will have the pleasure of working EEE on those switch ICs and in-SoC switches.
Tested-by: Daniel Golle <daniel@makrotopia.org> Reviewed-by: Daniel Golle <daniel@makrotopia.org>
[1]: https://git.openwrt.org/?p=openwrt/openwrt.git;a=commit;h=98f9154316fe8371c709bd11ae8f263e22075ec6
| |