[bug report] net: ethernet: ti: am65-cpsw-nuss: restore vlan configuration while down/up

[bug report] net: ethernet: ti: am65-cpsw-nuss: restore vlan configuration while down/up

[Dan Carpenter]

Hello Grygorii Strashko,

Commit 7bcffde02152 ("net: ethernet: ti: am65-cpsw-nuss: restore vlan
configuration while down/up") from Jun 26, 2020 (linux-next), leads
to the following Smatch static checker warning:

	drivers/net/ethernet/ti/am65-cpsw-nuss.c:319 am65_cpsw_nuss_ndo_slave_add_vid()
	warn: duplicate zero check 'vid' (previous on line 311)

drivers/net/ethernet/ti/am65-cpsw-nuss.c
    300 static int am65_cpsw_nuss_ndo_slave_add_vid(struct net_device *ndev,
    301                                             __be16 proto, u16 vid)
    302 {
    303         struct am65_cpsw_common *common = am65_ndev_to_common(ndev);
    304         struct am65_cpsw_port *port = am65_ndev_to_port(ndev);
    305         u32 port_mask, unreg_mcast = 0;
    306         int ret;
    307 
    308         if (!common->is_emac_mode)
    309                 return 0;
    310 
    311         if (!netif_running(ndev) || !vid)
    312                 return 0;

If vid is zero we return here.

    313 
    314         ret = pm_runtime_resume_and_get(common->dev);
    315         if (ret < 0)
    316                 return ret;
    317 
    318         port_mask = BIT(port->port_id) | ALE_PORT_HOST;
--> 319         if (!vid)
    320                 unreg_mcast = port_mask;

Which means that this is dead code.  unreg_mcast is always going to be
zero.

    321         dev_info(common->dev, "Adding vlan %d to vlan filter\n", vid);
    322         ret = cpsw_ale_vlan_add_modify(common->ale, vid, port_mask,
    323                                        unreg_mcast, port_mask, 0);
    324 
    325         pm_runtime_put(common->dev);
    326         return ret;
    327 }

regards,
dan carpenter

[bug report] net: ethernet: ti: am65-cpsw-nuss: restore vlan configuration while down/up

[Dan Carpenter]

Hello Grygorii Strashko,

The patch 7bcffde02152: "net: ethernet: ti: am65-cpsw-nuss: restore
vlan configuration while down/up" from Jun 26, 2020, leads to the
following Smatch static checker warning:

	drivers/net/ethernet/ti/am65-cpsw-nuss.c:188 am65_cpsw_nuss_ndo_slave_add_vid()
	warn: duplicate check 'vid' (previous on line 180)

drivers/net/ethernet/ti/am65-cpsw-nuss.c
    169 static int am65_cpsw_nuss_ndo_slave_add_vid(struct net_device *ndev,
    170                                             __be16 proto, u16 vid)
    171 {
    172         struct am65_cpsw_common *common = am65_ndev_to_common(ndev);
    173         struct am65_cpsw_port *port = am65_ndev_to_port(ndev);
    174         u32 port_mask, unreg_mcast = 0;
    175         int ret;
    176 
    177         if (!common->is_emac_mode)
    178                 return 0;
    179 
    180         if (!netif_running(ndev) || !vid)
                                            ^^^^
If "vid" is zero then we return here.

    181                 return 0;
    182 
    183         ret = pm_runtime_resume_and_get(common->dev);
    184         if (ret < 0)
    185                 return ret;
    186 
    187         port_mask = BIT(port->port_id) | ALE_PORT_HOST;
--> 188         if (!vid)
    189                 unreg_mcast = port_mask;
    190         dev_info(common->dev, "Adding vlan %d to vlan filter\n", vid);
    191         ret = cpsw_ale_vlan_add_modify(common->ale, vid, port_mask,
    192                                        unreg_mcast, port_mask, 0);

So we chould just remove the unreg_mcast variable and call?:

	ret = cpsw_ale_vlan_add_modify(common->ale, vid, port_mask,
				       port_mask, port_mask, 0);

    193 
    194         pm_runtime_put(common->dev);
    195         return ret;
    196 }

regards,
dan carpenter