diff mbox series

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

Message ID 20230326140818.246575-4-arinc.unal@arinc9.com (mailing list archive)
State New, archived
Headers show
Series net: dsa: mt7530: fix port 5 phylink, phy muxing, and port 6 | expand

Commit Message

Arınç ÜNAL March 26, 2023, 2:08 p.m. UTC 
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(-)

Comments

Vladimir Oltean March 27, 2023, 6:56 p.m. UTC | #1 
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?
Arınç ÜNAL March 27, 2023, 9:46 p.m. UTC | #2 
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ç
diff mbox series

Patch

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