Subject: [PATCH net-next v3 0/9] MT7530 DSA Subdriver Improvements Act III

Hello!

This is the third 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 v3:
- Patch 8
- Explain properly the behaviour of setting link down on all ports at
setup.
- Split the changes for simplifying the link settings operations out to
another patch.
- Link to v2: https://lore.kernel.org/r/20240216-for-netnext-mt7530-improvements-3-v2-0-094cae3ff23b@arinc9.com

Changes in v2:
- Patch 8
- Use a single mt7530_rmw() instead of two mt7530_clear() and
mt7530_set() commands.
- Link to v1: https://lore.kernel.org/r/20240208-for-netnext-mt7530-improvements-3-v1-0-d7c1cfd502ca@arinc9.com

---
Arınç ÜNAL (9):
net: dsa: mt7530: remove .mac_port_config for MT7988 and make it optional
net: dsa: mt7530: set interrupt register only for MT7530
net: dsa: mt7530: do not use SW_PHY_RST to reset MT7531 switch
net: dsa: mt7530: get rid of useless error returns on phylink code path
net: dsa: mt7530: get rid of priv->info->cpu_port_config()
net: dsa: mt7530: get rid of mt753x_mac_config()
net: dsa: mt7530: put initialising PCS devices code back to original order
net: dsa: mt7530: sort link settings ops and force link down on all ports
net: dsa: mt7530: simplify link operations

drivers/net/dsa/mt7530.c | 259 ++++++++---------------------------------------
drivers/net/dsa/mt7530.h | 19 +---
2 files changed, 47 insertions(+), 231 deletions(-)
---
base-commit: b6b614558ed5b2ca50edacc0f2fbf5f52158c86c
change-id: 20240131-for-netnext-mt7530-improvements-3-a8ac49d4f7c2

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



Subject: [PATCH net-next v3 1/9] net: dsa: mt7530: remove .mac_port_config for MT7988 and make it optional

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

For the switch on the MT7988 SoC, the mac_port_config member for ID_MT7988
in mt753x_table is not needed as the interfaces of all MACs are already
handled on mt7988_mac_port_get_caps().

Therefore, remove the mac_port_config member from ID_MT7988 in
mt753x_table. Before calling priv->info->mac_port_config(), if there's no
mac_port_config member in mt753x_table, exit mt753x_mac_config()
successfully.

Remove calling priv->info->mac_port_config() from the sanity check as the
sanity check requires a pointer to a mac_port_config function to be
non-NULL. This will fail for MT7988 as mac_port_config won't be a member of
its info table.

Co-developed-by: Daniel Golle <[email protected]>
Signed-off-by: Daniel Golle <[email protected]>
Signed-off-by: Arınç ÜNAL <[email protected]>
Reviewed-by: Vladimir Oltean <[email protected]>
---
drivers/net/dsa/mt7530.c | 18 ++++--------------
1 file changed, 4 insertions(+), 14 deletions(-)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 03d966fa67b2..94d4b2c87799 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -2655,17 +2655,6 @@ static bool mt753x_is_mac_port(u32 port)
return (port == 5 || port == 6);
}

-static int
-mt7988_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
- phy_interface_t interface)
-{
- if (dsa_is_cpu_port(ds, port) &&
- interface == PHY_INTERFACE_MODE_INTERNAL)
- return 0;
-
- return -EINVAL;
-}
-
static int
mt7531_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
phy_interface_t interface)
@@ -2706,6 +2695,9 @@ mt753x_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
{
struct mt7530_priv *priv = ds->priv;

+ if (!priv->info->mac_port_config)
+ return 0;
+
return priv->info->mac_port_config(ds, port, mode, state->interface);
}

@@ -3169,7 +3161,6 @@ const struct mt753x_info mt753x_table[] = {
.phy_write_c45 = mt7531_ind_c45_phy_write,
.cpu_port_config = mt7988_cpu_port_config,
.mac_port_get_caps = mt7988_mac_port_get_caps,
- .mac_port_config = mt7988_mac_config,
},
};
EXPORT_SYMBOL_GPL(mt753x_table);
@@ -3197,8 +3188,7 @@ mt7530_probe_common(struct mt7530_priv *priv)
* properly.
*/
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)
+ !priv->info->phy_write_c22 || !priv->info->mac_port_get_caps)
return -EINVAL;

priv->id = priv->info->id;

--
2.40.1


Subject: [PATCH net-next v3 7/9] net: dsa: mt7530: put initialising PCS devices code back to original order

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

The commit fae463084032 ("net: dsa: mt753x: fix pcs conversion regression")
fixes regression caused by cpu_port_config manually calling phylink
operations. cpu_port_config was deemed useless and was removed. Therefore,
put initialising PCS devices code back to its original order.

