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

Hello!

This patch series simplifies the MT7530 DSA subdriver and improves the
logic of the 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:
- Update the patches with the latest received trailers.
- Patch 1
- Declare the longest variable assignment first.
- Improve the patch log and the comment on the code.
- Don't do type cast for __ffs. Implicit type promotion takes place.
- Patch 4
- Put more relevant information on the comments on the code.
- Follow the 'commit <12+ chars of sha1> ("<title line>")' style on the
patch log to satisfy checkpatch errors.
- Patch 6
- Change the patch log to explain the change better.
- Link to v2: https://lore.kernel.org/r/[email protected]

Changes in v2:
- Shorten the patch series, include only 7 patches.
- Add the reviewed-by tags given.
- Patch 1
- Change variable ordering of mt753x_conduit_state_change().
- Define the mask variable and store BIT(cpu_dp->index) on it.
- Disable CPU_EN if priv->active_cpu_ports is 0.
- Patch 5
- On the patch log, take the irrelevant information about the
mt7530_setup_port5() call from mt7530_setup() out.
- Patch 6
- Change the patch log to reflect correct information.
- Link to v1: https://lore.kernel.org/r/[email protected]

---
Arınç ÜNAL (7):
net: dsa: mt7530: always trap frames to active CPU port on MT7530
net: dsa: mt7530: use p5_interface_select as data type for p5_intf_sel
net: dsa: mt7530: store port 5 SGMII capability of MT7531
net: dsa: mt7530: improve comments regarding switch ports
net: dsa: mt7530: improve code path for setting up port 5
net: dsa: mt7530: do not set priv->p5_interface on mt7530_setup_port5()
net: dsa: mt7530: do not run mt7530_setup_port5() if port 5 is disabled

drivers/net/dsa/mt7530-mdio.c | 7 +--
drivers/net/dsa/mt7530.c | 135 +++++++++++++++++++++++-------------------
drivers/net/dsa/mt7530.h | 16 +++--
3 files changed, 87 insertions(+), 71 deletions(-)
---
base-commit: 736b5545d39ca59d4332a60e56cc8a1a5e264a8e
change-id: 20240121-for-netnext-mt7530-improvements-1-6443549fb775

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



Subject: [PATCH net-next v3 3/7] net: dsa: mt7530: store port 5 SGMII capability of MT7531

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

Introduce the p5_sgmii field to store the information for whether port 5
has got SGMII or not. Instead of reading the MT7531_TOP_SIG_SR register
multiple times, the register will be read once and the value will be
stored on the p5_sgmii field. This saves unnecessary reads of the
register.

Move the comment about MT7531AE and MT7531BE to mt7531_setup(), where the
switch is identified.

Get rid of mt7531_dual_sgmii_supported() now that priv->p5_sgmii stores the
information. Address the code where mt7531_dual_sgmii_supported() is used.

Get rid of mt7531_is_rgmii_port() which just prints the opposite of
priv->p5_sgmii.

Instead of calling mt7531_pll_setup() then returning, do not call it if
port 5 is SGMII.

Remove P5_INTF_SEL_GMAC5_SGMII. The p5_interface_select enum is supposed to
represent the mode that port 5 is being used in, not the hardware
information of port 5. Set p5_intf_sel to P5_INTF_SEL_GMAC5 instead, if
port 5 is not dsa_is_unused_port().

Signed-off-by: Arınç ÜNAL <[email protected]>
Acked-by: Daniel Golle <[email protected]>
Reviewed-by: Vladimir Oltean <[email protected]>
---
drivers/net/dsa/mt7530-mdio.c | 7 ++-----
drivers/net/dsa/mt7530.c | 48 +++++++++++++++----------------------------
drivers/net/dsa/mt7530.h | 6 ++++--
3 files changed, 22 insertions(+), 39 deletions(-)

diff --git a/drivers/net/dsa/mt7530-mdio.c b/drivers/net/dsa/mt7530-mdio.c
index 088533663b83..fa3ee85a99c1 100644
--- a/drivers/net/dsa/mt7530-mdio.c
+++ b/drivers/net/dsa/mt7530-mdio.c
@@ -81,17 +81,14 @@ static const struct regmap_bus mt7530_regmap_bus = {
};

