[PATCH 1/3] net: dsa: mt7530: remove redundant clock enables

Subsystems: mediatek switch driver, networking drivers, networking [dsa], the rest

STALE1968d

9 messages, 4 authors, 2021-03-11 · open the first message on its own page

[PATCH 1/3] net: dsa: mt7530: remove redundant clock enables

From: Ilya Lipnitskiy <hidden>
Date: 2021-03-10 21:15:12

In RGMII mode, the REG_GSWCK_EN bit of CORE_TRGMII_GSW_CLK_CG gets
set three times in a row. In TRGMII mode, two times. Simplify the code
and only set it once for both modes.

Signed-off-by: Ilya Lipnitskiy <redacted>
---
 drivers/net/dsa/mt7530.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index f06f5fa2f898..e785f80f966b 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -461,12 +461,9 @@ mt7530_pad_clk_setup(struct dsa_switch *ds, phy_interface_t interface)
 			   RG_GSWPLL_POSDIV_200M(2) |
 			   RG_GSWPLL_FBKDIV_200M(32));
 
-		/* Enable MT7530 core clock */
-		core_set(priv, CORE_TRGMII_GSW_CLK_CG, REG_GSWCK_EN);
 	}
 
 	/* Setup the MT7530 TRGMII Tx Clock */
-	core_set(priv, CORE_TRGMII_GSW_CLK_CG, REG_GSWCK_EN);
 	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));
@@ -480,6 +477,8 @@ mt7530_pad_clk_setup(struct dsa_switch *ds, phy_interface_t interface)
 	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);
 
-- 
2.30.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/3] net: dsa: mt7530: setup core clock even in TRGMII mode

From: Ilya Lipnitskiy <hidden>
Date: 2021-03-10 21:15:12

3f9ef7785a9c ("MIPS: ralink: manage low reset lines") made it so mt7530
actually resets the switch on platforms such as mt7621 (where bit 2 is
the reset line for the switch). That exposed an issue where the switch
would not function properly in TRGMII mode after a reset.

Reconfigure core clock in TRGMII mode to fix the issue.

Also, disable both core and TRGMII Tx clocks prior to reconfiguring.
Previously, only the core clock was disabled, but not TRGMII Tx clock.

Tested on Ubiquity ER-X (MT7621) with TRGMII mode enabled.

Signed-off-by: Ilya Lipnitskiy <redacted>
---
 drivers/net/dsa/mt7530.c | 44 ++++++++++++++++++++--------------------
 1 file changed, 22 insertions(+), 22 deletions(-)
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index b106ea816778..7ef5e7c23e05 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -435,30 +435,30 @@ 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));
 
-	/* Setup core clock for MT7530 */
-	if (!trgint) {
-		/* Disable MT7530 core clock */
-		core_clear(priv, CORE_TRGMII_GSW_CLK_CG, REG_GSWCK_EN);
-
-		/* Disable PLL, since phy_device has not yet been created
-		 * provided for phy_[read,write]_mmd_indirect is called, we
-		 * provide our own core_write_mmd_indirect to complete this
-		 * function.
-		 */
-		core_write(priv, CORE_GSWPLL_GRP1, 0);
-
-		/* Set core clock into 500Mhz */
-		core_write(priv, CORE_GSWPLL_GRP2,
-			   RG_GSWPLL_POSDIV_500M(1) |
-			   RG_GSWPLL_FBKDIV_500M(25));
+	/* Since phy_device has not yet been created and
+	 * phy_[read,write]_mmd_indirect is not available, we provide our own
+	 * core_write_mmd_indirect with core_{clear,write,set} wrappers to
+	 * complete this function.
+	 */
 
-		/* Enable PLL */
-		core_write(priv, CORE_GSWPLL_GRP1,
-			   RG_GSWPLL_EN_PRE |
-			   RG_GSWPLL_POSDIV_200M(2) |
-			   RG_GSWPLL_FBKDIV_200M(32));
+	/* Disable MT7530 core and TRGMII Tx clocks */
+	core_clear(priv, CORE_TRGMII_GSW_CLK_CG,
+		   REG_GSWCK_EN | REG_TRGMIICK_EN);
 
-	}
+	/* Setup core clock for MT7530 */
+	/* Disable PLL */
+	core_write(priv, CORE_GSWPLL_GRP1, 0);
+
+	/* Set core clock into 500Mhz */
+	core_write(priv, CORE_GSWPLL_GRP2,
+		   RG_GSWPLL_POSDIV_500M(1) |
+		   RG_GSWPLL_FBKDIV_500M(25));
+
+	/* Enable PLL */
+	core_write(priv, CORE_GSWPLL_GRP1,
+		   RG_GSWPLL_EN_PRE |
+		   RG_GSWPLL_POSDIV_200M(2) |
+		   RG_GSWPLL_FBKDIV_200M(32));
 
 	/* Setup the MT7530 TRGMII Tx Clock */
 	core_write(priv, CORE_PLL_GROUP5, RG_LCDDS_PCW_NCPO1(ncpo1));
