net: dsa: mt7530: add EcoNet EN7528 built-in switch support

The EcoNet EN7528 is a MIPS SoC whose platform support is already upstream. It integrates an MT7530 switch, memory-mapped like the built-in switches of the MediaTek MT7988 and Airoha EN7581/AN7583 SoCs, but with a true MT7530 core, four Gigabit PHYs on ports 1-4 and a CPU port at a fixed 1000 Mbps full duplex link.

Patch 1 documents the compatible, patch 2 adds the driver support.

Ahmed Naseef (2):
dt-bindings: net: dsa: mediatek,mt7530: add econet,en7528-switch
net: dsa: mt7530: add EN7528 support

 .../bindings/net/dsa/mediatek,mt7530.yaml     |  5 ++
 drivers/net/dsa/mt7530-mmio.c                 |  1 +
 drivers/net/dsa/mt7530.c                      | 56 ++++++++++++++++++-
 drivers/net/dsa/mt7530.h                      |  1 +
 4 files changed, 62 insertions(+), 1 deletion(-)

base-commit: fe3e786ef4eb6e47d2901f568a27bd920477bbe9

2.34.1

The EcoNet EN7528 MIPS SoC integrates an MT7530 Gigabit switch, memory-mapped in the SoC register space like the built-in switches of the MediaTek MT7988 and Airoha EN7581/AN7583 SoCs. Its four user ports are connected to integrated Gigabit PHYs and its CPU port is connected internally to the SoC Ethernet MAC.

Add the econet,en7528-switch compatible, with the same constraints as the other built-in switches.

---
 .../devicetree/bindings/net/dsa/mediatek,mt7530.yaml         | 5 +++++
 1 file changed, 5 insertions(+)

index 815a90808901..90b3582b7619 100644
@@ -100,6 +100,10 @@ properties:
           Built-in switch of the Airoha AN7583 SoC
         const: airoha,an7583-switch
 
+      - description:
+          Built-in switch of the EcoNet EN7528 SoC
+        const: econet,en7528-switch
+
   reg:
     maxItems: 1
 

@@ -318,6 +322,7 @@ allOf:
             - mediatek,mt7988-switch
             - airoha,en7581-switch
             - airoha,an7583-switch
+            - econet,en7528-switch
     then:
       $ref: "#/$defs/builtin-dsa-port"
       properties:

2.34.1

Please note what actually makes this different from the other devices such that it cannot use a fallback. It's not clear from this description if the described features are shared with other devices or unique.
pw-bot: changes-requested

Thanks,
Conor.

The EcoNet EN7528 SoC integrates an MT7530 switch (the chip revision register reads 0x7530), memory-mapped in the SoC register space and reached through the same MMIO glue used for the built-in switches of the MediaTek MT7988 and Airoha EN7581/AN7583 SoCs. Its reset sequence and its PHY indirect access registers are the same as on those switches, so add an ID_EN7528 variant bound with the "econet,en7528-switch" compatible, reusing mt7988_setup() and the indirect PHY accessors.

The switch core, however, is an MT7530 and not an MT7531 derivative: it has no MT7531 CFC register, and the CPU port to trap frames to is set through the MT7530-style CPU_EN / CPU_PORT fields of the MFC register, so add it to the MT7530 handling in mt753x_conduit_state_change(). For the same reason the MT7530 mirror and force-mode register layouts already apply to it as the default of the MT753X_*() macros.

The four user ports (1-4) are connected to integrated Gigabit PHYs at MDIO addresses 9-12 of the switch internal MDIO bus. The CPU port (port
6) is connected to the SoC Ethernet MAC at a fixed 1000 Mbps full duplex link, so the port capabilities cannot be shared with the MT7988 and EN7581 switches, whose CPU ports run at 10 Gbps.

The LAN GPHYs advertise EEE by default, but negotiating EEE with some link partners results in an unstable link with dropped frames. Disable EEE advertisement on them at setup time, like mt7531_setup() does for the MT7531 switch PHYs.

---
 drivers/net/dsa/mt7530-mmio.c |  1 +
 drivers/net/dsa/mt7530.c      | 56 ++++++++++++++++++++++++++++++++++-
 drivers/net/dsa/mt7530.h      |  1 +
 3 files changed, 57 insertions(+), 1 deletion(-)

index 1dc8b93fb51a..1065671bcdd1 100644
@@ -13,6 +13,7 @@
 static const struct of_device_id mt7988_of_match[] = {
 	{ .compatible = "airoha,an7583-switch", .data = &mt753x_table[ID_AN7583], },
 	{ .compatible = "airoha,en7581-switch", .data = &mt753x_table[ID_EN7581], },
+	{ .compatible = "econet,en7528-switch", .data = &mt753x_table[ID_EN7528], },
 	{ .compatible = "mediatek,mt7988-switch", .data = &mt753x_table[ID_MT7988], },
 	{ /* sentinel */ },
 };

index dd33b0df3419..5f1e841f42c0 100644
@@ -21,6 +21,7 @@ enum mt753x_id {
 	ID_MT7988 = 3,
 	ID_EN7581 = 4,
 	ID_AN7583 = 5,
+	ID_EN7528 = 6,
 };
 
 #define	NUM_TRGMII_CTRL			5

2.34.1

Thanks, you are right , the register write is the wrong approach.

While looking at phylink_bringup_phy(), I noticed that the mt7530 driver sets config->lpi_capabilities but never config->lpi_interfaces. Since

        mac_supports_eee = mac_supports_eee_ops && lpi_capabilities &&
                           !phy_interface_empty(lpi_interfaces);

that leaves mac_supports_eee false on all mt753x ports, so phylink already takes the phy_disable_eee() path for every port and phylink_ethtool_set_eee() returns -EOPNOTSUPP. On the EN7528 I confirmed "ethtool --set-eee lan1 eee on" reports "Not supported", so EEE cannot currently be enabled from userspace on this driver at all. That looks like it unintentionally undid the "EEE can now be enabled using ethtool" behaviour from commit 06dfcd4098cf ("net: dsa: mt7530:
fix enabling EEE on MT7531 switch on all boards"), starting with commit 9cf21773f535 ("net: dsa: mt7530: convert to phylink managed EEE"). That is pre-existing and outside the scope of this series, just flagging it.

For the EN7528 the GPHYs report EEE capability, but the link stops passing traffic once LPI is entered, so EEE must stay off regardless. The register loop cannot express that to phylib, as you noted.

My plan for v2 is to not advertise lpi_capabilities for the EN7528, so mac_supports_eee stays false by design and phylink disables EEE on these PHYs via phy_disable_eee() and refuses to enable it from userspace. Does that sound like the right approach to you, or would you prefer it handled differently?

Thanks,
Ahmed

That sounds like the correct thing to do.

Andrew