[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <2ef6fb53-eee6-4c19-bb58-284ba8aa34f9@arinc9.com>
Date: Sun, 17 Mar 2024 15:38:10 +0300
From: Arınç ÜNAL <arinc.unal@...nc9.com>
To: Daniel Golle <daniel@...rotopia.org>
Cc: Frank Wunderlich <frank-w@...lic-files.de>,
netdev <netdev@...r.kernel.org>
Subject: Re: Energy Efficient Ethernet on MT7531 switch
On 14.03.2024 17:58, Daniel Golle wrote:
> Hi,
>
> On Thu, Mar 14, 2024 at 03:57:09PM +0300, Arınç ÜNAL wrote:
>> Hi Frank.
>>
>> Do you have a board with an external PHY that supports EEE connected to an
>> MT7531 switch?
>
> Good to hear you are working on supporting EEE -- something which has
> been neglected for too long imho.
>
> I got a bunch of such boards, all of them with different generations
> of RealTek RTL8226 or RTL8221 2.5G PHY which in theory supports EEE
> but the PHY driver in Linux at this point does not support EEE.
>
> However, as one of the SFP cages of the BPi-R3 is connected to the on-board
> MT7531 switch port 5 this would provide the option to basically test EEE
> with practically every PHY you could find inside an RJ-45 SFP module
> (spoiler: you will mostly find Marvell 88E1111, and I don't see support for
> EEE in neither the datasheet nor the responsible sub-driver in Linux).
>
> So looks like we will have to implement support for EEE for either
> RealTek's RTL8221B or the built-in PHYs of any of the MT753x, MT7621
> or MT7988 switch first.
I've got news. I believe I've made EEE work with MT7531 switch PHYs and
MACs.
I stated that EEE wasn't supported yet on MT7531 (and MT7530) switch PHYs
because EEE advertisement on switch PHYs is disabled on mediatek-ge [1].
In reality, the EEE advertisement disablement goes away once the DSA
subdriver starts controlling the switch and its PHYs. This goes for MT7530
too.
So I don't see any reason for that code on mediatek-ge to exist, I'd like
to remove it. I think there's a possible race condition with mediatek-ge
kicking in after the DSA subdriver so I'd like to submit the removal of
disabling EEE advertisement on mediatek-ge to the net tree. What do you
think Daniel?
EEE on MT7530 works without any changes to the current linux-next tree.
There's another step needed to enable EEE on MT7531 switch MACs. With this
diff [2], EEE is now enabled on the MAC side. So EEE starts working.
Testing EEE on MT7988 SoC PHYs is up to you, I don't have the hardware.
Without the diff:
# ethtool --show-eee lan0
EEE settings for lan0:
EEE status: not supported
With the diff:
# ethtool --show-eee lan0
EEE settings for lan0:
EEE status: enabled - active
Tx LPI: 30 (us)
Supported EEE link modes: 100baseT/Full
1000baseT/Full
Advertised EEE link modes: 100baseT/Full
1000baseT/Full
Link partner advertised EEE link modes: 100baseT/Full
1000baseT/Full
To confirm that the EEE advertisement disablement goes away once the DSA
subdriver starts controlling the switch and its PHYs, I've written this
debugfs code [3] to run code from userspace on demand. Testing disabling
EEE on the PHY, disable_eee1 pertains to lan0:
# mount -t debugfs none /sys/kernel/debug
# echo go > /sys/kernel/debug/mt7530/disable_eee1
# ethtool --show-eee lan0
EEE settings for lan0:
EEE status: disabled
Tx LPI: 30 (us)
Supported EEE link modes: 100baseT/Full
1000baseT/Full
Advertised EEE link modes: Not reported
Link partner advertised EEE link modes: 100baseT/Full
1000baseT/Full
# ethtool --set-eee lan0 eee on
[ 1781.317488] mt7530-mdio mdio-bus:1f lan0: Link is Down
[ 1787.308087] mt7530-mdio mdio-bus:1f lan0: Link is Up - 1Gbps/Full - flow control rx/tx
# ethtool --show-eee lan0
EEE settings for lan0:
EEE status: enabled - active
Tx LPI: 30 (us)
Supported EEE link modes: 100baseT/Full
1000baseT/Full
Advertised EEE link modes: 100baseT/Full
1000baseT/Full
Link partner advertised EEE link modes: 100baseT/Full
1000baseT/Full
[1] https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/tree/drivers/net/phy/mediatek-ge.c?id=98c485eaf509bc0e2a85f9b58d17cd501f274c4e#n26
[2]
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index e2a8ea337ab0..dfa5e610a6ee 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -2554,6 +2554,13 @@ 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);
+
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
[3]
diff --git a/drivers/net/phy/mediatek-ge.c b/drivers/net/phy/mediatek-ge.c
index a493ae01b267..e8822adbf3bc 100644
--- a/drivers/net/phy/mediatek-ge.c
+++ b/drivers/net/phy/mediatek-ge.c
@@ -1,5 +1,6 @@
// SPDX-License-Identifier: GPL-2.0+
#include <linux/bitfield.h>
+#include <linux/debugfs.h>
#include <linux/module.h>
#include <linux/phy.h>
@@ -11,6 +12,53 @@
#define MTK_PHY_PAGE_EXTENDED_2A30 0x2a30
#define MTK_PHY_PAGE_EXTENDED_52B5 0x52b5
+static int phy_number = 0;
+
+static ssize_t disable_eee_write(struct file *filp, const char __user *buffer,
+ size_t count, loff_t *ppos)
+{
+ struct phy_device *phydev = filp->private_data;
+
+ phy_write_mmd(phydev, MDIO_MMD_AN, MDIO_AN_EEE_ADV, 0);
+
+ return count;
+}
+
+static const struct file_operations disable_eee_fops = {
+ .owner = THIS_MODULE,
+ .open = simple_open,
+ .write = disable_eee_write,
+};
+
+static void mtk_gephy_debugfs_init(struct phy_device *phydev)
+{
+ struct dentry *dir;
+
+ dir = debugfs_lookup("mt7530", 0);
+ if (dir == NULL)
+ dir = debugfs_create_dir("mt7530", 0);
+
+ if (phy_number == 0) {
+ debugfs_create_file("disable_eee0", 0644, dir, phydev,
+ &disable_eee_fops);
+ } else if (phy_number == 1) {
+ debugfs_create_file("disable_eee1", 0644, dir, phydev,
+ &disable_eee_fops);
+ } else if (phy_number == 2) {
+ debugfs_create_file("disable_eee2", 0644, dir, phydev,
+ &disable_eee_fops);
+ } else if (phy_number == 3) {
+ debugfs_create_file("disable_eee3", 0644, dir, phydev,
+ &disable_eee_fops);
+ } else if (phy_number == 4) {
+ debugfs_create_file("disable_eee4", 0644, dir, phydev,
+ &disable_eee_fops);
+ }
+
+ if (phy_number < 4)
+ phy_number++;
+}
+
static int mtk_gephy_read_page(struct phy_device *phydev)
{
return __phy_read(phydev, MTK_EXT_PAGE_ACCESS);
@@ -41,6 +89,8 @@ static void mtk_gephy_config_init(struct phy_device *phydev)
/* Disable mcc */
phy_write_mmd(phydev, MDIO_MMD_VEND1, 0xa6, 0x300);
+
+ mtk_gephy_debugfs_init(phydev);
}
static int mt7530_phy_config_init(struct phy_device *phydev)
Arınç
Powered by blists - more mailing lists