Signed-off-by: Arınç ÜNAL <[email protected]>
---
drivers/net/dsa/mt7530.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index e353c03dd1db..5c8ad41ce8cd 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -2812,17 +2812,9 @@ static int
mt753x_setup(struct dsa_switch *ds)
{
struct mt7530_priv *priv = ds->priv;
- int i, ret;
-
- /* Initialise the PCS devices */
- for (i = 0; i < priv->ds->num_ports; i++) {
- priv->pcs[i].pcs.ops = priv->info->pcs_ops;
- priv->pcs[i].pcs.neg_mode = true;
- priv->pcs[i].priv = priv;
- priv->pcs[i].port = i;
- }
+ int ret = priv->info->sw_setup(ds);
+ int i;

- ret = priv->info->sw_setup(ds);
if (ret)
return ret;

@@ -2834,6 +2826,14 @@ mt753x_setup(struct dsa_switch *ds)
if (ret && priv->irq)
mt7530_free_irq_common(priv);

+ /* Initialise the PCS devices */
+ for (i = 0; i < priv->ds->num_ports; i++) {
+ priv->pcs[i].pcs.ops = priv->info->pcs_ops;
+ priv->pcs[i].pcs.neg_mode = true;
+ priv->pcs[i].priv = priv;
+ priv->pcs[i].port = i;
+ }
+
if (priv->create_sgmii) {
ret = priv->create_sgmii(priv);
if (ret && priv->irq)

--
2.40.1


Subject: [PATCH net-next v3 6/9] net: dsa: mt7530: get rid of mt753x_mac_config()

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

There is no need for a separate function to call
priv->info->mac_port_config(). Call it from mt753x_phylink_mac_config()
instead and remove mt753x_mac_config().

Signed-off-by: Arınç ÜNAL <[email protected]>
---
drivers/net/dsa/mt7530.c | 14 ++------------
1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 80b65f064310..e353c03dd1db 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -2634,16 +2634,6 @@ mt7531_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
}
}

-static void
-mt753x_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
- const struct phylink_link_state *state)
-{
- struct mt7530_priv *priv = ds->priv;
-
- if (priv->info->mac_port_config)
- priv->info->mac_port_config(ds, port, mode, state->interface);
-}
-
static struct phylink_pcs *
mt753x_phylink_mac_select_pcs(struct dsa_switch *ds, int port,
phy_interface_t interface)
@@ -2669,8 +2659,8 @@ mt753x_phylink_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
struct mt7530_priv *priv = ds->priv;
u32 mcr_cur, mcr_new;

- if (port == 5 || port == 6)
- mt753x_mac_config(ds, port, mode, state);
+ if ((port == 5 || port == 6) && priv->info->mac_port_config)
+ priv->info->mac_port_config(ds, port, mode, state->interface);

mcr_cur = mt7530_read(priv, MT7530_PMCR_P(port));
mcr_new = mcr_cur;

--
2.40.1


Subject: [PATCH net-next v3 4/9] net: dsa: mt7530: get rid of useless error returns on phylink code path

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

Remove error returns on the cases where they are already handled with the
function the mac_port_get_caps member in mt753x_table points to.

mt7531_mac_config() is also called from mt7531_cpu_port_config() outside of
phylink but the port and interface modes are already handled there.

Change the functions and the mac_port_config function pointer to void now
that there're no error returns anymore.

Remove mt753x_is_mac_port() that used to help the said error returns.

On mt7531_mac_config(), switch to if statements to simplify the code.

Remove internal phy cases from mt753x_phylink_mac_config(), there is no
need to check the interface mode as that's already handled with the
function the mac_port_get_caps member in mt753x_table points to.

Acked-by: Daniel Golle <[email protected]>
Tested-by: Daniel Golle <[email protected]>
Signed-off-by: Arınç ÜNAL <[email protected]>
---
drivers/net/dsa/mt7530.c | 81 ++++++++++--------------------------------------
drivers/net/dsa/mt7530.h | 6 ++--
2 files changed, 19 insertions(+), 68 deletions(-)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 296711fd5c43..c069bf273733 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -2587,7 +2587,7 @@ static void mt7988_mac_port_get_caps(struct dsa_switch *ds, int port,
}
}

-static int
+static void
mt7530_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
phy_interface_t interface)
{
@@ -2597,22 +2597,14 @@ mt7530_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
mt7530_setup_port5(priv->ds, interface);
else if (port == 6)
mt7530_setup_port6(priv->ds, interface);
-
- return 0;
}

-static int mt7531_rgmii_setup(struct mt7530_priv *priv, u32 port,
- phy_interface_t interface,
- struct phy_device *phydev)
+static void mt7531_rgmii_setup(struct mt7530_priv *priv, u32 port,
+ phy_interface_t interface,
+ struct phy_device *phydev)
{
u32 val;

- if (priv->p5_sgmii) {
- dev_err(priv->dev, "RGMII mode is not available for port %d\n",
- port);
- return -EINVAL;
- }
-
val = mt7530_read(priv, MT7531_CLKGEN_CTRL);
val |= GP_CLK_EN;
val &= ~GP_MODE_MASK;
@@ -2640,20 +2632,14 @@ static int mt7531_rgmii_setup(struct mt7530_priv *priv, u32 port,
case PHY_INTERFACE_MODE_RGMII_ID:
break;
default:
- return -EINVAL;
+ break;
}
}
- mt7530_write(priv, MT7531_CLKGEN_CTRL, val);
-
- return 0;
-}

