Re: [PATCH net v2 1/2] net: dsa: mt7530: fix enabling EEE on MT7531 switch on all boards

From: Arınç ÜNAL
Date: Thu Mar 28 2024 - 09:58:18 EST

Next message: Jonathan Cameron: "Re: [PATCH RFC 4/7] iio: addac: ad74115: Use devm_regulator_get_enable_get_voltage()"
Previous message: Mathieu Desnoyers: "Re: [PATCH 2/2] ARC: mm: fix new code about cache aliasing"
In reply to: SkyLake Huang (黃啟澤): "Re: [PATCH net v2 1/2] net: dsa: mt7530: fix enabling EEE on MT7531 switch on all boards"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Hello Huang.

It's great to see someone from MediaTek providing information on this
intellectual property.

On 27.03.2024 12:51, SkyLake Huang (黃啟澤) wrote:

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.

This won't interfere with the LEDs so it's better. I'll use this method,
thank you! This is the current diff [1].

I suppose bit 6 is an internal EEE switch only on MT7531? I see that bit 6
of 0x403 is unset on MT7530 yet EEE is still enabled on the switch MACs.

There's still packet loss when EEE is enabled by setting
MT7531_RG_SYSPLL_DMY2. Please let me know if MT7531 switch PHYs need
calibration for EEE like MT7986 and MT7988 SoC PHYs.

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.

I disagree. The operation concerns the switch MACs too, not just the switch
PHYs. And the operation is not per switch PHY.