static int
-mt7531_create_sgmii(struct mt7530_priv *priv, bool dual_sgmii)
+mt7531_create_sgmii(struct mt7530_priv *priv)
{
struct regmap_config *mt7531_pcs_config[2] = {};
struct phylink_pcs *pcs;
struct regmap *regmap;
int i, ret = 0;

- /* MT7531AE has two SGMII units for port 5 and port 6
- * MT7531BE has only one SGMII unit for port 6
- */
- for (i = dual_sgmii ? 0 : 1; i < 2; i++) {
+ for (i = priv->p5_sgmii ? 0 : 1; i < 2; i++) {
mt7531_pcs_config[i] = devm_kzalloc(priv->dev,
sizeof(struct regmap_config),
GFP_KERNEL);
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 761c4804449a..c77092506c3f 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -487,15 +487,6 @@ mt7530_pad_clk_setup(struct dsa_switch *ds, phy_interface_t interface)
return 0;
}

-static bool mt7531_dual_sgmii_supported(struct mt7530_priv *priv)
-{
- u32 val;
-
- val = mt7530_read(priv, MT7531_TOP_SIG_SR);
-
- return (val & PAD_DUAL_SGMII_EN) != 0;
-}
-
static int
mt7531_pad_setup(struct dsa_switch *ds, phy_interface_t interface)
{
@@ -510,9 +501,6 @@ mt7531_pll_setup(struct mt7530_priv *priv)
u32 xtal;
u32 val;

- if (mt7531_dual_sgmii_supported(priv))
- return;
-
val = mt7530_read(priv, MT7531_CREV);
top_sig = mt7530_read(priv, MT7531_TOP_SIG_SR);
hwstrap = mt7530_read(priv, MT7531_HWTRAP);
@@ -920,8 +908,6 @@ static const char *p5_intf_modes(unsigned int p5_interface)
return "PHY P4";
case P5_INTF_SEL_GMAC5:
return "GMAC5";
- case P5_INTF_SEL_GMAC5_SGMII:
- return "GMAC5_SGMII";
default:
return "unknown";
}
@@ -2470,6 +2456,12 @@ mt7531_setup(struct dsa_switch *ds)
return -ENODEV;
}

+ /* MT7531AE has got two SGMII units. One for port 5, one for port 6.
+ * MT7531BE has got only one SGMII unit which is for port 6.
+ */
+ val = mt7530_read(priv, MT7531_TOP_SIG_SR);
+ priv->p5_sgmii = !!(val & PAD_DUAL_SGMII_EN);
+
/* all MACs must be forced link-down before sw reset */
for (i = 0; i < MT7530_NUM_PORTS; i++)
mt7530_write(priv, MT7530_PMCR_P(i), MT7531_FORCE_LNK);
@@ -2479,21 +2471,18 @@ mt7531_setup(struct dsa_switch *ds)
SYS_CTRL_PHY_RST | SYS_CTRL_SW_RST |
SYS_CTRL_REG_RST);

- mt7531_pll_setup(priv);
-
- if (mt7531_dual_sgmii_supported(priv)) {
- priv->p5_intf_sel = P5_INTF_SEL_GMAC5_SGMII;
-
+ if (!priv->p5_sgmii) {
+ mt7531_pll_setup(priv);
+ } else {
/* Let ds->user_mii_bus be able to access external phy. */
mt7530_rmw(priv, MT7531_GPIO_MODE1, MT7531_GPIO11_RG_RXD2_MASK,
MT7531_EXT_P_MDC_11);
mt7530_rmw(priv, MT7531_GPIO_MODE1, MT7531_GPIO12_RG_RXD3_MASK,
MT7531_EXT_P_MDIO_12);
- } else {
- priv->p5_intf_sel = P5_INTF_SEL_GMAC5;
}
- dev_dbg(ds->dev, "P5 support %s interface\n",
- p5_intf_modes(priv->p5_intf_sel));
+
+ if (!dsa_is_unused_port(ds, 5))
+ priv->p5_intf_sel = P5_INTF_SEL_GMAC5;

mt7530_rmw(priv, MT7531_GPIO_MODE0, MT7531_GPIO0_MASK,
MT7531_GPIO0_INTERRUPT);
@@ -2553,11 +2542,6 @@ static void mt7530_mac_port_get_caps(struct dsa_switch *ds, int port,
}
}

