1. Patchew
  2. linux
  3. View series

[PATCH] net: ethernet: ti: am65-cpsw-nuss: remove dead vid check in slave_add_vid()

Alexander Vassilevski posted 1 patch 1 week, 2 days ago
There is a newer version of this series
drivers/net/ethernet/ti/am65-cpsw-nuss.c | 2 --
1 file changed, 2 deletions(-)
[PATCH] net: ethernet: ti: am65-cpsw-nuss: remove dead vid check in slave_add_vid()
Posted by Alexander Vassilevski 1 week, 2 days ago 
am65_cpsw_nuss_ndo_slave_add_vid() returns early at the top with:

    if (!netif_running(ndev) || !vid)
        return 0;

so vid is guaranteed to be non-zero in the rest of the function. The
subsequent

    if (!vid)
        unreg_mcast = port_mask;

is therefore unreachable. unreg_mcast stays at its initialized value of
zero and is passed as the unreg_mcast argument to
cpsw_ale_vlan_add_modify().

Drop the dead branch. No functional change.

Found by Smatch.

Fixes: 7bcffde02152 ("net: ethernet: ti: am65-cpsw-nuss: restore vlan configuration while down/up")
Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Closes: https://lore.kernel.org/kernel-janitors/aS_lhMwppbDHoEcX@stanley.mountain/
Signed-off-by: Alexander Vassilevski <oss@vassilevski.com>
---
 drivers/net/ethernet/ti/am65-cpsw-nuss.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
