[PATCH net-next 0/6] gianfar: Device configuration fixes

[PATCH net-next 0/6] gianfar: Device configuration fixes

[Claudiu Manoil]

Hi David,

This patchset represents the first part of an effort
to solve some old device configuration issues in gianfar,
especially run-time reset and re-configuration problems.
I'm referring to "on-the-fly" configuration of registers
against HW specification, concurrency issues during device
reset / re-configuration operations, and implementing HW
advisories for these operations.

There's also a good deal of code cleanup and refactoring,
and some other (minor) fixes as well.

Thank you. 


Claudiu Manoil (6):
  gianfar: Cleanup/Fix gfar_probe and the hw init code
  gianfar: Replace sysfs stubs with module params (fix)
  gianfar: Remove useless HAS_PADDING device flag
  gianfar: Factor out enabling/disabling of hw interrupts
  gianfar: Add missing graceful reset steps and fixes
  gianfar: Remove clean_rx_ring race from gfar_ethtool

 drivers/net/ethernet/freescale/Makefile          |   2 +-
 drivers/net/ethernet/freescale/gianfar.c         | 503 +++++++++++------------
 drivers/net/ethernet/freescale/gianfar.h         |  41 +-
 drivers/net/ethernet/freescale/gianfar_ethtool.c |  64 +--
 drivers/net/ethernet/freescale/gianfar_param.c   | 123 ++++++
 drivers/net/ethernet/freescale/gianfar_sysfs.c   | 340 ---------------
 6 files changed, 405 insertions(+), 668 deletions(-)
 create mode 100644 drivers/net/ethernet/freescale/gianfar_param.c
 delete mode 100644 drivers/net/ethernet/freescale/gianfar_sysfs.c

-- 
1.7.11.7

[PATCH net-next 1/6] gianfar: Cleanup/Fix gfar_probe and the hw init code

[Claudiu Manoil]

Factor out gfar_hw_init() to contain all the controller hw
initialization steps for a better control of register writes,
and to significantly simplify the tangled code from gfar_probe().
This results in code size and stack usage reduction (besides
code readability).

Fix memory leak on device removal, by freeing the rx_/tx_queue
structures.

Replace custom bit swapping function with a library one (bitrev8).

Move allocation of rx_/tx_queue struct arrays before the group
structure init, because in order to assign Rx/Tx queues
to groups we need to have the queues first.  This also allows
earlier bail out of gfar_probe(), in case the memory allocation
fails.

The flow control checks for maccfg1 were removed from gfar_probe(),
since flow control is disabled at probe time (priv->rx_/tx_pause_en
are 0). Redundant initializations (by 0) also removed.

Signed-off-by: Claudiu Manoil <claudiu.manoil@freescale.com>
---
 drivers/net/ethernet/freescale/gianfar.c | 331 +++++++++++++++----------------
 drivers/net/ethernet/freescale/gianfar.h |  34 +++-
 2 files changed, 188 insertions(+), 177 deletions(-)

diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
index ad5a5aa..ab915b0 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -9,7 +9,7 @@
  * Maintainer: Kumar Gala
  * Modifier: Sandeep Gopalpet <sandeep.kumar@freescale.com>
  *
- * Copyright 2002-2009, 2011 Freescale Semiconductor, Inc.
+ * Copyright 2002-2009, 2011-2013 Freescale Semiconductor, Inc.
  * Copyright 2007 MontaVista Software, Inc.
  *
  * This program is free software; you can redistribute  it and/or modify it
@@ -511,7 +511,43 @@ void unlock_tx_qs(struct gfar_private *priv)
 		spin_unlock(&priv->tx_queue[i]->txlock);
 }
 
-static void free_tx_pointers(struct gfar_private *priv)
+static int gfar_alloc_tx_queues(struct gfar_private *priv)
+{
+	int i;
+
+	for (i = 0; i < priv->num_tx_queues; i++) {
+		priv->tx_queue[i] = kzalloc(sizeof(struct gfar_priv_tx_q),
+					    GFP_KERNEL);
+		if (!priv->tx_queue[i])
+			return -ENOMEM;
+
+		priv->tx_queue[i]->tx_skbuff = NULL;
+		priv->tx_queue[i]->qindex = i;
+		priv->tx_queue[i]->dev = priv->ndev;
+		spin_lock_init(&(priv->tx_queue[i]->txlock));
+	}
+	return 0;
+}
+
+static int gfar_alloc_rx_queues(struct gfar_private *priv)
+{
+	int i;
+
+	for (i = 0; i < priv->num_rx_queues; i++) {
+		priv->rx_queue[i] = kzalloc(sizeof(struct gfar_priv_rx_q),
+					    GFP_KERNEL);
+		if (!priv->rx_queue[i])
+			return -ENOMEM;
+
+		priv->rx_queue[i]->rx_skbuff = NULL;
+		priv->rx_queue[i]->qindex = i;
+		priv->rx_queue[i]->dev = priv->ndev;
+		spin_lock_init(&(priv->rx_queue[i]->rxlock));
+	}
+	return 0;
+}
+
+static void gfar_free_tx_queues(struct gfar_private *priv)
 {
 	int i;
 
@@ -519,7 +555,7 @@ static void free_tx_pointers(struct gfar_private *priv)
 		kfree(priv->tx_queue[i]);
 }
 
-static void free_rx_pointers(struct gfar_private *priv)
+static void gfar_free_rx_queues(struct gfar_private *priv)
 {
 	int i;
 
@@ -608,6 +644,30 @@ static int gfar_parse_group(struct device_node *np,
 		grp->rx_bit_map = 0xFF;
 		grp->tx_bit_map = 0xFF;
 	}
+
+	/* bit_map's MSB is q0 (from q0 to q7) but, for_each_set_bit parses
+	 * right to left, so we need to revert the 8 bits to get the q index
+	 */
+	grp->rx_bit_map = bitrev8(grp->rx_bit_map);
+	grp->tx_bit_map = bitrev8(grp->tx_bit_map);
+
+	/* Calculate RSTAT, TSTAT, RQUEUE and TQUEUE values,
+	 * also assign queues to groups
+	 */
+	for_each_set_bit(i, &grp->rx_bit_map, priv->num_rx_queues) {
+		grp->num_rx_queues++;
+		grp->rstat |= (RSTAT_CLEAR_RHALT >> i);
+		priv->rqueue |= ((RQUEUE_EN0 | RQUEUE_EX0) >> i);
+		priv->rx_queue[i]->grp = grp;
+	}
+
+	for_each_set_bit(i, &grp->tx_bit_map, priv->num_tx_queues) {
+		grp->num_tx_queues++;
+		grp->tstat |= (TSTAT_CLEAR_THALT >> i);
+		priv->tqueue |= (TQUEUE_EN0 >> i);
+		priv->tx_queue[i]->grp = grp;
+	}
+
 	priv->num_grps++;
 
 	return 0;
@@ -664,7 +724,14 @@ static int gfar_of_init(struct platform_device *ofdev, struct net_device **pdev)
 	priv->num_tx_queues = num_tx_qs;
 	netif_set_real_num_rx_queues(dev, num_rx_qs);
 	priv->num_rx_queues = num_rx_qs;
-	priv->num_grps = 0x0;
+
+	err = gfar_alloc_tx_queues(priv);
+	if (err)
+		goto tx_alloc_failed;
+
+	err = gfar_alloc_rx_queues(priv);
+	if (err)
+		goto rx_alloc_failed;
 
 	/* Init Rx queue filer rule set linked list */
 	INIT_LIST_HEAD(&priv->rx_list.list);
@@ -691,38 +758,6 @@ static int gfar_of_init(struct platform_device *ofdev, struct net_device **pdev)
 			goto err_grp_init;
 	}
 
-	for (i = 0; i < priv->num_tx_queues; i++)
-		priv->tx_queue[i] = NULL;
-	for (i = 0; i < priv->num_rx_queues; i++)
-		priv->rx_queue[i] = NULL;
-
-	for (i = 0; i < priv->num_tx_queues; i++) {
-		priv->tx_queue[i] = kzalloc(sizeof(struct gfar_priv_tx_q),
-					    GFP_KERNEL);
-		if (!priv->tx_queue[i]) {
-			err = -ENOMEM;
-			goto tx_alloc_failed;
-		}
-		priv->tx_queue[i]->tx_skbuff = NULL;
-		priv->tx_queue[i]->qindex = i;
-		priv->tx_queue[i]->dev = dev;
-		spin_lock_init(&(priv->tx_queue[i]->txlock));
-	}
-
-	for (i = 0; i < priv->num_rx_queues; i++) {
-		priv->rx_queue[i] = kzalloc(sizeof(struct gfar_priv_rx_q),
-					    GFP_KERNEL);
-		if (!priv->rx_queue[i]) {
-			err = -ENOMEM;
-			goto rx_alloc_failed;
-		}
-		priv->rx_queue[i]->rx_skbuff = NULL;
-		priv->rx_queue[i]->qindex = i;
-		priv->rx_queue[i]->dev = dev;
-		spin_lock_init(&(priv->rx_queue[i]->rxlock));
-	}
-
-
 	stash = of_get_property(np, "bd-stash", NULL);
 
 	if (stash) {
@@ -784,12 +819,12 @@ static int gfar_of_init(struct platform_device *ofdev, struct net_device **pdev)
 
 	return 0;
 
-rx_alloc_failed:
-	free_rx_pointers(priv);
-tx_alloc_failed:
-	free_tx_pointers(priv);
 err_grp_init:
 	unmap_group_regs(priv);
+rx_alloc_failed:
+	gfar_free_rx_queues(priv);
+tx_alloc_failed:
+	gfar_free_tx_queues(priv);
 	free_gfar_dev(priv);
 	return err;
 }
@@ -875,19 +910,6 @@ static int gfar_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
 	return phy_mii_ioctl(priv->phydev, rq, cmd);
 }
 