-- 
2.30.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/3] net: dsa: mt7530: setup core clock even in TRGMII mode

From: Vladimir Oltean <olteanv@gmail.com>
Date: 2021-03-10 23:11:02

Hello Ilya,

On Wed, Mar 10, 2021 at 01:14:20PM -0800, Ilya Lipnitskiy wrote:
3f9ef7785a9c ("MIPS: ralink: manage low reset lines") made it so mt7530
actually resets the switch on platforms such as mt7621 (where bit 2 is
the reset line for the switch). That exposed an issue where the switch
would not function properly in TRGMII mode after a reset.

Reconfigure core clock in TRGMII mode to fix the issue.

Also, disable both core and TRGMII Tx clocks prior to reconfiguring.
Previously, only the core clock was disabled, but not TRGMII Tx clock.

Tested on Ubiquity ER-X (MT7621) with TRGMII mode enabled.

Signed-off-by: Ilya Lipnitskiy <redacted>
---
For the networking subsystem there are two git trees, "net" for bugfixes
and "net-next" for new features, and we specify the target tree using
git send-email --subject-prefix="PATCH net-next".

I assume you would like the v5.12 kernel to actually be functional on
the Ubiquiti ER-X switch, so I would recommend keeping this patch
minimal and splitting it out from the current series, and targeting it
towards the "net" tree, which will eventually get merged into one of the
v5.12 rc's and then into the final version. The other patches won't go
into v5.12 but into v5.13, hence the "next" name.

Also add these lines in your .gitconfig:

[core]
	abbrev = 12
[pretty]
	fixes = Fixes: %h (\"%s\")

and run:

git show 3f9ef7785a9c --pretty=fixes
Fixes: 3f9ef7785a9c ("MIPS: ralink: manage low reset lines")

and paste that "Fixes:" line in the commit message, right above your
Signed-off-by: tag.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/3] net: dsa: mt7530: setup core clock even in TRGMII mode

From: Ilya Lipnitskiy <hidden>
Date: 2021-03-11 02:40:14

Hi Vladimir,

On Wed, Mar 10, 2021 at 3:10 PM Vladimir Oltean [off-list ref] wrote:
Hello Ilya,

On Wed, Mar 10, 2021 at 01:14:20PM -0800, Ilya Lipnitskiy wrote:
quoted
3f9ef7785a9c ("MIPS: ralink: manage low reset lines") made it so mt7530
actually resets the switch on platforms such as mt7621 (where bit 2 is
the reset line for the switch). That exposed an issue where the switch
would not function properly in TRGMII mode after a reset.

Reconfigure core clock in TRGMII mode to fix the issue.

Also, disable both core and TRGMII Tx clocks prior to reconfiguring.
Previously, only the core clock was disabled, but not TRGMII Tx clock.

Tested on Ubiquity ER-X (MT7621) with TRGMII mode enabled.

Signed-off-by: Ilya Lipnitskiy <redacted>
---
For the networking subsystem there are two git trees, "net" for bugfixes
and "net-next" for new features, and we specify the target tree using
git send-email --subject-prefix="PATCH net-next".

I assume you would like the v5.12 kernel to actually be functional on
the Ubiquiti ER-X switch, so I would recommend keeping this patch
minimal and splitting it out from the current series, and targeting it
towards the "net" tree, which will eventually get merged into one of the
v5.12 rc's and then into the final version. The other patches won't go
into v5.12 but into v5.13, hence the "next" name.
I think I knew this, but didn't think it through. Thanks for your
guidance. I have submitted a single patch to "net" and that same patch
and two more to "net-next" - hopefully that looks better, but I'm sure
I have more to learn still.
Also add these lines in your .gitconfig:

[core]
        abbrev = 12
[pretty]
        fixes = Fixes: %h (\"%s\")

and run:

git show 3f9ef7785a9c --pretty=fixes
Fixes: 3f9ef7785a9c ("MIPS: ralink: manage low reset lines")

and paste that "Fixes:" line in the commit message, right above your
Signed-off-by: tag.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/3] net: dsa: mt7530: setup core clock even in TRGMII mode