index 7ac75fc8cd..77f59996a0 100644
--- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
+++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
@@ -316,8 +316,6 @@ static int am65_cpsw_nuss_ndo_slave_add_vid(struct net_device *ndev,
 		return ret;
 
 	port_mask = BIT(port->port_id) | ALE_PORT_HOST;
-	if (!vid)
-		unreg_mcast = port_mask;
 	dev_info(common->dev, "Adding vlan %d to vlan filter\n", vid);
 	ret = cpsw_ale_vlan_add_modify(common->ale, vid, port_mask,
 				       unreg_mcast, port_mask, 0);
-- 
2.43.0
Re: [PATCH] net: ethernet: ti: am65-cpsw-nuss: remove dead vid check in slave_add_vid()
Posted by Christophe JAILLET 1 week, 1 day ago 
Le 16/05/2026 à 00:57, Alexander Vassilevski a écrit :
> am65_cpsw_nuss_ndo_slave_add_vid() returns early at the top with:
> 
>      if (!netif_running(ndev) || !vid)
>          return 0;
> 
> so vid is guaranteed to be non-zero in the rest of the function. The
> subsequent
> 
>      if (!vid)
>          unreg_mcast = port_mask;
> 
> is therefore unreachable. unreg_mcast stays at its initialized value of
> zero and is passed as the unreg_mcast argument to
> cpsw_ale_vlan_add_modify().
> 
> Drop the dead branch. No functional change.
> 
> Found by Smatch.
> 
> Fixes: 7bcffde02152 ("net: ethernet: ti: am65-cpsw-nuss: restore vlan configuration while down/up")
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Closes: https://lore.kernel.org/kernel-janitors/aS_lhMwppbDHoEcX@stanley.mountain/
> Signed-off-by: Alexander Vassilevski <oss@vassilevski.com>
> ---
>   drivers/net/ethernet/ti/am65-cpsw-nuss.c | 2 --
>   1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> index 7ac75fc8cd..77f59996a0 100644
> --- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> +++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> @@ -316,8 +316,6 @@ static int am65_cpsw_nuss_ndo_slave_add_vid(struct net_device *ndev,
>   		return ret;
>   
>   	port_mask = BIT(port->port_id) | ALE_PORT_HOST;
> -	if (!vid)
> -		unreg_mcast = port_mask;

Maybe unreg_mcast could also be removed then.
It is known to be 0 and is unmodified.

CJ

>   	dev_info(common->dev, "Adding vlan %d to vlan filter\n", vid);
>   	ret = cpsw_ale_vlan_add_modify(common->ale, vid, port_mask,
>   				       unreg_mcast, port_mask, 0);
Re: [PATCH] net: ethernet: ti: am65-cpsw-nuss: remove dead vid check in slave_add_vid()
Posted by Siddharth Vadapalli 1 week, 2 days ago 
On 16/05/26 4:27 AM, Alexander Vassilevski wrote:
> am65_cpsw_nuss_ndo_slave_add_vid() returns early at the top with:
> 
>      if (!netif_running(ndev) || !vid)
>          return 0;
> 
> so vid is guaranteed to be non-zero in the rest of the function. The
> subsequent
> 
>      if (!vid)
>          unreg_mcast = port_mask;
> 
> is therefore unreachable. unreg_mcast stays at its initialized value of
> zero and is passed as the unreg_mcast argument to
> cpsw_ale_vlan_add_modify().
> 
> Drop the dead branch. No functional change.
> 
> Found by Smatch.
> 
> Fixes: 7bcffde02152 ("net: ethernet: ti: am65-cpsw-nuss: restore vlan configuration while down/up")
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Closes: https://lore.kernel.org/kernel-janitors/aS_lhMwppbDHoEcX@stanley.mountain/
> Signed-off-by: Alexander Vassilevski <oss@vassilevski.com>


Reviewed-by: Siddharth Vadapalli <s-vadapalli@ti.com>


> ---
>   drivers/net/ethernet/ti/am65-cpsw-nuss.c | 2 --
>   1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> index 7ac75fc8cd..77f59996a0 100644
> --- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> +++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> @@ -316,8 +316,6 @@ static int am65_cpsw_nuss_ndo_slave_add_vid(struct net_device *ndev,
>   		return ret;
>   
>   	port_mask = BIT(port->port_id) | ALE_PORT_HOST;
> -	if (!vid)
> -		unreg_mcast = port_mask;
>   	dev_info(common->dev, "Adding vlan %d to vlan filter\n", vid);
>   	ret = cpsw_ale_vlan_add_modify(common->ale, vid, port_mask,
>   				       unreg_mcast, port_mask, 0);
[PATCH v3] net: ethernet: ti: am65-cpsw-nuss: remove dead vid check in slave_add_vid()
Posted by Alexander Vassilevski 1 week ago 
am65_cpsw_nuss_ndo_slave_add_vid() returns early at the top with:

     if (!netif_running(ndev) || !vid)
         return 0;

so vid is guaranteed to be non-zero in the rest of the function. The
subsequent

     if (!vid)
         unreg_mcast = port_mask;

is therefore unreachable. Drop the dead branch.

With that branch gone, unreg_mcast is only ever its initializer value
of zero, so drop the variable and pass 0 directly to
cpsw_ale_vlan_add_modify().

No functional change.

Found by Smatch.

Fixes: 7bcffde02152 ("net: ethernet: ti: am65-cpsw-nuss: restore vlan configuration while down/up")
Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Closes: https://lore.kernel.org/kernel-janitors/aS_lhMwppbDHoEcX@stanley.mountain/
Signed-off-by: Alexander Vassilevski <oss@vassilevski.com>
Reviewed-by: Siddharth Vadapalli <s-vadapalli@ti.com>
---
Changes in v3:
  - Combine v1 and v2 into a single cumulative patch
  Link to v2: https://lore.kernel.org/kernel-janitors/20260517151611.393789-1-oss@vassilevski.com/

Changes in v2:
  - Also drop the now-unused unreg_mcast variable; pass 0 directly to
    cpsw_ale_vlan_add_modify()
  Link to v1: https://lore.kernel.org/kernel-janitors/20260515225715.3641804-1-oss@vassilevski.com/

 drivers/net/ethernet/ti/am65-cpsw-nuss.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
index 7ac75fc8cd..434a310808 100644
--- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
+++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
@@ -302,7 +302,7 @@ static int am65_cpsw_nuss_ndo_slave_add_vid(struct net_device *ndev,
 {
 	struct am65_cpsw_common *common = am65_ndev_to_common(ndev);
 	struct am65_cpsw_port *port = am65_ndev_to_port(ndev);
-	u32 port_mask, unreg_mcast = 0;
+	u32 port_mask;
 	int ret;
 
 	if (!common->is_emac_mode)
@@ -316,11 +316,9 @@ static int am65_cpsw_nuss_ndo_slave_add_vid(struct net_device *ndev,
 		return ret;
 
 	port_mask = BIT(port->port_id) | ALE_PORT_HOST;
-	if (!vid)
-		unreg_mcast = port_mask;
 	dev_info(common->dev, "Adding vlan %d to vlan filter\n", vid);
 	ret = cpsw_ale_vlan_add_modify(common->ale, vid, port_mask,
-				       unreg_mcast, port_mask, 0);
+				       0, port_mask, 0);
 
 	pm_runtime_put(common->dev);
 	return ret;

base-commit: e98d21c170b01ddef366f023bbfcf6b31509fa83
-- 
2.43.0
[PATCH v2] net: ethernet: ti: am65-cpsw-nuss: remove dead vid check in slave_add_vid()
Posted by Alexander Vassilevski 1 week ago 
am65_cpsw_nuss_ndo_slave_add_vid() returns early at the top with:

    if (!netif_running(ndev) || !vid)
        return 0;

so vid is guaranteed to be non-zero in the rest of the function. The
subsequent

    if (!vid)
        unreg_mcast = port_mask;

is therefore unreachable. Drop the dead branch.

With that branch gone, unreg_mcast is only ever its initializer value
of zero, so drop the variable and pass 0 directly to
cpsw_ale_vlan_add_modify().

No functional change.

Found by Smatch.

Fixes: 7bcffde02152 ("net: ethernet: ti: am65-cpsw-nuss: restore vlan configuration while down/up")
Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Closes: https://lore.kernel.org/kernel-janitors/aS_lhMwppbDHoEcX@stanley.mountain/
Suggested-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Reviewed-by: Siddharth Vadapalli <s-vadapalli@ti.com>
Signed-off-by: Alexander Vassilevski <oss@vassilevski.com>
---
I agree, Christophe.

 drivers/net/ethernet/ti/am65-cpsw-nuss.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
index 77f59996a0..434a310808 100644
--- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
+++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
@@ -302,7 +302,7 @@ static int am65_cpsw_nuss_ndo_slave_add_vid(struct net_device *ndev,
 {
 	struct am65_cpsw_common *common = am65_ndev_to_common(ndev);
 	struct am65_cpsw_port *port = am65_ndev_to_port(ndev);
-	u32 port_mask, unreg_mcast = 0;
+	u32 port_mask;
 	int ret;
 
 	if (!common->is_emac_mode)
@@ -318,7 +318,7 @@ static int am65_cpsw_nuss_ndo_slave_add_vid(struct net_device *ndev,
 	port_mask = BIT(port->port_id) | ALE_PORT_HOST;
 	dev_info(common->dev, "Adding vlan %d to vlan filter\n", vid);
 	ret = cpsw_ale_vlan_add_modify(common->ale, vid, port_mask,
-				       unreg_mcast, port_mask, 0);
+				       0, port_mask, 0);
 
 	pm_runtime_put(common->dev);
 	return ret;
-- 
2.43.0
Re: [PATCH v2] net: ethernet: ti: am65-cpsw-nuss: remove dead vid check in slave_add_vid()
Posted by Christophe JAILLET 1 week ago 
Le 17/05/2026 à 17:16, Alexander Vassilevski a écrit :
> am65_cpsw_nuss_ndo_slave_add_vid() returns early at the top with:
> 
>      if (!netif_running(ndev) || !vid)
>          return 0;
> 
> so vid is guaranteed to be non-zero in the rest of the function. The
> subsequent
> 
>      if (!vid)
>          unreg_mcast = port_mask;
> 
> is therefore unreachable. Drop the dead branch.
> 
> With that branch gone, unreg_mcast is only ever its initializer value
> of zero, so drop the variable and pass 0 directly to
> cpsw_ale_vlan_add_modify().
> 
> No functional change.
> 
> Found by Smatch.
> 
> Fixes: 7bcffde02152 ("net: ethernet: ti: am65-cpsw-nuss: restore vlan configuration while down/up")
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Closes: https://lore.kernel.org/kernel-janitors/aS_lhMwppbDHoEcX@stanley.mountain/
> Suggested-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>

Appreciated, but not really needed. I just made a comment about a tiny 
possible improvement.

> Reviewed-by: Siddharth Vadapalli <s-vadapalli@ti.com>

This should be placed after Signed-off-by:

> Signed-off-by: Alexander Vassilevski <oss@vassilevski.com>
> ---
> I agree, Christophe.

This part could be more descriptive.
You could for example have:

Changes in v2:
    - remove unreg_mcast which is now unneeded.

> 
>   drivers/net/ethernet/ti/am65-cpsw-nuss.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> index 77f59996a0..434a310808 100644
> --- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> +++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> @@ -302,7 +302,7 @@ static int am65_cpsw_nuss_ndo_slave_add_vid(struct net_device *ndev,
>   {
>   	struct am65_cpsw_common *common = am65_ndev_to_common(ndev);
>   	struct am65_cpsw_port *port = am65_ndev_to_port(ndev);
> -	u32 port_mask, unreg_mcast = 0;
> +	u32 port_mask;
>   	int ret;
>   
>   	if (!common->is_emac_mode)
> @@ -318,7 +318,7 @@ static int am65_cpsw_nuss_ndo_slave_add_vid(struct net_device *ndev,
>   	port_mask = BIT(port->port_id) | ALE_PORT_HOST;
>   	dev_info(common->dev, "Adding vlan %d to vlan filter\n", vid);
>   	ret = cpsw_ale_vlan_add_modify(common->ale, vid, port_mask,
> -				       unreg_mcast, port_mask, 0);
> +				       0, port_mask, 0);
>   
>   	pm_runtime_put(common->dev);
>   	return ret;

Patches don't work like that.

When you send a v2, it supersedes the previous v1.
So your dead-code removal, should also be included in the v2.
Otherwise, it is impossible for maintainers to know what to apply.

So, you should send a new cumulative version, as a v3.

CJ

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.