On Fri, Jul 23, 2021 at 11:01:08PM +0530, Prasanna Vengateshan wrote: > +static int lan937x_port_vlan_add(struct dsa_switch *ds, int port, > + const struct switchdev_obj_port_vlan *vlan, > + struct netlink_ext_ack *extack) > +{ > + bool untagged = vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED; > + struct ksz_device *dev = ds->priv; > + struct lan937x_vlan vlan_entry; > + int ret; > + > + ret = lan937x_get_vlan_table(dev, vlan->vid, &vlan_entry); > + if (ret < 0) { > + NL_SET_ERR_MSG_MOD(extack, "Failed to get vlan table\n"); The NL_SET_ERR_MSG_MOD function already adds the \n at the end. > + return ret; > + } > + > + vlan_entry.fid = lan937x_get_fid(vlan->vid); > + vlan_entry.valid = true; > + > + /* set/clear switch port when updating vlan table registers */ > + if (untagged) > + vlan_entry.untag_prtmap |= BIT(port); > + else > + vlan_entry.untag_prtmap &= ~BIT(port); > + > + vlan_entry.fwd_map |= BIT(port); > + > + ret = lan937x_set_vlan_table(dev, vlan->vid, &vlan_entry); > + if (ret < 0) { > + NL_SET_ERR_MSG_MOD(extack, "Failed to set vlan table\n"); > + return ret; > + } > + > + /* change PVID */ > + if (vlan->flags & BRIDGE_VLAN_INFO_PVID) { > + ret = lan937x_pwrite16(dev, port, REG_PORT_DEFAULT_VID, > + vlan->vid); > + if (ret < 0) { > + NL_SET_ERR_MSG_MOD(extack, "Failed to set pvid\n"); > + return ret; > + } > + } > + > + return 0; > +} Side question: do you think the ds->configure_vlan_while_not_filtering = false from ksz9477.c and ksz8795.c serve any purpose, considering that you did not need this setting for lan937x? If not, could you please send a patch to remove that setting from those 2 other KSZ drivers? Thanks.