Subject: [PATCH net-next v4 0/7] MT7530 DSA Subdriver Improvements Act II

Hello!

This is the second patch series with the goal of simplifying the MT7530 DSA
subdriver and improving support for MT7530, MT7531, and the switch on the
MT7988 SoC.

I have done a simple ping test to confirm basic communication on all switch
ports on MCM and standalone MT7530, and MT7531 switch with this patch
series applied.

MT7621 Unielec, MCM MT7530:

rgmii-only-gmac0-mt7621-unielec-u7621-06-16m.dtb
gmac0-and-gmac1-mt7621-unielec-u7621-06-16m.dtb

tftpboot 0x80008000 mips-uzImage.bin; tftpboot 0x83000000 mips-rootfs.cpio.uboot; tftpboot 0x83f00000 $dtb; bootm 0x80008000 0x83000000 0x83f00000

MT7622 Bananapi, MT7531:

gmac0-and-gmac1-mt7622-bananapi-bpi-r64.dtb

tftpboot 0x40000000 arm64-Image; tftpboot 0x45000000 arm64-rootfs.cpio.uboot; tftpboot 0x4a000000 $dtb; booti 0x40000000 0x45000000 0x4a000000

MT7623 Bananapi, standalone MT7530:

rgmii-only-gmac0-mt7623n-bananapi-bpi-r2.dtb
gmac0-and-gmac1-mt7623n-bananapi-bpi-r2.dtb

tftpboot 0x80008000 arm-zImage; tftpboot 0x83000000 arm-rootfs.cpio.uboot; tftpboot 0x83f00000 $dtb; bootz 0x80008000 0x83000000 0x83f00000

This patch series is the continuation of the patch series linked below.

https://lore.kernel.org/r/[email protected]

Signed-off-by: Arınç ÜNAL <[email protected]>
---
Changes in v4:
- Update the patches with the latest received trailers.
- Leave no error returns on mt7530_pad_clk_setup() before renaming it to
mt7530_setup_port6() and moving it to under mt7530_mac_config().
- Link to v3: https://lore.kernel.org/r/20240202-for-netnext-mt7530-improvements-2-v3-0-63d5adae99ca@arinc9.com

Changes in v3:
- Update the patches with the latest received trailers.
- Patch 5
- Disable TRGMII clocks for all cases.
- Link to v2: https://lore.kernel.org/r/20240130-for-netnext-mt7530-improvements-2-v2-0-ba06f5dd9eb0@arinc9.com

Changes in v2:
- Update the patches with the latest received trailers.
- Remove 'net: dsa: mt7530: move enabling port 6 to mt7530_setup_port6()'
which was patch 5. I will bring a more appropriate change with a later
patch series.
- Patch 5
- Set P6_INTF_MODE(0) and explain why on the patch log.
- Patch 6
- Mention the MT7988 document and explain more on the patch log.
- Patch 7
- Explain more on the patch log.
- Link to v1: https://lore.kernel.org/r/[email protected]

---
Arınç ÜNAL (7):
net: dsa: mt7530: empty default case on mt7530_setup_port5()
net: dsa: mt7530: move XTAL check to mt7530_setup()
net: dsa: mt7530: simplify mt7530_pad_clk_setup()
net: dsa: mt7530: call port 6 setup from mt7530_mac_config()
net: dsa: mt7530: remove pad_setup function pointer
net: dsa: mt7530: correct port capabilities of MT7988
net: dsa: mt7530: do not clear config->supported_interfaces

drivers/net/dsa/mt7530.c | 152 +++++++++++++++++------------------------------
drivers/net/dsa/mt7530.h | 3 -
2 files changed, 53 insertions(+), 102 deletions(-)
---
base-commit: 4acf4e62cd572b0c806035046b3698f5585ab821
change-id: 20240121-for-netnext-mt7530-improvements-2-b4f43661b485

Best regards,
--
Arınç ÜNAL <[email protected]>



Subject: [PATCH net-next v4 3/7] net: dsa: mt7530: simplify mt7530_pad_clk_setup()

From: Arınç ÜNAL <[email protected]>

This code is from before this driver was converted to phylink API. Phylink
deals with the unsupported interface cases before mt7530_pad_clk_setup() 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 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.

Disable the TRGMII clocks for all cases. They will be enabled if TRGMII is
being used.

Read XTAL after checking for RGMII as it's only needed for the TRGMII
interface mode.

