Patchwork [OpenWrt-Devel,v3,2/2] 802.1Q VLAN support for ADM6996M/ADM6996FC

login
register
Submitter Peter Lebbing
Date 2011-04-14 15:11:58
Message ID <1302794227.678571.1698.nullmailer@tweek.cal.digitalbrains.com>
Download mbox | patch
Permalink /patch/899/
State Superseded
Headers show

Comments

Peter Lebbing - 2011-04-14 15:11:58
This patch, when applied on top of [PATCH v3 1/2], will enable VLAN support for
the ADM6996FC. It updates the whole to be identical to the patch I sent in on
April 12, though that patch also includes a board fix for the Ubiquiti
RouterStation PHY masks that is not here. And it was against backfire instead of
trunk.

/Please do not include this in OpenWRT just yet/

I cannot test the functionality on the FC chip myself. Yeoh Chun Yeow reported
it works. He figured out the register bit to set so untagged and tagged
memberships both work, albeit not simultaneously on the same switch port.

Things that I think should be tested are, for example:

- Do "all" combinations of VLAN table entry and VLAN ID work? I.e., does the
"vid" property of a VLAN entry work and can it be set to anything in the range 0
through 1023 without limitations. Obviously I'm not advocating trying out all
sixteen thousand possible combinations :).

- Handling of VLAN ID 0, 1 and 1023. These have the possibility to be handled
specially on these chips. More in general, VLAN ID 0 is often a "default" one
and Linux even warns you about using VLAN ID 1. I'm not sure about the status of
VID 1023, but it might be considered invalid by some or a lot of devices. So it
makes sense to scrutinise these three IDs further to see that they don't do
unexpected, unwanted stuff.

The M chip handles VID 0, 1 and 1023 like any other. AFAIK obviously.

- "Unexpected" VIDs. What happens when a tagged packet comes in for a VLAN ID
not configured in the switch? What happens when a tagged packet comes in with a
VLAN ID that is in use on the switch, but the incoming port is not member of
that VLAN? What happens when an untagged packet comes in and the Primary VLAN ID
is set to an unconfigured VLAN or a VLAN of which the port is not a member?

In all these cases, the M chip drops the packet (source port filtering).

- How is the 802.1p priority field handled?

The M chip will pass through the priority field of the incoming packet. If the
incoming packet was untagged, the priority is set to 0.

- Enabling and disabling VLAN support: its effect on switch behaviour.

The two bugs in my original code where related to disabling the VLAN
(enable_vlan set to 0). At least I fell in the pit of testing all functionality,
and not testing the absence of functionality, in a way. The second bug, where
VLAN filters were being set even though enable_vlan = 0, had the potential of
corrupting non-VLAN switching behaviour through 'swconfig set vlan X ports'
statements that should have no effect with enable_vlan = 0 but did have an
effect. I haven't tested it, but I'm fairly sure they would have changed the
switch's behaviour.

---

 files/Documentation/networking/adm6996.txt |   20 +++++-
 files/drivers/net/phy/adm6996.c            |   87 ++++++++++++++++++++++++-----
 patches-2.6.30/620-phy_adm6996.patch       |    3 -
 patches-2.6.31/620-phy_adm6996.patch       |    3 -
 patches-2.6.32/620-phy_adm6996.patch       |    3 -
 patches-2.6.35/620-phy_adm6996.patch       |    3 -
 patches-2.6.36/620-phy_adm6996.patch       |    3 -
 patches-2.6.37/620-phy_adm6996.patch       |    3 -
 patches-2.6.38/620-phy_adm6996.patch       |    3 -
 patches-2.6.39/620-phy_adm6996.patch       |    3 -

Patch

--- target/linux/generic/patches-2.6.35/620-phy_adm6996.patch	(working copy)
+++ target/linux/generic/patches-2.6.35/620-phy_adm6996.patch	(working copy)
@@ -1,6 +1,6 @@ 
 --- a/drivers/net/phy/Kconfig
 +++ b/drivers/net/phy/Kconfig
-@@ -93,6 +93,13 @@ config MICREL_PHY
+@@ -93,6 +93,12 @@ config MICREL_PHY
  	---help---
  	  Supports the KSZ9021, VSC8201, KS8001 PHYs.
  
