[PATCH net v2 1/2] net: dsa: mt7530: populate lpi_interfaces to fix EEE support

mt753x_phylink_get_caps() fills in lpi_capabilities and
lpi_timer_default but never lpi_interfaces, so phylink treats the MAC
as not supporting EEE: ethtool reports "Not supported" and
phy_disable_eee() keeps userspace locked out. That undoes what
commit 06dfcd4098cf ("net: dsa: mt7530: fix enabling EEE on MT7531
switch on all boards") arranged: EEE off by default, but enableable
with ethtool.

Copy the supported interfaces into lpi_interfaces after
mac_port_get_caps() has populated them, and leave the speeds above
1 Gbps out for now: drop 2500BASE-X from the copy, drop MAC_2500FD
from lpi_capabilities, and skip ports that support neither 100 Mbps
nor 1 Gbps. PMCR folds SPEED_2500 and SPEED_10000 onto
PMCR_FORCE_SPEED_1000, so PMCR_FORCE_EEE1G is what would govern LPI on
those links, and it is unvalidated rather than unsupported: MediaTek's
SDK driver sets the EEE force bits for 100 Mbps and 1 Gbps link speed
only, EEE signalling on 2500BASE-X is outside 802.3, and the 1 us unit
of the wakeup timers is undocumented with the port clock at 2.5 times
the rate. Narrowing lpi_capabilities alone would not do it, since it
gates on the media speed a rate matching PHY reports rather than on
the speed the MAC runs at. On MT7988, EN7581 and AN7583 port 6 is
10 Gbps only and shares PHY_INTERFACE_MODE_INTERNAL with the user
ports, so the interface mask alone cannot tell them apart.

LPI stays off until userspace enables it. The EEE advertisement of a
PHY that advertises it out of reset does come back, since phylink
stops force-clearing it. MT7531's internal PHYs keep the
advertisement mt7531_setup() zeroed, and EN7528 keeps both bitmaps
empty, so EEE stays fully off there.

Fixes: 9cf21773f535 ("net: dsa: mt7530: convert to phylink managed EEE")
Signed-off-by: Aleksei Sviridkin <[email protected]>
---

Two pre-existing things this patch makes live, neither addressed here:

- The unit of LPI_THRESH is still unspecified, as the comment above
  lpi_timer_default says. With EEE reachable again, ethtool reports
  that raw value as microseconds and writes userspace values back
  unconverted, while mtk_eth_soc treats a structurally identical field
  as milliseconds (DIV_ROUND_UP(timer, 1000)). Reading the default back
  reproduces the same raw value whatever the unit is, but a timer set
  from userspace in microseconds would be off by 1000 if the field is
  in milliseconds. On an MT7531 board ethtool now reports 30 for a
  switch port, the raw LPI_THRESH field, against 1000 for the SoC MAC,
  which is a driver constant already in microseconds; whether the
  hardware means 30 microseconds is exactly the open question. Does
  anyone have the datasheet answer?

- mt753x_phylink_mac_enable_tx_lpi() sets the PMCR force-EEE bits
  without checking the resolved speed or interface, relying entirely on
  phylink never calling it above 1 Gbps. A check there would make the
  driver robust independently of lpi_interfaces being right; if that
  check is ever sent, it will warn loudly.

On leaving the higher speeds out: the absence of 2.5 Gbps EEE force
bits in PMCR is not the argument, since the same reasoning applied to
the speed field would say the switch cannot do 2.5 Gbps at all. The
masks say nobody has validated it, not that the hardware refuses.
Doing that here rather than in a separate patch changes nothing
observable: while lpi_interfaces was empty, lpi_capabilities never
reached phy->advertising_eee.

If PMCR_FORCE_EEE1G does work on the overclocked link, a rate matched
EN8811H port is exactly the setup that would benefit, so this is worth
an experiment later.
 drivers/net/dsa/mt7530.c | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 2b7be091c056..4ed6218df64b 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -3172,23 +3172,37 @@ static void mt753x_phylink_get_caps(struct dsa_switch *ds, int port,
 
 	config->mac_capabilities = MAC_ASYM_PAUSE | MAC_SYM_PAUSE;
 
+	priv->info->mac_port_get_caps(ds, port, config);
+
 	/* The EN7528 GPHYs report EEE capability, but negotiating EEE with
 	 * common link partners (e.g. Realtek GbE NICs) results in an unstable
 	 * link with dropped frames. Leave the LPI capabilities empty so that
 	 * phylink disables EEE on these PHYs and refuses to enable it from
-	 * userspace.
+	 * userspace. Ports that run at neither 100 Mbps nor 1 Gbps are left
+	 * empty too, since LPI above 1 Gbps is unvalidated.
 	 */
-	if (priv->id != ID_EN7528) {
+	if (priv->id != ID_EN7528 &&
+	    config->mac_capabilities & (MAC_100FD | MAC_1000FD)) {
 		u32 eeecr = mt7530_read(priv, MT753X_PMEEECR_P(port));
 
-		config->lpi_capabilities = MAC_100FD | MAC_1000FD | MAC_2500FD;
+		/* PMCR folds SPEED_2500 and SPEED_10000 onto
+		 * PMCR_FORCE_SPEED_1000, so LPI above 1 Gbps would be
+		 * governed by PMCR_FORCE_EEE1G and is unvalidated rather than
+		 * unsupported. Leave it out of both bitmaps: lpi_capabilities
+		 * gates on the media speed a rate matching PHY reports, not
+		 * on the speed the MAC runs at.
+		 */
+		config->lpi_capabilities = MAC_100FD | MAC_1000FD;
+		phy_interface_copy(config->lpi_interfaces,
+				   config->supported_interfaces);
+		__clear_bit(PHY_INTERFACE_MODE_2500BASEX,
+			    config->lpi_interfaces);
+
 		/* tx_lpi_timer should be in microseconds. The time units for
 		 * LPI threshold are unspecified.
 		 */
 		config->lpi_timer_default = FIELD_GET(LPI_THRESH_MASK, eeecr);
 	}
-
-	priv->info->mac_port_get_caps(ds, port, config);
 }
 
 static int mt753x_pcs_validate(struct phylink_pcs *pcs,
-- 
2.55.0