Reviewed-by: Daniel Golle <[email protected]>
Reviewed-by: Russell King (Oracle) <[email protected]>
Signed-off-by: Arınç ÜNAL <[email protected]>
---
drivers/net/dsa/mt7530.c | 91 +++++++++++++++++++++---------------------------
1 file changed, 40 insertions(+), 51 deletions(-)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 8b8469eba863..82e90da1e52b 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -418,64 +418,53 @@ 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, xtal;
+ u32 ncpo1, ssc_delta, xtal;

- xtal = mt7530_read(priv, MT7530_MHWTRAP) & HWTRAP_XTAL_MASK;
+ /* Disable the MT7530 TRGMII clocks */
+ core_clear(priv, CORE_TRGMII_GSW_CLK_CG, REG_TRGMIICK_EN);

- 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;
+ }
+
+ /* 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);

return 0;
}

--
2.40.1


Subject: [PATCH net-next v4 5/7] net: dsa: mt7530: remove pad_setup function pointer

From: Arınç ÜNAL <[email protected]>

The pad_setup function pointer was introduced with 88bdef8be9f6 ("net: dsa:
mt7530: Extend device data ready for adding a new hardware"). It was being
used to set up the core clock and port 6 of the MT7530 switch, and pll of
the MT7531 switch.

All of these were moved to more appropriate locations, and it was never
used for the switch on the MT7988 SoC. Therefore, this function pointer
hasn't got a use anymore. Remove it.

Acked-by: Daniel Golle <[email protected]>
Reviewed-by: Vladimir Oltean <[email protected]>
Reviewed-by: Russell King (Oracle) <[email protected]>
Signed-off-by: Arınç ÜNAL <[email protected]>
---
drivers/net/dsa/mt7530.c | 36 ++----------------------------------
drivers/net/dsa/mt7530.h | 3 ---
2 files changed, 2 insertions(+), 37 deletions(-)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 18842c759b63..8a6312b3bb06 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -467,18 +467,6 @@ mt7530_setup_port6(struct dsa_switch *ds, phy_interface_t interface)
core_set(priv, CORE_TRGMII_GSW_CLK_CG, REG_TRGMIICK_EN);
}

-static int
-mt7530_pad_clk_setup(struct dsa_switch *ds, phy_interface_t interface)
-{
- return 0;
-}
-
-static int
-mt7531_pad_setup(struct dsa_switch *ds, phy_interface_t interface)
-{
- return 0;
-}
-
static void
mt7531_pll_setup(struct mt7530_priv *priv)
{
@@ -2603,14 +2591,6 @@ static void mt7988_mac_port_get_caps(struct dsa_switch *ds, int port,
}
}

-static int
-mt753x_pad_setup(struct dsa_switch *ds, const struct phylink_link_state *state)
-{
- struct mt7530_priv *priv = ds->priv;
-
- return priv->info->pad_setup(ds, state->interface);
-}
-
static int
mt7530_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
phy_interface_t interface)
@@ -2776,8 +2756,6 @@ mt753x_phylink_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
if (priv->p6_interface == state->interface)
break;

- mt753x_pad_setup(ds, state);
-
if (mt753x_mac_config(ds, port, mode, state) < 0)
goto unsupported;

@@ -3095,11 +3073,6 @@ mt753x_conduit_state_change(struct dsa_switch *ds,
mt7530_rmw(priv, MT7530_MFC, CPU_EN | CPU_PORT_MASK, val);
}

-static int mt7988_pad_setup(struct dsa_switch *ds, phy_interface_t interface)
-{
- return 0;
-}
-
static int mt7988_setup(struct dsa_switch *ds)
{
struct mt7530_priv *priv = ds->priv;
@@ -3163,7 +3136,6 @@ const struct mt753x_info mt753x_table[] = {
.phy_write_c22 = mt7530_phy_write_c22,
.phy_read_c45 = mt7530_phy_read_c45,
.phy_write_c45 = mt7530_phy_write_c45,
- .pad_setup = mt7530_pad_clk_setup,
.mac_port_get_caps = mt7530_mac_port_get_caps,
.mac_port_config = mt7530_mac_config,
},
@@ -3175,7 +3147,6 @@ const struct mt753x_info mt753x_table[] = {
.phy_write_c22 = mt7530_phy_write_c22,
.phy_read_c45 = mt7530_phy_read_c45,
.phy_write_c45 = mt7530_phy_write_c45,
- .pad_setup = mt7530_pad_clk_setup,
.mac_port_get_caps = mt7530_mac_port_get_caps,
.mac_port_config = mt7530_mac_config,
},
@@ -3187,7 +3158,6 @@ const struct mt753x_info mt753x_table[] = {
.phy_write_c22 = mt7531_ind_c22_phy_write,
.phy_read_c45 = mt7531_ind_c45_phy_read,
.phy_write_c45 = mt7531_ind_c45_phy_write,
- .pad_setup = mt7531_pad_setup,
.cpu_port_config = mt7531_cpu_port_config,
.mac_port_get_caps = mt7531_mac_port_get_caps,
.mac_port_config = mt7531_mac_config,
@@ -3200,7 +3170,6 @@ const struct mt753x_info mt753x_table[] = {
.phy_write_c22 = mt7531_ind_c22_phy_write,
.phy_read_c45 = mt7531_ind_c45_phy_read,
.phy_write_c45 = mt7531_ind_c45_phy_write,
- .pad_setup = mt7988_pad_setup,
.cpu_port_config = mt7988_cpu_port_config,
.mac_port_get_caps = mt7988_mac_port_get_caps,
.mac_port_config = mt7988_mac_config,
@@ -3230,9 +3199,8 @@ mt7530_probe_common(struct mt7530_priv *priv)
/* Sanity check if these required device operations are filled
* properly.
*/
- if (!priv->info->sw_setup || !priv->info->pad_setup ||
- !priv->info->phy_read_c22 || !priv->info->phy_write_c22 ||
- !priv->info->mac_port_get_caps ||
+ if (!priv->info->sw_setup || !priv->info->phy_read_c22 ||
+ !priv->info->phy_write_c22 || !priv->info->mac_port_get_caps ||
!priv->info->mac_port_config)
return -EINVAL;

diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
index 80060cc740d2..26a6d2160c08 100644
--- a/drivers/net/dsa/mt7530.h
+++ b/drivers/net/dsa/mt7530.h
@@ -704,8 +704,6 @@ struct mt753x_pcs {
* @phy_write_c22: Holding the way writing PHY port using C22
* @phy_read_c45: Holding the way reading PHY port using C45
* @phy_write_c45: Holding the way writing PHY port using C45
- * @pad_setup: Holding the way setting up the bus pad for a certain
- * MAC port
* @phy_mode_supported: Check if the PHY type is being supported on a certain
* port
* @mac_port_validate: Holding the way to set addition validate type for a
@@ -726,7 +724,6 @@ struct mt753x_info {
int regnum);
int (*phy_write_c45)(struct mt7530_priv *priv, int port, int devad,
int regnum, u16 val);
- int (*pad_setup)(struct dsa_switch *ds, phy_interface_t interface);
int (*cpu_port_config)(struct dsa_switch *ds, int port);
void (*mac_port_get_caps)(struct dsa_switch *ds, int port,
struct phylink_config *config);

--
2.40.1


Subject: [PATCH net-next v4 4/7] net: dsa: mt7530: call port 6 setup from mt7530_mac_config()

From: Arınç ÜNAL <[email protected]>

mt7530_pad_clk_setup() is called if port 6 is enabled. It used to do more
things than setting up port 6. That part was moved to more appropriate
locations, mt7530_setup() and mt7530_pll_setup().

Now that all it does is set up port 6, rename it to mt7530_setup_port6(),
and move it to a more appropriate location, under mt7530_mac_config().

Change mt7530_setup_port6() to void as there're no error cases.

Leave an empty mt7530_pad_clk_setup() to satisfy the pad_setup function
pointer.

This is the code path for setting up the ports before:

dsa_switch_ops :: phylink_mac_config() -> mt753x_phylink_mac_config()
-> mt753x_mac_config()
-> mt753x_info :: mac_port_config() -> mt7530_mac_config()
-> mt7530_setup_port5()
-> mt753x_pad_setup()
-> mt753x_info :: pad_setup() -> mt7530_pad_clk_setup()

This is after:

dsa_switch_ops :: phylink_mac_config() -> mt753x_phylink_mac_config()
-> mt753x_mac_config()
-> mt753x_info :: mac_port_config() -> mt7530_mac_config()
-> mt7530_setup_port5()
-> mt7530_setup_port6()

Reviewed-by: Vladimir Oltean <[email protected]>
Reviewed-by: Russell King (Oracle) <[email protected]>
Signed-off-by: Arınç ÜNAL <[email protected]>
---
drivers/net/dsa/mt7530.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 82e90da1e52b..18842c759b63 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -414,8 +414,8 @@ mt753x_preferred_default_local_cpu_port(struct dsa_switch *ds)
}

/* Setup port 6 interface mode and TRGMII TX circuit */
-static int
-mt7530_pad_clk_setup(struct dsa_switch *ds, phy_interface_t interface)
+static void
+mt7530_setup_port6(struct dsa_switch *ds, phy_interface_t interface)
{
struct mt7530_priv *priv = ds->priv;
u32 ncpo1, ssc_delta, xtal;
@@ -465,7 +465,11 @@ 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);
+}

