| Message ID | 20240318-for-net-mt7530-fix-eee-for-mt7531-mt7988-v1-0-3f17226344e8@arinc9.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Fix EEE support for MT7531 and MT7988 SoC switch | expand |
On 3/18/2024 12:46 AM, Arınç ÜNAL via B4 Relay wrote: > From: Arınç ÜNAL <arinc.unal@arinc9.com> > > There's no need to disable Energy-Efficient Ethernet (EEE) advertisement on > the MT7530 and MT7531 switch PHYs. EEE works fine on MT7530 and MT7531 > switch PHYs. Remove the code where EEE advertisement is disabled. > > This is a bugfix because there's a possible race condition where the > mediatek-ge driver would kick in after the MT7530 DSA subdriver which would > have EEE disabled until manually enabled. > > Fixes: e40d2cca0189 ("net: phy: add MediaTek Gigabit Ethernet PHY driver") > Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com> Could we get Sean to chime in, maybe there was a valid reason to disable EEE advertisement in the first place?
On 3/18/2024 12:46 AM, 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 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. > > 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") > Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com> Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
On 18.03.2024 10:46, Arınç ÜNAL via B4 Relay wrote: > Hi. > > This patch series fixes EEE support for MT7531 and the switch on the MT7988 > SoC. EEE did not work on MT7531 on most boards before this, it is unclear > what's the status on MT7988 SoC switch as I don't have the hardware. > > Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com> I see the state of this patch series is deferred on patchwork. I see that I forgot to delegate this to the net tree. As I don't see any objections in this series, I'll send v2 with it tomorrow. Arınç
On Wed, Mar 20, 2024 at 11:10:19AM +0300, Arınç ÜNAL wrote: > On 18.03.2024 10:46, Arınç ÜNAL via B4 Relay wrote: > > Hi. > > > > This patch series fixes EEE support for MT7531 and the switch on the MT7988 > > SoC. EEE did not work on MT7531 on most boards before this, it is unclear > > what's the status on MT7988 SoC switch as I don't have the hardware. > > > > Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com> > > I see the state of this patch series is deferred on patchwork. I see that I > forgot to delegate this to the net tree. As I don't see any objections in > this series, I'll send v2 with it tomorrow. Sorry, but imho it should go to net-next, and you might have overlook it but there have been some concerns. For sure it should not go to net tree as you are enabling a new feature and not fixing anything which is broken. EEE on MT7988 works fine as of now (see my reply), EEE on MT7531 was supposedly intentionally disabled for reasons we should ask MTK people about.
On 20.03.2024 14:08, Daniel Golle wrote: > On Wed, Mar 20, 2024 at 11:10:19AM +0300, Arınç ÜNAL wrote: >> On 18.03.2024 10:46, Arınç ÜNAL via B4 Relay wrote: >>> Hi. >>> >>> This patch series fixes EEE support for MT7531 and the switch on the MT7988 >>> SoC. EEE did not work on MT7531 on most boards before this, it is unclear >>> what's the status on MT7988 SoC switch as I don't have the hardware. >>> >>> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com> >> >> I see the state of this patch series is deferred on patchwork. I see that I >> forgot to delegate this to the net tree. As I don't see any objections in >> this series, I'll send v2 with it tomorrow. > > Sorry, but imho it should go to net-next, and you might have overlook > it but there have been some concerns. I don't believe I overlooked anything. I've come to this conclusion after reading every response in this thread. > > For sure it should not go to net tree as you are enabling a new feature > and not fixing anything which is broken. EEE on MT7988 works fine as EEE support exists since the commit which I've mentioned on my patches here. I am fixing it. I thought I had explained this clearly on the patches. > of now (see my reply), EEE on MT7531 was supposedly intentionally > disabled for reasons we should ask MTK people about. Are you talking about the EEE_DIS bit on the trap? There's no default setting there, it's up to the board vendor to enable/disable EEE by pulling the affine pin low or high. So there's no intentional disabling by MediaTek there. I see no need to ask the corporate regarding this. This patch series does not in any way enable EEE on the switch PHYs and MACs when it's disabled by default. Arınç
Hi. This patch series fixes EEE support for MT7531 and the switch on the MT7988 SoC. EEE did not work on MT7531 on most boards before this, it is unclear what's the status on MT7988 SoC switch as I don't have the hardware. Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com> --- Arınç ÜNAL (3): net: dsa: mt7530: fix enabling EEE on MT7531 switch on all boards net: dsa: mt7530: fix disabling EEE on failure on MT7531 and MT7988 net: phy: mediatek-ge: do not disable EEE advertisement drivers/net/dsa/mt7530.c | 7 +++++++ drivers/net/dsa/mt7530.h | 7 ++++++- drivers/net/phy/mediatek-ge.c | 3 --- 3 files changed, 13 insertions(+), 4 deletions(-) --- base-commit: ea80e3ed09ab2c2b75724faf5484721753e92c31 change-id: 20240317-for-net-mt7530-fix-eee-for-mt7531-mt7988-a5c5453cc0e8 Best regards,