[PATCH net 1/3] net: dsa: mt7530: move enabling disabling core clock to mt7530_pll_setup()

[PATCH net 1/3] net: dsa: mt7530: move enabling disabling core clock to mt7530_pll_setup()

[arinc9.unal]

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

Split the code that enables and disables TRGMII clocks and core clock.
Move enabling and disabling core clock to mt7530_pll_setup() as it's
supposed to be run there.

Add 20 ms delay before enabling the core clock as seen on the U-Boot
MediaTek ethernet driver.

Change the comment for enabling and disabling TRGMII clocks as the code
seems to affect both TXC and RXC.

Tested rgmii and trgmii modes of port 6 and rgmii mode of port 5 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")
Link: https://source.denx.de/u-boot/u-boot/-/blob/29a48bf9ccba45a5e560bb564bbe76e42629325f/drivers/net/mtk_eth.c#L589
Tested-by: Arınç ÜNAL <arinc.unal@arinc9.com>
Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
---
 drivers/net/dsa/mt7530.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index c2d81b7a429d..d4a559007973 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -396,6 +396,9 @@ mt7530_fdb_write(struct mt7530_priv *priv, u16 vid,
 /* Set up switch core clock for MT7530 */
 static void mt7530_pll_setup(struct mt7530_priv *priv)
 {
+	/* Disable core clock */
+	core_clear(priv, CORE_TRGMII_GSW_CLK_CG, REG_GSWCK_EN);
+
 	/* Disable PLL */
 	core_write(priv, CORE_GSWPLL_GRP1, 0);
 
@@ -409,6 +412,11 @@ static void mt7530_pll_setup(struct mt7530_priv *priv)
 		   RG_GSWPLL_EN_PRE |
 		   RG_GSWPLL_POSDIV_200M(2) |
 		   RG_GSWPLL_FBKDIV_200M(32));
+
+	udelay(20);
+
+	/* Enable core clock */
+	core_set(priv, CORE_TRGMII_GSW_CLK_CG, REG_GSWCK_EN);
 }
 
 /* Setup TX circuit including relevant PAD and driving */
@@ -466,9 +474,8 @@ mt7530_pad_clk_setup(struct dsa_switch *ds, phy_interface_t interface)
 			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);