From: Ilya Lipnitskiy <hidden>
Date: 2021-03-11 03:18:55

Hi Vladimir,

On Wed, Mar 10, 2021 at 3:10 PM Vladimir Oltean [off-list ref] wrote:
Hello Ilya,

On Wed, Mar 10, 2021 at 01:14:20PM -0800, Ilya Lipnitskiy wrote:
quoted
3f9ef7785a9c ("MIPS: ralink: manage low reset lines") made it so mt7530
actually resets the switch on platforms such as mt7621 (where bit 2 is
the reset line for the switch). That exposed an issue where the switch
would not function properly in TRGMII mode after a reset.

Reconfigure core clock in TRGMII mode to fix the issue.

Also, disable both core and TRGMII Tx clocks prior to reconfiguring.
Previously, only the core clock was disabled, but not TRGMII Tx clock.

Tested on Ubiquity ER-X (MT7621) with TRGMII mode enabled.

Signed-off-by: Ilya Lipnitskiy <redacted>
---
For the networking subsystem there are two git trees, "net" for bugfixes
and "net-next" for new features, and we specify the target tree using
git send-email --subject-prefix="PATCH net-next".

I assume you would like the v5.12 kernel to actually be functional on
the Ubiquiti ER-X switch, so I would recommend keeping this patch
minimal and splitting it out from the current series, and targeting it
towards the "net" tree, which will eventually get merged into one of the
v5.12 rc's and then into the final version. The other patches won't go
into v5.12 but into v5.13, hence the "next" name.
I thought I figured it out - now I'm confused. Can you explain why
https://patchwork.kernel.org/project/netdevbpf/patch/20210311012108.7190-1-ilya.lipnitskiy@gmail.com/
is marked as supeseded?
Also add these lines in your .gitconfig:

[core]
        abbrev = 12
[pretty]
        fixes = Fixes: %h (\"%s\")

and run:

git show 3f9ef7785a9c --pretty=fixes
Fixes: 3f9ef7785a9c ("MIPS: ralink: manage low reset lines")

and paste that "Fixes:" line in the commit message, right above your
Signed-off-by: tag.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/3] net: dsa: mt7530: setup core clock even in TRGMII mode

From: Florian Fainelli <f.fainelli@gmail.com>
Date: 2021-03-11 03:21:37


On 3/10/2021 7:17 PM, Ilya Lipnitskiy wrote:
Hi Vladimir,

On Wed, Mar 10, 2021 at 3:10 PM Vladimir Oltean [off-list ref] wrote:
quoted
Hello Ilya,

On Wed, Mar 10, 2021 at 01:14:20PM -0800, Ilya Lipnitskiy wrote:
quoted
3f9ef7785a9c ("MIPS: ralink: manage low reset lines") made it so mt7530
actually resets the switch on platforms such as mt7621 (where bit 2 is
the reset line for the switch). That exposed an issue where the switch
would not function properly in TRGMII mode after a reset.

Reconfigure core clock in TRGMII mode to fix the issue.

Also, disable both core and TRGMII Tx clocks prior to reconfiguring.
Previously, only the core clock was disabled, but not TRGMII Tx clock.

Tested on Ubiquity ER-X (MT7621) with TRGMII mode enabled.

Signed-off-by: Ilya Lipnitskiy <redacted>
---
For the networking subsystem there are two git trees, "net" for bugfixes
and "net-next" for new features, and we specify the target tree using
git send-email --subject-prefix="PATCH net-next".

I assume you would like the v5.12 kernel to actually be functional on
the Ubiquiti ER-X switch, so I would recommend keeping this patch
minimal and splitting it out from the current series, and targeting it
towards the "net" tree, which will eventually get merged into one of the
v5.12 rc's and then into the final version. The other patches won't go
into v5.12 but into v5.13, hence the "next" name.
I thought I figured it out - now I'm confused. Can you explain why
https://patchwork.kernel.org/project/netdevbpf/patch/20210311012108.7190-1-ilya.lipnitskiy@gmail.com/
is marked as supeseded?
That looks like a mistake on the maintainer side, I do not believe that
patch should be Superseded since you just submitted it.
-- 
Florian

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/3] net: dsa: mt7530: setup core clock even in TRGMII mode

From: Ilya Lipnitskiy <hidden>
Date: 2021-03-11 03:36:08

Hi Florian,

On Wed, Mar 10, 2021 at 7:20 PM Florian Fainelli [off-list ref] wrote:


On 3/10/2021 7:17 PM, Ilya Lipnitskiy wrote:
quoted
Hi Vladimir,

