| Message ID | 20230208161749.331965-4-clement.leger@bootlin.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: dsa: rzn1-a5psw: add support for vlan and .port_bridge_flags | expand |
| 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 | Series has a cover letter |
| 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 10 of 10 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 | success | total: 0 errors, 0 warnings, 0 checks, 217 lines checked |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
On 2/8/23 08:17, 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> This looks good to me, just a few nits/suggestions below. > --- > drivers/net/dsa/rzn1_a5psw.c | 167 +++++++++++++++++++++++++++++++++++ > drivers/net/dsa/rzn1_a5psw.h | 8 +- > 2 files changed, 172 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/dsa/rzn1_a5psw.c b/drivers/net/dsa/rzn1_a5psw.c > index 0ce3948952db..de6b18ec647d 100644 > --- a/drivers/net/dsa/rzn1_a5psw.c > +++ b/drivers/net/dsa/rzn1_a5psw.c > @@ -583,6 +583,147 @@ 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) The name seems a bit misleading with respect to what it does, maybe a5psw_new_vlan_res_entry()? > +{ > + 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, unsigned 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, Likewise > + 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 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"); > + > + 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) > + return -EINVAL; return -ENOSPC? > + } > + > + 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); > + } > + > + return 0; > +} > + > +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 vlan_res_id; > + > + dev_dbg(a5psw->dev, "Removing VLAN %d on port %d\n", vid, port); > + > + vlan_res_id = a5psw_find_vlan_entry(a5psw, vid); > + if (vlan_res_id < 0) > + return -EINVAL; -EINVAL looks legit here > + > + 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); > + > + return 0; > +} > + > static u64 a5psw_read_stat(struct a5psw *a5psw, u32 offset, int port) > { > u32 reg_lo, reg_hi; > @@ -700,6 +841,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); If we always enable VLAN mode, which VLAN ID do switch ports not part of a VLAN aware bridge get classified into? > + > + /* 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); Sort of a follow-on to the previous question, what does transparent mean? Does that mean the frames ingressing with a certain VLAN tag will egress with the same VLAN tag in the absence of a VLAN configuration rewriting the tag? > +} > + > static int a5psw_setup(struct dsa_switch *ds) > { > struct a5psw *a5psw = ds->priv; > @@ -772,6 +934,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; > @@ -801,6 +965,9 @@ static const struct dsa_switch_ops a5psw_switch_ops = { > .port_bridge_flags = a5psw_port_bridge_flags, > .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, > diff --git a/drivers/net/dsa/rzn1_a5psw.h b/drivers/net/dsa/rzn1_a5psw.h > index c67abd49c013..2bad2e3edc2a 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))
On Wed, Feb 08, 2023 at 09:38:04AM -0800, Florian Fainelli wrote: > > + /* 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); > > If we always enable VLAN mode, which VLAN ID do switch ports not part of a > VLAN aware bridge get classified into? Good question. I'd guess 0, since otherwise, the VLAN-unaware FDB entries added with a5psw_port_fdb_add() wouldn't work. But the driver has to survive the following chain of commands, which, by looking at the current code structure, it doesn't: ip link add br0 type bridge vlan_filtering 0 ip link set swp0 master br0 # PVID should remain at a value chosen privately by the driver bridge vlan add dev swp0 vid 100 pvid untagged # PVID should not change in hardware yet ip link set br0 type bridge vlan_filtering 1 # PVID should change to 100 now ip link set br0 type bridge vlan_filtering 0 # PVID should change to the value chosen by the driver Essentially, what I'm saying is that VLANs added with "bridge vlan add" should only be active while vlan_filtering=1. If you search for "commit_pvid" in drivers/net/dsa, you'll find a number of drivers which have a more elaborate code structure which allows the commands above to work properly.
On Wed, Feb 08, 2023 at 05:17:49PM +0100, Clément Léger wrote: > +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); Is it intentional that this register is written twice? > + > + reg &= ~mask; > + reg |= val; > + a5psw_reg_writel(a5psw, vlan_res_off, reg); > +}
Hi Clement, On Wed, 2023-02-08 at 17:17 +0100, Clément Léger wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you > know the content is safe > > 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 | 167 > +++++++++++++++++++++++++++++++++++ > drivers/net/dsa/rzn1_a5psw.h | 8 +- > 2 files changed, 172 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/dsa/rzn1_a5psw.c > b/drivers/net/dsa/rzn1_a5psw.c > index 0ce3948952db..de6b18ec647d 100644 > --- a/drivers/net/dsa/rzn1_a5psw.c > +++ b/drivers/net/dsa/rzn1_a5psw.c > @@ -583,6 +583,147 @@ 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); nit: if initialization of mask is separated from declaration may increase the readability. > + 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 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); same here. > + 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+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 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"); > > + > > + 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) > > + return -EINVAL; > > + } > > + > > + 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); > > + } > > + > > + return 0; > > +} > > + >
Le Thu, 9 Feb 2023 00:02:19 +0200, Vladimir Oltean <olteanv@gmail.com> a écrit : > On Wed, Feb 08, 2023 at 09:38:04AM -0800, Florian Fainelli wrote: > > > + /* 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); > > > > If we always enable VLAN mode, which VLAN ID do switch ports not part of a > > VLAN aware bridge get classified into? > > Good question. I'd guess 0, since otherwise, the VLAN-unaware FDB > entries added with a5psw_port_fdb_add() wouldn't work. The name of the mode is probably missleading. When setting VLAN_IN_MODE with A5PSW_VLAN_IN_MODE_TAG_ALWAYS, the input packet will be tagged _only_ if VLAN_IN_MODE_ENA port bit is set. If this bit is not set, packet will passthrough transparently. This bit is actually enabled in a5psw_port_vlan_add() when a PVID is set and unset when the PVID is removed. Maybe the comment above these lines was not clear enough. > > But the driver has to survive the following chain of commands, which, by > looking at the current code structure, it doesn't: > > ip link add br0 type bridge vlan_filtering 0 > ip link set swp0 master br0 # PVID should remain at a value chosen privately by the driver > bridge vlan add dev swp0 vid 100 pvid untagged # PVID should not change in hardware yet > ip link set br0 type bridge vlan_filtering 1 # PVID should change to 100 now > ip link set br0 type bridge vlan_filtering 0 # PVID should change to the value chosen by the driver > > Essentially, what I'm saying is that VLANs added with "bridge vlan add" > should only be active while vlan_filtering=1. > > If you search for "commit_pvid" in drivers/net/dsa, you'll find a number > of drivers which have a more elaborate code structure which allows the > commands above to work properly.
Le Thu, 9 Feb 2023 00:03:09 +0200, Vladimir Oltean <olteanv@gmail.com> a écrit : > On Wed, Feb 08, 2023 at 05:17:49PM +0100, Clément Léger wrote: > > +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); > > Is it intentional that this register is written twice? Yes, the A5PSW_VLAN_RES_RD_TAGMASK bit is a toggle-bit (toggled by writing a 1 in it) and it allows to read the tagmask (for vlan tagging) instead of the portmask (for vlan membership): """ b28 read_tagmask: Select contents of mask bits (4:0) when reading the register. If this bit is set during a write into the register, all other bits of the write are ignored (i.e. 30,29,16:0) and the bit 28 of the register toggles (1-> 0; 0-> 1). This is used only to allow changing the bit 28 without changing any table contents. """ > > > + > > + reg &= ~mask; > > + reg |= val; > > + a5psw_reg_writel(a5psw, vlan_res_off, reg); > > +}
Le Wed, 8 Feb 2023 09:38:04 -0800, Florian Fainelli <f.fainelli@gmail.com> a écrit : > > +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); > > If we always enable VLAN mode, which VLAN ID do switch ports not part of > a VLAN aware bridge get classified into? As answered on Vladimir question, it is VLAN_IN_MODE_ENAnot always VLAN enabled as stated by the comment above but only if VLAN_IN_MODE_ENA is set (which is done when setting a PVID only). > > > + > > + /* 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); > > Sort of a follow-on to the previous question, what does transparent > mean? Does that mean the frames ingressing with a certain VLAN tag will > egress with the same VLAN tag in the absence of a VLAN configuration > rewriting the tag? Yes, here is an excerpt of the documentation which should clarified your question (VLAN Table is actually stored in VLAN_RES registers): - If frame’s VLAN id is found in the VLAN table (see Section 4.5.3.9(3)(b), VLAN Domain Resolution / VLAN Table) and the port is defined as tagged for the VLAN, the frame is not modified. - If frame’s VLAN id is found in the VLAN table and the port is defined as untagged for the VLAN, the first VLAN tag is removed from the frame. - If frame’s VLAN id is not found in the VLAN table, the frame is not modified. Thanks,
diff --git a/drivers/net/dsa/rzn1_a5psw.c b/drivers/net/dsa/rzn1_a5psw.c index 0ce3948952db..de6b18ec647d 100644 --- a/drivers/net/dsa/rzn1_a5psw.c +++ b/drivers/net/dsa/rzn1_a5psw.c @@ -583,6 +583,147 @@ 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 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"); + + 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) + return -EINVAL; + } + + 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); + } + + return 0; +} + +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 vlan_res_id; + + dev_dbg(a5psw->dev, "Removing VLAN %d on port %d\n", vid, port); + + vlan_res_id = a5psw_find_vlan_entry(a5psw, vid); + if (vlan_res_id < 0) + return -EINVAL; + + 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); + + return 0; +} + static u64 a5psw_read_stat(struct a5psw *a5psw, u32 offset, int port) { u32 reg_lo, reg_hi; @@ -700,6 +841,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; @@ -772,6 +934,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; @@ -801,6 +965,9 @@ static const struct dsa_switch_ops a5psw_switch_ops = { .port_bridge_flags = a5psw_port_bridge_flags, .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, diff --git a/drivers/net/dsa/rzn1_a5psw.h b/drivers/net/dsa/rzn1_a5psw.h index c67abd49c013..2bad2e3edc2a 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))
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 | 167 +++++++++++++++++++++++++++++++++++ drivers/net/dsa/rzn1_a5psw.h | 8 +- 2 files changed, 172 insertions(+), 3 deletions(-)