Messages in this thread | | | Date | Sat, 27 Apr 2024 03:24:18 +0100 | From | Daniel Golle <> | Subject | Re: [PATCH net-next v2 07/15] net: dsa: mt7530: move MT753X_MTRAP operations for MT7530 |
| |
Hi Arınç,
On Mon, Apr 22, 2024 at 10:15:14AM +0300, Arınç ÜNAL via B4 Relay wrote: > From: Arınç ÜNAL <arinc.unal@arinc9.com> > > On MT7530, the media-independent interfaces of port 5 and 6 are controlled > by the MT7530_P5_DIS and MT7530_P6_DIS bits of the hardware trap. Deal with > these bits only when the relevant port is being enabled or disabled. This > ensures that these ports will be disabled when they are not in use. > > Do not set MT7530_CHG_TRAP on mt7530_setup_port5() as that's already being > done on mt7530_setup().
Multiple users reported ([1], [2]) that after I've imported the series to OpenWrt they noticed that WAN connection on MT7621 boards using PHY-muxing to hook up either port 0 or port 4 to GMAC1 no longer works.
The link still seems to come up, but no data flows. I went ahead and confirmed the bug, then started bisecting the patches of this series, and ended up identifying this very patch being the culprit.
I can't exclude that what ever the issue may be is caused by other downstream patches we have, but can confirm that removing this patch of your series [3] in OpenWrt fixes the issue. Please take a look and as the cover letter states you have tested this on some MT7621 board, please make sure traffic actually flows on the PHY-muxed port on that board after this patch is applied, and if not, please figure out why and repost a fixed version of this patch.
Cheers
Daniel
[1]: https://github.com/openwrt/openwrt/issues/15273 [2]: https://github.com/openwrt/openwrt/issues/15279 [3]: https://git.openwrt.org/?p=openwrt/openwrt.git;a=commit;h=a8dde7e5bd6d289db6485cf57d3512ea62eaa827
> > Instead of globally setting MT7530_P5_MAC_SEL, clear it, then set it only > on the appropriate case. > > If PHY muxing is detected, clear MT7530_P5_DIS before calling > mt7530_setup_port5(). > > Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com> > --- > drivers/net/dsa/mt7530.c | 38 +++++++++++++++++++++++++++----------- > 1 file changed, 27 insertions(+), 11 deletions(-) > > diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c > index 606516206fb9..83436723cb16 100644 > --- a/drivers/net/dsa/mt7530.c > +++ b/drivers/net/dsa/mt7530.c > @@ -880,8 +880,7 @@ static void mt7530_setup_port5(struct dsa_switch *ds, phy_interface_t interface) > > val = mt7530_read(priv, MT753X_MTRAP); > > - val |= MT7530_CHG_TRAP | MT7530_P5_MAC_SEL | MT7530_P5_DIS; > - val &= ~MT7530_P5_RGMII_MODE & ~MT7530_P5_PHY0_SEL; > + val &= ~MT7530_P5_PHY0_SEL & ~MT7530_P5_MAC_SEL & ~MT7530_P5_RGMII_MODE; > > switch (priv->p5_mode) { > /* MUX_PHY_P0: P0 -> P5 -> SoC MAC */ > @@ -891,15 +890,13 @@ static void mt7530_setup_port5(struct dsa_switch *ds, phy_interface_t interface) > > /* MUX_PHY_P4: P4 -> P5 -> SoC MAC */ > case MUX_PHY_P4: > - val &= ~MT7530_P5_MAC_SEL & ~MT7530_P5_DIS; > - > /* Setup the MAC by default for the cpu port */ > mt7530_write(priv, MT753X_PMCR_P(5), 0x56300); > break; > > /* GMAC5: P5 -> SoC MAC or external PHY */ > default: > - val &= ~MT7530_P5_DIS; > + val |= MT7530_P5_MAC_SEL; > break; > } > > @@ -1193,6 +1190,14 @@ mt7530_port_enable(struct dsa_switch *ds, int port, > > mutex_unlock(&priv->reg_mutex); > > + if (priv->id != ID_MT7530 && priv->id != ID_MT7621) > + return 0; > + > + if (port == 5) > + mt7530_clear(priv, MT753X_MTRAP, MT7530_P5_DIS); > + else if (port == 6) > + mt7530_clear(priv, MT753X_MTRAP, MT7530_P6_DIS); > + > return 0; > } > > @@ -1211,6 +1216,14 @@ mt7530_port_disable(struct dsa_switch *ds, int port) > PCR_MATRIX_CLR); > > mutex_unlock(&priv->reg_mutex); > + > + if (priv->id != ID_MT7530 && priv->id != ID_MT7621) > + return; > + > + if (port == 5) > + mt7530_set(priv, MT753X_MTRAP, MT7530_P5_DIS); > + else if (port == 6) > + mt7530_set(priv, MT753X_MTRAP, MT7530_P6_DIS); > } > > static int > @@ -2401,11 +2414,11 @@ mt7530_setup(struct dsa_switch *ds) > mt7530_rmw(priv, MT7530_TRGMII_RD(i), > RD_TAP_MASK, RD_TAP(16)); > > - /* Enable port 6 */ > - val = mt7530_read(priv, MT753X_MTRAP); > - val &= ~MT7530_P6_DIS & ~MT7530_PHY_INDIRECT_ACCESS; > - val |= MT7530_CHG_TRAP; > - mt7530_write(priv, MT753X_MTRAP, val); > + /* Allow modifying the trap and directly access PHY registers via the > + * MDIO bus the switch is on. > + */ > + mt7530_rmw(priv, MT753X_MTRAP, MT7530_CHG_TRAP | > + MT7530_PHY_INDIRECT_ACCESS, MT7530_CHG_TRAP); > > if ((val & MT7530_XTAL_MASK) == MT7530_XTAL_40MHZ) > mt7530_pll_setup(priv); > @@ -2488,8 +2501,11 @@ mt7530_setup(struct dsa_switch *ds) > break; > } > > - if (priv->p5_mode == MUX_PHY_P0 || priv->p5_mode == MUX_PHY_P4) > + if (priv->p5_mode == MUX_PHY_P0 || > + priv->p5_mode == MUX_PHY_P4) { > + mt7530_clear(priv, MT753X_MTRAP, MT7530_P5_DIS); > mt7530_setup_port5(ds, interface); > + } > } > > #ifdef CONFIG_GPIOLIB > > -- > 2.40.1 > >
| |