| Message ID | 20240130-for-netnext-mt7530-improvements-2-v2-5-ba06f5dd9eb0@arinc9.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | MT7530 DSA Subdriver Improvements Act II | expand |
On Tue, Jan 30, 2024 at 06:20:51PM +0300, Arınç ÜNAL via B4 Relay wrote: > From: Arınç ÜNAL <arinc.unal@arinc9.com> > > This code is from before this driver was converted to phylink API. Phylink > deals with the unsupported interface cases before mt7530_setup_port6() is > run. Therefore, the default case would never run. However, it must be > defined nonetheless to handle all the remaining enumeration values, the > phy-modes. > > Switch to if statement for RGMII and return which simplifies the code and > saves an indent. > > Set P6_INTF_MODE, which is the the three least significant bits of the > MT7530_P6ECR register, to 0 for RGMII even though it will already be 0 > after reset. This is to keep supporting dynamic reconfiguration of the port > in the case the interface changes from TRGMII to RGMII. The core operations > for TRGMII does not interfere with RGMII so no need to undo them. That last sentence doesn't parse English gramar. "operations": plural "does": singular Should probably be either "The core operation for TRGMII does not..." or "The core operations for TRGMII do not..." As you are mentioning it, I'm now curious if you consider to dynamically reconfiguring TRGIII<->RGMII on port 6 depending on whether there is more then 1 GBit/s possible bandwidth needed between port 6 and the remaining ports? That could make sense for power management, but then we should at least again switch off the TRGMII clocks in the RGMII case before returning, see my suggestion inline below. > > Read XTAL after checking for RGMII as it's only needed for the TRGMII > interface mode. > > Change mt7530_setup_port6() to void now that there're no error cases left. > > Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com> Reviewed-by: Daniel Golle <daniel@makrotopia.org> > --- > drivers/net/dsa/mt7530.c | 103 ++++++++++++++++++++--------------------------- > 1 file changed, 43 insertions(+), 60 deletions(-) > > diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c > index c4d492e29fdf..36dc2bbcf3b6 100644 > --- a/drivers/net/dsa/mt7530.c > +++ b/drivers/net/dsa/mt7530.c > @@ -414,70 +414,57 @@ mt753x_preferred_default_local_cpu_port(struct dsa_switch *ds) > } > > /* Setup port 6 interface mode and TRGMII TX circuit */ > -static int > +static void > mt7530_setup_port6(struct dsa_switch *ds, phy_interface_t interface) > { > struct mt7530_priv *priv = ds->priv; > - u32 ncpo1, ssc_delta, trgint, xtal; > - > - xtal = mt7530_read(priv, MT7530_MHWTRAP) & HWTRAP_XTAL_MASK; > + u32 ncpo1, ssc_delta, xtal; > > - switch (interface) { > - case PHY_INTERFACE_MODE_RGMII: > - trgint = 0; > - break; > - case PHY_INTERFACE_MODE_TRGMII: > - trgint = 1; > - if (xtal == HWTRAP_XTAL_25MHZ) > - ssc_delta = 0x57; > - else > - ssc_delta = 0x87; > - if (priv->id == ID_MT7621) { > - /* PLL frequency: 125MHz: 1.0GBit */ > - if (xtal == HWTRAP_XTAL_40MHZ) > - ncpo1 = 0x0640; > - if (xtal == HWTRAP_XTAL_25MHZ) > - ncpo1 = 0x0a00; > - } else { /* PLL frequency: 250MHz: 2.0Gbit */ > - if (xtal == HWTRAP_XTAL_40MHZ) > - ncpo1 = 0x0c80; > - if (xtal == HWTRAP_XTAL_25MHZ) > - ncpo1 = 0x1400; > - } > - break; > - default: > - dev_err(priv->dev, "xMII interface %d not supported\n", > - interface); > - return -EINVAL; > + if (interface == PHY_INTERFACE_MODE_RGMII) { > + mt7530_rmw(priv, MT7530_P6ECR, P6_INTF_MODE_MASK, > + P6_INTF_MODE(0)); Maybe at least switch off TRGMIICK here because we are sure we don't need it in the RGMII case, ie: core_clear(priv, CORE_TRGMII_GSW_CLK_CG, REG_TRGMIICK_EN); And that then is another line of code already present just below which means you could keep variable trgint as it was and return after switching off TRGMIICK below anyway... > + return; > } > > - mt7530_rmw(priv, MT7530_P6ECR, P6_INTF_MODE_MASK, > - P6_INTF_MODE(trgint)); > + mt7530_rmw(priv, MT7530_P6ECR, P6_INTF_MODE_MASK, P6_INTF_MODE(1)); > > - if (trgint) { > - /* Disable the MT7530 TRGMII clocks */ > - core_clear(priv, CORE_TRGMII_GSW_CLK_CG, REG_TRGMIICK_EN); > + xtal = mt7530_read(priv, MT7530_MHWTRAP) & HWTRAP_XTAL_MASK; > > - /* Setup the MT7530 TRGMII Tx Clock */ > - core_write(priv, CORE_PLL_GROUP5, RG_LCDDS_PCW_NCPO1(ncpo1)); > - core_write(priv, CORE_PLL_GROUP6, RG_LCDDS_PCW_NCPO0(0)); > - core_write(priv, CORE_PLL_GROUP10, RG_LCDDS_SSC_DELTA(ssc_delta)); > - core_write(priv, CORE_PLL_GROUP11, RG_LCDDS_SSC_DELTA1(ssc_delta)); > - core_write(priv, CORE_PLL_GROUP4, > - RG_SYSPLL_DDSFBK_EN | RG_SYSPLL_BIAS_EN | > - RG_SYSPLL_BIAS_LPF_EN); > - core_write(priv, CORE_PLL_GROUP2, > - RG_SYSPLL_EN_NORMAL | RG_SYSPLL_VODEN | > - RG_SYSPLL_POSDIV(1)); > - core_write(priv, CORE_PLL_GROUP7, > - RG_LCDDS_PCW_NCPO_CHG | RG_LCCDS_C(3) | > - RG_LCDDS_PWDB | RG_LCDDS_ISO_EN); > + if (xtal == HWTRAP_XTAL_25MHZ) > + ssc_delta = 0x57; > + else > + ssc_delta = 0x87; > > - /* Enable the MT7530 TRGMII clocks */ > - core_set(priv, CORE_TRGMII_GSW_CLK_CG, REG_TRGMIICK_EN); > + if (priv->id == ID_MT7621) { > + /* PLL frequency: 125MHz: 1.0GBit */ > + if (xtal == HWTRAP_XTAL_40MHZ) > + ncpo1 = 0x0640; > + if (xtal == HWTRAP_XTAL_25MHZ) > + ncpo1 = 0x0a00; > + } else { /* PLL frequency: 250MHz: 2.0Gbit */ > + if (xtal == HWTRAP_XTAL_40MHZ) > + ncpo1 = 0x0c80; > + if (xtal == HWTRAP_XTAL_25MHZ) > + ncpo1 = 0x1400; > } > > - return 0; > + /* Disable the MT7530 TRGMII clocks */ > + core_clear(priv, CORE_TRGMII_GSW_CLK_CG, REG_TRGMIICK_EN); ... by moving this line up and letting it happen unconditionally for both RGMII and TRGMII (in case that works and doesn't break the RGMII case, but I assume it doesn't) > + > + /* Setup the MT7530 TRGMII Tx Clock */ > + core_write(priv, CORE_PLL_GROUP5, RG_LCDDS_PCW_NCPO1(ncpo1)); > + core_write(priv, CORE_PLL_GROUP6, RG_LCDDS_PCW_NCPO0(0)); > + core_write(priv, CORE_PLL_GROUP10, RG_LCDDS_SSC_DELTA(ssc_delta)); > + core_write(priv, CORE_PLL_GROUP11, RG_LCDDS_SSC_DELTA1(ssc_delta)); > + core_write(priv, CORE_PLL_GROUP4, RG_SYSPLL_DDSFBK_EN | > + RG_SYSPLL_BIAS_EN | RG_SYSPLL_BIAS_LPF_EN); > + core_write(priv, CORE_PLL_GROUP2, RG_SYSPLL_EN_NORMAL | > + RG_SYSPLL_VODEN | RG_SYSPLL_POSDIV(1)); > + core_write(priv, CORE_PLL_GROUP7, RG_LCDDS_PCW_NCPO_CHG | > + RG_LCCDS_C(3) | RG_LCDDS_PWDB | RG_LCDDS_ISO_EN); > + > + /* Enable the MT7530 TRGMII clocks */ > + core_set(priv, CORE_TRGMII_GSW_CLK_CG, REG_TRGMIICK_EN); > } > > static void > @@ -2609,15 +2596,11 @@ mt7530_mac_config(struct dsa_switch *ds, int port, unsigned int mode, > phy_interface_t interface) > { > struct mt7530_priv *priv = ds->priv; > - int ret; > > - if (port == 5) { > + if (port == 5) > mt7530_setup_port5(priv->ds, interface); > - } else if (port == 6) { > - ret = mt7530_setup_port6(priv->ds, interface); > - if (ret) > - return ret; > - } > + else if (port == 6) > + mt7530_setup_port6(priv->ds, interface); > > return 0; > } > > -- > 2.40.1 > >
On 30.01.2024 18:59, Daniel Golle wrote: > On Tue, Jan 30, 2024 at 06:20:51PM +0300, Arınç ÜNAL via B4 Relay wrote: >> From: Arınç ÜNAL <arinc.unal@arinc9.com> >> >> This code is from before this driver was converted to phylink API. Phylink >> deals with the unsupported interface cases before mt7530_setup_port6() is >> run. Therefore, the default case would never run. However, it must be >> defined nonetheless to handle all the remaining enumeration values, the >> phy-modes. >> >> Switch to if statement for RGMII and return which simplifies the code and >> saves an indent. >> >> Set P6_INTF_MODE, which is the the three least significant bits of the >> MT7530_P6ECR register, to 0 for RGMII even though it will already be 0 >> after reset. This is to keep supporting dynamic reconfiguration of the port >> in the case the interface changes from TRGMII to RGMII. The core operations >> for TRGMII does not interfere with RGMII so no need to undo them. > > That last sentence doesn't parse English gramar. > "operations": plural > "does": singular > > Should probably be either "The core operation for TRGMII does not..." > or "The core operations for TRGMII do not..." I'll use the latter, thanks. > > As you are mentioning it, I'm now curious if you consider to > dynamically reconfiguring TRGIII<->RGMII on port 6 depending on > whether there is more then 1 GBit/s possible bandwidth needed between > port 6 and the remaining ports? That could make sense for power > management, but then we should at least again switch off the TRGMII > clocks in the RGMII case before returning, see my suggestion inline > below. Turning off the TRGMII clocks for RGMII makes sense to me. But I don't see any cases where dynamic interface change between TRGMII and RGMII would ever occur. Speed too. No PHYs support TRGMII, only some MediaTek SoC MACs do. That means TRGMII would only be used in fixed links which there is no dynamic reconfiguration. My patch is about simplifying the code so I don't want to change the dynamic reconfiguration behaviour. That said, last year, I have very thoroughly tested this "turbo" RGMII interface between MT7530 standalone switch and MT7623NI SoC, which would supposedly achieve 2 Gbps TX & 2 Gbps RX. The performance was as if the link was regular RGMII. Unless the MediaTek SoC ethernet driver somehow caps TRGMII to 1 Gbps, I consider this whole TRGMII shenanigans a scam, to be extremely blunt. I'll give this a one last shot sometime before I push for the removal of TRGMII from Linux altogether and default to RGMII where it's used. Because of this, I don't want to spend too much time on this patch as it's potentially wasted effort. > >> >> Read XTAL after checking for RGMII as it's only needed for the TRGMII >> interface mode. >> >> Change mt7530_setup_port6() to void now that there're no error cases left. >> >> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com> > > Reviewed-by: Daniel Golle <daniel@makrotopia.org> > >> --- >> drivers/net/dsa/mt7530.c | 103 ++++++++++++++++++++--------------------------- >> 1 file changed, 43 insertions(+), 60 deletions(-) >> >> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c >> index c4d492e29fdf..36dc2bbcf3b6 100644 >> --- a/drivers/net/dsa/mt7530.c >> +++ b/drivers/net/dsa/mt7530.c >> @@ -414,70 +414,57 @@ mt753x_preferred_default_local_cpu_port(struct dsa_switch *ds) >> } >> >> /* Setup port 6 interface mode and TRGMII TX circuit */ >> -static int >> +static void >> mt7530_setup_port6(struct dsa_switch *ds, phy_interface_t interface) >> { >> struct mt7530_priv *priv = ds->priv; >> - u32 ncpo1, ssc_delta, trgint, xtal; >> - >> - xtal = mt7530_read(priv, MT7530_MHWTRAP) & HWTRAP_XTAL_MASK; >> + u32 ncpo1, ssc_delta, xtal; >> >> - switch (interface) { >> - case PHY_INTERFACE_MODE_RGMII: >> - trgint = 0; >> - break; >> - case PHY_INTERFACE_MODE_TRGMII: >> - trgint = 1; >> - if (xtal == HWTRAP_XTAL_25MHZ) >> - ssc_delta = 0x57; >> - else >> - ssc_delta = 0x87; >> - if (priv->id == ID_MT7621) { >> - /* PLL frequency: 125MHz: 1.0GBit */ >> - if (xtal == HWTRAP_XTAL_40MHZ) >> - ncpo1 = 0x0640; >> - if (xtal == HWTRAP_XTAL_25MHZ) >> - ncpo1 = 0x0a00; >> - } else { /* PLL frequency: 250MHz: 2.0Gbit */ >> - if (xtal == HWTRAP_XTAL_40MHZ) >> - ncpo1 = 0x0c80; >> - if (xtal == HWTRAP_XTAL_25MHZ) >> - ncpo1 = 0x1400; >> - } >> - break; >> - default: >> - dev_err(priv->dev, "xMII interface %d not supported\n", >> - interface); >> - return -EINVAL; >> + if (interface == PHY_INTERFACE_MODE_RGMII) { >> + mt7530_rmw(priv, MT7530_P6ECR, P6_INTF_MODE_MASK, >> + P6_INTF_MODE(0)); > > Maybe at least switch off TRGMIICK here because we are sure we don't need it > in the RGMII case, ie: > core_clear(priv, CORE_TRGMII_GSW_CLK_CG, REG_TRGMIICK_EN); > > And that then is another line of code already present just below which > means you could keep variable trgint as it was and return after > switching off TRGMIICK below anyway... > >> + return; >> } >> >> - mt7530_rmw(priv, MT7530_P6ECR, P6_INTF_MODE_MASK, >> - P6_INTF_MODE(trgint)); >> + mt7530_rmw(priv, MT7530_P6ECR, P6_INTF_MODE_MASK, P6_INTF_MODE(1)); >> >> - if (trgint) { >> - /* Disable the MT7530 TRGMII clocks */ >> - core_clear(priv, CORE_TRGMII_GSW_CLK_CG, REG_TRGMIICK_EN); >> + xtal = mt7530_read(priv, MT7530_MHWTRAP) & HWTRAP_XTAL_MASK; >> >> - /* Setup the MT7530 TRGMII Tx Clock */ >> - core_write(priv, CORE_PLL_GROUP5, RG_LCDDS_PCW_NCPO1(ncpo1)); >> - core_write(priv, CORE_PLL_GROUP6, RG_LCDDS_PCW_NCPO0(0)); >> - core_write(priv, CORE_PLL_GROUP10, RG_LCDDS_SSC_DELTA(ssc_delta)); >> - core_write(priv, CORE_PLL_GROUP11, RG_LCDDS_SSC_DELTA1(ssc_delta)); >> - core_write(priv, CORE_PLL_GROUP4, >> - RG_SYSPLL_DDSFBK_EN | RG_SYSPLL_BIAS_EN | >> - RG_SYSPLL_BIAS_LPF_EN); >> - core_write(priv, CORE_PLL_GROUP2, >> - RG_SYSPLL_EN_NORMAL | RG_SYSPLL_VODEN | >> - RG_SYSPLL_POSDIV(1)); >> - core_write(priv, CORE_PLL_GROUP7, >> - RG_LCDDS_PCW_NCPO_CHG | RG_LCCDS_C(3) | >> - RG_LCDDS_PWDB | RG_LCDDS_ISO_EN); >> + if (xtal == HWTRAP_XTAL_25MHZ) >> + ssc_delta = 0x57; >> + else >> + ssc_delta = 0x87; >> >> - /* Enable the MT7530 TRGMII clocks */ >> - core_set(priv, CORE_TRGMII_GSW_CLK_CG, REG_TRGMIICK_EN); >> + if (priv->id == ID_MT7621) { >> + /* PLL frequency: 125MHz: 1.0GBit */ >> + if (xtal == HWTRAP_XTAL_40MHZ) >> + ncpo1 = 0x0640; >> + if (xtal == HWTRAP_XTAL_25MHZ) >> + ncpo1 = 0x0a00; >> + } else { /* PLL frequency: 250MHz: 2.0Gbit */ >> + if (xtal == HWTRAP_XTAL_40MHZ) >> + ncpo1 = 0x0c80; >> + if (xtal == HWTRAP_XTAL_25MHZ) >> + ncpo1 = 0x1400; >> } >> >> - return 0; >> + /* Disable the MT7530 TRGMII clocks */ >> + core_clear(priv, CORE_TRGMII_GSW_CLK_CG, REG_TRGMIICK_EN); > > ... by moving this line up and letting it happen unconditionally for > both RGMII and TRGMII (in case that works and doesn't break the RGMII > case, but I assume it doesn't) I've just tested this, works fine. This looks simpler than bringing back the trgint variable. static void mt7530_setup_port6(struct dsa_switch *ds, phy_interface_t interface) { struct mt7530_priv *priv = ds->priv; u32 ncpo1, ssc_delta, xtal; /* Disable the MT7530 TRGMII clocks */ core_clear(priv, CORE_TRGMII_GSW_CLK_CG, REG_TRGMIICK_EN); if (interface == PHY_INTERFACE_MODE_RGMII) { mt7530_rmw(priv, MT7530_P6ECR, P6_INTF_MODE_MASK, P6_INTF_MODE(0)); return; } mt7530_rmw(priv, MT7530_P6ECR, P6_INTF_MODE_MASK, P6_INTF_MODE(1)); ... Arınç
On Tue, Jan 30, 2024 at 08:46:04PM +0300, Arınç ÜNAL wrote: > would supposedly achieve 2 Gbps TX & 2 Gbps RX Source? Commit 8efaa653a8a5 ("net: ethernet: mediatek: Add MT7621 TRGMII mode support") says "TRGMII speed is 1200MBit.". > Unless the MediaTek SoC ethernet driver somehow caps TRGMII to 1 Gbps, > I consider this whole TRGMII shenanigans a scam I laughed :) You have to see whether the CPU isn't in fact at 100% already, becoming a bottleneck before the interface speed does. Also, mtk_eth_soc.c has an interesting comment "TRGMII is not permitted on MT7621 if using DDR2" - not sure if applicable to your setup or not. I just got myself an ASUS RT-AX1800U (uses the mt7621_asus_rt-ax53u.dts device tree AFAICT) which I'll be setting up with OpenWrt in the weeks to come, and on which I might also be able to run some tests from time to time.
On 2.02.2024 02:57, Vladimir Oltean wrote: > On Tue, Jan 30, 2024 at 08:46:04PM +0300, Arınç ÜNAL wrote: >> would supposedly achieve 2 Gbps TX & 2 Gbps RX > > Source? Commit 8efaa653a8a5 ("net: ethernet: mediatek: Add MT7621 TRGMII > mode support") says "TRGMII speed is 1200MBit.". That is for MT7621. It's claimed that TRGMII on MT7621 can only handle that much. I already told you I'm doing the test on MT7623NI SoC. MT7623 is ARM and more powerful. On that one, the PLL frequency can be set all the way up to 362.5 MHz to provide 2900 Mbps (allegedly). You can check the repository that the commit above links to for more details: https://github.com/BPI-SINOVOIP/BPI-R2-bsp/blob/591910e127cd9c811fe9e811ddb6c7278d8ed934/linux-mt/drivers/net/ethernet/raeth/Kconfig#L141 https://github.com/BPI-SINOVOIP/BPI-R2-bsp/blob/591910e127cd9c811fe9e811ddb6c7278d8ed934/linux-mt/drivers/net/ethernet/raeth/raeth_config.h#L201 https://github.com/BPI-SINOVOIP/BPI-R2-bsp/blob/591910e127cd9c811fe9e811ddb6c7278d8ed934/u-boot-mt/drivers/net/rt2880_eth.c#L2178 > >> Unless the MediaTek SoC ethernet driver somehow caps TRGMII to 1 Gbps, >> I consider this whole TRGMII shenanigans a scam > > I laughed :) > > You have to see whether the CPU isn't in fact at 100% already, becoming > a bottleneck before the interface speed does. I'm happy I'm entertaining you but you've got to give me a little credit. :) MT7621 won't even handle 1 Gbps RX & 1 Gbps TX. But if the IP traffic is offloaded to the packet processing engine (drivers/net/ethernet/mediatek/mtk_ppe_offload.c), there won't be any load on the CPU. table ip global { flowtable f { hook ingress priority 0 devices = { wan, lan1, lan2, lan3, lan4 } flags offload } chain forward { type filter hook forward priority 0 ip protocol { tcp, udp } flow offload @f } chain postrouting { type nat hook postrouting priority 0 oifname "wan" masquerade } } MT7623 can handle 1 Gbps RX & 1 Gbps without much CPU load. It performs the same with or without hardware flow offloading, unlike MT7621. The way I test this: I do the test on a single computer. I have two gigabit ports on my motherboard. I isolate a port by putting it on another network namespace to do the test. Client Network iperf client: 192.168.2.2/24 router: 192.168.2.1/24 Server Network router: 192.168.3.2/24 iperf server: 192.168.3.1/24 iperf Client ip a add 192.168.2.2/24 dev enp9s0 ip l set up enp9s0 ip route add 192.168.3.1 via 192.168.2.1 iperf3 -c 192.168.3.1 --bidir -t 20 iperf Server ip netns add iperfserver ip link set dev eno1 netns iperfserver ip netns exec iperfserver ip a add 192.168.3.1/24 dev eno1 ip netns exec iperfserver ip l set up eno1 ip netns exec iperfserver iperf3 -s I did say I've done thorough testing. > > Also, mtk_eth_soc.c has an interesting comment "TRGMII is not permitted > on MT7621 if using DDR2" - not sure if applicable to your setup or not. My device has DDR3 memory. Also, with a device tree defining trgmii on a link of MediaTek SoC MAC, that check should prevent the mtk_eth_soc driver from configuring the MAC if the device has DDR2 memory, no? > > I just got myself an ASUS RT-AX1800U (uses the mt7621_asus_rt-ax53u.dts > device tree AFAICT) which I'll be setting up with OpenWrt in the weeks > to come, and on which I might also be able to run some tests from time > to time. Doing tests on MT7621 will be useless without utilising the PPE. To use it, you can add these to /etc/config/firewall: config defaults ... option flow_offloading '1' option flow_offloading_hw '1' Or enable software flow offloading and hardware flow offloading options if using LuCI. When both options are enabled, hardware flow offloading will be used. Make sure to change the PLL frequency on the MT7530 side to 150 MHz. It operates at the standard RGMII frequency since commit 37c218d8021e ("net: dsa: mt7530: fix corrupt frames using trgmii on 40 MHz XTAL MT7621"). For 40MHz XTAL: 0x0640 x 0d1,2 = 0x0780 For 25MHz XTAL: 0x0a00 x 0d1,2 = 0x0c00 Arınç
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c index c4d492e29fdf..36dc2bbcf3b6 100644 --- a/drivers/net/dsa/mt7530.c +++ b/drivers/net/dsa/mt7530.c @@ -414,70 +414,57 @@ mt753x_preferred_default_local_cpu_port(struct dsa_switch *ds) } /* Setup port 6 interface mode and TRGMII TX circuit */ -static int +static void mt7530_setup_port6(struct dsa_switch *ds, phy_interface_t interface) { struct mt7530_priv *priv = ds->priv; - u32 ncpo1, ssc_delta, trgint, xtal; - - xtal = mt7530_read(priv, MT7530_MHWTRAP) & HWTRAP_XTAL_MASK; + u32 ncpo1, ssc_delta, xtal; - switch (interface) { - case PHY_INTERFACE_MODE_RGMII: - trgint = 0; - break; - case PHY_INTERFACE_MODE_TRGMII: - trgint = 1; - if (xtal == HWTRAP_XTAL_25MHZ) - ssc_delta = 0x57; - else - ssc_delta = 0x87; - if (priv->id == ID_MT7621) { - /* PLL frequency: 125MHz: 1.0GBit */ - if (xtal == HWTRAP_XTAL_40MHZ) - ncpo1 = 0x0640; - if (xtal == HWTRAP_XTAL_25MHZ) - ncpo1 = 0x0a00; - } else { /* PLL frequency: 250MHz: 2.0Gbit */ - if (xtal == HWTRAP_XTAL_40MHZ) - ncpo1 = 0x0c80; - if (xtal == HWTRAP_XTAL_25MHZ) - ncpo1 = 0x1400; - } - break; - default: - dev_err(priv->dev, "xMII interface %d not supported\n", - interface); - return -EINVAL; + if (interface == PHY_INTERFACE_MODE_RGMII) { + mt7530_rmw(priv, MT7530_P6ECR, P6_INTF_MODE_MASK, + P6_INTF_MODE(0)); + return; } - mt7530_rmw(priv, MT7530_P6ECR, P6_INTF_MODE_MASK, - P6_INTF_MODE(trgint)); + mt7530_rmw(priv, MT7530_P6ECR, P6_INTF_MODE_MASK, P6_INTF_MODE(1)); - if (trgint) { - /* Disable the MT7530 TRGMII clocks */ - core_clear(priv, CORE_TRGMII_GSW_CLK_CG, REG_TRGMIICK_EN); + xtal = mt7530_read(priv, MT7530_MHWTRAP) & HWTRAP_XTAL_MASK; - /* Setup the MT7530 TRGMII Tx Clock */ - core_write(priv, CORE_PLL_GROUP5, RG_LCDDS_PCW_NCPO1(ncpo1)); - core_write(priv, CORE_PLL_GROUP6, RG_LCDDS_PCW_NCPO0(0)); - core_write(priv, CORE_PLL_GROUP10, RG_LCDDS_SSC_DELTA(ssc_delta)); - core_write(priv, CORE_PLL_GROUP11, RG_LCDDS_SSC_DELTA1(ssc_delta)); - core_write(priv, CORE_PLL_GROUP4, - RG_SYSPLL_DDSFBK_EN | RG_SYSPLL_BIAS_EN | - RG_SYSPLL_BIAS_LPF_EN); - core_write(priv, CORE_PLL_GROUP2, - RG_SYSPLL_EN_NORMAL | RG_SYSPLL_VODEN | - RG_SYSPLL_POSDIV(1)); - core_write(priv, CORE_PLL_GROUP7, - RG_LCDDS_PCW_NCPO_CHG | RG_LCCDS_C(3) | - RG_LCDDS_PWDB | RG_LCDDS_ISO_EN); + if (xtal == HWTRAP_XTAL_25MHZ) + ssc_delta = 0x57; + else + ssc_delta = 0x87; - /* Enable the MT7530 TRGMII clocks */ - core_set(priv, CORE_TRGMII_GSW_CLK_CG, REG_TRGMIICK_EN); + if (priv->id == ID_MT7621) { + /* PLL frequency: 125MHz: 1.0GBit */ + if (xtal == HWTRAP_XTAL_40MHZ) + ncpo1 = 0x0640; + if (xtal == HWTRAP_XTAL_25MHZ) + ncpo1 = 0x0a00; + } else { /* PLL frequency: 250MHz: 2.0Gbit */ + if (xtal == HWTRAP_XTAL_40MHZ) + ncpo1 = 0x0c80; + if (xtal == HWTRAP_XTAL_25MHZ) + ncpo1 = 0x1400; } - return 0; + /* Disable the MT7530 TRGMII clocks */ + core_clear(priv, CORE_TRGMII_GSW_CLK_CG, REG_TRGMIICK_EN); + + /* Setup the MT7530 TRGMII Tx Clock */ + core_write(priv, CORE_PLL_GROUP5, RG_LCDDS_PCW_NCPO1(ncpo1)); + core_write(priv, CORE_PLL_GROUP6, RG_LCDDS_PCW_NCPO0(0)); + core_write(priv, CORE_PLL_GROUP10, RG_LCDDS_SSC_DELTA(ssc_delta)); + core_write(priv, CORE_PLL_GROUP11, RG_LCDDS_SSC_DELTA1(ssc_delta)); + core_write(priv, CORE_PLL_GROUP4, RG_SYSPLL_DDSFBK_EN | + RG_SYSPLL_BIAS_EN | RG_SYSPLL_BIAS_LPF_EN); + core_write(priv, CORE_PLL_GROUP2, RG_SYSPLL_EN_NORMAL | + RG_SYSPLL_VODEN | RG_SYSPLL_POSDIV(1)); + core_write(priv, CORE_PLL_GROUP7, RG_LCDDS_PCW_NCPO_CHG | + RG_LCCDS_C(3) | RG_LCDDS_PWDB | RG_LCDDS_ISO_EN); + + /* Enable the MT7530 TRGMII clocks */ + core_set(priv, CORE_TRGMII_GSW_CLK_CG, REG_TRGMIICK_EN); } static void @@ -2609,15 +2596,11 @@ mt7530_mac_config(struct dsa_switch *ds, int port, unsigned int mode, phy_interface_t interface) { struct mt7530_priv *priv = ds->priv; - int ret; - if (port == 5) { + if (port == 5) mt7530_setup_port5(priv->ds, interface); - } else if (port == 6) { - ret = mt7530_setup_port6(priv->ds, interface); - if (ret) - return ret; - } + else if (port == 6) + mt7530_setup_port6(priv->ds, interface); return 0; }