[PATCH net 1/2] net: dsa: sja1105: fix SGMII PCS being forced to SPEED_UNKNOWN instead of SPEED_10

[PATCH net 1/2] net: dsa: sja1105: fix SGMII PCS being forced to SPEED_UNKNOWN instead of SPEED_10

[Vladimir Oltean]

From: Vladimir Oltean <vladimir.oltean@nxp.com>

When using MLO_AN_PHY or MLO_AN_FIXED, the MII_BMCR of the SGMII PCS is
read before resetting the switch so it can be reprogrammed afterwards.
This works for the speeds of 1Gbps and 100Mbps, but not for 10Mbps,
because SPEED_10 is actually 0, so AND-ing anything with 0 is false,
therefore that last branch is dead code.

Do what others do (genphy_read_status_fixed, phy_mii_ioctl) and just
remove the check for SPEED_10, let it fall into the default case.

Fixes: ffe10e679cec ("net: dsa: sja1105: Add support for the SGMII port")
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/dsa/sja1105/sja1105_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index 7692338730df..c1982615c631 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -1922,7 +1922,7 @@ int sja1105_static_config_reload(struct sja1105_private *priv,
 				speed = SPEED_1000;
 			else if (bmcr & BMCR_SPEED100)
 				speed = SPEED_100;
-			else if (bmcr & BMCR_SPEED10)
+			else
 				speed = SPEED_10;
 
 			sja1105_sgmii_pcs_force_speed(priv, speed);
-- 
2.25.1

[PATCH net 2/2] net: dsa: sja1105: fix ucast/bcast flooding always remaining enabled

[Vladimir Oltean]

From: Vladimir Oltean <vladimir.oltean@nxp.com>

In the blamed patch I managed to introduce a bug while moving code
around: the same logic is applied to the ucast_egress_floods and
bcast_egress_floods variables both on the "if" and the "else" branches.

This is clearly an unintended change compared to how the code used to be
prior to that bugfix, so restore it.

Fixes: 7f7ccdea8c73 ("net: dsa: sja1105: fix leakage of flooded frames outside bridging domain")
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/dsa/sja1105/sja1105_main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index c1982615c631..51ea104c63bb 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -3369,14 +3369,14 @@ static int sja1105_port_ucast_bcast_flood(struct sja1105_private *priv, int to,
 		if (flags.val & BR_FLOOD)
 			priv->ucast_egress_floods |= BIT(to);
 		else
-			priv->ucast_egress_floods |= BIT(to);
+			priv->ucast_egress_floods &= ~BIT(to);
 	}
 
 	if (flags.mask & BR_BCAST_FLOOD) {
 		if (flags.val & BR_BCAST_FLOOD)
 			priv->bcast_egress_floods |= BIT(to);
 		else
-			priv->bcast_egress_floods |= BIT(to);
+			priv->bcast_egress_floods &= ~BIT(to);
 	}
 
 	return sja1105_manage_flood_domains(priv);
-- 
2.25.1

Re: [PATCH net 2/2] net: dsa: sja1105: fix ucast/bcast flooding always remaining enabled

[Andrew Lunn]

On Thu, Mar 04, 2021 at 12:56:54PM +0200, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> In the blamed patch I managed to introduce a bug while moving code
> around: the same logic is applied to the ucast_egress_floods and
> bcast_egress_floods variables both on the "if" and the "else" branches.

Some static analysers will report this.
 
> This is clearly an unintended change compared to how the code used to be
> prior to that bugfix, so restore it.
> 
> Fixes: 7f7ccdea8c73 ("net: dsa: sja1105: fix leakage of flooded frames outside bridging domain")
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH net 1/2] net: dsa: sja1105: fix SGMII PCS being forced to SPEED_UNKNOWN instead of SPEED_10

[Andrew Lunn]

On Thu, Mar 04, 2021 at 12:56:53PM +0200, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> When using MLO_AN_PHY or MLO_AN_FIXED, the MII_BMCR of the SGMII PCS is
> read before resetting the switch so it can be reprogrammed afterwards.
> This works for the speeds of 1Gbps and 100Mbps, but not for 10Mbps,
> because SPEED_10 is actually 0, so AND-ing anything with 0 is false,
> therefore that last branch is dead code.
> 
> Do what others do (genphy_read_status_fixed, phy_mii_ioctl) and just
> remove the check for SPEED_10, let it fall into the default case.
> 
> Fixes: ffe10e679cec ("net: dsa: sja1105: Add support for the SGMII port")
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH net 1/2] net: dsa: sja1105: fix SGMII PCS being forced to SPEED_UNKNOWN instead of SPEED_10

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (refs/heads/master):

On Thu,  4 Mar 2021 12:56:53 +0200 you wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> When using MLO_AN_PHY or MLO_AN_FIXED, the MII_BMCR of the SGMII PCS is
> read before resetting the switch so it can be reprogrammed afterwards.
> This works for the speeds of 1Gbps and 100Mbps, but not for 10Mbps,
> because SPEED_10 is actually 0, so AND-ing anything with 0 is false,
> therefore that last branch is dead code.
> 
> [...]

Here is the summary with links:
  - [net,1/2] net: dsa: sja1105: fix SGMII PCS being forced to SPEED_UNKNOWN instead of SPEED_10
    https://git.kernel.org/netdev/net/c/053d8ad10d58
  - [net,2/2] net: dsa: sja1105: fix ucast/bcast flooding always remaining enabled
    https://git.kernel.org/netdev/net/c/6a5166e07c02

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