-static unsigned int reverse_bitmap(unsigned int bit_map, unsigned int max_qs)
-{
-	unsigned int new_bit_map = 0x0;
-	int mask = 0x1 << (max_qs - 1), i;
-
-	for (i = 0; i < max_qs; i++) {
-		if (bit_map & mask)
-			new_bit_map = new_bit_map + (1 << i);
-		mask = mask >> 0x1;
-	}
-	return new_bit_map;
-}
-
 static u32 cluster_entry_per_class(struct gfar_private *priv, u32 rqfar,
 				   u32 class)
 {
@@ -1005,19 +1027,88 @@ static void gfar_detect_errata(struct gfar_private *priv)
 			 priv->errata);
 }
 
+static void gfar_hw_init(struct gfar_private *priv)
+{
+	struct gfar __iomem *regs = priv->gfargrp[0].regs;
+	u32 tempval;
+
+	/* Reset MAC layer */
+	gfar_write(&regs->maccfg1, MACCFG1_SOFT_RESET);
+
+	/* We need to delay at least 3 TX clocks */
+	udelay(2);
+
+	/* the soft reset bit is not self-resetting, so we need to
+	 * clear it before resuming normal operation
+	 */
+	gfar_write(&regs->maccfg1, 0);
+
+	/* Initialize MACCFG2. */
+	tempval = MACCFG2_INIT_SETTINGS;
+	if (gfar_has_errata(priv, GFAR_ERRATA_74))
+		tempval |= MACCFG2_HUGEFRAME | MACCFG2_LENGTHCHECK;
+	gfar_write(&regs->maccfg2, tempval);
+
+	/* Initialize ECNTRL */
+	gfar_write(&regs->ecntrl, ECNTRL_INIT_SETTINGS);
+
+	/* Program the interrupt steering regs, only for MG devices */
+	if (priv->num_grps > 1)
+		gfar_write_isrg(priv);
+
+	/* Enable all Rx/Tx queues after MAC reset */
+	gfar_write(&regs->rqueue, priv->rqueue);
+	gfar_write(&regs->tqueue, priv->tqueue);
+}
+
+static void __init gfar_init_addr_hash_table(struct gfar_private *priv)
+{
+	struct gfar __iomem *regs = priv->gfargrp[0].regs;
+
+	if (priv->device_flags & FSL_GIANFAR_DEV_HAS_EXTENDED_HASH) {
+		priv->extended_hash = 1;
+		priv->hash_width = 9;
+
+		priv->hash_regs[0] = &regs->igaddr0;
+		priv->hash_regs[1] = &regs->igaddr1;
+		priv->hash_regs[2] = &regs->igaddr2;
+		priv->hash_regs[3] = &regs->igaddr3;
+		priv->hash_regs[4] = &regs->igaddr4;
+		priv->hash_regs[5] = &regs->igaddr5;
+		priv->hash_regs[6] = &regs->igaddr6;
+		priv->hash_regs[7] = &regs->igaddr7;
+		priv->hash_regs[8] = &regs->gaddr0;
+		priv->hash_regs[9] = &regs->gaddr1;
+		priv->hash_regs[10] = &regs->gaddr2;
+		priv->hash_regs[11] = &regs->gaddr3;
+		priv->hash_regs[12] = &regs->gaddr4;
+		priv->hash_regs[13] = &regs->gaddr5;
+		priv->hash_regs[14] = &regs->gaddr6;
+		priv->hash_regs[15] = &regs->gaddr7;
+
+	} else {
+		priv->extended_hash = 0;
+		priv->hash_width = 8;
+
+		priv->hash_regs[0] = &regs->gaddr0;
+		priv->hash_regs[1] = &regs->gaddr1;
+		priv->hash_regs[2] = &regs->gaddr2;
+		priv->hash_regs[3] = &regs->gaddr3;
+		priv->hash_regs[4] = &regs->gaddr4;
+		priv->hash_regs[5] = &regs->gaddr5;
+		priv->hash_regs[6] = &regs->gaddr6;
+		priv->hash_regs[7] = &regs->gaddr7;
+	}
+}
+
 /* Set up the ethernet device structure, private data,
  * and anything else we need before we start
  */
 static int gfar_probe(struct platform_device *ofdev)
 {
-	u32 tempval;
 	struct net_device *dev = NULL;
 	struct gfar_private *priv = NULL;
-	struct gfar __iomem *regs = NULL;
-	int err = 0, i, grp_idx = 0;
-	u32 rstat = 0, tstat = 0, rqueue = 0, tqueue = 0;
-	u32 isrg = 0;
-	u32 __iomem *baddr;
+	int err = 0, i;
 
 	err = gfar_of_init(ofdev, &dev);
 
@@ -1034,7 +1125,6 @@ static int gfar_probe(struct platform_device *ofdev)
 	INIT_WORK(&priv->reset_task, gfar_reset_task);
 
 	platform_set_drvdata(ofdev, priv);
-	regs = priv->gfargrp[0].regs;
 
 	gfar_detect_errata(priv);
 
@@ -1043,33 +1133,10 @@ static int gfar_probe(struct platform_device *ofdev)
 	 */
 	gfar_halt(dev);
 
-	/* Reset MAC layer */
-	gfar_write(&regs->maccfg1, MACCFG1_SOFT_RESET);
-
-	/* We need to delay at least 3 TX clocks */
-	udelay(2);
-
-	tempval = 0;
-	if (!priv->pause_aneg_en && priv->tx_pause_en)
-		tempval |= MACCFG1_TX_FLOW;
-	if (!priv->pause_aneg_en && priv->rx_pause_en)
-		tempval |= MACCFG1_RX_FLOW;
-	/* the soft reset bit is not self-resetting, so we need to
-	 * clear it before resuming normal operation
-	 */
-	gfar_write(&regs->maccfg1, tempval);
-
-	/* Initialize MACCFG2. */
-	tempval = MACCFG2_INIT_SETTINGS;
-	if (gfar_has_errata(priv, GFAR_ERRATA_74))
-		tempval |= MACCFG2_HUGEFRAME | MACCFG2_LENGTHCHECK;
-	gfar_write(&regs->maccfg2, tempval);
-
-	/* Initialize ECNTRL */
-	gfar_write(&regs->ecntrl, ECNTRL_INIT_SETTINGS);
+	gfar_hw_init(priv);
 
 	/* Set the dev->base_addr to the gfar reg region */
-	dev->base_addr = (unsigned long) regs;
+	dev->base_addr = (unsigned long) priv->gfargrp[0].regs;
 
 	/* Fill in the dev structure */
 	dev->watchdog_timeo = TX_TIMEOUT;
@@ -1099,40 +1166,7 @@ static int gfar_probe(struct platform_device *ofdev)
 		dev->features |= NETIF_F_HW_VLAN_CTAG_RX;
 	}
 
-	if (priv->device_flags & FSL_GIANFAR_DEV_HAS_EXTENDED_HASH) {
-		priv->extended_hash = 1;
-		priv->hash_width = 9;
-
-		priv->hash_regs[0] = &regs->igaddr0;
-		priv->hash_regs[1] = &regs->igaddr1;
-		priv->hash_regs[2] = &regs->igaddr2;
-		priv->hash_regs[3] = &regs->igaddr3;
-		priv->hash_regs[4] = &regs->igaddr4;
-		priv->hash_regs[5] = &regs->igaddr5;
-		priv->hash_regs[6] = &regs->igaddr6;
-		priv->hash_regs[7] = &regs->igaddr7;
-		priv->hash_regs[8] = &regs->gaddr0;
-		priv->hash_regs[9] = &regs->gaddr1;
-		priv->hash_regs[10] = &regs->gaddr2;
-		priv->hash_regs[11] = &regs->gaddr3;
-		priv->hash_regs[12] = &regs->gaddr4;
-		priv->hash_regs[13] = &regs->gaddr5;
-		priv->hash_regs[14] = &regs->gaddr6;
-		priv->hash_regs[15] = &regs->gaddr7;
-
-	} else {
-		priv->extended_hash = 0;
-		priv->hash_width = 8;
-
-		priv->hash_regs[0] = &regs->gaddr0;
-		priv->hash_regs[1] = &regs->gaddr1;
-		priv->hash_regs[2] = &regs->gaddr2;
-		priv->hash_regs[3] = &regs->gaddr3;
-		priv->hash_regs[4] = &regs->gaddr4;
-		priv->hash_regs[5] = &regs->gaddr5;
-		priv->hash_regs[6] = &regs->gaddr6;
-		priv->hash_regs[7] = &regs->gaddr7;
-	}
+	gfar_init_addr_hash_table(priv);
 
 	if (priv->device_flags & FSL_GIANFAR_DEV_HAS_PADDING)
 		priv->padding = DEFAULT_PADDING;
@@ -1143,59 +1177,6 @@ static int gfar_probe(struct platform_device *ofdev)
 	    priv->device_flags & FSL_GIANFAR_DEV_HAS_TIMER)
 		dev->needed_headroom = GMAC_FCB_LEN;
 