-static bool mt753x_is_mac_port(u32 port)
-{
- return (port == 5 || port == 6);
+ mt7530_write(priv, MT7531_CLKGEN_CTRL, val);
}

-static int
+static void
mt7531_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
phy_interface_t interface)
{
@@ -2661,42 +2647,21 @@ mt7531_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
struct phy_device *phydev;
struct dsa_port *dp;

- if (!mt753x_is_mac_port(port)) {
- dev_err(priv->dev, "port %d is not a MAC port\n", port);
- return -EINVAL;
- }
-
- switch (interface) {
- case PHY_INTERFACE_MODE_RGMII:
- case PHY_INTERFACE_MODE_RGMII_ID:
- case PHY_INTERFACE_MODE_RGMII_RXID:
- case PHY_INTERFACE_MODE_RGMII_TXID:
+ if (phy_interface_mode_is_rgmii(interface)) {
dp = dsa_to_port(ds, port);
phydev = dp->user->phydev;
- return mt7531_rgmii_setup(priv, port, interface, phydev);
- case PHY_INTERFACE_MODE_SGMII:
- case PHY_INTERFACE_MODE_NA:
- case PHY_INTERFACE_MODE_1000BASEX:
- case PHY_INTERFACE_MODE_2500BASEX:
- /* handled in SGMII PCS driver */
- return 0;
- default:
- return -EINVAL;
+ mt7531_rgmii_setup(priv, port, interface, phydev);
}
-
- return -EINVAL;
}

-static int
+static void
mt753x_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
const struct phylink_link_state *state)
{
struct mt7530_priv *priv = ds->priv;

- if (!priv->info->mac_port_config)
- return 0;
-
- return priv->info->mac_port_config(ds, port, mode, state->interface);
+ if (priv->info->mac_port_config)
+ priv->info->mac_port_config(ds, port, mode, state->interface);
}

static struct phylink_pcs *
@@ -2725,17 +2690,11 @@ mt753x_phylink_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
u32 mcr_cur, mcr_new;

switch (port) {
- case 0 ... 4:
- if (state->interface != PHY_INTERFACE_MODE_GMII &&
- state->interface != PHY_INTERFACE_MODE_INTERNAL)
- goto unsupported;
- break;
case 5:
if (priv->p5_interface == state->interface)
break;

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

if (priv->p5_intf_sel != P5_DISABLED)
priv->p5_interface = state->interface;
@@ -2744,16 +2703,10 @@ mt753x_phylink_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
if (priv->p6_interface == state->interface)
break;

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

priv->p6_interface = state->interface;
break;
- default:
-unsupported:
- dev_err(ds->dev, "%s: unsupported %s port: %i\n",
- __func__, phy_modes(state->interface), port);
- return;
}

mcr_cur = mt7530_read(priv, MT7530_PMCR_P(port));
@@ -2836,7 +2789,6 @@ mt7531_cpu_port_config(struct dsa_switch *ds, int port)
struct mt7530_priv *priv = ds->priv;
phy_interface_t interface;
int speed;
- int ret;

