diff mbox series

[net-next] net: dsa: rzn1-a5psw: Add vlan support

Message ID 20230111115607.1146502-1-clement.leger@bootlin.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series [net-next] net: dsa: rzn1-a5psw: Add vlan support | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Single patches do not need cover letters
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers success CCed 11 of 11 maintainers
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch warning CHECK: struct mutex definition without comment
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Clément Léger Jan. 11, 2023, 11:56 a.m. UTC
Add support for vlan operation (add, del, filtering) on the RZN1
driver. The a5psw switch supports up to 32 VLAN IDs with filtering,
tagged/untagged VLANs and PVID for each ports.

Signed-off-by: Clément Léger <clement.leger@bootlin.com>
---
 drivers/net/dsa/rzn1_a5psw.c | 182 +++++++++++++++++++++++++++++++++++
 drivers/net/dsa/rzn1_a5psw.h |  10 +-
 2 files changed, 189 insertions(+), 3 deletions(-)

Comments

Jakub Kicinski Jan. 13, 2023, 5:37 a.m. UTC | #1
On Wed, 11 Jan 2023 12:56:07 +0100 Clément Léger wrote:
> Add support for vlan operation (add, del, filtering) on the RZN1
> driver. The a5psw switch supports up to 32 VLAN IDs with filtering,
> tagged/untagged VLANs and PVID for each ports.

noob question - do you need that mutex? 
aren't those ops all under rtnl_lock?
Andrew Lunn Jan. 13, 2023, 2:12 p.m. UTC | #2
On Thu, Jan 12, 2023 at 09:37:55PM -0800, Jakub Kicinski wrote:
> On Wed, 11 Jan 2023 12:56:07 +0100 Clément Léger wrote:
> > Add support for vlan operation (add, del, filtering) on the RZN1
> > driver. The a5psw switch supports up to 32 VLAN IDs with filtering,
> > tagged/untagged VLANs and PVID for each ports.
> 
> noob question - do you need that mutex? 
> aren't those ops all under rtnl_lock?

Hi Jakub

Not commenting about this specific patch, but not everything in DSA is
done under RTNL. So you need to deal with some parallel API calls. But
they tend to be in different areas. I would not expect to see two VLAN
changes as the same time, but maybe VLAN and polling in a workqueue to
update the statistics for example could happen. Depending on the
switch, some protect might be needed to stop these operations
interfering with each other. And DSA drivers in general tend to KISS
and over lock. Nothing here is particularly hot path, the switch
itself is on the end of a slow bus, so the overhead of locks are
minimum.

	Andrew