On Wed, Mar 10, 2021 at 3:10 PM Vladimir Oltean [off-list ref] wrote:
quoted
Hello Ilya,

On Wed, Mar 10, 2021 at 01:14:20PM -0800, Ilya Lipnitskiy wrote:
quoted
3f9ef7785a9c ("MIPS: ralink: manage low reset lines") made it so mt7530
actually resets the switch on platforms such as mt7621 (where bit 2 is
the reset line for the switch). That exposed an issue where the switch
would not function properly in TRGMII mode after a reset.

Reconfigure core clock in TRGMII mode to fix the issue.

Also, disable both core and TRGMII Tx clocks prior to reconfiguring.
Previously, only the core clock was disabled, but not TRGMII Tx clock.

Tested on Ubiquity ER-X (MT7621) with TRGMII mode enabled.

Signed-off-by: Ilya Lipnitskiy <redacted>
---
For the networking subsystem there are two git trees, "net" for bugfixes
and "net-next" for new features, and we specify the target tree using
git send-email --subject-prefix="PATCH net-next".

I assume you would like the v5.12 kernel to actually be functional on
the Ubiquiti ER-X switch, so I would recommend keeping this patch
minimal and splitting it out from the current series, and targeting it
towards the "net" tree, which will eventually get merged into one of the
v5.12 rc's and then into the final version. The other patches won't go
into v5.12 but into v5.13, hence the "next" name.
I thought I figured it out - now I'm confused. Can you explain why
https://patchwork.kernel.org/project/netdevbpf/patch/20210311012108.7190-1-ilya.lipnitskiy@gmail.com/
is marked as supeseded?
That looks like a mistake on the maintainer side, I do not believe that
patch should be Superseded since you just submitted it.
Thanks for taking a look. I thought maybe I did something wrong with
submitting the same patch to net and net-next, but the net-next series
(https://patchwork.kernel.org/project/netdevbpf/patch/20210311020954.842341-1-ilya.lipnitskiy@gmail.com/)
depends on it, so the way I did it made the most sense for me. Let me
know if I did something wrong.
--
Florian
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/3] net: dsa: mt7530: use core_write wrapper

From: Ilya Lipnitskiy <hidden>
Date: 2021-03-10 21:15:12

When disabling PLL, there is no need to call core_write_mmd_indirect
directly, use the core_write wrapper instead like the rest of the code
in the function does. This change helps with consistency and
readability.

Signed-off-by: Ilya Lipnitskiy <redacted>
---
 drivers/net/dsa/mt7530.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index e785f80f966b..b106ea816778 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -445,10 +445,7 @@ mt7530_pad_clk_setup(struct dsa_switch *ds, phy_interface_t interface)
 		 * provide our own core_write_mmd_indirect to complete this
 		 * function.
 		 */
-		core_write_mmd_indirect(priv,
-					CORE_GSWPLL_GRP1,
-					MDIO_MMD_VEND2,
-					0);
+		core_write(priv, CORE_GSWPLL_GRP1, 0);
 
 		/* Set core clock into 500Mhz */
 		core_write(priv, CORE_GSWPLL_GRP2,
-- 
2.30.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/3] net: dsa: mt7530: use core_write wrapper

From: Andrew Lunn <andrew@lunn.ch>
Date: 2021-03-11 17:36:06

On Wed, Mar 10, 2021 at 01:14:19PM -0800, Ilya Lipnitskiy wrote:
quoted hunk
When disabling PLL, there is no need to call core_write_mmd_indirect
directly, use the core_write wrapper instead like the rest of the code
in the function does. This change helps with consistency and
readability.

Signed-off-by: Ilya Lipnitskiy <redacted>
---
 drivers/net/dsa/mt7530.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index e785f80f966b..b106ea816778 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -445,10 +445,7 @@ mt7530_pad_clk_setup(struct dsa_switch *ds, phy_interface_t interface)
 		 * provide our own core_write_mmd_indirect to complete this
 		 * function.
 		 */
-		core_write_mmd_indirect(priv,
-					CORE_GSWPLL_GRP1,
-					MDIO_MMD_VEND2,
-					0);
+		core_write(priv, CORE_GSWPLL_GRP1, 0);
		/* Disable PLL, since phy_device has not yet been created
		 * provided for phy_[read,write]_mmd_indirect is called, we
		 * provide our own core_write_mmd_indirect to complete this
		 * function.
		 */
		core_write_mmd_indirect(priv,
					CORE_GSWPLL_GRP1,
					MDIO_MMD_VEND2,
					0);

What about the comment? Seems odd to reference
core_write_mmd_indirect() when it is not actually called here after
your change.

     Andrew

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help