Second batch of cpsw patches

Second batch of cpsw patches

[Sebastian Andrzej Siewior]

Hi,

this is my second batch fixing modules load/unload and the threaded
irq thingy which started this :)
I'm not sure about the slave & dual_mac thingy but I think I got it right.

Do you think it would split the net_device struct away from cpsw_priv so
you copy almost everything from one struct to the other in case of dual_mac?

Sebastian

[PATCH 1/4] net/ti: add MODULE_DEVICE_TABLE + MODULE_LICENSE

[Sebastian Andrzej Siewior]

If compiled as modules each one of these modules is missing something.
With this patch the modules are loaded on demand and don't taint the
kernel due to license issues.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/net/ethernet/ti/cpsw.c          |    1 +
 drivers/net/ethernet/ti/davinci_cpdma.c |    2 ++
 drivers/net/ethernet/ti/davinci_mdio.c  |    1 +
 3 files changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 81cec00..f91c8ab 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -1974,6 +1974,7 @@ static const struct of_device_id cpsw_of_mtable[] = {
 	{ .compatible = "ti,cpsw", },
 	{ /* sentinel */ },
 };
+MODULE_DEVICE_TABLE(of, cpsw_of_mtable);
 
 static struct platform_driver cpsw_driver = {
 	.driver = {
diff --git a/drivers/net/ethernet/ti/davinci_cpdma.c b/drivers/net/ethernet/ti/davinci_cpdma.c
index 6b0a89f..49dfd59 100644
--- a/drivers/net/ethernet/ti/davinci_cpdma.c
+++ b/drivers/net/ethernet/ti/davinci_cpdma.c
@@ -1040,3 +1040,5 @@ int cpdma_control_set(struct cpdma_ctlr *ctlr, int control, int value)
 	return ret;
 }
 EXPORT_SYMBOL_GPL(cpdma_control_set);
+
+MODULE_LICENSE("GPL");
diff --git a/drivers/net/ethernet/ti/davinci_mdio.c b/drivers/net/ethernet/ti/davinci_mdio.c
index d04a622..12aec17 100644
--- a/drivers/net/ethernet/ti/davinci_mdio.c
+++ b/drivers/net/ethernet/ti/davinci_mdio.c
@@ -485,6 +485,7 @@ static const struct of_device_id davinci_mdio_of_mtable[] = {
 	{ .compatible = "ti,davinci_mdio", },
 	{ /* sentinel */ },
 };
+MODULE_DEVICE_TABLE(of, davinci_mdio_of_mtable);
 
 static struct platform_driver davinci_mdio_driver = {
 	.driver = {
-- 
1.7.10.4

[PATCH 2/4] net/cpsw: make sure modules remove does not leak any ressources

[Sebastian Andrzej Siewior]

This driver does not clean up properly after leaving. Here is a list:
- Use unregister_netdev(). free_netdev() is good but not enough
- Use the above also on the other ndev in case of dual mac
- Free data.slave_data. The name of the strucre makes it look like
  it is platform_data but it is not. It is just a trick!
- Free all irqs. Again: freeing one irq is good start, but freeing all
  of them is better.

With this rmmod & modprobe of cpsw seems to work. The remaining issue
is:
|WARNING: at fs/sysfs/dir.c:536 sysfs_add_one+0x9c/0xd4()
|sysfs: cannot create duplicate filename '/devices/ocp.2/4a100000.ethernet/4a101000.mdio'
|WARNING: at lib/kobject.c:196 kobject_add_internal+0x1a4/0x1c8()

comming from of_platform_populate() and I am not sure that this belongs
here.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/net/ethernet/ti/cpsw.c |   23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index f91c8ab..1983f23 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -1636,7 +1636,7 @@ static int cpsw_probe_dual_emac(struct platform_device *pdev,
 
 static int cpsw_probe(struct platform_device *pdev)
 {
-	struct cpsw_platform_data	*data = pdev->dev.platform_data;
+	struct cpsw_platform_data	*data;
 	struct net_device		*ndev;
 	struct cpsw_priv		*priv;
 	struct cpdma_params		dma_params;
@@ -1849,7 +1849,7 @@ static int cpsw_probe(struct platform_device *pdev)
 				goto clean_ale_ret;
 			}
 			priv->irqs_table[k] = i;
-			priv->num_irqs = k;
+			priv->num_irqs = k + 1;
 		}
 		k++;
 	}
@@ -1887,7 +1887,8 @@ static int cpsw_probe(struct platform_device *pdev)
 	return 0;
 
 clean_irq_ret:
-	free_irq(ndev->irq, priv);
+	for (i = 0; i < priv->num_irqs; i++)
+		free_irq(priv->irqs_table[i], priv);
 clean_ale_ret:
 	cpsw_ale_destroy(priv->ale);
 clean_dma_ret:
@@ -1910,7 +1911,8 @@ static int cpsw_probe(struct platform_device *pdev)
 	pm_runtime_disable(&pdev->dev);
 	kfree(priv->slaves);
 clean_ndev_ret:
-	free_netdev(ndev);
+	kfree(priv->data.slave_data);
+	free_netdev(priv->ndev);
 	return ret;
 }
 
@@ -1918,12 +1920,17 @@ static int cpsw_remove(struct platform_device *pdev)
 {
 	struct net_device *ndev = platform_get_drvdata(pdev);
 	struct cpsw_priv *priv = netdev_priv(ndev);
+	int i;
 
-	pr_info("removing device");
 	platform_set_drvdata(pdev, NULL);
+	if (priv->data.dual_emac)
+		unregister_netdev(cpsw_get_slave_ndev(priv, 1));
+	unregister_netdev(ndev);
 
 	cpts_unregister(priv->cpts);
-	free_irq(ndev->irq, priv);
+	for (i = 0; i < priv->num_irqs; i++)
+		free_irq(priv->irqs_table[i], priv);
+
 	cpsw_ale_destroy(priv->ale);
 	cpdma_chan_destroy(priv->txch);
 	cpdma_chan_destroy(priv->rxch);
@@ -1937,8 +1944,10 @@ static int cpsw_remove(struct platform_device *pdev)
 	pm_runtime_disable(&pdev->dev);
 	clk_put(priv->clk);
 	kfree(priv->slaves);
+	kfree(priv->data.slave_data);
+	if (priv->data.dual_emac)
+		free_netdev(cpsw_get_slave_ndev(priv, 1));
 	free_netdev(ndev);
-
 	return 0;
 }
 
-- 
1.7.10.4

[PATCH 3/4] net/cpsw: optimize the for_each_slave_macro()

[Sebastian Andrzej Siewior]

 text    data     bss     dec     hex filename
15530      92       4   15626    3d0a cpsw.o.before
15478      92       4   15574    3cd6 cpsw.o.after

52 bytes smaller, 13 for each invocation.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/net/ethernet/ti/cpsw.c |    9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 1983f23..8b643d9 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -355,12 +355,15 @@ struct cpsw_priv {
 #define napi_to_priv(napi)	container_of(napi, struct cpsw_priv, napi)
 #define for_each_slave(priv, func, arg...)				\
 	do {								\
-		int idx;						\
+		struct cpsw_slave *slave;				\
+		int n;							\
 		if (priv->data.dual_emac)				\
 			(func)((priv)->slaves + priv->emac_port, ##arg);\
 		else							\
-			for (idx = 0; idx < (priv)->data.slaves; idx++)	\
-				(func)((priv)->slaves + idx, ##arg);	\
+			for (n = (priv)->data.slaves,			\
+					slave = (priv)->slaves;		\
+					n; n--)				\
+				(func)(slave++, ##arg);			\
 	} while (0)
 #define cpsw_get_slave_ndev(priv, __slave_no__)				\
 	(priv->slaves[__slave_no__].ndev)
-- 
1.7.10.4

[PATCH 4/4] net/cpsw: fix irq_disable() with threaded interrupts

[Sebastian Andrzej Siewior]

During high throughput it is likely that we receive both: an RX and TX
interrupt. The normal behaviour is that once we enter the ISR the
interrupts are disabled in the IRQ chip and so the ISR is invoked only
once and the interrupt line is disabled once. It will be re-enabled
after napi completes.
With threaded interrupts on the other hand the interrupt the interrupt
is disabled immediately and the ISR is marked for "later". By having TX
and RX interrupt marked pending we invoke them both and disable the
interrupt line twice. The napi callback is still executed once and so
after it completes we remain with interrupts disabled.

The initial patch simply removed the cpsw_{enable|disable}_irq() calls
and it worked well on my AM335X ES1.0 (beagle bone). On ES2.0 (beagle
bone black) it caused an never ending interrupt (even after the mask via
cpsw_intr_disable()) according to Mugunthan V N. Since I don't have the
ES2.0 and no idea what is going on this patch tracks the state of the
irq_disable() call and execute it only when not yet done.
The book keeping is done on the first struct since with dual_emac we can
have two of those and only one interrupt line.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/net/ethernet/ti/cpsw.c |   25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 8b643d9..2633be6 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -348,6 +348,7 @@ struct cpsw_priv {
 	/* snapshot of IRQ numbers */
 	u32 irqs_table[4];
 	u32 num_irqs;
+	bool irq_enabled;
 	struct cpts *cpts;
 	u32 emac_port;
 };
@@ -515,7 +516,10 @@ static irqreturn_t cpsw_interrupt(int irq, void *dev_id)
 		return IRQ_NONE;
 
 	cpsw_intr_disable(priv);
-	cpsw_disable_irq(priv);
+	if (priv->irq_enabled == true) {
+		cpsw_disable_irq(priv);
+		priv->irq_enabled = false;
+	}
 
 	if (netif_running(priv->ndev)) {
 		napi_schedule(&priv->napi);
@@ -544,10 +548,16 @@ static int cpsw_poll(struct napi_struct *napi, int budget)
 
 	num_rx = cpdma_chan_process(priv->rxch, budget);
 	if (num_rx < budget) {
+		struct cpsw_priv *prim_cpsw;
+
 		napi_complete(napi);
 		cpsw_intr_enable(priv);
 		cpdma_ctlr_eoi(priv->dma, CPDMA_EOI_RX);
-		cpsw_enable_irq(priv);
+		prim_cpsw = cpsw_get_slave_priv(priv, 0);
+		if (prim_cpsw->irq_enabled == false) {
+			cpsw_enable_irq(priv);
+			prim_cpsw->irq_enabled = true;
+		}
 	}
 
 	if (num_rx || num_tx)
@@ -886,6 +896,7 @@ static void cpsw_slave_stop(struct cpsw_slave *slave, struct cpsw_priv *priv)
 static int cpsw_ndo_open(struct net_device *ndev)
 {
 	struct cpsw_priv *priv = netdev_priv(ndev);
+	struct cpsw_priv *prim_cpsw;
 	int i, ret;
 	u32 reg;
 
@@ -953,6 +964,14 @@ static int cpsw_ndo_open(struct net_device *ndev)
 		cpsw_set_coalesce(ndev, &coal);
 	}
 
+	prim_cpsw = cpsw_get_slave_priv(priv, 0);
+	if (prim_cpsw->irq_enabled == false) {
+		if ((priv == prim_cpsw) || !netif_running(prim_cpsw->ndev)) {
+			prim_cpsw->irq_enabled = true;
+			cpsw_enable_irq(prim_cpsw);
+		}
+	}
+
 	cpdma_ctlr_start(priv->dma);
 	cpsw_intr_enable(priv);
 	napi_enable(&priv->napi);
@@ -1856,7 +1875,7 @@ static int cpsw_probe(struct platform_device *pdev)
 		}
 		k++;
 	}
-
+	priv->irq_enabled = true;
 	ndev->features |= NETIF_F_HW_VLAN_FILTER;
 
 	ndev->netdev_ops = &cpsw_netdev_ops;
-- 
1.7.10.4

Re: [PATCH 1/4] net/ti: add MODULE_DEVICE_TABLE + MODULE_LICENSE

[Mugunthan V N]

On 4/25/2013 12:18 AM, Sebastian Andrzej Siewior wrote:
> If compiled as modules each one of these modules is missing something.
> With this patch the modules are loaded on demand and don't taint the
> kernel due to license issues.
>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
>   drivers/net/ethernet/ti/cpsw.c          |    1 +
>   drivers/net/ethernet/ti/davinci_cpdma.c |    2 ++
>   drivers/net/ethernet/ti/davinci_mdio.c  |    1 +
>   3 files changed, 4 insertions(+)
>
> diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
> index 81cec00..f91c8ab 100644
> --- a/drivers/net/ethernet/ti/cpsw.c
> +++ b/drivers/net/ethernet/ti/cpsw.c
> @@ -1974,6 +1974,7 @@ static const struct of_device_id cpsw_of_mtable[] = {
>   	{ .compatible = "ti,cpsw", },
>   	{ /* sentinel */ },
>   };
> +MODULE_DEVICE_TABLE(of, cpsw_of_mtable);
>   
>   static struct platform_driver cpsw_driver = {
>   	.driver = {
> diff --git a/drivers/net/ethernet/ti/davinci_cpdma.c b/drivers/net/ethernet/ti/davinci_cpdma.c
> index 6b0a89f..49dfd59 100644
> --- a/drivers/net/ethernet/ti/davinci_cpdma.c
> +++ b/drivers/net/ethernet/ti/davinci_cpdma.c
> @@ -1040,3 +1040,5 @@ int cpdma_control_set(struct cpdma_ctlr *ctlr, int control, int value)
>   	return ret;
>   }
>   EXPORT_SYMBOL_GPL(cpdma_control_set);
> +
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/net/ethernet/ti/davinci_mdio.c b/drivers/net/ethernet/ti/davinci_mdio.c
> index d04a622..12aec17 100644
> --- a/drivers/net/ethernet/ti/davinci_mdio.c
> +++ b/drivers/net/ethernet/ti/davinci_mdio.c
> @@ -485,6 +485,7 @@ static const struct of_device_id davinci_mdio_of_mtable[] = {
>   	{ .compatible = "ti,davinci_mdio", },
>   	{ /* sentinel */ },
>   };
> +MODULE_DEVICE_TABLE(of, davinci_mdio_of_mtable);
>   
>   static struct platform_driver davinci_mdio_driver = {
>   	.driver = {

Acked-by: Mugunthan V N <mugunthanvnm@ti.com>

Regards
Mugunthan V N

Re: [PATCH 2/4] net/cpsw: make sure modules remove does not leak any ressources

[Mugunthan V N]

On 4/25/2013 12:18 AM, Sebastian Andrzej Siewior wrote:
> This driver does not clean up properly after leaving. Here is a list:
> - Use unregister_netdev(). free_netdev() is good but not enough
> - Use the above also on the other ndev in case of dual mac
> - Free data.slave_data. The name of the strucre makes it look like
>    it is platform_data but it is not. It is just a trick!
> - Free all irqs. Again: freeing one irq is good start, but freeing all
>    of them is better.
>
> With this rmmod & modprobe of cpsw seems to work. The remaining issue
> is:
> |WARNING: at fs/sysfs/dir.c:536 sysfs_add_one+0x9c/0xd4()
> |sysfs: cannot create duplicate filename '/devices/ocp.2/4a100000.ethernet/4a101000.mdio'
> |WARNING: at lib/kobject.c:196 kobject_add_internal+0x1a4/0x1c8()
>
> comming from of_platform_populate() and I am not sure that this belongs
> here.
>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
>   drivers/net/ethernet/ti/cpsw.c |   23 ++++++++++++++++-------
>   1 file changed, 16 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
> index f91c8ab..1983f23 100644
> --- a/drivers/net/ethernet/ti/cpsw.c
> +++ b/drivers/net/ethernet/ti/cpsw.c
> @@ -1636,7 +1636,7 @@ static int cpsw_probe_dual_emac(struct platform_device *pdev,
>   
>   static int cpsw_probe(struct platform_device *pdev)
>   {
> -	struct cpsw_platform_data	*data = pdev->dev.platform_data;
> +	struct cpsw_platform_data	*data;
>   	struct net_device		*ndev;
>   	struct cpsw_priv		*priv;
>   	struct cpdma_params		dma_params;
> @@ -1849,7 +1849,7 @@ static int cpsw_probe(struct platform_device *pdev)
>   				goto clean_ale_ret;
>   			}
>   			priv->irqs_table[k] = i;
> -			priv->num_irqs = k;
> +			priv->num_irqs = k + 1;
>   		}
>   		k++;
>   	}
> @@ -1887,7 +1887,8 @@ static int cpsw_probe(struct platform_device *pdev)
>   	return 0;
>   
>   clean_irq_ret:
> -	free_irq(ndev->irq, priv);
> +	for (i = 0; i < priv->num_irqs; i++)
> +		free_irq(priv->irqs_table[i], priv);
>   clean_ale_ret:
>   	cpsw_ale_destroy(priv->ale);
>   clean_dma_ret:
> @@ -1910,7 +1911,8 @@ static int cpsw_probe(struct platform_device *pdev)
>   	pm_runtime_disable(&pdev->dev);
>   	kfree(priv->slaves);
>   clean_ndev_ret:
> -	free_netdev(ndev);
> +	kfree(priv->data.slave_data);
> +	free_netdev(priv->ndev);
>   	return ret;
>   }
>   
> @@ -1918,12 +1920,17 @@ static int cpsw_remove(struct platform_device *pdev)
>   {
>   	struct net_device *ndev = platform_get_drvdata(pdev);
>   	struct cpsw_priv *priv = netdev_priv(ndev);
> +	int i;
>   
> -	pr_info("removing device");
>   	platform_set_drvdata(pdev, NULL);
> +	if (priv->data.dual_emac)
> +		unregister_netdev(cpsw_get_slave_ndev(priv, 1));
> +	unregister_netdev(ndev);
>   
>   	cpts_unregister(priv->cpts);
> -	free_irq(ndev->irq, priv);
> +	for (i = 0; i < priv->num_irqs; i++)
> +		free_irq(priv->irqs_table[i], priv);
> +
>   	cpsw_ale_destroy(priv->ale);
>   	cpdma_chan_destroy(priv->txch);
>   	cpdma_chan_destroy(priv->rxch);
> @@ -1937,8 +1944,10 @@ static int cpsw_remove(struct platform_device *pdev)
>   	pm_runtime_disable(&pdev->dev);
>   	clk_put(priv->clk);
>   	kfree(priv->slaves);
> +	kfree(priv->data.slave_data);
> +	if (priv->data.dual_emac)
> +		free_netdev(cpsw_get_slave_ndev(priv, 1));
>   	free_netdev(ndev);
> -
>   	return 0;
>   }
>   
Acked-by: Mugunthan V N <mugunthanvnm@ti.com>

Regards
Mugunthan V N

Re: [PATCH 3/4] net/cpsw: optimize the for_each_slave_macro()

[Mugunthan V N]

On 4/25/2013 12:18 AM, Sebastian Andrzej Siewior wrote:
>   text    data     bss     dec     hex filename
> 15530      92       4   15626    3d0a cpsw.o.before
> 15478      92       4   15574    3cd6 cpsw.o.after
>
> 52 bytes smaller, 13 for each invocation.
>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
>   drivers/net/ethernet/ti/cpsw.c |    9 ++++++---
>   1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
> index 1983f23..8b643d9 100644
> --- a/drivers/net/ethernet/ti/cpsw.c
> +++ b/drivers/net/ethernet/ti/cpsw.c
> @@ -355,12 +355,15 @@ struct cpsw_priv {
>   #define napi_to_priv(napi)	container_of(napi, struct cpsw_priv, napi)
>   #define for_each_slave(priv, func, arg...)				\
>   	do {								\
> -		int idx;						\
> +		struct cpsw_slave *slave;				\
> +		int n;							\
>   		if (priv->data.dual_emac)				\
>   			(func)((priv)->slaves + priv->emac_port, ##arg);\
>   		else							\
> -			for (idx = 0; idx < (priv)->data.slaves; idx++)	\
> -				(func)((priv)->slaves + idx, ##arg);	\
> +			for (n = (priv)->data.slaves,			\
> +					slave = (priv)->slaves;		\
> +					n; n--)				\
> +				(func)(slave++, ##arg);			\
>   	} while (0)
>   #define cpsw_get_slave_ndev(priv, __slave_no__)				\
>   	(priv->slaves[__slave_no__].ndev)

Acked-by: Mugunthan V N <mugunthanvnm@ti.com>

Regards
Mugunthan V N

Re: [PATCH 4/4] net/cpsw: fix irq_disable() with threaded interrupts

[Mugunthan V N]

On 4/25/2013 12:18 AM, Sebastian Andrzej Siewior wrote:
> During high throughput it is likely that we receive both: an RX and TX
> interrupt. The normal behaviour is that once we enter the ISR the
> interrupts are disabled in the IRQ chip and so the ISR is invoked only
> once and the interrupt line is disabled once. It will be re-enabled
> after napi completes.
> With threaded interrupts on the other hand the interrupt the interrupt
> is disabled immediately and the ISR is marked for "later". By having TX
> and RX interrupt marked pending we invoke them both and disable the
> interrupt line twice. The napi callback is still executed once and so
> after it completes we remain with interrupts disabled.
>
> The initial patch simply removed the cpsw_{enable|disable}_irq() calls
> and it worked well on my AM335X ES1.0 (beagle bone). On ES2.0 (beagle
> bone black) it caused an never ending interrupt (even after the mask via
> cpsw_intr_disable()) according to Mugunthan V N. Since I don't have the
> ES2.0 and no idea what is going on this patch tracks the state of the
> irq_disable() call and execute it only when not yet done.
> The book keeping is done on the first struct since with dual_emac we can
> have two of those and only one interrupt line.
>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
>   drivers/net/ethernet/ti/cpsw.c |   25 ++++++++++++++++++++++---
>   1 file changed, 22 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
> index 8b643d9..2633be6 100644
> --- a/drivers/net/ethernet/ti/cpsw.c
> +++ b/drivers/net/ethernet/ti/cpsw.c
> @@ -348,6 +348,7 @@ struct cpsw_priv {
>   	/* snapshot of IRQ numbers */
>   	u32 irqs_table[4];
>   	u32 num_irqs;
> +	bool irq_enabled;
>   	struct cpts *cpts;
>   	u32 emac_port;
>   };
> @@ -515,7 +516,10 @@ static irqreturn_t cpsw_interrupt(int irq, void *dev_id)
>   		return IRQ_NONE;
>   
>   	cpsw_intr_disable(priv);
> -	cpsw_disable_irq(priv);
> +	if (priv->irq_enabled == true) {
> +		cpsw_disable_irq(priv);
> +		priv->irq_enabled = false;
> +	}
>   
>   	if (netif_running(priv->ndev)) {
>   		napi_schedule(&priv->napi);
> @@ -544,10 +548,16 @@ static int cpsw_poll(struct napi_struct *napi, int budget)
>   
>   	num_rx = cpdma_chan_process(priv->rxch, budget);
>   	if (num_rx < budget) {
> +		struct cpsw_priv *prim_cpsw;
> +
>   		napi_complete(napi);
>   		cpsw_intr_enable(priv);
>   		cpdma_ctlr_eoi(priv->dma, CPDMA_EOI_RX);
> -		cpsw_enable_irq(priv);
> +		prim_cpsw = cpsw_get_slave_priv(priv, 0);
> +		if (prim_cpsw->irq_enabled == false) {
> +			cpsw_enable_irq(priv);
> +			prim_cpsw->irq_enabled = true;
> +		}
>   	}
>   
>   	if (num_rx || num_tx)
> @@ -886,6 +896,7 @@ static void cpsw_slave_stop(struct cpsw_slave *slave, struct cpsw_priv *priv)
>   static int cpsw_ndo_open(struct net_device *ndev)
>   {
>   	struct cpsw_priv *priv = netdev_priv(ndev);
> +	struct cpsw_priv *prim_cpsw;
>   	int i, ret;
>   	u32 reg;
>   
> @@ -953,6 +964,14 @@ static int cpsw_ndo_open(struct net_device *ndev)
>   		cpsw_set_coalesce(ndev, &coal);
>   	}
>   
> +	prim_cpsw = cpsw_get_slave_priv(priv, 0);
> +	if (prim_cpsw->irq_enabled == false) {
> +		if ((priv == prim_cpsw) || !netif_running(prim_cpsw->ndev)) {
> +			prim_cpsw->irq_enabled = true;
> +			cpsw_enable_irq(prim_cpsw);
> +		}
> +	}
> +
>   	cpdma_ctlr_start(priv->dma);
>   	cpsw_intr_enable(priv);
>   	napi_enable(&priv->napi);
> @@ -1856,7 +1875,7 @@ static int cpsw_probe(struct platform_device *pdev)
>   		}
>   		k++;
>   	}
> -
> +	priv->irq_enabled = true;
>   	ndev->features |= NETIF_F_HW_VLAN_FILTER;
>   
>   	ndev->netdev_ops = &cpsw_netdev_ops;

Acked-by: Mugunthan V N <mugunthanvnm@ti.com>

Regards
Mugunthan V N

Re: [PATCH 2/4] net/cpsw: make sure modules remove does not leak any ressources

[Sergei Shtylyov]

Hello.

On 24-04-2013 22:48, Sebastian Andrzej Siewior wrote:

> This driver does not clean up properly after leaving. Here is a list:
> - Use unregister_netdev(). free_netdev() is good but not enough
> - Use the above also on the other ndev in case of dual mac
> - Free data.slave_data. The name of the strucre makes it look like

    s/strucre/structure/

>    it is platform_data but it is not. It is just a trick!
> - Free all irqs. Again: freeing one irq is good start, but freeing all
>    of them is better.

> With this rmmod & modprobe of cpsw seems to work. The remaining issue
> is:
> |WARNING: at fs/sysfs/dir.c:536 sysfs_add_one+0x9c/0xd4()
> |sysfs: cannot create duplicate filename '/devices/ocp.2/4a100000.ethernet/4a101000.mdio'
> |WARNING: at lib/kobject.c:196 kobject_add_internal+0x1a4/0x1c8()

> comming from of_platform_populate() and I am not sure that this belongs
> here.

> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

WBR, Sergei