Arun Ramadoss Jan. 13, 2023, 2:40 p.m. UTC | #3
Hi Clement,
On Wed, 2023-01-11 at 12:56 +0100, Clément Léger wrote:
> Add support for vlan operation (add, del, filtering) on the RZN1
> driver. The a5psw switch supports up to 32 VLAN IDs with filtering,
> tagged/untagged VLANs and PVID for each ports.
> 
> Signed-off-by: Clément Léger <clement.leger@bootlin.com>
> ---
>  drivers/net/dsa/rzn1_a5psw.c | 182
> +++++++++++++++++++++++++++++++++++
>  drivers/net/dsa/rzn1_a5psw.h |  10 +-
>  2 files changed, 189 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/dsa/rzn1_a5psw.c
> b/drivers/net/dsa/rzn1_a5psw.c
> index ed413d555bec..8ecb9214b5e6 100644
> --- a/drivers/net/dsa/rzn1_a5psw.c
> +++ b/drivers/net/dsa/rzn1_a5psw.c
> @@ -540,6 +540,161 @@ static int a5psw_port_fdb_dump(struct
> dsa_switch *ds, int port,
>  	return ret;
>  }
>  
> +static int a5psw_port_vlan_filtering(struct dsa_switch *ds, int
> port,
> +				     bool vlan_filtering,
> +				     struct netlink_ext_ack *extack)
> +{
> +	u32 mask = BIT(port + A5PSW_VLAN_VERI_SHIFT)
> +		   | BIT(port + A5PSW_VLAN_DISC_SHIFT);

Operator | at the end of line

> +	struct a5psw *a5psw = ds->priv;
> +	u32 val = 0;
> +
> +	if (vlan_filtering)
> +		val = BIT(port + A5PSW_VLAN_VERI_SHIFT)
> +		      | BIT(port + A5PSW_VLAN_DISC_SHIFT);

Operator | at the end of line

> +
> +	a5psw_reg_rmw(a5psw, A5PSW_VLAN_VERIFY, mask, val);
> +
> +	return 0;
> +}
> +
> +static int a5psw_port_vlan_add(struct dsa_switch *ds, int port,
> +			       const struct switchdev_obj_port_vlan
> *vlan,
> +			       struct netlink_ext_ack *extack)
> +{
> +	bool tagged = !(vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED);
> +	bool pvid = vlan->flags & BRIDGE_VLAN_INFO_PVID;
> +	struct a5psw *a5psw = ds->priv;
> +	u16 vid = vlan->vid;
> +	int ret = -EINVAL;
> +	int vlan_res_id;
> +
> +	dev_dbg(a5psw->dev, "Add VLAN %d on port %d, %s, %s\n",
> +		vid, port, tagged ? "tagged" : "untagged",
> +		pvid ? "PVID" : "no PVID");
> +
> +	mutex_lock(&a5psw->vlan_lock);
> +
> +	vlan_res_id = a5psw_find_vlan_entry(a5psw, vid);
> +	if (vlan_res_id < 0) {
> +		vlan_res_id = a5psw_get_vlan_res_entry(a5psw, vid);
> +		if (vlan_res_id < 0)

nit: We can initialize ret = 0 initially, and assign ret = -EINVAL here
& remove ret = 0 at end of function.

> +			goto out;
> +	}
> +
> +	a5psw_port_vlan_cfg(a5psw, vlan_res_id, port, true);
> +	if (tagged)
> +		a5psw_port_vlan_tagged_cfg(a5psw, vlan_res_id, port,
> true);
> +
> +	if (pvid) {
> +		a5psw_reg_rmw(a5psw, A5PSW_VLAN_IN_MODE_ENA, BIT(port),
> +			      BIT(port));
> +		a5psw_reg_writel(a5psw, A5PSW_SYSTEM_TAGINFO(port),
> vid);
> +	}
> +
> +	ret = 0;
> +out:
> +	mutex_unlock(&a5psw->vlan_lock);
> +
> +	return ret;
> +}
> +
> +static int a5psw_port_vlan_del(struct dsa_switch *ds, int port,
> +			       const struct switchdev_obj_port_vlan
> *vlan)
> +{
> +	struct a5psw *a5psw = ds->priv;
> +	u16 vid = vlan->vid;
> +	int ret = -EINVAL;

Simillarly here.

> +	int vlan_res_id;
> +
> +	dev_dbg(a5psw->dev, "Removing VLAN %d on port %d\n", vid,
> port);
> +
> +	mutex_lock(&a5psw->vlan_lock);
> +
> +	vlan_res_id = a5psw_find_vlan_entry(a5psw, vid);
> +	if (vlan_res_id < 0)
> +		goto out;
> +
> +	a5psw_port_vlan_cfg(a5psw, vlan_res_id, port, false);
> +	a5psw_port_vlan_tagged_cfg(a5psw, vlan_res_id, port, false);
> +
> +	/* Disable PVID if the vid is matching the port one */
> +	if (vid == a5psw_reg_readl(a5psw, A5PSW_SYSTEM_TAGINFO(port)))
> +		a5psw_reg_rmw(a5psw, A5PSW_VLAN_IN_MODE_ENA, BIT(port),
> 0);
> +
> +	ret = 0;
> +out:
> +	mutex_unlock(&a5psw->vlan_lock);
> +
> +	return ret;
> +}
> +
>
Vladimir Oltean Jan. 13, 2023, 3:12 p.m. UTC | #4
On Wed, Jan 11, 2023 at 12:56:07PM +0100, Clément Léger wrote:
> Add support for vlan operation (add, del, filtering) on the RZN1
> driver. The a5psw switch supports up to 32 VLAN IDs with filtering,
> tagged/untagged VLANs and PVID for each ports.
> 
> Signed-off-by: Clément Léger <clement.leger@bootlin.com>
> ---