-	/* Program the isrg regs only if number of grps > 1 */
-	if (priv->num_grps > 1) {
-		baddr = &regs->isrg0;
-		for (i = 0; i < priv->num_grps; i++) {
-			isrg |= (priv->gfargrp[i].rx_bit_map << ISRG_SHIFT_RX);
-			isrg |= (priv->gfargrp[i].tx_bit_map << ISRG_SHIFT_TX);
-			gfar_write(baddr, isrg);
-			baddr++;
-			isrg = 0x0;
-		}
-	}
-
-	/* Need to reverse the bit maps as  bit_map's MSB is q0
-	 * but, for_each_set_bit parses from right to left, which
-	 * basically reverses the queue numbers
-	 */
-	for (i = 0; i< priv->num_grps; i++) {
-		priv->gfargrp[i].tx_bit_map =
-			reverse_bitmap(priv->gfargrp[i].tx_bit_map, MAX_TX_QS);
-		priv->gfargrp[i].rx_bit_map =
-			reverse_bitmap(priv->gfargrp[i].rx_bit_map, MAX_RX_QS);
-	}
-
-	/* Calculate RSTAT, TSTAT, RQUEUE and TQUEUE values,
-	 * also assign queues to groups
-	 */
-	for (grp_idx = 0; grp_idx < priv->num_grps; grp_idx++) {
-		priv->gfargrp[grp_idx].num_rx_queues = 0x0;
-
-		for_each_set_bit(i, &priv->gfargrp[grp_idx].rx_bit_map,
-				 priv->num_rx_queues) {
-			priv->gfargrp[grp_idx].num_rx_queues++;
-			priv->rx_queue[i]->grp = &priv->gfargrp[grp_idx];
-			rstat = rstat | (RSTAT_CLEAR_RHALT >> i);
-			rqueue = rqueue | ((RQUEUE_EN0 | RQUEUE_EX0) >> i);
-		}
-		priv->gfargrp[grp_idx].num_tx_queues = 0x0;
-
-		for_each_set_bit(i, &priv->gfargrp[grp_idx].tx_bit_map,
-				 priv->num_tx_queues) {
-			priv->gfargrp[grp_idx].num_tx_queues++;
-			priv->tx_queue[i]->grp = &priv->gfargrp[grp_idx];
-			tstat = tstat | (TSTAT_CLEAR_THALT >> i);
-			tqueue = tqueue | (TQUEUE_EN0 >> i);
-		}
-		priv->gfargrp[grp_idx].rstat = rstat;
-		priv->gfargrp[grp_idx].tstat = tstat;
-		rstat = tstat =0;
-	}
-
-	gfar_write(&regs->rqueue, rqueue);
-	gfar_write(&regs->tqueue, tqueue);
-
 	priv->rx_buffer_size = DEFAULT_RX_BUFFER_SIZE;
 
 	/* Initializing some of the rx/tx queue level parameters */
@@ -1272,8 +1253,8 @@ static int gfar_probe(struct platform_device *ofdev)
 
 register_fail:
 	unmap_group_regs(priv);
-	free_tx_pointers(priv);
-	free_rx_pointers(priv);
+	gfar_free_rx_queues(priv);
+	gfar_free_tx_queues(priv);
 	if (priv->phy_node)
 		of_node_put(priv->phy_node);
 	if (priv->tbi_node)
@@ -1293,6 +1274,8 @@ static int gfar_remove(struct platform_device *ofdev)
 
 	unregister_netdev(priv->ndev);
 	unmap_group_regs(priv);
+	gfar_free_rx_queues(priv);
+	gfar_free_tx_queues(priv);
 	free_gfar_dev(priv);
 
 	return 0;
diff --git a/drivers/net/ethernet/freescale/gianfar.h b/drivers/net/ethernet/freescale/gianfar.h
index 52bb2b0..63c830c 100644
--- a/drivers/net/ethernet/freescale/gianfar.h
+++ b/drivers/net/ethernet/freescale/gianfar.h
@@ -9,7 +9,7 @@
  * Maintainer: Kumar Gala
  * Modifier: Sandeep Gopalpet <sandeep.kumar@freescale.com>
  *
- * Copyright 2002-2009, 2011 Freescale Semiconductor, Inc.
+ * Copyright 2002-2009, 2011-2013 Freescale Semiconductor, Inc.
  *
  * This program is free software; you can redistribute  it and/or modify it
  * under  the terms of  the GNU General  Public License as published by the
@@ -892,8 +892,8 @@ struct gfar {
 #define DEFAULT_MAPPING 	0xFF
 #endif
 
-#define ISRG_SHIFT_TX	0x10
-#define ISRG_SHIFT_RX	0x18
+#define ISRG_RR0	0x80000000
+#define ISRG_TR0	0x00800000
 
 /* The same driver can operate in two modes */
 /* SQ_SG_MODE: Single Queue Single Group Mode
@@ -1113,6 +1113,9 @@ struct gfar_private {
 	unsigned int total_tx_ring_size;
 	unsigned int total_rx_ring_size;
 
+	u32 rqueue;
+	u32 tqueue;
+
 	/* RX per device parameters */
 	unsigned int rx_stash_size;
 	unsigned int rx_stash_index;
@@ -1176,6 +1179,31 @@ static inline void gfar_read_filer(struct gfar_private *priv,
 	*fpr = gfar_read(&regs->rqfpr);
 }
 
+static inline void gfar_write_isrg(struct gfar_private *priv)
+{
+	struct gfar __iomem *regs = priv->gfargrp[0].regs;
+	u32 __iomem *baddr = &regs->isrg0;
+	u32 isrg = 0;
+	int grp_idx, i;
+
+	for (grp_idx = 0; grp_idx < priv->num_grps; grp_idx++) {
+		struct gfar_priv_grp *grp = &priv->gfargrp[grp_idx];
+
+		for_each_set_bit(i, &grp->rx_bit_map, priv->num_rx_queues) {
+			isrg |= (ISRG_RR0 >> i);
+		}
+
+		for_each_set_bit(i, &grp->tx_bit_map, priv->num_tx_queues) {
+			isrg |= (ISRG_TR0 >> i);
+		}
+
+		gfar_write(baddr, isrg);
+
+		baddr++;
+		isrg = 0;
+	}
+}
+
 void lock_rx_qs(struct gfar_private *priv);
 void lock_tx_qs(struct gfar_private *priv);
 void unlock_rx_qs(struct gfar_private *priv);
-- 
1.7.11.7

[PATCH net-next 2/6] gianfar: Replace sysfs stubs with module params (fix)

[Claudiu Manoil]

Removing the sysfs stubs for the Tx FIFOCFG and ATTRELI
(stashing) config registers, as these registers may only
be configured after a MAC reset, with the controller stopped
(i.e. during hw init, at probe() time). The current sysfs
stubs allow on-the-fly updates of these registers (the locking
measures are useless and only add unecessary code).

Changing these registers on-the-fly is strogly discouraged.
In this regard, this patch may be seen as a security fix.

To address this issue and not lose entirely these config
params, they are now accessible as driver module parameters,
and their names and default values have been preserved.

Moreover, the stasing configuration options were effectively
disabled (didn't get to the hw anyway if changed) because
the stashing device_flags (HAS_BD_STASHING|HAS_BUF_STASHING)
were "accidentally" cleared during probe(). The patch fixes
this bug as well.

Signed-off-by: Claudiu Manoil <claudiu.manoil@freescale.com>
---
 drivers/net/ethernet/freescale/Makefile        |   2 +-
 drivers/net/ethernet/freescale/gianfar.c       |  60 ++---
 drivers/net/ethernet/freescale/gianfar.h       |   3 +-
 drivers/net/ethernet/freescale/gianfar_param.c | 123 +++++++++
 drivers/net/ethernet/freescale/gianfar_sysfs.c | 340 -------------------------
 5 files changed, 155 insertions(+), 373 deletions(-)
 create mode 100644 drivers/net/ethernet/freescale/gianfar_param.c
 delete mode 100644 drivers/net/ethernet/freescale/gianfar_sysfs.c

diff --git a/drivers/net/ethernet/freescale/Makefile b/drivers/net/ethernet/freescale/Makefile
index 549ce13..e324b7d 100644
--- a/drivers/net/ethernet/freescale/Makefile
+++ b/drivers/net/ethernet/freescale/Makefile
@@ -15,6 +15,6 @@ obj-$(CONFIG_GIANFAR) += gianfar_driver.o
 obj-$(CONFIG_PTP_1588_CLOCK_GIANFAR) += gianfar_ptp.o
 gianfar_driver-objs := gianfar.o \
 		gianfar_ethtool.o \
-		gianfar_sysfs.o
+		gianfar_param.o
 obj-$(CONFIG_UCC_GETH) += ucc_geth_driver.o
 ucc_geth_driver-objs := ucc_geth.o ucc_geth_ethtool.o
diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
index ab915b0..27cc8b2 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -338,7 +338,6 @@ static void gfar_init_mac(struct net_device *ndev)
 	struct gfar __iomem *regs = priv->gfargrp[0].regs;
 	u32 rctrl = 0;
 	u32 tctrl = 0;
-	u32 attrs = 0;
 
 	/* write the tx/rx base registers */
 	gfar_init_tx_rx_base(priv);
@@ -409,29 +408,6 @@ static void gfar_init_mac(struct net_device *ndev)
 	}
 
 	gfar_write(&regs->tctrl, tctrl);
-
-	/* Set the extraction length and index */
-	attrs = ATTRELI_EL(priv->rx_stash_size) |
-		ATTRELI_EI(priv->rx_stash_index);
-
-	gfar_write(&regs->attreli, attrs);
-
-	/* Start with defaults, and add stashing or locking
-	 * depending on the approprate variables
-	 */
-	attrs = ATTR_INIT_SETTINGS;
-
-	if (priv->bd_stash_en)
-		attrs |= ATTR_BDSTASH;
-
-	if (priv->rx_stash_size != 0)
-		attrs |= ATTR_BUFSTASH;
-
-	gfar_write(&regs->attr, attrs);
-
-	gfar_write(&regs->fifo_tx_thr, priv->fifo_threshold);
-	gfar_write(&regs->fifo_tx_starve, priv->fifo_starve);
-	gfar_write(&regs->fifo_tx_starve_shutoff, priv->fifo_starve_off);
 }
 
 static struct net_device_stats *gfar_get_stats(struct net_device *dev)
@@ -784,13 +760,13 @@ static int gfar_of_init(struct platform_device *ofdev, struct net_device **pdev)
 		memcpy(dev->dev_addr, mac_addr, ETH_ALEN);
 
 	if (model && !strcasecmp(model, "TSEC"))
-		priv->device_flags = FSL_GIANFAR_DEV_HAS_GIGABIT |
+		priv->device_flags |= FSL_GIANFAR_DEV_HAS_GIGABIT |
 				     FSL_GIANFAR_DEV_HAS_COALESCE |
 				     FSL_GIANFAR_DEV_HAS_RMON |
 				     FSL_GIANFAR_DEV_HAS_MULTI_INTR;
 
 	if (model && !strcasecmp(model, "eTSEC"))
