Hi Dan,
+ Siddharth
On 4/20/2023 2:03 PM, Dan Carpenter wrote:
> Hello Vignesh Raghavendra,
>
> The patch 86e8b070b25e: "net: ti: am65-cpsw-nuss: Add switchdev
> support" from Feb 11, 2021, leads to the following Smatch static
> checker warning:
>
> drivers/net/ethernet/ti/am65-cpsw-switchdev.c:187 am65_cpsw_port_vlan_add()
> warn: missing error code? 'ret'
>
> drivers/net/ethernet/ti/am65-cpsw-switchdev.c
> 149 static int am65_cpsw_port_vlan_add(struct am65_cpsw_port *port, bool untag, bool pvid,
> 150 u16 vid, struct net_device *orig_dev)
> 151 {
> 152 bool cpu_port = netif_is_bridge_master(orig_dev);
> 153 struct am65_cpsw_common *cpsw = port->common;
> 154 int unreg_mcast_mask = 0;
> 155 int reg_mcast_mask = 0;
> 156 int untag_mask = 0;
> 157 int port_mask;
> 158 int ret = 0;
> 159 u32 flags;
> 160
> 161 if (cpu_port) {
> 162 port_mask = BIT(HOST_PORT_NUM);
> 163 flags = orig_dev->flags;
> 164 unreg_mcast_mask = port_mask;
> 165 } else {
> 166 port_mask = BIT(port->port_id);
> 167 flags = port->ndev->flags;
> 168 }
> 169
> 170 if (flags & IFF_MULTICAST)
> 171 reg_mcast_mask = port_mask;
> 172
> 173 if (untag)
> 174 untag_mask = port_mask;
> 175
> 176 ret = cpsw_ale_vlan_add_modify(cpsw->ale, vid, port_mask, untag_mask,
> 177 reg_mcast_mask, unreg_mcast_mask);
> 178 if (ret) {
> ^^^
> ret is zero.
>
> 179 netdev_err(port->ndev, "Unable to add vlan\n");
> 180 return ret;
> 181 }
> 182
> 183 if (cpu_port)
> 184 cpsw_ale_add_ucast(cpsw->ale, port->slave.mac_addr,
> 185 HOST_PORT_NUM, ALE_VLAN | ALE_SECURE, vid);
> 186 if (!pvid)
> --> 187 return ret;
>
> I think returning zero might have been intentional here, but it would
> be better to do an explicit "return 0;"
>
Correct
> 188
> 189 am65_cpsw_set_pvid(port, vid, 0, 0);
> 190
> 191 netdev_dbg(port->ndev, "VID add: %s: vid:%u ports:%X\n",
> 192 port->ndev->name, vid, port_mask);
> 193
> 194 return ret;
>
> Same here too, I suppose, although Smatch doesn't care about this.
Your suggestions make sense. Thanks for the report. I or Siddharth will
send a fix.
>
> 195 }
>
Regards
Vignesh