@@ -9,7 +9,6 @@ 
 +	select SWCONFIG
 +	---help---
 +	  Currently supports the ADM6996FC and ADM6996M switches.
-+	  Support for FC is very limited.
 +
  config FIXED_PHY
  	bool "Driver for MDIO Bus/PHY emulation with fixed speed/link PHYs"
--- target/linux/generic/patches-2.6.36/620-phy_adm6996.patch	(working copy)
+++ target/linux/generic/patches-2.6.36/620-phy_adm6996.patch	(working copy)
@@ -1,6 +1,6 @@ 
 --- a/drivers/net/phy/Kconfig
 +++ b/drivers/net/phy/Kconfig
-@@ -93,6 +93,13 @@ config MICREL_PHY
+@@ -93,6 +93,12 @@ config MICREL_PHY
  	---help---
  	  Supports the KSZ9021, VSC8201, KS8001 PHYs.
  
@@ -9,7 +9,6 @@ 
 +	select SWCONFIG
 +	---help---
 +	  Currently supports the ADM6996FC and ADM6996M switches.
-+	  Support for FC is very limited.
 +
  config FIXED_PHY
  	bool "Driver for MDIO Bus/PHY emulation with fixed speed/link PHYs"
--- target/linux/generic/patches-2.6.37/620-phy_adm6996.patch	(working copy)
+++ target/linux/generic/patches-2.6.37/620-phy_adm6996.patch	(working copy)
@@ -1,6 +1,6 @@ 
 --- a/drivers/net/phy/Kconfig
 +++ b/drivers/net/phy/Kconfig
-@@ -92,6 +92,13 @@ config MICREL_PHY
+@@ -92,6 +92,12 @@ config MICREL_PHY
  	---help---
  	  Supports the KSZ9021, VSC8201, KS8001 PHYs.
  
@@ -9,7 +9,6 @@ 
 +	select SWCONFIG
 +	---help---
 +	  Currently supports the ADM6996FC and ADM6996M switches.
-+	  Support for FC is very limited.
 +
  config FIXED_PHY
  	bool "Driver for MDIO Bus/PHY emulation with fixed speed/link PHYs"
--- target/linux/generic/patches-2.6.38/620-phy_adm6996.patch	(working copy)
+++ target/linux/generic/patches-2.6.38/620-phy_adm6996.patch	(working copy)
@@ -1,6 +1,6 @@ 
 --- a/drivers/net/phy/Kconfig
 +++ b/drivers/net/phy/Kconfig
-@@ -92,6 +92,13 @@ config MICREL_PHY
+@@ -92,6 +92,12 @@ config MICREL_PHY
  	---help---
  	  Supports the KSZ9021, VSC8201, KS8001 PHYs.
  
@@ -9,7 +9,6 @@ 
 +	select SWCONFIG
 +	---help---
 +	  Currently supports the ADM6996FC and ADM6996M switches.
-+	  Support for FC is very limited.
 +
  config FIXED_PHY
  	bool "Driver for MDIO Bus/PHY emulation with fixed speed/link PHYs"
--- target/linux/generic/patches-2.6.39/620-phy_adm6996.patch	(working copy)
+++ target/linux/generic/patches-2.6.39/620-phy_adm6996.patch	(working copy)
@@ -1,6 +1,6 @@ 
 --- a/drivers/net/phy/Kconfig
 +++ b/drivers/net/phy/Kconfig
-@@ -91,6 +91,13 @@ config MICREL_PHY
+@@ -91,6 +91,12 @@ config MICREL_PHY
  	---help---
  	  Supports the KSZ9021, VSC8201, KS8001 PHYs.
  
@@ -9,7 +9,6 @@ 
 +	select SWCONFIG
 +	---help---
 +	  Currently supports the ADM6996FC and ADM6996M switches.
-+	  Support for FC is very limited.
 +
  config FIXED_PHY
  	bool "Driver for MDIO Bus/PHY emulation with fixed speed/link PHYs"
--- target/linux/generic/files/Documentation/networking/adm6996.txt	(revision 0)
+++ target/linux/generic/files/Documentation/networking/adm6996.txt	(revision 0)
@@ -8,10 +8,6 @@ 
   This driver supports the FC and M models only. The ADM6996F and L are
   completely different chips.
   