Have you run the bridge_vlan_aware.sh and bridge_vlan_unaware.sh from
tools/testing/selftests/drivers/net/dsa/?
Vladimir Oltean Jan. 13, 2023, 3:37 p.m. UTC | #5
On Fri, Jan 13, 2023 at 03:12:40PM +0100, Andrew Lunn wrote:
> On Thu, Jan 12, 2023 at 09:37:55PM -0800, Jakub Kicinski wrote:
> > On Wed, 11 Jan 2023 12:56:07 +0100 Clément Léger wrote:
> > > Add support for vlan operation (add, del, filtering) on the RZN1
> > > driver. The a5psw switch supports up to 32 VLAN IDs with filtering,
> > > tagged/untagged VLANs and PVID for each ports.
> > 
> > noob question - do you need that mutex? 
> > aren't those ops all under rtnl_lock?
> 
> Hi Jakub
> 
> Not commenting about this specific patch, but not everything in DSA is
> done under RTNL. So you need to deal with some parallel API calls. But
> they tend to be in different areas. I would not expect to see two VLAN
> changes as the same time, but maybe VLAN and polling in a workqueue to
> update the statistics for example could happen. Depending on the
> switch, some protect might be needed to stop these operations
> interfering with each other. And DSA drivers in general tend to KISS
> and over lock. Nothing here is particularly hot path, the switch
> itself is on the end of a slow bus, so the overhead of locks are
> minimum.

That being said, port_vlan_add(), port_vlan_del() and port_vlan_filtering()
are all serialized by the rtnl_lock().
Clément Léger Jan. 16, 2023, 8:08 a.m. UTC | #6
Le Fri, 13 Jan 2023 17:37:30 +0200,
Vladimir Oltean <olteanv@gmail.com> a écrit :

> On Fri, Jan 13, 2023 at 03:12:40PM +0100, Andrew Lunn wrote:
> > On Thu, Jan 12, 2023 at 09:37:55PM -0800, Jakub Kicinski wrote:  
> > > On Wed, 11 Jan 2023 12:56:07 +0100 Clément Léger wrote:  
> > > > Add support for vlan operation (add, del, filtering) on the RZN1
> > > > driver. The a5psw switch supports up to 32 VLAN IDs with filtering,
> > > > tagged/untagged VLANs and PVID for each ports.  
> > > 
> > > noob question - do you need that mutex? 
> > > aren't those ops all under rtnl_lock?  
> > 
> > Hi Jakub
> > 
> > Not commenting about this specific patch, but not everything in DSA is
> > done under RTNL. So you need to deal with some parallel API calls. But
> > they tend to be in different areas. I would not expect to see two VLAN
> > changes as the same time, but maybe VLAN and polling in a workqueue to
> > update the statistics for example could happen. Depending on the
> > switch, some protect might be needed to stop these operations
> > interfering with each other. And DSA drivers in general tend to KISS
> > and over lock. Nothing here is particularly hot path, the switch
> > itself is on the end of a slow bus, so the overhead of locks are
> > minimum.  
> 
> That being said, port_vlan_add(), port_vlan_del() and port_vlan_filtering()
> are all serialized by the rtnl_lock().

Ok then, I'll remove this lock and rely on the RTNL lock.

Thanks,
Clément Léger Jan. 16, 2023, 8:48 a.m. UTC | #7
Le Fri, 13 Jan 2023 14:40:26 +0000,
<Arun.Ramadoss@microchip.com> a écrit :

