Patchwork [v2,2/2] drivers: net:ethernet: cpsw: add support for VLAN

login
register
mail settings
Submitter Mugunthan V N
Date Jan. 30, 2013, 7:07 p.m.
Message ID <1359572855-12344-3-git-send-email-mugunthanvnm@ti.com>
Download mbox | patch
Permalink /patch/216962/
State Changes Requested
Delegated to: David Miller
Headers show

Comments

Mugunthan V N - Jan. 30, 2013, 7:07 p.m.
adding support for VLAN interface for cpsw.

CPSW VLAN Capability
* Can filter VLAN packets in Hardware

Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
---
 Documentation/devicetree/bindings/net/cpsw.txt |    2 +
 drivers/net/ethernet/ti/cpsw.c                 |   92 +++++++++++++++++++++++-
 include/linux/platform_data/cpsw.h             |    1 +
 3 files changed, 93 insertions(+), 2 deletions(-)
fran├žois romieu - Jan. 30, 2013, 10:02 p.m.
Mugunthan V N <mugunthanvnm@ti.com> :
[...]
> diff --git a/Documentation/devicetree/bindings/net/cpsw.txt b/Documentation/devicetree/bindings/net/cpsw.txt
> index 6ddd028..99696bf 100644
> --- a/Documentation/devicetree/bindings/net/cpsw.txt
> +++ b/Documentation/devicetree/bindings/net/cpsw.txt
> @@ -24,6 +24,8 @@ Required properties:
>  Optional properties:
>  - ti,hwmods		: Must be "cpgmac0"
>  - no_bd_ram		: Must be 0 or 1
> +- default_vlan		: Specifies Default VLAN for non tagged packets
> +			  ALE processing

Isn't it a device-tree hack for what should belong to a common API ?

[...]
> diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
> index a40750e..6c66f01 100644
> --- a/drivers/net/ethernet/ti/cpsw.c
> +++ b/drivers/net/ethernet/ti/cpsw.c
[...]
> @@ -607,14 +611,41 @@ static void cpsw_slave_open(struct cpsw_slave *slave, struct cpsw_priv *priv)
>  	}
>  }
>  
> +static inline void cpsw_add_default_vlan(struct cpsw_priv *priv)
> +{
> +	writel(priv->data.default_vlan, &priv->host_port_regs->port_vlan);
> +	if (priv->version == CPSW_VERSION_1) {
> +		slave_write(&priv->slaves[0], priv->data.default_vlan,
> +			    CPSW1_PORT_VLAN);
> +		slave_write(&priv->slaves[1], priv->data.default_vlan,
> +			    CPSW1_PORT_VLAN);
> +	} else {
> +		slave_write(&priv->slaves[0], priv->data.default_vlan,
> +			    CPSW2_PORT_VLAN);
> +		slave_write(&priv->slaves[1], priv->data.default_vlan,
> +			    CPSW2_PORT_VLAN);
> +	}
> +	cpsw_ale_add_vlan(priv->ale, priv->data.default_vlan,
> +			ALE_ALL_PORTS << priv->host_port,
> +			ALE_ALL_PORTS << priv->host_port,
> +			ALE_ALL_PORTS << priv->host_port,
> +			(BIT(1) | BIT(2)) << priv->host_port);
> +}

static inline void cpsw_add_default_vlan(struct cpsw_priv *priv)
{
	const int vlan = priv->data.default_vlan;
	const int port = priv->host_port;
	u32 reg;
	int i;

	reg = (priv->version == CPSW_VERSION_1) ? CPSW1_PORT_VLAN :
	      CPSW2_PORT_VLAN;

	writel(vlan, &priv->host_port_regs->port_vlan);

	for (int i = 0; i < 2; i++)
		slave_write(priv->slaves + i, vlan, reg);

	cpsw_ale_add_vlan(priv->ale, vlan, ALE_ALL_PORTS << port,
			  ALE_ALL_PORTS << port, ALE_ALL_PORTS << port,
			  (BIT(1) | BIT(2)) << port);
}

... or somewhere between both. Your call.

