Thanks for the review. You're absolutely right about the RGMII delay semantics — phy-mode describes what the PHY/PCB provides, not an instruction for the MAC to automatically compensate. I've dropped the RXID/TXID delay changes from this patch and left the original behaviour (all *ID modes disable both MAC-side delays) untouched.
The remaining changes are:
- Remove the magic number
5 by adding MT7620_GSW_EXTERNAL_PORT to the shared header.
- Rename
mt7620_gsw_config_external_port() → mt7620_gsw_config_rgmii1() and drop the redundant port parameter.
- Introduce
mib_gige_ports so the fast MIB poll (20 ms) only covers ports actually running at 1 Gbps, not merely enabled ones.
- Reset
mib_port_intervals on port disable/enable to keep the cadence consistent.
--- a/target/linux/ramips/files/drivers/net/ethernet/ralink/gsw_mt7620.h
+++ b/target/linux/ramips/files/drivers/net/ethernet/ralink/gsw_mt7620.h
@@ -103,6 +103,8 @@ enum {
GSW_ATTR_PORT_UNTAG,
};
+#define MT7620_GSW_EXTERNAL_PORT 5
+
struct mt7620_gsw_vlan {
u16 vid;
u8 members;
@@ -127,6 +129,7 @@ struct mt7620_gsw {
struct mt7620_gsw_vlan vlans[GSW_NUM_VLANS];
struct delayed_work mib_work;
unsigned long mib_active_ports;
+ unsigned long mib_gige_ports;
u8 mib_port_intervals;
/* Protects the software-extended MIB counter state. */
spinlock_t mib_lock;
@@ -138,8 +141,7 @@ struct mt7620_gsw {
void mtk_switch_w32(struct mt7620_gsw *gsw, u32 val, unsigned reg);
u32 mtk_switch_r32(struct mt7620_gsw *gsw, unsigned reg);
-int mt7620_gsw_config_external_port(struct mt7620_gsw *gsw, int port,
- phy_interface_t interface);
+int mt7620_gsw_config_rgmii1(struct mt7620_gsw *gsw, phy_interface_t interface);
int mtk_gsw_init(struct fe_priv *priv);
#if IS_ENABLED(CONFIG_NET_DSA_MT7620)
int mt7620_gsw_dsa_device_register(struct mt7620_gsw *gsw,
--- a/target/linux/ramips/files/drivers/net/ethernet/ralink/gsw_mt7620.c
+++ b/target/linux/ramips/files/drivers/net/ethernet/ralink/gsw_mt7620.c
@@ -35,16 +35,12 @@ u32 mtk_switch_r32(struct mt7620_gsw *gsw, unsigned reg)
}
EXPORT_SYMBOL_GPL(mtk_switch_r32);
-int mt7620_gsw_config_external_port(struct mt7620_gsw *gsw, int port,
- phy_interface_t interface)
+int mt7620_gsw_config_rgmii1(struct mt7620_gsw *gsw, phy_interface_t interface)
{
u32 mode, val;
u32 delay_mask = GSW_REG_GPCx_TXDELAY | GSW_REG_GPCx_RXDELAY;
u32 delay_val = 0;
- /* Port 5 is the external MAC exposed through RGMII1 on MT7620. */
- if (port != 5)
- return -EINVAL;
switch (interface) {
case PHY_INTERFACE_MODE_RGMII:
@@ -96,7 +92,7 @@ int mt7620_gsw_config_external_port(struct mt7620_gsw *gsw, int port,
return 0;
}
-EXPORT_SYMBOL_GPL(mt7620_gsw_config_external_port);
+EXPORT_SYMBOL_GPL(mt7620_gsw_config_rgmii1);
static irqreturn_t gsw_interrupt_mt7620(int irq, void *_priv)
{
--- a/target/linux/ramips/files/drivers/net/ethernet/ralink/gsw_mt7620_dsa.c
+++ b/target/linux/ramips/files/drivers/net/ethernet/ralink/gsw_mt7620_dsa.c
@@ -23,7 +23,7 @@
#define MT7620_DSA_NUM_PORTS 7
#define MT7620_DSA_CPU_PORT 6
-#define MT7620_DSA_EXTERNAL_PORT 5
+#define MT7620_DSA_EXTERNAL_PORT MT7620_GSW_EXTERNAL_PORT
#define MT7620_DSA_INTERNAL_PORTS GENMASK(4, 0)
#define MT7620_DSA_ALL_PORTS GENMASK(6, 0)
@@ -177,7 +177,7 @@ static void mt7620_gsw_mib_work(struct work_struct *work)
struct mt7620_gsw *gsw =
container_of(to_delayed_work(work), struct mt7620_gsw, mib_work);
unsigned long ports = BIT(MT7620_DSA_CPU_PORT) |
- (READ_ONCE(gsw->mib_active_ports) &
+ (READ_ONCE(gsw->mib_gige_ports) &
BIT(MT7620_DSA_EXTERNAL_PORT));
if (++gsw->mib_port_intervals == MT7620_MIB_PORT_INTERVALS) {
@@ -575,6 +575,7 @@ static int mt7620_gsw_setup(struct dsa_switch *ds)
memset(gsw->mib_stats, 0, sizeof(gsw->mib_stats));
gsw->mib_initialized = false;
gsw->mib_active_ports = 0;
+ gsw->mib_gige_ports = 0;
gsw->mib_port_intervals = 0;
mt7620_gsw_mib_update(gsw, MT7620_DSA_ALL_PORTS);
@@ -596,10 +597,12 @@ static int mt7620_gsw_port_enable(struct dsa_switch *ds, int port,
mt7620_gsw_set_port_matrix(gsw, port,
BIT(MT7620_DSA_CPU_PORT));
start_mib = !READ_ONCE(gsw->mib_active_ports);
set_bit(port, &gsw->mib_active_ports);
- if (start_mib)
+ if (start_mib) {
+ gsw->mib_port_intervals = 0;
schedule_delayed_work(&gsw->mib_work,
MT7620_MIB_CPU_INTERVAL);
+ }
}
return 0;
@@ -611,9 +614,11 @@ static void mt7620_gsw_port_disable(struct dsa_switch *ds, int port)
if (dsa_is_user_port(ds, port)) {
mt7620_gsw_set_port_matrix(gsw, port, 0);
clear_bit(port, &gsw->mib_active_ports);
+ clear_bit(port, &gsw->mib_gige_ports);
if (!READ_ONCE(gsw->mib_active_ports)) {
cancel_delayed_work_sync(&gsw->mib_work);
mt7620_gsw_mib_update(gsw, MT7620_DSA_ALL_PORTS);
+ gsw->mib_port_intervals = 0;
}
}
}
@@ -932,6 +937,11 @@ static void mt7620_gsw_mac_link_up(struct phylink_config *config,
hw_speed = mt7620_gsw_pmcr_speed(speed);
if (hw_speed < 0)
return;
+ if (speed == SPEED_1000)
+ set_bit(dp->index, &gsw->mib_gige_ports);
+ else
+ clear_bit(dp->index, &gsw->mib_gige_ports);
+
val = PMCR_IPG | PMCR_MAC_MODE | PMCR_FORCE | PMCR_TX_EN |
PMCR_RX_EN | PMCR_BACKOFF | PMCR_BACKPRES | PMCR_LINK |
PMCR_SPEED(hw_speed);
@@ -954,6 +964,8 @@ static void mt7620_gsw_mac_link_down(struct phylink_config *config,
if (dsa_is_cpu_port(dp->ds, dp->index))
return;
+ clear_bit(dp->index, &gsw->mib_gige_ports);
+
mt7620_gsw_rmw(gsw, GSW_REG_PORT_PMCR(dp->index),
PMCR_FORCE | PMCR_LINK, PMCR_FORCE);
}
@@ -982,7 +994,7 @@ static void mt7620_gsw_mac_config(struct phylink_config *config,
if (dp->index != MT7620_DSA_EXTERNAL_PORT)
return;
- ret = mt7620_gsw_config_external_port(gsw, dp->index, state->interface);
+ ret = mt7620_gsw_config_rgmii1(gsw, state->interface);
if (ret)
dev_err(gsw->dev, "port %d: unsupported PHY interface %d\n",
dp->index, state->interface);
I don't think splitting is worth the overhead here. The external port support and the DAP-1620 A2 conversion are tightly coupled — the device conversion is the only in-tree user of the new RGMII1 code right now, and the total diff is small enough to review in one go. Keeping it together also avoids the chicken-and-egg problem of merging a generic PR with no immediate consumer.
