[PATCH 0/7] net: dsa: mt7530: fix port 5 phylink, phy muxing, and port 6

[PATCH 0/7] net: dsa: mt7530: fix port 5 phylink, phy muxing, and port 6

[arinc9.unal]

Hello!

This patch series is mainly focused on fixing the support for port 5 and
setting up port 6.

The only missing piece to properly support port 5 as a CPU port is the
fixes [0] [1] [2] from Richard.

Russell, looking forward to your review regarding phylink.

I have very thoroughly tested the patch series with every possible mode to
use. I'll let the name of the dtb files speak for themselves.

MT7621 Unielec:

only-gmac0-mt7621-unielec-u7621-06-16m.dtb
rgmii-only-gmac0-mt7621-unielec-u7621-06-16m.dtb
only-gmac1-mt7621-unielec-u7621-06-16m.dtb
gmac0-and-gmac1-mt7621-unielec-u7621-06-16m.dtb
phy0-muxing-mt7621-unielec-u7621-06-16m.dtb
phy4-muxing-mt7621-unielec-u7621-06-16m.dtb
port5-as-user-mt7621-unielec-u7621-06-16m.dtb

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

MT7623 Bananapi:

only-gmac0-mt7623n-bananapi-bpi-r2.dtb
rgmii-only-gmac0-mt7623n-bananapi-bpi-r2.dtb
only-gmac1-mt7623n-bananapi-bpi-r2.dtb
gmac0-and-gmac1-mt7623n-bananapi-bpi-r2.dtb
phy0-muxing-mt7623n-bananapi-bpi-r2.dtb
phy4-muxing-mt7623n-bananapi-bpi-r2.dtb
port5-as-user-mt7623n-bananapi-bpi-r2.dtb

tftpboot 0x80008000 arm-uImage; tftpboot 0x83000000 arm-rootfs.cpio.uboot; tftpboot 0x83f00000 $dtb; bootm 0x80008000 0x83000000 0x83f00000

Current CPU ports setup of MT7530:

mt7530_setup()
-> mt7530_setup_port5()

mt753x_phylink_mac_config()
-> mt753x_mac_config()
   -> mt7530_mac_config()
      -> mt7530_setup_port5()
-> mt753x_pad_setup()
   -> mt7530_pad_clk_setup() sets up port 6, rename to mt7530_setup_port6()

How it will be with the patch series:

mt7530_setup()
-> mt7530_setup_port5() runs if the port is not used as a CPU or user port

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

CPU ports setup of MT7531 for reference:

mt7531_setup()
-> mt753x_cpu_port_enable()
   -> mt7531_cpu_port_config()
      -> mt7531_mac_config()
         -> mt7531_rgmii_setup()
         -> mt7531_sgmii_setup_mode_an()
         -> etc.

mt753x_phylink_mac_config()
-> mt753x_mac_config()
   -> mt7531_mac_config()
      -> mt7531_rgmii_setup()
      -> mt7531_sgmii_setup_mode_an()
      -> etc.

[0] https://lore.kernel.org/netdev/20230212213949.672443-1-richard@routerhints.com/
[1] https://lore.kernel.org/netdev/20230212215152.673221-1-richard@routerhints.com/
[2] https://lore.kernel.org/netdev/20230212214027.672501-1-richard@routerhints.com/

Arınç

Arınç ÜNAL (7):
  net: dsa: mt7530: fix comments regarding port 5 and 6 for both switches
  net: dsa: mt7530: fix phylink for port 5 and fix port 5 modes
  net: dsa: mt7530: do not run mt7530_setup_port5() if port 5 is disabled
  net: dsa: mt7530: set both CPU port interfaces to PHY_INTERFACE_MODE_NA
  net: dsa: mt7530: set up port 5 before CPU ports are enabled
  net: dsa: mt7530: call port 6 setup from mt7530_mac_config()
  net: dsa: mt7530: remove pad_setup function pointer

 drivers/net/dsa/mt7530.c | 154 +++++++++++++++++++-----------------------
 drivers/net/dsa/mt7530.h |   3 -
 2 files changed, 70 insertions(+), 87 deletions(-)

[PATCH net 1/7] net: dsa: mt7530: fix comments regarding port 5 and 6 for both switches

[arinc9.unal]

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

There's no logic to numerically order the CPU ports. State the port number
and its being a CPU port instead.

Remove the irrelevant PHY muxing information from
mt7530_mac_port_get_caps(). Explain the supported MII modes instead.

Remove the out of place PHY muxing information from
mt753x_phylink_mac_config(). The function is for both the MT7530 and MT7531
switches but there's no phy muxing on MT7531.

Fixes: ca366d6c889b ("net: dsa: mt7530: Convert to PHYLINK API")
Fixes: 38f790a80560 ("net: dsa: mt7530: Add support for port 5")
Fixes: 88bdef8be9f6 ("net: dsa: mt7530: Extend device data ready for adding a new hardware")
Fixes: c288575f7810 ("net: dsa: mt7530: Add the support of MT7531 switch")
Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
---
 drivers/net/dsa/mt7530.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 02410ac439b7..62a4b899a961 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -2454,7 +2454,7 @@ static void mt7530_mac_port_get_caps(struct dsa_switch *ds, int port,
 			  config->supported_interfaces);
 		break;
 