+static int
+mt7530_pad_clk_setup(struct dsa_switch *ds, phy_interface_t interface)
+{
return 0;
}

@@ -2613,11 +2617,10 @@ mt7530_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
{
struct mt7530_priv *priv = ds->priv;

- /* Only need to setup port5. */
- if (port != 5)
- return 0;
-
- mt7530_setup_port5(priv->ds, interface);
+ if (port == 5) {
+ mt7530_setup_port5(priv->ds, interface);
+ else if (port == 6)
+ mt7530_setup_port6(priv->ds, interface);

return 0;
}

--
2.40.1


Subject: [PATCH net-next v4 6/7] net: dsa: mt7530: correct port capabilities of MT7988

From: Arınç ÜNAL <[email protected]>

On the switch on the MT7988 SoC, as shown in Block Diagram 8.1.1.3 on page
125 of "MT7988A Wi-Fi 7 Generation Router Platform: Datasheet (Open
Version) v0.1", there are only 4 PHYs. That's port 0 to 3. Set the case for
ports which connect to switch PHYs to '0 ... 3'.

Port 4 and 5 are not used at all in this design.

Link: https://wiki.banana-pi.org/Banana_Pi_BPI-R4#Documents [1]
Acked-by: Daniel Golle <[email protected]>
Reviewed-by: Vladimir Oltean <[email protected]>
Signed-off-by: Arınç ÜNAL <[email protected]>
---
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 8a6312b3bb06..08d5d7dd5969 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -2577,7 +2577,7 @@ static void mt7988_mac_port_get_caps(struct dsa_switch *ds, int port,

switch (port) {
/* Ports which are connected to switch PHYs. There is no MII pinout. */
- case 0 ... 4:
+ case 0 ... 3:
__set_bit(PHY_INTERFACE_MODE_INTERNAL,
config->supported_interfaces);
break;

--
2.40.1


Subject: [PATCH net-next v4 7/7] net: dsa: mt7530: do not clear config->supported_interfaces

From: Arınç ÜNAL <[email protected]>

There's no need to clear the config->supported_interfaces bitmap before
reporting the supported interfaces as all bits in the bitmap will already
be initialized to zero when the phylink_config structure is allocated. The
"config" pointer points to &dp->phylink_config, and "dp" is allocated by
dsa_port_touch() with kzalloc(), so all its fields are filled with zeroes.

There's no code that would change the bitmap beforehand. Remove it.

Acked-by: Daniel Golle <[email protected]>
Reviewed-by: Vladimir Oltean <[email protected]>
Reviewed-by: Russell King (Oracle) <[email protected]>
Signed-off-by: Arınç ÜNAL <[email protected]>
---
drivers/net/dsa/mt7530.c | 2 --
1 file changed, 2 deletions(-)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 08d5d7dd5969..c7e1af1ea4bb 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -2573,8 +2573,6 @@ static void mt7531_mac_port_get_caps(struct dsa_switch *ds, int port,
static void mt7988_mac_port_get_caps(struct dsa_switch *ds, int port,
struct phylink_config *config)
{
- phy_interface_zero(config->supported_interfaces);
-
switch (port) {
/* Ports which are connected to switch PHYs. There is no MII pinout. */
case 0 ... 3:

--
2.40.1


2024-02-04 18:03:43

by Arınç ÜNAL

[permalink] [raw]
Subject: Re: [PATCH net-next v4 3/7] net: dsa: mt7530: simplify mt7530_pad_clk_setup()

On 4.02.2024 19:34, Arınç ÜNAL via B4 Relay wrote:
> From: Arınç ÜNAL <[email protected]>
>
> This code is from before this driver was converted to phylink API. Phylink
> deals with the unsupported interface cases before mt7530_pad_clk_setup() 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 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.
>
> Disable the TRGMII clocks for all cases. They will be enabled if TRGMII is
> being used.
>
> Read XTAL after checking for RGMII as it's only needed for the TRGMII
> interface mode.
>
> Reviewed-by: Daniel Golle <[email protected]>
> Reviewed-by: Russell King (Oracle) <[email protected]>
> Signed-off-by: Arınç ÜNAL <[email protected]>
> ---
> drivers/net/dsa/mt7530.c | 91 +++++++++++++++++++++---------------------------
> 1 file changed, 40 insertions(+), 51 deletions(-)
>
> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
> index 8b8469eba863..82e90da1e52b 100644
> --- a/drivers/net/dsa/mt7530.c
> +++ b/drivers/net/dsa/mt7530.c
> @@ -418,64 +418,53 @@ 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, xtal;
> + u32 ncpo1, ssc_delta, xtal;
>
> - xtal = mt7530_read(priv, MT7530_MHWTRAP) & HWTRAP_XTAL_MASK;
> + /* Disable the MT7530 TRGMII clocks */
> + core_clear(priv, CORE_TRGMII_GSW_CLK_CG, REG_TRGMIICK_EN);
>
> - 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;

This should be "return 0;". I will address this.

pw-bot: cr

Arınç

2024-02-04 18:11:25

by Arınç ÜNAL

[permalink] [raw]
Subject: Re: [PATCH net-next v4 4/7] net: dsa: mt7530: call port 6 setup from mt7530_mac_config()

On 4.02.2024 19:34, Arınç ÜNAL via B4 Relay wrote:
> From: Arınç ÜNAL <[email protected]>
>
> mt7530_pad_clk_setup() is called if port 6 is enabled. It used to do more
> things than setting up port 6. That part was moved to more appropriate
> locations, mt7530_setup() and mt7530_pll_setup().
>
> Now that all it does is set up port 6, rename it to mt7530_setup_port6(),
> and move it to a more appropriate location, under mt7530_mac_config().
>
> Change mt7530_setup_port6() to void as there're no error cases.
>
> Leave an empty mt7530_pad_clk_setup() to satisfy the pad_setup function
> pointer.
>
> This is the code path for setting up the ports before:
>
> dsa_switch_ops :: phylink_mac_config() -> mt753x_phylink_mac_config()
> -> mt753x_mac_config()
> -> mt753x_info :: mac_port_config() -> mt7530_mac_config()
> -> mt7530_setup_port5()
> -> mt753x_pad_setup()
> -> mt753x_info :: pad_setup() -> mt7530_pad_clk_setup()
>
> This is after:
>
> dsa_switch_ops :: phylink_mac_config() -> mt753x_phylink_mac_config()
> -> mt753x_mac_config()
> -> mt753x_info :: mac_port_config() -> mt7530_mac_config()
> -> mt7530_setup_port5()
> -> mt7530_setup_port6()
>
> Reviewed-by: Vladimir Oltean <[email protected]>
> Reviewed-by: Russell King (Oracle) <[email protected]>
> Signed-off-by: Arınç ÜNAL <[email protected]>
> ---
> drivers/net/dsa/mt7530.c | 17 ++++++++++-------
> 1 file changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
> index 82e90da1e52b..18842c759b63 100644
> --- a/drivers/net/dsa/mt7530.c
> +++ b/drivers/net/dsa/mt7530.c
> @@ -414,8 +414,8 @@ mt753x_preferred_default_local_cpu_port(struct dsa_switch *ds)
> }
>
> /* Setup port 6 interface mode and TRGMII TX circuit */
> -static int
> -mt7530_pad_clk_setup(struct dsa_switch *ds, phy_interface_t interface)
> +static void
> +mt7530_setup_port6(struct dsa_switch *ds, phy_interface_t interface)
> {
> struct mt7530_priv *priv = ds->priv;
> u32 ncpo1, ssc_delta, xtal;
> @@ -465,7 +465,11 @@ 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);
> +}
>
> +static int
> +mt7530_pad_clk_setup(struct dsa_switch *ds, phy_interface_t interface)
> +{
> return 0;
> }
>
> @@ -2613,11 +2617,10 @@ mt7530_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
> {
> struct mt7530_priv *priv = ds->priv;
>
> - /* Only need to setup port5. */
> - if (port != 5)
> - return 0;
> -
> - mt7530_setup_port5(priv->ds, interface);
> + if (port == 5) {

Stray brace, will remove.

Arınç