switch (port) {
case 5:
@@ -2861,9 +2813,8 @@ mt7531_cpu_port_config(struct dsa_switch *ds, int port)
else
speed = SPEED_1000;

- ret = mt7531_mac_config(ds, port, MLO_AN_FIXED, interface);
- if (ret)
- return ret;
+ mt7531_mac_config(ds, port, MLO_AN_FIXED, interface);
+
mt7530_write(priv, MT7530_PMCR_P(port),
PMCR_CPU_PORT_SETTING(priv->id));
mt753x_phylink_mac_link_up(ds, port, MLO_AN_FIXED, interface, NULL,
diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
index 26a6d2160c08..b1665de7f640 100644
--- a/drivers/net/dsa/mt7530.h
+++ b/drivers/net/dsa/mt7530.h
@@ -730,9 +730,9 @@ struct mt753x_info {
void (*mac_port_validate)(struct dsa_switch *ds, int port,
phy_interface_t interface,
unsigned long *supported);
- int (*mac_port_config)(struct dsa_switch *ds, int port,
- unsigned int mode,
- phy_interface_t interface);
+ void (*mac_port_config)(struct dsa_switch *ds, int port,
+ unsigned int mode,
+ phy_interface_t interface);
};

/* struct mt7530_priv - This is the main data structure for holding the state

--
2.40.1


Subject: [PATCH net-next v3 5/9] net: dsa: mt7530: get rid of priv->info->cpu_port_config()

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

priv->info->cpu_port_config() is used for MT7531 and the switch on the
MT7988 SoC. It sets up the ports described as a CPU port earlier than the
phylink code path would do.

This function is useless as:
- Configuring the MACs can be done from the phylink_mac_config code path
instead.
- All the link configuration it does on the CPU ports are later undone with
the port_enable, phylink_mac_config, and then phylink_mac_link_up code
path [1].

priv->p5_interface and priv->p6_interface were being used to prevent
configuring the MACs from the phylink_mac_config code path. Remove them now
that they hold no purpose.

Remove priv->info->cpu_port_config(). On mt753x_phylink_mac_config, switch
to if statements to simplify the code.

Remove the overwriting of the speed and duplex interfaces for certain
interface modes. Phylink already provides the speed and duplex variables
with proper values. Phylink already sets the max speed of TRGMII to
SPEED_1000. Add SPEED_2500 for PHY_INTERFACE_MODE_2500BASEX to where the
speed and EEE bits are set instead.

On the switch on the MT7988 SoC, PHY_INTERFACE_MODE_INTERNAL is being used
to describe the interface mode of the 10G MAC, which is of port 6. On
mt7988_cpu_port_config() PMCR_FORCE_SPEED_1000 was set via the
PMCR_CPU_PORT_SETTING() mask. Add SPEED_10000 case to where the speed bits
are set to cover this. No need to add it to where the EEE bits are set as
the "MT7988A Wi-Fi 7 Generation Router Platform: Datasheet (Open Version)
v0.1" document shows that these bits don't exist on the MT7530_PMCR_P(6)
register.

Remove the definition of PMCR_CPU_PORT_SETTING() now that it holds no
purpose.

Change mt753x_cpu_port_enable() to void now that there're no error cases
left.

Link: https://lore.kernel.org/netdev/[email protected]/ [1]
Suggested-by: Russell King (Oracle) <[email protected]>
Signed-off-by: Arınç ÜNAL <[email protected]>
---
drivers/net/dsa/mt7530.c | 114 +++--------------------------------------------
drivers/net/dsa/mt7530.h | 11 -----
2 files changed, 7 insertions(+), 118 deletions(-)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index c069bf273733..80b65f064310 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -966,18 +966,10 @@ mt753x_trap_frames(struct mt7530_priv *priv)
MT753X_R0E_PORT_FW(MT753X_BPDU_CPU_ONLY));
}

-static int
+static void
mt753x_cpu_port_enable(struct dsa_switch *ds, int port)
{
struct mt7530_priv *priv = ds->priv;
- int ret;
-
- /* Setup max capability of CPU port at first */
- if (priv->info->cpu_port_config) {
- ret = priv->info->cpu_port_config(ds, port);
- if (ret)
- return ret;
- }

/* Enable Mediatek header mode on the cpu port */
mt7530_write(priv, MT7530_PVC_P(port),
@@ -1003,8 +995,6 @@ mt753x_cpu_port_enable(struct dsa_switch *ds, int port)
/* Set to fallback mode for independent VLAN learning */
mt7530_rmw(priv, MT7530_PCR_P(port), PCR_PORT_VLAN_MASK,
MT7530_PORT_FALLBACK_MODE);
-
- return 0;
}

static int
@@ -2261,8 +2251,6 @@ mt7530_setup(struct dsa_switch *ds)
val |= MHWTRAP_MANUAL;
mt7530_write(priv, MT7530_MHWTRAP, val);

- priv->p6_interface = PHY_INTERFACE_MODE_NA;
-
mt753x_trap_frames(priv);

/* Enable and reset MIB counters */
@@ -2277,9 +2265,7 @@ mt7530_setup(struct dsa_switch *ds)
mt7530_set(priv, MT7530_PSC_P(i), SA_DIS);

if (dsa_is_cpu_port(ds, i)) {
- ret = mt753x_cpu_port_enable(ds, i);
- if (ret)
- return ret;
+ mt753x_cpu_port_enable(ds, i);
} else {
mt7530_port_disable(ds, i);

@@ -2383,9 +2369,7 @@ mt7531_setup_common(struct dsa_switch *ds)
mt7530_set(priv, MT7531_DBG_CNT(i), MT7531_DIS_CLR);

if (dsa_is_cpu_port(ds, i)) {
- ret = mt753x_cpu_port_enable(ds, i);
- if (ret)
- return ret;
+ mt753x_cpu_port_enable(ds, i);
} else {
mt7530_port_disable(ds, i);

@@ -2474,10 +2458,6 @@ mt7531_setup(struct dsa_switch *ds)
mt7530_rmw(priv, MT7531_GPIO_MODE0, MT7531_GPIO0_MASK,
MT7531_GPIO0_INTERRUPT);

- /* Let phylink decide the interface later. */
- priv->p5_interface = PHY_INTERFACE_MODE_NA;
- priv->p6_interface = PHY_INTERFACE_MODE_NA;
-
/* Enable PHY core PLL, since phy_device has not yet been created
* provided for phy_[read,write]_mmd_indirect is called, we provide
* our own mt7531_ind_mmd_phy_[read,write] to complete this
@@ -2689,26 +2669,9 @@ mt753x_phylink_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
struct mt7530_priv *priv = ds->priv;
u32 mcr_cur, mcr_new;

- switch (port) {
- case 5:
- if (priv->p5_interface == state->interface)
- break;
-
+ if (port == 5 || port == 6)
mt753x_mac_config(ds, port, mode, state);

- if (priv->p5_intf_sel != P5_DISABLED)
- priv->p5_interface = state->interface;
- break;
- case 6:
- if (priv->p6_interface == state->interface)
- break;
-
- mt753x_mac_config(ds, port, mode, state);
-
- priv->p6_interface = state->interface;
- break;
- }
-
mcr_cur = mt7530_read(priv, MT7530_PMCR_P(port));
mcr_new = mcr_cur;
mcr_new &= ~PMCR_LINK_SETTINGS_MASK;
@@ -2744,17 +2707,10 @@ static void mt753x_phylink_mac_link_up(struct dsa_switch *ds, int port,

mcr = PMCR_RX_EN | PMCR_TX_EN | PMCR_FORCE_LNK;

- /* MT753x MAC works in 1G full duplex mode for all up-clocked
- * variants.
- */
- if (interface == PHY_INTERFACE_MODE_TRGMII ||
- (phy_interface_mode_is_8023z(interface))) {
- speed = SPEED_1000;
- duplex = DUPLEX_FULL;
- }
-
switch (speed) {
case SPEED_1000:
+ case SPEED_2500:
+ case SPEED_10000:
mcr |= PMCR_FORCE_SPEED_1000;
break;
case SPEED_100:
@@ -2772,6 +2728,7 @@ static void mt753x_phylink_mac_link_up(struct dsa_switch *ds, int port,
if (mode == MLO_AN_PHY && phydev && phy_init_eee(phydev, false) >= 0) {
switch (speed) {
case SPEED_1000:
+ case SPEED_2500:
mcr |= PMCR_FORCE_EEE1G;
break;
case SPEED_100:
@@ -2783,61 +2740,6 @@ static void mt753x_phylink_mac_link_up(struct dsa_switch *ds, int port,
mt7530_set(priv, MT7530_PMCR_P(port), mcr);
}

-static int
-mt7531_cpu_port_config(struct dsa_switch *ds, int port)
-{
- struct mt7530_priv *priv = ds->priv;
- phy_interface_t interface;
- int speed;
-
- switch (port) {
- case 5:
- if (!priv->p5_sgmii)
- interface = PHY_INTERFACE_MODE_RGMII;
- else
- interface = PHY_INTERFACE_MODE_2500BASEX;
-
- priv->p5_interface = interface;
- break;
- case 6:
- interface = PHY_INTERFACE_MODE_2500BASEX;
-
- priv->p6_interface = interface;
- break;
- default:
- return -EINVAL;
- }
-
- if (interface == PHY_INTERFACE_MODE_2500BASEX)
- speed = SPEED_2500;
- else
- speed = SPEED_1000;
-
- mt7531_mac_config(ds, port, MLO_AN_FIXED, interface);
-
- mt7530_write(priv, MT7530_PMCR_P(port),
- PMCR_CPU_PORT_SETTING(priv->id));
- mt753x_phylink_mac_link_up(ds, port, MLO_AN_FIXED, interface, NULL,
- speed, DUPLEX_FULL, true, true);
-
- return 0;
-}
-
-static int
-mt7988_cpu_port_config(struct dsa_switch *ds, int port)
-{
- struct mt7530_priv *priv = ds->priv;
-
- mt7530_write(priv, MT7530_PMCR_P(port),
- PMCR_CPU_PORT_SETTING(priv->id));
-
- mt753x_phylink_mac_link_up(ds, port, MLO_AN_FIXED,
- PHY_INTERFACE_MODE_INTERNAL, NULL,
- SPEED_10000, DUPLEX_FULL, true, true);
-
- return 0;
-}
-
static void mt753x_phylink_get_caps(struct dsa_switch *ds, int port,
struct phylink_config *config)
{
@@ -3096,7 +2998,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,
- .cpu_port_config = mt7531_cpu_port_config,
.mac_port_get_caps = mt7531_mac_port_get_caps,
.mac_port_config = mt7531_mac_config,
},
@@ -3108,7 +3009,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,
- .cpu_port_config = mt7988_cpu_port_config,
.mac_port_get_caps = mt7988_mac_port_get_caps,
},
};
diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
index b1665de7f640..8a8144868eaa 100644
--- a/drivers/net/dsa/mt7530.h
+++ b/drivers/net/dsa/mt7530.h
@@ -311,13 +311,6 @@ enum mt7530_vlan_port_acc_frm {
PMCR_TX_FC_EN | PMCR_RX_FC_EN | \
PMCR_FORCE_FDX | PMCR_FORCE_LNK | \
PMCR_FORCE_EEE1G | PMCR_FORCE_EEE100)
-#define PMCR_CPU_PORT_SETTING(id) (PMCR_FORCE_MODE_ID((id)) | \
- PMCR_IFG_XMIT(1) | PMCR_MAC_MODE | \
- PMCR_BACKOFF_EN | PMCR_BACKPR_EN | \
- PMCR_TX_EN | PMCR_RX_EN | \
- PMCR_TX_FC_EN | PMCR_RX_FC_EN | \
- PMCR_FORCE_SPEED_1000 | \
- PMCR_FORCE_FDX | PMCR_FORCE_LNK)

#define MT7530_PMEEECR_P(x) (0x3004 + (x) * 0x100)
#define WAKEUP_TIME_1000(x) (((x) & 0xFF) << 24)
@@ -724,7 +717,6 @@ struct mt753x_info {
int regnum);
int (*phy_write_c45)(struct mt7530_priv *priv, int port, int devad,
int regnum, u16 val);
- 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);
void (*mac_port_validate)(struct dsa_switch *ds, int port,
@@ -750,7 +742,6 @@ struct mt753x_info {
* @ports: Holding the state among ports
* @reg_mutex: The lock for protecting among process accessing
* registers
- * @p6_interface Holding the current port 6 interface
* @p5_intf_sel: Holding the current port 5 interface select
* @p5_sgmii: Flag for distinguishing if port 5 of the MT7531 switch
* has got SGMII
@@ -772,8 +763,6 @@ struct mt7530_priv {
const struct mt753x_info *info;
unsigned int id;
bool mcm;
- phy_interface_t p6_interface;
- phy_interface_t p5_interface;
enum p5_interface_select p5_intf_sel;
bool p5_sgmii;
u8 mirror_rx;

--
2.40.1


Subject: [PATCH net-next v3 8/9] net: dsa: mt7530: sort link settings ops and force link down on all ports

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

port_enable and port_disable clears the link settings. Move that to
mt7530_setup() and mt7531_setup_common() which set up the switches. This
way, the link settings are cleared on all ports at setup, and then only
once with phylink_mac_link_down() when a link goes down.

Enable force mode at setup to apply the force part of the link settings.
This ensures that disabled ports will have their link down.

Suggested-by: Vladimir Oltean <[email protected]>
Signed-off-by: Arınç ÜNAL <[email protected]>
---
drivers/net/dsa/mt7530.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 5c8ad41ce8cd..91d927bb49f7 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -1018,7 +1018,6 @@ mt7530_port_enable(struct dsa_switch *ds, int port,
priv->ports[port].enable = true;
mt7530_rmw(priv, MT7530_PCR_P(port), PCR_MATRIX_MASK,
priv->ports[port].pm);
- mt7530_clear(priv, MT7530_PMCR_P(port), PMCR_LINK_SETTINGS_MASK);

mutex_unlock(&priv->reg_mutex);

@@ -1038,7 +1037,6 @@ mt7530_port_disable(struct dsa_switch *ds, int port)
priv->ports[port].enable = false;
mt7530_rmw(priv, MT7530_PCR_P(port), PCR_MATRIX_MASK,
PCR_MATRIX_CLR);
- mt7530_clear(priv, MT7530_PMCR_P(port), PMCR_LINK_SETTINGS_MASK);

mutex_unlock(&priv->reg_mutex);
}
@@ -2257,6 +2255,12 @@ mt7530_setup(struct dsa_switch *ds)
mt7530_mib_reset(ds);

for (i = 0; i < MT7530_NUM_PORTS; i++) {
+ /* Clear link settings and enable force mode to force link down
+ * on all ports until they're enabled later.
+ */
+ mt7530_rmw(priv, MT7530_PMCR_P(i), PMCR_LINK_SETTINGS_MASK |
+ PMCR_FORCE_MODE, PMCR_FORCE_MODE);
+
/* Disable forwarding by default on all ports */
mt7530_rmw(priv, MT7530_PCR_P(i), PCR_MATRIX_MASK,
PCR_MATRIX_CLR);
@@ -2359,6 +2363,12 @@ mt7531_setup_common(struct dsa_switch *ds)
UNU_FFP_MASK);

for (i = 0; i < MT7530_NUM_PORTS; i++) {
+ /* Clear link settings and enable force mode to force link down
+ * on all ports until they're enabled later.
+ */
+ mt7530_rmw(priv, MT7530_PMCR_P(i), PMCR_LINK_SETTINGS_MASK |
+ MT7531_FORCE_MODE, MT7531_FORCE_MODE);
+
/* Disable forwarding by default on all ports */
mt7530_rmw(priv, MT7530_PCR_P(i), PCR_MATRIX_MASK,
PCR_MATRIX_CLR);

--
2.40.1


Subject: [PATCH net-next v3 9/9] net: dsa: mt7530: simplify link operations

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

The "MT7621 Giga Switch Programming Guide v0.3", "MT7531 Reference Manual
for Development Board v1.0", and "MT7988A Wi-Fi 7 Generation Router
Platform: Datasheet (Open Version) v0.1" documents show that these bits are
enabled at reset:

PMCR_IFG_XMIT(1) (not part of PMCR_LINK_SETTINGS_MASK)
PMCR_MAC_MODE (not part of PMCR_LINK_SETTINGS_MASK)
PMCR_TX_EN
PMCR_RX_EN
PMCR_BACKOFF_EN (not part of PMCR_LINK_SETTINGS_MASK)
PMCR_BACKPR_EN (not part of PMCR_LINK_SETTINGS_MASK)
PMCR_TX_FC_EN
PMCR_RX_FC_EN

These bits also don't exist on the MT7530_PMCR_P(6) register of the switch
on the MT7988 SoC:

PMCR_IFG_XMIT()
PMCR_MAC_MODE
PMCR_BACKOFF_EN
PMCR_BACKPR_EN

Remove the setting of the bits not part of PMCR_LINK_SETTINGS_MASK on
phylink_mac_config as they're already set.

The bit for setting the port on force mode is already done on
mt7530_setup() and mt7531_setup_common(). So get rid of
PMCR_FORCE_MODE_ID() which helped determine which bit to use for the switch
model.

Signed-off-by: Arınç ÜNAL <[email protected]>
---
drivers/net/dsa/mt7530.c | 12 +-----------
drivers/net/dsa/mt7530.h | 2 --
2 files changed, 1 insertion(+), 13 deletions(-)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 91d927bb49f7..1ae11b180d54 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -2667,23 +2667,13 @@ mt753x_phylink_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
const struct phylink_link_state *state)
{
struct mt7530_priv *priv = ds->priv;
- u32 mcr_cur, mcr_new;

if ((port == 5 || port == 6) && priv->info->mac_port_config)
priv->info->mac_port_config(ds, port, mode, state->interface);

- mcr_cur = mt7530_read(priv, MT7530_PMCR_P(port));
- mcr_new = mcr_cur;
- mcr_new &= ~PMCR_LINK_SETTINGS_MASK;
- mcr_new |= PMCR_IFG_XMIT(1) | PMCR_MAC_MODE | PMCR_BACKOFF_EN |
- PMCR_BACKPR_EN | PMCR_FORCE_MODE_ID(priv->id);
-
/* Are we connected to external phy */
if (port == 5 && dsa_is_user_port(ds, 5))
- mcr_new |= PMCR_EXT_PHY;
-
- if (mcr_new != mcr_cur)
- mt7530_write(priv, MT7530_PMCR_P(port), mcr_new);
+ mt7530_set(priv, MT7530_PMCR_P(port), PMCR_EXT_PHY);
}

static void mt753x_phylink_mac_link_down(struct dsa_switch *ds, int port,
diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
index 8a8144868eaa..a71166e0a7fc 100644
--- a/drivers/net/dsa/mt7530.h
+++ b/drivers/net/dsa/mt7530.h
@@ -304,8 +304,6 @@ enum mt7530_vlan_port_acc_frm {
MT7531_FORCE_DPX | \
MT7531_FORCE_RX_FC | \
MT7531_FORCE_TX_FC)
-#define PMCR_FORCE_MODE_ID(id) ((((id) == ID_MT7531) || ((id) == ID_MT7988)) ? \
- MT7531_FORCE_MODE : PMCR_FORCE_MODE)
#define PMCR_LINK_SETTINGS_MASK (PMCR_TX_EN | PMCR_FORCE_SPEED_1000 | \
PMCR_RX_EN | PMCR_FORCE_SPEED_100 | \
PMCR_TX_FC_EN | PMCR_RX_FC_EN | \

--
2.40.1


2024-03-01 14:53:22

by Russell King (Oracle)

[permalink] [raw]
Subject: Re: [PATCH net-next v3 4/9] net: dsa: mt7530: get rid of useless error returns on phylink code path

On Fri, Mar 01, 2024 at 12:43:00PM +0200, Arınç ÜNAL via B4 Relay wrote:
> From: Arınç ÜNAL <[email protected]>
>
> Remove error returns on the cases where they are already handled with the
> function the mac_port_get_caps member in mt753x_table points to.
>
> mt7531_mac_config() is also called from mt7531_cpu_port_config() outside of
> phylink but the port and interface modes are already handled there.
>
> Change the functions and the mac_port_config function pointer to void now
> that there're no error returns anymore.
>
> Remove mt753x_is_mac_port() that used to help the said error returns.
>
> On mt7531_mac_config(), switch to if statements to simplify the code.
>
> Remove internal phy cases from mt753x_phylink_mac_config(), there is no
> need to check the interface mode as that's already handled with the
> function the mac_port_get_caps member in mt753x_table points to.
>
> Acked-by: Daniel Golle <[email protected]>
> Tested-by: Daniel Golle <[email protected]>
> Signed-off-by: Arınç ÜNAL <[email protected]>

Reviewed-by: Russell King (Oracle) <[email protected]>

Thanks!

--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

2024-03-01 14:50:13

by Russell King (Oracle)

[permalink] [raw]
Subject: Re: [PATCH net-next v3 1/9] net: dsa: mt7530: remove .mac_port_config for MT7988 and make it optional

On Fri, Mar 01, 2024 at 12:42:57PM +0200, Arınç ÜNAL via B4 Relay wrote:
> From: Arınç ÜNAL <[email protected]>
>
> For the switch on the MT7988 SoC, the mac_port_config member for ID_MT7988
> in mt753x_table is not needed as the interfaces of all MACs are already
> handled on mt7988_mac_port_get_caps().
>
> Therefore, remove the mac_port_config member from ID_MT7988 in
> mt753x_table. Before calling priv->info->mac_port_config(), if there's no
> mac_port_config member in mt753x_table, exit mt753x_mac_config()
> successfully.
>
> Remove calling priv->info->mac_port_config() from the sanity check as the
> sanity check requires a pointer to a mac_port_config function to be
> non-NULL. This will fail for MT7988 as mac_port_config won't be a member of
> its info table.
>
> Co-developed-by: Daniel Golle <[email protected]>
> Signed-off-by: Daniel Golle <[email protected]>
> Signed-off-by: Arınç ÜNAL <[email protected]>
> Reviewed-by: Vladimir Oltean <[email protected]>

Reviewed-by: Russell King (Oracle) <[email protected]>

Thanks!

--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

2024-03-05 11:38:02

by Russell King (Oracle)

[permalink] [raw]
Subject: Re: [PATCH net-next v3 0/9] MT7530 DSA Subdriver Improvements Act III

On Tue, Mar 05, 2024 at 12:30:20PM +0100, Paolo Abeni wrote:
> On Fri, 2024-03-01 at 12:42 +0200, Arınç ÜNAL wrote:
> > This is the third 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]>
>
> @Russell, I see you went through some patches; my understanding is that
> there are no objection to this series in the current form. The series
> LGTM, so I'm going to apply it: I think it would a pity if it should
> miss this cycle.

That's fine - I did read through the entire series, but only gave my
r-b on the ones I felt I'd done a good enough job on. You may have
noticed I haven't submitted much network stuff this cycle... I have
limited bandwidth at the moment.

--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

2024-03-05 11:47:26

by Paolo Abeni

[permalink] [raw]
Subject: Re: [PATCH net-next v3 0/9] MT7530 DSA Subdriver Improvements Act III

On Fri, 2024-03-01 at 12:42 +0200, Arınç ÜNAL wrote:
> This is the third 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]>

@Russell, I see you went through some patches; my understanding is that
there are no objection to this series in the current form. The series
LGTM, so I'm going to apply it: I think it would a pity if it should
miss this cycle.

Cheers,

Paolo


2024-03-05 12:00:59

by patchwork-bot+netdevbpf

[permalink] [raw]
Subject: Re: [PATCH net-next v3 0/9] MT7530 DSA Subdriver Improvements Act III

Hello:

This series was applied to netdev/net-next.git (main)
by Paolo Abeni <[email protected]>:

On Fri, 01 Mar 2024 12:42:56 +0200 you wrote:
> Hello!
>
> This is the third 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.
>
> [...]

Here is the summary with links:
- [net-next,v3,1/9] net: dsa: mt7530: remove .mac_port_config for MT7988 and make it optional
https://git.kernel.org/netdev/net-next/c/6ebe414b48cf
- [net-next,v3,2/9] net: dsa: mt7530: set interrupt register only for MT7530
https://git.kernel.org/netdev/net-next/c/804cd5f7059e
- [net-next,v3,3/9] net: dsa: mt7530: do not use SW_PHY_RST to reset MT7531 switch
https://git.kernel.org/netdev/net-next/c/a565f98d7d25
- [net-next,v3,4/9] net: dsa: mt7530: get rid of useless error returns on phylink code path
https://git.kernel.org/netdev/net-next/c/adf4ae24ba42
- [net-next,v3,5/9] net: dsa: mt7530: get rid of priv->info->cpu_port_config()
https://git.kernel.org/netdev/net-next/c/22fa10170af5
- [net-next,v3,6/9] net: dsa: mt7530: get rid of mt753x_mac_config()
https://git.kernel.org/netdev/net-next/c/1192ed898c97
- [net-next,v3,7/9] net: dsa: mt7530: put initialising PCS devices code back to original order
https://git.kernel.org/netdev/net-next/c/3a87131e3d72
- [net-next,v3,8/9] net: dsa: mt7530: sort link settings ops and force link down on all ports
https://git.kernel.org/netdev/net-next/c/6324230b3b67
- [net-next,v3,9/9] net: dsa: mt7530: simplify link operations
https://git.kernel.org/netdev/net-next/c/b04097c7a745

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