-  Support for the FC model is extremely limited at the moment. There is no VLAN
-  support as of yet. The driver will not offer an swconfig interface for the FC
-  chip.
- 
 1.1 VLAN IDs
 
   It is possible to define 16 different VLANs. Every VLAN has an identifier, its
@@ -68,7 +64,21 @@ 
   performing a warm reboot might still leave the chip in a "broken" state. Only
   a hardware reset will bring it back in the default state.
 
-2. Technical details on PHYs and the ADM6996
+2. ADM6996FC specific information
+
+  A port can be:
+    - either an untagged member of exactly one VLAN,
+    - or a tagged member of one or more VLANs.
+
+  So a mix of untagged and tagged like the M model supports is not possible.
+  When you isssue a 'vlan X set ports' swconfig command, any existing port
+  membership for the ports will be adjusted to meet these constraints. For
+  clarity and your own sanity, I strongly recommend you obey those constraints
+  yourself while defining membership. 'swconfig dev ethX show' will always show
+  the state that will be set once applied, so there it is What You See Is What
+  You Get.
+
+3. Technical details on PHYs and the ADM6996
 
   From the viewpoint of the Linux kernel, it is common that an Ethernet adapter
   can be seen as a separate MAC entity and a separate PHY entity. The PHY entity
--- target/linux/generic/files/drivers/net/phy/adm6996.c	(working copy)
+++ target/linux/generic/files/drivers/net/phy/adm6996.c	(working copy)
@@ -271,6 +271,27 @@ 
 	return 0;
 };
 
+/*
+ * Clear VLAN memberships for a port. If @untagged, only clear untagged
+ * memberships. Otherwise clear all.
+ */
+static void
+adm6996_clear_memberships (struct adm6996_priv *priv, int port, bool untagged)
+{
+	int i;
+
+	if (untagged) {
+		for (i = 0; i < ADM_NUM_VLANS; i++)
+			if (!(priv->vlan_tagged[i] & (1 << port)))
+				priv->vlan_table[i] &= ~(1 << port);
+	} else {
+		for (i = 0; i < ADM_NUM_VLANS; i++) {
+			priv->vlan_table[i] &= ~(1 << port);
+			priv->vlan_tagged[i] &= ~(1 << port);
+		}
+	}
+}
+
 static int
 adm6996_set_ports(struct switch_dev *dev, struct switch_val *val)
 {
@@ -294,6 +315,13 @@ 
 			""));
 #endif
 
+		if (priv->model == ADM6996FC) {
+			bool untagged_only;
+			untagged_only =
+				p ->flags & (1 << SWITCH_PORT_FLAG_TAGGED);
+			adm6996_clear_memberships (priv, p->id, untagged_only);
+		}
+
 		if (p->flags & (1 << SWITCH_PORT_FLAG_TAGGED))
 			*tagged |= (1 << p->id);
 
@@ -431,6 +459,38 @@ 
 	}
 }
 
+/*
+ * Set "Output Tagged" for ports that are tagged members of a VLAN.
+ *
+ * The ADM6996FC seems to need this, but it should not be used for the M
+ * model. The M can support both tagged and untagged on the same port.
+ *
+ * Precondition: reg_mutex must be held
+ */
+static void adm6996_set_tagged_ports (struct adm6996_priv *priv)
+{
+	u8 tagged = 0;
+	u16 reg;
+	int i;
+
+	for (i = 0; i < ADM_NUM_VLANS; i++) {
+		tagged |= priv->vlan_tagged[i];
+	}
+
+	for (i = 0; i < ADM_NUM_PORTS; i++) {
+		reg = r16(priv->phydev, adm_portcfg[i]);
+
+		if (tagged & (1 << i)) {
+			dev_dbg(&priv->phydev->dev, "port %d tagged\n", i);
+			reg |= ADM_PORTCFG_OT;
+		} else {
+			reg &= ~(ADM_PORTCFG_OT);
+		}
+
+		w16(priv->phydev, adm_portcfg[i], reg);
+	}
+}
+
 static int
 adm6996_hw_apply(struct switch_dev *dev)
 {
@@ -456,6 +516,9 @@ 
 	adm6996_apply_port_pvids(priv);
 	adm6996_apply_vlan_filters(priv);
 
+	if (priv->model == ADM6996FC)
+		adm6996_set_tagged_ports(priv);
+
 out:
 	mutex_unlock(&priv->reg_mutex);
 
@@ -500,13 +563,11 @@ 
 		priv->vlan_tagged[i] = 0;
 	}
 
