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

From: Arun.Ramadoss
Date: Fri Jan 13 2023 - 09:55:07 EST


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@xxxxxxxxxxx>
> ---
> 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;
> +}
> +
>