-	case 5: /* 2nd cpu port with phy of port 0 or 4 / external phy */
+	case 5: /* Port 5, a CPU port, supports rgmii, mii, and gmii. */
 		phy_interface_set_rgmii(config->supported_interfaces);
 		__set_bit(PHY_INTERFACE_MODE_MII,
 			  config->supported_interfaces);
@@ -2462,7 +2462,7 @@ static void mt7530_mac_port_get_caps(struct dsa_switch *ds, int port,
 			  config->supported_interfaces);
 		break;
 
-	case 6: /* 1st cpu port */
+	case 6: /* Port 6, a CPU port, supports rgmii and trgmii. */
 		__set_bit(PHY_INTERFACE_MODE_RGMII,
 			  config->supported_interfaces);
 		__set_bit(PHY_INTERFACE_MODE_TRGMII,
@@ -2487,14 +2487,14 @@ static void mt7531_mac_port_get_caps(struct dsa_switch *ds, int port,
 			  config->supported_interfaces);
 		break;
 
-	case 5: /* 2nd cpu port supports either rgmii or sgmii/8023z */
+	case 5: /* Port 5, a CPU port, supports rgmii and sgmii/802.3z. */
 		if (mt7531_is_rgmii_port(priv, port)) {
 			phy_interface_set_rgmii(config->supported_interfaces);
 			break;
 		}
 		fallthrough;
 
-	case 6: /* 1st cpu port supports sgmii/8023z only */
+	case 6: /* Port 6, a CPU port, supports sgmii/802.3z only. */
 		__set_bit(PHY_INTERFACE_MODE_SGMII,
 			  config->supported_interfaces);
 		__set_bit(PHY_INTERFACE_MODE_1000BASEX,
@@ -2772,7 +2772,7 @@ mt753x_phylink_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
 		if (state->interface != PHY_INTERFACE_MODE_GMII)
 			goto unsupported;
 		break;
-	case 5: /* 2nd cpu port with phy of port 0 or 4 / external phy */
+	case 5: /* Port 5, a CPU port. */
 		if (priv->p5_interface == state->interface)
 			break;
 
@@ -2782,7 +2782,7 @@ mt753x_phylink_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
 		if (priv->p5_intf_sel != P5_DISABLED)
 			priv->p5_interface = state->interface;
 		break;
-	case 6: /* 1st cpu port */
+	case 6: /* Port 6, a CPU port. */
 		if (priv->p6_interface == state->interface)
 			break;
 
-- 
2.37.2

[PATCH net 2/7] net: dsa: mt7530: fix phylink for port 5 and fix port 5 modes

[arinc9.unal]

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

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

mt7530_setup()
-> mt7530_setup_port5()

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

The first call path is supposed to run when phy muxing is being used. In
this case, port 5 is somewhat of a hidden port. It won't be defined on the
devicetree so phylink can't be used to manage the port.

The second call path used to call mt7530_setup_port5() directly under case
5 on mt7530_phylink_mac_config() before it was moved to mt7530_mac_config()
with 88bdef8be9f6 ("net: dsa: mt7530: Extend device data ready for adding a
new hardware"). mt7530_setup_port5() will never run through this call path
because the current code on mt7530_setup() bypasses phylink for all cases
of port 5.

Leave it to phylink if port 5 is used as a CPU port or a user port. For the
cases of phy muxing or the port being disabled, call mt7530_setup_port5()
directly from mt7530_setup_port5() without involving phylink.

Move setting the interface and P5_DISABLED mode to a more specific
location. They're supposed to be overwritten if phy muxing is detected.

Add comments which explain the process.

Fixes: 38f790a80560 ("net: dsa: mt7530: Add support for port 5")
Tested-by: Arınç ÜNAL <arinc.unal@arinc9.com>
Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
---
 drivers/net/dsa/mt7530.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 62a4b899a961..eba356249ada 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -2247,16 +2247,18 @@ 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)) {
+		/* Set the interface selection of port 5 to GMAC5 when it's used
+		 * as a CPU port or a user port. Let phylink handle the rest.
+		 */
 		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.
+		 * Determine if phy muxing is defined and which phy to mux.
+		 */
+		priv->p5_intf_sel = P5_DISABLED;
+		interface = PHY_INTERFACE_MODE_NA;
+
 		for_each_child_of_node(dn, mac_np) {
 			if (!of_device_is_compatible(mac_np,
 						     "mediatek,eth-mac"))
@@ -2287,6 +2289,8 @@ mt7530_setup(struct dsa_switch *ds)
 			of_node_put(phy_node);
 			break;
 		}
+
+		mt7530_setup_port5(ds, interface);
 	}
 
 #ifdef CONFIG_GPIOLIB
@@ -2297,8 +2301,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.37.2

Re: [PATCH net 2/7] net: dsa: mt7530: fix phylink for port 5 and fix port 5 modes

[Vladimir Oltean]

On Sun, Mar 26, 2023 at 05:08:13PM +0300, arinc9.unal@gmail.com wrote:
> From: Arınç ÜNAL <arinc.unal@arinc9.com>
> 
> There're two call paths for setting up port 5:
> 
> mt7530_setup()
> -> mt7530_setup_port5()
> 
> mt753x_phylink_mac_config()
> -> mt753x_mac_config()
>    -> mt7530_mac_config()
>       -> mt7530_setup_port5()
> 
> The first call path is supposed to run when phy muxing is being used. In
> this case, port 5 is somewhat of a hidden port. It won't be defined on the
> devicetree so phylink can't be used to manage the port.
> 
> The second call path used to call mt7530_setup_port5() directly under case
> 5 on mt7530_phylink_mac_config() before it was moved to mt7530_mac_config()
> with 88bdef8be9f6 ("net: dsa: mt7530: Extend device data ready for adding a
> new hardware"). mt7530_setup_port5() will never run through this call path
> because the current code on mt7530_setup() bypasses phylink for all cases
> of port 5.
> 
> Leave it to phylink if port 5 is used as a CPU port or a user port. For the
> cases of phy muxing or the port being disabled, call mt7530_setup_port5()
> directly from mt7530_setup_port5() without involving phylink.

You probably don't mean "call X() from X()" (that would make it recursive),
but maybe from mt7530_setup(). But it was already called from mt7530_setup(),
so I don't understand what is being transmitted here...

> 
> Move setting the interface and P5_DISABLED mode to a more specific
> location. They're supposed to be overwritten if phy muxing is detected.
> 
> Add comments which explain the process.
> 
> Fixes: 38f790a80560 ("net: dsa: mt7530: Add support for port 5")
> Tested-by: Arınç ÜNAL <arinc.unal@arinc9.com>
> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
> ---

Sorry, I didn't understand... so what was the problem, and how does the
movement of the mt7530_setup_port5() call that isn't under phylink solve
that problem?

Re: [PATCH net 2/7] net: dsa: mt7530: fix phylink for port 5 and fix port 5 modes

[Arınç ÜNAL]

On 27.03.2023 21:49, Vladimir Oltean wrote:
> On Sun, Mar 26, 2023 at 05:08:13PM +0300, arinc9.unal@gmail.com wrote:
>> From: Arınç ÜNAL <arinc.unal@arinc9.com>
>>
>> There're two call paths for setting up port 5:
>>
>> mt7530_setup()
>> -> mt7530_setup_port5()
>>
>> mt753x_phylink_mac_config()
>> -> mt753x_mac_config()
>>     -> mt7530_mac_config()
>>        -> mt7530_setup_port5()
>>
>> The first call path is supposed to run when phy muxing is being used. In
>> this case, port 5 is somewhat of a hidden port. It won't be defined on the
>> devicetree so phylink can't be used to manage the port.
>>
>> The second call path used to call mt7530_setup_port5() directly under case
>> 5 on mt7530_phylink_mac_config() before it was moved to mt7530_mac_config()
>> with 88bdef8be9f6 ("net: dsa: mt7530: Extend device data ready for adding a
>> new hardware"). mt7530_setup_port5() will never run through this call path
>> because the current code on mt7530_setup() bypasses phylink for all cases
>> of port 5.
>>
>> Leave it to phylink if port 5 is used as a CPU port or a user port. For the
>> cases of phy muxing or the port being disabled, call mt7530_setup_port5()
>> directly from mt7530_setup_port5() without involving phylink.
> 
> You probably don't mean "call X() from X()" (that would make it recursive),
> but maybe from mt7530_setup(). But it was already called from mt7530_setup(),
> so I don't understand what is being transmitted here...

Oops, I meant to say call mt7530_setup_port5() directly from 
mt7530_setup() without involving phylink. Will fix.

> 
>>
>> Move setting the interface and P5_DISABLED mode to a more specific
>> location. They're supposed to be overwritten if phy muxing is detected.
>>
>> Add comments which explain the process.
>>
>> Fixes: 38f790a80560 ("net: dsa: mt7530: Add support for port 5")
>> Tested-by: Arınç ÜNAL <arinc.unal@arinc9.com>
>> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
>> ---
> 
> Sorry, I didn't understand... so what was the problem, and how does the
> movement of the mt7530_setup_port5() call that isn't under phylink solve
> that problem?

Port 5 being used as a CPU or user port was being set up from 
mt7530_setup() instead of using phylink. This patch fixes that.

Arınç

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

[arinc9.unal]

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

There's no need to run all the code on mt7530_setup_port5() if port 5 is
disabled. Run mt7530_setup_port5() if priv->p5_intf_sel is not P5_DISABLED
and 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.

Fixes: 38f790a80560 ("net: dsa: mt7530: Add support for port 5")
Tested-by: Arınç ÜNAL <arinc.unal@arinc9.com>
Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
---
 drivers/net/dsa/mt7530.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index eba356249ada..6d33c1050458 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -949,9 +949,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);
@@ -2257,7 +2254,6 @@ mt7530_setup(struct dsa_switch *ds)
 		 * Determine if phy muxing is defined and which phy to mux.
 		 */
 		priv->p5_intf_sel = P5_DISABLED;
-		interface = PHY_INTERFACE_MODE_NA;
 
 		for_each_child_of_node(dn, mac_np) {
 			if (!of_device_is_compatible(mac_np,
@@ -2290,7 +2286,8 @@ mt7530_setup(struct dsa_switch *ds)
 			break;
 		}
 
-		mt7530_setup_port5(ds, interface);
+		if (priv->p5_intf_sel != P5_DISABLED)
+			mt7530_setup_port5(ds, interface);
 	}
 
 #ifdef CONFIG_GPIOLIB
-- 
2.37.2

Re: [PATCH net 3/7] net: dsa: mt7530: do not run mt7530_setup_port5() if port 5 is disabled

[Vladimir Oltean]

On Sun, Mar 26, 2023 at 05:08:14PM +0300, arinc9.unal@gmail.com wrote:
> From: Arınç ÜNAL <arinc.unal@arinc9.com>
> 
> There's no need to run all the code on mt7530_setup_port5() if port 5 is
> disabled. Run mt7530_setup_port5() if priv->p5_intf_sel is not P5_DISABLED
> and 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.
> 
> Fixes: 38f790a80560 ("net: dsa: mt7530: Add support for port 5")
> Tested-by: Arınç ÜNAL <arinc.unal@arinc9.com>
> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
> ---

Again, not sure what is the problem, and how this solution addresses
that problem. I see Fixes tags for all patches, but I don't understand
what they fix, what didn't work before that works now?

Re: [PATCH net 3/7] net: dsa: mt7530: do not run mt7530_setup_port5() if port 5 is disabled

[Arınç ÜNAL]

On 27.03.2023 21:56, Vladimir Oltean wrote:
> On Sun, Mar 26, 2023 at 05:08:14PM +0300, arinc9.unal@gmail.com wrote:
>> From: Arınç ÜNAL <arinc.unal@arinc9.com>
>>
>> There's no need to run all the code on mt7530_setup_port5() if port 5 is
>> disabled. Run mt7530_setup_port5() if priv->p5_intf_sel is not P5_DISABLED
>> and 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.
>>
>> Fixes: 38f790a80560 ("net: dsa: mt7530: Add support for port 5")
>> Tested-by: Arınç ÜNAL <arinc.unal@arinc9.com>
>> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
>> ---
> 
> Again, not sure what is the problem, and how this solution addresses
> that problem. I see Fixes tags for all patches, but I don't understand
> what they fix, what didn't work before that works now?

It really depends on what you call working. Does this patch fix any 
feature of the switch that didn't work before? No.

Does it fix a bad logic introduced with the said commit? Yes.

Arınç

[PATCH net 4/7] net: dsa: mt7530: set both CPU port interfaces to PHY_INTERFACE_MODE_NA

[arinc9.unal]

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

Set interfaces of both CPU ports to PHY_INTERFACE_MODE_NA. Either phylink
or mt7530_setup_port5() on mt7530_setup() will handle the rest.

This is already being done for port 6, do it for port 5 as well.

Fixes: 38f790a80560 ("net: dsa: mt7530: Add support for port 5")
Tested-by: Arınç ÜNAL <arinc.unal@arinc9.com>
Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
---
 drivers/net/dsa/mt7530.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 6d33c1050458..3deebdcfeedf 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -2203,14 +2203,18 @@ mt7530_setup(struct dsa_switch *ds)
 		mt7530_rmw(priv, MT7530_TRGMII_RD(i),
 			   RD_TAP_MASK, RD_TAP(16));
 
+	/* Let phylink decide the interface later. If port 5 is used for phy
+	 * muxing, its interface will be handled without involving phylink.
+	 */
+	priv->p5_interface = PHY_INTERFACE_MODE_NA;
+	priv->p6_interface = PHY_INTERFACE_MODE_NA;
+
 	/* Enable port 6 */
 	val = mt7530_read(priv, MT7530_MHWTRAP);
 	val &= ~MHWTRAP_P6_DIS & ~MHWTRAP_PHY_ACCESS;
 	val |= MHWTRAP_MANUAL;
 	mt7530_write(priv, MT7530_MHWTRAP, val);
 
-	priv->p6_interface = PHY_INTERFACE_MODE_NA;
-
 	/* Enable and reset MIB counters */
 	mt7530_mib_reset(ds);
 
-- 
2.37.2

Re: [PATCH net 4/7] net: dsa: mt7530: set both CPU port interfaces to PHY_INTERFACE_MODE_NA

[Vladimir Oltean]

On Sun, Mar 26, 2023 at 05:08:15PM +0300, arinc9.unal@gmail.com wrote:
> From: Arınç ÜNAL <arinc.unal@arinc9.com>
> 
> Set interfaces of both CPU ports to PHY_INTERFACE_MODE_NA. Either phylink
> or mt7530_setup_port5() on mt7530_setup() will handle the rest.
> 
> This is already being done for port 6, do it for port 5 as well.
> 
> Fixes: 38f790a80560 ("net: dsa: mt7530: Add support for port 5")

This is getting comical.. I think I'm putting too much energy in
trying to understand the hidden meaning of this patch set.

In include/linux/phy.h we have:

typedef enum {
	PHY_INTERFACE_MODE_NA,

In lack of other initializer, the first element of an enum gets the
value 0 in C.

Then, "priv" is allocated by this driver with devm_kzalloc(), which
means that its entire memory is zero-filled. So priv->p5_interface and
priv->p6_interface are already set to 0, or PHY_INTERFACE_MODE_NA.

There is no code path between the devm_kzalloc() and the position in
mt7530_setup() that would change the value of priv->p5_interface or
priv->p6_interface from their value of 0 (PHY_INTERFACE_MODE_NA).
For example, mt753x_phylink_mac_config() can only be called from
phylink, after dsa_port_phylink_create() was called. But
dsa_port_phylink_create() comes later than ds->ops->setup() - one comes
from dsa_tree_setup_ports(), and the other from dsa_tree_setup_switches().

The movement of the priv->p6_interface assignment with a few lines
earlier does not change anything relative to the other call sites which
assign different values to priv->p6_interface, so there isn't any
functional change there, either.

So this patch is putting 0 into a variable containing 0, and claiming,
through the presence of the Fixes: tag and the submission to the "net"
tree, that it is a bug fix which should be backported to "stable".

Can it be that you are abusing the meaning of a "bug fix", and that I'm
trying too hard to take this patch set seriously?

> Tested-by: Arınç ÜNAL <arinc.unal@arinc9.com>
> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
> ---
>  drivers/net/dsa/mt7530.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
> index 6d33c1050458..3deebdcfeedf 100644
> --- a/drivers/net/dsa/mt7530.c
> +++ b/drivers/net/dsa/mt7530.c
> @@ -2203,14 +2203,18 @@ mt7530_setup(struct dsa_switch *ds)
>  		mt7530_rmw(priv, MT7530_TRGMII_RD(i),
>  			   RD_TAP_MASK, RD_TAP(16));
>  
> +	/* Let phylink decide the interface later. If port 5 is used for phy
> +	 * muxing, its interface will be handled without involving phylink.
> +	 */
> +	priv->p5_interface = PHY_INTERFACE_MODE_NA;
> +	priv->p6_interface = PHY_INTERFACE_MODE_NA;
> +
>  	/* Enable port 6 */
>  	val = mt7530_read(priv, MT7530_MHWTRAP);
>  	val &= ~MHWTRAP_P6_DIS & ~MHWTRAP_PHY_ACCESS;
>  	val |= MHWTRAP_MANUAL;
>  	mt7530_write(priv, MT7530_MHWTRAP, val);
>  
> -	priv->p6_interface = PHY_INTERFACE_MODE_NA;
> -
>  	/* Enable and reset MIB counters */
>  	mt7530_mib_reset(ds);
>  
> -- 
> 2.37.2
> 

Re: [PATCH net 4/7] net: dsa: mt7530: set both CPU port interfaces to PHY_INTERFACE_MODE_NA

[Arınç ÜNAL]

On 27.03.2023 22:12, Vladimir Oltean wrote:
> On Sun, Mar 26, 2023 at 05:08:15PM +0300, arinc9.unal@gmail.com wrote:
>> From: Arınç ÜNAL <arinc.unal@arinc9.com>
>>
>> Set interfaces of both CPU ports to PHY_INTERFACE_MODE_NA. Either phylink
>> or mt7530_setup_port5() on mt7530_setup() will handle the rest.
>>
>> This is already being done for port 6, do it for port 5 as well.
>>
>> Fixes: 38f790a80560 ("net: dsa: mt7530: Add support for port 5")
> 
> This is getting comical.. I think I'm putting too much energy in
> trying to understand the hidden meaning of this patch set.
> 
> In include/linux/phy.h we have:
> 
> typedef enum {
> 	PHY_INTERFACE_MODE_NA,
> 
> In lack of other initializer, the first element of an enum gets the
> value 0 in C.
> 
> Then, "priv" is allocated by this driver with devm_kzalloc(), which
> means that its entire memory is zero-filled. So priv->p5_interface and
> priv->p6_interface are already set to 0, or PHY_INTERFACE_MODE_NA.
> 
> There is no code path between the devm_kzalloc() and the position in
> mt7530_setup() that would change the value of priv->p5_interface or
> priv->p6_interface from their value of 0 (PHY_INTERFACE_MODE_NA).
> For example, mt753x_phylink_mac_config() can only be called from
> phylink, after dsa_port_phylink_create() was called. But
> dsa_port_phylink_create() comes later than ds->ops->setup() - one comes
> from dsa_tree_setup_ports(), and the other from dsa_tree_setup_switches().
> 
> The movement of the priv->p6_interface assignment with a few lines
> earlier does not change anything relative to the other call sites which
> assign different values to priv->p6_interface, so there isn't any
> functional change there, either.
> 
> So this patch is putting 0 into a variable containing 0, and claiming,
> through the presence of the Fixes: tag and the submission to the "net"
> tree, that it is a bug fix which should be backported to "stable".
> 
> Can it be that you are abusing the meaning of a "bug fix", and that I'm
> trying too hard to take this patch set seriously?

I don't appreciate your consistent use of the word "abuse" on my 
patches. I'm by no means a senior C programmer. I'm doing my best to 
correct the driver.

Thank you for explaining the process of phylink with DSA, I will adjust 
my patches accordingly.

I suggest you don't take my patches seriously for a while, until I know 
better.

Arınç

Re: [PATCH net 4/7] net: dsa: mt7530: set both CPU port interfaces to PHY_INTERFACE_MODE_NA

[Jakub Kicinski]

On Tue, 28 Mar 2023 00:57:57 +0300 Arınç ÜNAL wrote:
> I don't appreciate your consistent use of the word "abuse" on my 
> patches. I'm by no means a senior C programmer. I'm doing my best to 
> correct the driver.
> 
> Thank you for explaining the process of phylink with DSA, I will adjust 
> my patches accordingly.
> 
> I suggest you don't take my patches seriously for a while, until I know 
> better.

Maybe my bad, I should have sent you a note on your previous series
already. The patches may be fine, but the commit messages need to do
a better job of describing what the goal of the change is, functionally.

For fixes the bar is even higher because, as Vladimir points out,
commit messages for fixes need to explain what user visible problem 
the patch is resolving. Think of it as a letter to a person using the
switch who hits a problem and wants to look thru the upstream commits
to see if it's already fixed.

Re: [PATCH net 4/7] net: dsa: mt7530: set both CPU port interfaces to PHY_INTERFACE_MODE_NA

[Vladimir Oltean]

On Tue, Mar 28, 2023 at 12:57:57AM +0300, Arınç ÜNAL wrote:
> I don't appreciate your consistent use of the word "abuse" on my patches.

Consistent would mean that, when given the same kind of input, I respond
with the same kind of output. I'm thinking you'd want a reviewer to do that?

Last time I said: "It's best not to abuse the net.git tree with non-bugfix patches."
https://patchwork.kernel.org/project/netdevbpf/patch/20230307220328.11186-1-arinc.unal@arinc9.com/

If anything, Jakub was/is slightly inconsistent by accepting those previous
non-bugfix patches to the net.git tree, and then agreeing with me. He probably
did that thinking it wasn't a hill worth dying on, which I can agree with.
But I'm afraid that this didn't help you realize that yes, maybe you really
are abusing the process by submitting exclusively non-bugfix commits to the
net tree. There's a fine balance between trying to be nice and trying not to
transmit the wrong message.

It would be good if you could clarify your objection regarding my consistent
use of the word "abuse" on your patches.

There is a document at Documentation/process/stable-kernel-rules.rst
which I remember having shared with you before, where there are some
indications as to what constitutes a legitimate candidate for "stable"
and what does not.

> I'm by no means a senior C programmer. I'm doing my best to correct the
> driver.
> 
> Thank you for explaining the process of phylink with DSA, I will adjust my
> patches accordingly.
> 
> I suggest you don't take my patches seriously for a while, until I know
> better.

Whether you're a junior or a senior C programmer is entirely irrelevant
here. I have no choice but to take your patches seriously unless otherwise
specified, in the commit message, cover letter, or by marking them as
RFC/RFT (but even then, their intention must be very clearly specified,
so that I know what to comment on, or test).

I don't think you really want what you're asking for, which is for
people to not take your patches seriously. I recommend forming a smaller
community of people which does preliminary patch review and discusses
issues around the hardware you're working on, prior to upstream submission.
That would, at least, be more productive.

Re: [PATCH net 4/7] net: dsa: mt7530: set both CPU port interfaces to PHY_INTERFACE_MODE_NA

[Arınç ÜNAL]

On 28/03/2023 14:20, Vladimir Oltean wrote:
> On Tue, Mar 28, 2023 at 12:57:57AM +0300, Arınç ÜNAL wrote:
>> I don't appreciate your consistent use of the word "abuse" on my patches.
> 
> Consistent would mean that, when given the same kind of input, I respond
> with the same kind of output. I'm thinking you'd want a reviewer to do that?

Of course.

> 
> Last time I said: "It's best not to abuse the net.git tree with non-bugfix patches."
> https://patchwork.kernel.org/project/netdevbpf/patch/20230307220328.11186-1-arinc.unal@arinc9.com/
> 
> If anything, Jakub was/is slightly inconsistent by accepting those previous
> non-bugfix patches to the net.git tree, and then agreeing with me. He probably
> did that thinking it wasn't a hill worth dying on, which I can agree with.
> But I'm afraid that this didn't help you realize that yes, maybe you really
> are abusing the process by submitting exclusively non-bugfix commits to the
> net tree. There's a fine balance between trying to be nice and trying not to
> transmit the wrong message.
> 
> It would be good if you could clarify your objection regarding my consistent
> use of the word "abuse" on your patches.
> 
> There is a document at Documentation/process/stable-kernel-rules.rst
> which I remember having shared with you before, where there are some
> indications as to what constitutes a legitimate candidate for "stable"
> and what does not.

I forgot this existed, sorry about that. I had this thought left in my 
mind that "any changes that are not new features must go to the net 
tree", which clearly is not the case. I see what you mean now. None of 
my patches on the series satisfy all of the rules specified on the document.

I just think your response could've been less harsh considering I didn't 
intentionally do this. Anyway, it's all resolved now so let's not drag 
this further.

> 
>> I'm by no means a senior C programmer. I'm doing my best to correct the
>> driver.
>>
>> Thank you for explaining the process of phylink with DSA, I will adjust my
>> patches accordingly.
>>
>> I suggest you don't take my patches seriously for a while, until I know
>> better.
> 
> Whether you're a junior or a senior C programmer is entirely irrelevant
> here. I have no choice but to take your patches seriously unless otherwise
> specified, in the commit message, cover letter, or by marking them as
> RFC/RFT (but even then, their intention must be very clearly specified,
> so that I know what to comment on, or test).
> 
> I don't think you really want what you're asking for, which is for
> people to not take your patches seriously. I recommend forming a smaller
> community of people which does preliminary patch review and discusses
> issues around the hardware you're working on, prior to upstream submission.
> That would, at least, be more productive.

Yes, of course. I'm actually planning something similar that involves 
OpenWrt, thanks.

Arınç

[PATCH net 5/7] net: dsa: mt7530: set up port 5 before CPU ports are enabled

[arinc9.unal]

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

Set priv->p5_intf_sel before the CPU ports are enabled.

This makes sure the 'if (priv->p5_intf_sel != P5_DISABLED)' check on
mt753x_phylink_mac_config() runs with priv->p5_intf_sel initialised.

Set up port 5 for phy muxing right after priv->p5_interface is set to
PHY_INTERFACE_MODE_NA.

Fixes: 38f790a80560 ("net: dsa: mt7530: Add support for port 5")
Tested-by: Arınç ÜNAL <arinc.unal@arinc9.com>
Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
---
 drivers/net/dsa/mt7530.c | 76 ++++++++++++++++++++--------------------
 1 file changed, 38 insertions(+), 38 deletions(-)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 3deebdcfeedf..2397d63cec29 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -2209,44 +2209,6 @@ mt7530_setup(struct dsa_switch *ds)
 	priv->p5_interface = PHY_INTERFACE_MODE_NA;
 	priv->p6_interface = PHY_INTERFACE_MODE_NA;
 
-	/* Enable port 6 */
-	val = mt7530_read(priv, MT7530_MHWTRAP);
-	val &= ~MHWTRAP_P6_DIS & ~MHWTRAP_PHY_ACCESS;
-	val |= MHWTRAP_MANUAL;
-	mt7530_write(priv, MT7530_MHWTRAP, val);
-
-	/* Enable and reset MIB counters */
-	mt7530_mib_reset(ds);
-
-	for (i = 0; i < MT7530_NUM_PORTS; i++) {
-		/* Disable forwarding by default on all ports */
-		mt7530_rmw(priv, MT7530_PCR_P(i), PCR_MATRIX_MASK,
-			   PCR_MATRIX_CLR);
-
-		/* Disable learning by default on all ports */
-		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;
-		} else {
-			mt7530_port_disable(ds, i);
-
-			/* Set default PVID to 0 on all user ports */
-			mt7530_rmw(priv, MT7530_PPBV1_P(i), G0_PORT_VID_MASK,
-				   G0_PORT_VID_DEF);
-		}
-		/* Enable consistent egress tag */
-		mt7530_rmw(priv, MT7530_PVC_P(i), PVC_EG_TAG_MASK,
-			   PVC_EG_TAG(MT7530_VLAN_EG_CONSISTENT));
-	}
-
-	/* Setup VLAN ID 0 for VLAN-unaware bridges */
-	ret = mt7530_setup_vlan0(priv);
-	if (ret)
-		return ret;
-
 	/* Setup port 5 */
 	if (!dsa_is_unused_port(ds, 5)) {
 		/* Set the interface selection of port 5 to GMAC5 when it's used
@@ -2294,6 +2256,44 @@ mt7530_setup(struct dsa_switch *ds)
 			mt7530_setup_port5(ds, interface);
 	}
 
+	/* Enable port 6 */
+	val = mt7530_read(priv, MT7530_MHWTRAP);
+	val &= ~MHWTRAP_P6_DIS & ~MHWTRAP_PHY_ACCESS;
+	val |= MHWTRAP_MANUAL;
+	mt7530_write(priv, MT7530_MHWTRAP, val);
+
+	/* Enable and reset MIB counters */
+	mt7530_mib_reset(ds);
+
+	for (i = 0; i < MT7530_NUM_PORTS; i++) {
+		/* Disable forwarding by default on all ports */
+		mt7530_rmw(priv, MT7530_PCR_P(i), PCR_MATRIX_MASK,
+			   PCR_MATRIX_CLR);
+
+		/* Disable learning by default on all ports */
+		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;
+		} else {
+			mt7530_port_disable(ds, i);
+
+			/* Set default PVID to 0 on all user ports */
+			mt7530_rmw(priv, MT7530_PPBV1_P(i), G0_PORT_VID_MASK,
+				   G0_PORT_VID_DEF);
+		}
+		/* Enable consistent egress tag */
+		mt7530_rmw(priv, MT7530_PVC_P(i), PVC_EG_TAG_MASK,
+			   PVC_EG_TAG(MT7530_VLAN_EG_CONSISTENT));
+	}
+
+	/* Setup VLAN ID 0 for VLAN-unaware bridges */
+	ret = mt7530_setup_vlan0(priv);
+	if (ret)
+		return ret;
+
 #ifdef CONFIG_GPIOLIB
 	if (of_property_read_bool(priv->dev->of_node, "gpio-controller")) {
 		ret = mt7530_setup_gpio(priv);
-- 
2.37.2

[PATCH net 6/7] net: dsa: mt7530: call port 6 setup from mt7530_mac_config()

[arinc9.unal]

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

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().

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

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

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

This is after:

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

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

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 2397d63cec29..8d49803f7522 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -421,7 +421,7 @@ static void mt7530_pll_setup(struct mt7530_priv *priv)
 
 /* Setup port 6 interface mode and TRGMII TX circuit */
 static int
-mt7530_pad_clk_setup(struct dsa_switch *ds, phy_interface_t interface)
+mt7530_setup_port6(struct dsa_switch *ds, phy_interface_t interface)
 {
 	struct mt7530_priv *priv = ds->priv;
 	u32 ncpo1, ssc_delta, trgint, xtal;
@@ -493,6 +493,12 @@ mt7530_pad_clk_setup(struct dsa_switch *ds, phy_interface_t interface)
 	return 0;
 }
 
+static int
+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;
@@ -2523,12 +2529,15 @@ mt7530_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
 		  phy_interface_t interface)
 {
 	struct mt7530_priv *priv = ds->priv;
+	int ret;
 
-	/* 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) {
+		ret = mt7530_setup_port6(priv->ds, interface);
+		if (ret)
+			return ret;
+	}
 
 	return 0;
 }
-- 
2.37.2

[PATCH net 7/7] net: dsa: mt7530: remove pad_setup function pointer

[arinc9.unal]

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

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 so this function
pointer hasn't got a use anymore. Remove it.

Fixes: 88bdef8be9f6 ("net: dsa: mt7530: Extend device data ready for adding a new hardware")
Tested-by: Arınç ÜNAL <arinc.unal@arinc9.com>
Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
---
 drivers/net/dsa/mt7530.c | 30 ++----------------------------
 drivers/net/dsa/mt7530.h |  3 ---
 2 files changed, 2 insertions(+), 31 deletions(-)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 8d49803f7522..83dcd888f82b 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -493,12 +493,6 @@ mt7530_setup_port6(struct dsa_switch *ds, phy_interface_t interface)
 	return 0;
 }
 
-static int
-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;
@@ -508,12 +502,6 @@ static bool mt7531_dual_sgmii_supported(struct mt7530_priv *priv)
 	return (val & PAD_DUAL_SGMII_EN) != 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)
 {
@@ -2516,14 +2504,6 @@ static void mt7531_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)
@@ -2798,8 +2778,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;
 
@@ -3215,7 +3193,6 @@ static 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,
 	},
@@ -3227,7 +3204,6 @@ static 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,
 	},
@@ -3239,7 +3215,6 @@ static 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,
@@ -3297,9 +3272,8 @@ mt7530_probe(struct mdio_device *mdiodev)
 	/* 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 6b2fc6290ea8..fd050d3110c6 100644
--- a/drivers/net/dsa/mt7530.h
+++ b/drivers/net/dsa/mt7530.h
@@ -754,8 +754,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
@@ -776,7 +774,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.37.2

Re: [PATCH 0/7] net: dsa: mt7530: fix port 5 phylink, phy muxing, and port 6

[Vladimir Oltean]

On Sun, Mar 26, 2023 at 05:08:11PM +0300, arinc9.unal@gmail.com wrote:
> Hello!
> 
> This patch series is mainly focused on fixing the support for port 5 and
> setting up port 6.
> 
> The only missing piece to properly support port 5 as a CPU port is the
> fixes [0] [1] [2] from Richard.
> 
> Russell, looking forward to your review regarding phylink.
> 
> I have very thoroughly tested the patch series with every possible mode to
> use. I'll let the name of the dtb files speak for themselves.

This patch needs to be resubmitted (potentially as RFC) to the net-next
tree, and the commits readjusted to clarify what are fixes and what is
pure refactoring. With the mentioned that I have not reviewed the entire
series and I will not do that, either.

Re: [PATCH 0/7] net: dsa: mt7530: fix port 5 phylink, phy muxing, and port 6

[Arınç ÜNAL]

On 28/03/2023 14:21, Vladimir Oltean wrote:
> On Sun, Mar 26, 2023 at 05:08:11PM +0300, arinc9.unal@gmail.com wrote:
>> Hello!
>>
>> This patch series is mainly focused on fixing the support for port 5 and
>> setting up port 6.
>>
>> The only missing piece to properly support port 5 as a CPU port is the
>> fixes [0] [1] [2] from Richard.
>>
>> Russell, looking forward to your review regarding phylink.
>>
>> I have very thoroughly tested the patch series with every possible mode to
>> use. I'll let the name of the dtb files speak for themselves.
> 
> This patch needs to be resubmitted (potentially as RFC) to the net-next
> tree, and the commits readjusted to clarify what are fixes and what is
> pure refactoring. With the mentioned that I have not reviewed the entire
> series and I will not do that, either.

Makes sense, will do.

Arınç