-	if (priv->model == ADM6996M) {
-		/* Clear VLAN priority map so prio's are unused */
-		w16 (priv->phydev, ADM_VLAN_PRIOMAP, 0);
+	/* Clear VLAN priority map so prio's are unused */
+	w16(priv->phydev, ADM_VLAN_PRIOMAP, 0);
 
-		adm6996_disable_vlan(priv);
-		adm6996_apply_port_pvids(priv);
-	}
+	adm6996_disable_vlan(priv);
+	adm6996_apply_port_pvids(priv);
 }
 
 static int
@@ -641,11 +702,9 @@ 
 	adm6996_perform_reset (priv);
 	mutex_unlock(&priv->reg_mutex);
 
-	if (priv->model == ADM6996M) {
-		if ((ret = register_switch(swdev, pdev->attached_dev)) < 0) {
-			kfree(priv);
-			return ret;
-		}
+	if ((ret = register_switch(swdev, pdev->attached_dev)) < 0) {
+		kfree(priv);
+		return ret;
 	}
 
 	return 0;
@@ -702,10 +761,10 @@ 
 {
 	struct adm6996_priv *priv = phy_to_adm(pdev);
 
-	if (priv != NULL && priv->model == ADM6996M)
+	if (priv != NULL) {
 		unregister_switch(&priv->dev);
-
-	kfree(priv);
+		kfree(priv);
+	}
 }
 
 
--- target/linux/generic/patches-2.6.30/620-phy_adm6996.patch	(working copy)
+++ target/linux/generic/patches-2.6.30/620-phy_adm6996.patch	(working copy)
@@ -1,6 +1,6 @@ 
 --- a/drivers/net/phy/Kconfig
 +++ b/drivers/net/phy/Kconfig
-@@ -82,6 +82,13 @@ config LSI_ET1011C_PHY
+@@ -82,6 +82,12 @@ config LSI_ET1011C_PHY
  	---help---
  	  Supports the LSI ET1011C PHY.
  
@@ -9,7 +9,6 @@ 
 +	select SWCONFIG
 +	---help---
 +	  Currently supports the ADM6996FC and ADM6996M switches.
-+	  Support for FC is very limited.
 +
  config FIXED_PHY
  	bool "Driver for MDIO Bus/PHY emulation with fixed speed/link PHYs"
--- target/linux/generic/patches-2.6.31/620-phy_adm6996.patch	(working copy)
+++ target/linux/generic/patches-2.6.31/620-phy_adm6996.patch	(working copy)
@@ -1,6 +1,6 @@ 
 --- a/drivers/net/phy/Kconfig
 +++ b/drivers/net/phy/Kconfig
-@@ -82,6 +82,13 @@ config LSI_ET1011C_PHY
+@@ -82,6 +82,12 @@ config LSI_ET1011C_PHY
  	---help---
  	  Supports the LSI ET1011C PHY.
  
@@ -9,7 +9,6 @@ 
 +	select SWCONFIG
 +	---help---
 +	  Currently supports the ADM6996FC and ADM6996M switches.
-+	  Support for FC is very limited.
 +
  config FIXED_PHY
  	bool "Driver for MDIO Bus/PHY emulation with fixed speed/link PHYs"
--- target/linux/generic/patches-2.6.32/620-phy_adm6996.patch	(working copy)
+++ target/linux/generic/patches-2.6.32/620-phy_adm6996.patch	(working copy)
@@ -1,6 +1,6 @@ 
 --- a/drivers/net/phy/Kconfig
 +++ b/drivers/net/phy/Kconfig
-@@ -88,6 +88,13 @@ config LSI_ET1011C_PHY
+@@ -88,6 +88,12 @@ config LSI_ET1011C_PHY
  	---help---
  	  Supports the LSI ET1011C PHY.
  
@@ -9,7 +9,6 @@ 
 +	select SWCONFIG
 +	---help---
 +	  Currently supports the ADM6996FC and ADM6996M switches.
-+	  Support for FC is very limited.
 +
  config FIXED_PHY
  	bool "Driver for MDIO Bus/PHY emulation with fixed speed/link PHYs"