+		/* 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));
@@ -485,9 +492,8 @@ mt7530_pad_clk_setup(struct dsa_switch *ds, phy_interface_t interface)
 			   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);
+		/* Enable the MT7530 TRGMII clocks */
+		core_set(priv, CORE_TRGMII_GSW_CLK_CG, REG_TRGMIICK_EN);
 	} else {
 		for (i = 0 ; i < NUM_TRGMII_CTRL; i++)
 			mt7530_rmw(priv, MT7530_TRGMII_RD(i),
-- 
2.37.2

[PATCH net 2/3] net: dsa: mt7530: move lowering TRGMII driving to mt7530_setup()

[arinc9.unal]

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

Move lowering the TRGMII Tx clock driving to mt7530_setup(), after setting
the core clock, as seen on the U-Boot MediaTek ethernet driver.

Move the code which looks like it lowers the TRGMII Rx clock driving to
after the TRGMII Tx clock driving is lowered. This is run after lowering
the Tx clock driving on the U-Boot MediaTek ethernet driver as well.

This way, the switch should consume less power regardless of port 6 being
used.

Update the comment explaining mt7530_pad_clk_setup().

Tested rgmii and trgmii modes of port 6 and rgmii mode of port 5 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")
Link: https://source.denx.de/u-boot/u-boot/-/blob/29a48bf9ccba45a5e560bb564bbe76e42629325f/drivers/net/mtk_eth.c#L682
Tested-by: Arınç ÜNAL <arinc.unal@arinc9.com>
Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
---

I asked this before, MT7530 DSA driver maintainers, please explain the code
I mentioned on the second paragraph.

I intend to send a patch to remove the maintainers, Sean Wang, Landen Chao
DENG Qingfang, listed on the MAINTAINERS file and change the status to
orphan if none of them respond to this question or review the patches.

I think a full week is a reasonable amount of time to receive a response
from a maintainer.

Arınç

---
 drivers/net/dsa/mt7530.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index d4a559007973..8831bd409a40 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -419,12 +419,12 @@ static void mt7530_pll_setup(struct mt7530_priv *priv)
 	core_set(priv, CORE_TRGMII_GSW_CLK_CG, REG_GSWCK_EN);
 }
 
-/* Setup TX circuit including relevant PAD and driving */
+/* Setup port 6 interface mode and TRGMII TX circuit */
 static int
 mt7530_pad_clk_setup(struct dsa_switch *ds, phy_interface_t interface)
 {
 	struct mt7530_priv *priv = ds->priv;
-	u32 ncpo1, ssc_delta, trgint, i, xtal;
+	u32 ncpo1, ssc_delta, trgint, xtal;
 
 	xtal = mt7530_read(priv, MT7530_MHWTRAP) & HWTRAP_XTAL_MASK;
 
@@ -469,11 +469,6 @@ mt7530_pad_clk_setup(struct dsa_switch *ds, phy_interface_t interface)
 		   P6_INTF_MODE(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 the MT7530 TRGMII clocks */
 		core_clear(priv, CORE_TRGMII_GSW_CLK_CG, REG_TRGMIICK_EN);
 
@@ -494,10 +489,6 @@ mt7530_pad_clk_setup(struct dsa_switch *ds, phy_interface_t interface)
 
 		/* Enable the MT7530 TRGMII clocks */
 		core_set(priv, CORE_TRGMII_GSW_CLK_CG, 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;
@@ -2207,6 +2198,15 @@ mt7530_setup(struct dsa_switch *ds)
 
 	mt7530_pll_setup(priv);
 
+	/* 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));
+
+	for (i = 0; i < NUM_TRGMII_CTRL; i++)
+		mt7530_rmw(priv, MT7530_TRGMII_RD(i),
+			   RD_TAP_MASK, RD_TAP(16));
+
 	/* Enable port 6 */
 	val = mt7530_read(priv, MT7530_MHWTRAP);
 	val &= ~MHWTRAP_P6_DIS & ~MHWTRAP_PHY_ACCESS;
-- 
2.37.2

[PATCH net 3/3] net: dsa: mt7530: move setting ssc_delta to PHY_INTERFACE_MODE_TRGMII case

[arinc9.unal]

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

Move setting the ssc_delta variable to under the PHY_INTERFACE_MODE_TRGMII
case as it's only needed when trgmii is used.

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

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 8831bd409a40..02410ac439b7 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -441,6 +441,10 @@ mt7530_pad_clk_setup(struct dsa_switch *ds, phy_interface_t interface)
 		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: 150MHz: 1.2GBit */
 			if (xtal == HWTRAP_XTAL_40MHZ)
@@ -460,11 +464,6 @@ mt7530_pad_clk_setup(struct dsa_switch *ds, phy_interface_t interface)
 		return -EINVAL;
 	}
 
-	if (xtal == HWTRAP_XTAL_25MHZ)
-		ssc_delta = 0x57;
-	else
-		ssc_delta = 0x87;
-
 	mt7530_rmw(priv, MT7530_P6ECR, P6_INTF_MODE_MASK,
 		   P6_INTF_MODE(trgint));
 
-- 
2.37.2

Re: [PATCH net 2/3] net: dsa: mt7530: move lowering TRGMII driving to mt7530_setup()

[Jakub Kicinski]

On Mon, 20 Mar 2023 22:05:19 +0300 arinc9.unal@gmail.com wrote:
> I asked this before, MT7530 DSA driver maintainers, please explain the code
> I mentioned on the second paragraph.
> 
> I intend to send a patch to remove the maintainers, Sean Wang, Landen Chao
> DENG Qingfang, listed on the MAINTAINERS file and change the status to
> orphan if none of them respond to this question or review the patches.

Sounds fair. 

Re: [PATCH net 1/3] net: dsa: mt7530: move enabling disabling core clock to mt7530_pll_setup()

[patchwork-bot+netdevbpf]

Hello:

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

On Mon, 20 Mar 2023 22:05:18 +0300 you wrote:
> From: Arınç ÜNAL <arinc.unal@arinc9.com>
> 
> Split the code that enables and disables TRGMII clocks and core clock.
> Move enabling and disabling core clock to mt7530_pll_setup() as it's
> supposed to be run there.
> 
> Add 20 ms delay before enabling the core clock as seen on the U-Boot
> MediaTek ethernet driver.
> 
> [...]

Here is the summary with links:
  - [net,1/3] net: dsa: mt7530: move enabling disabling core clock to mt7530_pll_setup()
    https://git.kernel.org/netdev/net/c/8f058a6ef99f
  - [net,2/3] net: dsa: mt7530: move lowering TRGMII driving to mt7530_setup()
    https://git.kernel.org/netdev/net/c/fdcc8ccd8237
  - [net,3/3] net: dsa: mt7530: move setting ssc_delta to PHY_INTERFACE_MODE_TRGMII case
    https://git.kernel.org/netdev/net/c/407b508bdd70

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

Re: [PATCH net 2/3] net: dsa: mt7530: move lowering TRGMII driving to mt7530_setup()

[Landen Chao (趙皎宏)]

On Mon, 2023-03-20 at 22:05 +0300, arinc9.unal@gmail.com wrote:
> 
> I asked this before, MT7530 DSA driver maintainers, please explain
> the code
> I mentioned on the second paragraph.
> 
> ---
> @@ -2207,6 +2198,15 @@ mt7530_setup(struct dsa_switch *ds)
> 
>         mt7530_pll_setup(priv);
> 
> +       /* 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));
> +
I guess you ask this part, and I try my best to recall what the
original author said years ago.
It is used to adjust the RX delay of port 6 to match the tx
signal of the link partner.
> +       for (i = 0; i < NUM_TRGMII_CTRL; i++)
> +               mt7530_rmw(priv, MT7530_TRGMII_RD(i),
> +                          RD_TAP_MASK, RD_TAP(16));
> +
> 
> 

Re: [PATCH net 2/3] net: dsa: mt7530: move lowering TRGMII driving to mt7530_setup()

[Arınç ÜNAL]

On 26 March 2023 21:13:49 GMT+03:00, "Landen Chao (趙皎宏)" <Landen.Chao@mediatek.com> wrote:
>On Mon, 2023-03-20 at 22:05 +0300, arinc9.unal@gmail.com wrote:
>> 
>> I asked this before, MT7530 DSA driver maintainers, please explain
>> the code
>> I mentioned on the second paragraph.
>> 
>> ---
>> @@ -2207,6 +2198,15 @@ mt7530_setup(struct dsa_switch *ds)
>> 
>>         mt7530_pll_setup(priv);
>> 
>> +       /* 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));
>> +
>I guess you ask this part, and I try my best to recall what the
>original author said years ago.
>It is used to adjust the RX delay of port 6 to match the tx
>signal of the link partner.

Ok, thanks for replying. I will move this at the end, inside 'if (trgint)'. Since this doesn't lower the driving, there's no apparent benefit to run this if trgmii is not being used.

Arınç