diff mbox series

Energy Efficient Ethernet on MT7531 switch

Message ID 5f1f8827-730e-4f36-bc0a-fec6f5558e93@arinc9.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series Energy Efficient Ethernet on MT7531 switch | expand

Checks

Context Check Description
netdev/tree_selection success Guessing tree name failed - patch did not apply

Commit Message

Arınç ÜNAL March 14, 2024, 12:57 p.m. UTC 
Hi Frank.

Do you have a board with an external PHY that supports EEE connected to an
MT7531 switch? I've stumbled across an option on the trap register of
MT7531 that claims that EEE is disabled switch-wide by default after reset.

I'm specifically asking for an external PHY because the MT7531 switch PHYs
don't support EEE yet. But the MT753X DSA subdriver claims to support EEE,
so the remaining option is external PHYs.

It'd be great if you can test with and without this diff [1] and see if you
see EEE supported on ethtool on a computer connected to the external PHY.

Example output on the computer side:

$ sudo ethtool --show-eee eno1
EEE settings for eno1:
	EEE status: enabled - active
	Tx LPI: 17 (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

I'm also CC'ing Daniel and the netdev mailing list, if someone else would
like to chime in.

[1]

Thanks a lot!
Arınç

Comments

Daniel Golle March 14, 2024, 2:58 p.m. UTC | #1 
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 stumbled across an option on the trap register of
> MT7531 that claims that EEE is disabled switch-wide by default after reset.
> 
> I'm specifically asking for an external PHY because the MT7531 switch PHYs
> don't support EEE yet. But the MT753X DSA subdriver claims to support EEE,
> so the remaining option is external PHYs.
> 
> It'd be great if you can test with and without this diff [1] and see if you
> see EEE supported on ethtool on a computer connected to the external PHY.
> 
> Example output on the computer side:
> 
> $ sudo ethtool --show-eee eno1
> EEE settings for eno1:
> 	EEE status: enabled - active
> 	Tx LPI: 17 (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
> 
> I'm also CC'ing Daniel and the netdev mailing list, if someone else would
> like to chime in.
> 
> [1]
> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
> index b347d8ab2541..4ef3948d310d 100644
> --- a/drivers/net/dsa/mt7530.c
> +++ b/drivers/net/dsa/mt7530.c
> @@ -2499,6 +2499,8 @@ mt7531_setup(struct dsa_switch *ds)
>  	mt7531_ind_c45_phy_write(priv, MT753X_CTRL_PHY_ADDR, MDIO_MMD_VEND2,
>  				 CORE_PLL_GROUP4, val);
> +	mt7530_rmw(priv, MT7530_MHWTRAP, CHG_STRAP | EEE_DIS, CHG_STRAP);
> +
>  	mt7531_setup_common(ds);
>  	/* Setup VLAN ID 0 for VLAN-unaware bridges */
> diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
> index 3c3e7ae0e09b..1b3e81f6c90e 100644
> --- a/drivers/net/dsa/mt7530.h
> +++ b/drivers/net/dsa/mt7530.h
> @@ -299,11 +299,15 @@ enum mt7530_vlan_port_acc_frm {
>  #define  MT7531_FORCE_DPX		BIT(29)
>  #define  MT7531_FORCE_RX_FC		BIT(28)
>  #define  MT7531_FORCE_TX_FC		BIT(27)
> +#define  MT7531_FORCE_EEE100		BIT(26)
> +#define  MT7531_FORCE_EEE1G		BIT(25)
>  #define  MT7531_FORCE_MODE		(MT7531_FORCE_LNK | \
>  					 MT7531_FORCE_SPD | \
>  					 MT7531_FORCE_DPX | \
>  					 MT7531_FORCE_RX_FC | \
> -					 MT7531_FORCE_TX_FC)
> +					 MT7531_FORCE_TX_FC | \
> +					 MT7531_FORCE_EEE100 | \
> +					 MT7531_FORCE_EEE1G)
>  #define  PMCR_LINK_SETTINGS_MASK	(PMCR_TX_EN | PMCR_FORCE_SPEED_1000 | \
>  					 PMCR_RX_EN | PMCR_FORCE_SPEED_100 | \
>  					 PMCR_TX_FC_EN | PMCR_RX_FC_EN | \
> @@ -457,6 +461,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
> 
> Thanks a lot!
> Arınç
>
Heiner Kallweit March 14, 2024, 5:01 p.m. UTC | #2 
On 14.03.2024 15: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.
> 

With linux-next / net-next also 2.5G EEE should be configurable for these
PHY's. Or what are you missing in the Realtek PHY driver?

> 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 stumbled across an option on the trap register of
>> MT7531 that claims that EEE is disabled switch-wide by default after reset.
>>
>> I'm specifically asking for an external PHY because the MT7531 switch PHYs
>> don't support EEE yet. But the MT753X DSA subdriver claims to support EEE,
>> so the remaining option is external PHYs.
>>
>> It'd be great if you can test with and without this diff [1] and see if you
>> see EEE supported on ethtool on a computer connected to the external PHY.
>>
>> Example output on the computer side:
>>
>> $ sudo ethtool --show-eee eno1
>> EEE settings for eno1:
>> 	EEE status: enabled - active
>> 	Tx LPI: 17 (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
>>
>> I'm also CC'ing Daniel and the netdev mailing list, if someone else would
>> like to chime in.
>>
>> [1]
>> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
>> index b347d8ab2541..4ef3948d310d 100644
>> --- a/drivers/net/dsa/mt7530.c
>> +++ b/drivers/net/dsa/mt7530.c
>> @@ -2499,6 +2499,8 @@ mt7531_setup(struct dsa_switch *ds)
>>  	mt7531_ind_c45_phy_write(priv, MT753X_CTRL_PHY_ADDR, MDIO_MMD_VEND2,
>>  				 CORE_PLL_GROUP4, val);
>> +	mt7530_rmw(priv, MT7530_MHWTRAP, CHG_STRAP | EEE_DIS, CHG_STRAP);
>> +
>>  	mt7531_setup_common(ds);
>>  	/* Setup VLAN ID 0 for VLAN-unaware bridges */
>> diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
>> index 3c3e7ae0e09b..1b3e81f6c90e 100644
>> --- a/drivers/net/dsa/mt7530.h
>> +++ b/drivers/net/dsa/mt7530.h
>> @@ -299,11 +299,15 @@ enum mt7530_vlan_port_acc_frm {
>>  #define  MT7531_FORCE_DPX		BIT(29)
>>  #define  MT7531_FORCE_RX_FC		BIT(28)
>>  #define  MT7531_FORCE_TX_FC		BIT(27)
>> +#define  MT7531_FORCE_EEE100		BIT(26)
>> +#define  MT7531_FORCE_EEE1G		BIT(25)
>>  #define  MT7531_FORCE_MODE		(MT7531_FORCE_LNK | \
>>  					 MT7531_FORCE_SPD | \
>>  					 MT7531_FORCE_DPX | \
>>  					 MT7531_FORCE_RX_FC | \
>> -					 MT7531_FORCE_TX_FC)
>> +					 MT7531_FORCE_TX_FC | \
>> +					 MT7531_FORCE_EEE100 | \
>> +					 MT7531_FORCE_EEE1G)
>>  #define  PMCR_LINK_SETTINGS_MASK	(PMCR_TX_EN | PMCR_FORCE_SPEED_1000 | \
>>  					 PMCR_RX_EN | PMCR_FORCE_SPEED_100 | \
>>  					 PMCR_TX_FC_EN | PMCR_RX_FC_EN | \
>> @@ -457,6 +461,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
>>
>> Thanks a lot!
>> Arınç
>>
>
Arınç ÜNAL March 17, 2024, 12:38 p.m. UTC | #3 
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ç
diff mbox series

Patch

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index b347d8ab2541..4ef3948d310d 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -2499,6 +2499,8 @@  mt7531_setup(struct dsa_switch *ds)
  	mt7531_ind_c45_phy_write(priv, MT753X_CTRL_PHY_ADDR, MDIO_MMD_VEND2,
  				 CORE_PLL_GROUP4, val);
  
+	mt7530_rmw(priv, MT7530_MHWTRAP, CHG_STRAP | EEE_DIS, CHG_STRAP);
+
  	mt7531_setup_common(ds);
  
  	/* Setup VLAN ID 0 for VLAN-unaware bridges */
diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
index 3c3e7ae0e09b..1b3e81f6c90e 100644
--- a/drivers/net/dsa/mt7530.h
+++ b/drivers/net/dsa/mt7530.h
@@ -299,11 +299,15 @@  enum mt7530_vlan_port_acc_frm {
  #define  MT7531_FORCE_DPX		BIT(29)
  #define  MT7531_FORCE_RX_FC		BIT(28)
  #define  MT7531_FORCE_TX_FC		BIT(27)
+#define  MT7531_FORCE_EEE100		BIT(26)
+#define  MT7531_FORCE_EEE1G		BIT(25)
  #define  MT7531_FORCE_MODE		(MT7531_FORCE_LNK | \
  					 MT7531_FORCE_SPD | \
  					 MT7531_FORCE_DPX | \
  					 MT7531_FORCE_RX_FC | \
-					 MT7531_FORCE_TX_FC)
+					 MT7531_FORCE_TX_FC | \
+					 MT7531_FORCE_EEE100 | \
+					 MT7531_FORCE_EEE1G)
  #define  PMCR_LINK_SETTINGS_MASK	(PMCR_TX_EN | PMCR_FORCE_SPEED_1000 | \
  					 PMCR_RX_EN | PMCR_FORCE_SPEED_100 | \
  					 PMCR_TX_FC_EN | PMCR_RX_FC_EN | \
@@ -457,6 +461,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