> Hi Clement,
> On Wed, 2023-01-11 at 12:56 +0100, Clément Léger wrote:
> > Add support for vlan operation (add, del, filtering) on the RZN1
> > driver. The a5psw switch supports up to 32 VLAN IDs with filtering,
> > tagged/untagged VLANs and PVID for each ports.
> > 
> > Signed-off-by: Clément Léger <clement.leger@bootlin.com>
> > ---
> >  drivers/net/dsa/rzn1_a5psw.c | 182
> > +++++++++++++++++++++++++++++++++++
> >  drivers/net/dsa/rzn1_a5psw.h |  10 +-
> >  2 files changed, 189 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/net/dsa/rzn1_a5psw.c
> > b/drivers/net/dsa/rzn1_a5psw.c
> > index ed413d555bec..8ecb9214b5e6 100644
> > --- a/drivers/net/dsa/rzn1_a5psw.c
> > +++ b/drivers/net/dsa/rzn1_a5psw.c
> > @@ -540,6 +540,161 @@ static int a5psw_port_fdb_dump(struct
> > dsa_switch *ds, int port,
> >  	return ret;
> >  }
> >  
> > +static int a5psw_port_vlan_filtering(struct dsa_switch *ds, int
> > port,
> > +				     bool vlan_filtering,
> > +				     struct netlink_ext_ack *extack)
> > +{
> > +	u32 mask = BIT(port + A5PSW_VLAN_VERI_SHIFT)
> > +		   | BIT(port + A5PSW_VLAN_DISC_SHIFT);  
> 
> Operator | at the end of line
> 
> > +	struct a5psw *a5psw = ds->priv;
> > +	u32 val = 0;
> > +
> > +	if (vlan_filtering)
> > +		val = BIT(port + A5PSW_VLAN_VERI_SHIFT)
> > +		      | BIT(port + A5PSW_VLAN_DISC_SHIFT);  
> 
> Operator | at the end of line

Hi Arun,

I'll fix that.

> 
> > +
> > +	a5psw_reg_rmw(a5psw, A5PSW_VLAN_VERIFY, mask, val);
> > +
> > +	return 0;
> > +}
> > +
> > +static int a5psw_port_vlan_add(struct dsa_switch *ds, int port,
> > +			       const struct switchdev_obj_port_vlan
> > *vlan,
> > +			       struct netlink_ext_ack *extack)
> > +{
> > +	bool tagged = !(vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED);
> > +	bool pvid = vlan->flags & BRIDGE_VLAN_INFO_PVID;
> > +	struct a5psw *a5psw = ds->priv;
> > +	u16 vid = vlan->vid;
> > +	int ret = -EINVAL;
> > +	int vlan_res_id;
> > +
> > +	dev_dbg(a5psw->dev, "Add VLAN %d on port %d, %s, %s\n",
> > +		vid, port, tagged ? "tagged" : "untagged",
> > +		pvid ? "PVID" : "no PVID");
> > +
> > +	mutex_lock(&a5psw->vlan_lock);
> > +
> > +	vlan_res_id = a5psw_find_vlan_entry(a5psw, vid);
> > +	if (vlan_res_id < 0) {
> > +		vlan_res_id = a5psw_get_vlan_res_entry(a5psw, vid);
> > +		if (vlan_res_id < 0)  
> 
> nit: We can initialize ret = 0 initially, and assign ret = -EINVAL here
> & remove ret = 0 at end of function.
> 
> > +			goto out;
> > +	}
> > +
> > +	a5psw_port_vlan_cfg(a5psw, vlan_res_id, port, true);
> > +	if (tagged)
> > +		a5psw_port_vlan_tagged_cfg(a5psw, vlan_res_id, port,
> > true);
> > +
> > +	if (pvid) {
> > +		a5psw_reg_rmw(a5psw, A5PSW_VLAN_IN_MODE_ENA, BIT(port),
> > +			      BIT(port));
> > +		a5psw_reg_writel(a5psw, A5PSW_SYSTEM_TAGINFO(port),
> > vid);
> > +	}
> > +
> > +	ret = 0;
> > +out:
> > +	mutex_unlock(&a5psw->vlan_lock);
> > +
> > +	return ret;
> > +}
> > +
> > +static int a5psw_port_vlan_del(struct dsa_switch *ds, int port,
> > +			       const struct switchdev_obj_port_vlan
> > *vlan)
> > +{
> > +	struct a5psw *a5psw = ds->priv;
> > +	u16 vid = vlan->vid;
> > +	int ret = -EINVAL;  
> 
> Simillarly here.

Since I removed the mutex thanks to the previous comments, I have
removed all the "ret" usage.

Thanks,
Clément Léger Jan. 16, 2023, 9:19 a.m. UTC | #8
Le Fri, 13 Jan 2023 17:12:48 +0200,
Vladimir Oltean <olteanv@gmail.com> a écrit :

> On Wed, Jan 11, 2023 at 12:56:07PM +0100, Clément Léger wrote:
> > Add support for vlan operation (add, del, filtering) on the RZN1
> > driver. The a5psw switch supports up to 32 VLAN IDs with filtering,
> > tagged/untagged VLANs and PVID for each ports.
> > 
> > Signed-off-by: Clément Léger <clement.leger@bootlin.com>
> > ---  
> 
> Have you run the bridge_vlan_aware.sh and bridge_vlan_unaware.sh from
> tools/testing/selftests/drivers/net/dsa/?

Nope, I will do that.
Clément Léger Feb. 8, 2023, 2:33 p.m. UTC | #9
Le Mon, 16 Jan 2023 10:19:14 +0100,
Clément Léger <clement.leger@bootlin.com> a écrit :

> Le Fri, 13 Jan 2023 17:12:48 +0200,
> Vladimir Oltean <olteanv@gmail.com> a écrit :
> 
> > On Wed, Jan 11, 2023 at 12:56:07PM +0100, Clément Léger wrote:  
> > > Add support for vlan operation (add, del, filtering) on the RZN1
> > > driver. The a5psw switch supports up to 32 VLAN IDs with filtering,
> > > tagged/untagged VLANs and PVID for each ports.
> > > 
> > > Signed-off-by: Clément Léger <clement.leger@bootlin.com>
> > > ---    
> > 
> > Have you run the bridge_vlan_aware.sh and bridge_vlan_unaware.sh from
> > tools/testing/selftests/drivers/net/dsa/?  
> 
> Nope, I will do that.
> 

Finally found the time to run them and both bridge_vlan_aware.sh and
bridge_vlan_unaware.sh all tests yields a [ OK ] status.

I'll resubmit a V2 with Arun Ramadoss comments fixed.
diff mbox series

Patch

diff --git a/drivers/net/dsa/rzn1_a5psw.c b/drivers/net/dsa/rzn1_a5psw.c
index ed413d555bec..8ecb9214b5e6 100644
--- a/drivers/net/dsa/rzn1_a5psw.c
+++ b/drivers/net/dsa/rzn1_a5psw.c
@@ -540,6 +540,161 @@  static int a5psw_port_fdb_dump(struct dsa_switch *ds, int port,
 	return ret;
 }
 
+static int a5psw_port_vlan_filtering(struct dsa_switch *ds, int port,
+				     bool vlan_filtering,
+				     struct netlink_ext_ack *extack)
+{
+	u32 mask = BIT(port + A5PSW_VLAN_VERI_SHIFT)
+		   | BIT(port + A5PSW_VLAN_DISC_SHIFT);
+	struct a5psw *a5psw = ds->priv;
+	u32 val = 0;
+
+	if (vlan_filtering)
+		val = BIT(port + A5PSW_VLAN_VERI_SHIFT)
+		      | BIT(port + A5PSW_VLAN_DISC_SHIFT);
+
+	a5psw_reg_rmw(a5psw, A5PSW_VLAN_VERIFY, mask, val);
+
+	return 0;
+}
+
+static int a5psw_find_vlan_entry(struct a5psw *a5psw, u16 vid)
+{
+	u32 vlan_res;
+	int i;
+
+	/* Find vlan for this port */
+	for (i = 0; i < A5PSW_VLAN_COUNT; i++) {
+		vlan_res = a5psw_reg_readl(a5psw, A5PSW_VLAN_RES(i));
+		if (FIELD_GET(A5PSW_VLAN_RES_VLANID, vlan_res) == vid)
+			return i;
+	}
+
+	return -1;
+}
+
+static int a5psw_get_vlan_res_entry(struct a5psw *a5psw, u16 newvid)
+{
+	u32 vlan_res;
+	int i;
+
+	/* Find a free VLAN entry */
+	for (i = 0; i < A5PSW_VLAN_COUNT; i++) {
+		vlan_res = a5psw_reg_readl(a5psw, A5PSW_VLAN_RES(i));
+		if (!(FIELD_GET(A5PSW_VLAN_RES_PORTMASK, vlan_res))) {
+			vlan_res = FIELD_PREP(A5PSW_VLAN_RES_VLANID, newvid);
+			a5psw_reg_writel(a5psw, A5PSW_VLAN_RES(i), vlan_res);
+			return i;
+		}
+	}
+
+	return -1;
+}
+
+static void a5psw_port_vlan_tagged_cfg(struct a5psw *a5psw, int vlan_res_id,
+				       int port, bool set)
+{
+	u32 mask = A5PSW_VLAN_RES_WR_PORTMASK | A5PSW_VLAN_RES_RD_TAGMASK |
+		   BIT(port);
+	u32 vlan_res_off = A5PSW_VLAN_RES(vlan_res_id);
+	u32 val = A5PSW_VLAN_RES_WR_TAGMASK, reg;
+
+	if (set)
+		val |= BIT(port);
+
+	/* Toggle tag mask read */
+	a5psw_reg_writel(a5psw, vlan_res_off, A5PSW_VLAN_RES_RD_TAGMASK);
+	reg = a5psw_reg_readl(a5psw, vlan_res_off);
+	a5psw_reg_writel(a5psw, vlan_res_off, A5PSW_VLAN_RES_RD_TAGMASK);
+
+	reg &= ~mask;
+	reg |= val;
+	a5psw_reg_writel(a5psw, vlan_res_off, reg);
+}
+
+static void a5psw_port_vlan_cfg(struct a5psw *a5psw, int vlan_res_id, int port,
+				bool set)
+{
+	u32 mask = A5PSW_VLAN_RES_WR_TAGMASK | BIT(port);
+	u32 reg = A5PSW_VLAN_RES_WR_PORTMASK;
+
+	if (set)
+		reg |= BIT(port);
+
+	a5psw_reg_rmw(a5psw, A5PSW_VLAN_RES(vlan_res_id), mask, reg);
+}
+
+static int a5psw_port_vlan_add(struct dsa_switch *ds, int port,
+			       const struct switchdev_obj_port_vlan *vlan,
+			       struct netlink_ext_ack *extack)
+{
+	bool tagged = !(vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED);
+	bool pvid = vlan->flags & BRIDGE_VLAN_INFO_PVID;
+	struct a5psw *a5psw = ds->priv;
+	u16 vid = vlan->vid;
+	int ret = -EINVAL;
+	int vlan_res_id;
+
+	dev_dbg(a5psw->dev, "Add VLAN %d on port %d, %s, %s\n",
+		vid, port, tagged ? "tagged" : "untagged",
+		pvid ? "PVID" : "no PVID");
+
+	mutex_lock(&a5psw->vlan_lock);
+
+	vlan_res_id = a5psw_find_vlan_entry(a5psw, vid);
+	if (vlan_res_id < 0) {
+		vlan_res_id = a5psw_get_vlan_res_entry(a5psw, vid);
+		if (vlan_res_id < 0)
+			goto out;
+	}
+
+	a5psw_port_vlan_cfg(a5psw, vlan_res_id, port, true);
+	if (tagged)
+		a5psw_port_vlan_tagged_cfg(a5psw, vlan_res_id, port, true);
+
+	if (pvid) {
+		a5psw_reg_rmw(a5psw, A5PSW_VLAN_IN_MODE_ENA, BIT(port),
+			      BIT(port));
+		a5psw_reg_writel(a5psw, A5PSW_SYSTEM_TAGINFO(port), vid);
+	}
+
+	ret = 0;
+out:
+	mutex_unlock(&a5psw->vlan_lock);
+
+	return ret;
+}
+
+static int a5psw_port_vlan_del(struct dsa_switch *ds, int port,
+			       const struct switchdev_obj_port_vlan *vlan)
+{
+	struct a5psw *a5psw = ds->priv;
+	u16 vid = vlan->vid;
+	int ret = -EINVAL;
+	int vlan_res_id;
+
+	dev_dbg(a5psw->dev, "Removing VLAN %d on port %d\n", vid, port);
+
+	mutex_lock(&a5psw->vlan_lock);
+
+	vlan_res_id = a5psw_find_vlan_entry(a5psw, vid);
+	if (vlan_res_id < 0)
+		goto out;
+
+	a5psw_port_vlan_cfg(a5psw, vlan_res_id, port, false);
+	a5psw_port_vlan_tagged_cfg(a5psw, vlan_res_id, port, false);
+
+	/* Disable PVID if the vid is matching the port one */
+	if (vid == a5psw_reg_readl(a5psw, A5PSW_SYSTEM_TAGINFO(port)))
+		a5psw_reg_rmw(a5psw, A5PSW_VLAN_IN_MODE_ENA, BIT(port), 0);
+
+	ret = 0;
+out:
+	mutex_unlock(&a5psw->vlan_lock);
+
+	return ret;
+}
+
 static u64 a5psw_read_stat(struct a5psw *a5psw, u32 offset, int port)
 {
 	u32 reg_lo, reg_hi;
@@ -657,6 +812,27 @@  static void a5psw_get_eth_ctrl_stats(struct dsa_switch *ds, int port,
 	ctrl_stats->MACControlFramesReceived = stat;
 }
 
+static void a5psw_vlan_setup(struct a5psw *a5psw, int port)
+{
+	u32 reg;
+
+	/* Enable TAG always mode for the port, this is actually controlled
+	 * by VLAN_IN_MODE_ENA field which will be used for PVID insertion
+	 */
+	reg = A5PSW_VLAN_IN_MODE_TAG_ALWAYS;
+	reg <<= A5PSW_VLAN_IN_MODE_PORT_SHIFT(port);
+	a5psw_reg_rmw(a5psw, A5PSW_VLAN_IN_MODE, A5PSW_VLAN_IN_MODE_PORT(port),
+		      reg);
+
+	/* Set transparent mode for output frame manipulation, this will depend
+	 * on the VLAN_RES configuration mode
+	 */
+	reg = A5PSW_VLAN_OUT_MODE_TRANSPARENT;
+	reg <<= A5PSW_VLAN_OUT_MODE_PORT_SHIFT(port);
+	a5psw_reg_rmw(a5psw, A5PSW_VLAN_OUT_MODE,
+		      A5PSW_VLAN_OUT_MODE_PORT(port), reg);
+}
+
 static int a5psw_setup(struct dsa_switch *ds)
 {
 	struct a5psw *a5psw = ds->priv;
@@ -729,6 +905,8 @@  static int a5psw_setup(struct dsa_switch *ds)
 		/* Enable management forward only for user ports */
 		if (dsa_port_is_user(dp))
 			a5psw_port_mgmtfwd_set(a5psw, port, true);
+
+		a5psw_vlan_setup(a5psw, port);
 	}
 
 	return 0;
@@ -756,6 +934,9 @@  static const struct dsa_switch_ops a5psw_switch_ops = {
 	.port_bridge_leave = a5psw_port_bridge_leave,
 	.port_stp_state_set = a5psw_port_stp_state_set,
 	.port_fast_age = a5psw_port_fast_age,
+	.port_vlan_filtering = a5psw_port_vlan_filtering,
+	.port_vlan_add = a5psw_port_vlan_add,
+	.port_vlan_del = a5psw_port_vlan_del,
 	.port_fdb_add = a5psw_port_fdb_add,
 	.port_fdb_del = a5psw_port_fdb_del,
 	.port_fdb_dump = a5psw_port_fdb_dump,
@@ -945,6 +1126,7 @@  static int a5psw_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	a5psw->dev = dev;
+	mutex_init(&a5psw->vlan_lock);
 	mutex_init(&a5psw->lk_lock);
 	spin_lock_init(&a5psw->reg_lock);
 	a5psw->base = devm_platform_ioremap_resource(pdev, 0);
diff --git a/drivers/net/dsa/rzn1_a5psw.h b/drivers/net/dsa/rzn1_a5psw.h
index c67abd49c013..614453d9a59e 100644
--- a/drivers/net/dsa/rzn1_a5psw.h
+++ b/drivers/net/dsa/rzn1_a5psw.h
@@ -50,7 +50,9 @@ 
 #define A5PSW_VLAN_IN_MODE_TAG_ALWAYS		0x2
 
 #define A5PSW_VLAN_OUT_MODE		0x2C
-#define A5PSW_VLAN_OUT_MODE_PORT(port)	(GENMASK(1, 0) << ((port) * 2))
+#define A5PSW_VLAN_OUT_MODE_PORT_SHIFT(port)	((port) * 2)
+#define A5PSW_VLAN_OUT_MODE_PORT(port)	(GENMASK(1, 0) << \
+					A5PSW_VLAN_OUT_MODE_PORT_SHIFT(port))
 #define A5PSW_VLAN_OUT_MODE_DIS		0x0
 #define A5PSW_VLAN_OUT_MODE_STRIP	0x1
 #define A5PSW_VLAN_OUT_MODE_TAG_THROUGH	0x2
@@ -59,7 +61,7 @@ 
 #define A5PSW_VLAN_IN_MODE_ENA		0x30
 #define A5PSW_VLAN_TAG_ID		0x34
 
-#define A5PSW_SYSTEM_TAGINFO(port)	(0x200 + A5PSW_PORT_OFFSET(port))
+#define A5PSW_SYSTEM_TAGINFO(port)	(0x200 + 4 * (port))
 
 #define A5PSW_AUTH_PORT(port)		(0x240 + 4 * (port))
 #define A5PSW_AUTH_PORT_AUTHORIZED	BIT(0)
@@ -68,7 +70,7 @@ 
 #define A5PSW_VLAN_RES_WR_PORTMASK	BIT(30)
 #define A5PSW_VLAN_RES_WR_TAGMASK	BIT(29)
 #define A5PSW_VLAN_RES_RD_TAGMASK	BIT(28)
-#define A5PSW_VLAN_RES_ID		GENMASK(16, 5)
+#define A5PSW_VLAN_RES_VLANID		GENMASK(16, 5)
 #define A5PSW_VLAN_RES_PORTMASK		GENMASK(4, 0)
 
 #define A5PSW_RXMATCH_CONFIG(port)	(0x3e80 + 4 * (port))
@@ -240,6 +242,7 @@  union lk_data {
  * @ds: DSA switch struct
  * @stats_lock: lock to access statistics (shared HI counter)
  * @lk_lock: Lock for the lookup table
+ * @vlan_lock: Lock for the vlan operation
  * @reg_lock: Lock for register read-modify-write operation
  * @bridged_ports: Mask of ports that are bridged and should be flooded
  * @br_dev: Bridge net device
@@ -253,6 +256,7 @@  struct a5psw {
 	struct phylink_pcs *pcs[A5PSW_PORTS_NUM - 1];
 	struct dsa_switch ds;
 	struct mutex lk_lock;
+	struct mutex vlan_lock;
 	spinlock_t reg_lock;
 	u32 bridged_ports;
 	struct net_device *br_dev;