-		priv->device_flags = FSL_GIANFAR_DEV_HAS_GIGABIT |
+		priv->device_flags |= FSL_GIANFAR_DEV_HAS_GIGABIT |
 				     FSL_GIANFAR_DEV_HAS_COALESCE |
 				     FSL_GIANFAR_DEV_HAS_RMON |
 				     FSL_GIANFAR_DEV_HAS_MULTI_INTR |
@@ -1030,7 +1006,7 @@ static void gfar_detect_errata(struct gfar_private *priv)
 static void gfar_hw_init(struct gfar_private *priv)
 {
 	struct gfar __iomem *regs = priv->gfargrp[0].regs;
-	u32 tempval;
+	u32 tempval, attrs;
 
 	/* Reset MAC layer */
 	gfar_write(&regs->maccfg1, MACCFG1_SOFT_RESET);
@@ -1052,6 +1028,30 @@ static void gfar_hw_init(struct gfar_private *priv)
 	/* Initialize ECNTRL */
 	gfar_write(&regs->ecntrl, ECNTRL_INIT_SETTINGS);
 
+	/* Set the extraction length and index */
+	attrs = ATTRELI_EL(priv->rx_stash_size) |
+		ATTRELI_EI(priv->rx_stash_index);
+
+	gfar_write(&regs->attreli, attrs);
+
+	/* Start with defaults, and add stashing
+	 * depending on driver parameters
+	 */
+	attrs = ATTR_INIT_SETTINGS;
+
+	if (priv->bd_stash_en)
+		attrs |= ATTR_BDSTASH;
+
+	if (priv->rx_stash_size != 0)
+		attrs |= ATTR_BUFSTASH;
+
+	gfar_write(&regs->attr, attrs);
+
+	/* FIFO configs */
+	gfar_write(&regs->fifo_tx_thr, priv->fifo_threshold);
+	gfar_write(&regs->fifo_tx_starve, priv->fifo_starve);
+	gfar_write(&regs->fifo_tx_starve_shutoff, priv->fifo_starve_off);
+
 	/* Program the interrupt steering regs, only for MG devices */
 	if (priv->num_grps > 1)
 		gfar_write_isrg(priv);
@@ -1108,6 +1108,7 @@ static int gfar_probe(struct platform_device *ofdev)
 {
 	struct net_device *dev = NULL;
 	struct gfar_private *priv = NULL;
+	static int dev_idx;
 	int err = 0, i;
 
 	err = gfar_of_init(ofdev, &dev);
@@ -1128,6 +1129,8 @@ static int gfar_probe(struct platform_device *ofdev)
 
 	gfar_detect_errata(priv);
 
+	gfar_get_params(dev, dev_idx++);
+
 	/* Stop the DMA engine now, in case it was running before
 	 * (The firmware could have used it, and left it running).
 	 */
@@ -1232,9 +1235,6 @@ static int gfar_probe(struct platform_device *ofdev)
 	/* Initialize the filer table */
 	gfar_init_filer_table(priv);
 
-	/* Create all the sysfs files */
-	gfar_init_sysfs(dev);
-
 	/* Print out the device info */
 	netdev_info(dev, "mac: %pM\n", dev->dev_addr);
 
diff --git a/drivers/net/ethernet/freescale/gianfar.h b/drivers/net/ethernet/freescale/gianfar.h
index 63c830c..20121ef 100644
--- a/drivers/net/ethernet/freescale/gianfar.h
+++ b/drivers/net/ethernet/freescale/gianfar.h
@@ -17,7 +17,6 @@
  * option) any later version.
  *
  *  Still left to do:
- *      -Add support for module parameters
  *	-Add patch for ethtool phys id
  */
 #ifndef __GIANFAR_H
@@ -1215,7 +1214,7 @@ void gfar_halt(struct net_device *dev);
 void gfar_phy_test(struct mii_bus *bus, struct phy_device *phydev, int enable,
 		   u32 regnum, u32 read);
 void gfar_configure_coalescing_all(struct gfar_private *priv);
-void gfar_init_sysfs(struct net_device *dev);
+void gfar_get_params(struct net_device *dev, int i);
 int gfar_set_features(struct net_device *dev, netdev_features_t features);
 void gfar_check_rx_parser_mode(struct gfar_private *priv);
 void gfar_vlan_mode(struct net_device *dev, netdev_features_t features);
diff --git a/drivers/net/ethernet/freescale/gianfar_param.c b/drivers/net/ethernet/freescale/gianfar_param.c
new file mode 100644
index 0000000..7ed6745
--- /dev/null
+++ b/drivers/net/ethernet/freescale/gianfar_param.c
@@ -0,0 +1,123 @@
+/* Copyright 2002-2009, 2013 Freescale Semiconductor, Inc.
+ *
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ *
+ * Derived from:
+ * drivers/net/ethernet/freescale/gianfar_sysfs.c
+ */
+
+#include "gianfar.h"
+
+#define GFAR_DEV_MAXCNT 8
+
+#define PARAM_UNSET	-1
+
+#define GFAR_PARAM_INIT { [0 ... GFAR_DEV_MAXCNT - 1] = PARAM_UNSET }
+#define GFAR_PARAM(X, desc) \
+	static int X[GFAR_DEV_MAXCNT] = GFAR_PARAM_INIT; \
+	module_param_array_named(X, X, int, NULL, S_IRUGO); \
+	MODULE_PARM_DESC(X, desc);
+
+
+GFAR_PARAM(fifo_threshold, "Tx FIFO threshold to trigger frame unload");
+GFAR_PARAM(fifo_starve, "Tx FIFO thershold to enter starve mode");
+GFAR_PARAM(fifo_starve_off, "Tx FIFO thershold to exit starve mode");
+GFAR_PARAM(bd_stash, "Stash Rx buffer descriptors to L2");
+GFAR_PARAM(rx_stash_size, "Stash this many Rx frame bytes to L2");
+GFAR_PARAM(rx_stash_index, "Stash Rx frame starting from this offset");
+
+
+/* FIFO Tx Threshold - the numerical SRAM entry (0-511 for 2-Kbyte FIFO)
+ * to trigger the unloading of FIFO data to the PHY
+ */
+static void gfar_fifo_threshold_update(struct gfar_private *priv, int thr)
+{
+	if (thr >= 0 && thr <= GFAR_MAX_FIFO_THRESHOLD)
+		priv->fifo_threshold = thr;
+}
+
+/* FIFO Tx Starve/Shutoff - numerical SRAM entry (0-511 for 2-Kbyte FIFO).
+ * If the # of valid entries in the FIFO is less than or equal to the
+ * starve register value the controller enters starve state
+ * (i.e. triggers diagnostic interrupt to signal that Tx underrun is
+ * imminent and increases priority of Tx memory transactions).
+ * If the starve state is in effect and the number of valid entries in the
+ * FIFO becomes greater than or equal to the value in the FIFO transmit starve
+ * shutoff register, the starve condition ends.
+ */
+static void gfar_fifo_starve_update(struct gfar_private *priv,
+				    int thr_strv, int thr_strv_off)
+{
+	if (thr_strv >= 0 && thr_strv <= GFAR_MAX_FIFO_STARVE &&
+	    thr_strv_off >= 0 && thr_strv_off <= GFAR_MAX_FIFO_STARVE_OFF &&
+	    thr_strv < thr_strv_off) {
+		priv->fifo_starve = thr_strv;
+		priv->fifo_starve_off = thr_strv_off;
+	}
+}
+
+static void gfar_bd_stash_update(struct gfar_private *priv, int en)
+{
+	if (!(priv->device_flags & FSL_GIANFAR_DEV_HAS_BD_STASHING))
+		return;
+
+	if (en == 0 || en == 1)
+		priv->bd_stash_en = en;
+}
+
+static void gfar_rx_stash_update(struct gfar_private *priv, int size, int idx)
+{
+	if (!(priv->device_flags & FSL_GIANFAR_DEV_HAS_BUF_STASHING))
+		return;
+
+	/* valid sizes are multiples of 8 */
+	if (size <= 0 || size > DEFAULT_RX_BUFFER_SIZE || size & 0x7)
+		return;
+
+	/* index must be multiple of 64 */
+	if (idx < 0 || idx > size || idx & 0x3f)
+		return;
+
+	priv->rx_stash_index = idx;
+	priv->rx_stash_size = size;
+}
+
+void __init gfar_get_params(struct net_device *ndev, int i)
+{
+	struct gfar_private *priv = netdev_priv(ndev);
+
+	/* Initialize the default values */
+	priv->fifo_threshold = DEFAULT_FIFO_TX_THR;
+	priv->fifo_starve = DEFAULT_FIFO_TX_STARVE;
+	priv->fifo_starve_off = DEFAULT_FIFO_TX_STARVE_OFF;
+
+	if (i >= GFAR_DEV_MAXCNT) {
+		netdev_warn(ndev, "dev config missing, using defaults\n");
+		return;
+	}
+
+	/* FIFO threshold validation */
+	if (fifo_threshold[i] != PARAM_UNSET)
+		gfar_fifo_threshold_update(priv, fifo_threshold[i]);
+
+	/* FIFO starve mode hysteresis thresholds validation */
+	if (fifo_starve[i] != PARAM_UNSET &&
+	    fifo_starve_off[i] != PARAM_UNSET) {
+		gfar_fifo_starve_update(priv, fifo_starve[i],
+					fifo_starve_off[i]);
+	}
+
+	/* BD stashing validation */
+	if (bd_stash[i] != PARAM_UNSET)
+		gfar_bd_stash_update(priv, bd_stash[i]);
+
+	/* Rx frame stashing validation */
+	if (rx_stash_size[i] != PARAM_UNSET &&
+	    rx_stash_index[i] != PARAM_UNSET) {
+		gfar_rx_stash_update(priv, rx_stash_size[i],
+				     rx_stash_index[i]);
+	}
+}
diff --git a/drivers/net/ethernet/freescale/gianfar_sysfs.c b/drivers/net/ethernet/freescale/gianfar_sysfs.c
deleted file mode 100644
index e02dd13..0000000
--- a/drivers/net/ethernet/freescale/gianfar_sysfs.c
+++ /dev/null
@@ -1,340 +0,0 @@
-/*
- * drivers/net/ethernet/freescale/gianfar_sysfs.c
- *
- * Gianfar Ethernet Driver
- * This driver is designed for the non-CPM ethernet controllers
- * on the 85xx and 83xx family of integrated processors
- * Based on 8260_io/fcc_enet.c
- *
- * Author: Andy Fleming
- * Maintainer: Kumar Gala (galak@kernel.crashing.org)
- * Modifier: Sandeep Gopalpet <sandeep.kumar@freescale.com>
- *
- * Copyright 2002-2009 Freescale Semiconductor, Inc.
- *
- * This program is free software; you can redistribute  it and/or modify it
- * under  the terms of  the GNU General  Public License as published by the
- * Free Software Foundation;  either version 2 of the  License, or (at your
- * option) any later version.
- *
- * Sysfs file creation and management
- */
-
-#include <linux/kernel.h>
-#include <linux/string.h>
-#include <linux/errno.h>
-#include <linux/unistd.h>
-#include <linux/delay.h>
-#include <linux/etherdevice.h>
-#include <linux/spinlock.h>
-#include <linux/mm.h>
-#include <linux/device.h>
-
-#include <asm/uaccess.h>
-#include <linux/module.h>
-
-#include "gianfar.h"
-
-static ssize_t gfar_show_bd_stash(struct device *dev,
-				  struct device_attribute *attr, char *buf)
-{
-	struct gfar_private *priv = netdev_priv(to_net_dev(dev));
-
-	return sprintf(buf, "%s\n", priv->bd_stash_en ? "on" : "off");
-}
-
-static ssize_t gfar_set_bd_stash(struct device *dev,
-				 struct device_attribute *attr,
-				 const char *buf, size_t count)
-{
-	struct gfar_private *priv = netdev_priv(to_net_dev(dev));
-	struct gfar __iomem *regs = priv->gfargrp[0].regs;
-	int new_setting = 0;
-	u32 temp;
-	unsigned long flags;
-
-	if (!(priv->device_flags & FSL_GIANFAR_DEV_HAS_BD_STASHING))
-		return count;
-
-
-	/* Find out the new setting */
-	if (!strncmp("on", buf, count - 1) || !strncmp("1", buf, count - 1))
-		new_setting = 1;
-	else if (!strncmp("off", buf, count - 1) ||
-		 !strncmp("0", buf, count - 1))
-		new_setting = 0;
-	else
-		return count;
-
-
-	local_irq_save(flags);
-	lock_rx_qs(priv);
-
-	/* Set the new stashing value */
-	priv->bd_stash_en = new_setting;
-
-	temp = gfar_read(&regs->attr);
-
-	if (new_setting)
-		temp |= ATTR_BDSTASH;
-	else
-		temp &= ~(ATTR_BDSTASH);
-
-	gfar_write(&regs->attr, temp);
-
-	unlock_rx_qs(priv);
-	local_irq_restore(flags);
-
-	return count;
-}
-
-static DEVICE_ATTR(bd_stash, 0644, gfar_show_bd_stash, gfar_set_bd_stash);
-
-static ssize_t gfar_show_rx_stash_size(struct device *dev,
-				       struct device_attribute *attr, char *buf)
-{
-	struct gfar_private *priv = netdev_priv(to_net_dev(dev));
-
-	return sprintf(buf, "%d\n", priv->rx_stash_size);
-}
-
-static ssize_t gfar_set_rx_stash_size(struct device *dev,
-				      struct device_attribute *attr,
-				      const char *buf, size_t count)
-{
-	struct gfar_private *priv = netdev_priv(to_net_dev(dev));
-	struct gfar __iomem *regs = priv->gfargrp[0].regs;
-	unsigned int length = simple_strtoul(buf, NULL, 0);
-	u32 temp;
-	unsigned long flags;
-
-	if (!(priv->device_flags & FSL_GIANFAR_DEV_HAS_BUF_STASHING))
-		return count;
-
-	local_irq_save(flags);
-	lock_rx_qs(priv);
-
-	if (length > priv->rx_buffer_size)
-		goto out;
-
-	if (length == priv->rx_stash_size)
-		goto out;
-
-	priv->rx_stash_size = length;
-
-	temp = gfar_read(&regs->attreli);
-	temp &= ~ATTRELI_EL_MASK;
-	temp |= ATTRELI_EL(length);
-	gfar_write(&regs->attreli, temp);
-
-	/* Turn stashing on/off as appropriate */
-	temp = gfar_read(&regs->attr);
-
-	if (length)
-		temp |= ATTR_BUFSTASH;
-	else
-		temp &= ~(ATTR_BUFSTASH);
-
-	gfar_write(&regs->attr, temp);
-
-out:
-	unlock_rx_qs(priv);
-	local_irq_restore(flags);
-
-	return count;
-}
-
-static DEVICE_ATTR(rx_stash_size, 0644, gfar_show_rx_stash_size,
-		   gfar_set_rx_stash_size);
-
-/* Stashing will only be enabled when rx_stash_size != 0 */
-static ssize_t gfar_show_rx_stash_index(struct device *dev,
-					struct device_attribute *attr,
-					char *buf)
-{
-	struct gfar_private *priv = netdev_priv(to_net_dev(dev));
-
-	return sprintf(buf, "%d\n", priv->rx_stash_index);
-}
-
-static ssize_t gfar_set_rx_stash_index(struct device *dev,
-				       struct device_attribute *attr,
-				       const char *buf, size_t count)
-{
-	struct gfar_private *priv = netdev_priv(to_net_dev(dev));
-	struct gfar __iomem *regs = priv->gfargrp[0].regs;
-	unsigned short index = simple_strtoul(buf, NULL, 0);
-	u32 temp;
-	unsigned long flags;
-
-	if (!(priv->device_flags & FSL_GIANFAR_DEV_HAS_BUF_STASHING))
-		return count;
-
-	local_irq_save(flags);
-	lock_rx_qs(priv);
-
-	if (index > priv->rx_stash_size)
-		goto out;
-
-	if (index == priv->rx_stash_index)
-		goto out;
-
-	priv->rx_stash_index = index;
-
-	temp = gfar_read(&regs->attreli);
-	temp &= ~ATTRELI_EI_MASK;
-	temp |= ATTRELI_EI(index);
-	gfar_write(&regs->attreli, temp);
-
-out:
-	unlock_rx_qs(priv);
-	local_irq_restore(flags);
-
-	return count;
-}
-
-static DEVICE_ATTR(rx_stash_index, 0644, gfar_show_rx_stash_index,
-		   gfar_set_rx_stash_index);
-
-static ssize_t gfar_show_fifo_threshold(struct device *dev,
-					struct device_attribute *attr,
-					char *buf)
-{
-	struct gfar_private *priv = netdev_priv(to_net_dev(dev));
-
-	return sprintf(buf, "%d\n", priv->fifo_threshold);
-}
-
-static ssize_t gfar_set_fifo_threshold(struct device *dev,
-				       struct device_attribute *attr,
-				       const char *buf, size_t count)
-{
-	struct gfar_private *priv = netdev_priv(to_net_dev(dev));
-	struct gfar __iomem *regs = priv->gfargrp[0].regs;
-	unsigned int length = simple_strtoul(buf, NULL, 0);
-	u32 temp;
-	unsigned long flags;
-
-	if (length > GFAR_MAX_FIFO_THRESHOLD)
-		return count;
-
-	local_irq_save(flags);
-	lock_tx_qs(priv);
-
-	priv->fifo_threshold = length;
-
-	temp = gfar_read(&regs->fifo_tx_thr);
-	temp &= ~FIFO_TX_THR_MASK;
-	temp |= length;
-	gfar_write(&regs->fifo_tx_thr, temp);
-
-	unlock_tx_qs(priv);
-	local_irq_restore(flags);
-
-	return count;
-}
-
-static DEVICE_ATTR(fifo_threshold, 0644, gfar_show_fifo_threshold,
-		   gfar_set_fifo_threshold);
-
-static ssize_t gfar_show_fifo_starve(struct device *dev,
-				     struct device_attribute *attr, char *buf)
-{
-	struct gfar_private *priv = netdev_priv(to_net_dev(dev));
-
-	return sprintf(buf, "%d\n", priv->fifo_starve);
-}
-
-static ssize_t gfar_set_fifo_starve(struct device *dev,
-				    struct device_attribute *attr,
-				    const char *buf, size_t count)
-{
-	struct gfar_private *priv = netdev_priv(to_net_dev(dev));
-	struct gfar __iomem *regs = priv->gfargrp[0].regs;
-	unsigned int num = simple_strtoul(buf, NULL, 0);
-	u32 temp;
-	unsigned long flags;
-
-	if (num > GFAR_MAX_FIFO_STARVE)
-		return count;
-
-	local_irq_save(flags);
-	lock_tx_qs(priv);
-
-	priv->fifo_starve = num;
-
-	temp = gfar_read(&regs->fifo_tx_starve);
-	temp &= ~FIFO_TX_STARVE_MASK;
-	temp |= num;
-	gfar_write(&regs->fifo_tx_starve, temp);
-
-	unlock_tx_qs(priv);
-	local_irq_restore(flags);
-
-	return count;
-}
-
-static DEVICE_ATTR(fifo_starve, 0644, gfar_show_fifo_starve,
-		   gfar_set_fifo_starve);
-
-static ssize_t gfar_show_fifo_starve_off(struct device *dev,
-					 struct device_attribute *attr,
-					 char *buf)
-{
-	struct gfar_private *priv = netdev_priv(to_net_dev(dev));
-
-	return sprintf(buf, "%d\n", priv->fifo_starve_off);
-}
-
-static ssize_t gfar_set_fifo_starve_off(struct device *dev,
-					struct device_attribute *attr,
-					const char *buf, size_t count)
-{
-	struct gfar_private *priv = netdev_priv(to_net_dev(dev));
-	struct gfar __iomem *regs = priv->gfargrp[0].regs;
-	unsigned int num = simple_strtoul(buf, NULL, 0);
-	u32 temp;
-	unsigned long flags;
-
-	if (num > GFAR_MAX_FIFO_STARVE_OFF)
-		return count;
-
-	local_irq_save(flags);
-	lock_tx_qs(priv);
-
-	priv->fifo_starve_off = num;
-
-	temp = gfar_read(&regs->fifo_tx_starve_shutoff);
-	temp &= ~FIFO_TX_STARVE_OFF_MASK;
-	temp |= num;
-	gfar_write(&regs->fifo_tx_starve_shutoff, temp);
-
-	unlock_tx_qs(priv);
-	local_irq_restore(flags);
-
-	return count;
-}
-
-static DEVICE_ATTR(fifo_starve_off, 0644, gfar_show_fifo_starve_off,
-		   gfar_set_fifo_starve_off);
-
-void gfar_init_sysfs(struct net_device *dev)
-{
-	struct gfar_private *priv = netdev_priv(dev);
-	int rc;
-
-	/* Initialize the default values */
-	priv->fifo_threshold = DEFAULT_FIFO_TX_THR;
-	priv->fifo_starve = DEFAULT_FIFO_TX_STARVE;
-	priv->fifo_starve_off = DEFAULT_FIFO_TX_STARVE_OFF;
-
-	/* Create our sysfs files */
-	rc = device_create_file(&dev->dev, &dev_attr_bd_stash);
-	rc |= device_create_file(&dev->dev, &dev_attr_rx_stash_size);
-	rc |= device_create_file(&dev->dev, &dev_attr_rx_stash_index);
-	rc |= device_create_file(&dev->dev, &dev_attr_fifo_threshold);
-	rc |= device_create_file(&dev->dev, &dev_attr_fifo_starve);
-	rc |= device_create_file(&dev->dev, &dev_attr_fifo_starve_off);
-	if (rc)
-		dev_err(&dev->dev, "Error creating gianfar sysfs files\n");
-}
-- 
1.7.11.7

[PATCH net-next 3/6] gianfar: Remove useless HAS_PADDING device flag

[Claudiu Manoil]

The RCTRL updates of the FSL_GIANFAR_DEV_HAS_PADDING device
flag get overriden by the FSL_GIANFAR_DEV_HAS_TIMER flag
settings, which impose a Rx padding alignment of 8 bytes.
As all the eTSEC devices that set HAS_PADDING also set the
HAS_TIMER flag, the HAS_PADDING flag is now obsolete.

Signed-off-by: Claudiu Manoil <claudiu.manoil@freescale.com>
---
 drivers/net/ethernet/freescale/gianfar.c | 15 +++------------
 drivers/net/ethernet/freescale/gianfar.h |  1 -
 2 files changed, 3 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
index 27cc8b2..7c7d3aa 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -375,13 +375,6 @@ static void gfar_init_mac(struct net_device *ndev)
 		rctrl |= RCTRL_PADDING(priv->padding);
 	}
 