[...]
> @@ -933,6 +967,55 @@ static void cpsw_ndo_poll_controller(struct net_device *ndev)
>  }
>  #endif
>  
> +static inline void cpsw_add_vlan_ale_entry(struct cpsw_priv *priv,
> +				unsigned short vid)
> +{
> +	cpsw_ale_add_vlan(priv->ale, vid, ALE_ALL_PORTS << priv->host_port,
> +			0, ALE_ALL_PORTS << priv->host_port,
> +			(BIT(1) | BIT(2)) << priv->host_port);

"(BIT(1) | BIT(2))" is repeated a couple of times.

[...]
> +static int cpsw_ndo_vlan_rx_add_vid(struct net_device *ndev,
> +		unsigned short vid)
> +{
> +	struct cpsw_priv *priv = netdev_priv(ndev);
> +
> +	if (vid == priv->data.default_vlan)
> +		return 0;
> +
> +	spin_lock(&priv->lock);
> +
> +	dev_info(priv->dev, "Adding vlanid %d to vlan filter\n", vid);
> +	cpsw_add_vlan_ale_entry(priv, vid);
> +
> +	spin_unlock(&priv->lock);
> +	return 0;
> +}
> +
> +static int cpsw_ndo_vlan_rx_kill_vid(struct net_device *ndev,
> +		unsigned short vid)
> +{
> +	struct cpsw_priv *priv = netdev_priv(ndev);
> +
> +	if (vid == priv->data.default_vlan)
> +		return 0;
> +
> +	spin_lock(&priv->lock);
> +
> +	dev_info(priv->dev, "removing vlanid %d from vlan filter\n", vid);
> +	cpsw_ale_del_vlan(priv->ale, vid, 0);
> +	cpsw_ale_del_ucast(priv->ale, priv->mac_addr,
> +			   priv->host_port, ALE_VLAN, vid);
> +	cpsw_ale_del_mcast(priv->ale, priv->ndev->broadcast, 0, ALE_VLAN, vid);
> +
> +	spin_unlock(&priv->lock);

What are you trying to achieve with the lock ?

It is not used anywhere else and both cpsw_ndo_vlan_rx_{add, kill}_vid are
called under RTNL.
Mugunthan V N - Jan. 31, 2013, 11:09 a.m.
On 1/31/2013 3:32 AM, Francois Romieu wrote:
> Mugunthan V N <mugunthanvnm@ti.com> :
> [...]
>> diff --git a/Documentation/devicetree/bindings/net/cpsw.txt b/Documentation/devicetree/bindings/net/cpsw.txt
>> index 6ddd028..99696bf 100644
>> --- a/Documentation/devicetree/bindings/net/cpsw.txt
>> +++ b/Documentation/devicetree/bindings/net/cpsw.txt
>> @@ -24,6 +24,8 @@ Required properties:
>>   Optional properties:
>>   - ti,hwmods		: Must be "cpgmac0"
>>   - no_bd_ram		: Must be 0 or 1
>> +- default_vlan		: Specifies Default VLAN for non tagged packets
>> +			  ALE processing
> Isn't it a device-tree hack for what should belong to a common API ?
Its a hardware feature which stack will not be aware of. It is used in 
the ALE filtering
process with a non-tagged packet arrives.
>
> [...]
>> diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
>> index a40750e..6c66f01 100644
>> --- a/drivers/net/ethernet/ti/cpsw.c
>> +++ b/drivers/net/ethernet/ti/cpsw.c
> [...]
>> @@ -607,14 +611,41 @@ static void cpsw_slave_open(struct cpsw_slave *slave, struct cpsw_priv *priv)
>>   	}
>>   }
>>   
>> +static inline void cpsw_add_default_vlan(struct cpsw_priv *priv)
>> +{
>> +	writel(priv->data.default_vlan, &priv->host_port_regs->port_vlan);
>> +	if (priv->version == CPSW_VERSION_1) {
>> +		slave_write(&priv->slaves[0], priv->data.default_vlan,
>> +			    CPSW1_PORT_VLAN);
>> +		slave_write(&priv->slaves[1], priv->data.default_vlan,
>> +			    CPSW1_PORT_VLAN);
>> +	} else {
>> +		slave_write(&priv->slaves[0], priv->data.default_vlan,
>> +			    CPSW2_PORT_VLAN);
>> +		slave_write(&priv->slaves[1], priv->data.default_vlan,
>> +			    CPSW2_PORT_VLAN);
>> +	}
>> +	cpsw_ale_add_vlan(priv->ale, priv->data.default_vlan,
>> +			ALE_ALL_PORTS << priv->host_port,
>> +			ALE_ALL_PORTS << priv->host_port,
>> +			ALE_ALL_PORTS << priv->host_port,
>> +			(BIT(1) | BIT(2)) << priv->host_port);
>> +}
> static inline void cpsw_add_default_vlan(struct cpsw_priv *priv)
> {
> 	const int vlan = priv->data.default_vlan;
> 	const int port = priv->host_port;
> 	u32 reg;
> 	int i;
>
> 	reg = (priv->version == CPSW_VERSION_1) ? CPSW1_PORT_VLAN :
> 	      CPSW2_PORT_VLAN;
>
> 	writel(vlan, &priv->host_port_regs->port_vlan);
>
> 	for (int i = 0; i < 2; i++)
> 		slave_write(priv->slaves + i, vlan, reg);
>
> 	cpsw_ale_add_vlan(priv->ale, vlan, ALE_ALL_PORTS << port,
> 			  ALE_ALL_PORTS << port, ALE_ALL_PORTS << port,
> 			  (BIT(1) | BIT(2)) << port);
> }
>
> ... or somewhere between both. Your call.
Will modify the code as this looks simpler
>
> [...]
>> @@ -933,6 +967,55 @@ static void cpsw_ndo_poll_controller(struct net_device *ndev)
>>   }
>>   #endif
>>   
>> +static inline void cpsw_add_vlan_ale_entry(struct cpsw_priv *priv,
>> +				unsigned short vid)
>> +{
>> +	cpsw_ale_add_vlan(priv->ale, vid, ALE_ALL_PORTS << priv->host_port,
>> +			0, ALE_ALL_PORTS << priv->host_port,
>> +			(BIT(1) | BIT(2)) << priv->host_port);
> "(BIT(1) | BIT(2))" is repeated a couple of times.
Will replace with port number defines.
>
> [...]
>> +static int cpsw_ndo_vlan_rx_add_vid(struct net_device *ndev,
>> +		unsigned short vid)
>> +{
>> +	struct cpsw_priv *priv = netdev_priv(ndev);
>> +
>> +	if (vid == priv->data.default_vlan)
>> +		return 0;
>> +
>> +	spin_lock(&priv->lock);
>> +
>> +	dev_info(priv->dev, "Adding vlanid %d to vlan filter\n", vid);
>> +	cpsw_add_vlan_ale_entry(priv, vid);
>> +
>> +	spin_unlock(&priv->lock);
>> +	return 0;
>> +}
>> +
>> +static int cpsw_ndo_vlan_rx_kill_vid(struct net_device *ndev,
>> +		unsigned short vid)
>> +{
>> +	struct cpsw_priv *priv = netdev_priv(ndev);
>> +
>> +	if (vid == priv->data.default_vlan)
>> +		return 0;
>> +
>> +	spin_lock(&priv->lock);
>> +
>> +	dev_info(priv->dev, "removing vlanid %d from vlan filter\n", vid);
>> +	cpsw_ale_del_vlan(priv->ale, vid, 0);
>> +	cpsw_ale_del_ucast(priv->ale, priv->mac_addr,
>> +			   priv->host_port, ALE_VLAN, vid);
>> +	cpsw_ale_del_mcast(priv->ale, priv->ndev->broadcast, 0, ALE_VLAN, vid);
>> +
>> +	spin_unlock(&priv->lock);
> What are you trying to achieve with the lock ?
>
> It is not used anywhere else and both cpsw_ndo_vlan_rx_{add, kill}_vid are
> called under RTNL.
Will remove the lock from both apis

Regards
Mugunthan V N
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/Documentation/devicetree/bindings/net/cpsw.txt b/Documentation/devicetree/bindings/net/cpsw.txt
index 6ddd028..99696bf 100644
--- a/Documentation/devicetree/bindings/net/cpsw.txt
+++ b/Documentation/devicetree/bindings/net/cpsw.txt
@@ -24,6 +24,8 @@  Required properties:
 Optional properties:
 - ti,hwmods		: Must be "cpgmac0"
 - no_bd_ram		: Must be 0 or 1
+- default_vlan		: Specifies Default VLAN for non tagged packets
+			  ALE processing
 
 Note: "ti,hwmods" field is used to fetch the base address and irq
 resources from TI, omap hwmod data base during device registration.
diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index a40750e..6c66f01 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -32,6 +32,7 @@ 
 #include <linux/of.h>
 #include <linux/of_net.h>
 #include <linux/of_device.h>
+#include <linux/if_vlan.h>
 
 #include <linux/platform_data/cpsw.h>
 
@@ -118,6 +119,9 @@  do {								\
 #define TX_PRIORITY_MAPPING	0x33221100
 #define CPDMA_TX_PRIORITY_MAP	0x76543210
 
+#define CPSW_VLAN_AWARE		BIT(1)
+#define CPSW_ALE_VLAN_AWARE	1
+
 #define cpsw_enable_irq(priv)	\
 	do {			\
 		u32 i;		\
@@ -607,14 +611,41 @@  static void cpsw_slave_open(struct cpsw_slave *slave, struct cpsw_priv *priv)
 	}
 }
 
+static inline void cpsw_add_default_vlan(struct cpsw_priv *priv)
+{
+	writel(priv->data.default_vlan, &priv->host_port_regs->port_vlan);
+	if (priv->version == CPSW_VERSION_1) {
+		slave_write(&priv->slaves[0], priv->data.default_vlan,
+			    CPSW1_PORT_VLAN);
+		slave_write(&priv->slaves[1], priv->data.default_vlan,
+			    CPSW1_PORT_VLAN);
+	} else {
+		slave_write(&priv->slaves[0], priv->data.default_vlan,
+			    CPSW2_PORT_VLAN);
+		slave_write(&priv->slaves[1], priv->data.default_vlan,
+			    CPSW2_PORT_VLAN);
+	}
+	cpsw_ale_add_vlan(priv->ale, priv->data.default_vlan,
+			ALE_ALL_PORTS << priv->host_port,
+			ALE_ALL_PORTS << priv->host_port,
+			ALE_ALL_PORTS << priv->host_port,
+			(BIT(1) | BIT(2)) << priv->host_port);
+}
+
 static void cpsw_init_host_port(struct cpsw_priv *priv)
 {
+	u32 control_reg;
+
 	/* soft reset the controller and initialize ale */
 	soft_reset("cpsw", &priv->regs->soft_reset);
 	cpsw_ale_start(priv->ale);
 
 	/* switch to vlan unaware mode */
-	cpsw_ale_control_set(priv->ale, 0, ALE_VLAN_AWARE, 0);
+	cpsw_ale_control_set(priv->ale, priv->host_port, ALE_VLAN_AWARE,
+			     CPSW_ALE_VLAN_AWARE);
+	control_reg = readl(&priv->regs->control);
+	control_reg |= CPSW_VLAN_AWARE;
+	writel(control_reg, &priv->regs->control);
 
 	/* setup host port priority mapping */
 	__raw_writel(CPDMA_TX_PRIORITY_MAP,
@@ -650,6 +681,9 @@  static int cpsw_ndo_open(struct net_device *ndev)
 	cpsw_init_host_port(priv);
 	for_each_slave(priv, cpsw_slave_open, priv);
 
+	/* Add default VLAN */
+	cpsw_add_default_vlan(priv);
+
 	/* setup tx dma to fixed prio and zero offset */
 	cpdma_control_set(priv->dma, CPDMA_TX_PRIO_FIXED, 1);
 	cpdma_control_set(priv->dma, CPDMA_RX_BUFFER_OFFSET, 0);
@@ -933,6 +967,55 @@  static void cpsw_ndo_poll_controller(struct net_device *ndev)
 }
 #endif
 
+static inline void cpsw_add_vlan_ale_entry(struct cpsw_priv *priv,
+				unsigned short vid)
+{
+	cpsw_ale_add_vlan(priv->ale, vid, ALE_ALL_PORTS << priv->host_port,
+			0, ALE_ALL_PORTS << priv->host_port,
+			(BIT(1) | BIT(2)) << priv->host_port);
+	cpsw_ale_add_ucast(priv->ale, priv->mac_addr,
+			priv->host_port, ALE_VLAN, vid);
+	cpsw_ale_add_mcast(priv->ale, priv->ndev->broadcast,
+			ALE_ALL_PORTS << priv->host_port, ALE_VLAN, vid, 0);
+}
+
+static int cpsw_ndo_vlan_rx_add_vid(struct net_device *ndev,
+		unsigned short vid)
+{
+	struct cpsw_priv *priv = netdev_priv(ndev);
+
+	if (vid == priv->data.default_vlan)
+		return 0;
+
+	spin_lock(&priv->lock);
+
+	dev_info(priv->dev, "Adding vlanid %d to vlan filter\n", vid);
+	cpsw_add_vlan_ale_entry(priv, vid);
+
+	spin_unlock(&priv->lock);
+	return 0;
+}
+
+static int cpsw_ndo_vlan_rx_kill_vid(struct net_device *ndev,
+		unsigned short vid)
+{
+	struct cpsw_priv *priv = netdev_priv(ndev);
+
+	if (vid == priv->data.default_vlan)
+		return 0;
+
+	spin_lock(&priv->lock);
+
+	dev_info(priv->dev, "removing vlanid %d from vlan filter\n", vid);
+	cpsw_ale_del_vlan(priv->ale, vid, 0);
+	cpsw_ale_del_ucast(priv->ale, priv->mac_addr,
+			   priv->host_port, ALE_VLAN, vid);
+	cpsw_ale_del_mcast(priv->ale, priv->ndev->broadcast, 0, ALE_VLAN, vid);
+
+	spin_unlock(&priv->lock);
+	return 0;
+}
+
 static const struct net_device_ops cpsw_netdev_ops = {
 	.ndo_open		= cpsw_ndo_open,
 	.ndo_stop		= cpsw_ndo_stop,
@@ -947,6 +1030,8 @@  static const struct net_device_ops cpsw_netdev_ops = {
 #ifdef CONFIG_NET_POLL_CONTROLLER
 	.ndo_poll_controller	= cpsw_ndo_poll_controller,
 #endif
+	.ndo_vlan_rx_add_vid	= cpsw_ndo_vlan_rx_add_vid,
+	.ndo_vlan_rx_kill_vid	= cpsw_ndo_vlan_rx_kill_vid,
 };
 
 static void cpsw_get_drvinfo(struct net_device *ndev,
@@ -1103,6 +1188,9 @@  static int cpsw_probe_dt(struct cpsw_platform_data *data,
 	}
 	data->mac_control = prop;
 
+	if (!of_property_read_u32(node, "default_vlan", &prop))
+		data->default_vlan = prop;
+
 	/*
 	 * Populate all the child nodes here...
 	 */
@@ -1356,7 +1444,7 @@  static int cpsw_probe(struct platform_device *pdev)
 		k++;
 	}
 
-	ndev->flags |= IFF_ALLMULTI;	/* see cpsw_ndo_change_rx_flags() */
+	ndev->features |= NETIF_F_HW_VLAN_FILTER;
 
 	ndev->netdev_ops = &cpsw_netdev_ops;
 	SET_ETHTOOL_OPS(ndev, &cpsw_ethtool_ops);
diff --git a/include/linux/platform_data/cpsw.h b/include/linux/platform_data/cpsw.h
index 24368a2..e962cfd 100644
--- a/include/linux/platform_data/cpsw.h
+++ b/include/linux/platform_data/cpsw.h
@@ -35,6 +35,7 @@  struct cpsw_platform_data {
 	u32	bd_ram_size;  /*buffer descriptor ram size */
 	u32	rx_descs;	/* Number of Rx Descriptios */
 	u32	mac_control;	/* Mac control register */
+	u16	default_vlan;	/* Def VLAN for ALE lookup in VLAN aware mode*/
 };
 
 #endif /* __CPSW_H__ */