+
+/* 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?

Disabling EEE advertisement on the PHY driver doesn't fail. To confirm that
and that MDIO_AN_EEE_ADV is populated after it is unset by the PHY driver,
I've done some tests with this diff [2].

Currently:

[ 2.189149] MediaTek MT7531 PHY mt7530-0:00: MDIO_AN_EEE_ADV is 00000000
[ 2.204792] MediaTek MT7531 PHY mt7530-0:00: MDIO_AN_EEE_ADV is 00000000
[ 2.221413] mt7530-mdio mdio-bus:1f wan (uninitialized): PHY [mt7530-0:00] driver [MediaTek MT7531 PHY] (irq=139)
[ 2.233762] MediaTek MT7531 PHY mt7530-0:01: MDIO_AN_EEE_ADV is 00000000
[ 2.242555] MediaTek MT7531 PHY mt7530-0:01: MDIO_AN_EEE_ADV is 00000000
[ 2.259011] mt7530-mdio mdio-bus:1f lan0 (uninitialized): PHY [mt7530-0:01] driver [MediaTek MT7531 PHY] (irq=140)
[ 2.271052] MediaTek MT7531 PHY mt7530-0:02: MDIO_AN_EEE_ADV is 00000000
[ 2.279842] MediaTek MT7531 PHY mt7530-0:02: MDIO_AN_EEE_ADV is 00000000
[ 2.296297] mt7530-mdio mdio-bus:1f lan1 (uninitialized): PHY [mt7530-0:02] driver [MediaTek MT7531 PHY] (irq=141)
[ 2.308331] MediaTek MT7531 PHY mt7530-0:03: MDIO_AN_EEE_ADV is 00000000
[ 2.317134] MediaTek MT7531 PHY mt7530-0:03: MDIO_AN_EEE_ADV is 00000000
[ 2.333597] mt7530-mdio mdio-bus:1f lan2 (uninitialized): PHY [mt7530-0:03] driver [MediaTek MT7531 PHY] (irq=142)
[ 2.345653] MediaTek MT7531 PHY mt7530-0:04: MDIO_AN_EEE_ADV is 00000000
[ 2.354423] MediaTek MT7531 PHY mt7530-0:04: MDIO_AN_EEE_ADV is 00000000
[ 2.370885] mt7530-mdio mdio-bus:1f lan3 (uninitialized): PHY [mt7530-0:04] driver [MediaTek MT7531 PHY] (irq=143)

# mount -t debugfs none /sys/kernel/debug
# echo go > /sys/kernel/debug/mt7530/eee_adv
[ 33.055962] mt7530-mdio mdio-bus:1f: phy0: MDIO_AN_EEE_ADV is 00000000
[ 33.062943] mt7530-mdio mdio-bus:1f: phy1: MDIO_AN_EEE_ADV is 00000000
[ 33.069909] mt7530-mdio mdio-bus:1f: phy2: MDIO_AN_EEE_ADV is 00000000
[ 33.076871] mt7530-mdio mdio-bus:1f: phy3: MDIO_AN_EEE_ADV is 00000000
[ 33.083822] mt7530-mdio mdio-bus:1f: phy4: MDIO_AN_EEE_ADV is 00000000

# ethtool --show-eee lan0
EEE settings for lan0:
EEE status: not supported

---

If EEE_DIS is unset or MT7531_RG_SYSPLL_DMY2 is set, MDIO_AN_EEE_ADV is
populated. This also happens on the MT7530 switch:

[ 2.254291] MediaTek MT7531 PHY mt7530-0:00: MDIO_AN_EEE_ADV is 00000006
[ 2.270015] MediaTek MT7531 PHY mt7530-0:00: MDIO_AN_EEE_ADV is 00000000
[ 2.286572] mt7530-mdio mdio-bus:1f wan (uninitialized): PHY [mt7530-0:00] driver [MediaTek MT7531 PHY] (irq=139)
[ 2.298921] MediaTek MT7531 PHY mt7530-0:01: MDIO_AN_EEE_ADV is 00000006
[ 2.307712] MediaTek MT7531 PHY mt7530-0:01: MDIO_AN_EEE_ADV is 00000000
[ 2.324162] mt7530-mdio mdio-bus:1f lan0 (uninitialized): PHY [mt7530-0:01] driver [MediaTek MT7531 PHY] (irq=140)
[ 2.336204] MediaTek MT7531 PHY mt7530-0:02: MDIO_AN_EEE_ADV is 00000006
[ 2.344974] MediaTek MT7531 PHY mt7530-0:02: MDIO_AN_EEE_ADV is 00000000
[ 2.361445] mt7530-mdio mdio-bus:1f lan1 (uninitialized): PHY [mt7530-0:02] driver [MediaTek MT7531 PHY] (irq=141)
[ 2.373475] MediaTek MT7531 PHY mt7530-0:03: MDIO_AN_EEE_ADV is 00000006
[ 2.382273] MediaTek MT7531 PHY mt7530-0:03: MDIO_AN_EEE_ADV is 00000000
[ 2.398728] mt7530-mdio mdio-bus:1f lan2 (uninitialized): PHY [mt7530-0:03] driver [MediaTek MT7531 PHY] (irq=142)
[ 2.410780] MediaTek MT7531 PHY mt7530-0:04: MDIO_AN_EEE_ADV is 00000006
[ 2.419571] MediaTek MT7531 PHY mt7530-0:04: MDIO_AN_EEE_ADV is 00000000
[ 2.436028] mt7530-mdio mdio-bus:1f lan3 (uninitialized): PHY [mt7530-0:04] driver [MediaTek MT7531 PHY] (irq=143)

# mount -t debugfs none /sys/kernel/debug
# echo go > /sys/kernel/debug/mt7530/eee_adv
[ 8.731911] mt7530-mdio mdio-bus:1f: phy0: MDIO_AN_EEE_ADV is 00000006
[ 8.738908] mt7530-mdio mdio-bus:1f: phy1: MDIO_AN_EEE_ADV is 00000006
[ 8.745861] mt7530-mdio mdio-bus:1f: phy2: MDIO_AN_EEE_ADV is 00000006
[ 8.752816] mt7530-mdio mdio-bus:1f: phy3: MDIO_AN_EEE_ADV is 00000006
[ 8.759776] mt7530-mdio mdio-bus:1f: phy4: MDIO_AN_EEE_ADV is 00000006

# ethtool --show-eee lan0
EEE settings for lan0:
EEE status: enabled - inactive
Tx LPI: disabled
Supported EEE link modes: 100baseT/Full
1000baseT/Full
Advertised EEE link modes: 100baseT/Full
1000baseT/Full
Link partner advertised EEE link modes: Not reported

---

If MDIO_AN_EEE_ADV is unset before the PHY driver kicks in, it stays unset:

[ 2.266561] MediaTek MT7531 PHY mt7530-0:00: MDIO_AN_EEE_ADV is 00000000
[ 2.282268] MediaTek MT7531 PHY mt7530-0:00: MDIO_AN_EEE_ADV is 00000000
[ 2.298832] mt7530-mdio mdio-bus:1f wan (uninitialized): PHY [mt7530-0:00] driver [MediaTek MT7531 PHY] (irq=139)
[ 2.311187] MediaTek MT7531 PHY mt7530-0:01: MDIO_AN_EEE_ADV is 00000000
[ 2.319976] MediaTek MT7531 PHY mt7530-0:01: MDIO_AN_EEE_ADV is 00000000
[ 2.336443] mt7530-mdio mdio-bus:1f lan0 (uninitialized): PHY [mt7530-0:01] driver [MediaTek MT7531 PHY] (irq=140)
[ 2.348488] MediaTek MT7531 PHY mt7530-0:02: MDIO_AN_EEE_ADV is 00000000
[ 2.357280] MediaTek MT7531 PHY mt7530-0:02: MDIO_AN_EEE_ADV is 00000000
[ 2.373728] mt7530-mdio mdio-bus:1f lan1 (uninitialized): PHY [mt7530-0:02] driver [MediaTek MT7531 PHY] (irq=141)
[ 2.385780] MediaTek MT7531 PHY mt7530-0:03: MDIO_AN_EEE_ADV is 00000000
[ 2.394551] MediaTek MT7531 PHY mt7530-0:03: MDIO_AN_EEE_ADV is 00000000
[ 2.411011] mt7530-mdio mdio-bus:1f lan2 (uninitialized): PHY [mt7530-0:03] driver [MediaTek MT7531 PHY] (irq=142)
[ 2.423047] MediaTek MT7531 PHY mt7530-0:04: MDIO_AN_EEE_ADV is 00000000
[ 2.431849] MediaTek MT7531 PHY mt7530-0:04: MDIO_AN_EEE_ADV is 00000000
[ 2.448324] mt7530-mdio mdio-bus:1f lan3 (uninitialized): PHY [mt7530-0:04] driver [MediaTek MT7531 PHY] (irq=143)

# mount -t debugfs none /sys/kernel/debug
# echo go > /sys/kernel/debug/mt7530/eee_adv
[ 33.906140] mt7530-mdio mdio-bus:1f: phy0: MDIO_AN_EEE_ADV is 00000000
[ 33.913121] mt7530-mdio mdio-bus:1f: phy1: MDIO_AN_EEE_ADV is 00000000
[ 33.920094] mt7530-mdio mdio-bus:1f: phy2: MDIO_AN_EEE_ADV is 00000000
[ 33.927054] mt7530-mdio mdio-bus:1f: phy3: MDIO_AN_EEE_ADV is 00000000
[ 33.934005] mt7530-mdio mdio-bus:1f: phy4: MDIO_AN_EEE_ADV is 00000000

# ethtool --show-eee lan0
EEE settings for lan0:
EEE status: disabled
Tx LPI: disabled
Supported EEE link modes: 100baseT/Full
1000baseT/Full
Advertised EEE link modes: Not reported
Link partner advertised EEE link modes: Not reported

[1]
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index fb3d2a53bbd9..9a351d11f349 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -2630,18 +2630,25 @@ mt7531_setup(struct dsa_switch *ds)
mt7530_rmw(priv, MT7531_GPIO_MODE0, MT7531_GPIO0_MASK,
MT7531_GPIO0_INTERRUPT);
- /* Enable PHY core PLL, since phy_device has not yet been created
- * provided for phy_[read,write]_mmd_indirect is called, we provide
- * our own mt7531_ind_mmd_phy_[read,write] to complete this
- * function.
+ /* Enable Energy-Efficient Ethernet (EEE) and PHY core PLL, since
+ * phy_device has not yet been created provided for
+ * phy_[read,write]_mmd_indirect is called, we provide our own
+ * mt7531_ind_mmd_phy_[read,write] to complete this function.
*/
val = mt7531_ind_c45_phy_read(priv, MT753X_CTRL_PHY_ADDR,
MDIO_MMD_VEND2, CORE_PLL_GROUP4);
- val |= MT7531_PHY_PLL_BYPASS_MODE;
+ val |= MT7531_RG_SYSPLL_DMY2 | MT7531_PHY_PLL_BYPASS_MODE;
val &= ~MT7531_PHY_PLL_OFF;
mt7531_ind_c45_phy_write(priv, MT753X_CTRL_PHY_ADDR, MDIO_MMD_VEND2,
CORE_PLL_GROUP4, 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);
+ }
+
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 d17b318e6ee4..9439db495001 100644
--- a/drivers/net/dsa/mt7530.h
+++ b/drivers/net/dsa/mt7530.h
@@ -616,6 +616,7 @@ enum mt7531_clk_skew {
#define RG_SYSPLL_DDSFBK_EN BIT(12)
#define RG_SYSPLL_BIAS_EN BIT(11)
#define RG_SYSPLL_BIAS_LPF_EN BIT(10)
+#define MT7531_RG_SYSPLL_DMY2 BIT(6)
#define MT7531_PHY_PLL_OFF BIT(5)
#define MT7531_PHY_PLL_BYPASS_MODE BIT(4)

[2]
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 35906e669259..fb3d2a53bbd9 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -845,6 +845,22 @@ static ssize_t reg_value_write(struct file *filp, const char __user *buffer,
return count;
}
+static ssize_t eee_adv_write(struct file *filp, const char __user *buffer,
+ size_t count, loff_t *ppos)
+{
+ struct mt7530_priv *priv = filp->private_data;
+ int i;
+
+ for (i = MT753X_CTRL_PHY_ADDR;
+ i < MT753X_CTRL_PHY_ADDR + MT7530_NUM_PHYS; i++) {
+ dev_info(priv->dev, "phy%d: MDIO_AN_EEE_ADV is %08x", i,
+ mt7531_ind_c45_phy_read(priv, i, MDIO_MMD_AN,
+ MDIO_AN_EEE_ADV));
+ }
+
+ return count;
+}
+
static const struct file_operations reg_address_fops = {
.owner = THIS_MODULE,
.open = simple_open,
@@ -859,6 +875,12 @@ static const struct file_operations reg_value_fops = {
.write = reg_value_write,
};
+static const struct file_operations eee_adv_fops = {
+ .owner = THIS_MODULE,
+ .open = simple_open,
+ .write = eee_adv_write,
+};
+
static void mt7530_debugfs_init(struct mt7530_priv *priv)
{
struct dentry *dir;
@@ -869,6 +891,7 @@ static void mt7530_debugfs_init(struct mt7530_priv *priv)
debugfs_create_file("reg_address", 0644, dir, priv, &reg_address_fops);
debugfs_create_file("reg_value", 0644, dir, priv, &reg_value_fops);
+ debugfs_create_file("eee_adv", 0644, dir, priv, &eee_adv_fops);
}
static void
diff --git a/drivers/net/phy/mediatek-ge.c b/drivers/net/phy/mediatek-ge.c
index e8822adbf3bc..aacef7091d8f 100644
--- a/drivers/net/phy/mediatek-ge.c
+++ b/drivers/net/phy/mediatek-ge.c
@@ -71,9 +71,15 @@ static int mtk_gephy_write_page(struct phy_device *phydev, int page)
static void mtk_gephy_config_init(struct phy_device *phydev)
{
+ dev_info(&phydev->mdio.dev, "MDIO_AN_EEE_ADV is %08x",
+ phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_AN_EEE_ADV));
+
/* Disable EEE */
phy_write_mmd(phydev, MDIO_MMD_AN, MDIO_AN_EEE_ADV, 0);
+ dev_info(&phydev->mdio.dev, "MDIO_AN_EEE_ADV is %08x",
+ phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_AN_EEE_ADV));
+
/* Enable HW auto downshift */
phy_modify_paged(phydev, MTK_PHY_PAGE_EXTENDED, 0x14, 0, BIT(4));

Arınç

Next message: Jonathan Cameron: "Re: [PATCH RFC 4/7] iio: addac: ad74115: Use devm_regulator_get_enable_get_voltage()"
Previous message: Mathieu Desnoyers: "Re: [PATCH 2/2] ARC: mm: fix new code about cache aliasing"
In reply to: SkyLake Huang (黃啟澤): "Re: [PATCH net v2 1/2] net: dsa: mt7530: fix enabling EEE on MT7531 switch on all boards"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]