-	/* Insert receive time stamps into padding alignment bytes */
-	if (priv->device_flags & FSL_GIANFAR_DEV_HAS_TIMER) {
-		rctrl &= ~RCTRL_PAL_MASK;
-		rctrl |= RCTRL_PADDING(8);
-		priv->padding = 8;
-	}
-
 	/* Enable HW time stamping if requested from user space */
 	if (priv->hwts_rx_en) {
 		rctrl |= RCTRL_PRSDEP_INIT | RCTRL_TS_ENABLE;
@@ -770,7 +763,6 @@ static int gfar_of_init(struct platform_device *ofdev, struct net_device **pdev)
 				     FSL_GIANFAR_DEV_HAS_COALESCE |
 				     FSL_GIANFAR_DEV_HAS_RMON |
 				     FSL_GIANFAR_DEV_HAS_MULTI_INTR |
-				     FSL_GIANFAR_DEV_HAS_PADDING |
 				     FSL_GIANFAR_DEV_HAS_CSUM |
 				     FSL_GIANFAR_DEV_HAS_VLAN |
 				     FSL_GIANFAR_DEV_HAS_MAGIC_PACKET |
@@ -1171,10 +1163,9 @@ static int gfar_probe(struct platform_device *ofdev)
 
 	gfar_init_addr_hash_table(priv);
 
-	if (priv->device_flags & FSL_GIANFAR_DEV_HAS_PADDING)
-		priv->padding = DEFAULT_PADDING;
-	else
-		priv->padding = 0;
+	/* Insert receive time stamps into padding alignment bytes */
+	if (priv->device_flags & FSL_GIANFAR_DEV_HAS_TIMER)
+		priv->padding = 8;
 
 	if (dev->features & NETIF_F_IP_CSUM ||
 	    priv->device_flags & FSL_GIANFAR_DEV_HAS_TIMER)
diff --git a/drivers/net/ethernet/freescale/gianfar.h b/drivers/net/ethernet/freescale/gianfar.h
index 20121ef..9fcde7c 100644
--- a/drivers/net/ethernet/freescale/gianfar.h
+++ b/drivers/net/ethernet/freescale/gianfar.h
@@ -879,7 +879,6 @@ struct gfar {
 #define FSL_GIANFAR_DEV_HAS_CSUM		0x00000010
 #define FSL_GIANFAR_DEV_HAS_VLAN		0x00000020
 #define FSL_GIANFAR_DEV_HAS_EXTENDED_HASH	0x00000040
-#define FSL_GIANFAR_DEV_HAS_PADDING		0x00000080
 #define FSL_GIANFAR_DEV_HAS_MAGIC_PACKET	0x00000100
 #define FSL_GIANFAR_DEV_HAS_BD_STASHING		0x00000200
 #define FSL_GIANFAR_DEV_HAS_BUF_STASHING	0x00000400
-- 
1.7.11.7

[PATCH net-next 4/6] gianfar: Factor out enabling/disabling of hw interrupts

[Claudiu Manoil]

Throughout the code there are places where the controller's
hw interrupt sources need to get disabled/enabled (masked/
un-masked) all at once.  The recommendation for disabling
the interrupts is to clear the ievent first then the imask
register (not the other way around).
Use the gfar_ints_enable/disable() helpers to make these
operations consistent.

Signed-off-by: Claudiu Manoil <claudiu.manoil@freescale.com>
---
 drivers/net/ethernet/freescale/gianfar.c | 60 ++++++++++++++++----------------
 1 file changed, 30 insertions(+), 30 deletions(-)

diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
index 7c7d3aa..158c4e8 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -448,6 +448,29 @@ static const struct net_device_ops gfar_netdev_ops = {
 #endif
 };
 
+static void gfar_ints_disable(struct gfar_private *priv)
+{
+	int i;
+	for (i = 0; i < priv->num_grps; i++) {
+		struct gfar __iomem *regs = priv->gfargrp[i].regs;
+		/* Clear IEVENT */
+		gfar_write(&regs->ievent, IEVENT_INIT_CLEAR);
+
+		/* Initialize IMASK */
+		gfar_write(&regs->imask, IMASK_INIT_CLEAR);
+	}
+}
+
+static void gfar_ints_enable(struct gfar_private *priv)
+{
+	int i;
+	for (i = 0; i < priv->num_grps; i++) {
+		struct gfar __iomem *regs = priv->gfargrp[i].regs;
+		/* Unmask the interrupts we look for */
+		gfar_write(&regs->imask, IMASK_DEFAULT);
+	}
+}
+
 void lock_rx_qs(struct gfar_private *priv)
 {
 	int i;
@@ -1551,19 +1574,10 @@ static void gfar_configure_serdes(struct net_device *dev)
 static void init_registers(struct net_device *dev)
 {
 	struct gfar_private *priv = netdev_priv(dev);
-	struct gfar __iomem *regs = NULL;
-	int i;
-
-	for (i = 0; i < priv->num_grps; i++) {
-		regs = priv->gfargrp[i].regs;
-		/* Clear IEVENT */
-		gfar_write(&regs->ievent, IEVENT_INIT_CLEAR);
+	struct gfar __iomem *regs = priv->gfargrp[0].regs;
 
-		/* Initialize IMASK */
-		gfar_write(&regs->imask, IMASK_INIT_CLEAR);
-	}
+	gfar_ints_disable(priv);
 
-	regs = priv->gfargrp[0].regs;
 	/* Init hash registers to zero */
 	gfar_write(&regs->igaddr0, 0);
 	gfar_write(&regs->igaddr1, 0);
@@ -1625,20 +1639,11 @@ static int __gfar_is_rx_idle(struct gfar_private *priv)
 static void gfar_halt_nodisable(struct net_device *dev)
 {
 	struct gfar_private *priv = netdev_priv(dev);
-	struct gfar __iomem *regs = NULL;
+	struct gfar __iomem *regs = priv->gfargrp[0].regs;
 	u32 tempval;
-	int i;
-
-	for (i = 0; i < priv->num_grps; i++) {
-		regs = priv->gfargrp[i].regs;
-		/* Mask all interrupts */
-		gfar_write(&regs->imask, IMASK_INIT_CLEAR);
 
-		/* Clear all interrupts */
-		gfar_write(&regs->ievent, IEVENT_INIT_CLEAR);
-	}
+	gfar_ints_disable(priv);
 
-	regs = priv->gfargrp[0].regs;
 	/* Stop the DMA, and wait for it to stop */
 	tempval = gfar_read(&regs->dmactrl);
 	if ((tempval & (DMACTRL_GRS | DMACTRL_GTS)) !=
@@ -1826,10 +1831,10 @@ void gfar_start(struct net_device *dev)
 		/* Clear THLT/RHLT, so that the DMA starts polling now */
 		gfar_write(&regs->tstat, priv->gfargrp[i].tstat);
 		gfar_write(&regs->rstat, priv->gfargrp[i].rstat);
-		/* Unmask the interrupts we look for */
-		gfar_write(&regs->imask, IMASK_DEFAULT);
 	}
 
+	gfar_ints_enable(priv);
+
 	dev->trans_start = jiffies; /* prevent tx timeout */
 }
 
@@ -1934,15 +1939,10 @@ err_irq_fail:
 int startup_gfar(struct net_device *ndev)
 {
 	struct gfar_private *priv = netdev_priv(ndev);
-	struct gfar __iomem *regs = NULL;
 	int err, i, j;
 
-	for (i = 0; i < priv->num_grps; i++) {
-		regs= priv->gfargrp[i].regs;
-		gfar_write(&regs->imask, IMASK_INIT_CLEAR);
-	}
+	gfar_ints_disable(priv);
 
-	regs= priv->gfargrp[0].regs;
 	err = gfar_alloc_skb_resources(ndev);
 	if (err)
 		return err;
-- 
1.7.11.7

[PATCH net-next 5/6] gianfar: Add missing graceful reset steps and fixes

[Claudiu Manoil]

gfar_halt() and gfar_start() are responsible for stopping
and starting the DMA and the Rx/Tx hw rings. They implement
the support for the "graceful Rx/Tx stop/start" hw procedure,
and also disable/enable eTSEC's hw interrupts in the process.

The GRS/GTS procedure requires however to have the RQUEUE/TQUEUE
registers cleared first and to wait for a period of time for the
current frame to pass through the interface (around ~10ms for a
jumbo frame). Only then may the GTS and GRS bits from DMACTRL be
set to shut down the DMA, and finally the Tx_EN and Rx_EN bits in
MACCFG1 may be cleared to disable the Tx/Rx blocks.

The same register programming order applies to start the Rx/Tx:
enabling the RQUEUE/TQUEUE *before* clearing the GRS/GTS bits.

This is a HW recommendation in order to avoid a possible
controller "lock up" during graceful reset.

Cleanup the gfar_halt()/start() prototypes, to take priv instead
of ndev as their purpose is to operate on HW. Enabling the
RQUEUE/TQUEUE in the hw_init() is not needed anymore since
that's the job of gfar_start().

Signed-off-by: Claudiu Manoil <claudiu.manoil@freescale.com>
---
 drivers/net/ethernet/freescale/gianfar.c         | 53 ++++++++++++------------
 drivers/net/ethernet/freescale/gianfar.h         |  3 +-
 drivers/net/ethernet/freescale/gianfar_ethtool.c |  5 +--
 3 files changed, 31 insertions(+), 30 deletions(-)

diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
index 158c4e8..7418b2c 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -138,9 +138,7 @@ int gfar_clean_rx_ring(struct gfar_priv_rx_q *rx_queue, int rx_work_limit);
 static void gfar_clean_tx_ring(struct gfar_priv_tx_q *tx_queue);
 static void gfar_process_frame(struct net_device *dev, struct sk_buff *skb,
 			       int amount_pull, struct napi_struct *napi);
-void gfar_halt(struct net_device *dev);
-static void gfar_halt_nodisable(struct net_device *dev);
-void gfar_start(struct net_device *dev);
+static void gfar_halt_nodisable(struct gfar_private *priv);
 static void gfar_clear_exact_match(struct net_device *dev);
 static void gfar_set_mac_for_addr(struct net_device *dev, int num,
 				  const u8 *addr);
@@ -1070,10 +1068,6 @@ static void gfar_hw_init(struct gfar_private *priv)
 	/* Program the interrupt steering regs, only for MG devices */
 	if (priv->num_grps > 1)
 		gfar_write_isrg(priv);
-
-	/* Enable all Rx/Tx queues after MAC reset */
-	gfar_write(&regs->rqueue, priv->rqueue);
-	gfar_write(&regs->tqueue, priv->tqueue);
 }
 
 static void __init gfar_init_addr_hash_table(struct gfar_private *priv)
@@ -1149,7 +1143,7 @@ static int gfar_probe(struct platform_device *ofdev)
 	/* Stop the DMA engine now, in case it was running before
 	 * (The firmware could have used it, and left it running).
 	 */
-	gfar_halt(dev);
+	gfar_halt(priv);
 
 	gfar_hw_init(priv);
 
@@ -1317,7 +1311,7 @@ static int gfar_suspend(struct device *dev)
 		lock_tx_qs(priv);
 		lock_rx_qs(priv);
 
-		gfar_halt_nodisable(ndev);
+		gfar_halt_nodisable(priv);
 
 		/* Disable Tx, and Rx if wake-on-LAN is disabled. */
 		tempval = gfar_read(&regs->maccfg1);
@@ -1381,7 +1375,7 @@ static int gfar_resume(struct device *dev)
 	tempval &= ~MACCFG2_MPEN;
 	gfar_write(&regs->maccfg2, tempval);
 
-	gfar_start(ndev);
+	gfar_start(priv);
 
 	unlock_rx_qs(priv);
 	unlock_tx_qs(priv);
@@ -1413,7 +1407,7 @@ static int gfar_restore(struct device *dev)
 	init_registers(ndev);
 	gfar_set_mac_address(ndev);
 	gfar_init_mac(ndev);
-	gfar_start(ndev);
+	gfar_start(priv);
 
 	priv->oldlink = 0;
 	priv->oldspeed = 0;
@@ -1636,9 +1630,8 @@ static int __gfar_is_rx_idle(struct gfar_private *priv)
 }
 
 /* Halt the receive and transmit queues */
-static void gfar_halt_nodisable(struct net_device *dev)
+static void gfar_halt_nodisable(struct gfar_private *priv)
 {
-	struct gfar_private *priv = netdev_priv(dev);
 	struct gfar __iomem *regs = priv->gfargrp[0].regs;
 	u32 tempval;
 
@@ -1664,15 +1657,20 @@ static void gfar_halt_nodisable(struct net_device *dev)
 }
 
 /* Halt the receive and transmit queues */
-void gfar_halt(struct net_device *dev)
+void gfar_halt(struct gfar_private *priv)
 {
-	struct gfar_private *priv = netdev_priv(dev);
 	struct gfar __iomem *regs = priv->gfargrp[0].regs;
 	u32 tempval;
 
-	gfar_halt_nodisable(dev);
+	/* Dissable the Rx/Tx hw queues */
+	gfar_write(&regs->rqueue, 0);
+	gfar_write(&regs->tqueue, 0);
 
-	/* Disable Rx and Tx */
+	mdelay(10);
+
+	gfar_halt_nodisable(priv);
+
+	/* Disable Rx/Tx DMA */
 	tempval = gfar_read(&regs->maccfg1);
 	tempval &= ~(MACCFG1_RX_EN | MACCFG1_TX_EN);
 	gfar_write(&regs->maccfg1, tempval);
@@ -1699,7 +1697,7 @@ void stop_gfar(struct net_device *dev)
 	lock_tx_qs(priv);
 	lock_rx_qs(priv);
 
-	gfar_halt(dev);
+	gfar_halt(priv);
 
 	unlock_rx_qs(priv);
 	unlock_tx_qs(priv);
@@ -1804,17 +1802,15 @@ static void free_skb_resources(struct gfar_private *priv)
 			  priv->tx_queue[0]->tx_bd_dma_base);
 }
 
-void gfar_start(struct net_device *dev)
+void gfar_start(struct gfar_private *priv)
 {
-	struct gfar_private *priv = netdev_priv(dev);
 	struct gfar __iomem *regs = priv->gfargrp[0].regs;
 	u32 tempval;
 	int i = 0;
 
-	/* Enable Rx and Tx in MACCFG1 */
-	tempval = gfar_read(&regs->maccfg1);
-	tempval |= (MACCFG1_RX_EN | MACCFG1_TX_EN);
-	gfar_write(&regs->maccfg1, tempval);
+	/* Enable Rx/Tx hw queues */
+	gfar_write(&regs->rqueue, priv->rqueue);
+	gfar_write(&regs->tqueue, priv->tqueue);
 
 	/* Initialize DMACTRL to have WWR and WOP */
 	tempval = gfar_read(&regs->dmactrl);
@@ -1833,9 +1829,14 @@ void gfar_start(struct net_device *dev)
 		gfar_write(&regs->rstat, priv->gfargrp[i].rstat);
 	}
 
+	/* Enable Rx/Tx DMA */
+	tempval = gfar_read(&regs->maccfg1);
+	tempval |= (MACCFG1_RX_EN | MACCFG1_TX_EN);
+	gfar_write(&regs->maccfg1, tempval);
+
 	gfar_ints_enable(priv);
 
-	dev->trans_start = jiffies; /* prevent tx timeout */
+	priv->ndev->trans_start = jiffies; /* prevent tx timeout */
 }
 
 static void gfar_configure_coalescing(struct gfar_private *priv,
@@ -1959,7 +1960,7 @@ int startup_gfar(struct net_device *ndev)
 	}
 
 	/* Start the controller */
-	gfar_start(ndev);
+	gfar_start(priv);
 
 	phy_start(priv->phydev);
 
diff --git a/drivers/net/ethernet/freescale/gianfar.h b/drivers/net/ethernet/freescale/gianfar.h
index 9fcde7c..f288ee9 100644
--- a/drivers/net/ethernet/freescale/gianfar.h
+++ b/drivers/net/ethernet/freescale/gianfar.h
@@ -1209,7 +1209,8 @@ void unlock_tx_qs(struct gfar_private *priv);
 irqreturn_t gfar_receive(int irq, void *dev_id);
 int startup_gfar(struct net_device *dev);
 void stop_gfar(struct net_device *dev);
-void gfar_halt(struct net_device *dev);
+void gfar_halt(struct gfar_private *priv);
+void gfar_start(struct gfar_private *priv);
 void gfar_phy_test(struct mii_bus *bus, struct phy_device *phydev, int enable,
 		   u32 regnum, u32 read);
 void gfar_configure_coalescing_all(struct gfar_private *priv);
diff --git a/drivers/net/ethernet/freescale/gianfar_ethtool.c b/drivers/net/ethernet/freescale/gianfar_ethtool.c
index 63d2344..69fab72 100644
--- a/drivers/net/ethernet/freescale/gianfar_ethtool.c
+++ b/drivers/net/ethernet/freescale/gianfar_ethtool.c
@@ -44,7 +44,6 @@
 
 #include "gianfar.h"
 
-extern void gfar_start(struct net_device *dev);
 extern int gfar_clean_rx_ring(struct gfar_priv_rx_q *rx_queue,
 			      int rx_work_limit);
 
@@ -504,7 +503,7 @@ static int gfar_sringparam(struct net_device *dev,
 		lock_tx_qs(priv);
 		lock_rx_qs(priv);
 
-		gfar_halt(dev);
+		gfar_halt(priv);
 
 		unlock_rx_qs(priv);
 		unlock_tx_qs(priv);
@@ -627,7 +626,7 @@ int gfar_set_features(struct net_device *dev, netdev_features_t features)
 		lock_tx_qs(priv);
 		lock_rx_qs(priv);
 
-		gfar_halt(dev);
+		gfar_halt(priv);
 
 		unlock_tx_qs(priv);
 		unlock_rx_qs(priv);
-- 
1.7.11.7

[PATCH net-next 6/6] gianfar: Remove clean_rx_ring race from gfar_ethtool

[Claudiu Manoil]

gfar_clean_rx_ring() was designed to be called from napi
(rx softirq) context to do the Rx processing. Calling it
from a process context like this is a bug as it will
clearly race with the napi Rx processing.

There's also no point in initializing num_txbdfree since
startup_gfar() already does that, when bringing the device
up again (after reset). Changing num_txbdfree "on-the-fly"
like this is also subject to race conditions.  num_txbdfree
is handled by the Tx processing path and the device reset
procedure.  Also, don't assume that num_rx_queues is always
equal to num_tx_queues.

Signed-off-by: Claudiu Manoil <claudiu.manoil@freescale.com>
---
 drivers/net/ethernet/freescale/gianfar_ethtool.c | 63 +++---------------------
 1 file changed, 8 insertions(+), 55 deletions(-)

diff --git a/drivers/net/ethernet/freescale/gianfar_ethtool.c b/drivers/net/ethernet/freescale/gianfar_ethtool.c
index 69fab72..19557ec 100644
--- a/drivers/net/ethernet/freescale/gianfar_ethtool.c
+++ b/drivers/net/ethernet/freescale/gianfar_ethtool.c
@@ -44,9 +44,6 @@
 
 #include "gianfar.h"
 
-extern int gfar_clean_rx_ring(struct gfar_priv_rx_q *rx_queue,
-			      int rx_work_limit);
-
 #define GFAR_MAX_COAL_USECS 0xffff
 #define GFAR_MAX_COAL_FRAMES 0xff
 static void gfar_fill_stats(struct net_device *dev, struct ethtool_stats *dummy,
@@ -466,15 +463,13 @@ static void gfar_gringparam(struct net_device *dev,
 }
 
 /* Change the current ring parameters, stopping the controller if
- * necessary so that we don't mess things up while we're in
- * motion.  We wait for the ring to be clean before reallocating
- * the rings.
+ * necessary so that we don't mess things up while we're in motion.
  */
 static int gfar_sringparam(struct net_device *dev,
 			   struct ethtool_ringparam *rvals)
 {
 	struct gfar_private *priv = netdev_priv(dev);
-	int err = 0, i = 0;
+	int err = 0, i;
 
 	if (rvals->rx_pending > GFAR_RX_MAX_RING_SIZE)
 		return -EINVAL;
@@ -492,38 +487,15 @@ static int gfar_sringparam(struct net_device *dev,
 		return -EINVAL;
 	}
 
-
-	if (dev->flags & IFF_UP) {
-		unsigned long flags;
-
-		/* Halt TX and RX, and process the frames which
-		 * have already been received
-		 */
-		local_irq_save(flags);
-		lock_tx_qs(priv);
-		lock_rx_qs(priv);
-
-		gfar_halt(priv);
-
-		unlock_rx_qs(priv);
-		unlock_tx_qs(priv);
-		local_irq_restore(flags);
-
-		for (i = 0; i < priv->num_rx_queues; i++)
-			gfar_clean_rx_ring(priv->rx_queue[i],
-					   priv->rx_queue[i]->rx_ring_size);
-
-		/* Now we take down the rings to rebuild them */
+	if (dev->flags & IFF_UP)
 		stop_gfar(dev);
-	}
 
-	/* Change the size */
-	for (i = 0; i < priv->num_rx_queues; i++) {
+	/* Change the sizes */
+	for (i = 0; i < priv->num_rx_queues; i++)
 		priv->rx_queue[i]->rx_ring_size = rvals->rx_pending;
+
+	for (i = 0; i < priv->num_tx_queues; i++)
 		priv->tx_queue[i]->tx_ring_size = rvals->tx_pending;
-		priv->tx_queue[i]->num_txbdfree =
-			priv->tx_queue[i]->tx_ring_size;
-	}
 
 	/* Rebuild the rings with the new size */
 	if (dev->flags & IFF_UP) {
@@ -607,10 +579,8 @@ static int gfar_spauseparam(struct net_device *dev,
 
 int gfar_set_features(struct net_device *dev, netdev_features_t features)
 {
-	struct gfar_private *priv = netdev_priv(dev);
-	unsigned long flags;
-	int err = 0, i = 0;
 	netdev_features_t changed = dev->features ^ features;
+	int err = 0;
 
 	if (changed & (NETIF_F_HW_VLAN_CTAG_TX|NETIF_F_HW_VLAN_CTAG_RX))
 		gfar_vlan_mode(dev, features);
@@ -619,23 +589,6 @@ int gfar_set_features(struct net_device *dev, netdev_features_t features)
 		return 0;
 
 	if (dev->flags & IFF_UP) {
-		/* Halt TX and RX, and process the frames which
-		 * have already been received
-		 */
-		local_irq_save(flags);
-		lock_tx_qs(priv);
-		lock_rx_qs(priv);
-
-		gfar_halt(priv);
-
-		unlock_tx_qs(priv);
-		unlock_rx_qs(priv);
-		local_irq_restore(flags);
-
-		for (i = 0; i < priv->num_rx_queues; i++)
-			gfar_clean_rx_ring(priv->rx_queue[i],
-					   priv->rx_queue[i]->rx_ring_size);
-
 		/* Now we take down the rings to rebuild them */
 		stop_gfar(dev);
 
-- 
1.7.11.7

Re: [PATCH net-next 2/6] gianfar: Replace sysfs stubs with module params (fix)

[David Miller]

From: Claudiu Manoil <claudiu.manoil@freescale.com>
Date: Fri, 14 Feb 2014 14:04:01 +0200

> Removing the sysfs stubs for the Tx FIFOCFG and ATTRELI
> (stashing) config registers, as these registers may only
> be configured after a MAC reset, with the controller stopped
> (i.e. during hw init, at probe() time). The current sysfs
> stubs allow on-the-fly updates of these registers (the locking
> measures are useless and only add unecessary code).
> 
> Changing these registers on-the-fly is strogly discouraged.
> In this regard, this patch may be seen as a security fix.
> 
> To address this issue and not lose entirely these config
> params, they are now accessible as driver module parameters,
> and their names and default values have been preserved.
> 
> Moreover, the stasing configuration options were effectively
> disabled (didn't get to the hw anyway if changed) because
> the stashing device_flags (HAS_BD_STASHING|HAS_BUF_STASHING)
> were "accidentally" cleared during probe(). The patch fixes
> this bug as well.
> 
> Signed-off-by: Claudiu Manoil <claudiu.manoil@freescale.com>

Sorry, no new module parameters.

And as you state these never actually reached the hardware so they
_never worked_.  Please just remove them.

Re: [PATCH net-next 2/6] gianfar: Replace sysfs stubs with module params (fix)

[Claudiu Manoil]

On 2/17/2014 6:48 AM, David Miller wrote:
> From: Claudiu Manoil <claudiu.manoil@freescale.com>
> Date: Fri, 14 Feb 2014 14:04:01 +0200
>
>> Removing the sysfs stubs for the Tx FIFOCFG and ATTRELI
>> (stashing) config registers, as these registers may only
>> be configured after a MAC reset, with the controller stopped
>> (i.e. during hw init, at probe() time). The current sysfs
>> stubs allow on-the-fly updates of these registers (the locking
>> measures are useless and only add unecessary code).
>>
>> Changing these registers on-the-fly is strogly discouraged.
>> In this regard, this patch may be seen as a security fix.
>>
>> To address this issue and not lose entirely these config
>> params, they are now accessible as driver module parameters,
>> and their names and default values have been preserved.
>>
>> Moreover, the stasing configuration options were effectively
>> disabled (didn't get to the hw anyway if changed) because
>> the stashing device_flags (HAS_BD_STASHING|HAS_BUF_STASHING)
>> were "accidentally" cleared during probe(). The patch fixes
>> this bug as well.
>>
>> Signed-off-by: Claudiu Manoil <claudiu.manoil@freescale.com>
>
> Sorry, no new module parameters.
>
> And as you state these never actually reached the hardware so they
> _never worked_.  Please just remove them.
>
>

Sounds reasonable.
Only the stashing parameters (ATTRELI) didn't reach the HW.
The FIFO cfg ones did, but changing these defaults is also
discouraged (as documented in gianfar.txt).
Will send a V2.

Thanks.
Claudiu