-static bool mt7531_is_rgmii_port(struct mt7530_priv *priv, u32 port)
-{
- return (port == 5) && (priv->p5_intf_sel != P5_INTF_SEL_GMAC5_SGMII);
-}
-
static void mt7531_mac_port_get_caps(struct dsa_switch *ds, int port,
struct phylink_config *config)
{
@@ -2570,7 +2554,7 @@ static void mt7531_mac_port_get_caps(struct dsa_switch *ds, int port,
break;

case 5: /* 2nd cpu port supports either rgmii or sgmii/8023z */
- if (mt7531_is_rgmii_port(priv, port)) {
+ if (!priv->p5_sgmii) {
phy_interface_set_rgmii(config->supported_interfaces);
break;
}
@@ -2637,7 +2621,7 @@ static int mt7531_rgmii_setup(struct mt7530_priv *priv, u32 port,
{
u32 val;

- if (!mt7531_is_rgmii_port(priv, port)) {
+ if (priv->p5_sgmii) {
dev_err(priv->dev, "RGMII mode is not available for port %d\n",
port);
return -EINVAL;
@@ -2881,7 +2865,7 @@ mt7531_cpu_port_config(struct dsa_switch *ds, int port)

switch (port) {
case 5:
- if (mt7531_is_rgmii_port(priv, port))
+ if (!priv->p5_sgmii)
interface = PHY_INTERFACE_MODE_RGMII;
else
interface = PHY_INTERFACE_MODE_2500BASEX;
@@ -3033,7 +3017,7 @@ mt753x_setup(struct dsa_switch *ds)
mt7530_free_irq_common(priv);

if (priv->create_sgmii) {
- ret = priv->create_sgmii(priv, mt7531_dual_sgmii_supported(priv));
+ ret = priv->create_sgmii(priv);
if (ret && priv->irq)
mt7530_free_irq(priv);
}
diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
index 9cbf18efa416..80060cc740d2 100644
--- a/drivers/net/dsa/mt7530.h
+++ b/drivers/net/dsa/mt7530.h
@@ -687,7 +687,6 @@ enum p5_interface_select {
P5_INTF_SEL_PHY_P0,
P5_INTF_SEL_PHY_P4,
P5_INTF_SEL_GMAC5,
- P5_INTF_SEL_GMAC5_SGMII,
};

struct mt7530_priv;
@@ -756,6 +755,8 @@ struct mt753x_info {
* 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
* @irq: IRQ number of the switch
* @irq_domain: IRQ domain of the switch irq_chip
* @irq_enable: IRQ enable bits, synced to SYS_INT_EN
@@ -777,6 +778,7 @@ struct mt7530_priv {
phy_interface_t p6_interface;
phy_interface_t p5_interface;
enum p5_interface_select p5_intf_sel;
+ bool p5_sgmii;
u8 mirror_rx;
u8 mirror_tx;
struct mt7530_port ports[MT7530_NUM_PORTS];
@@ -786,7 +788,7 @@ struct mt7530_priv {
int irq;
struct irq_domain *irq_domain;
u32 irq_enable;
- int (*create_sgmii)(struct mt7530_priv *priv, bool dual_sgmii);
+ int (*create_sgmii)(struct mt7530_priv *priv);
u8 active_cpu_ports;
};


--
2.40.1


Subject: [PATCH net-next v3 1/7] net: dsa: mt7530: always trap frames to active CPU port on MT7530

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

On the MT7530 switch, the CPU_PORT field indicates which CPU port to trap
frames to, regardless of the affinity of the inbound user port.

When multiple CPU ports are in use, if the DSA conduit interface is down,
trapped frames won't be passed to the conduit interface.

To make trapping frames work including this case, implement
ds->ops->conduit_state_change() on this subdriver and set the CPU_PORT
field to the numerically smallest CPU port whose conduit interface is up.
Introduce the active_cpu_ports field to store the information of the active
CPU ports. Correct the macros, CPU_PORT is bits 4 through 6 of the
register.

Add a comment to explain frame trapping for this switch.

Currently, the driver doesn't support the use of multiple CPU ports so this
is not necessarily a bug fix.

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

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 391c4dbdff42..761c4804449a 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -1035,10 +1035,6 @@ mt753x_cpu_port_enable(struct dsa_switch *ds, int port)
mt7530_set(priv, MT7530_MFC, BC_FFP(BIT(port)) | UNM_FFP(BIT(port)) |
UNU_FFP(BIT(port)));

- /* Set CPU port number */
- if (priv->id == ID_MT7530 || priv->id == ID_MT7621)
- mt7530_rmw(priv, MT7530_MFC, CPU_MASK, CPU_EN | CPU_PORT(port));
-
/* Add the CPU port to the CPU port bitmap for MT7531 and the switch on
* the MT7988 SoC. Trapped frames will be forwarded to the CPU port that
* is affine to the inbound user port.
@@ -3075,6 +3071,36 @@ static int mt753x_set_mac_eee(struct dsa_switch *ds, int port,
return 0;
}

+static void
+mt753x_conduit_state_change(struct dsa_switch *ds,
+ const struct net_device *conduit,
+ bool operational)
+{
+ struct dsa_port *cpu_dp = conduit->dsa_ptr;
+ struct mt7530_priv *priv = ds->priv;
+ int val = 0;
+ u8 mask;
+
+ /* Set the CPU port to trap frames to for MT7530. Trapped frames will be
+ * forwarded to the numerically smallest CPU port whose conduit
+ * interface is up.
+ */
+ if (priv->id != ID_MT7530 && priv->id != ID_MT7621)
+ return;
+
+ mask = BIT(cpu_dp->index);
+
+ if (operational)
+ priv->active_cpu_ports |= mask;
+ else
+ priv->active_cpu_ports &= ~mask;
+
+ if (priv->active_cpu_ports)
+ val = CPU_EN | CPU_PORT(__ffs(priv->active_cpu_ports));
+
+ 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;
@@ -3130,6 +3156,7 @@ const struct dsa_switch_ops mt7530_switch_ops = {
.phylink_mac_link_up = mt753x_phylink_mac_link_up,
.get_mac_eee = mt753x_get_mac_eee,
.set_mac_eee = mt753x_set_mac_eee,
+ .conduit_state_change = mt753x_conduit_state_change,
};
EXPORT_SYMBOL_GPL(mt7530_switch_ops);

diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
index 17e42d30fff4..ebfb3a7acfcd 100644
--- a/drivers/net/dsa/mt7530.h
+++ b/drivers/net/dsa/mt7530.h
@@ -41,8 +41,8 @@ enum mt753x_id {
#define UNU_FFP(x) (((x) & 0xff) << 8)
#define UNU_FFP_MASK UNU_FFP(~0)
#define CPU_EN BIT(7)
-#define CPU_PORT(x) ((x) << 4)
-#define CPU_MASK (0xf << 4)
+#define CPU_PORT_MASK GENMASK(6, 4)
+#define CPU_PORT(x) FIELD_PREP(CPU_PORT_MASK, x)
#define MIRROR_EN BIT(3)
#define MIRROR_PORT(x) ((x) & 0x7)
#define MIRROR_MASK 0x7
@@ -760,6 +760,7 @@ struct mt753x_info {
* @irq_domain: IRQ domain of the switch irq_chip
* @irq_enable: IRQ enable bits, synced to SYS_INT_EN
* @create_sgmii: Pointer to function creating SGMII PCS instance(s)
+ * @active_cpu_ports: Holding the active CPU ports
*/
struct mt7530_priv {
struct device *dev;
@@ -786,6 +787,7 @@ struct mt7530_priv {
struct irq_domain *irq_domain;
u32 irq_enable;
int (*create_sgmii)(struct mt7530_priv *priv, bool dual_sgmii);
+ u8 active_cpu_ports;
};

struct mt7530_hw_vlan_entry {

--
2.40.1


Subject: [PATCH net-next v3 7/7] net: dsa: mt7530: do not run mt7530_setup_port5() if port 5 is disabled

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

There's no need to run all the code on mt7530_setup_port5() if port 5 is
disabled. The only case for calling mt7530_setup_port5() from
mt7530_setup() is when PHY muxing is enabled. That is because port 5 is not
defined as a port on the devicetree, therefore, it cannot be controlled by
phylink.

Because of this, run mt7530_setup_port5() if priv->p5_intf_sel is
P5_INTF_SEL_PHY_P0 or P5_INTF_SEL_PHY_P4. Remove the P5_DISABLED case from
mt7530_setup_port5().

Stop initialising the interface variable as the remaining cases will always
call mt7530_setup_port5() with it initialised.

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

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 5394d8c6a40e..cd7673ecea51 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -942,9 +942,6 @@ static void mt7530_setup_port5(struct dsa_switch *ds, phy_interface_t interface)
/* MT7530_P5_MODE_GMAC: P5 -> External phy or 2nd GMAC */
val &= ~MHWTRAP_P5_DIS;
break;
- case P5_DISABLED:
- interface = PHY_INTERFACE_MODE_NA;
- break;
default:
dev_err(ds->dev, "Unsupported p5_intf_sel %d\n",
priv->p5_intf_sel);
@@ -2313,8 +2310,6 @@ mt7530_setup(struct dsa_switch *ds)
* Set priv->p5_intf_sel to the appropriate value if PHY muxing
* is detected.
*/
- interface = PHY_INTERFACE_MODE_NA;
-
for_each_child_of_node(dn, mac_np) {
if (!of_device_is_compatible(mac_np,
"mediatek,eth-mac"))
@@ -2346,7 +2341,9 @@ mt7530_setup(struct dsa_switch *ds)
break;
}

- mt7530_setup_port5(ds, interface);
+ if (priv->p5_intf_sel == P5_INTF_SEL_PHY_P0 ||
+ priv->p5_intf_sel == P5_INTF_SEL_PHY_P4)
+ mt7530_setup_port5(ds, interface);
}

#ifdef CONFIG_GPIOLIB

--
2.40.1


Subject: [PATCH net-next v3 6/7] net: dsa: mt7530: do not set priv->p5_interface on mt7530_setup_port5()

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

Running mt7530_setup_port5() from mt7530_setup() used to handle all cases
of configuring port 5, including phylink.

Setting priv->p5_interface under mt7530_setup_port5() makes sure that
mt7530_setup_port5() from mt753x_phylink_mac_config() won't run.

The commit ("net: dsa: mt7530: improve code path for setting up port 5")
makes so that mt7530_setup_port5() from mt7530_setup() runs only on
non-phylink cases.

Get rid of unnecessarily setting priv->p5_interface under
mt7530_setup_port5() as port 5 phylink configuration will be done by
running mt7530_setup_port5() from mt753x_phylink_mac_config() now.

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 33c15f10de34..5394d8c6a40e 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -978,8 +978,6 @@ static void mt7530_setup_port5(struct dsa_switch *ds, phy_interface_t interface)
dev_dbg(ds->dev, "Setup P5, HWTRAP=0x%x, intf_sel=%s, phy-mode=%s\n",
val, p5_intf_modes(priv->p5_intf_sel), phy_modes(interface));

- priv->p5_interface = interface;
-
unlock_exit:
mutex_unlock(&priv->reg_mutex);
}

--
2.40.1


Subject: [PATCH net-next v3 5/7] net: dsa: mt7530: improve code path for setting up port 5

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

There're two code paths for setting up port 5:

mt7530_setup()
-> mt7530_setup_port5()

mt753x_phylink_mac_config()
-> mt753x_mac_config()
-> mt7530_mac_config()
-> mt7530_setup_port5()

Currently mt7530_setup_port5() from mt7530_setup() always runs. If port 5
is used as a CPU, DSA, or user port, mt7530_setup_port5() from
mt753x_phylink_mac_config() won't run. That is because priv->p5_interface
set on mt7530_setup_port5() will match state->interface on
mt753x_phylink_mac_config() which will stop running mt7530_setup_port5()
again.

Therefore, mt7530_setup_port5() will never run from
mt753x_phylink_mac_config().

Address this by not running mt7530_setup_port5() from mt7530_setup() if
port 5 is used as a CPU, DSA, or user port. This driver isn't in the
dsa_switches_apply_workarounds[] array so phylink will always be present.

To keep the cases where port 5 isn't controlled by phylink working as
before, preserve the mt7530_setup_port5() call from mt7530_setup().

Do not set priv->p5_intf_sel to P5_DISABLED. It is already set to that when
"priv" is allocated.

Move setting the interface to a more specific location. It's supposed to be
overwritten if PHY muxing is detected.

Improve the comment which explains the process.

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

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 93d8498ce274..33c15f10de34 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -2308,16 +2308,15 @@ mt7530_setup(struct dsa_switch *ds)
return ret;

/* Setup port 5 */
- priv->p5_intf_sel = P5_DISABLED;
- interface = PHY_INTERFACE_MODE_NA;
-
if (!dsa_is_unused_port(ds, 5)) {
priv->p5_intf_sel = P5_INTF_SEL_GMAC5;
- ret = of_get_phy_mode(dsa_to_port(ds, 5)->dn, &interface);
- if (ret && ret != -ENODEV)
- return ret;
} else {
- /* Scan the ethernet nodes. look for GMAC1, lookup used phy */
+ /* Scan the ethernet nodes. Look for GMAC1, lookup the used PHY.
+ * Set priv->p5_intf_sel to the appropriate value if PHY muxing
+ * is detected.
+ */
+ interface = PHY_INTERFACE_MODE_NA;
+
for_each_child_of_node(dn, mac_np) {
if (!of_device_is_compatible(mac_np,
"mediatek,eth-mac"))
@@ -2348,6 +2347,8 @@ mt7530_setup(struct dsa_switch *ds)
of_node_put(phy_node);
break;
}
+
+ mt7530_setup_port5(ds, interface);
}

#ifdef CONFIG_GPIOLIB
@@ -2358,8 +2359,6 @@ mt7530_setup(struct dsa_switch *ds)
}
#endif /* CONFIG_GPIOLIB */

- mt7530_setup_port5(ds, interface);
-
/* Flush the FDB table */
ret = mt7530_fdb_cmd(priv, MT7530_FDB_FLUSH, NULL);
if (ret < 0)

--
2.40.1


2024-01-25 09:33:24

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [PATCH net-next v3 1/7] net: dsa: mt7530: always trap frames to active CPU port on MT7530

On Mon, Jan 22, 2024 at 08:35:52AM +0300, Arınç ÜNAL via B4 Relay wrote:
> From: Arınç ÜNAL <[email protected]>
>
> On the MT7530 switch, the CPU_PORT field indicates which CPU port to trap
> frames to, regardless of the affinity of the inbound user port.
>
> When multiple CPU ports are in use, if the DSA conduit interface is down,
> trapped frames won't be passed to the conduit interface.
>
> To make trapping frames work including this case, implement
> ds->ops->conduit_state_change() on this subdriver and set the CPU_PORT
> field to the numerically smallest CPU port whose conduit interface is up.
> Introduce the active_cpu_ports field to store the information of the active
> CPU ports. Correct the macros, CPU_PORT is bits 4 through 6 of the
> register.
>
> Add a comment to explain frame trapping for this switch.
>
> Currently, the driver doesn't support the use of multiple CPU ports so this
> is not necessarily a bug fix.
>
> Suggested-by: Vladimir Oltean <[email protected]>
> Suggested-by: Russell King (Oracle) <[email protected]>
> Signed-off-by: Arınç ÜNAL <[email protected]>
> ---

Reviewed-by: Vladimir Oltean <[email protected]>

2024-01-25 23:03:26

by Arınç ÜNAL

[permalink] [raw]
Subject: Re: [PATCH net-next v3 6/7] net: dsa: mt7530: do not set priv->p5_interface on mt7530_setup_port5()

Vladimir could you review this too? This is the only patch remaining
without a review or ACK.

Arınç

On 22.01.2024 08:35, Arınç ÜNAL via B4 Relay wrote:
> From: Arınç ÜNAL <[email protected]>
>
> Running mt7530_setup_port5() from mt7530_setup() used to handle all cases
> of configuring port 5, including phylink.
>
> Setting priv->p5_interface under mt7530_setup_port5() makes sure that
> mt7530_setup_port5() from mt753x_phylink_mac_config() won't run.
>
> The commit ("net: dsa: mt7530: improve code path for setting up port 5")
> makes so that mt7530_setup_port5() from mt7530_setup() runs only on
> non-phylink cases.
>
> Get rid of unnecessarily setting priv->p5_interface under
> mt7530_setup_port5() as port 5 phylink configuration will be done by
> running mt7530_setup_port5() from mt753x_phylink_mac_config() now.
>
> 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 33c15f10de34..5394d8c6a40e 100644
> --- a/drivers/net/dsa/mt7530.c
> +++ b/drivers/net/dsa/mt7530.c
> @@ -978,8 +978,6 @@ static void mt7530_setup_port5(struct dsa_switch *ds, phy_interface_t interface)
> dev_dbg(ds->dev, "Setup P5, HWTRAP=0x%x, intf_sel=%s, phy-mode=%s\n",
> val, p5_intf_modes(priv->p5_intf_sel), phy_modes(interface));
>
> - priv->p5_interface = interface;
> -
> unlock_exit:
> mutex_unlock(&priv->reg_mutex);
> }
>

2024-01-29 12:55:35

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [PATCH net-next v3 6/7] net: dsa: mt7530: do not set priv->p5_interface on mt7530_setup_port5()

On Mon, Jan 22, 2024 at 08:35:57AM +0300, Arınç ÜNAL via B4 Relay wrote:
> From: Arınç ÜNAL <[email protected]>
>
> Running mt7530_setup_port5() from mt7530_setup() used to handle all cases
> of configuring port 5, including phylink.
>
> Setting priv->p5_interface under mt7530_setup_port5() makes sure that
> mt7530_setup_port5() from mt753x_phylink_mac_config() won't run.
>
> The commit ("net: dsa: mt7530: improve code path for setting up port 5")
> makes so that mt7530_setup_port5() from mt7530_setup() runs only on
> non-phylink cases.
>
> Get rid of unnecessarily setting priv->p5_interface under
> mt7530_setup_port5() as port 5 phylink configuration will be done by
> running mt7530_setup_port5() from mt753x_phylink_mac_config() now.
>
> Signed-off-by: Arınç ÜNAL <[email protected]>
> ---

Reviewed-by: Vladimir Oltean <[email protected]>

I hope this moves the patch set out of the 'deferred' state.

---
pw-bot: under-review

2024-01-29 16:24:03

by Arınç ÜNAL

[permalink] [raw]
Subject: Re: [PATCH net-next v3 6/7] net: dsa: mt7530: do not set priv->p5_interface on mt7530_setup_port5()

On 29.01.2024 15:52, Vladimir Oltean wrote:
> On Mon, Jan 22, 2024 at 08:35:57AM +0300, Arınç ÜNAL via B4 Relay wrote:
>> From: Arınç ÜNAL <[email protected]>
>>
>> Running mt7530_setup_port5() from mt7530_setup() used to handle all cases
>> of configuring port 5, including phylink.
>>
>> Setting priv->p5_interface under mt7530_setup_port5() makes sure that
>> mt7530_setup_port5() from mt753x_phylink_mac_config() won't run.
>>
>> The commit ("net: dsa: mt7530: improve code path for setting up port 5")
>> makes so that mt7530_setup_port5() from mt7530_setup() runs only on
>> non-phylink cases.
>>
>> Get rid of unnecessarily setting priv->p5_interface under
>> mt7530_setup_port5() as port 5 phylink configuration will be done by
>> running mt7530_setup_port5() from mt753x_phylink_mac_config() now.
>>
>> Signed-off-by: Arınç ÜNAL <[email protected]>
>> ---
>
> Reviewed-by: Vladimir Oltean <[email protected]>
>
> I hope this moves the patch set out of the 'deferred' state.
>
> ---
> pw-bot: under-review

I still see deferred. I guess I'll have to submit this again. :/

Arınç

2024-01-29 16:29:33

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [PATCH net-next v3 6/7] net: dsa: mt7530: do not set priv->p5_interface on mt7530_setup_port5()

On Mon, Jan 29, 2024 at 07:22:28PM +0300, Arınç ÜNAL wrote:
> On 29.01.2024 15:52, Vladimir Oltean wrote:
> > On Mon, Jan 22, 2024 at 08:35:57AM +0300, Arınç ÜNAL via B4 Relay wrote:
> > > From: Arınç ÜNAL <[email protected]>
> > >
> > > Running mt7530_setup_port5() from mt7530_setup() used to handle all cases
> > > of configuring port 5, including phylink.
> > >
> > > Setting priv->p5_interface under mt7530_setup_port5() makes sure that
> > > mt7530_setup_port5() from mt753x_phylink_mac_config() won't run.
> > >
> > > The commit ("net: dsa: mt7530: improve code path for setting up port 5")
> > > makes so that mt7530_setup_port5() from mt7530_setup() runs only on
> > > non-phylink cases.
> > >
> > > Get rid of unnecessarily setting priv->p5_interface under
> > > mt7530_setup_port5() as port 5 phylink configuration will be done by
> > > running mt7530_setup_port5() from mt753x_phylink_mac_config() now.
> > >
> > > Signed-off-by: Arınç ÜNAL <[email protected]>
> > > ---
> >
> > Reviewed-by: Vladimir Oltean <[email protected]>
> >
> > I hope this moves the patch set out of the 'deferred' state.
> >
> > ---
> > pw-bot: under-review
>
> I still see deferred. I guess I'll have to submit this again. :/
>
> Arınç

Please wait for a few more hours for one of the networking maintainers
to have a chance to see this and ask you to resend, if necessary.

2024-01-29 16:34:51

by Jakub Kicinski

[permalink] [raw]
Subject: Re: [PATCH net-next v3 6/7] net: dsa: mt7530: do not set priv->p5_interface on mt7530_setup_port5()

On Mon, 29 Jan 2024 19:22:28 +0300 Arınç ÜNAL wrote:
> > I hope this moves the patch set out of the 'deferred' state.
> >
> > ---
> > pw-bot: under-review
>
> I still see deferred. I guess I'll have to submit this again. :/

Took me an hour to fix the mailbot:
https://github.com/kuba-moo/nipa/commit/6766e97e72ac91ffb42ed2259bc8e2ace446d0ef
email is the most quirky thing ever.

2024-01-29 16:53:18

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [PATCH net-next v3 6/7] net: dsa: mt7530: do not set priv->p5_interface on mt7530_setup_port5()

On Mon, Jan 29, 2024 at 08:31:52AM -0800, Jakub Kicinski wrote:
> On Mon, 29 Jan 2024 19:22:28 +0300 Arınç ÜNAL wrote:
> > > I hope this moves the patch set out of the 'deferred' state.
> > >
> > > ---
> > > pw-bot: under-review
> >
> > I still see deferred. I guess I'll have to submit this again. :/
>
> Took me an hour to fix the mailbot:
> https://github.com/kuba-moo/nipa/commit/6766e97e72ac91ffb42ed2259bc8e2ace446d0ef
> email is the most quirky thing ever.

Ah, so it was my neomutt encoding email as base64...

I see Arınç's series is now in the proper state, thanks!

2024-01-29 17:01:23

by Jakub Kicinski

[permalink] [raw]
Subject: Re: [PATCH net-next v3 6/7] net: dsa: mt7530: do not set priv->p5_interface on mt7530_setup_port5()

On Mon, 29 Jan 2024 18:52:01 +0200 Vladimir Oltean wrote:
> > > I still see deferred. I guess I'll have to submit this again. :/
> >
> > Took me an hour to fix the mailbot:
> > https://github.com/kuba-moo/nipa/commit/6766e97e72ac91ffb42ed2259bc8e2ace446d0ef
> > email is the most quirky thing ever.
>
> Ah, so it was my neomutt encoding email as base64...

Something magical going on there, the email is encoded.. twice?
See the attachment. That's already thru a round of base64 decode
and there's another copy of the email with base64 inside it :o
Anyway, unwrapping it once is good enough for the bot to see the
command, and enough time spent on this ;)

> I see Arınç's series is now in the proper state, thanks!

np!


Attachments:
(No filename) (748.00 B)
t (8.90 kB)
Download all attachments

2024-01-30 02:15:03

by patchwork-bot+netdevbpf

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

Hello:

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

On Mon, 22 Jan 2024 08:35:51 +0300 you wrote:
> Hello!
>
> This patch series simplifies the MT7530 DSA subdriver and improves the
> logic of the 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/7] net: dsa: mt7530: always trap frames to active CPU port on MT7530
https://git.kernel.org/netdev/net-next/c/024d8577f534
- [net-next,v3,2/7] net: dsa: mt7530: use p5_interface_select as data type for p5_intf_sel
https://git.kernel.org/netdev/net-next/c/b198c9097f06
- [net-next,v3,3/7] net: dsa: mt7530: store port 5 SGMII capability of MT7531
https://git.kernel.org/netdev/net-next/c/1f4a85f2eaa8
- [net-next,v3,4/7] net: dsa: mt7530: improve comments regarding switch ports
https://git.kernel.org/netdev/net-next/c/05957aa77ed8
- [net-next,v3,5/7] net: dsa: mt7530: improve code path for setting up port 5
https://git.kernel.org/netdev/net-next/c/152f8e8e7458
- [net-next,v3,6/7] net: dsa: mt7530: do not set priv->p5_interface on mt7530_setup_port5()
https://git.kernel.org/netdev/net-next/c/6537973f2a5d
- [net-next,v3,7/7] net: dsa: mt7530: do not run mt7530_setup_port5() if port 5 is disabled
https://git.kernel.org/netdev/net-next/c/04a22bef5fc2

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



2024-01-30 14:27:08

by Arınç ÜNAL

[permalink] [raw]
Subject: Re: [PATCH net-next v3 6/7] net: dsa: mt7530: do not set priv->p5_interface on mt7530_setup_port5()



On 29.01.2024 20:00, Jakub Kicinski wrote:
> On Mon, 29 Jan 2024 18:52:01 +0200 Vladimir Oltean wrote:
>>>> I still see deferred. I guess I'll have to submit this again. :/
>>>
>>> Took me an hour to fix the mailbot:
>>> https://github.com/kuba-moo/nipa/commit/6766e97e72ac91ffb42ed2259bc8e2ace446d0ef
>>> email is the most quirky thing ever.
>>
>> Ah, so it was my neomutt encoding email as base64...
>
> Something magical going on there, the email is encoded.. twice?
> See the attachment. That's already thru a round of base64 decode
> and there's another copy of the email with base64 inside it :o
> Anyway, unwrapping it once is good enough for the bot to see the
> command, and enough time spent on this ;)

I don't claim to be an email expert. I've received Vladimir's email with
the "Content-Transfer-Encoding: 8bit" header. The body was plaintext, not
base64 encoded. I have checked how the netdev mailing list distributed
Vladimir's email, its body is plaintext as well, not base64 encoded. Only
the linux-arm-kernel mailing list distributed the body base64 encoded, the
header is "Content-Transfer-Encoding: base64".

And the attachment you've provided seems to be from the raw output of
lore.kernel.org/all which seems to put together the email distribution from
all mailing lists.

raw from all:

https://lore.kernel.org/all/20240129125241.gu4srgufad6hpwor@skbuf/raw

raw from netdev:

https://lore.kernel.org/netdev/20240129125241.gu4srgufad6hpwor@skbuf/raw

raw from linux-arm-kernel:

https://lore.kernel.org/linux-arm-kernel/20240129125241.gu4srgufad6hpwor@skbuf/raw

I don't know which mailing list mailbot looks at in case of an email is
sent with multiple mailing lists being CC'd or TO'd. It seems to be that it
looked at linux-arm-kernel in this instance.

Arınç

2024-01-30 16:03:13

by Jakub Kicinski

[permalink] [raw]
Subject: Re: [PATCH net-next v3 6/7] net: dsa: mt7530: do not set priv->p5_interface on mt7530_setup_port5()

On Tue, 30 Jan 2024 17:26:29 +0300 Arınç ÜNAL wrote:
> I don't claim to be an email expert. I've received Vladimir's email with
> the "Content-Transfer-Encoding: 8bit" header. The body was plaintext, not
> base64 encoded. I have checked how the netdev mailing list distributed
> Vladimir's email, its body is plaintext as well, not base64 encoded. Only
> the linux-arm-kernel mailing list distributed the body base64 encoded, the
> header is "Content-Transfer-Encoding: base64".
>
> And the attachment you've provided seems to be from the raw output of
> lore.kernel.org/all which seems to put together the email distribution from
> all mailing lists.
>
> raw from all:
>
> https://lore.kernel.org/all/20240129125241.gu4srgufad6hpwor@skbuf/raw
>
> raw from netdev:
>
> https://lore.kernel.org/netdev/20240129125241.gu4srgufad6hpwor@skbuf/raw
>
> raw from linux-arm-kernel:
>
> https://lore.kernel.org/linux-arm-kernel/20240129125241.gu4srgufad6hpwor@skbuf/raw
>
> I don't know which mailing list mailbot looks at in case of an email is
> sent with multiple mailing lists being CC'd or TO'd. It seems to be that it
> looked at linux-arm-kernel in this instance.

It's the Python library that base-encodes it for some reason :o

>>>>> $ tail -20 raw.5
> mt7530_setup_port5() from mt753x_phylink_mac_config() won't run.
>
> The commit ("net: dsa: mt7530: improve code path for setting up port 5")
> makes so that mt7530_setup_port5() from mt7530_setup() runs only on
> non-phylink cases.
>
> Get rid of unnecessarily setting priv->p5_interface under
> mt7530_setup_port5() as port 5 phylink configuration will be done by
> running mt7530_setup_port5() from mt753x_phylink_mac_config() now.
>
> Signed-off-by: Arınç ÜNAL <[email protected]>
> ---

Reviewed-by: Vladimir Oltean <[email protected]>

I hope this moves the patch set out of the 'deferred' state.

---
pw-bot: under-review


>>>>> $ cat /tmp/p.py
#!/bin/env python3

import email
from email.policy import default
import sys

with open(sys.argv[1], 'rb') as fp:
raw = fp.read()

msg = email.message_from_bytes(raw, policy=default)
print(msg.get_body())


>>>>> $ /tmp/p.py raw.5 | tail -20
<20240122-for-netnext-mt7530-improvements-1-v3-6-042401f2b279@arinc9.com>

T24gTW9uLCBKYW4gMjIsIDIwMjQgYXQgMDg6MzU6NTdBTSArMDMwMCwgQXLEsW7DpyDDnE5BTCB2
aWEgQjQgUmVsYXkgd3JvdGU6Cj4gRnJvbTogQXLEsW7DpyDDnE5BTCA8YXJpbmMudW5hbEBhcmlu
YzkuY29tPgo+IAo+IFJ1bm5pbmcgbXQ3NTMwX3NldHVwX3BvcnQ1KCkgZnJvbSBtdDc1MzBfc2V0
dXAoKSB1c2VkIHRvIGhhbmRsZSBhbGwgY2FzZXMKPiBvZiBjb25maWd1cmluZyBwb3J0IDUsIGlu
Y2x1ZGluZyBwaHlsaW5rLgo+IAo+IFNldHRpbmcgcHJpdi0+cDVfaW50ZXJmYWNlIHVuZGVyIG10
NzUzMF9zZXR1cF9wb3J0NSgpIG1ha2VzIHN1cmUgdGhhdAo+IG10NzUzMF9zZXR1cF9wb3J0NSgp
IGZyb20gbXQ3NTN4X3BoeWxpbmtfbWFjX2NvbmZpZygpIHdvbid0IHJ1bi4KPiAKPiBUaGUgY29t
bWl0ICgibmV0OiBkc2E6IG10NzUzMDogaW1wcm92ZSBjb2RlIHBhdGggZm9yIHNldHRpbmcgdXAg
cG9ydCA1IikKPiBtYWtlcyBzbyB0aGF0IG10NzUzMF9zZXR1cF9wb3J0NSgpIGZyb20gbXQ3NTMw
X3NldHVwKCkgcnVucyBvbmx5IG9uCj4gbm9uLXBoeWxpbmsgY2FzZXMuCj4gCj4gR2V0IHJpZCBv
ZiB1bm5lY2Vzc2FyaWx5IHNldHRpbmcgcHJpdi0+cDVfaW50ZXJmYWNlIHVuZGVyCj4gbXQ3NTMw
X3NldHVwX3BvcnQ1KCkgYXMgcG9ydCA1IHBoeWxpbmsgY29uZmlndXJhdGlvbiB3aWxsIGJlIGRv
bmUgYnkKPiBydW5uaW5nIG10NzUzMF9zZXR1cF9wb3J0NSgpIGZyb20gbXQ3NTN4X3BoeWxpbmtf
bWFjX2NvbmZpZygpIG5vdy4KPiAKPiBTaWduZWQtb2ZmLWJ5OiBBcsSxbsOnIMOcTkFMIDxhcmlu
Yy51bmFsQGFyaW5jOS5jb20+Cj4gLS0tCgpSZXZpZXdlZC1ieTogVmxhZGltaXIgT2x0ZWFuIDxv
bHRlYW52QGdtYWlsLmNvbT4KCkkgaG9wZSB0aGlzIG1vdmVzIHRoZSBwYXRjaCBzZXQgb3V0IG9m
IHRoZSAnZGVmZXJyZWQnIHN0YXRlLgoKLS0tCnB3LWJvdDogdW5kZXItcmV2aWV3Cgo=