| Message ID | 20240321-for-net-mt7530-fix-eee-for-mt7531-mt7988-v2-1-9af9d5041bfe@arinc9.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fix EEE support for MT7531 and MT7988 SoC switch | expand |
On Thu, 2024-03-21 at 19:29 +0300, Arınç ÜNAL via B4 Relay wrote: > > External email : Please do not click links or open attachments until > you have verified the sender or the content. > 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 switch MACs. EEE > is > enabled on MT7531 switch MACs either by pulling the LAN2LED0 pin low > on the > board (bootstrapping), or unsetting the EEE_DIS bit on the trap > register. > > There are existing boards that were not designed to pull the pin low. > Therefore, unset the EEE_DIS bit on the trap register. > > Unlike MT7530, the modifiable trap register won't be populated > identical to > the trap status register after reset. Therefore, read from the trap > status > register, modify the bits, then write to the modifiable trap > 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. > > With this change, EEE can now be enabled using ethtool. > > The disable EEE 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. > > 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> > --- > drivers/net/dsa/mt7530.c | 14 ++++++++++++++ > drivers/net/dsa/mt7530.h | 1 + > 2 files changed, 15 insertions(+) > > diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c > index 678b51f9cea6..6aa99b590329 100644 > --- a/drivers/net/dsa/mt7530.c > +++ b/drivers/net/dsa/mt7530.c > @@ -2458,6 +2458,20 @@ mt7531_setup(struct dsa_switch *ds) > /* Reset the switch through internal reset */ > mt7530_write(priv, MT7530_SYS_CTRL, SYS_CTRL_SW_RST | > SYS_CTRL_REG_RST); > > +/* Allow modifying the trap and enable Energy-Efficient Ethernet > (EEE). > + */ > +val = mt7530_read(priv, MT7531_HWTRAP); > +val |= CHG_STRAP; > +val &= ~EEE_DIS; > +mt7530_write(priv, MT7530_MHWTRAP, val); You may try to set bit 6 of CL45 register dev=0x1f, reg=0x403(PLL_GROUP_CTL_REG), which is an internal EEE switch called RG_SYSPLL_DMY2. Read it out first with "switch command", if you have it: root@OpenWrt:/# switch phy cl45 r 0 0x1f 0x403 Phy read dev_num=0x1f, reg=0x403, value=0x1091 And then set bit 6: root@OpenWrt:/# $ switch phy cl45 w 0 0x1f 0x403 0x10d1 root@OpenWrt:/# ethtool --set-eee lan1 eee on tx-lpi on tx-timer 0x1e advertise 0x28 root@OpenWrt:/# switch phy cl45 r 0 0x7 0x3c Phy read dev_num=0x7, reg=0x3c, value=0x6 Then you should be able to enable EEE via ethtool without clearing EEE_DIS bit of MT7530_HWTRAP. If above CL45 command is good for you, I think this can be moved to mtk_gephy_config_init() @ mediatek-ge.c, which will lead to cleaner implementation. > + > +/* Disable EEE advertisement on the switch PHYs. */ > +for (i = MT753X_CTRL_PHY_ADDR; > + i < MT753X_CTRL_PHY_ADDR + MT7530_NUM_PHYS; i++) { > +mt7531_ind_c45_phy_write(priv, i, MDIO_MMD_AN, MDIO_AN_EEE_ADV, > + 0); > +} Sorry, I still can't figure out why this is needed since we disable MDIO_AN_EEE_ADV in mediatek-ge.c after reading previous threads. Would you please provide something else (like dmesg log?) to show that settings in mtk_gephy_config_init() may fail? > + > if (!priv->p5_sgmii) { > mt7531_pll_setup(priv); > } else { > diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h > index a71166e0a7fc..509ed5362236 100644 > --- a/drivers/net/dsa/mt7530.h > +++ b/drivers/net/dsa/mt7530.h > @@ -457,6 +457,7 @@ enum mt7531_clk_skew { > #define XTAL_FSEL_MBIT(7) > #define PHY_ENBIT(6) > #define CHG_STRAPBIT(8) > +#define EEE_DISBIT(4) > > /* Register for hw trap modification */ > #define MT7530_MHWTRAP0x7804 > > -- > 2.40.1 > >
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c index 678b51f9cea6..6aa99b590329 100644 --- a/drivers/net/dsa/mt7530.c +++ b/drivers/net/dsa/mt7530.c @@ -2458,6 +2458,20 @@ mt7531_setup(struct dsa_switch *ds) /* Reset the switch through internal reset */ mt7530_write(priv, MT7530_SYS_CTRL, SYS_CTRL_SW_RST | SYS_CTRL_REG_RST); + /* Allow modifying the trap and enable Energy-Efficient Ethernet (EEE). + */ + val = mt7530_read(priv, MT7531_HWTRAP); + val |= CHG_STRAP; + val &= ~EEE_DIS; + mt7530_write(priv, MT7530_MHWTRAP, val); + + /* Disable EEE advertisement on the switch PHYs. */ + for (i = MT753X_CTRL_PHY_ADDR; + i < MT753X_CTRL_PHY_ADDR + MT7530_NUM_PHYS; i++) { + mt7531_ind_c45_phy_write(priv, i, MDIO_MMD_AN, MDIO_AN_EEE_ADV, + 0); + } + if (!priv->p5_sgmii) { mt7531_pll_setup(priv); } else { diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h index a71166e0a7fc..509ed5362236 100644 --- a/drivers/net/dsa/mt7530.h +++ b/drivers/net/dsa/mt7530.h @@ -457,6 +457,7 @@ enum mt7531_clk_skew { #define XTAL_FSEL_M BIT(7) #define PHY_EN BIT(6) #define CHG_STRAP BIT(8) +#define EEE_DIS BIT(4) /* Register for hw trap modification */ #define MT7530_MHWTRAP 0x7804