[PATCH v3 net 1/2] net: dsa: mt7530: remove now incorrect comment regarding port 5

[PATCH v3 net 1/2] net: dsa: mt7530: remove now incorrect comment regarding port 5

[arinc9.unal]

From: Arınç ÜNAL <arinc.unal@arinc9.com>

Remove now incorrect comment regarding port 5 as GMAC5. This is supposed to
be supported since commit 38f790a80560 ("net: dsa: mt7530: Add support for
port 5") under mt7530_setup_port5().

Fixes: 38f790a80560 ("net: dsa: mt7530: Add support for port 5")
Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
---

v3: Resend so the bot can test it now.

---
 drivers/net/dsa/mt7530.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index a508402c4ecb..b1a79460df0e 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -2201,7 +2201,7 @@ mt7530_setup(struct dsa_switch *ds)
 
 	mt7530_pll_setup(priv);
 
-	/* Enable Port 6 only; P5 as GMAC5 which currently is not supported */
+	/* Enable port 6 */
 	val = mt7530_read(priv, MT7530_MHWTRAP);
 	val &= ~MHWTRAP_P6_DIS & ~MHWTRAP_PHY_ACCESS;
 	val |= MHWTRAP_MANUAL;
-- 
2.37.2

[PATCH v3 net 2/2] net: dsa: mt7530: set PLL frequency and trgmii only when trgmii is used

[arinc9.unal]

From: Arınç ÜNAL <arinc.unal@arinc9.com>

As my testing on the MCM MT7530 switch on MT7621 SoC shows, setting the PLL
frequency does not affect MII modes other than trgmii on port 5 and port 6.
So the assumption is that the operation here called "setting the PLL
frequency" actually sets the frequency of the TRGMII TX clock.

Make it so that it and the rest of the trgmii setup run only when the
trgmii mode is used.

Tested rgmii and trgmii modes of port 6 on MCM MT7530 on MT7621AT Unielec
U7621-06 and standalone MT7530 on MT7623NI Bananapi BPI-R2.

Fixes: b8f126a8d543 ("net-next: dsa: add dsa support for Mediatek MT7530 switch")
Tested-by: Arınç ÜNAL <arinc.unal@arinc9.com>
Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
---
 drivers/net/dsa/mt7530.c | 62 ++++++++++++++++++++--------------------
 1 file changed, 31 insertions(+), 31 deletions(-)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index b1a79460df0e..c2d81b7a429d 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -430,8 +430,6 @@ mt7530_pad_clk_setup(struct dsa_switch *ds, phy_interface_t interface)
 	switch (interface) {
 	case PHY_INTERFACE_MODE_RGMII:
 		trgint = 0;
-		/* PLL frequency: 125MHz */
-		ncpo1 = 0x0c80;
 		break;
 	case PHY_INTERFACE_MODE_TRGMII:
 		trgint = 1;
@@ -462,38 +460,40 @@ mt7530_pad_clk_setup(struct dsa_switch *ds, phy_interface_t interface)
 	mt7530_rmw(priv, MT7530_P6ECR, P6_INTF_MODE_MASK,
 		   P6_INTF_MODE(trgint));
 
-	/* Lower Tx Driving for TRGMII path */
-	for (i = 0 ; i < NUM_TRGMII_CTRL ; i++)
-		mt7530_write(priv, MT7530_TRGMII_TD_ODT(i),
-			     TD_DM_DRVP(8) | TD_DM_DRVN(8));
-
-	/* Disable MT7530 core and TRGMII Tx clocks */
-	core_clear(priv, CORE_TRGMII_GSW_CLK_CG,
-		   REG_GSWCK_EN | 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 MT7530 core and TRGMII Tx clocks */
-	core_set(priv, CORE_TRGMII_GSW_CLK_CG,
-		 REG_GSWCK_EN | REG_TRGMIICK_EN);
-
-	if (!trgint)
+	if (trgint) {
+		/* Lower Tx Driving for TRGMII path */
+		for (i = 0 ; i < NUM_TRGMII_CTRL ; i++)
+			mt7530_write(priv, MT7530_TRGMII_TD_ODT(i),
+				     TD_DM_DRVP(8) | TD_DM_DRVN(8));
+
+		/* Disable MT7530 core and TRGMII Tx clocks */
+		core_clear(priv, CORE_TRGMII_GSW_CLK_CG,
+			   REG_GSWCK_EN | 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 MT7530 core and TRGMII Tx clocks */
+		core_set(priv, CORE_TRGMII_GSW_CLK_CG,
+			 REG_GSWCK_EN | REG_TRGMIICK_EN);
+	} else {
 		for (i = 0 ; i < NUM_TRGMII_CTRL; i++)
 			mt7530_rmw(priv, MT7530_TRGMII_RD(i),
 				   RD_TAP_MASK, RD_TAP(16));
+	}
+
 	return 0;
 }
 
-- 
2.37.2

Re: [PATCH v3 net 2/2] net: dsa: mt7530: set PLL frequency and trgmii only when trgmii is used

[Arınç ÜNAL]

On 10.03.2023 10:33, arinc9.unal@gmail.com wrote:
> From: Arınç ÜNAL <arinc.unal@arinc9.com>
> 
> As my testing on the MCM MT7530 switch on MT7621 SoC shows, setting the PLL
> frequency does not affect MII modes other than trgmii on port 5 and port 6.
> So the assumption is that the operation here called "setting the PLL
> frequency" actually sets the frequency of the TRGMII TX clock.
> 
> Make it so that it and the rest of the trgmii setup run only when the
> trgmii mode is used.
> 
> Tested rgmii and trgmii modes of port 6 on MCM MT7530 on MT7621AT Unielec
> U7621-06 and standalone MT7530 on MT7623NI Bananapi BPI-R2.
> 
> Fixes: b8f126a8d543 ("net-next: dsa: add dsa support for Mediatek MT7530 switch")
> Tested-by: Arınç ÜNAL <arinc.unal@arinc9.com>
> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
> ---
>   drivers/net/dsa/mt7530.c | 62 ++++++++++++++++++++--------------------
>   1 file changed, 31 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
> index b1a79460df0e..c2d81b7a429d 100644
> --- a/drivers/net/dsa/mt7530.c
> +++ b/drivers/net/dsa/mt7530.c
> @@ -430,8 +430,6 @@ mt7530_pad_clk_setup(struct dsa_switch *ds, phy_interface_t interface)
>   	switch (interface) {
>   	case PHY_INTERFACE_MODE_RGMII:
>   		trgint = 0;
> -		/* PLL frequency: 125MHz */
> -		ncpo1 = 0x0c80;
>   		break;
>   	case PHY_INTERFACE_MODE_TRGMII:
>   		trgint = 1;
> @@ -462,38 +460,40 @@ mt7530_pad_clk_setup(struct dsa_switch *ds, phy_interface_t interface)
>   	mt7530_rmw(priv, MT7530_P6ECR, P6_INTF_MODE_MASK,
>   		   P6_INTF_MODE(trgint));
>   
> -	/* Lower Tx Driving for TRGMII path */
> -	for (i = 0 ; i < NUM_TRGMII_CTRL ; i++)
> -		mt7530_write(priv, MT7530_TRGMII_TD_ODT(i),
> -			     TD_DM_DRVP(8) | TD_DM_DRVN(8));
> -
> -	/* Disable MT7530 core and TRGMII Tx clocks */
> -	core_clear(priv, CORE_TRGMII_GSW_CLK_CG,
> -		   REG_GSWCK_EN | 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 MT7530 core and TRGMII Tx clocks */
> -	core_set(priv, CORE_TRGMII_GSW_CLK_CG,
> -		 REG_GSWCK_EN | REG_TRGMIICK_EN);
> -
> -	if (!trgint)
> +	if (trgint) {
> +		/* Lower Tx Driving for TRGMII path */
> +		for (i = 0 ; i < NUM_TRGMII_CTRL ; i++)
> +			mt7530_write(priv, MT7530_TRGMII_TD_ODT(i),
> +				     TD_DM_DRVP(8) | TD_DM_DRVN(8));
> +
> +		/* Disable MT7530 core and TRGMII Tx clocks */
> +		core_clear(priv, CORE_TRGMII_GSW_CLK_CG,
> +			   REG_GSWCK_EN | 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 MT7530 core and TRGMII Tx clocks */
> +		core_set(priv, CORE_TRGMII_GSW_CLK_CG,
> +			 REG_GSWCK_EN | REG_TRGMIICK_EN);
> +	} else {
>   		for (i = 0 ; i < NUM_TRGMII_CTRL; i++)
>   			mt7530_rmw(priv, MT7530_TRGMII_RD(i),
>   				   RD_TAP_MASK, RD_TAP(16));

This code runs if the phy mode is not trgmii. Other than trgmii, only 
the rgmii mode is supported on the hardware so this runs when the rgmii 
mode is used on port 6.

I've tested the rgmii mode on MCM and standalone MT7530 without running 
this code and it works fine. Close to gigabit download/upload speed and 
no packet loss. I don't understand why the TRGMII RX registers are 
modified when the trgmii mode is not used at all.

I don't suppose anyone from MediaTek would clarify, so this presumably 
dead code will remain.

Arınç

Re: [PATCH v3 net 1/2] net: dsa: mt7530: remove now incorrect comment regarding port 5

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Fri, 10 Mar 2023 10:33:37 +0300 you wrote:
> From: Arınç ÜNAL <arinc.unal@arinc9.com>
> 
> Remove now incorrect comment regarding port 5 as GMAC5. This is supposed to
> be supported since commit 38f790a80560 ("net: dsa: mt7530: Add support for
> port 5") under mt7530_setup_port5().
> 
> Fixes: 38f790a80560 ("net: dsa: mt7530: Add support for port 5")
> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
> 
> [...]

Here is the summary with links:
  - [v3,net,1/2] net: dsa: mt7530: remove now incorrect comment regarding port 5
    https://git.kernel.org/netdev/net/c/feb03fd11c56
  - [v3,net,2/2] net: dsa: mt7530: set PLL frequency and trgmii only when trgmii is used
    https://git.kernel.org/netdev/net/c/0b086d76e7b0

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html