Messages in this thread | | | From | Paweł Dembicki <> | Date | Thu, 29 Jun 2023 23:00:06 +0200 | Subject | Re: [PATCH net-next v2 4/7] net: dsa: vsc73xx: Add dsa tagging based on 8021q |
| |
niedz., 25 cze 2023 o 14:47 Vladimir Oltean <olteanv@gmail.com> napisał(a): > > On Sun, Jun 25, 2023 at 01:53:39PM +0200, Pawel Dembicki wrote: > > This patch is simple implementation of 8021q tagging in vsc73xx driver. > > At this moment devices with DSA_TAG_PROTO_NONE are useless. VSC73XX > > family doesn't provide any tag support for external ethernet ports. > > > > The only way is vlan-based tagging. It require constant hardware vlan > > filtering. VSC73XX family support provider bridging but QinQ only without > > fully implemented 802.1AD. It allow only doubled 0x8100 TPID. > > > > In simple port mode QinQ is enabled to preserve forwarding vlan tagged > > frames. > > > > Tag driver introduce most simple funcionality required for proper taging > > support. > > > > Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > > Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com> > > --- > > v2: > > - change poll loop into dedicated macro > > - fix style issues > > > > drivers/net/dsa/Kconfig | 2 +- > > drivers/net/dsa/vitesse-vsc73xx-core.c | 532 +++++++++++++++++++++---- > > include/net/dsa.h | 2 + > > net/dsa/Kconfig | 6 + > > net/dsa/Makefile | 1 + > > net/dsa/tag_vsc73xx_8021q.c | 87 ++++ > > 6 files changed, 544 insertions(+), 86 deletions(-) > > create mode 100644 net/dsa/tag_vsc73xx_8021q.c > > > > diff --git a/drivers/net/dsa/Kconfig b/drivers/net/dsa/Kconfig > > index 3ed5391bb18d..4cf0166fef7b 100644 > > --- a/drivers/net/dsa/Kconfig > > +++ b/drivers/net/dsa/Kconfig > > @@ -125,7 +125,7 @@ config NET_DSA_SMSC_LAN9303_MDIO > > > > config NET_DSA_VITESSE_VSC73XX > > tristate > > - select NET_DSA_TAG_NONE > > + select NET_DSA_TAG_VSC73XX > > select FIXED_PHY > > select VITESSE_PHY > > select GPIOLIB > > diff --git a/drivers/net/dsa/vitesse-vsc73xx-core.c b/drivers/net/dsa/vitesse-vsc73xx-core.c > > index f123ce2ed244..f7c38f9a81a8 100644 > > --- a/drivers/net/dsa/vitesse-vsc73xx-core.c > > +++ b/drivers/net/dsa/vitesse-vsc73xx-core.c > > @@ -17,6 +17,7 @@ > > #include <linux/kernel.h> > > #include <linux/module.h> > > #include <linux/device.h> > > +#include <linux/iopoll.h> > > #include <linux/of.h> > > #include <linux/of_device.h> > > #include <linux/of_mdio.h> > > @@ -25,6 +26,7 @@ > > #include <linux/etherdevice.h> > > #include <linux/gpio/consumer.h> > > #include <linux/gpio/driver.h> > > +#include <linux/dsa/8021q.h> > > #include <linux/random.h> > > #include <net/dsa.h> > > > > @@ -62,6 +64,8 @@ > > #define VSC73XX_CAT_DROP 0x6e > > #define VSC73XX_CAT_PR_MISC_L2 0x6f > > #define VSC73XX_CAT_PR_USR_PRIO 0x75 > > +#define VSC73XX_CAT_VLAN_MISC 0x79 > > +#define VSC73XX_CAT_PORT_VLAN 0x7a > > #define VSC73XX_Q_MISC_CONF 0xdf > > > > /* MAC_CFG register bits */ > > @@ -122,6 +126,17 @@ > > #define VSC73XX_ADVPORTM_IO_LOOPBACK BIT(1) > > #define VSC73XX_ADVPORTM_HOST_LOOPBACK BIT(0) > > > > +/* TXUPDCFG transmit modify setup bits */ > > +#define VSC73XX_TXUPDCFG_DSCP_REWR_MODE GENMASK(20, 19) > > +#define VSC73XX_TXUPDCFG_DSCP_REWR_ENA BIT(18) > > +#define VSC73XX_TXUPDCFG_TX_INT_TO_USRPRIO_ENA BIT(17) > > +#define VSC73XX_TXUPDCFG_TX_UNTAGGED_VID GENMASK(15, 4) > > +#define VSC73XX_TXUPDCFG_TX_UNTAGGED_VID_ENA BIT(3) > > +#define VSC73XX_TXUPDCFG_TX_UPDATE_CRC_CPU_ENA BIT(1) > > +#define VSC73XX_TXUPDCFG_TX_INSERT_TAG BIT(0) > > + > > +#define VSC73XX_TXUPDCFG_TX_UNTAGGED_VID_SHIFT 4 > > + > > /* CAT_DROP categorizer frame dropping register bits */ > > #define VSC73XX_CAT_DROP_DROP_MC_SMAC_ENA BIT(6) > > #define VSC73XX_CAT_DROP_FWD_CTRL_ENA BIT(4) > > @@ -135,6 +150,15 @@ > > #define VSC73XX_Q_MISC_CONF_EARLY_TX_512 (1 << 1) > > #define VSC73XX_Q_MISC_CONF_MAC_PAUSE_MODE BIT(0) > > > > +/* CAT_VLAN_MISC categorizer VLAN miscellaneous bits*/ > > +#define VSC73XX_CAT_VLAN_MISC_VLAN_TCI_IGNORE_ENA BIT(8) > > +#define VSC73XX_CAT_VLAN_MISC_VLAN_KEEP_TAG_ENA BIT(7) > > + > > +/* CAT_PORT_VLAN categorizer port VLAN*/ > > +#define VSC73XX_CAT_PORT_VLAN_VLAN_CFI BIT(15) > > +#define VSC73XX_CAT_PORT_VLAN_VLAN_USR_PRIO GENMASK(14, 12) > > +#define VSC73XX_CAT_PORT_VLAN_VLAN_VID GENMASK(11, 0) > > + > > /* Frame analyzer block 2 registers */ > > #define VSC73XX_STORMLIMIT 0x02 > > #define VSC73XX_ADVLEARN 0x03 > > @@ -189,7 +213,8 @@ > > #define VSC73XX_VLANACCESS_VLAN_MIRROR BIT(29) > > #define VSC73XX_VLANACCESS_VLAN_SRC_CHECK BIT(28) > > #define VSC73XX_VLANACCESS_VLAN_PORT_MASK GENMASK(9, 2) > > -#define VSC73XX_VLANACCESS_VLAN_TBL_CMD_MASK GENMASK(2, 0) > > +#define VSC73XX_VLANACCESS_VLAN_PORT_MASK_SHIFT 2 > > +#define VSC73XX_VLANACCESS_VLAN_TBL_CMD_MASK GENMASK(1, 0) > > #define VSC73XX_VLANACCESS_VLAN_TBL_CMD_IDLE 0 > > #define VSC73XX_VLANACCESS_VLAN_TBL_CMD_READ_ENTRY 1 > > #define VSC73XX_VLANACCESS_VLAN_TBL_CMD_WRITE_ENTRY 2 > > @@ -344,6 +369,13 @@ static const struct vsc73xx_counter vsc73xx_tx_counters[] = { > > { 29, "TxQoSClass3" }, /* non-standard counter */ > > }; > > > > +enum vsc73xx_port_vlan_conf { > > + VSC73XX_VLAN_UNAWARE, > > + VSC73XX_VLAN_AWARE, > > + VSC73XX_DOUBLE_VLAN_AWARE, > > + VSC73XX_DOUBLE_VLAN_CPU_AWARE, > > +}; > > + > > int vsc73xx_is_addr_valid(u8 block, u8 subblock) > > { > > switch (block) { > > @@ -558,90 +590,7 @@ static enum dsa_tag_protocol vsc73xx_get_tag_protocol(struct dsa_switch *ds, > > * cannot access the tag. (See "Internal frame header" section > > * 3.9.1 in the manual.) > > */ > > - return DSA_TAG_PROTO_NONE; > > -} > > - > > -static int vsc73xx_setup(struct dsa_switch *ds) > > -{ > > - struct vsc73xx *vsc = ds->priv; > > - int i; > > - > > - dev_info(vsc->dev, "set up the switch\n"); > > - > > - /* Issue RESET */ > > - vsc73xx_write(vsc, VSC73XX_BLOCK_SYSTEM, 0, VSC73XX_GLORESET, > > - VSC73XX_GLORESET_MASTER_RESET); > > - usleep_range(125, 200); > > - > > - /* Initialize memory, initialize RAM bank 0..15 except 6 and 7 > > - * This sequence appears in the > > - * VSC7385 SparX-G5 datasheet section 6.6.1 > > - * VSC7395 SparX-G5e datasheet section 6.6.1 > > - * "initialization sequence". > > - * No explanation is given to the 0x1010400 magic number. > > - */ > > - for (i = 0; i <= 15; i++) { > > - if (i != 6 && i != 7) { > > - vsc73xx_write(vsc, VSC73XX_BLOCK_MEMINIT, > > - 2, > > - 0, 0x1010400 + i); > > - mdelay(1); > > - } > > - } > > - mdelay(30); > > - > > - /* Clear MAC table */ > > - vsc73xx_write(vsc, VSC73XX_BLOCK_ANALYZER, 0, > > - VSC73XX_MACACCESS, > > - VSC73XX_MACACCESS_CMD_CLEAR_TABLE); > > - > > - /* Clear VLAN table */ > > - vsc73xx_write(vsc, VSC73XX_BLOCK_ANALYZER, 0, > > - VSC73XX_VLANACCESS, > > - VSC73XX_VLANACCESS_VLAN_TBL_CMD_CLEAR_TABLE); > > - > > - msleep(40); > > - > > - /* Use 20KiB buffers on all ports on VSC7395 > > - * The VSC7385 has 16KiB buffers and that is the > > - * default if we don't set this up explicitly. > > - * Port "31" is "all ports". > > - */ > > - if (IS_739X(vsc)) > > - vsc73xx_write(vsc, VSC73XX_BLOCK_MAC, 0x1f, > > - VSC73XX_Q_MISC_CONF, > > - VSC73XX_Q_MISC_CONF_EXTENT_MEM); > > - > > - /* Put all ports into reset until enabled */ > > - for (i = 0; i < 7; i++) { > > - if (i == 5) > > - continue; > > - vsc73xx_write(vsc, VSC73XX_BLOCK_MAC, 4, > > - VSC73XX_MAC_CFG, VSC73XX_MAC_CFG_RESET); > > - } > > - > > - /* MII delay, set both GTX and RX delay to 2 ns */ > > - vsc73xx_write(vsc, VSC73XX_BLOCK_SYSTEM, 0, VSC73XX_GMIIDELAY, > > - VSC73XX_GMIIDELAY_GMII0_GTXDELAY_2_0_NS | > > - VSC73XX_GMIIDELAY_GMII0_RXDELAY_2_0_NS); > > - /* IP multicast flood mask (table 144) */ > > - vsc73xx_write(vsc, VSC73XX_BLOCK_ANALYZER, 0, VSC73XX_IFLODMSK, > > - 0xff); > > - > > - mdelay(50); > > - > > - /*configure forward map to CPU <-> port only*/ > > - for (i = 0; i < vsc->ds->num_ports; i++) > > - vsc->forward_map[i] = VSC73XX_SRCMASKS_PORTS_MASK & BIT(CPU_PORT); > > - vsc->forward_map[CPU_PORT] = VSC73XX_SRCMASKS_PORTS_MASK & ~BIT(CPU_PORT); > > - > > - /* Release reset from the internal PHYs */ > > - vsc73xx_write(vsc, VSC73XX_BLOCK_SYSTEM, 0, VSC73XX_GLORESET, > > - VSC73XX_GLORESET_PHY_RESET); > > - > > - udelay(4); > > - > > - return 0; > > Try to make this code movement part of a separate patch. It's > distracting and makes it hard to see what really changed. > > > + return DSA_TAG_PROTO_VSC73XX_8021Q; > > } > > > > static void vsc73xx_init_port(struct vsc73xx *vsc, int port) > > @@ -1078,6 +1027,417 @@ static void vsc73xx_port_stp_state_set(struct dsa_switch *ds, int port, > > VSC73XX_SRCMASKS_PORTS_MASK, 0); > > } > > > > +static int > > +vsc73xx_port_wait_for_vlan_table_cmd(struct vsc73xx *vsc) > > +{ > > + u32 val; > > + int ret, err; > > + > > + ret = read_poll_timeout(vsc73xx_read, err, > > + err < 0 || ((val & VSC73XX_VLANACCESS_VLAN_TBL_CMD_MASK) == > > + VSC73XX_VLANACCESS_VLAN_TBL_CMD_IDLE), > > + 1000, 10000, false, vsc, VSC73XX_BLOCK_ANALYZER, > > + 0, VSC73XX_VLANACCESS, &val); > > + if (ret) > > + return ret; > > + return err; > > +} > > + > > +static int > > +vsc73xx_port_read_vlan_table_entry(struct dsa_switch *ds, u16 vid, u8 *portmap) > > +{ > > + struct vsc73xx *vsc = ds->priv; > > + u32 val; > > + int ret; > > + > > + if (vid > 4095) > > + return -EPERM; > > Unnecessary defensive programming. > > > + vsc73xx_write(vsc, VSC73XX_BLOCK_ANALYZER, 0, VSC73XX_VLANTIDX, vid); > > + ret = vsc73xx_port_wait_for_vlan_table_cmd(vsc); > > + if (ret) > > + return ret; > > + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_ANALYZER, 0, VSC73XX_VLANACCESS, > > + VSC73XX_VLANACCESS_VLAN_TBL_CMD_MASK, > > + VSC73XX_VLANACCESS_VLAN_TBL_CMD_READ_ENTRY); > > + ret = vsc73xx_port_wait_for_vlan_table_cmd(vsc); > > + if (ret) > > + return ret; > > + vsc73xx_read(vsc, VSC73XX_BLOCK_ANALYZER, 0, VSC73XX_VLANACCESS, &val); > > + *portmap = (val & VSC73XX_VLANACCESS_VLAN_PORT_MASK) >> > > + VSC73XX_VLANACCESS_VLAN_PORT_MASK_SHIFT; > > + return 0; > > +} > > + > > +static int > > +vsc73xx_port_write_vlan_table_entry(struct dsa_switch *ds, u16 vid, u8 portmap) > > +{ > > + struct vsc73xx *vsc = ds->priv; > > + int ret; > > + > > + if (vid > 4095) > > + return -EPERM; > > Unnecessary defensive programming. > > > + vsc73xx_write(vsc, VSC73XX_BLOCK_ANALYZER, 0, VSC73XX_VLANTIDX, vid); > > + ret = vsc73xx_port_wait_for_vlan_table_cmd(vsc); > > + if (ret) > > + return ret; > > + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_ANALYZER, 0, VSC73XX_VLANACCESS, > > + VSC73XX_VLANACCESS_VLAN_TBL_CMD_MASK | > > + VSC73XX_VLANACCESS_VLAN_SRC_CHECK | > > + VSC73XX_VLANACCESS_VLAN_PORT_MASK, > > + VSC73XX_VLANACCESS_VLAN_TBL_CMD_WRITE_ENTRY | > > + VSC73XX_VLANACCESS_VLAN_SRC_CHECK | > > + (portmap << > > + VSC73XX_VLANACCESS_VLAN_PORT_MASK_SHIFT)); > > + ret = vsc73xx_port_wait_for_vlan_table_cmd(vsc); > > + if (ret) > > + return ret; > > + return 0; > > +} > > + > > +static int > > +vsc73xx_port_update_vlan_table(struct dsa_switch *ds, int port, u16 vid, > > + bool set) > > +{ > > + u8 portmap; > > + int ret; > > + > > + if (vid > 4095) > > + return -EPERM; > > Unnecessary defensive programming. > > > + > > + ret = vsc73xx_port_read_vlan_table_entry(ds, vid, &portmap); > > + if (ret) > > + return ret; > > + > > + if (set) > > + portmap |= BIT(port) | BIT(CPU_PORT); > > + else > > + portmap &= ~BIT(port); > > + > > + if (portmap == BIT(CPU_PORT)) > > + portmap = 0; > > + > > + ret = vsc73xx_port_write_vlan_table_entry(ds, vid, portmap); > > + > > + return ret; > > return vsc73xx_port_write_vlan_table_entry(...) > > > +} > > + > > +static void > > +vsc73xx_port_set_vlan_conf(struct dsa_switch *ds, int port, > > + enum vsc73xx_port_vlan_conf port_vlan_conf) > > +{ > > + struct vsc73xx *vsc = ds->priv; > > + > > + if (port_vlan_conf == VSC73XX_VLAN_UNAWARE) { > > + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, > > + VSC73XX_MAC_CFG, > > + VSC73XX_MAC_CFG_VLAN_AWR, 0); > > + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, > > + VSC73XX_MAC_CFG, > > + VSC73XX_MAC_CFG_VLAN_DBLAWR, 0); > > + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, > > + VSC73XX_CAT_VLAN_MISC, > > + VSC73XX_CAT_VLAN_MISC_VLAN_TCI_IGNORE_ENA, > > + VSC73XX_CAT_VLAN_MISC_VLAN_TCI_IGNORE_ENA); > > + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, > > + VSC73XX_CAT_VLAN_MISC, > > + VSC73XX_CAT_VLAN_MISC_VLAN_KEEP_TAG_ENA, > > + VSC73XX_CAT_VLAN_MISC_VLAN_KEEP_TAG_ENA); > > + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, > > + VSC73XX_CAT_DROP, > > + VSC73XX_CAT_DROP_TAGGED_ENA, 0); > > + } else { > > + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, > > + VSC73XX_MAC_CFG, > > + VSC73XX_MAC_CFG_VLAN_AWR, > > + VSC73XX_MAC_CFG_VLAN_AWR); > > + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, > > + VSC73XX_CAT_DROP, > > + VSC73XX_CAT_DROP_TAGGED_ENA, 0); > > + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, > > + VSC73XX_CAT_DROP, > > + VSC73XX_CAT_DROP_UNTAGGED_ENA, > > + VSC73XX_CAT_DROP_UNTAGGED_ENA); > > + > > + if (port_vlan_conf == VSC73XX_DOUBLE_VLAN_CPU_AWARE) > > + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, > > + VSC73XX_MAC_CFG, > > + VSC73XX_MAC_CFG_VLAN_DBLAWR, > > + VSC73XX_MAC_CFG_VLAN_DBLAWR); > > + else > > + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, > > + VSC73XX_MAC_CFG, > > + VSC73XX_MAC_CFG_VLAN_DBLAWR, 0); > > + > > + if (port_vlan_conf == VSC73XX_DOUBLE_VLAN_AWARE) { > > + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, > > + VSC73XX_CAT_VLAN_MISC, > > + VSC73XX_CAT_VLAN_MISC_VLAN_TCI_IGNORE_ENA, > > + VSC73XX_CAT_VLAN_MISC_VLAN_TCI_IGNORE_ENA); > > + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, > > + VSC73XX_CAT_VLAN_MISC, > > + VSC73XX_CAT_VLAN_MISC_VLAN_KEEP_TAG_ENA, > > + VSC73XX_CAT_VLAN_MISC_VLAN_KEEP_TAG_ENA); > > + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, > > + VSC73XX_TXUPDCFG, > > + VSC73XX_TXUPDCFG_TX_INSERT_TAG, 0); > > + } else { > > + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, > > + VSC73XX_CAT_VLAN_MISC, > > + VSC73XX_CAT_VLAN_MISC_VLAN_TCI_IGNORE_ENA, > > + 0); > > + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, > > + VSC73XX_CAT_VLAN_MISC, > > + VSC73XX_CAT_VLAN_MISC_VLAN_KEEP_TAG_ENA, > > + 0); > > + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, > > + VSC73XX_TXUPDCFG, > > + VSC73XX_TXUPDCFG_TX_INSERT_TAG, > > + VSC73XX_TXUPDCFG_TX_INSERT_TAG); > > + } > > + > > + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, > > + VSC73XX_TXUPDCFG, > > + VSC73XX_TXUPDCFG_TX_UNTAGGED_VID_ENA, 0); > > + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, > > + VSC73XX_TXUPDCFG, > > + VSC73XX_TXUPDCFG_TX_UNTAGGED_VID, 0); > > + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, > > + VSC73XX_CAT_PORT_VLAN, > > + VSC73XX_CAT_PORT_VLAN_VLAN_VID, 0); > > + } > > +} > > + > > I need some time to understand what is being done here. > > > +static int vsc73xx_port_set_double_vlan_aware(struct dsa_switch *ds, int port) > > +{ > > + int i, ret; > > + > > + if (port == CPU_PORT) > > + vsc73xx_port_set_vlan_conf(ds, port, > > + VSC73XX_DOUBLE_VLAN_CPU_AWARE); > > + else > > + vsc73xx_port_set_vlan_conf(ds, port, > > + VSC73XX_DOUBLE_VLAN_AWARE); > > + > > + for (i = 0; i <= 4095; i++) { > > i < VLAN_N_VID > > > + ret = vsc73xx_port_update_vlan_table(ds, port, i, 0); > > + if (ret) > > + return ret; > > Clearing the VLAN table absolutely does not belong in a function that > makes a port aware of double VLANs. And neither does it belong in a > function that changes VLAN awareness, but that's not part of this patch, > but 6/7. The bridge expects the VLAN table to be able to be populated > asynchronously relative to enabling/disabling it. The same for the > VLAN-aware FDB and MDB entries. > > > + } > > + return ret; > > +} > > + > > +static int vsc73xx_vlan_set_untagged(struct dsa_switch *ds, int port, u16 vid, > > + bool port_vlan) > > +{ > > + struct vsc73xx *vsc = ds->priv; > > + u16 vlan_no; > > + u32 val; > > + > > + vsc73xx_read(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_TXUPDCFG, &val); > > + > > + if (port_vlan && (val & VSC73XX_TXUPDCFG_TX_UNTAGGED_VID_ENA)) { > > + vsc73xx_read(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_TXUPDCFG, > > + &val); > > + vlan_no = (val & VSC73XX_TXUPDCFG_TX_UNTAGGED_VID) >> > > + VSC73XX_TXUPDCFG_TX_UNTAGGED_VID_SHIFT; > > + if (!vid_is_dsa_8021q(vlan_no) && !vid_is_dsa_8021q(vid) && > > + vlan_no != vid) { > > + dev_warn(vsc->dev, > > + "Port %d can have only one untagged vid! Now is: %d.\n", > > + port, vlan_no); > > + return -EPERM; > > drop indentation by 1 level > > don't return -EPERM, as that indicates a permissions-related issue to > the user process. Perhaps -EOPNOTSUPP. > > use the extack for the message.. we now also have NL_SET_ERR_MSG_FMT() > > > + } > > + } > > + > > + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_CAT_DROP, > > + VSC73XX_CAT_DROP_UNTAGGED_ENA, 0); > > + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_TXUPDCFG, > > + VSC73XX_TXUPDCFG_TX_UNTAGGED_VID_ENA, > > + VSC73XX_TXUPDCFG_TX_UNTAGGED_VID_ENA); > > + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_TXUPDCFG, > > + VSC73XX_TXUPDCFG_TX_UNTAGGED_VID, > > + (vid << VSC73XX_TXUPDCFG_TX_UNTAGGED_VID_SHIFT) & > > + VSC73XX_TXUPDCFG_TX_UNTAGGED_VID); > > + return 0; > > +} > > + > > +static int vsc73xx_vlan_set_pvid(struct dsa_switch *ds, int port, u16 vid, > > + bool port_vlan) > > +{ > > + struct dsa_port *dsa_port = dsa_to_port(ds, port); > > + struct vsc73xx *vsc = ds->priv; > > + u16 vlan_no; > > + u32 val; > > + > > + if (!dsa_port) > > + return -EINVAL; > > + > > + vsc73xx_read(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_CAT_PORT_VLAN, &val); > > + vlan_no = val & VSC73XX_CAT_PORT_VLAN_VLAN_VID; > > + > > + if (port_vlan && vlan_no && !vid_is_dsa_8021q(vlan_no) && > > + !vid_is_dsa_8021q(vid) && vlan_no != vid) { > > + dev_warn(vsc->dev, > > + "Port %d can have only one pvid! Now is: %d.\n", > > + port, vlan_no); > > + return -EPERM; > > + } > > + > > + if (dsa_port_is_vlan_filtering(dsa_port)) > > + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, > > + VSC73XX_CAT_VLAN_MISC, > > + VSC73XX_CAT_VLAN_MISC_VLAN_TCI_IGNORE_ENA, > > + 0); > > + else > > + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, > > + VSC73XX_CAT_VLAN_MISC, > > + VSC73XX_CAT_VLAN_MISC_VLAN_TCI_IGNORE_ENA, > > + VSC73XX_CAT_VLAN_MISC_VLAN_TCI_IGNORE_ENA); > > + > > + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_CAT_VLAN_MISC, > > + VSC73XX_CAT_VLAN_MISC_VLAN_KEEP_TAG_ENA, > > + VSC73XX_CAT_VLAN_MISC_VLAN_KEEP_TAG_ENA); > > + > > + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_CAT_PORT_VLAN, > > + VSC73XX_CAT_PORT_VLAN_VLAN_VID, > > + vid & VSC73XX_CAT_PORT_VLAN_VLAN_VID); > > Why would changing the PVID (what this function does) alter VLAN_TCI_IGNORE_ENA > and VLAN_KEEP_TAG_ENA? > > > + return 0; > > +} > > + > > +static int vsc73xx_tag_8021q_vlan_add(struct dsa_switch *ds, int port, u16 vid, > > + u16 flags) > > +{ > > + bool untagged = flags & BRIDGE_VLAN_INFO_UNTAGGED; > > + bool pvid = flags & BRIDGE_VLAN_INFO_PVID; > > + int ret; > > + > > + if (untagged) { > > + ret = vsc73xx_vlan_set_untagged(ds, port, vid, false); > > + if (ret) > > + return ret; > > + } > > + if (pvid) { > > + ret = vsc73xx_vlan_set_pvid(ds, port, vid, false); > > + if (ret) > > + return ret; > > + } > > + ret = vsc73xx_port_update_vlan_table(ds, port, vid, 1); > > return vsc73xx_port_update_vlan_table(...) > > > + if (ret) > > + return ret; > > + > > + return 0; > > +} > > + > > +static int vsc73xx_tag_8021q_vlan_del(struct dsa_switch *ds, int port, u16 vid) > > +{ > > + return vsc73xx_port_update_vlan_table(ds, port, vid, 0); > > +} > > + > > +static int vsc73xx_setup(struct dsa_switch *ds) > > +{ > > + struct vsc73xx *vsc = ds->priv; > > + int i, ret; > > + > > + dev_info(vsc->dev, "set up the switch\n"); > > + > > + ds->vlan_filtering_is_global = false; > > No reason to set this, it's false by default. > > > + ds->configure_vlan_while_not_filtering = false; > > This is a legacy flag that should be set to true (default value) for all > drivers and then eliminated. > > To check whether your implementation is doing the right thing, you must > remove this line, and make sure that after this configuration: > > ip link add br0 type bridge vlan_filtering 0 > ip link set swp0 master br0 > bridge vlan add dev swp0 vid 100 pvid untagged > > the VLAN-unaware forwarding of the switch is not affected. This means > that the PVID of the port that is committed to hardware (and thus, the > allowed destination port mask) must remain what the driver set it to, > while the bridge vlan_filtering option is not enabled. The hardware PVID > must only follow the bridge port PVID while in vlan_filtering=true mode. > > > + > > + /* Issue RESET */ > > + vsc73xx_write(vsc, VSC73XX_BLOCK_SYSTEM, 0, VSC73XX_GLORESET, > > + VSC73XX_GLORESET_MASTER_RESET); > > + usleep_range(125, 200); > > + > > + /* Initialize memory, initialize RAM bank 0..15 except 6 and 7 > > + * This sequence appears in the > > + * VSC7385 SparX-G5 datasheet section 6.6.1 > > + * VSC7395 SparX-G5e datasheet section 6.6.1 > > + * "initialization sequence". > > + * No explanation is given to the 0x1010400 magic number. > > + */ > > + for (i = 0; i <= 15; i++) { > > + if (i != 6 && i != 7) { > > + vsc73xx_write(vsc, VSC73XX_BLOCK_MEMINIT, > > + 2, > > + 0, 0x1010400 + i); > > + mdelay(1); > > + } > > + } > > + mdelay(30); > > + > > + /* Clear MAC table */ > > + vsc73xx_write(vsc, VSC73XX_BLOCK_ANALYZER, 0, > > + VSC73XX_MACACCESS, > > + VSC73XX_MACACCESS_CMD_CLEAR_TABLE); > > + > > + /* Clear VLAN table */ > > + vsc73xx_write(vsc, VSC73XX_BLOCK_ANALYZER, 0, > > + VSC73XX_VLANACCESS, > > + VSC73XX_VLANACCESS_VLAN_TBL_CMD_CLEAR_TABLE); > > + > > + msleep(40); > > + > > + /* Use 20KiB buffers on all ports on VSC7395 > > + * The VSC7385 has 16KiB buffers and that is the > > + * default if we don't set this up explicitly. > > + * Port "31" is "all ports". > > + */ > > + if (IS_739X(vsc)) > > + vsc73xx_write(vsc, VSC73XX_BLOCK_MAC, 0x1f, > > + VSC73XX_Q_MISC_CONF, > > + VSC73XX_Q_MISC_CONF_EXTENT_MEM); > > + > > + /* Put all ports into reset until enabled */ > > + for (i = 0; i < 7; i++) { > > + if (i == 5) > > + continue; > > + vsc73xx_write(vsc, VSC73XX_BLOCK_MAC, 4, > > + VSC73XX_MAC_CFG, VSC73XX_MAC_CFG_RESET); > > + } > > + > > + /* MII delay, set both GTX and RX delay to 2 ns */ > > + vsc73xx_write(vsc, VSC73XX_BLOCK_SYSTEM, 0, VSC73XX_GMIIDELAY, > > + VSC73XX_GMIIDELAY_GMII0_GTXDELAY_2_0_NS | > > + VSC73XX_GMIIDELAY_GMII0_RXDELAY_2_0_NS); > > + /* Ingess VLAN reception mask (table 145) */ > > s/ingess/ingress/ > > > + vsc73xx_write(vsc, VSC73XX_BLOCK_ANALYZER, 0, VSC73XX_VLANMASK, > > + 0x5f); > > + /* IP multicast flood mask (table 144) */ > > + vsc73xx_write(vsc, VSC73XX_BLOCK_ANALYZER, 0, VSC73XX_IFLODMSK, > > + 0xff); > > + > > + mdelay(50); > > + > > + for (i = 0; i < vsc->ds->num_ports; i++) { > > + if (i == 5) > > + continue; > > dsa_switch_for_each_available_port() > > > + ret = vsc73xx_port_set_double_vlan_aware(ds, i); > > + if (ret) > > + return ret; > > If ports are made VLAN-aware by default (including in standalone mode), > shouldn't this be reflected in ds->needs_standalone_vlan_filtering? >
Not for QinQ. Hardware filtering without vlan_filtering=1 is used only for separation of outer vlans.
> > + } > > + > > + rtnl_lock(); > > + ret = dsa_tag_8021q_register(ds, htons(ETH_P_8021Q)); > > + rtnl_unlock(); > > + if (ret) > > + return ret; > > + > > + /*configure forward map to CPU <-> port only*/ > > style: space after /* and before */ (also present in a few other places) > > > + for (i = 0; i < vsc->ds->num_ports; i++) > > + vsc->forward_map[i] = VSC73XX_SRCMASKS_PORTS_MASK & > > + BIT(CPU_PORT); > > + vsc->forward_map[CPU_PORT] = VSC73XX_SRCMASKS_PORTS_MASK & > > + ~BIT(CPU_PORT); > > + > > + /* Release reset from the internal PHYs */ > > + vsc73xx_write(vsc, VSC73XX_BLOCK_SYSTEM, 0, VSC73XX_GLORESET, > > + VSC73XX_GLORESET_PHY_RESET); > > + > > + udelay(4); > > + > > + return 0; > > +} > > + > > static const struct dsa_switch_ops vsc73xx_ds_ops = { > > .get_tag_protocol = vsc73xx_get_tag_protocol, > > .setup = vsc73xx_setup, > > @@ -1095,6 +1455,8 @@ static const struct dsa_switch_ops vsc73xx_ds_ops = { > > .port_change_mtu = vsc73xx_change_mtu, > > .port_max_mtu = vsc73xx_get_max_mtu, > > .port_stp_state_set = vsc73xx_port_stp_state_set, > > + .tag_8021q_vlan_add = vsc73xx_tag_8021q_vlan_add, > > + .tag_8021q_vlan_del = vsc73xx_tag_8021q_vlan_del, > > }; > > > > static int vsc73xx_gpio_get(struct gpio_chip *chip, unsigned int offset) > > diff --git a/include/net/dsa.h b/include/net/dsa.h > > index 75022cf771cf..2440df7ea6c9 100644 > > --- a/include/net/dsa.h > > +++ b/include/net/dsa.h > > @@ -56,6 +56,7 @@ struct phylink_link_state; > > #define DSA_TAG_PROTO_RTL8_4T_VALUE 25 > > #define DSA_TAG_PROTO_RZN1_A5PSW_VALUE 26 > > #define DSA_TAG_PROTO_LAN937X_VALUE 27 > > +#define DSA_TAG_PROTO_VSC73XX_8021Q_VALUE 28 > > > > enum dsa_tag_protocol { > > DSA_TAG_PROTO_NONE = DSA_TAG_PROTO_NONE_VALUE, > > @@ -86,6 +87,7 @@ enum dsa_tag_protocol { > > DSA_TAG_PROTO_RTL8_4T = DSA_TAG_PROTO_RTL8_4T_VALUE, > > DSA_TAG_PROTO_RZN1_A5PSW = DSA_TAG_PROTO_RZN1_A5PSW_VALUE, > > DSA_TAG_PROTO_LAN937X = DSA_TAG_PROTO_LAN937X_VALUE, > > + DSA_TAG_PROTO_VSC73XX_8021Q = DSA_TAG_PROTO_VSC73XX_8021Q_VALUE, > > }; > > > > struct dsa_switch; > > diff --git a/net/dsa/Kconfig b/net/dsa/Kconfig > > index 8e698bea99a3..e59360071c67 100644 > > --- a/net/dsa/Kconfig > > +++ b/net/dsa/Kconfig > > @@ -166,6 +166,12 @@ config NET_DSA_TAG_TRAILER > > Say Y or M if you want to enable support for tagging frames at > > with a trailed. e.g. Marvell 88E6060. > > > > +config NET_DSA_TAG_VSC73XX_8021Q > > + tristate "Tag driver for Microchip/Vitesse VSC73xx family of switches, using VLAN" > > + help > > + Say Y or M if you want to enable support for tagging frames with a > > + custom VLAN-based header. > > + > > Separate patches for the tagger and for the driver infrastructure please. > It's hard to review. > > Also, there's some very strange splitting between this patch and 6/7 > ("net: dsa: vsc73xx: Add vlan filtering"). I would have expected that to > contain the basic infrastructure (thus appearing first), then the > driver-side tag_8021q infra, then the tagger. It's impossible for me to > comment on things that have to do with VLAN infrastructure when it's > split between 2 patches.
I will rework it: move vsc73xx_setup movement to a different commit, the same with tagger code and squash all vlan stuff. In this version, I decided to make tag 8021q first, because it's impossible/very hard to use and test the driver with TAG_NONE at this moment.
-- Pawel Dembicki
| |