Netdev List
 help / color / mirror / Atom feed
* RE: [PATCH RFC] c_can_pci: generic module for c_can on PCI
From: Bhupesh SHARMA @ 2012-06-05 13:22 UTC (permalink / raw)
  To: rubini@gnudd.com
  Cc: federico.vaga@gmail.com, alan@lxorguk.ukuu.org.uk,
	wg@grandegger.com, mkl@pengutronix.de, Giancarlo ASNAGHI,
	alan@linux.intel.com, linux-can@vger.kernel.org,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20120605131337.GA15432@mail.gnudd.com>



> -----Original Message-----
> From: rubini@gnudd.com [mailto:rubini@gnudd.com]
> Sent: Tuesday, June 05, 2012 6:44 PM
> To: Bhupesh SHARMA
> Cc: federico.vaga@gmail.com; alan@lxorguk.ukuu.org.uk;
> wg@grandegger.com; mkl@pengutronix.de; Giancarlo ASNAGHI;
> alan@linux.intel.com; linux-can@vger.kernel.org;
> netdev@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH RFC] c_can_pci: generic module for c_can on PCI
> 
> >> My implementation is align to 32, but I'm trying to make a generic
> PCI
> >> wrapper (some other could be aligned to 16)
> 
> > So it means your implementation is also flaky and you are probably
> > wasting HW memory space while integrating the Bosch C_CAN module in
> > your SoC :)
> 
> Then I may say _your_ implementation is flaky because it wastes one
> bit in the address decoder and a lot of logic gates in the data
> bus. It's normal to align registers at 32 bits, as it's simpler and
> faster.  Most SoCs have only 32-bit aligned registers, for a reason.

You missed my original point. I mentioned in my first mail itself, that I studied a
few SoCs integrating the C_CAN module from Bosch before writing the driver.
Not all have aligned their register space to a 32-bit boundary. 
My platform driver still supports them. This _does_ not imply that our
SoC has/may have the same problem :)

Each SoC designer can have his/her own different view on this sort of implementation.
The platform driver was written to support both the implementations (SW is supposed
to support all sort of HW design constraints :) ).

> > I am not a big fan of adding platform specific flakes in any core
> > file, that why we keep the platform file separate from the core
> > ones.
> 
> A number of other drivers have a shift parameter, because it's very
> common for the hardware integrator to feel free to choose the easiest
> wiring for the device.  The choice to keep the platform driver
> separate from the core driver only adds complication in my opinion:
> you need to export 4 symbols and yhen every user must duplicate code
> (like federico is replicating theplatform driver in the pci driver).
> 
> I'd really prefer to have the core driver be a platform driver, and
> the others just add platform data to describe how it is wired. That's
> actually the reason why the platform bus exists.
> 
> > But I will left Marc and Wolfgang to further comment on the same.
> 
> I agree: let them decide.

Sure..

Regards,
Bhupesh

^ permalink raw reply

* Re: [PATCH 1/1] net: add dev_loopback_xmit() to avoid duplicate code
From: Michel Machado @ 2012-06-05 13:23 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, linux-kernel
In-Reply-To: <1337782819.2779.20.camel@Thor>

>I'm getting really tired of saying this.
>
>As I announced several days ago, it is absolutely not appropriate
>to submit patches other than bug fixes at this time because we are
>in the merge window and the net-next tree is frozen.
>
>And even once I do announce here that the net-next tree is open
>once more, and patches like this one are appropriate, you must
>indicate in the subject line which of the 'net' or 'net-next'
>tree you are targetting your patch at.
>
>Please pay attention to what's going on, and what state the networking
>development trees are in, before submitting changes.

I am sorry. I'm still new to this process, and I clearly haven't found
my way yet. Accept my apology and consider that others making the same
mistake are likely trying to find their ways as well. Rest sure that I
do appreciate your time and work, I'll pay more attention on those time
windows.

Just to avoid bothering you further, should I resend this patch or not?
I saw at the link below that it's already marked as "Deferred".

http://patchwork.ozlabs.org/patch/160929/

Is there a link where I can check the state (i.e. open, frozen) of 'net'
and 'net-next' trees? I think keeping such page, and adding this link to
Documentation/SubmittingPatches would reduce similar mistakes.

-- 
[ ]'s
Michel Machado

^ permalink raw reply

* Re: [PATCH RFC] c_can_pci: generic module for c_can on PCI
From: Alessandro Rubini @ 2012-06-05 13:30 UTC (permalink / raw)
  To: mkl
  Cc: bhupesh.sharma, federico.vaga, alan, wg, giancarlo.asnaghi, alan,
	linux-can, netdev, linux-kernel
In-Reply-To: <4FCE07EE.40003@pengutronix.de>

> I personally like the "pci device sets up a platform device" idea.

Good. Than me or federico will submit a proposal. 
 
> My question is, is this considered being a good practise?

I don't think there are many pci bridges around, but platform drivers
exists just for that reason: to be instantiated when you know how the
wiring ("platform") details.  I.e., somebody registers the platform
device associated to the driver.

Sometimes the platform device is compiled in, sometimes it comes from
the device tree. I think it can come from PCI as well.

thanks
/alessandro, apologizing with Bhupesh Sharma for his tone in the previous mail

^ permalink raw reply

* [PATCH v3 net-next 0/5] gianfar: coding style cleanups
From: Jan Ceuleers @ 2012-06-05 13:42 UTC (permalink / raw)
  To: David S. Miller; +Cc: Jiajun Wu, Joe Perches, netdev

Various coding style cleanups, mostly whitespace and comment reformatting.

I have left a number of lines untouched where I felt that the changes I
was considering were not improving readability. But de gustibus etc.

Patch 4/5 also removes some superfluous local variable initialisations.
These are obviously correct because the variable is initialised again
right after. But you might consider this to be unnecessary code churn
because the compiler is unlikely to generate better code due to this patch.

WARNING: Compile-tested only. I do not have the hardware.

This is my first patch _series_ submission; please be kind.

v3: no changes from v2, except now compile-tested using powerpc crosstool

v2: fixed typo in 5/5 (found by David)

Jan Ceuleers (5):
  gianfar: whitespace cleanup - pointers and multiplications
  gianfar: comment cleanup
  gianfar: various coding style and whitespace cleanups
  gianfar: Remove superfluous initialisations
  gianfar_ethtool: coding style and whitespace cleanups

 drivers/net/ethernet/freescale/gianfar.c         |  491 +++++++++++-----------
 drivers/net/ethernet/freescale/gianfar_ethtool.c |  420 +++++++++---------
 2 files changed, 479 insertions(+), 432 deletions(-)

-- 
1.7.9.5

^ permalink raw reply

* [PATCH v3 net-next 1/5] gianfar: whitespace cleanup - pointers and multiplications
From: Jan Ceuleers @ 2012-06-05 13:42 UTC (permalink / raw)
  To: David S. Miller; +Cc: Jiajun Wu, Joe Perches, netdev
In-Reply-To: <1338903735-24527-1-git-send-email-jan.ceuleers@computer.org>

Signed-off-by: Jan Ceuleers <jan.ceuleers@computer.org>
---
 drivers/net/ethernet/freescale/gianfar.c |   16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
index 0741ade..5ca7b9e 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -266,8 +266,8 @@ static int gfar_alloc_skb_resources(struct net_device *ndev)
 		tx_queue->tx_bd_dma_base = addr;
 		tx_queue->dev = ndev;
 		/* enet DMA only understands physical addresses */
-		addr    += sizeof(struct txbd8) *tx_queue->tx_ring_size;
-		vaddr   += sizeof(struct txbd8) *tx_queue->tx_ring_size;
+		addr    += sizeof(struct txbd8) * tx_queue->tx_ring_size;
+		vaddr   += sizeof(struct txbd8) * tx_queue->tx_ring_size;
 	}
 
 	/* Start the rx descriptor ring where the tx ring leaves off */
@@ -276,8 +276,8 @@ static int gfar_alloc_skb_resources(struct net_device *ndev)
 		rx_queue->rx_bd_base = vaddr;
 		rx_queue->rx_bd_dma_base = addr;
 		rx_queue->dev = ndev;
-		addr    += sizeof (struct rxbd8) * rx_queue->rx_ring_size;
-		vaddr   += sizeof (struct rxbd8) * rx_queue->rx_ring_size;
+		addr    += sizeof(struct rxbd8) * rx_queue->rx_ring_size;
+		vaddr   += sizeof(struct rxbd8) * rx_queue->rx_ring_size;
 	}
 
 	/* Setup the skbuff rings */
@@ -2590,7 +2590,7 @@ static void gfar_new_rxbdp(struct gfar_priv_rx_q *rx_queue, struct rxbd8 *bdp,
 	gfar_init_rxbdp(rx_queue, bdp, buf);
 }
 
-static struct sk_buff * gfar_alloc_skb(struct net_device *dev)
+static struct sk_buff *gfar_alloc_skb(struct net_device *dev)
 {
 	struct gfar_private *priv = netdev_priv(dev);
 	struct sk_buff *skb = NULL;
@@ -2604,7 +2604,7 @@ static struct sk_buff * gfar_alloc_skb(struct net_device *dev)
 	return skb;
 }
 
-struct sk_buff * gfar_new_skb(struct net_device *dev)
+struct sk_buff *gfar_new_skb(struct net_device *dev)
 {
 	struct gfar_private *priv = netdev_priv(dev);
 	struct sk_buff *skb = NULL;
@@ -2728,8 +2728,8 @@ static int gfar_process_frame(struct net_device *dev, struct sk_buff *skb,
 }
 
 /* gfar_clean_rx_ring() -- Processes each frame in the rx ring
- *   until the budget/quota has been reached. Returns the number
- *   of frames handled
+ * until the budget/quota has been reached. Returns the number
+ * of frames handled
  */
 int gfar_clean_rx_ring(struct gfar_priv_rx_q *rx_queue, int rx_work_limit)
 {
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH v3 net-next 4/5] gianfar: Remove superfluous initialisations
From: Jan Ceuleers @ 2012-06-05 13:42 UTC (permalink / raw)
  To: David S. Miller; +Cc: Jiajun Wu, Joe Perches, netdev
In-Reply-To: <1338903735-24527-1-git-send-email-jan.ceuleers@computer.org>

Signed-off-by: Jan Ceuleers <jan.ceuleers@computer.org>
---
 drivers/net/ethernet/freescale/gianfar.c |   30 ++++++++++++++----------------
 1 file changed, 14 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
index ed0b136..f00a095 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -428,7 +428,7 @@ static struct net_device_stats *gfar_get_stats(struct net_device *dev)
 	struct gfar_private *priv = netdev_priv(dev);
 	unsigned long rx_packets = 0, rx_bytes = 0, rx_dropped = 0;
 	unsigned long tx_packets = 0, tx_bytes = 0;
-	int i = 0;
+	int i;
 
 	for (i = 0; i < priv->num_rx_queues; i++) {
 		rx_packets += priv->rx_queue[i]->stats.rx_packets;
@@ -470,7 +470,7 @@ static const struct net_device_ops gfar_netdev_ops = {
 
 void lock_rx_qs(struct gfar_private *priv)
 {
-	int i = 0x0;
+	int i;
 
 	for (i = 0; i < priv->num_rx_queues; i++)
 		spin_lock(&priv->rx_queue[i]->rxlock);
@@ -478,7 +478,7 @@ void lock_rx_qs(struct gfar_private *priv)
 
 void lock_tx_qs(struct gfar_private *priv)
 {
-	int i = 0x0;
+	int i;
 
 	for (i = 0; i < priv->num_tx_queues; i++)
 		spin_lock(&priv->tx_queue[i]->txlock);
@@ -486,7 +486,7 @@ void lock_tx_qs(struct gfar_private *priv)
 
 void unlock_rx_qs(struct gfar_private *priv)
 {
-	int i = 0x0;
+	int i;
 
 	for (i = 0; i < priv->num_rx_queues; i++)
 		spin_unlock(&priv->rx_queue[i]->rxlock);
@@ -494,7 +494,7 @@ void unlock_rx_qs(struct gfar_private *priv)
 
 void unlock_tx_qs(struct gfar_private *priv)
 {
-	int i = 0x0;
+	int i;
 
 	for (i = 0; i < priv->num_tx_queues; i++)
 		spin_unlock(&priv->tx_queue[i]->txlock);
@@ -516,7 +516,7 @@ static inline int gfar_uses_fcb(struct gfar_private *priv)
 
 static void free_tx_pointers(struct gfar_private *priv)
 {
-	int i = 0;
+	int i;
 
 	for (i = 0; i < priv->num_tx_queues; i++)
 		kfree(priv->tx_queue[i]);
@@ -524,7 +524,7 @@ static void free_tx_pointers(struct gfar_private *priv)
 
 static void free_rx_pointers(struct gfar_private *priv)
 {
-	int i = 0;
+	int i;
 
 	for (i = 0; i < priv->num_rx_queues; i++)
 		kfree(priv->rx_queue[i]);
@@ -532,7 +532,7 @@ static void free_rx_pointers(struct gfar_private *priv)
 
 static void unmap_group_regs(struct gfar_private *priv)
 {
-	int i = 0;
+	int i;
 
 	for (i = 0; i < MAXGROUPS; i++)
 		if (priv->gfargrp[i].regs)
@@ -541,7 +541,7 @@ static void unmap_group_regs(struct gfar_private *priv)
 
 static void disable_napi(struct gfar_private *priv)
 {
-	int i = 0;
+	int i;
 
 	for (i = 0; i < priv->num_grps; i++)
 		napi_disable(&priv->gfargrp[i].napi);
@@ -549,7 +549,7 @@ static void disable_napi(struct gfar_private *priv)
 
 static void enable_napi(struct gfar_private *priv)
 {
-	int i = 0;
+	int i;
 
 	for (i = 0; i < priv->num_grps; i++)
 		napi_enable(&priv->gfargrp[i].napi);
@@ -1514,7 +1514,7 @@ static void init_registers(struct net_device *dev)
 {
 	struct gfar_private *priv = netdev_priv(dev);
 	struct gfar __iomem *regs = NULL;
-	int i = 0;
+	int i;
 
 	for (i = 0; i < priv->num_grps; i++) {
 		regs = priv->gfargrp[i].regs;
@@ -1589,7 +1589,7 @@ static void gfar_halt_nodisable(struct net_device *dev)
 	struct gfar_private *priv = netdev_priv(dev);
 	struct gfar __iomem *regs = NULL;
 	u32 tempval;
-	int i = 0;
+	int i;
 
 	for (i = 0; i < priv->num_grps; i++) {
 		regs = priv->gfargrp[i].regs;
@@ -1976,13 +1976,11 @@ static inline struct txfcb *gfar_add_fcb(struct sk_buff *skb)
 static inline void gfar_tx_checksum(struct sk_buff *skb, struct txfcb *fcb,
 				    int fcb_length)
 {
-	u8 flags = 0;
-
 	/* If we're here, it's a IP packet with a TCP or UDP
 	 * payload.  We set it to checksum, using a pseudo-header
 	 * we provide
 	 */
-	flags = TXFCB_DEFAULT;
+	u8 flags = TXFCB_DEFAULT;
 
 	/* Tell the controller what the protocol is
 	 * And provide the already calculated phcs
@@ -2900,7 +2898,7 @@ static int gfar_poll(struct napi_struct *napi, int budget)
 static void gfar_netpoll(struct net_device *dev)
 {
 	struct gfar_private *priv = netdev_priv(dev);
-	int i = 0;
+	int i;
 
 	/* If the device has multiple interrupts, run tx/rx */
 	if (priv->device_flags & FSL_GIANFAR_DEV_HAS_MULTI_INTR) {
-- 
1.7.9.5

^ permalink raw reply related

* Re: [PATCH net-next 5/5] gianfar_ethtool: coding style and whitespace cleanups
From: Jan Ceuleers @ 2012-06-05 13:42 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: David Miller, b06378, joe, netdev
In-Reply-To: <4550590.BhQA539plh@flexo>

On 06/05/2012 12:54 PM, Florian Fainelli wrote:
> What about you setup a cross-compiler and build for one of these architectures 
> where the driver is enabled?

I have now managed to cross-compile the two files I'm touching; thanks
very much to Florian for assisting me off-list. Will send v3 (no changes
relative to RFC v2).

Jan

^ permalink raw reply

* [PATCH v3 net-next 2/5] gianfar: comment cleanup
From: Jan Ceuleers @ 2012-06-05 13:42 UTC (permalink / raw)
  To: David S. Miller; +Cc: Jiajun Wu, Joe Perches, netdev
In-Reply-To: <1338903735-24527-1-git-send-email-jan.ceuleers@computer.org>

Signed-off-by: Jan Ceuleers <jan.ceuleers@computer.org>
---
 drivers/net/ethernet/freescale/gianfar.c |  155 ++++++++++++++++--------------
 1 file changed, 83 insertions(+), 72 deletions(-)

diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
index 5ca7b9e..3144560 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -1,5 +1,4 @@
-/*
- * drivers/net/ethernet/freescale/gianfar.c
+/* drivers/net/ethernet/freescale/gianfar.c
  *
  * Gianfar Ethernet Driver
  * This driver is designed for the non-CPM ethernet controllers
@@ -405,7 +404,8 @@ static void gfar_init_mac(struct net_device *ndev)
 	gfar_write(&regs->attreli, attrs);
 
 	/* Start with defaults, and add stashing or locking
-	 * depending on the approprate variables */
+	 * depending on the approprate variables
+	 */
 	attrs = ATTR_INIT_SETTINGS;
 
 	if (priv->bd_stash_en)
@@ -652,7 +652,7 @@ static int gfar_of_init(struct platform_device *ofdev, struct net_device **pdev)
 	priv->num_rx_queues = num_rx_qs;
 	priv->num_grps = 0x0;
 
-	/* Init Rx queue filer rule set linked list*/
+	/* Init Rx queue filer rule set linked list */
 	INIT_LIST_HEAD(&priv->rx_list.list);
 	priv->rx_list.count = 0;
 	mutex_init(&priv->rx_queue_access);
@@ -960,7 +960,8 @@ static void gfar_detect_errata(struct gfar_private *priv)
 }
 
 /* Set up the ethernet device structure, private data,
- * and anything else we need before we start */
+ * and anything else we need before we start
+ */
 static int gfar_probe(struct platform_device *ofdev)
 {
 	u32 tempval;
@@ -991,8 +992,9 @@ static int gfar_probe(struct platform_device *ofdev)
 
 	gfar_detect_errata(priv);
 
-	/* Stop the DMA engine now, in case it was running before */
-	/* (The firmware could have used it, and left it running). */
+	/* Stop the DMA engine now, in case it was running before
+	 * (The firmware could have used it, and left it running).
+	 */
 	gfar_halt(dev);
 
 	/* Reset MAC layer */
@@ -1098,7 +1100,8 @@ static int gfar_probe(struct platform_device *ofdev)
 
 	/* 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 */
+	 * 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);
@@ -1107,7 +1110,8 @@ static int gfar_probe(struct platform_device *ofdev)
 	}
 
 	/* Calculate RSTAT, TSTAT, RQUEUE and TQUEUE values,
-	 * also assign queues to groups */
+	 * 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,
@@ -1149,7 +1153,7 @@ static int gfar_probe(struct platform_device *ofdev)
 		priv->rx_queue[i]->rxic = DEFAULT_RXIC;
 	}
 
-	/* always enable rx filer*/
+	/* always enable rx filer */
 	priv->rx_filer_enable = 1;
 	/* Enable most messages by default */
 	priv->msg_enable = (NETIF_MSG_IFUP << 1 ) - 1;
@@ -1189,8 +1193,9 @@ static int gfar_probe(struct platform_device *ofdev)
 	/* Print out the device info */
 	netdev_info(dev, "mac: %pM\n", dev->dev_addr);
 
-	/* Even more device info helps when determining which kernel */
-	/* provided which set of benchmarks. */
+	/* Even more device info helps when determining which kernel
+	 * provided which set of benchmarks.
+	 */
 	netdev_info(dev, "Running with NAPI enabled\n");
 	for (i = 0; i < priv->num_rx_queues; i++)
 		netdev_info(dev, "RX BD ring size for Q[%d]: %d\n",
@@ -1398,8 +1403,7 @@ static phy_interface_t gfar_get_interface(struct net_device *dev)
 		else {
 			phy_interface_t interface = priv->interface;
 
-			/*
-			 * This isn't autodetected right now, so it must
+			/* This isn't autodetected right now, so it must
 			 * be set by the device tree or platform code.
 			 */
 			if (interface == PHY_INTERFACE_MODE_RGMII_ID)
@@ -1453,8 +1457,7 @@ static int init_phy(struct net_device *dev)
 	return 0;
 }
 
-/*
- * Initialize TBI PHY interface for communicating with the
+/* Initialize TBI PHY interface for communicating with the
  * SERDES lynx PHY on the chip.  We communicate with this PHY
  * through the MDIO bus on each controller, treating it as a
  * "normal" PHY at the address found in the TBIPA register.  We assume
@@ -1479,8 +1482,7 @@ static void gfar_configure_serdes(struct net_device *dev)
 		return;
 	}
 
-	/*
-	 * If the link is already up, we must already be ok, and don't need to
+	/* If the link is already up, we must already be ok, and don't need to
 	 * configure and reset the TBI<->SerDes link.  Maybe U-Boot configured
 	 * everything for us?  Resetting it takes the link down and requires
 	 * several seconds for it to come back.
@@ -1554,15 +1556,13 @@ static int __gfar_is_rx_idle(struct gfar_private *priv)
 {
 	u32 res;
 
-	/*
-	 * Normaly TSEC should not hang on GRS commands, so we should
+	/* Normaly TSEC should not hang on GRS commands, so we should
 	 * actually wait for IEVENT_GRSC flag.
 	 */
 	if (likely(!gfar_has_errata(priv, GFAR_ERRATA_A002)))
 		return 0;
 
-	/*
-	 * Read the eTSEC register at offset 0xD1C. If bits 7-14 are
+	/* Read the eTSEC register at offset 0xD1C. If bits 7-14 are
 	 * the same as bits 23-30, the eTSEC Rx is assumed to be idle
 	 * and the Rx can be safely reset.
 	 */
@@ -1718,7 +1718,8 @@ static void free_skb_rx_queue(struct gfar_priv_rx_q *rx_queue)
 }
 
 /* If there are any tx skbs or rx skbs still around, free them.
- * Then free tx_skbuff and rx_skbuff */
+ * Then free tx_skbuff and rx_skbuff
+ */
 static void free_skb_resources(struct gfar_private *priv)
 {
 	struct gfar_priv_tx_q *tx_queue = NULL;
@@ -1827,10 +1828,12 @@ static int register_grp_irqs(struct gfar_priv_grp *grp)
 	int err;
 
 	/* If the device has multiple interrupts, register for
-	 * them.  Otherwise, only register for the one */
+	 * them.  Otherwise, only register for the one
+	 */
 	if (priv->device_flags & FSL_GIANFAR_DEV_HAS_MULTI_INTR) {
 		/* Install our interrupt handlers for Error,
-		 * Transmit, and Receive */
+		 * Transmit, and Receive
+		 */
 		if ((err = request_irq(grp->interruptError, gfar_error, 0,
 				grp->int_name_er,grp)) < 0) {
 			netif_err(priv, intr, dev, "Can't get IRQ %d\n",
@@ -1914,8 +1917,9 @@ irq_fail:
 	return err;
 }
 
-/* Called when something needs to use the ethernet device */
-/* Returns 0 for success. */
+/* Called when something needs to use the ethernet device
+ * Returns 0 for success.
+ */
 static int gfar_enet_open(struct net_device *dev)
 {
 	struct gfar_private *priv = netdev_priv(dev);
@@ -1970,8 +1974,9 @@ static inline void gfar_tx_checksum(struct sk_buff *skb, struct txfcb *fcb,
 	 */
 	flags = TXFCB_DEFAULT;
 
-	/* Tell the controller what the protocol is */
-	/* And provide the already calculated phcs */
+	/* Tell the controller what the protocol is
+	 * And provide the already calculated phcs
+	 */
 	if (ip_hdr(skb)->protocol == IPPROTO_UDP) {
 		flags |= TXFCB_UDP;
 		fcb->phcs = udp_hdr(skb)->check;
@@ -1981,7 +1986,8 @@ static inline void gfar_tx_checksum(struct sk_buff *skb, struct txfcb *fcb,
 	/* l3os is the distance between the start of the
 	 * frame (skb->data) and the start of the IP hdr.
 	 * l4os is the distance between the start of the
-	 * l3 hdr and the l4 hdr */
+	 * l3 hdr and the l4 hdr
+	 */
 	fcb->l3os = (u16)(skb_network_offset(skb) - fcb_length);
 	fcb->l4os = skb_network_header_len(skb);
 
@@ -2008,8 +2014,9 @@ static inline struct txbd8 *next_txbd(struct txbd8 *bdp, struct txbd8 *base,
 	return skip_txbd(bdp, 1, base, ring_size);
 }
 
-/* This is called by the kernel when a frame is ready for transmission. */
-/* It is pointed to by the dev->hard_start_xmit function pointer */
+/* This is called by the kernel when a frame is ready for transmission.
+ * It is pointed to by the dev->hard_start_xmit function pointer
+ */
 static int gfar_start_xmit(struct sk_buff *skb, struct net_device *dev)
 {
 	struct gfar_private *priv = netdev_priv(dev);
@@ -2024,8 +2031,7 @@ static int gfar_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	unsigned long flags;
 	unsigned int nr_frags, nr_txbds, length, fcb_length = GMAC_FCB_LEN;
 
-	/*
-	 * TOE=1 frames larger than 2500 bytes may see excess delays
+	/* TOE=1 frames larger than 2500 bytes may see excess delays
 	 * before start of transmission.
 	 */
 	if (unlikely(gfar_has_errata(priv, GFAR_ERRATA_76) &&
@@ -2177,8 +2183,7 @@ static int gfar_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	txbdp_start->bufPtr = dma_map_single(&priv->ofdev->dev, skb->data,
 			skb_headlen(skb), DMA_TO_DEVICE);
 
-	/*
-	 * If time stamping is requested one additional TxBD must be set up. The
+	/* If time stamping is requested one additional TxBD must be set up. The
 	 * first TxBD points to the FCB and must have a data length of
 	 * GMAC_FCB_LEN. The second TxBD points to the actual frame data with
 	 * the full frame length.
@@ -2194,8 +2199,7 @@ static int gfar_start_xmit(struct sk_buff *skb, struct net_device *dev)
 
 	netdev_tx_sent_queue(txq, skb->len);
 
-	/*
-	 * We can work in parallel with gfar_clean_tx_ring(), except
+	/* We can work in parallel with gfar_clean_tx_ring(), except
 	 * when modifying num_txbdfree. Note that we didn't grab the lock
 	 * when we were reading the num_txbdfree and checking for available
 	 * space, that's because outside of this function it can only grow,
@@ -2208,8 +2212,7 @@ static int gfar_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	 */
 	spin_lock_irqsave(&tx_queue->txlock, flags);
 
-	/*
-	 * The powerpc-specific eieio() is used, as wmb() has too strong
+	/* The powerpc-specific eieio() is used, as wmb() has too strong
 	 * semantics (it requires synchronization between cacheable and
 	 * uncacheable mappings, which eieio doesn't provide and which we
 	 * don't need), thus requiring a more expensive sync instruction.  At
@@ -2225,7 +2228,8 @@ static int gfar_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	tx_queue->tx_skbuff[tx_queue->skb_curtx] = skb;
 
 	/* Update the current skb pointer to the next entry we will use
-	 * (wrapping if necessary) */
+	 * (wrapping if necessary)
+	 */
 	tx_queue->skb_curtx = (tx_queue->skb_curtx + 1) &
 		TX_RING_MOD_MASK(tx_queue->tx_ring_size);
 
@@ -2235,7 +2239,8 @@ static int gfar_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	tx_queue->num_txbdfree -= (nr_txbds);
 
 	/* If the next BD still needs to be cleaned up, then the bds
-	   are full.  We need to tell the kernel to stop sending us stuff. */
+	 * are full.  We need to tell the kernel to stop sending us stuff.
+	 */
 	if (!tx_queue->num_txbdfree) {
 		netif_tx_stop_queue(txq);
 
@@ -2365,7 +2370,8 @@ static int gfar_change_mtu(struct net_device *dev, int new_mtu)
 	    INCREMENTAL_BUFFER_SIZE;
 
 	/* Only stop and start the controller if it isn't already
-	 * stopped, and we changed something */
+	 * stopped, and we changed something
+	 */
 	if ((oldsize != tempsize) && (dev->flags & IFF_UP))
 		stop_gfar(dev);
 
@@ -2378,7 +2384,8 @@ static int gfar_change_mtu(struct net_device *dev, int new_mtu)
 
 	/* If the mtu is larger than the max size for standard
 	 * ethernet frames (ie, a jumbo frame), then set maccfg2
-	 * to allow huge frames, and to check the length */
+	 * to allow huge frames, and to check the length
+	 */
 	tempval = gfar_read(&regs->maccfg2);
 
 	if (priv->rx_buffer_size > DEFAULT_RX_BUFFER_SIZE ||
@@ -2464,8 +2471,7 @@ static int gfar_clean_tx_ring(struct gfar_priv_tx_q *tx_queue)
 
 		frags = skb_shinfo(skb)->nr_frags;
 
-		/*
-		 * When time stamping, one additional TxBD must be freed.
+		/* When time stamping, one additional TxBD must be freed.
 		 * Also, we need to dma_unmap_single() the TxPAL.
 		 */
 		if (unlikely(skb_shinfo(skb)->tx_flags & SKBTX_IN_PROGRESS))
@@ -2516,8 +2522,7 @@ static int gfar_clean_tx_ring(struct gfar_priv_tx_q *tx_queue)
 
 		bytes_sent += skb->len;
 
-		/*
-		 * If there's room in the queue (limit it to rx_buffer_size)
+		/* If there's room in the queue (limit it to rx_buffer_size)
 		 * we add this skb back into the pool, if it's the right size
 		 */
 		if (skb_queue_len(&priv->rx_recycle) < rx_queue->rx_ring_size &&
@@ -2561,8 +2566,7 @@ static void gfar_schedule_cleanup(struct gfar_priv_grp *gfargrp)
 		gfar_write(&gfargrp->regs->imask, IMASK_RTX_DISABLED);
 		__napi_schedule(&gfargrp->napi);
 	} else {
-		/*
-		 * Clear IEVENT, so interrupts aren't called again
+		/* Clear IEVENT, so interrupts aren't called again
 		 * because of the packets that have already arrived.
 		 */
 		gfar_write(&gfargrp->regs->ievent, IEVENT_RTX_MASK);
@@ -2622,8 +2626,7 @@ static inline void count_errors(unsigned short status, struct net_device *dev)
 	struct net_device_stats *stats = &dev->stats;
 	struct gfar_extra_stats *estats = &priv->extra_stats;
 
-	/* If the packet was truncated, none of the other errors
-	 * matter */
+	/* If the packet was truncated, none of the other errors matter */
 	if (status & RXBD_TRUNCATED) {
 		stats->rx_length_errors++;
 
@@ -2664,7 +2667,8 @@ static inline void gfar_rx_checksum(struct sk_buff *skb, struct rxfcb *fcb)
 {
 	/* If valid headers were found, and valid sums
 	 * were verified, then we tell the kernel that no
-	 * checksumming is necessary.  Otherwise, it is */
+	 * checksumming is necessary.  Otherwise, it is [FIXME]
+	 */
 	if ((fcb->flags & RXFCB_CSUM_MASK) == (RXFCB_CIP | RXFCB_CTU))
 		skb->ip_summed = CHECKSUM_UNNECESSARY;
 	else
@@ -2672,8 +2676,7 @@ static inline void gfar_rx_checksum(struct sk_buff *skb, struct rxfcb *fcb)
 }
 
 
-/* gfar_process_frame() -- handle one incoming packet if skb
- * isn't NULL.  */
+/* gfar_process_frame() -- handle one incoming packet if skb isn't NULL. */
 static int gfar_process_frame(struct net_device *dev, struct sk_buff *skb,
 			      int amount_pull, struct napi_struct *napi)
 {
@@ -2685,8 +2688,9 @@ static int gfar_process_frame(struct net_device *dev, struct sk_buff *skb,
 	/* fcb is at the beginning if exists */
 	fcb = (struct rxfcb *)skb->data;
 
-	/* Remove the FCB from the skb */
-	/* Remove the padded bytes, if there are any */
+	/* Remove the FCB from the skb
+	 * Remove the padded bytes, if there are any
+	 */
 	if (amount_pull) {
 		skb_record_rx_queue(skb, fcb->rq);
 		skb_pull(skb, amount_pull);
@@ -2709,8 +2713,7 @@ static int gfar_process_frame(struct net_device *dev, struct sk_buff *skb,
 	/* Tell the skb what kind of packet this is */
 	skb->protocol = eth_type_trans(skb, dev);
 
-	/*
-	 * There's need to check for NETIF_F_HW_VLAN_RX here.
+	/* There's need to check for NETIF_F_HW_VLAN_RX here.
 	 * Even if vlan rx accel is disabled, on some chips
 	 * RXFCB_VLN is pseudo randomly set.
 	 */
@@ -2831,7 +2834,8 @@ static int gfar_poll(struct napi_struct *napi, int budget)
 	budget_per_queue = budget/num_queues;
 
 	/* Clear IEVENT, so interrupts aren't called again
-	 * because of the packets that have already arrived */
+	 * because of the packets that have already arrived
+	 */
 	gfar_write(&regs->ievent, IEVENT_RTX_MASK);
 
 	while (num_queues && left_over_budget) {
@@ -2869,8 +2873,9 @@ static int gfar_poll(struct napi_struct *napi, int budget)
 
 		gfar_write(&regs->imask, IMASK_DEFAULT);
 
-		/* If we are coalescing interrupts, update the timer */
-		/* Otherwise, clear it */
+		/* If we are coalescing interrupts, update the timer
+		 * Otherwise, clear it
+		 */
 		gfar_configure_coalescing(priv,
 				gfargrp->rx_bit_map, gfargrp->tx_bit_map);
 	}
@@ -2879,8 +2884,7 @@ static int gfar_poll(struct napi_struct *napi, int budget)
 }
 
 #ifdef CONFIG_NET_POLL_CONTROLLER
-/*
- * Polling 'interrupt' - used by things like netconsole to send skbs
+/* Polling 'interrupt' - used by things like netconsole to send skbs
  * without having to re-enable interrupts. It's not called while
  * the interrupt routine is executing.
  */
@@ -2957,7 +2961,8 @@ static void adjust_link(struct net_device *dev)
 		u32 ecntrl = gfar_read(&regs->ecntrl);
 
 		/* Now we make sure that we can be in full duplex mode.
-		 * If not, we operate in half-duplex mode. */
+		 * If not, we operate in half-duplex mode.
+		 */
 		if (phydev->duplex != priv->oldduplex) {
 			new_state = 1;
 			if (!(phydev->duplex))
@@ -2983,7 +2988,8 @@ static void adjust_link(struct net_device *dev)
 				    ((tempval & ~(MACCFG2_IF)) | MACCFG2_MII);
 
 				/* Reduced mode distinguishes
-				 * between 10 and 100 */
+				 * between 10 and 100
+				 */
 				if (phydev->speed == SPEED_100)
 					ecntrl |= ECNTRL_R100;
 				else
@@ -3022,7 +3028,8 @@ static void adjust_link(struct net_device *dev)
 /* Update the hash table based on the current list of multicast
  * addresses we subscribe to.  Also, change the promiscuity of
  * the device based on the flags (this function is called
- * whenever dev->flags is changed */
+ * whenever dev->flags is changed
+ */
 static void gfar_set_multi(struct net_device *dev)
 {
 	struct netdev_hw_addr *ha;
@@ -3084,7 +3091,8 @@ static void gfar_set_multi(struct net_device *dev)
 
 		/* If we have extended hash tables, we need to
 		 * clear the exact match registers to prepare for
-		 * setting them */
+		 * setting them
+		 */
 		if (priv->extended_hash) {
 			em_num = GFAR_EM_NUM + 1;
 			gfar_clear_exact_match(dev);
@@ -3110,7 +3118,8 @@ static void gfar_set_multi(struct net_device *dev)
 
 
 /* Clears each of the exact match registers to zero, so they
- * don't interfere with normal reception */
+ * don't interfere with normal reception
+ */
 static void gfar_clear_exact_match(struct net_device *dev)
 {
 	int idx;
@@ -3132,7 +3141,8 @@ static void gfar_clear_exact_match(struct net_device *dev)
  * hash index which gaddr register to use, and the 5 other bits
  * indicate which bit (assuming an IBM numbering scheme, which
  * for PowerPC (tm) is usually the case) in the register holds
- * the entry. */
+ * the entry.
+ */
 static void gfar_set_hash_for_addr(struct net_device *dev, u8 *addr)
 {
 	u32 tempval;
@@ -3164,8 +3174,9 @@ static void gfar_set_mac_for_addr(struct net_device *dev, int num,
 
 	macptr += num*2;
 
-	/* Now copy it into the mac registers backwards, cuz */
-	/* little endian is silly */
+	/* Now copy it into the mac registers backwards, cuz
+	 * little endian is silly
+	 */
 	for (idx = 0; idx < ETH_ALEN; idx++)
 		tmpbuf[ETH_ALEN - 1 - idx] = addr[idx];
 
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH v3 net-next 3/5] gianfar: various coding style and whitespace cleanups
From: Jan Ceuleers @ 2012-06-05 13:42 UTC (permalink / raw)
  To: David S. Miller; +Cc: Jiajun Wu, Joe Perches, netdev
In-Reply-To: <1338903735-24527-1-git-send-email-jan.ceuleers@computer.org>

Signed-off-by: Jan Ceuleers <jan.ceuleers@computer.org>
---
 drivers/net/ethernet/freescale/gianfar.c |  298 +++++++++++++++---------------
 1 file changed, 154 insertions(+), 144 deletions(-)

diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
index 3144560..ed0b136 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -113,7 +113,7 @@ static void gfar_timeout(struct net_device *dev);
 static int gfar_close(struct net_device *dev);
 struct sk_buff *gfar_new_skb(struct net_device *dev);
 static void gfar_new_rxbdp(struct gfar_priv_rx_q *rx_queue, struct rxbd8 *bdp,
-		struct sk_buff *skb);
+			   struct sk_buff *skb);
 static int gfar_set_mac_address(struct net_device *dev);
 static int gfar_change_mtu(struct net_device *dev, int new_mtu);
 static irqreturn_t gfar_error(int irq, void *dev_id);
@@ -265,8 +265,8 @@ static int gfar_alloc_skb_resources(struct net_device *ndev)
 		tx_queue->tx_bd_dma_base = addr;
 		tx_queue->dev = ndev;
 		/* enet DMA only understands physical addresses */
-		addr    += sizeof(struct txbd8) * tx_queue->tx_ring_size;
-		vaddr   += sizeof(struct txbd8) * tx_queue->tx_ring_size;
+		addr  += sizeof(struct txbd8) * tx_queue->tx_ring_size;
+		vaddr += sizeof(struct txbd8) * tx_queue->tx_ring_size;
 	}
 
 	/* Start the rx descriptor ring where the tx ring leaves off */
@@ -275,15 +275,16 @@ static int gfar_alloc_skb_resources(struct net_device *ndev)
 		rx_queue->rx_bd_base = vaddr;
 		rx_queue->rx_bd_dma_base = addr;
 		rx_queue->dev = ndev;
-		addr    += sizeof(struct rxbd8) * rx_queue->rx_ring_size;
-		vaddr   += sizeof(struct rxbd8) * rx_queue->rx_ring_size;
+		addr  += sizeof(struct rxbd8) * rx_queue->rx_ring_size;
+		vaddr += sizeof(struct rxbd8) * rx_queue->rx_ring_size;
 	}
 
 	/* Setup the skbuff rings */
 	for (i = 0; i < priv->num_tx_queues; i++) {
 		tx_queue = priv->tx_queue[i];
 		tx_queue->tx_skbuff = kmalloc(sizeof(*tx_queue->tx_skbuff) *
-				  tx_queue->tx_ring_size, GFP_KERNEL);
+					      tx_queue->tx_ring_size,
+					      GFP_KERNEL);
 		if (!tx_queue->tx_skbuff) {
 			netif_err(priv, ifup, ndev,
 				  "Could not allocate tx_skbuff\n");
@@ -297,7 +298,8 @@ static int gfar_alloc_skb_resources(struct net_device *ndev)
 	for (i = 0; i < priv->num_rx_queues; i++) {
 		rx_queue = priv->rx_queue[i];
 		rx_queue->rx_skbuff = kmalloc(sizeof(*rx_queue->rx_skbuff) *
-				  rx_queue->rx_ring_size, GFP_KERNEL);
+					      rx_queue->rx_ring_size,
+					      GFP_KERNEL);
 
 		if (!rx_queue->rx_skbuff) {
 			netif_err(priv, ifup, ndev,
@@ -326,15 +328,15 @@ static void gfar_init_tx_rx_base(struct gfar_private *priv)
 	int i;
 
 	baddr = &regs->tbase0;
-	for(i = 0; i < priv->num_tx_queues; i++) {
+	for (i = 0; i < priv->num_tx_queues; i++) {
 		gfar_write(baddr, priv->tx_queue[i]->tx_bd_dma_base);
-		baddr	+= 2;
+		baddr += 2;
 	}
 
 	baddr = &regs->rbase0;
-	for(i = 0; i < priv->num_rx_queues; i++) {
+	for (i = 0; i < priv->num_rx_queues; i++) {
 		gfar_write(baddr, priv->rx_queue[i]->rx_bd_dma_base);
-		baddr   += 2;
+		baddr += 2;
 	}
 }
 
@@ -430,12 +432,12 @@ static struct net_device_stats *gfar_get_stats(struct net_device *dev)
 
 	for (i = 0; i < priv->num_rx_queues; i++) {
 		rx_packets += priv->rx_queue[i]->stats.rx_packets;
-		rx_bytes += priv->rx_queue[i]->stats.rx_bytes;
+		rx_bytes   += priv->rx_queue[i]->stats.rx_bytes;
 		rx_dropped += priv->rx_queue[i]->stats.rx_dropped;
 	}
 
 	dev->stats.rx_packets = rx_packets;
-	dev->stats.rx_bytes = rx_bytes;
+	dev->stats.rx_bytes   = rx_bytes;
 	dev->stats.rx_dropped = rx_dropped;
 
 	for (i = 0; i < priv->num_tx_queues; i++) {
@@ -443,7 +445,7 @@ static struct net_device_stats *gfar_get_stats(struct net_device *dev)
 		tx_packets += priv->tx_queue[i]->stats.tx_packets;
 	}
 
-	dev->stats.tx_bytes = tx_bytes;
+	dev->stats.tx_bytes   = tx_bytes;
 	dev->stats.tx_packets = tx_packets;
 
 	return &dev->stats;
@@ -508,8 +510,8 @@ static bool gfar_is_vlan_on(struct gfar_private *priv)
 static inline int gfar_uses_fcb(struct gfar_private *priv)
 {
 	return gfar_is_vlan_on(priv) ||
-		(priv->ndev->features & NETIF_F_RXCSUM) ||
-		(priv->device_flags & FSL_GIANFAR_DEV_HAS_TIMER);
+	       (priv->ndev->features & NETIF_F_RXCSUM) ||
+	       (priv->device_flags & FSL_GIANFAR_DEV_HAS_TIMER);
 }
 
 static void free_tx_pointers(struct gfar_private *priv)
@@ -554,7 +556,7 @@ static void enable_napi(struct gfar_private *priv)
 }
 
 static int gfar_parse_group(struct device_node *np,
-		struct gfar_private *priv, const char *model)
+			    struct gfar_private *priv, const char *model)
 {
 	u32 *queue_mask;
 
@@ -580,15 +582,13 @@ static int gfar_parse_group(struct device_node *np,
 	priv->gfargrp[priv->num_grps].grp_id = priv->num_grps;
 	priv->gfargrp[priv->num_grps].priv = priv;
 	spin_lock_init(&priv->gfargrp[priv->num_grps].grplock);
-	if(priv->mode == MQ_MG_MODE) {
-		queue_mask = (u32 *)of_get_property(np,
-					"fsl,rx-bit-map", NULL);
-		priv->gfargrp[priv->num_grps].rx_bit_map =
-			queue_mask ?  *queue_mask :(DEFAULT_MAPPING >> priv->num_grps);
-		queue_mask = (u32 *)of_get_property(np,
-					"fsl,tx-bit-map", NULL);
-		priv->gfargrp[priv->num_grps].tx_bit_map =
-			queue_mask ? *queue_mask : (DEFAULT_MAPPING >> priv->num_grps);
+	if (priv->mode == MQ_MG_MODE) {
+		queue_mask = (u32 *)of_get_property(np, "fsl,rx-bit-map", NULL);
+		priv->gfargrp[priv->num_grps].rx_bit_map = queue_mask ?
+			*queue_mask : (DEFAULT_MAPPING >> priv->num_grps);
+		queue_mask = (u32 *)of_get_property(np, "fsl,tx-bit-map", NULL);
+		priv->gfargrp[priv->num_grps].tx_bit_map = queue_mask ?
+			*queue_mask : (DEFAULT_MAPPING >> priv->num_grps);
 	} else {
 		priv->gfargrp[priv->num_grps].rx_bit_map = 0xFF;
 		priv->gfargrp[priv->num_grps].tx_bit_map = 0xFF;
@@ -673,7 +673,7 @@ static int gfar_of_init(struct platform_device *ofdev, struct net_device **pdev)
 	} else {
 		priv->mode = SQ_SG_MODE;
 		err = gfar_parse_group(np, priv, model);
-		if(err)
+		if (err)
 			goto err_grp_init;
 	}
 
@@ -730,27 +730,27 @@ static int gfar_of_init(struct platform_device *ofdev, struct net_device **pdev)
 		priv->device_flags |= FSL_GIANFAR_DEV_HAS_BUF_STASHING;
 
 	mac_addr = of_get_mac_address(np);
+
 	if (mac_addr)
 		memcpy(dev->dev_addr, mac_addr, ETH_ALEN);
 
 	if (model && !strcasecmp(model, "TSEC"))
-		priv->device_flags =
-			FSL_GIANFAR_DEV_HAS_GIGABIT |
-			FSL_GIANFAR_DEV_HAS_COALESCE |
-			FSL_GIANFAR_DEV_HAS_RMON |
-			FSL_GIANFAR_DEV_HAS_MULTI_INTR;
+		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 |
-			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 |
-			FSL_GIANFAR_DEV_HAS_EXTENDED_HASH |
-			FSL_GIANFAR_DEV_HAS_TIMER;
+		priv->device_flags = FSL_GIANFAR_DEV_HAS_GIGABIT |
+				     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 |
+				     FSL_GIANFAR_DEV_HAS_EXTENDED_HASH |
+				     FSL_GIANFAR_DEV_HAS_TIMER;
 
 	ctype = of_get_property(np, "phy-connection-type", NULL);
 
@@ -781,7 +781,7 @@ err_grp_init:
 }
 
 static int gfar_hwtstamp_ioctl(struct net_device *netdev,
-			struct ifreq *ifr, int cmd)
+			       struct ifreq *ifr, int cmd)
 {
 	struct hwtstamp_config config;
 	struct gfar_private *priv = netdev_priv(netdev);
@@ -851,6 +851,7 @@ 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);
@@ -936,22 +937,22 @@ static void gfar_detect_errata(struct gfar_private *priv)
 
 	/* MPC8313 Rev 2.0 and higher; All MPC837x */
 	if ((pvr == 0x80850010 && mod == 0x80b0 && rev >= 0x0020) ||
-			(pvr == 0x80861010 && (mod & 0xfff9) == 0x80c0))
+	    (pvr == 0x80861010 && (mod & 0xfff9) == 0x80c0))
 		priv->errata |= GFAR_ERRATA_74;
 
 	/* MPC8313 and MPC837x all rev */
 	if ((pvr == 0x80850010 && mod == 0x80b0) ||
-			(pvr == 0x80861010 && (mod & 0xfff9) == 0x80c0))
+	    (pvr == 0x80861010 && (mod & 0xfff9) == 0x80c0))
 		priv->errata |= GFAR_ERRATA_76;
 
 	/* MPC8313 and MPC837x all rev */
 	if ((pvr == 0x80850010 && mod == 0x80b0) ||
-			(pvr == 0x80861010 && (mod & 0xfff9) == 0x80c0))
+	    (pvr == 0x80861010 && (mod & 0xfff9) == 0x80c0))
 		priv->errata |= GFAR_ERRATA_A002;
 
 	/* MPC8313 Rev < 2.0, MPC8548 rev 2.0 */
 	if ((pvr == 0x80850010 && mod == 0x80b0 && rev < 0x0020) ||
-			(pvr == 0x80210020 && mod == 0x8030 && rev == 0x0020))
+	    (pvr == 0x80210020 && mod == 0x8030 && rev == 0x0020))
 		priv->errata |= GFAR_ERRATA_12;
 
 	if (priv->errata)
@@ -1028,13 +1029,14 @@ static int gfar_probe(struct platform_device *ofdev)
 
 	/* Register for napi ...We are registering NAPI for each grp */
 	for (i = 0; i < priv->num_grps; i++)
-		netif_napi_add(dev, &priv->gfargrp[i].napi, gfar_poll, GFAR_DEV_WEIGHT);
+		netif_napi_add(dev, &priv->gfargrp[i].napi, gfar_poll,
+			       GFAR_DEV_WEIGHT);
 
 	if (priv->device_flags & FSL_GIANFAR_DEV_HAS_CSUM) {
 		dev->hw_features = NETIF_F_IP_CSUM | NETIF_F_SG |
-			NETIF_F_RXCSUM;
+				   NETIF_F_RXCSUM;
 		dev->features |= NETIF_F_IP_CSUM | NETIF_F_SG |
-			NETIF_F_RXCSUM | NETIF_F_HIGHDMA;
+				 NETIF_F_RXCSUM | NETIF_F_HIGHDMA;
 	}
 
 	if (priv->device_flags & FSL_GIANFAR_DEV_HAS_VLAN) {
@@ -1083,7 +1085,7 @@ static int gfar_probe(struct platform_device *ofdev)
 		priv->padding = 0;
 
 	if (dev->features & NETIF_F_IP_CSUM ||
-			priv->device_flags & FSL_GIANFAR_DEV_HAS_TIMER)
+	    priv->device_flags & FSL_GIANFAR_DEV_HAS_TIMER)
 		dev->needed_headroom = GMAC_FCB_LEN;
 
 	/* Program the isrg regs only if number of grps > 1 */
@@ -1103,10 +1105,10 @@ static int gfar_probe(struct platform_device *ofdev)
 	 * 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);
+		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,
@@ -1114,16 +1116,18 @@ static int gfar_probe(struct platform_device *ofdev)
 	 */
 	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->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->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);
@@ -1169,7 +1173,8 @@ static int gfar_probe(struct platform_device *ofdev)
 	}
 
 	device_init_wakeup(&dev->dev,
-		priv->device_flags & FSL_GIANFAR_DEV_HAS_MAGIC_PACKET);
+			   priv->device_flags &
+			   FSL_GIANFAR_DEV_HAS_MAGIC_PACKET);
 
 	/* fill out IRQ number and name fields */
 	for (i = 0; i < priv->num_grps; i++) {
@@ -1200,7 +1205,7 @@ static int gfar_probe(struct platform_device *ofdev)
 	for (i = 0; i < priv->num_rx_queues; i++)
 		netdev_info(dev, "RX BD ring size for Q[%d]: %d\n",
 			    i, priv->rx_queue[i]->rx_ring_size);
-	for(i = 0; i < priv->num_tx_queues; i++)
+	for (i = 0; i < priv->num_tx_queues; i++)
 		netdev_info(dev, "TX BD ring size for Q[%d]: %d\n",
 			    i, priv->tx_queue[i]->tx_ring_size);
 
@@ -1247,7 +1252,8 @@ static int gfar_suspend(struct device *dev)
 	u32 tempval;
 
 	int magic_packet = priv->wol_en &&
-		(priv->device_flags & FSL_GIANFAR_DEV_HAS_MAGIC_PACKET);
+			   (priv->device_flags &
+			    FSL_GIANFAR_DEV_HAS_MAGIC_PACKET);
 
 	netif_device_detach(ndev);
 
@@ -1299,7 +1305,8 @@ static int gfar_resume(struct device *dev)
 	unsigned long flags;
 	u32 tempval;
 	int magic_packet = priv->wol_en &&
-		(priv->device_flags & FSL_GIANFAR_DEV_HAS_MAGIC_PACKET);
+			   (priv->device_flags &
+			    FSL_GIANFAR_DEV_HAS_MAGIC_PACKET);
 
 	if (!netif_running(ndev)) {
 		netif_device_attach(ndev);
@@ -1398,8 +1405,9 @@ static phy_interface_t gfar_get_interface(struct net_device *dev)
 	}
 
 	if (ecntrl & ECNTRL_REDUCED_MODE) {
-		if (ecntrl & ECNTRL_REDUCED_MII_MODE)
+		if (ecntrl & ECNTRL_REDUCED_MII_MODE) {
 			return PHY_INTERFACE_MODE_RMII;
+		}
 		else {
 			phy_interface_t interface = priv->interface;
 
@@ -1494,11 +1502,12 @@ static void gfar_configure_serdes(struct net_device *dev)
 	phy_write(tbiphy, MII_TBICON, TBICON_CLK_SELECT);
 
 	phy_write(tbiphy, MII_ADVERTISE,
-			ADVERTISE_1000XFULL | ADVERTISE_1000XPAUSE |
-			ADVERTISE_1000XPSE_ASYM);
+		  ADVERTISE_1000XFULL | ADVERTISE_1000XPAUSE |
+		  ADVERTISE_1000XPSE_ASYM);
 
-	phy_write(tbiphy, MII_BMCR, BMCR_ANENABLE |
-			BMCR_ANRESTART | BMCR_FULLDPLX | BMCR_SPEED1000);
+	phy_write(tbiphy, MII_BMCR,
+		  BMCR_ANENABLE | BMCR_ANRESTART | BMCR_FULLDPLX |
+		  BMCR_SPEED1000);
 }
 
 static void init_registers(struct net_device *dev)
@@ -1594,8 +1603,8 @@ static void gfar_halt_nodisable(struct net_device *dev)
 	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))
-	    != (DMACTRL_GRS | DMACTRL_GTS)) {
+	if ((tempval & (DMACTRL_GRS | DMACTRL_GTS)) !=
+	    (DMACTRL_GRS | DMACTRL_GTS)) {
 		int ret;
 
 		tempval |= (DMACTRL_GRS | DMACTRL_GTS);
@@ -1660,7 +1669,7 @@ void stop_gfar(struct net_device *dev)
 	} else {
 		for (i = 0; i < priv->num_grps; i++)
 			free_irq(priv->gfargrp[i].interruptTransmit,
-					&priv->gfargrp[i]);
+				 &priv->gfargrp[i]);
 	}
 
 	free_skb_resources(priv);
@@ -1679,13 +1688,13 @@ static void free_skb_tx_queue(struct gfar_priv_tx_q *tx_queue)
 			continue;
 
 		dma_unmap_single(&priv->ofdev->dev, txbdp->bufPtr,
-				txbdp->length, DMA_TO_DEVICE);
+				 txbdp->length, DMA_TO_DEVICE);
 		txbdp->lstatus = 0;
 		for (j = 0; j < skb_shinfo(tx_queue->tx_skbuff[i])->nr_frags;
-				j++) {
+		     j++) {
 			txbdp++;
 			dma_unmap_page(&priv->ofdev->dev, txbdp->bufPtr,
-					txbdp->length, DMA_TO_DEVICE);
+				       txbdp->length, DMA_TO_DEVICE);
 		}
 		txbdp++;
 		dev_kfree_skb_any(tx_queue->tx_skbuff[i]);
@@ -1705,8 +1714,8 @@ static void free_skb_rx_queue(struct gfar_priv_rx_q *rx_queue)
 	for (i = 0; i < rx_queue->rx_ring_size; i++) {
 		if (rx_queue->rx_skbuff[i]) {
 			dma_unmap_single(&priv->ofdev->dev,
-					rxbdp->bufPtr, priv->rx_buffer_size,
-					DMA_FROM_DEVICE);
+					 rxbdp->bufPtr, priv->rx_buffer_size,
+					 DMA_FROM_DEVICE);
 			dev_kfree_skb_any(rx_queue->rx_skbuff[i]);
 			rx_queue->rx_skbuff[i] = NULL;
 		}
@@ -1729,24 +1738,25 @@ static void free_skb_resources(struct gfar_private *priv)
 	/* Go through all the buffer descriptors and free their data buffers */
 	for (i = 0; i < priv->num_tx_queues; i++) {
 		struct netdev_queue *txq;
+
 		tx_queue = priv->tx_queue[i];
 		txq = netdev_get_tx_queue(tx_queue->dev, tx_queue->qindex);
-		if(tx_queue->tx_skbuff)
+		if (tx_queue->tx_skbuff)
 			free_skb_tx_queue(tx_queue);
 		netdev_tx_reset_queue(txq);
 	}
 
 	for (i = 0; i < priv->num_rx_queues; i++) {
 		rx_queue = priv->rx_queue[i];
-		if(rx_queue->rx_skbuff)
+		if (rx_queue->rx_skbuff)
 			free_skb_rx_queue(rx_queue);
 	}
 
 	dma_free_coherent(&priv->ofdev->dev,
-			sizeof(struct txbd8) * priv->total_tx_ring_size +
-			sizeof(struct rxbd8) * priv->total_rx_ring_size,
-			priv->tx_queue[0]->tx_bd_base,
-			priv->tx_queue[0]->tx_bd_dma_base);
+			  sizeof(struct txbd8) * priv->total_tx_ring_size +
+			  sizeof(struct rxbd8) * priv->total_rx_ring_size,
+			  priv->tx_queue[0]->tx_bd_base,
+			  priv->tx_queue[0]->tx_bd_dma_base);
 	skb_queue_purge(&priv->rx_recycle);
 }
 
@@ -1785,7 +1795,7 @@ void gfar_start(struct net_device *dev)
 }
 
 void gfar_configure_coalescing(struct gfar_private *priv,
-	unsigned long tx_mask, unsigned long rx_mask)
+			       unsigned long tx_mask, unsigned long rx_mask)
 {
 	struct gfar __iomem *regs = priv->gfargrp[0].regs;
 	u32 __iomem *baddr;
@@ -1795,11 +1805,11 @@ void gfar_configure_coalescing(struct gfar_private *priv,
 	 * multiple queues, there's only single reg to program
 	 */
 	gfar_write(&regs->txic, 0);
-	if(likely(priv->tx_queue[0]->txcoalescing))
+	if (likely(priv->tx_queue[0]->txcoalescing))
 		gfar_write(&regs->txic, priv->tx_queue[0]->txic);
 
 	gfar_write(&regs->rxic, 0);
-	if(unlikely(priv->rx_queue[0]->rxcoalescing))
+	if (unlikely(priv->rx_queue[0]->rxcoalescing))
 		gfar_write(&regs->rxic, priv->rx_queue[0]->rxic);
 
 	if (priv->mode == MQ_MG_MODE) {
@@ -1834,8 +1844,8 @@ static int register_grp_irqs(struct gfar_priv_grp *grp)
 		/* Install our interrupt handlers for Error,
 		 * Transmit, and Receive
 		 */
-		if ((err = request_irq(grp->interruptError, gfar_error, 0,
-				grp->int_name_er,grp)) < 0) {
+		if ((err = request_irq(grp->interruptError, gfar_error,
+				       0, grp->int_name_er, grp)) < 0) {
 			netif_err(priv, intr, dev, "Can't get IRQ %d\n",
 				  grp->interruptError);
 
@@ -1843,21 +1853,21 @@ static int register_grp_irqs(struct gfar_priv_grp *grp)
 		}
 
 		if ((err = request_irq(grp->interruptTransmit, gfar_transmit,
-				0, grp->int_name_tx, grp)) < 0) {
+				       0, grp->int_name_tx, grp)) < 0) {
 			netif_err(priv, intr, dev, "Can't get IRQ %d\n",
 				  grp->interruptTransmit);
 			goto tx_irq_fail;
 		}
 
-		if ((err = request_irq(grp->interruptReceive, gfar_receive, 0,
-				grp->int_name_rx, grp)) < 0) {
+		if ((err = request_irq(grp->interruptReceive, gfar_receive,
+				       0, grp->int_name_rx, grp)) < 0) {
 			netif_err(priv, intr, dev, "Can't get IRQ %d\n",
 				  grp->interruptReceive);
 			goto rx_irq_fail;
 		}
 	} else {
-		if ((err = request_irq(grp->interruptTransmit, gfar_interrupt, 0,
-				grp->int_name_tx, grp)) < 0) {
+		if ((err = request_irq(grp->interruptTransmit, gfar_interrupt,
+				       0, grp->int_name_tx, grp)) < 0) {
 			netif_err(priv, intr, dev, "Can't get IRQ %d\n",
 				  grp->interruptTransmit);
 			goto err_irq_fail;
@@ -1964,7 +1974,7 @@ static inline struct txfcb *gfar_add_fcb(struct sk_buff *skb)
 }
 
 static inline void gfar_tx_checksum(struct sk_buff *skb, struct txfcb *fcb,
-		int fcb_length)
+				    int fcb_length)
 {
 	u8 flags = 0;
 
@@ -2001,7 +2011,7 @@ void inline gfar_tx_vlan(struct sk_buff *skb, struct txfcb *fcb)
 }
 
 static inline struct txbd8 *skip_txbd(struct txbd8 *bdp, int stride,
-			       struct txbd8 *base, int ring_size)
+				      struct txbd8 *base, int ring_size)
 {
 	struct txbd8 *new_bd = bdp + stride;
 
@@ -2009,7 +2019,7 @@ static inline struct txbd8 *skip_txbd(struct txbd8 *bdp, int stride,
 }
 
 static inline struct txbd8 *next_txbd(struct txbd8 *bdp, struct txbd8 *base,
-		int ring_size)
+				      int ring_size)
 {
 	return skip_txbd(bdp, 1, base, ring_size);
 }
@@ -2035,8 +2045,8 @@ static int gfar_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	 * before start of transmission.
 	 */
 	if (unlikely(gfar_has_errata(priv, GFAR_ERRATA_76) &&
-			skb->ip_summed == CHECKSUM_PARTIAL &&
-			skb->len > 2500)) {
+		     skb->ip_summed == CHECKSUM_PARTIAL &&
+		     skb->len > 2500)) {
 		int ret;
 
 		ret = skb_checksum_help(skb);
@@ -2052,16 +2062,16 @@ static int gfar_start_xmit(struct sk_buff *skb, struct net_device *dev)
 
 	/* check if time stamp should be generated */
 	if (unlikely(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP &&
-			priv->hwts_tx_en)) {
+		     priv->hwts_tx_en)) {
 		do_tstamp = 1;
 		fcb_length = GMAC_FCB_LEN + GMAC_TXPAL_LEN;
 	}
 
 	/* make space for additional header when fcb is needed */
 	if (((skb->ip_summed == CHECKSUM_PARTIAL) ||
-			vlan_tx_tag_present(skb) ||
-			unlikely(do_tstamp)) &&
-			(skb_headroom(skb) < fcb_length)) {
+	     vlan_tx_tag_present(skb) ||
+	     unlikely(do_tstamp)) &&
+	    (skb_headroom(skb) < fcb_length)) {
 		struct sk_buff *skb_new;
 
 		skb_new = skb_realloc_headroom(skb, fcb_length);
@@ -2105,12 +2115,12 @@ static int gfar_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	/* Time stamp insertion requires one additional TxBD */
 	if (unlikely(do_tstamp))
 		txbdp_tstamp = txbdp = next_txbd(txbdp, base,
-				tx_queue->tx_ring_size);
+						 tx_queue->tx_ring_size);
 
 	if (nr_frags == 0) {
 		if (unlikely(do_tstamp))
 			txbdp_tstamp->lstatus |= BD_LFLAG(TXBD_LAST |
-					TXBD_INTERRUPT);
+							  TXBD_INTERRUPT);
 		else
 			lstatus |= BD_LFLAG(TXBD_LAST | TXBD_INTERRUPT);
 	} else {
@@ -2122,7 +2132,7 @@ static int gfar_start_xmit(struct sk_buff *skb, struct net_device *dev)
 			length = skb_shinfo(skb)->frags[i].size;
 
 			lstatus = txbdp->lstatus | length |
-				BD_LFLAG(TXBD_READY);
+				  BD_LFLAG(TXBD_READY);
 
 			/* Handle the last BD specially */
 			if (i == nr_frags - 1)
@@ -2152,8 +2162,8 @@ static int gfar_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	if (CHECKSUM_PARTIAL == skb->ip_summed) {
 		fcb = gfar_add_fcb(skb);
 		/* as specified by errata */
-		if (unlikely(gfar_has_errata(priv, GFAR_ERRATA_12)
-			     && ((unsigned long)fcb % 0x20) > 0x18)) {
+		if (unlikely(gfar_has_errata(priv, GFAR_ERRATA_12) &&
+			     ((unsigned long)fcb % 0x20) > 0x18)) {
 			__skb_pull(skb, GMAC_FCB_LEN);
 			skb_checksum_help(skb);
 		} else {
@@ -2181,7 +2191,7 @@ static int gfar_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	}
 
 	txbdp_start->bufPtr = dma_map_single(&priv->ofdev->dev, skb->data,
-			skb_headlen(skb), DMA_TO_DEVICE);
+					     skb_headlen(skb), DMA_TO_DEVICE);
 
 	/* If time stamping is requested one additional TxBD must be set up. The
 	 * first TxBD points to the FCB and must have a data length of
@@ -2191,7 +2201,7 @@ static int gfar_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	if (unlikely(do_tstamp)) {
 		txbdp_tstamp->bufPtr = txbdp_start->bufPtr + fcb_length;
 		txbdp_tstamp->lstatus |= BD_LFLAG(TXBD_READY) |
-				(skb_headlen(skb) - fcb_length);
+					 (skb_headlen(skb) - fcb_length);
 		lstatus |= BD_LFLAG(TXBD_CRC | TXBD_READY) | GMAC_FCB_LEN;
 	} else {
 		lstatus |= BD_LFLAG(TXBD_CRC | TXBD_READY) | skb_headlen(skb);
@@ -2231,7 +2241,7 @@ static int gfar_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	 * (wrapping if necessary)
 	 */
 	tx_queue->skb_curtx = (tx_queue->skb_curtx + 1) &
-		TX_RING_MOD_MASK(tx_queue->tx_ring_size);
+			      TX_RING_MOD_MASK(tx_queue->tx_ring_size);
 
 	tx_queue->cur_tx = next_txbd(txbdp, base, tx_queue->tx_ring_size);
 
@@ -2365,9 +2375,8 @@ static int gfar_change_mtu(struct net_device *dev, int new_mtu)
 
 	frame_size += priv->padding;
 
-	tempsize =
-	    (frame_size & ~(INCREMENTAL_BUFFER_SIZE - 1)) +
-	    INCREMENTAL_BUFFER_SIZE;
+	tempsize = (frame_size & ~(INCREMENTAL_BUFFER_SIZE - 1)) +
+		   INCREMENTAL_BUFFER_SIZE;
 
 	/* Only stop and start the controller if it isn't already
 	 * stopped, and we changed something
@@ -2389,7 +2398,7 @@ static int gfar_change_mtu(struct net_device *dev, int new_mtu)
 	tempval = gfar_read(&regs->maccfg2);
 
 	if (priv->rx_buffer_size > DEFAULT_RX_BUFFER_SIZE ||
-			gfar_has_errata(priv, GFAR_ERRATA_74))
+	    gfar_has_errata(priv, GFAR_ERRATA_74))
 		tempval |= (MACCFG2_HUGEFRAME | MACCFG2_LENGTHCHECK);
 	else
 		tempval &= ~(MACCFG2_HUGEFRAME | MACCFG2_LENGTHCHECK);
@@ -2410,7 +2419,7 @@ static int gfar_change_mtu(struct net_device *dev, int new_mtu)
 static void gfar_reset_task(struct work_struct *work)
 {
 	struct gfar_private *priv = container_of(work, struct gfar_private,
-			reset_task);
+						 reset_task);
 	struct net_device *dev = priv->ndev;
 
 	if (dev->flags & IFF_UP) {
@@ -2437,7 +2446,7 @@ static void gfar_align_skb(struct sk_buff *skb)
 	 * as many bytes as needed to align the data properly
 	 */
 	skb_reserve(skb, RXBUF_ALIGNMENT -
-		(((unsigned long) skb->data) & (RXBUF_ALIGNMENT - 1)));
+		    (((unsigned long) skb->data) & (RXBUF_ALIGNMENT - 1)));
 }
 
 /* Interrupt Handler for Transmit complete */
@@ -2485,7 +2494,7 @@ static int gfar_clean_tx_ring(struct gfar_priv_tx_q *tx_queue)
 
 		/* Only clean completed frames */
 		if ((lstatus & BD_LFLAG(TXBD_READY)) &&
-				(lstatus & BD_LENGTH_MASK))
+		    (lstatus & BD_LENGTH_MASK))
 			break;
 
 		if (unlikely(skb_shinfo(skb)->tx_flags & SKBTX_IN_PROGRESS)) {
@@ -2495,11 +2504,12 @@ static int gfar_clean_tx_ring(struct gfar_priv_tx_q *tx_queue)
 			buflen = bdp->length;
 
 		dma_unmap_single(&priv->ofdev->dev, bdp->bufPtr,
-				buflen, DMA_TO_DEVICE);
+				 buflen, DMA_TO_DEVICE);
 
 		if (unlikely(skb_shinfo(skb)->tx_flags & SKBTX_IN_PROGRESS)) {
 			struct skb_shared_hwtstamps shhwtstamps;
 			u64 *ns = (u64*) (((u32)skb->data + 0x10) & ~0x7);
+
 			memset(&shhwtstamps, 0, sizeof(shhwtstamps));
 			shhwtstamps.hwtstamp = ns_to_ktime(*ns);
 			skb_pull(skb, GMAC_FCB_LEN + GMAC_TXPAL_LEN);
@@ -2512,10 +2522,8 @@ static int gfar_clean_tx_ring(struct gfar_priv_tx_q *tx_queue)
 		bdp = next_txbd(bdp, base, tx_ring_size);
 
 		for (i = 0; i < frags; i++) {
-			dma_unmap_page(&priv->ofdev->dev,
-					bdp->bufPtr,
-					bdp->length,
-					DMA_TO_DEVICE);
+			dma_unmap_page(&priv->ofdev->dev, bdp->bufPtr,
+				       bdp->length, DMA_TO_DEVICE);
 			bdp->lstatus &= BD_LFLAG(TXBD_WRAP);
 			bdp = next_txbd(bdp, base, tx_ring_size);
 		}
@@ -2526,8 +2534,8 @@ static int gfar_clean_tx_ring(struct gfar_priv_tx_q *tx_queue)
 		 * we add this skb back into the pool, if it's the right size
 		 */
 		if (skb_queue_len(&priv->rx_recycle) < rx_queue->rx_ring_size &&
-				skb_recycle_check(skb, priv->rx_buffer_size +
-					RXBUF_ALIGNMENT)) {
+		    skb_recycle_check(skb, priv->rx_buffer_size +
+				      RXBUF_ALIGNMENT)) {
 			gfar_align_skb(skb);
 			skb_queue_head(&priv->rx_recycle, skb);
 		} else
@@ -2536,7 +2544,7 @@ static int gfar_clean_tx_ring(struct gfar_priv_tx_q *tx_queue)
 		tx_queue->tx_skbuff[skb_dirtytx] = NULL;
 
 		skb_dirtytx = (skb_dirtytx + 1) &
-			TX_RING_MOD_MASK(tx_ring_size);
+			      TX_RING_MOD_MASK(tx_ring_size);
 
 		howmany++;
 		spin_lock_irqsave(&tx_queue->txlock, flags);
@@ -2583,7 +2591,7 @@ static irqreturn_t gfar_transmit(int irq, void *grp_id)
 }
 
 static void gfar_new_rxbdp(struct gfar_priv_rx_q *rx_queue, struct rxbd8 *bdp,
-		struct sk_buff *skb)
+			   struct sk_buff *skb)
 {
 	struct net_device *dev = rx_queue->dev;
 	struct gfar_private *priv = netdev_priv(dev);
@@ -2700,6 +2708,7 @@ static int gfar_process_frame(struct net_device *dev, struct sk_buff *skb,
 	if (priv->hwts_rx_en) {
 		struct skb_shared_hwtstamps *shhwtstamps = skb_hwtstamps(skb);
 		u64 *ns = (u64 *) skb->data;
+
 		memset(shhwtstamps, 0, sizeof(*shhwtstamps));
 		shhwtstamps->hwtstamp = ns_to_ktime(*ns);
 	}
@@ -2752,6 +2761,7 @@ int gfar_clean_rx_ring(struct gfar_priv_rx_q *rx_queue, int rx_work_limit)
 
 	while (!((bdp->status & RXBD_EMPTY) || (--rx_work_limit < 0))) {
 		struct sk_buff *newskb;
+
 		rmb();
 
 		/* Add another skb for the future */
@@ -2760,15 +2770,15 @@ int gfar_clean_rx_ring(struct gfar_priv_rx_q *rx_queue, int rx_work_limit)
 		skb = rx_queue->rx_skbuff[rx_queue->skb_currx];
 
 		dma_unmap_single(&priv->ofdev->dev, bdp->bufPtr,
-				priv->rx_buffer_size, DMA_FROM_DEVICE);
+				 priv->rx_buffer_size, DMA_FROM_DEVICE);
 
 		if (unlikely(!(bdp->status & RXBD_ERR) &&
-				bdp->length > priv->rx_buffer_size))
+			     bdp->length > priv->rx_buffer_size))
 			bdp->status = RXBD_LARGE;
 
 		/* We drop the frame if we failed to allocate a new buffer */
 		if (unlikely(!newskb || !(bdp->status & RXBD_LAST) ||
-				 bdp->status & RXBD_ERR)) {
+			     bdp->status & RXBD_ERR)) {
 			count_errors(bdp->status, dev);
 
 			if (unlikely(!newskb))
@@ -2787,7 +2797,7 @@ int gfar_clean_rx_ring(struct gfar_priv_rx_q *rx_queue, int rx_work_limit)
 				rx_queue->stats.rx_bytes += pkt_len;
 				skb_record_rx_queue(skb, rx_queue->qindex);
 				gfar_process_frame(dev, skb, amount_pull,
-						&rx_queue->grp->napi);
+						   &rx_queue->grp->napi);
 
 			} else {
 				netif_warn(priv, rx_err, dev, "Missing skb!\n");
@@ -2806,9 +2816,8 @@ int gfar_clean_rx_ring(struct gfar_priv_rx_q *rx_queue, int rx_work_limit)
 		bdp = next_bd(bdp, base, rx_queue->rx_ring_size);
 
 		/* update to point at the next skb */
-		rx_queue->skb_currx =
-		    (rx_queue->skb_currx + 1) &
-		    RX_RING_MOD_MASK(rx_queue->rx_ring_size);
+		rx_queue->skb_currx = (rx_queue->skb_currx + 1) &
+				      RX_RING_MOD_MASK(rx_queue->rx_ring_size);
 	}
 
 	/* Update the current rxbd pointer to be the next one */
@@ -2819,8 +2828,8 @@ int gfar_clean_rx_ring(struct gfar_priv_rx_q *rx_queue, int rx_work_limit)
 
 static int gfar_poll(struct napi_struct *napi, int budget)
 {
-	struct gfar_priv_grp *gfargrp = container_of(napi,
-			struct gfar_priv_grp, napi);
+	struct gfar_priv_grp *gfargrp =
+		container_of(napi, struct gfar_priv_grp, napi);
 	struct gfar_private *priv = gfargrp->priv;
 	struct gfar __iomem *regs = gfargrp->regs;
 	struct gfar_priv_tx_q *tx_queue = NULL;
@@ -2839,7 +2848,6 @@ static int gfar_poll(struct napi_struct *napi, int budget)
 	gfar_write(&regs->ievent, IEVENT_RTX_MASK);
 
 	while (num_queues && left_over_budget) {
-
 		budget_per_queue = left_over_budget/num_queues;
 		left_over_budget = 0;
 
@@ -2850,12 +2858,13 @@ static int gfar_poll(struct napi_struct *napi, int budget)
 			tx_queue = priv->tx_queue[rx_queue->qindex];
 
 			tx_cleaned += gfar_clean_tx_ring(tx_queue);
-			rx_cleaned_per_queue = gfar_clean_rx_ring(rx_queue,
-							budget_per_queue);
+			rx_cleaned_per_queue =
+				gfar_clean_rx_ring(rx_queue, budget_per_queue);
 			rx_cleaned += rx_cleaned_per_queue;
-			if(rx_cleaned_per_queue < budget_per_queue) {
+			if (rx_cleaned_per_queue < budget_per_queue) {
 				left_over_budget = left_over_budget +
-					(budget_per_queue - rx_cleaned_per_queue);
+					(budget_per_queue -
+					 rx_cleaned_per_queue);
 				set_bit(i, &serviced_queues);
 				num_queues--;
 			}
@@ -2876,8 +2885,8 @@ static int gfar_poll(struct napi_struct *napi, int budget)
 		/* If we are coalescing interrupts, update the timer
 		 * Otherwise, clear it
 		 */
-		gfar_configure_coalescing(priv,
-				gfargrp->rx_bit_map, gfargrp->tx_bit_map);
+		gfar_configure_coalescing(priv, gfargrp->rx_bit_map,
+					  gfargrp->tx_bit_map);
 	}
 
 	return rx_cleaned;
@@ -2900,7 +2909,7 @@ static void gfar_netpoll(struct net_device *dev)
 			disable_irq(priv->gfargrp[i].interruptReceive);
 			disable_irq(priv->gfargrp[i].interruptError);
 			gfar_interrupt(priv->gfargrp[i].interruptTransmit,
-						&priv->gfargrp[i]);
+				       &priv->gfargrp[i]);
 			enable_irq(priv->gfargrp[i].interruptError);
 			enable_irq(priv->gfargrp[i].interruptReceive);
 			enable_irq(priv->gfargrp[i].interruptTransmit);
@@ -2909,7 +2918,7 @@ static void gfar_netpoll(struct net_device *dev)
 		for (i = 0; i < priv->num_grps; i++) {
 			disable_irq(priv->gfargrp[i].interruptTransmit);
 			gfar_interrupt(priv->gfargrp[i].interruptTransmit,
-						&priv->gfargrp[i]);
+				       &priv->gfargrp[i]);
 			enable_irq(priv->gfargrp[i].interruptTransmit);
 		}
 	}
@@ -3125,7 +3134,7 @@ static void gfar_clear_exact_match(struct net_device *dev)
 	int idx;
 	static const u8 zero_arr[ETH_ALEN] = {0, 0, 0, 0, 0, 0};
 
-	for(idx = 1;idx < GFAR_EM_NUM + 1;idx++)
+	for (idx = 1; idx < GFAR_EM_NUM + 1; idx++)
 		gfar_set_mac_for_addr(dev, idx, zero_arr);
 }
 
@@ -3208,7 +3217,8 @@ static irqreturn_t gfar_error(int irq, void *grp_id)
 
 	/* Hmm... */
 	if (netif_msg_rx_err(priv) || netif_msg_tx_err(priv))
-		netdev_dbg(dev, "error interrupt (ievent=0x%08x imask=0x%08x)\n",
+		netdev_dbg(dev,
+			   "error interrupt (ievent=0x%08x imask=0x%08x)\n",
 			   events, gfar_read(&regs->imask));
 
 	/* Update the error counters */
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH v3 net-next 5/5] gianfar_ethtool: coding style and whitespace cleanups
From: Jan Ceuleers @ 2012-06-05 13:42 UTC (permalink / raw)
  To: David S. Miller; +Cc: Jiajun Wu, Joe Perches, netdev
In-Reply-To: <1338903735-24527-1-git-send-email-jan.ceuleers@computer.org>

Signed-off-by: Jan Ceuleers <jan.ceuleers@computer.org>
---
 drivers/net/ethernet/freescale/gianfar_ethtool.c |  420 ++++++++++++----------
 1 file changed, 224 insertions(+), 196 deletions(-)

diff --git a/drivers/net/ethernet/freescale/gianfar_ethtool.c b/drivers/net/ethernet/freescale/gianfar_ethtool.c
index 8a02557..8971921 100644
--- a/drivers/net/ethernet/freescale/gianfar_ethtool.c
+++ b/drivers/net/ethernet/freescale/gianfar_ethtool.c
@@ -46,18 +46,24 @@
 #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);
+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,
-		     u64 * buf);
+			    u64 *buf);
 static void gfar_gstrings(struct net_device *dev, u32 stringset, u8 * buf);
-static int gfar_gcoalesce(struct net_device *dev, struct ethtool_coalesce *cvals);
-static int gfar_scoalesce(struct net_device *dev, struct ethtool_coalesce *cvals);
-static void gfar_gringparam(struct net_device *dev, struct ethtool_ringparam *rvals);
-static int gfar_sringparam(struct net_device *dev, struct ethtool_ringparam *rvals);
-static void gfar_gdrvinfo(struct net_device *dev, struct ethtool_drvinfo *drvinfo);
+static int gfar_gcoalesce(struct net_device *dev,
+			  struct ethtool_coalesce *cvals);
+static int gfar_scoalesce(struct net_device *dev,
+			  struct ethtool_coalesce *cvals);
+static void gfar_gringparam(struct net_device *dev,
+			    struct ethtool_ringparam *rvals);
+static int gfar_sringparam(struct net_device *dev,
+			   struct ethtool_ringparam *rvals);
+static void gfar_gdrvinfo(struct net_device *dev,
+			  struct ethtool_drvinfo *drvinfo);
 
 static const char stat_gstrings[][ETH_GSTRING_LEN] = {
 	"rx-dropped-by-kernel",
@@ -130,14 +136,15 @@ static void gfar_gstrings(struct net_device *dev, u32 stringset, u8 * buf)
 		memcpy(buf, stat_gstrings, GFAR_STATS_LEN * ETH_GSTRING_LEN);
 	else
 		memcpy(buf, stat_gstrings,
-				GFAR_EXTRA_STATS_LEN * ETH_GSTRING_LEN);
+		       GFAR_EXTRA_STATS_LEN * ETH_GSTRING_LEN);
 }
 
 /* Fill in an array of 64-bit statistics from various sources.
  * This array will be appended to the end of the ethtool_stats
  * structure, and returned to user space
  */
-static void gfar_fill_stats(struct net_device *dev, struct ethtool_stats *dummy, u64 * buf)
+static void gfar_fill_stats(struct net_device *dev, struct ethtool_stats *dummy,
+			    u64 *buf)
 {
 	int i;
 	struct gfar_private *priv = netdev_priv(dev);
@@ -174,8 +181,8 @@ static int gfar_sset_count(struct net_device *dev, int sset)
 }
 
 /* Fills in the drvinfo structure with some basic info */
-static void gfar_gdrvinfo(struct net_device *dev, struct
-	      ethtool_drvinfo *drvinfo)
+static void gfar_gdrvinfo(struct net_device *dev,
+			  struct ethtool_drvinfo *drvinfo)
 {
 	strncpy(drvinfo->driver, DRV_NAME, GFAR_INFOSTR_LEN);
 	strncpy(drvinfo->version, gfar_driver_version, GFAR_INFOSTR_LEN);
@@ -226,7 +233,8 @@ static int gfar_reglen(struct net_device *dev)
 }
 
 /* Return a dump of the GFAR register space */
-static void gfar_get_regs(struct net_device *dev, struct ethtool_regs *regs, void *regbuf)
+static void gfar_get_regs(struct net_device *dev, struct ethtool_regs *regs,
+			  void *regbuf)
 {
 	int i;
 	struct gfar_private *priv = netdev_priv(dev);
@@ -239,7 +247,8 @@ static void gfar_get_regs(struct net_device *dev, struct ethtool_regs *regs, voi
 
 /* Convert microseconds to ethernet clock ticks, which changes
  * depending on what speed the controller is running at */
-static unsigned int gfar_usecs2ticks(struct gfar_private *priv, unsigned int usecs)
+static unsigned int gfar_usecs2ticks(struct gfar_private *priv,
+				     unsigned int usecs)
 {
 	unsigned int count;
 
@@ -263,7 +272,8 @@ static unsigned int gfar_usecs2ticks(struct gfar_private *priv, unsigned int use
 }
 
 /* Convert ethernet clock ticks to microseconds */
-static unsigned int gfar_ticks2usecs(struct gfar_private *priv, unsigned int ticks)
+static unsigned int gfar_ticks2usecs(struct gfar_private *priv,
+				     unsigned int ticks)
 {
 	unsigned int count;
 
@@ -288,7 +298,8 @@ static unsigned int gfar_ticks2usecs(struct gfar_private *priv, unsigned int tic
 
 /* Get the coalescing parameters, and put them in the cvals
  * structure.  */
-static int gfar_gcoalesce(struct net_device *dev, struct ethtool_coalesce *cvals)
+static int gfar_gcoalesce(struct net_device *dev,
+			  struct ethtool_coalesce *cvals)
 {
 	struct gfar_private *priv = netdev_priv(dev);
 	struct gfar_priv_rx_q *rx_queue = NULL;
@@ -353,7 +364,8 @@ static int gfar_gcoalesce(struct net_device *dev, struct ethtool_coalesce *cvals
  * Both cvals->*_usecs and cvals->*_frames have to be > 0
  * in order for coalescing to be active
  */
-static int gfar_scoalesce(struct net_device *dev, struct ethtool_coalesce *cvals)
+static int gfar_scoalesce(struct net_device *dev,
+			  struct ethtool_coalesce *cvals)
 {
 	struct gfar_private *priv = netdev_priv(dev);
 	int i = 0;
@@ -364,7 +376,8 @@ static int gfar_scoalesce(struct net_device *dev, struct ethtool_coalesce *cvals
 	/* Set up rx coalescing */
 	/* As of now, we will enable/disable coalescing for all
 	 * queues together in case of eTSEC2, this will be modified
-	 * along with the ethtool interface */
+	 * along with the ethtool interface
+	 */
 	if ((cvals->rx_coalesce_usecs == 0) ||
 	    (cvals->rx_max_coalesced_frames == 0)) {
 		for (i = 0; i < priv->num_rx_queues; i++)
@@ -433,7 +446,8 @@ static int gfar_scoalesce(struct net_device *dev, struct ethtool_coalesce *cvals
 /* Fills in rvals with the current ring parameters.  Currently,
  * rx, rx_mini, and rx_jumbo rings are the same size, as mini and
  * jumbo are ignored by the driver */
-static void gfar_gringparam(struct net_device *dev, struct ethtool_ringparam *rvals)
+static void gfar_gringparam(struct net_device *dev,
+			    struct ethtool_ringparam *rvals)
 {
 	struct gfar_private *priv = netdev_priv(dev);
 	struct gfar_priv_tx_q *tx_queue = NULL;
@@ -459,8 +473,10 @@ static void gfar_gringparam(struct net_device *dev, struct ethtool_ringparam *rv
 /* 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. */
-static int gfar_sringparam(struct net_device *dev, struct ethtool_ringparam *rvals)
+ * the rings.
+ */
+static int gfar_sringparam(struct net_device *dev,
+			   struct ethtool_ringparam *rvals)
 {
 	struct gfar_private *priv = netdev_priv(dev);
 	int err = 0, i = 0;
@@ -486,7 +502,8 @@ static int gfar_sringparam(struct net_device *dev, struct ethtool_ringparam *rva
 		unsigned long flags;
 
 		/* Halt TX and RX, and process the frames which
-		 * have already been received */
+		 * have already been received
+		 */
 		local_irq_save(flags);
 		lock_tx_qs(priv);
 		lock_rx_qs(priv);
@@ -499,7 +516,7 @@ static int gfar_sringparam(struct net_device *dev, struct ethtool_ringparam *rva
 
 		for (i = 0; i < priv->num_rx_queues; i++)
 			gfar_clean_rx_ring(priv->rx_queue[i],
-					priv->rx_queue[i]->rx_ring_size);
+					   priv->rx_queue[i]->rx_ring_size);
 
 		/* Now we take down the rings to rebuild them */
 		stop_gfar(dev);
@@ -509,7 +526,8 @@ static int gfar_sringparam(struct net_device *dev, struct ethtool_ringparam *rva
 	for (i = 0; i < priv->num_rx_queues; i++) {
 		priv->rx_queue[i]->rx_ring_size = rvals->rx_pending;
 		priv->tx_queue[i]->tx_ring_size = rvals->tx_pending;
-		priv->tx_queue[i]->num_txbdfree = priv->tx_queue[i]->tx_ring_size;
+		priv->tx_queue[i]->num_txbdfree =
+			priv->tx_queue[i]->tx_ring_size;
 	}
 
 	/* Rebuild the rings with the new size */
@@ -535,7 +553,8 @@ int gfar_set_features(struct net_device *dev, netdev_features_t features)
 
 	if (dev->flags & IFF_UP) {
 		/* Halt TX and RX, and process the frames which
-		 * have already been received */
+		 * have already been received
+		 */
 		local_irq_save(flags);
 		lock_tx_qs(priv);
 		lock_rx_qs(priv);
@@ -548,7 +567,7 @@ int gfar_set_features(struct net_device *dev, netdev_features_t features)
 
 		for (i = 0; i < priv->num_rx_queues; i++)
 			gfar_clean_rx_ring(priv->rx_queue[i],
-					priv->rx_queue[i]->rx_ring_size);
+					   priv->rx_queue[i]->rx_ring_size);
 
 		/* Now we take down the rings to rebuild them */
 		stop_gfar(dev);
@@ -564,12 +583,14 @@ int gfar_set_features(struct net_device *dev, netdev_features_t features)
 static uint32_t gfar_get_msglevel(struct net_device *dev)
 {
 	struct gfar_private *priv = netdev_priv(dev);
+
 	return priv->msg_enable;
 }
 
 static void gfar_set_msglevel(struct net_device *dev, uint32_t data)
 {
 	struct gfar_private *priv = netdev_priv(dev);
+
 	priv->msg_enable = data;
 }
 
@@ -614,14 +635,14 @@ static void ethflow_to_filer_rules (struct gfar_private *priv, u64 ethflow)
 
 	if (ethflow & RXH_L2DA) {
 		fcr = RQFCR_PID_DAH |RQFCR_CMP_NOMATCH |
-			RQFCR_HASH | RQFCR_AND | RQFCR_HASHTBL_0;
+		      RQFCR_HASH | RQFCR_AND | RQFCR_HASHTBL_0;
 		priv->ftp_rqfpr[priv->cur_filer_idx] = fpr;
 		priv->ftp_rqfcr[priv->cur_filer_idx] = fcr;
 		gfar_write_filer(priv, priv->cur_filer_idx, fcr, fpr);
 		priv->cur_filer_idx = priv->cur_filer_idx - 1;
 
 		fcr = RQFCR_PID_DAL | RQFCR_AND | RQFCR_CMP_NOMATCH |
-				RQFCR_HASH | RQFCR_AND | RQFCR_HASHTBL_0;
+		      RQFCR_HASH | RQFCR_AND | RQFCR_HASHTBL_0;
 		priv->ftp_rqfpr[priv->cur_filer_idx] = fpr;
 		priv->ftp_rqfcr[priv->cur_filer_idx] = fcr;
 		gfar_write_filer(priv, priv->cur_filer_idx, fcr, fpr);
@@ -630,7 +651,7 @@ static void ethflow_to_filer_rules (struct gfar_private *priv, u64 ethflow)
 
 	if (ethflow & RXH_VLAN) {
 		fcr = RQFCR_PID_VID | RQFCR_CMP_NOMATCH | RQFCR_HASH |
-				RQFCR_AND | RQFCR_HASHTBL_0;
+		      RQFCR_AND | RQFCR_HASHTBL_0;
 		gfar_write_filer(priv, priv->cur_filer_idx, fcr, fpr);
 		priv->ftp_rqfpr[priv->cur_filer_idx] = fpr;
 		priv->ftp_rqfcr[priv->cur_filer_idx] = fcr;
@@ -639,7 +660,7 @@ static void ethflow_to_filer_rules (struct gfar_private *priv, u64 ethflow)
 
 	if (ethflow & RXH_IP_SRC) {
 		fcr = RQFCR_PID_SIA | RQFCR_CMP_NOMATCH | RQFCR_HASH |
-			RQFCR_AND | RQFCR_HASHTBL_0;
+		      RQFCR_AND | RQFCR_HASHTBL_0;
 		priv->ftp_rqfpr[priv->cur_filer_idx] = fpr;
 		priv->ftp_rqfcr[priv->cur_filer_idx] = fcr;
 		gfar_write_filer(priv, priv->cur_filer_idx, fcr, fpr);
@@ -648,7 +669,7 @@ static void ethflow_to_filer_rules (struct gfar_private *priv, u64 ethflow)
 
 	if (ethflow & (RXH_IP_DST)) {
 		fcr = RQFCR_PID_DIA | RQFCR_CMP_NOMATCH | RQFCR_HASH |
-			RQFCR_AND | RQFCR_HASHTBL_0;
+		      RQFCR_AND | RQFCR_HASHTBL_0;
 		priv->ftp_rqfpr[priv->cur_filer_idx] = fpr;
 		priv->ftp_rqfcr[priv->cur_filer_idx] = fcr;
 		gfar_write_filer(priv, priv->cur_filer_idx, fcr, fpr);
@@ -657,7 +678,7 @@ static void ethflow_to_filer_rules (struct gfar_private *priv, u64 ethflow)
 
 	if (ethflow & RXH_L3_PROTO) {
 		fcr = RQFCR_PID_L4P | RQFCR_CMP_NOMATCH | RQFCR_HASH |
-			RQFCR_AND | RQFCR_HASHTBL_0;
+		      RQFCR_AND | RQFCR_HASHTBL_0;
 		priv->ftp_rqfpr[priv->cur_filer_idx] = fpr;
 		priv->ftp_rqfcr[priv->cur_filer_idx] = fcr;
 		gfar_write_filer(priv, priv->cur_filer_idx, fcr, fpr);
@@ -666,7 +687,7 @@ static void ethflow_to_filer_rules (struct gfar_private *priv, u64 ethflow)
 
 	if (ethflow & RXH_L4_B_0_1) {
 		fcr = RQFCR_PID_SPT | RQFCR_CMP_NOMATCH | RQFCR_HASH |
-			RQFCR_AND | RQFCR_HASHTBL_0;
+		      RQFCR_AND | RQFCR_HASHTBL_0;
 		priv->ftp_rqfpr[priv->cur_filer_idx] = fpr;
 		priv->ftp_rqfcr[priv->cur_filer_idx] = fcr;
 		gfar_write_filer(priv, priv->cur_filer_idx, fcr, fpr);
@@ -675,7 +696,7 @@ static void ethflow_to_filer_rules (struct gfar_private *priv, u64 ethflow)
 
 	if (ethflow & RXH_L4_B_2_3) {
 		fcr = RQFCR_PID_DPT | RQFCR_CMP_NOMATCH | RQFCR_HASH |
-			RQFCR_AND | RQFCR_HASHTBL_0;
+		      RQFCR_AND | RQFCR_HASHTBL_0;
 		priv->ftp_rqfpr[priv->cur_filer_idx] = fpr;
 		priv->ftp_rqfcr[priv->cur_filer_idx] = fcr;
 		gfar_write_filer(priv, priv->cur_filer_idx, fcr, fpr);
@@ -683,7 +704,8 @@ static void ethflow_to_filer_rules (struct gfar_private *priv, u64 ethflow)
 	}
 }
 
-static int gfar_ethflow_to_filer_table(struct gfar_private *priv, u64 ethflow, u64 class)
+static int gfar_ethflow_to_filer_table(struct gfar_private *priv, u64 ethflow,
+				       u64 class)
 {
 	unsigned int last_rule_idx = priv->cur_filer_idx;
 	unsigned int cmp_rqfpr;
@@ -694,9 +716,9 @@ static int gfar_ethflow_to_filer_table(struct gfar_private *priv, u64 ethflow, u
 	int ret = 1;
 
 	local_rqfpr = kmalloc(sizeof(unsigned int) * (MAX_FILER_IDX + 1),
-		GFP_KERNEL);
+			      GFP_KERNEL);
 	local_rqfcr = kmalloc(sizeof(unsigned int) * (MAX_FILER_IDX + 1),
-		GFP_KERNEL);
+			      GFP_KERNEL);
 	if (!local_rqfpr || !local_rqfcr) {
 		pr_err("Out of memory\n");
 		ret = 0;
@@ -726,9 +748,9 @@ static int gfar_ethflow_to_filer_table(struct gfar_private *priv, u64 ethflow, u
 		local_rqfpr[j] = priv->ftp_rqfpr[i];
 		local_rqfcr[j] = priv->ftp_rqfcr[i];
 		j--;
-		if ((priv->ftp_rqfcr[i] == (RQFCR_PID_PARSE |
-			RQFCR_CLE |RQFCR_AND)) &&
-			(priv->ftp_rqfpr[i] == cmp_rqfpr))
+		if ((priv->ftp_rqfcr[i] ==
+		     (RQFCR_PID_PARSE | RQFCR_CLE | RQFCR_AND)) &&
+		    (priv->ftp_rqfpr[i] == cmp_rqfpr))
 			break;
 	}
 
@@ -743,12 +765,12 @@ static int gfar_ethflow_to_filer_table(struct gfar_private *priv, u64 ethflow, u
 	 */
 	for (l = i+1; l < MAX_FILER_IDX; l++) {
 		if ((priv->ftp_rqfcr[l] & RQFCR_CLE) &&
-			!(priv->ftp_rqfcr[l] & RQFCR_AND)) {
+		    !(priv->ftp_rqfcr[l] & RQFCR_AND)) {
 			priv->ftp_rqfcr[l] = RQFCR_CLE | RQFCR_CMP_EXACT |
-				RQFCR_HASHTBL_0 | RQFCR_PID_MASK;
+					     RQFCR_HASHTBL_0 | RQFCR_PID_MASK;
 			priv->ftp_rqfpr[l] = FPR_FILER_MASK;
 			gfar_write_filer(priv, l, priv->ftp_rqfcr[l],
-				priv->ftp_rqfpr[l]);
+					 priv->ftp_rqfpr[l]);
 			break;
 		}
 
@@ -773,7 +795,7 @@ static int gfar_ethflow_to_filer_table(struct gfar_private *priv, u64 ethflow, u
 		priv->ftp_rqfpr[priv->cur_filer_idx] = local_rqfpr[k];
 		priv->ftp_rqfcr[priv->cur_filer_idx] = local_rqfcr[k];
 		gfar_write_filer(priv, priv->cur_filer_idx,
-				local_rqfcr[k], local_rqfpr[k]);
+				 local_rqfcr[k], local_rqfpr[k]);
 		if (!priv->cur_filer_idx)
 			break;
 		priv->cur_filer_idx = priv->cur_filer_idx - 1;
@@ -785,7 +807,8 @@ err:
 	return ret;
 }
 
-static int gfar_set_hash_opts(struct gfar_private *priv, struct ethtool_rxnfc *cmd)
+static int gfar_set_hash_opts(struct gfar_private *priv,
+			      struct ethtool_rxnfc *cmd)
 {
 	/* write the filer rules here */
 	if (!gfar_ethflow_to_filer_table(priv, cmd->data, cmd->flow_type))
@@ -810,10 +833,10 @@ static int gfar_check_filer_hardware(struct gfar_private *priv)
 		i &= RCTRL_PRSDEP_MASK | RCTRL_PRSFM;
 		if (i == (RCTRL_PRSDEP_MASK | RCTRL_PRSFM)) {
 			netdev_info(priv->ndev,
-					"Receive Queue Filtering enabled\n");
+				    "Receive Queue Filtering enabled\n");
 		} else {
 			netdev_warn(priv->ndev,
-					"Receive Queue Filtering disabled\n");
+				    "Receive Queue Filtering disabled\n");
 			return -EOPNOTSUPP;
 		}
 	}
@@ -823,16 +846,17 @@ static int gfar_check_filer_hardware(struct gfar_private *priv)
 		i &= RCTRL_PRSDEP_MASK;
 		if (i == RCTRL_PRSDEP_MASK) {
 			netdev_info(priv->ndev,
-					"Receive Queue Filtering enabled\n");
+				    "Receive Queue Filtering enabled\n");
 		} else {
 			netdev_warn(priv->ndev,
-					"Receive Queue Filtering disabled\n");
+				    "Receive Queue Filtering disabled\n");
 			return -EOPNOTSUPP;
 		}
 	}
 
 	/* Sets the properties for arbitrary filer rule
-	 * to the first 4 Layer 4 Bytes */
+	 * to the first 4 Layer 4 Bytes
+	 */
 	regs->rbifx = 0xC0C1C2C3;
 	return 0;
 }
@@ -870,14 +894,14 @@ static void gfar_set_mask(u32 mask, struct filer_table *tab)
 static void gfar_set_parse_bits(u32 value, u32 mask, struct filer_table *tab)
 {
 	gfar_set_mask(mask, tab);
-	tab->fe[tab->index].ctrl = RQFCR_CMP_EXACT | RQFCR_PID_PARSE
-			| RQFCR_AND;
+	tab->fe[tab->index].ctrl = RQFCR_CMP_EXACT | RQFCR_PID_PARSE |
+				   RQFCR_AND;
 	tab->fe[tab->index].prop = value;
 	tab->index++;
 }
 
 static void gfar_set_general_attribute(u32 value, u32 mask, u32 flag,
-		struct filer_table *tab)
+				       struct filer_table *tab)
 {
 	gfar_set_mask(mask, tab);
 	tab->fe[tab->index].ctrl = RQFCR_CMP_EXACT | RQFCR_AND | flag;
@@ -885,8 +909,7 @@ static void gfar_set_general_attribute(u32 value, u32 mask, u32 flag,
 	tab->index++;
 }
 
-/*
- * For setting a tuple of value and mask of type flag
+/* For setting a tuple of value and mask of type flag
  * Example:
  * IP-Src = 10.0.0.0/255.0.0.0
  * value: 0x0A000000 mask: FF000000 flag: RQFPR_IPV4
@@ -901,7 +924,7 @@ static void gfar_set_general_attribute(u32 value, u32 mask, u32 flag,
  * Further the all masks are one-padded for better hardware efficiency.
  */
 static void gfar_set_attribute(u32 value, u32 mask, u32 flag,
-		struct filer_table *tab)
+			       struct filer_table *tab)
 {
 	switch (flag) {
 		/* 3bit */
@@ -959,7 +982,8 @@ static void gfar_set_attribute(u32 value, u32 mask, u32 flag,
 
 /* Translates value and mask for UDP, TCP or SCTP */
 static void gfar_set_basic_ip(struct ethtool_tcpip4_spec *value,
-		struct ethtool_tcpip4_spec *mask, struct filer_table *tab)
+			      struct ethtool_tcpip4_spec *mask,
+			      struct filer_table *tab)
 {
 	gfar_set_attribute(value->ip4src, mask->ip4src, RQFCR_PID_SIA, tab);
 	gfar_set_attribute(value->ip4dst, mask->ip4dst, RQFCR_PID_DIA, tab);
@@ -970,97 +994,92 @@ static void gfar_set_basic_ip(struct ethtool_tcpip4_spec *value,
 
 /* Translates value and mask for RAW-IP4 */
 static void gfar_set_user_ip(struct ethtool_usrip4_spec *value,
-		struct ethtool_usrip4_spec *mask, struct filer_table *tab)
+			     struct ethtool_usrip4_spec *mask,
+			     struct filer_table *tab)
 {
 	gfar_set_attribute(value->ip4src, mask->ip4src, RQFCR_PID_SIA, tab);
 	gfar_set_attribute(value->ip4dst, mask->ip4dst, RQFCR_PID_DIA, tab);
 	gfar_set_attribute(value->tos, mask->tos, RQFCR_PID_TOS, tab);
 	gfar_set_attribute(value->proto, mask->proto, RQFCR_PID_L4P, tab);
 	gfar_set_attribute(value->l4_4_bytes, mask->l4_4_bytes, RQFCR_PID_ARB,
-			tab);
+			   tab);
 
 }
 
 /* Translates value and mask for ETHER spec */
 static void gfar_set_ether(struct ethhdr *value, struct ethhdr *mask,
-		struct filer_table *tab)
+			   struct filer_table *tab)
 {
 	u32 upper_temp_mask = 0;
 	u32 lower_temp_mask = 0;
+
 	/* Source address */
 	if (!is_broadcast_ether_addr(mask->h_source)) {
-
 		if (is_zero_ether_addr(mask->h_source)) {
 			upper_temp_mask = 0xFFFFFFFF;
 			lower_temp_mask = 0xFFFFFFFF;
 		} else {
-			upper_temp_mask = mask->h_source[0] << 16
-					| mask->h_source[1] << 8
-					| mask->h_source[2];
-			lower_temp_mask = mask->h_source[3] << 16
-					| mask->h_source[4] << 8
-					| mask->h_source[5];
+			upper_temp_mask = mask->h_source[0] << 16 |
+					  mask->h_source[1] << 8  |
+					  mask->h_source[2];
+			lower_temp_mask = mask->h_source[3] << 16 |
+					  mask->h_source[4] << 8  |
+					  mask->h_source[5];
 		}
 		/* Upper 24bit */
-		gfar_set_attribute(
-				value->h_source[0] << 16 | value->h_source[1]
-						<< 8 | value->h_source[2],
-				upper_temp_mask, RQFCR_PID_SAH, tab);
+		gfar_set_attribute(value->h_source[0] << 16 |
+				   value->h_source[1] << 8  |
+				   value->h_source[2],
+				   upper_temp_mask, RQFCR_PID_SAH, tab);
 		/* And the same for the lower part */
-		gfar_set_attribute(
-				value->h_source[3] << 16 | value->h_source[4]
-						<< 8 | value->h_source[5],
-				lower_temp_mask, RQFCR_PID_SAL, tab);
+		gfar_set_attribute(value->h_source[3] << 16 |
+				   value->h_source[4] << 8  |
+				   value->h_source[5],
+				   lower_temp_mask, RQFCR_PID_SAL, tab);
 	}
 	/* Destination address */
 	if (!is_broadcast_ether_addr(mask->h_dest)) {
-
 		/* Special for destination is limited broadcast */
-		if ((is_broadcast_ether_addr(value->h_dest)
-				&& is_zero_ether_addr(mask->h_dest))) {
+		if ((is_broadcast_ether_addr(value->h_dest) &&
+		    is_zero_ether_addr(mask->h_dest))) {
 			gfar_set_parse_bits(RQFPR_EBC, RQFPR_EBC, tab);
 		} else {
-
 			if (is_zero_ether_addr(mask->h_dest)) {
 				upper_temp_mask = 0xFFFFFFFF;
 				lower_temp_mask = 0xFFFFFFFF;
 			} else {
-				upper_temp_mask = mask->h_dest[0] << 16
-						| mask->h_dest[1] << 8
-						| mask->h_dest[2];
-				lower_temp_mask = mask->h_dest[3] << 16
-						| mask->h_dest[4] << 8
-						| mask->h_dest[5];
+				upper_temp_mask = mask->h_dest[0] << 16 |
+						  mask->h_dest[1] << 8  |
+						  mask->h_dest[2];
+				lower_temp_mask = mask->h_dest[3] << 16 |
+						  mask->h_dest[4] << 8  |
+						  mask->h_dest[5];
 			}
 
 			/* Upper 24bit */
-			gfar_set_attribute(
-					value->h_dest[0] << 16
-							| value->h_dest[1] << 8
-							| value->h_dest[2],
-					upper_temp_mask, RQFCR_PID_DAH, tab);
+			gfar_set_attribute(value->h_dest[0] << 16 |
+					   value->h_dest[1] << 8  |
+					   value->h_dest[2],
+					   upper_temp_mask, RQFCR_PID_DAH, tab);
 			/* And the same for the lower part */
-			gfar_set_attribute(
-					value->h_dest[3] << 16
-							| value->h_dest[4] << 8
-							| value->h_dest[5],
-					lower_temp_mask, RQFCR_PID_DAL, tab);
+			gfar_set_attribute(value->h_dest[3] << 16 |
+					   value->h_dest[4] << 8  |
+					   value->h_dest[5],
+					   lower_temp_mask, RQFCR_PID_DAL, tab);
 		}
 	}
 
 	gfar_set_attribute(value->h_proto, mask->h_proto, RQFCR_PID_ETY, tab);
-
 }
 
 /* Convert a rule to binary filter format of gianfar */
 static int gfar_convert_to_filer(struct ethtool_rx_flow_spec *rule,
-		struct filer_table *tab)
+				 struct filer_table *tab)
 {
 	u32 vlan = 0, vlan_mask = 0;
 	u32 id = 0, id_mask = 0;
 	u32 cfi = 0, cfi_mask = 0;
 	u32 prio = 0, prio_mask = 0;
-
 	u32 old_index = tab->index;
 
 	/* Check if vlan is wanted */
@@ -1076,13 +1095,16 @@ static int gfar_convert_to_filer(struct ethtool_rx_flow_spec *rule,
 		id_mask = rule->m_ext.vlan_tci & VLAN_VID_MASK;
 		cfi = rule->h_ext.vlan_tci & VLAN_CFI_MASK;
 		cfi_mask = rule->m_ext.vlan_tci & VLAN_CFI_MASK;
-		prio = (rule->h_ext.vlan_tci & VLAN_PRIO_MASK) >> VLAN_PRIO_SHIFT;
-		prio_mask = (rule->m_ext.vlan_tci & VLAN_PRIO_MASK) >> VLAN_PRIO_SHIFT;
+		prio = (rule->h_ext.vlan_tci & VLAN_PRIO_MASK) >>
+		       VLAN_PRIO_SHIFT;
+		prio_mask = (rule->m_ext.vlan_tci & VLAN_PRIO_MASK) >>
+			    VLAN_PRIO_SHIFT;
 
 		if (cfi == VLAN_TAG_PRESENT && cfi_mask == VLAN_TAG_PRESENT) {
 			vlan |= RQFPR_CFI;
 			vlan_mask |= RQFPR_CFI;
-		} else if (cfi != VLAN_TAG_PRESENT && cfi_mask == VLAN_TAG_PRESENT) {
+		} else if (cfi != VLAN_TAG_PRESENT &&
+			   cfi_mask == VLAN_TAG_PRESENT) {
 			vlan_mask |= RQFPR_CFI;
 		}
 	}
@@ -1090,34 +1112,36 @@ static int gfar_convert_to_filer(struct ethtool_rx_flow_spec *rule,
 	switch (rule->flow_type & ~FLOW_EXT) {
 	case TCP_V4_FLOW:
 		gfar_set_parse_bits(RQFPR_IPV4 | RQFPR_TCP | vlan,
-				RQFPR_IPV4 | RQFPR_TCP | vlan_mask, tab);
+				    RQFPR_IPV4 | RQFPR_TCP | vlan_mask, tab);
 		gfar_set_basic_ip(&rule->h_u.tcp_ip4_spec,
-				&rule->m_u.tcp_ip4_spec, tab);
+				  &rule->m_u.tcp_ip4_spec, tab);
 		break;
 	case UDP_V4_FLOW:
 		gfar_set_parse_bits(RQFPR_IPV4 | RQFPR_UDP | vlan,
-				RQFPR_IPV4 | RQFPR_UDP | vlan_mask, tab);
+				    RQFPR_IPV4 | RQFPR_UDP | vlan_mask, tab);
 		gfar_set_basic_ip(&rule->h_u.udp_ip4_spec,
-				&rule->m_u.udp_ip4_spec, tab);
+				  &rule->m_u.udp_ip4_spec, tab);
 		break;
 	case SCTP_V4_FLOW:
 		gfar_set_parse_bits(RQFPR_IPV4 | vlan, RQFPR_IPV4 | vlan_mask,
-				tab);
+				    tab);
 		gfar_set_attribute(132, 0, RQFCR_PID_L4P, tab);
-		gfar_set_basic_ip((struct ethtool_tcpip4_spec *) &rule->h_u,
-				(struct ethtool_tcpip4_spec *) &rule->m_u, tab);
+		gfar_set_basic_ip((struct ethtool_tcpip4_spec *)&rule->h_u,
+				  (struct ethtool_tcpip4_spec *)&rule->m_u,
+				  tab);
 		break;
 	case IP_USER_FLOW:
 		gfar_set_parse_bits(RQFPR_IPV4 | vlan, RQFPR_IPV4 | vlan_mask,
-				tab);
+				    tab);
 		gfar_set_user_ip((struct ethtool_usrip4_spec *) &rule->h_u,
-				(struct ethtool_usrip4_spec *) &rule->m_u, tab);
+				 (struct ethtool_usrip4_spec *) &rule->m_u,
+				 tab);
 		break;
 	case ETHER_FLOW:
 		if (vlan)
 			gfar_set_parse_bits(vlan, vlan_mask, tab);
 		gfar_set_ether((struct ethhdr *) &rule->h_u,
-				(struct ethhdr *) &rule->m_u, tab);
+			       (struct ethhdr *) &rule->m_u, tab);
 		break;
 	default:
 		return -1;
@@ -1152,7 +1176,9 @@ static int gfar_convert_to_filer(struct ethtool_rx_flow_spec *rule,
 		tab->fe[tab->index - 1].ctrl |= RQFCR_CLE;
 	}
 
-	/* In rare cases the cache can be full while there is free space in hw */
+	/* In rare cases the cache can be full while there is
+	 * free space in hw
+	 */
 	if (tab->index > MAX_FILER_CACHE_IDX - 1)
 		return -EBUSY;
 
@@ -1161,7 +1187,7 @@ static int gfar_convert_to_filer(struct ethtool_rx_flow_spec *rule,
 
 /* Copy size filer entries */
 static void gfar_copy_filer_entries(struct gfar_filer_entry dst[0],
-		struct gfar_filer_entry src[0], s32 size)
+				    struct gfar_filer_entry src[0], s32 size)
 {
 	while (size > 0) {
 		size--;
@@ -1171,10 +1197,12 @@ static void gfar_copy_filer_entries(struct gfar_filer_entry dst[0],
 }
 
 /* Delete the contents of the filer-table between start and end
- * and collapse them */
+ * and collapse them
+ */
 static int gfar_trim_filer_entries(u32 begin, u32 end, struct filer_table *tab)
 {
 	int length;
+
 	if (end > MAX_FILER_CACHE_IDX || end < begin)
 		return -EINVAL;
 
@@ -1200,14 +1228,14 @@ static int gfar_trim_filer_entries(u32 begin, u32 end, struct filer_table *tab)
 
 /* Make space on the wanted location */
 static int gfar_expand_filer_entries(u32 begin, u32 length,
-		struct filer_table *tab)
+				     struct filer_table *tab)
 {
-	if (length == 0 || length + tab->index > MAX_FILER_CACHE_IDX || begin
-			> MAX_FILER_CACHE_IDX)
+	if (length == 0 || length + tab->index > MAX_FILER_CACHE_IDX ||
+	    begin > MAX_FILER_CACHE_IDX)
 		return -EINVAL;
 
 	gfar_copy_filer_entries(&(tab->fe[begin + length]), &(tab->fe[begin]),
-			tab->index - length + 1);
+				tab->index - length + 1);
 
 	tab->index += length;
 	return 0;
@@ -1215,9 +1243,10 @@ static int gfar_expand_filer_entries(u32 begin, u32 length,
 
 static int gfar_get_next_cluster_start(int start, struct filer_table *tab)
 {
-	for (; (start < tab->index) && (start < MAX_FILER_CACHE_IDX - 1); start++) {
-		if ((tab->fe[start].ctrl & (RQFCR_AND | RQFCR_CLE))
-				== (RQFCR_AND | RQFCR_CLE))
+	for (; (start < tab->index) && (start < MAX_FILER_CACHE_IDX - 1);
+	     start++) {
+		if ((tab->fe[start].ctrl & (RQFCR_AND | RQFCR_CLE)) ==
+		    (RQFCR_AND | RQFCR_CLE))
 			return start;
 	}
 	return -1;
@@ -1225,16 +1254,16 @@ static int gfar_get_next_cluster_start(int start, struct filer_table *tab)
 
 static int gfar_get_next_cluster_end(int start, struct filer_table *tab)
 {
-	for (; (start < tab->index) && (start < MAX_FILER_CACHE_IDX - 1); start++) {
-		if ((tab->fe[start].ctrl & (RQFCR_AND | RQFCR_CLE))
-				== (RQFCR_CLE))
+	for (; (start < tab->index) && (start < MAX_FILER_CACHE_IDX - 1);
+	     start++) {
+		if ((tab->fe[start].ctrl & (RQFCR_AND | RQFCR_CLE)) ==
+		    (RQFCR_CLE))
 			return start;
 	}
 	return -1;
 }
 
-/*
- * Uses hardwares clustering option to reduce
+/* Uses hardwares clustering option to reduce
  * the number of filer table entries
  */
 static void gfar_cluster_filer(struct filer_table *tab)
@@ -1244,8 +1273,7 @@ static void gfar_cluster_filer(struct filer_table *tab)
 	while ((i = gfar_get_next_cluster_start(++i, tab)) != -1) {
 		j = i;
 		while ((j = gfar_get_next_cluster_start(++j, tab)) != -1) {
-			/*
-			 * The cluster entries self and the previous one
+			/* The cluster entries self and the previous one
 			 * (a mask) must be identical!
 			 */
 			if (tab->fe[i].ctrl != tab->fe[j].ctrl)
@@ -1260,21 +1288,21 @@ static void gfar_cluster_filer(struct filer_table *tab)
 			jend = gfar_get_next_cluster_end(j, tab);
 			if (jend == -1 || iend == -1)
 				break;
-			/*
-			 * First we make some free space, where our cluster
+
+			/* First we make some free space, where our cluster
 			 * element should be. Then we copy it there and finally
 			 * delete in from its old location.
 			 */
-
-			if (gfar_expand_filer_entries(iend, (jend - j), tab)
-					== -EINVAL)
+			if (gfar_expand_filer_entries(iend, (jend - j), tab) ==
+			    -EINVAL)
 				break;
 
 			gfar_copy_filer_entries(&(tab->fe[iend + 1]),
-					&(tab->fe[jend + 1]), jend - j);
+						&(tab->fe[jend + 1]), jend - j);
 
 			if (gfar_trim_filer_entries(jend - 1,
-					jend + (jend - j), tab) == -EINVAL)
+						    jend + (jend - j),
+						    tab) == -EINVAL)
 				return;
 
 			/* Mask out cluster bit */
@@ -1285,8 +1313,9 @@ static void gfar_cluster_filer(struct filer_table *tab)
 
 /* Swaps the masked bits of a1<>a2 and b1<>b2 */
 static void gfar_swap_bits(struct gfar_filer_entry *a1,
-		struct gfar_filer_entry *a2, struct gfar_filer_entry *b1,
-		struct gfar_filer_entry *b2, u32 mask)
+			   struct gfar_filer_entry *a2,
+			   struct gfar_filer_entry *b1,
+			   struct gfar_filer_entry *b2, u32 mask)
 {
 	u32 temp[4];
 	temp[0] = a1->ctrl & mask;
@@ -1305,13 +1334,12 @@ static void gfar_swap_bits(struct gfar_filer_entry *a1,
 	b2->ctrl |= temp[2];
 }
 
-/*
- * Generate a list consisting of masks values with their start and
+/* Generate a list consisting of masks values with their start and
  * end of validity and block as indicator for parts belonging
  * together (glued by ANDs) in mask_table
  */
 static u32 gfar_generate_mask_table(struct gfar_mask_entry *mask_table,
-		struct filer_table *tab)
+				    struct filer_table *tab)
 {
 	u32 i, and_index = 0, block_index = 1;
 
@@ -1327,13 +1355,13 @@ static u32 gfar_generate_mask_table(struct gfar_mask_entry *mask_table,
 			and_index++;
 		}
 		/* cluster starts and ends will be separated because they should
-		 * hold their position */
+		 * hold their position
+		 */
 		if (tab->fe[i].ctrl & RQFCR_CLE)
 			block_index++;
 		/* A not set AND indicates the end of a depended block */
 		if (!(tab->fe[i].ctrl & RQFCR_AND))
 			block_index++;
-
 	}
 
 	mask_table[and_index - 1].end = i - 1;
@@ -1341,14 +1369,13 @@ static u32 gfar_generate_mask_table(struct gfar_mask_entry *mask_table,
 	return and_index;
 }
 
-/*
- * Sorts the entries of mask_table by the values of the masks.
+/* Sorts the entries of mask_table by the values of the masks.
  * Important: The 0xFF80 flags of the first and last entry of a
  * block must hold their position (which queue, CLusterEnable, ReJEct,
  * AND)
  */
 static void gfar_sort_mask_table(struct gfar_mask_entry *mask_table,
-		struct filer_table *temp_table, u32 and_index)
+				 struct filer_table *temp_table, u32 and_index)
 {
 	/* Pointer to compare function (_asc or _desc) */
 	int (*gfar_comp)(const void *, const void *);
@@ -1359,16 +1386,16 @@ static void gfar_sort_mask_table(struct gfar_mask_entry *mask_table,
 	gfar_comp = &gfar_comp_desc;
 
 	for (i = 0; i < and_index; i++) {
-
 		if (prev != mask_table[i].block) {
 			old_first = mask_table[start].start + 1;
 			old_last = mask_table[i - 1].end;
 			sort(mask_table + start, size,
-					sizeof(struct gfar_mask_entry),
-					gfar_comp, &gfar_swap);
+			     sizeof(struct gfar_mask_entry),
+			     gfar_comp, &gfar_swap);
 
 			/* Toggle order for every block. This makes the
-			 * thing more efficient! */
+			 * thing more efficient!
+			 */
 			if (gfar_comp == gfar_comp_desc)
 				gfar_comp = &gfar_comp_asc;
 			else
@@ -1378,12 +1405,11 @@ static void gfar_sort_mask_table(struct gfar_mask_entry *mask_table,
 			new_last = mask_table[i - 1].end;
 
 			gfar_swap_bits(&temp_table->fe[new_first],
-					&temp_table->fe[old_first],
-					&temp_table->fe[new_last],
-					&temp_table->fe[old_last],
-					RQFCR_QUEUE | RQFCR_CLE |
-						RQFCR_RJE | RQFCR_AND
-					);
+				       &temp_table->fe[old_first],
+				       &temp_table->fe[new_last],
+				       &temp_table->fe[old_last],
+				       RQFCR_QUEUE | RQFCR_CLE |
+				       RQFCR_RJE | RQFCR_AND);
 
 			start = i;
 			size = 0;
@@ -1391,11 +1417,9 @@ static void gfar_sort_mask_table(struct gfar_mask_entry *mask_table,
 		size++;
 		prev = mask_table[i].block;
 	}
-
 }
 
-/*
- * Reduces the number of masks needed in the filer table to save entries
+/* Reduces the number of masks needed in the filer table to save entries
  * This is done by sorting the masks of a depended block. A depended block is
  * identified by gluing ANDs or CLE. The sorting order toggles after every
  * block. Of course entries in scope of a mask must change their location with
@@ -1410,13 +1434,14 @@ static int gfar_optimize_filer_masks(struct filer_table *tab)
 	s32 ret = 0;
 
 	/* We need a copy of the filer table because
-	 * we want to change its order */
+	 * we want to change its order
+	 */
 	temp_table = kmemdup(tab, sizeof(*temp_table), GFP_KERNEL);
 	if (temp_table == NULL)
 		return -ENOMEM;
 
 	mask_table = kcalloc(MAX_FILER_CACHE_IDX / 2 + 1,
-			sizeof(struct gfar_mask_entry), GFP_KERNEL);
+			     sizeof(struct gfar_mask_entry), GFP_KERNEL);
 
 	if (mask_table == NULL) {
 		ret = -ENOMEM;
@@ -1428,7 +1453,8 @@ static int gfar_optimize_filer_masks(struct filer_table *tab)
 	gfar_sort_mask_table(mask_table, temp_table, and_index);
 
 	/* Now we can copy the data from our duplicated filer table to
-	 * the real one in the order the mask table says */
+	 * the real one in the order the mask table says
+	 */
 	for (i = 0; i < and_index; i++) {
 		size = mask_table[i].end - mask_table[i].start + 1;
 		gfar_copy_filer_entries(&(tab->fe[j]),
@@ -1437,7 +1463,8 @@ static int gfar_optimize_filer_masks(struct filer_table *tab)
 	}
 
 	/* And finally we just have to check for duplicated masks and drop the
-	 * second ones */
+	 * second ones
+	 */
 	for (i = 0; i < tab->index && i < MAX_FILER_CACHE_IDX; i++) {
 		if (tab->fe[i].ctrl == 0x80) {
 			previous_mask = i++;
@@ -1448,7 +1475,8 @@ static int gfar_optimize_filer_masks(struct filer_table *tab)
 		if (tab->fe[i].ctrl == 0x80) {
 			if (tab->fe[i].prop == tab->fe[previous_mask].prop) {
 				/* Two identical ones found!
-				 * So drop the second one! */
+				 * So drop the second one!
+				 */
 				gfar_trim_filer_entries(i, i, tab);
 			} else
 				/* Not identical! */
@@ -1463,7 +1491,7 @@ end:	kfree(temp_table);
 
 /* Write the bit-pattern from software's buffer to hardware registers */
 static int gfar_write_filer_table(struct gfar_private *priv,
-		struct filer_table *tab)
+				  struct filer_table *tab)
 {
 	u32 i = 0;
 	if (tab->index > MAX_FILER_IDX - 1)
@@ -1473,13 +1501,15 @@ static int gfar_write_filer_table(struct gfar_private *priv,
 	lock_rx_qs(priv);
 
 	/* Fill regular entries */
-	for (; i < MAX_FILER_IDX - 1 && (tab->fe[i].ctrl | tab->fe[i].ctrl); i++)
+	for (; i < MAX_FILER_IDX - 1 && (tab->fe[i].ctrl | tab->fe[i].ctrl);
+	     i++)
 		gfar_write_filer(priv, i, tab->fe[i].ctrl, tab->fe[i].prop);
 	/* Fill the rest with fall-troughs */
 	for (; i < MAX_FILER_IDX - 1; i++)
 		gfar_write_filer(priv, i, 0x60, 0xFFFFFFFF);
 	/* Last entry must be default accept
-	 * because that's what people expect */
+	 * because that's what people expect
+	 */
 	gfar_write_filer(priv, i, 0x20, 0x0);
 
 	unlock_rx_qs(priv);
@@ -1488,21 +1518,21 @@ static int gfar_write_filer_table(struct gfar_private *priv,
 }
 
 static int gfar_check_capability(struct ethtool_rx_flow_spec *flow,
-		struct gfar_private *priv)
+				 struct gfar_private *priv)
 {
 
 	if (flow->flow_type & FLOW_EXT)	{
 		if (~flow->m_ext.data[0] || ~flow->m_ext.data[1])
 			netdev_warn(priv->ndev,
-					"User-specific data not supported!\n");
+				    "User-specific data not supported!\n");
 		if (~flow->m_ext.vlan_etype)
 			netdev_warn(priv->ndev,
-					"VLAN-etype not supported!\n");
+				    "VLAN-etype not supported!\n");
 	}
 	if (flow->flow_type == IP_USER_FLOW)
 		if (flow->h_u.usr_ip4_spec.ip_ver != ETH_RX_NFC_IP4)
 			netdev_warn(priv->ndev,
-					"IP-Version differing from IPv4 not supported!\n");
+				    "IP-Version differing from IPv4 not supported!\n");
 
 	return 0;
 }
@@ -1520,15 +1550,18 @@ static int gfar_process_filer_changes(struct gfar_private *priv)
 		return -ENOMEM;
 
 	/* Now convert the existing filer data from flow_spec into
-	 * filer tables binary format */
+	 * filer tables binary format
+	 */
 	list_for_each_entry(j, &priv->rx_list.list, list) {
 		ret = gfar_convert_to_filer(&j->fs, tab);
 		if (ret == -EBUSY) {
-			netdev_err(priv->ndev, "Rule not added: No free space!\n");
+			netdev_err(priv->ndev,
+				   "Rule not added: No free space!\n");
 			goto end;
 		}
 		if (ret == -1) {
-			netdev_err(priv->ndev, "Rule not added: Unsupported Flow-type!\n");
+			netdev_err(priv->ndev,
+				   "Rule not added: Unsupported Flow-type!\n");
 			goto end;
 		}
 	}
@@ -1540,9 +1573,9 @@ static int gfar_process_filer_changes(struct gfar_private *priv)
 	gfar_optimize_filer_masks(tab);
 
 	pr_debug("\n\tSummary:\n"
-		"\tData on hardware: %d\n"
-		"\tCompression rate: %d%%\n",
-		tab->index, 100 - (100 * tab->index) / i);
+		 "\tData on hardware: %d\n"
+		 "\tCompression rate: %d%%\n",
+		 tab->index, 100 - (100 * tab->index) / i);
 
 	/* Write everything to hardware */
 	ret = gfar_write_filer_table(priv, tab);
@@ -1551,7 +1584,8 @@ static int gfar_process_filer_changes(struct gfar_private *priv)
 		goto end;
 	}
 
-end:	kfree(tab);
+end:
+	kfree(tab);
 	return ret;
 }
 
@@ -1569,7 +1603,7 @@ static void gfar_invert_masks(struct ethtool_rx_flow_spec *flow)
 }
 
 static int gfar_add_cls(struct gfar_private *priv,
-		struct ethtool_rx_flow_spec *flow)
+			struct ethtool_rx_flow_spec *flow)
 {
 	struct ethtool_flow_spec_container *temp, *comp;
 	int ret = 0;
@@ -1591,7 +1625,6 @@ static int gfar_add_cls(struct gfar_private *priv,
 		list_add(&temp->list, &priv->rx_list.list);
 		goto process;
 	} else {
-
 		list_for_each_entry(comp, &priv->rx_list.list, list) {
 			if (comp->fs.location > flow->location) {
 				list_add_tail(&temp->list, &comp->list);
@@ -1599,8 +1632,8 @@ static int gfar_add_cls(struct gfar_private *priv,
 			}
 			if (comp->fs.location == flow->location) {
 				netdev_err(priv->ndev,
-						"Rule not added: ID %d not free!\n",
-					flow->location);
+					   "Rule not added: ID %d not free!\n",
+					   flow->location);
 				ret = -EBUSY;
 				goto clean_mem;
 			}
@@ -1642,7 +1675,6 @@ static int gfar_del_cls(struct gfar_private *priv, u32 loc)
 	}
 
 	return ret;
-
 }
 
 static int gfar_get_cls(struct gfar_private *priv, struct ethtool_rxnfc *cmd)
@@ -1663,7 +1695,7 @@ static int gfar_get_cls(struct gfar_private *priv, struct ethtool_rxnfc *cmd)
 }
 
 static int gfar_get_cls_all(struct gfar_private *priv,
-		struct ethtool_rxnfc *cmd, u32 *rule_locs)
+			    struct ethtool_rxnfc *cmd, u32 *rule_locs)
 {
 	struct ethtool_flow_spec_container *comp;
 	u32 i = 0;
@@ -1714,7 +1746,7 @@ static int gfar_set_nfc(struct net_device *dev, struct ethtool_rxnfc *cmd)
 }
 
 static int gfar_get_nfc(struct net_device *dev, struct ethtool_rxnfc *cmd,
-		u32 *rule_locs)
+			u32 *rule_locs)
 {
 	struct gfar_private *priv = netdev_priv(dev);
 	int ret = 0;
@@ -1748,23 +1780,19 @@ static int gfar_get_ts_info(struct net_device *dev,
 	struct gfar_private *priv = netdev_priv(dev);
 
 	if (!(priv->device_flags & FSL_GIANFAR_DEV_HAS_TIMER)) {
-		info->so_timestamping =
-			SOF_TIMESTAMPING_RX_SOFTWARE |
-			SOF_TIMESTAMPING_SOFTWARE;
+		info->so_timestamping = SOF_TIMESTAMPING_RX_SOFTWARE |
+					SOF_TIMESTAMPING_SOFTWARE;
 		info->phc_index = -1;
 		return 0;
 	}
-	info->so_timestamping =
-		SOF_TIMESTAMPING_TX_HARDWARE |
-		SOF_TIMESTAMPING_RX_HARDWARE |
-		SOF_TIMESTAMPING_RAW_HARDWARE;
+	info->so_timestamping = SOF_TIMESTAMPING_TX_HARDWARE |
+				SOF_TIMESTAMPING_RX_HARDWARE |
+				SOF_TIMESTAMPING_RAW_HARDWARE;
 	info->phc_index = gfar_phc_index;
-	info->tx_types =
-		(1 << HWTSTAMP_TX_OFF) |
-		(1 << HWTSTAMP_TX_ON);
-	info->rx_filters =
-		(1 << HWTSTAMP_FILTER_NONE) |
-		(1 << HWTSTAMP_FILTER_ALL);
+	info->tx_types = (1 << HWTSTAMP_TX_OFF) |
+			 (1 << HWTSTAMP_TX_ON);
+	info->rx_filters = (1 << HWTSTAMP_FILTER_NONE) |
+			   (1 << HWTSTAMP_FILTER_ALL);
 	return 0;
 }
 
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH] snmp: fix OutOctets counter to include forwarded datagrams
From: Vincent Bernat @ 2012-06-05 13:41 UTC (permalink / raw)
  To: netdev; +Cc: davem, yoshfuji, Vincent Bernat

RFC 4293 defines ipIfStatsOutOctets (similar definition for
ipSystemStatsOutOctets):

   The total number of octets in IP datagrams delivered to the lower
   layers for transmission.  Octets from datagrams counted in
   ipIfStatsOutTransmits MUST be counted here.

And ipIfStatsOutTransmits:

   The total number of IP datagrams that this entity supplied to the
   lower layers for transmission.  This includes datagrams generated
   locally and those forwarded by this entity.

Therefore, IPSTATS_MIB_OUTOCTETS must be incremented when incrementing
IPSTATS_MIB_OUTFORWDATAGRAMS.

IP_UPD_PO_STATS is not used since ipIfStatsOutRequests must not
include forwarded datagrams:

   The total number of IP datagrams that local IP user-protocols
   (including ICMP) supplied to IP in requests for transmission.  Note
   that this counter does not include any datagrams counted in
   ipIfStatsOutForwDatagrams.

Signed-off-by: Vincent Bernat <bernat@luffy.cx>
---
 net/ipv4/ip_forward.c |    1 +
 net/ipv4/ipmr.c       |    1 +
 net/ipv6/ip6_output.c |    1 +
 net/ipv6/ip6mr.c      |    2 ++
 4 files changed, 5 insertions(+)

diff --git a/net/ipv4/ip_forward.c b/net/ipv4/ip_forward.c
index e5c44fc..ab09b12 100644
--- a/net/ipv4/ip_forward.c
+++ b/net/ipv4/ip_forward.c
@@ -44,6 +44,7 @@ static int ip_forward_finish(struct sk_buff *skb)
 	struct ip_options *opt	= &(IPCB(skb)->opt);
 
 	IP_INC_STATS_BH(dev_net(skb_dst(skb)->dev), IPSTATS_MIB_OUTFORWDATAGRAMS);
+	IP_ADD_STATS_BH(dev_net(skb_dst(skb)->dev), IPSTATS_MIB_OUTOCTETS, skb->len);
 
 	if (unlikely(opt->optlen))
 		ip_forward_options(skb);
diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index a9e519a..c94bbc6 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -1574,6 +1574,7 @@ static inline int ipmr_forward_finish(struct sk_buff *skb)
 	struct ip_options *opt = &(IPCB(skb)->opt);
 
 	IP_INC_STATS_BH(dev_net(skb_dst(skb)->dev), IPSTATS_MIB_OUTFORWDATAGRAMS);
+	IP_ADD_STATS_BH(dev_net(skb_dst(skb)->dev), IPSTATS_MIB_OUTOCTETS, skb->len);
 
 	if (unlikely(opt->optlen))
 		ip_forward_options(skb);
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 17b8c67..decc21d1 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -526,6 +526,7 @@ int ip6_forward(struct sk_buff *skb)
 	hdr->hop_limit--;
 
 	IP6_INC_STATS_BH(net, ip6_dst_idev(dst), IPSTATS_MIB_OUTFORWDATAGRAMS);
+	IP6_ADD_STATS_BH(net, ip6_dst_idev(dst), IPSTATS_MIB_OUTOCTETS, skb->len);
 	return NF_HOOK(NFPROTO_IPV6, NF_INET_FORWARD, skb, skb->dev, dst->dev,
 		       ip6_forward_finish);
 
diff --git a/net/ipv6/ip6mr.c b/net/ipv6/ip6mr.c
index b15dc08..461e47c 100644
--- a/net/ipv6/ip6mr.c
+++ b/net/ipv6/ip6mr.c
@@ -1886,6 +1886,8 @@ static inline int ip6mr_forward2_finish(struct sk_buff *skb)
 {
 	IP6_INC_STATS_BH(dev_net(skb_dst(skb)->dev), ip6_dst_idev(skb_dst(skb)),
 			 IPSTATS_MIB_OUTFORWDATAGRAMS);
+	IP6_ADD_STATS_BH(dev_net(skb_dst(skb)->dev), ip6_dst_idev(skb_dst(skb)),
+			 IPSTATS_MIB_OUTOCTETS, skb->len);
 	return dst_output(skb);
 }
 
-- 
1.7.10

^ permalink raw reply related

* [PATCH] usbnet: Activate the halt interrupt endpoint to fix endless "XactErr" error
From: Huajun Li @ 2012-06-05 14:12 UTC (permalink / raw)
  To: David S. Miller
  Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
	Huajun Li

There prints endless "XactErr" error msg once switch my device to the
configuration
 which needs cdc_ether driver, the root cause is the interrupt endpoint halts.
Maybe this is a common issue, so fix it by activating the endpoint
once the error occurs.

Signed-off-by: Huajun Li <huajun.li.lee-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/net/usb/usbnet.c   |   33 +++++++++++++++++++++++++++++++++
 include/linux/usb/usbnet.h |   15 ++++++++-------
 2 files changed, 41 insertions(+), 7 deletions(-)

diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 9f58330..f13922b 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -537,6 +537,11 @@ static void intr_complete (struct urb *urb)
 			  "intr shutdown, code %d\n", status);
 		return;

+	case -EPIPE:
+	case -EPROTO:
+		usbnet_defer_kevent(dev, EVENT_STS_HALT);
+		return;
+
 	/* NOTE:  not throttling like RX/TX, since this endpoint
 	 * already polls infrequently
 	 */
@@ -967,6 +972,34 @@ fail_halt:
 		}
 	}

+	if (test_bit(EVENT_STS_HALT, &dev->flags)) {
+		unsigned pipe;
+		struct usb_endpoint_descriptor *desc;
+
+		desc = &dev->status->desc;
+		pipe = usb_rcvintpipe(dev->udev,
+			desc->bEndpointAddress & USB_ENDPOINT_NUMBER_MASK);
+		status = usb_autopm_get_interface(dev->intf);
+		if (status < 0)
+			goto fail_sts;
+		status = usb_clear_halt(dev->udev, pipe);
+		usb_autopm_put_interface(dev->intf);
+
+		if (status < 0 && status != -EPIPE && status != -ESHUTDOWN) {
+fail_sts:
+			netdev_err(dev->net,
+				"can't clear intr halt, status %d\n", status);
+		} else {
+			clear_bit(EVENT_STS_HALT, &dev->flags);
+			memset(dev->interrupt->transfer_buffer, 0,
+				dev->interrupt->transfer_buffer_length);
+			status = usb_submit_urb(dev->interrupt, GFP_KERNEL);
+			if (status != 0)
+				netif_err(dev, timer, dev->net,
+					"intr resubmit --> %d\n", status);
+		}
+	}
+
 	/* tasklet could resubmit itself forever if memory is tight */
 	if (test_bit (EVENT_RX_MEMORY, &dev->flags)) {
 		struct urb	*urb = NULL;
diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
index 76f4396..c0bcb61 100644
--- a/include/linux/usb/usbnet.h
+++ b/include/linux/usb/usbnet.h
@@ -62,13 +62,14 @@ struct usbnet {
 	unsigned long		flags;
 #		define EVENT_TX_HALT	0
 #		define EVENT_RX_HALT	1
-#		define EVENT_RX_MEMORY	2
-#		define EVENT_STS_SPLIT	3
-#		define EVENT_LINK_RESET	4
-#		define EVENT_RX_PAUSED	5
-#		define EVENT_DEV_WAKING 6
-#		define EVENT_DEV_ASLEEP 7
-#		define EVENT_DEV_OPEN	8
+#		define EVENT_STS_HALT	2
+#		define EVENT_RX_MEMORY	3
+#		define EVENT_STS_SPLIT	4
+#		define EVENT_LINK_RESET	5
+#		define EVENT_RX_PAUSED	6
+#		define EVENT_DEV_WAKING 7
+#		define EVENT_DEV_ASLEEP 8
+#		define EVENT_DEV_OPEN	9
 };

 static inline struct usb_driver *driver_of(struct usb_interface *intf)
-- 
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* Re: tcp wifi upload performance and lots of ACKs
From: Glen Turner @ 2012-06-05 14:28 UTC (permalink / raw)
  To: Ben Greear; +Cc: netdev
In-Reply-To: <4FCCFE76.3060304@candelatech.com>


On 05/06/2012, at 3:59 AM, Ben Greear wrote:
> 
> Anyone know of any tuning parameters that would let the receiving socket wait a
> bit longer and send more ACK data in fewer packets?

It's perhaps better to think of Acks as offering an opportunity for the transmitting TCP algorithm to sample the congestion of the path. That's particularly important in Slow Start (when the transmitting algorithm is trying to send data exponentially faster) and both traditional TCP and Linux's CUBIC+Hystart will not ramp up transmission as quickly as they otherwise would when starved of samples.

Of course you've got to weigh slower ramp up in transmission rate against the loss in throughput due to the  turn-around time on the half-duplex wireless media. But I don't see the proposed patch doing any weighing of considerations at all, but punting that to user space which has even less information about the condition of TCP (particularly the receiver's guess as to the slow-start/congestion-avoidance mode of the transmitter) and less information about the attributes of the media (particularly the turn-around time of half-duplex media on the path) to make a decision with.

There's a nice summary of the tradeoffs of delayed Acks in RFC2525 section 2.13, although the discussion is dated (pre-ABC, pre-HyStart).

Note the firm requirement in RFC2581 section 4.2 that an Ack be sent within 500ms. There are implementations in the field which have delayed Acks out to 2s, these are  notoriously painful and many modern algorithms drop back into a low performance mode when facing those receivers.

- Glen

^ permalink raw reply

* Re: Reoccuring kern.log events after running xl2tp with ethernet adapter Realtek 8111E
From: Dustin Schumm @ 2012-06-05 14:56 UTC (permalink / raw)
  To: romieu; +Cc: netdev
In-Reply-To: <CANcVnzi2mXKXZLcaAt9rJ9OOdK_2=V8JV9uebVeBcsDu6j+DhQ@mail.gmail.com>

Let me start off by thanking you for helping with this.
I'm not that experienced with Linux so as to provide all surrounding
information, so let me know if you need more.
Forgive me for sending this in plain test as I got a reject from
netdev for html.

The 816x for Ubuntu on kernel 3.2.0
r8169 0000:0c:00.0: eth0: RTL8168e/8111e at 0xf845e000,
bc:5f:f4:10:75:4b, XID 0c200000 IRQ 58
ethtool info
driver: r8169
version: 2.3LK-NAPI
firmware-version: rtl_nic/rtl8168e-2.fw
bus-info: 0000:0c:00.0

I've also tested with Fedora 17 using kernel 3.3.7
http://codepad.org/lxkcRLqp
This paste is from Fedora. Like Ubuntu, it starts with "general
protection fault: 0000 [#1] SMP" but does not give the other looping
messages after as in Ubuntu.
The 816x for Fedora is
r8169 0000:0c:00.0: eth0: RTL8168e/8111e at 0xf7e78000,
bc:5f:f4:10:75:4b, XID 0c200000 IRQ 60
ethtool info
driver: r8169
version: 2.3LK-NAPI
firmware-version: rtl_nic/rtl8168e-2.fw
bus-info: 0000:0c:00.0

and tested again with Cent OS 6.2 using kernel 2.6.32
Everything was functional without error here. I had replaced CentOS by
the time I got this mail so I hadn't captured the 816x here and forgot
to obtain the kern.log
ethtool info
driver: r8169
version: 2.3LK-NAPI
firmware-version:
bus-info: 0000:0c:00.0


>
> On Mon, Jun 4, 2012 at 7:10 PM, Francois Romieu <romieu@fr.zoreil.com> wrote:
> Dustin Schumm <shodid@gmail.com> :
> [...]
> > I'm more than happy to provide whatever more is needed.
>
> It does not look like our usual 816x bug reports... Neither does your
> setup actually.
>
> Which kind of 816x is it (see 'dmesg | grep XID' output) ?
>
> --
> Ueimor


> On Mon, Jun 4, 2012 at 1:39 PM, Dustin Schumm wrote:
>
> I'm using Ubuntu 12.04 kernel 3.2.0 and trying to set up L2TP over IPSec using openswan and xl2tpd.  I've got no problem with the ipsec and I get SAs established. When I start the L2TP connection with xl2tpd, the kernel log starts filling with looping messages and PC becomes unusable.
>
> I have tested the exact same configuration down to every package in visualization and all is working. I believe it has something to do with my ethernet hardware/driver on this machine or kernel interaction with that device. I'm looking for evidence that points to either the driver or kernel and possible solutions.
>
> I've tried compiling and installing the drivers from Realtek for the RTL8111E integrated adapter and using that, but it produced the same result. I've also tried a backported kernel 3.4.0
>
> I've posted a portion of my kern.log here from where i think the problem starts, http://codepad.org/QFbELkCr
>
> I'm more than happy to provide whatever more is needed.
>
> Thank you,
> Dustin S.

^ permalink raw reply

* RE: [PATCH RFC] c_can_pci: generic module for c_can on PCI
From: AnilKumar, Chimata @ 2012-06-05 15:12 UTC (permalink / raw)
  To: Alessandro Rubini, mkl@pengutronix.de
  Cc: bhupesh.sharma@st.com, federico.vaga@gmail.com,
	alan@lxorguk.ukuu.org.uk, wg@grandegger.com,
	giancarlo.asnaghi@st.com, alan@linux.intel.com,
	linux-can@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <20120605133013.GA16108@mail.gnudd.com>

Hi Alessandro/Federico,

On Tue, Jun 05, 2012 at 19:00:13, Alessandro Rubini wrote:
> > I personally like the "pci device sets up a platform device" idea.
> 
> Good. Than me or federico will submit a proposal. 
>  

I am late to the discussion, is there any specific reason to maintain a
separate platform file (c_can_pci.c). I think 90% of the code is copied
from c_can_paltform.c, code changes will be less if you merge to existing
c_can platform driver. You can look at D_CAN integration to C_CAN driver.

[1] https://gitorious.org/linux-can/linux-can-next

Regards
AnilKumar

^ permalink raw reply

* Re: Reoccuring kern.log events after running xl2tp with ethernet adapter Realtek 8111E
From: Francois Romieu @ 2012-06-05 15:37 UTC (permalink / raw)
  To: Dustin Schumm; +Cc: netdev
In-Reply-To: <CANcVnzivnQrhSderZ+b5yoy+F1OiO7Ajv7WeijqzQKyAkYRqug@mail.gmail.com>

Dustin Schumm <shodid@gmail.com> :
[...]
> Forgive me for sending this in plain test as I got a reject from
> netdev for html.

Please:
- no full quote
- no top answer
- no html

Thanks.

[...]
> I've also tested with Fedora 17 using kernel 3.3.7
> http://codepad.org/lxkcRLqp
> This paste is from Fedora. Like Ubuntu, it starts with "general
> protection fault: 0000 [#1] SMP" but does not give the other looping
> messages after as in Ubuntu.

This is with an usual 1500 bytes MTU and without TSO (see 'ethtool -k ethX'),
right ?

-- 
Ueimor

^ permalink raw reply

* [PATCH] Allow receiving packets on the fallback tunnel if they pass sanity checks
From: Phil Dibowitz @ 2012-06-05 15:40 UTC (permalink / raw)
  To: davem; +Cc: netdev, phild

Ville Nuorvala wrote this patch to solve a problem we were having at Facebook.
He hasn't had time to send it upstream yet, but gave me permission to do it,
and I want to make sure it makes it upstream by World v6 Launch Day.

>From b413062771afbba064ae9bc49b5daed7abb1243d Mon Sep 17 00:00:00 2001
From: Ville Nuorvala <ville.nuorvala@gmail.com>
Subject: [PATCH] Allow receiving packets on the fallback tunnel if they pass sanity checks

The same IPv6 address checks are performed as with any normal tunnel,
but as the fallback tunnel endpoint addresses are unspecified, the checks
must be performed on a per-packet basis, rather than at tunnel
configuration time.

Signed-off-by: Ville Nuorvala <ville.nuorvala@gmail.com>
Tested-by: Phil Dibowitz <phil@ipom.com>

---
 include/net/ip6_tunnel.h |    2 +
 net/ipv6/ip6_tunnel.c    |   65 +++++++++++++++++++++++++++-------------------
 2 files changed, 40 insertions(+), 27 deletions(-)

diff --git a/include/net/ip6_tunnel.h b/include/net/ip6_tunnel.h
index fc73e66..358fb86 100644
--- a/include/net/ip6_tunnel.h
+++ b/include/net/ip6_tunnel.h
@@ -9,6 +9,8 @@
 #define IP6_TNL_F_CAP_XMIT 0x10000
 /* capable of receiving packets */
 #define IP6_TNL_F_CAP_RCV 0x20000
+/* determine capability on a per-packet basis */
+#define IP6_TNL_F_CAP_PER_PACKET 0x40000
 
 /* IPv6 tunnel */
 
diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
index c9015fa..04a3cba 100644
--- a/net/ipv6/ip6_tunnel.c
+++ b/net/ipv6/ip6_tunnel.c
@@ -684,24 +684,50 @@ static void ip6ip6_dscp_ecn_decapsulate(const struct ip6_tnl *t,
 		IP6_ECN_set_ce(ipv6_hdr(skb));
 }
 
+static __u32 ip6_tnl_get_cap(struct ip6_tnl *t,
+			     const struct in6_addr *laddr,
+			     const struct in6_addr *raddr)
+{
+	struct ip6_tnl_parm *p = &t->parms;
+	int ltype = ipv6_addr_type(laddr);
+	int rtype = ipv6_addr_type(raddr);
+	__u32 flags = 0;
+
+	if (ltype == IPV6_ADDR_ANY || rtype == IPV6_ADDR_ANY) {
+		flags = IP6_TNL_F_CAP_PER_PACKET;
+	} else if (ltype & (IPV6_ADDR_UNICAST|IPV6_ADDR_MULTICAST) &&
+		   rtype & (IPV6_ADDR_UNICAST|IPV6_ADDR_MULTICAST) &&
+		   !((ltype|rtype) & IPV6_ADDR_LOOPBACK) &&
+		   (!((ltype|rtype) & IPV6_ADDR_LINKLOCAL) || p->link)) {
+		if (ltype&IPV6_ADDR_UNICAST)
+			flags |= IP6_TNL_F_CAP_XMIT;
+		if (rtype&IPV6_ADDR_UNICAST)
+			flags |= IP6_TNL_F_CAP_RCV;
+	}
+	return flags;
+}
+
 /* called with rcu_read_lock() */
-static inline int ip6_tnl_rcv_ctl(struct ip6_tnl *t)
+static inline int ip6_tnl_rcv_ctl(struct ip6_tnl *t,
+				  const struct in6_addr *laddr,
+				  const struct in6_addr *raddr)
 {
 	struct ip6_tnl_parm *p = &t->parms;
 	int ret = 0;
 	struct net *net = dev_net(t->dev);
 
-	if (p->flags & IP6_TNL_F_CAP_RCV) {
+	if ((p->flags & IP6_TNL_F_CAP_RCV) ||
+	    ((p->flags & IP6_TNL_F_CAP_PER_PACKET) &&
+	     (ip6_tnl_get_cap(t, laddr, raddr) & IP6_TNL_F_CAP_RCV))) {
 		struct net_device *ldev = NULL;
 
 		if (p->link)
 			ldev = dev_get_by_index_rcu(net, p->link);
 
-		if ((ipv6_addr_is_multicast(&p->laddr) ||
-		     likely(ipv6_chk_addr(net, &p->laddr, ldev, 0))) &&
-		    likely(!ipv6_chk_addr(net, &p->raddr, NULL, 0)))
+		if ((ipv6_addr_is_multicast(laddr) ||
+		     likely(ipv6_chk_addr(net, laddr, ldev, 0))) &&
+		    likely(!ipv6_chk_addr(net, raddr, NULL, 0)))
 			ret = 1;
-
 	}
 	return ret;
 }
@@ -740,7 +766,7 @@ static int ip6_tnl_rcv(struct sk_buff *skb, __u16 protocol,
 			goto discard;
 		}
 
-		if (!ip6_tnl_rcv_ctl(t)) {
+		if (!ip6_tnl_rcv_ctl(t, &ipv6h->daddr, &ipv6h->saddr)) {
 			t->dev->stats.rx_dropped++;
 			rcu_read_unlock();
 			goto discard;
@@ -1114,25 +1140,6 @@ tx_err:
 	return NETDEV_TX_OK;
 }
 
-static void ip6_tnl_set_cap(struct ip6_tnl *t)
-{
-	struct ip6_tnl_parm *p = &t->parms;
-	int ltype = ipv6_addr_type(&p->laddr);
-	int rtype = ipv6_addr_type(&p->raddr);
-
-	p->flags &= ~(IP6_TNL_F_CAP_XMIT|IP6_TNL_F_CAP_RCV);
-
-	if (ltype & (IPV6_ADDR_UNICAST|IPV6_ADDR_MULTICAST) &&
-	    rtype & (IPV6_ADDR_UNICAST|IPV6_ADDR_MULTICAST) &&
-	    !((ltype|rtype) & IPV6_ADDR_LOOPBACK) &&
-	    (!((ltype|rtype) & IPV6_ADDR_LINKLOCAL) || p->link)) {
-		if (ltype&IPV6_ADDR_UNICAST)
-			p->flags |= IP6_TNL_F_CAP_XMIT;
-		if (rtype&IPV6_ADDR_UNICAST)
-			p->flags |= IP6_TNL_F_CAP_RCV;
-	}
-}
-
 static void ip6_tnl_link_config(struct ip6_tnl *t)
 {
 	struct net_device *dev = t->dev;
@@ -1153,7 +1160,8 @@ static void ip6_tnl_link_config(struct ip6_tnl *t)
 	if (!(p->flags&IP6_TNL_F_USE_ORIG_FLOWLABEL))
 		fl6->flowlabel |= IPV6_FLOWLABEL_MASK & p->flowinfo;
 
-	ip6_tnl_set_cap(t);
+	p->flags &= ~(IP6_TNL_F_CAP_XMIT|IP6_TNL_F_CAP_RCV|IP6_TNL_F_CAP_PER_PACKET);
+	p->flags |= ip6_tnl_get_cap(t, &p->laddr, &p->raddr);
 
 	if (p->flags&IP6_TNL_F_CAP_XMIT && p->flags&IP6_TNL_F_CAP_RCV)
 		dev->flags |= IFF_POINTOPOINT;
@@ -1438,6 +1446,9 @@ static int __net_init ip6_fb_tnl_dev_init(struct net_device *dev)
 
 	t->parms.proto = IPPROTO_IPV6;
 	dev_hold(dev);
+
+	ip6_tnl_link_config(t);
+
 	rcu_assign_pointer(ip6n->tnls_wc[0], t);
 	return 0;
 }

-- 
Phil Dibowitz                             phil@ipom.com
Open Source software and tech docs        Insanity Palace of Metallica
http://www.phildev.net/                   http://www.ipom.com/

"Be who you are and say what you feel, because those who mind don't matter
 and those who matter don't mind."
 - Dr. Seuss

^ permalink raw reply related

* Re: Reoccuring kern.log events after running xl2tp with ethernet adapter Realtek 8111E
From: Dustin Schumm @ 2012-06-05 15:59 UTC (permalink / raw)
  To: Francois Romieu; +Cc: netdev
In-Reply-To: <20120605153753.GA12514@electric-eye.fr.zoreil.com>

On Tue, Jun 5, 2012 at 11:37 AM, Francois Romieu <romieu@fr.zoreil.com> wrote:
>
> This is with an usual 1500 bytes MTU and without TSO (see 'ethtool -k ethX'),
> right ?
>
> --
> Ueimor

Yes,

MTU:1500
tcp-segmentation-offload: off

^ permalink raw reply

* Re: [PATCH RFC] c_can_pci: generic module for c_can on PCI
From: Alessandro Rubini @ 2012-06-05 16:50 UTC (permalink / raw)
  To: anilkumar
  Cc: mkl, bhupesh.sharma, federico.vaga, alan, wg, giancarlo.asnaghi,
	alan, linux-can, netdev, linux-kernel
In-Reply-To: <331ABD5ECB02734CA317220B2BBEABC13E9CB18A@DBDE01.ent.ti.com>

> I am late to the discussion, is there any specific reason to maintain a
> separate platform file (c_can_pci.c).

Because it depends on pci and ifdef is bad.

> I think 90% of the code is copied from c_can_paltform.c, code
> changes will be less if you merge to existing c_can platform driver.

Yes, but then we need to ifdef around, which merges two bad files
into a single but worse file.

But since the only current user of c_can is the platform device, why
not merging the platform with the core and having pci just register a
platform device?  The only problem I see is that we need cooperation,
because neither me nor federico have a c_can equipped board besides
the pci one.

thanks
/alessandro

^ permalink raw reply

* [PATCH IPROUTE2] tc: Update manpage
From: Vijay Subramanian @ 2012-06-05 17:13 UTC (permalink / raw)
  To: netdev; +Cc: Stephen Hemminger, Eric Dumazet, Vijay Subramanian

Add a reference to fq_codel to the SEE ALSO section.

Signed-off-by: Vijay Subramanian <subramanian.vijay@gmail.com>
---
 man/man8/tc.8 |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/man/man8/tc.8 b/man/man8/tc.8
index 14a1cd8..5d73c98 100644
--- a/man/man8/tc.8
+++ b/man/man8/tc.8
@@ -369,6 +369,7 @@ was written by Alexey N. Kuznetsov and added in Linux 2.2.
 .BR tc-cbq (8),
 .BR tc-choke (8),
 .BR tc-codel (8),
+.BR tc-fq_codel (8),
 .BR tc-drr (8),
 .BR tc-htb (8),
 .BR tc-hfsc (8),
-- 
1.7.0.4

^ permalink raw reply related

* Re: [PATCH 3/3] Revert Backoff [v3]: Calculate TCP's connection close threshold as a time value.
From: Jerry Chu @ 2012-06-05 17:42 UTC (permalink / raw)
  To: Damian Lukowski; +Cc: Netdev, David Miller, Ilpo Järvinen
In-Reply-To: <CAPshTChwnEaMTOsofFDP0ZY8U3VCP2ZrtXjdhKG03rRsx7kryA@mail.gmail.com>

On Mon, Jun 4, 2012 at 4:50 PM, Jerry Chu <hkchu@google.com> wrote:
> Hi Damian,
>
> On Mon, Jun 4, 2012 at 10:50 AM, Damian Lukowski
> <damian@tvk.rwth-aachen.de> wrote:
>> Hi Jerry,
>>
>> please verify, I understood you correctly.
>>
>> You have set TCP_RTO_MIN to a lower value, e.g. 0.002 seconds to improve
>> your internal low-latency traffic. Because of the improvement, R1
>> timeouts are triggered too fast for external high-RTT traffic. Is that
>> correct?
>
> Correct.
>
>> If so, may I suggest to set tcp_retries1 to a higher value? For
>> TCP_RTO_MIN == 0.002 and tcp_retries1 ==  10, R1 will be calculated to
>> approximately 4 seconds.
>
> I think hacking tcp_retries1 is the wrong solution. E.g., 10 retries may be too
> generous for those short RTT flows.
>
> I think the fundamental problem is - the ideal fix for your original RTO revert
> problem should've used the per-flow RTO to compute R1 & R2. But that
> computation may be too expensive so you used TCP_RTO_MIN as an
> approximation - not a good idea IMHO!

Just realized the correct fix of using the original, non-backoff per flow RTO is
not any more expensive than the current code through ilog2(). What's needed
is a new field "base_rto" to record the original RTO before backoff. I'm leaning
toward this more accurate fix now without any fudge because fudging almost
always causes bugs.

Any comment is welcome. I'm not sure in the existing code if it makes sense
to apply the exponential backoff based computation to thin stream but it's a
separate question so I won't touch it.

Jerry

>
> The easiest solution I can see so far is to replace the check
>
> if (!inet_csk(sk)->icsk_retransmits)
>                return false;
>
> at the beginning of retransmits_timed_out() with
>
> if (inet_csk(sk)->icsk_retransmits < boundary)
>                return false;
>
> Best,
>
> Jerry
>
>>
>> Is that ok?
>>
>> Best regards
>>  Damian
>>
>> Am Freitag, den 01.06.2012, 15:58 -0700 schrieb Jerry Chu:
>>> > From: Damian Lukowski <damian@tvk.rwth-aachen.de>
>>> > Date: Wed, Aug 26, 2009 at 3:16 AM
>>> > Subject: [PATCH 3/3] Revert Backoff [v3]: Calculate TCP's connection close
>>> > threshold as a time value.
>>> > To: Netdev <netdev@vger.kernel.org>
>>> >
>>> >
>>> > RFC 1122 specifies two threshold values R1 and R2 for connection timeouts,
>>> > which may represent a number of allowed retransmissions or a timeout value.
>>> > Currently linux uses sysctl_tcp_retries{1,2} to specify the thresholds
>>> > in number of allowed retransmissions.
>>> >
>>> > For any desired threshold R2 (by means of time) one can specify tcp_retries2
>>> > (by means of number of retransmissions) such that TCP will not time out
>>> > earlier than R2. This is the case, because the RTO schedule follows a fixed
>>> > pattern, namely exponential backoff.
>>> >
>>> > However, the RTO behaviour is not predictable any more if RTO backoffs can
>>> > be
>>> > reverted, as it is the case in the draft
>>> > "Make TCP more Robust to Long Connectivity Disruptions"
>>> > (http://tools.ietf.org/html/draft-zimmermann-tcp-lcd).
>>> >
>>> > In the worst case TCP would time out a connection after 3.2 seconds, if the
>>> > initial RTO equaled MIN_RTO and each backoff has been reverted.
>>> >
>>> > This patch introduces a function retransmits_timed_out(N),
>>> > which calculates the timeout of a TCP connection, assuming an initial
>>> > RTO of MIN_RTO and N unsuccessful, exponentially backed-off retransmissions.
>>> >
>>> > Whenever timeout decisions are made by comparing the retransmission counter
>>> > to some value N, this function can be used, instead.
>>> >
>>> > The meaning of tcp_retries2 will be changed, as many more RTO
>>> > retransmissions
>>> > can occur than the value indicates. However, it yields a timeout which is
>>> > similar to the one of an unpatched, exponentially backing off TCP in the
>>> > same
>>> > scenario. As no application could rely on an RTO greater than MIN_RTO, there
>>> > should be no risk of a regression.
>>>
>>> This looks like a typical "fix one problem, introducing a few more" patch :(.
>>> What do you mean by "no application could rely on an RTO greater than
>>> MIN_RTO..."
>>> above? How can you make the assumption that RTO is not too far off
>>> from TCP_RTO_MIN?
>>>
>>> While you tried to address a problem where the retransmission count
>>> was high but the actual
>>> timeout duration was too short, have you considered the other case
>>> around, i.e., the timeout
>>> duration is long but the retransmission count is too short? This is
>>> exactly what's happening
>>> to us with your patch. We've much reduced TCP_RTO_MIN for our internal
>>> traffic, but not
>>> noticing your change has severely shortened the R1 & R2 recommended by
>>> RFC1122 for our
>>> long haul traffic until now. In many cases R1 threshold was met upon
>>> the first retrans timeout.
>>>
>>> I think retransmits_timed_out() should check against both time
>>> duration and retrans count
>>> (icsk_retransmits).
>>>
>>> Thought?
>>>
>>> Jerry
>>>
>>> >
>>> > Signed-off-by: Damian Lukowski <damian@tvk.rwth-aachen.de>
>>> > ---
>>> >  include/net/tcp.h    |   18 ++++++++++++++++++
>>> >  net/ipv4/tcp_timer.c |   11 +++++++----
>>> >  2 files changed, 25 insertions(+), 4 deletions(-)
>>> >
>>> > diff --git a/include/net/tcp.h b/include/net/tcp.h
>>> > index c35b329..17d1a88 100644
>>> > --- a/include/net/tcp.h
>>> > +++ b/include/net/tcp.h
>>> > @@ -1247,6 +1247,24 @@ static inline struct sk_buff
>>> > *tcp_write_queue_prev(struct sock *sk, struct sk_bu
>>> >  #define tcp_for_write_queue_from_safe(skb, tmp, sk)                    \
>>> >        skb_queue_walk_from_safe(&(sk)->sk_write_queue, skb, tmp)
>>> >
>>> > +static inline bool retransmits_timed_out(const struct sock *sk,
>>> > +                                        unsigned int boundary)
>>> > +{
>>> > +       int limit, K;
>>> > +       if (!inet_csk(sk)->icsk_retransmits)
>>> > +               return false;
>>> > +
>>> > +       K = ilog2(TCP_RTO_MAX/TCP_RTO_MIN);
>>> > +
>>> > +       if (boundary <= K)
>>> > +               limit = ((2 << boundary) - 1) * TCP_RTO_MIN;
>>> > +       else
>>> > +               limit = ((2 << K) - 1) * TCP_RTO_MIN +
>>> > +                       (boundary - K) * TCP_RTO_MAX;
>>> > +
>>> > +       return (tcp_time_stamp - tcp_sk(sk)->retrans_stamp) >= limit;
>>> > +}
>>> > +
>>> >  static inline struct sk_buff *tcp_send_head(struct sock *sk)
>>> >  {
>>> >        return sk->sk_send_head;
>>> > diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
>>> > index a3ba494..2972d7b 100644
>>> > --- a/net/ipv4/tcp_timer.c
>>> > +++ b/net/ipv4/tcp_timer.c
>>> > @@ -137,13 +137,14 @@ static int tcp_write_timeout(struct sock *sk)
>>> >  {
>>> >        struct inet_connection_sock *icsk = inet_csk(sk);
>>> >        int retry_until;
>>> > +       bool do_reset;
>>> >
>>> >        if ((1 << sk->sk_state) & (TCPF_SYN_SENT | TCPF_SYN_RECV)) {
>>> >                if (icsk->icsk_retransmits)
>>> >                        dst_negative_advice(&sk->sk_dst_cache);
>>> >                retry_until = icsk->icsk_syn_retries ? :
>>> > sysctl_tcp_syn_retries;
>>> >        } else {
>>> > -               if (icsk->icsk_retransmits >= sysctl_tcp_retries1) {
>>> > +               if (retransmits_timed_out(sk, sysctl_tcp_retries1)) {
>>> >                        /* Black hole detection */
>>> >                        tcp_mtu_probing(icsk, sk);
>>> >
>>> > @@ -155,13 +156,15 @@ static int tcp_write_timeout(struct sock *sk)
>>> >                        const int alive = (icsk->icsk_rto < TCP_RTO_MAX);
>>> >
>>> >                        retry_until = tcp_orphan_retries(sk, alive);
>>> > +                       do_reset = alive ||
>>> > +                                  !retransmits_timed_out(sk, retry_until);
>>> >
>>> > -                       if (tcp_out_of_resources(sk, alive ||
>>> > icsk->icsk_retransmits < retry_until))
>>> > +                       if (tcp_out_of_resources(sk, do_reset))
>>> >                                return 1;
>>> >                }
>>> >        }
>>> >
>>> > -       if (icsk->icsk_retransmits >= retry_until) {
>>> > +       if (retransmits_timed_out(sk, retry_until)) {
>>> >                /* Has it gone just too far? */
>>> >                tcp_write_err(sk);
>>> >                return 1;
>>> > @@ -385,7 +388,7 @@ void tcp_retransmit_timer(struct sock *sk)
>>> >  out_reset_timer:
>>> >        icsk->icsk_rto = min(icsk->icsk_rto << 1, TCP_RTO_MAX);
>>> >        inet_csk_reset_xmit_timer(sk, ICSK_TIME_RETRANS, icsk->icsk_rto,
>>> > TCP_RTO_MAX);
>>> > -       if (icsk->icsk_retransmits > sysctl_tcp_retries1)
>>> > +       if (retransmits_timed_out(sk, sysctl_tcp_retries1 + 1))
>>> >                __sk_dst_reset(sk);
>>> >
>>> >  out:;
>>> > --
>>> > 1.6.3.3
>>> >
>>> > --
>>> > 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
>>> >
>>
>>

^ permalink raw reply

* Re: [PATCH IPROUTE2] tc: Update manpage
From: Jan Ceuleers @ 2012-06-05 17:46 UTC (permalink / raw)
  To: Vijay Subramanian; +Cc: netdev, Stephen Hemminger, Eric Dumazet
In-Reply-To: <1338916400-9190-1-git-send-email-subramanian.vijay@gmail.com>

On 06/05/2012 07:13 PM, Vijay Subramanian wrote:
> @@ -369,6 +369,7 @@ was written by Alexey N. Kuznetsov and added in Linux 2.2.
>  .BR tc-cbq (8),
>  .BR tc-choke (8),
>  .BR tc-codel (8),
> +.BR tc-fq_codel (8),
>  .BR tc-drr (8),
>  .BR tc-htb (8),
>  .BR tc-hfsc (8),

Could I suggest maintaining alphabetic order?

^ permalink raw reply

* [PATCH IPROUTE2 V2] tc: Update manpage
From: Vijay Subramanian @ 2012-06-05 18:41 UTC (permalink / raw)
  To: netdev; +Cc: Stephen Hemminger, Eric Dumazet, jan.ceuleers, Vijay Subramanian

This makes 2 changes:
1: Add fq_codel to SEE ALSO section in tc manpage.
2: Reorder the SEE ALSO section to make the order alphabetical
(suggested by Jan Ceuleers ).

Signed-off-by: Vijay Subramanian <subramanian.vijay@gmail.com>
---
 V2: Reorder the SEE ALSO section to make it alphabetical.

 man/man8/tc.8 |   15 ++++++++-------
 1 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/man/man8/tc.8 b/man/man8/tc.8
index 14a1cd8..958ab98 100644
--- a/man/man8/tc.8
+++ b/man/man8/tc.8
@@ -366,21 +366,22 @@ print rates in IEC units (ie. 1K = 1024).
 .B tc
 was written by Alexey N. Kuznetsov and added in Linux 2.2.
 .SH SEE ALSO
+.BR tc-bfifo (8),
 .BR tc-cbq (8),
 .BR tc-choke (8),
 .BR tc-codel (8),
 .BR tc-drr (8),
-.BR tc-htb (8),
-.BR tc-hfsc (8),
+.BR tc-fq_codel (8),
 .BR tc-hfsc (7),
-.BR tc-sfb (8),
-.BR tc-sfq (8),
-.BR tc-red (8),
-.BR tc-tbf (8),
+.BR tc-hfsc (8),
+.BR tc-htb (8),
 .BR tc-pfifo (8),
-.BR tc-bfifo (8),
 .BR tc-pfifo_fast (8),
+.BR tc-red (8),
+.BR tc-sfb (8),
+.BR tc-sfq (8),
 .BR tc-stab (8),
+.BR tc-tbf (8),
 .br
 .RB "User documentation at " http://lartc.org/ ", but please direct bugreports and patches to: " <netdev@vger.kernel.org>
 
-- 
1.7.0.4

^ permalink raw reply related

* Re: [PATCH 3/3] Revert Backoff [v3]: Calculate TCP's connection close threshold as a time value.
From: Damian Lukowski @ 2012-06-05 18:39 UTC (permalink / raw)
  To: Jerry Chu; +Cc: Netdev, David Miller, Ilpo Järvinen
In-Reply-To: <CAPshTCg8W0QxubvO_dWBsSOqvCEdBSWLKv3cNOvn=VbyqHA1zg@mail.gmail.com>

Am Dienstag, den 05.06.2012, 10:42 -0700 schrieb Jerry Chu:
> On Mon, Jun 4, 2012 at 4:50 PM, Jerry Chu <hkchu@google.com> wrote:
> > Hi Damian,
> >
> > On Mon, Jun 4, 2012 at 10:50 AM, Damian Lukowski
> > <damian@tvk.rwth-aachen.de> wrote:
> >> Hi Jerry,
> >>
> >> please verify, I understood you correctly.
> >>
> >> You have set TCP_RTO_MIN to a lower value, e.g. 0.002 seconds to improve
> >> your internal low-latency traffic. Because of the improvement, R1
> >> timeouts are triggered too fast for external high-RTT traffic. Is that
> >> correct?
> >
> > Correct.
> >
> >> If so, may I suggest to set tcp_retries1 to a higher value? For
> >> TCP_RTO_MIN == 0.002 and tcp_retries1 ==  10, R1 will be calculated to
> >> approximately 4 seconds.
> >
> > I think hacking tcp_retries1 is the wrong solution. E.g., 10 retries may be too
> > generous for those short RTT flows.
> >
> > I think the fundamental problem is - the ideal fix for your original RTO revert
> > problem should've used the per-flow RTO to compute R1 & R2. But that
> > computation may be too expensive so you used TCP_RTO_MIN as an
> > approximation - not a good idea IMHO!
> 
> Just realized the correct fix of using the original, non-backoff per flow RTO is
> not any more expensive than the current code through ilog2(). What's needed
> is a new field "base_rto" to record the original RTO before backoff. I'm leaning
> toward this more accurate fix now without any fudge because fudging almost
> always causes bugs.


The current version of retransmits_timed_out() uses such a field
already. I suppose, we can do a combination like the following?

-       unsigned int rto_base = syn_set ? TCP_TIMEOUT_INIT : TCP_RTO_MIN;
+       unsigned int rto_base = syn_set ? TCP_TIMEOUT_INIT : __tcp_set_rto(tcp_sk(sk));
+       rto_base = rto_base ? : TCP_RTO_MIN;
 
-       if (!inet_csk(sk)->icsk_retransmits)
+       if (inet_csk(sk)->icsk_retransmits < boundary)
 

Regards
 Damian

> 
> Any comment is welcome. I'm not sure in the existing code if it makes sense
> to apply the exponential backoff based computation to thin stream but it's a
> separate question so I won't touch it.
> 
> Jerry
> 
> >
> > The easiest solution I can see so far is to replace the check
> >
> > if (!inet_csk(sk)->icsk_retransmits)
> >                return false;
> >
> > at the beginning of retransmits_timed_out() with
> >
> > if (inet_csk(sk)->icsk_retransmits < boundary)
> >                return false;
> >
> > Best,
> >
> > Jerry
> >
> >>
> >> Is that ok?
> >>
> >> Best regards
> >>  Damian
> >>
> >> Am Freitag, den 01.06.2012, 15:58 -0700 schrieb Jerry Chu:
> >>> > From: Damian Lukowski <damian@tvk.rwth-aachen.de>
> >>> > Date: Wed, Aug 26, 2009 at 3:16 AM
> >>> > Subject: [PATCH 3/3] Revert Backoff [v3]: Calculate TCP's connection close
> >>> > threshold as a time value.
> >>> > To: Netdev <netdev@vger.kernel.org>
> >>> >
> >>> >
> >>> > RFC 1122 specifies two threshold values R1 and R2 for connection timeouts,
> >>> > which may represent a number of allowed retransmissions or a timeout value.
> >>> > Currently linux uses sysctl_tcp_retries{1,2} to specify the thresholds
> >>> > in number of allowed retransmissions.
> >>> >
> >>> > For any desired threshold R2 (by means of time) one can specify tcp_retries2
> >>> > (by means of number of retransmissions) such that TCP will not time out
> >>> > earlier than R2. This is the case, because the RTO schedule follows a fixed
> >>> > pattern, namely exponential backoff.
> >>> >
> >>> > However, the RTO behaviour is not predictable any more if RTO backoffs can
> >>> > be
> >>> > reverted, as it is the case in the draft
> >>> > "Make TCP more Robust to Long Connectivity Disruptions"
> >>> > (http://tools.ietf.org/html/draft-zimmermann-tcp-lcd).
> >>> >
> >>> > In the worst case TCP would time out a connection after 3.2 seconds, if the
> >>> > initial RTO equaled MIN_RTO and each backoff has been reverted.
> >>> >
> >>> > This patch introduces a function retransmits_timed_out(N),
> >>> > which calculates the timeout of a TCP connection, assuming an initial
> >>> > RTO of MIN_RTO and N unsuccessful, exponentially backed-off retransmissions.
> >>> >
> >>> > Whenever timeout decisions are made by comparing the retransmission counter
> >>> > to some value N, this function can be used, instead.
> >>> >
> >>> > The meaning of tcp_retries2 will be changed, as many more RTO
> >>> > retransmissions
> >>> > can occur than the value indicates. However, it yields a timeout which is
> >>> > similar to the one of an unpatched, exponentially backing off TCP in the
> >>> > same
> >>> > scenario. As no application could rely on an RTO greater than MIN_RTO, there
> >>> > should be no risk of a regression.
> >>>
> >>> This looks like a typical "fix one problem, introducing a few more" patch :(.
> >>> What do you mean by "no application could rely on an RTO greater than
> >>> MIN_RTO..."
> >>> above? How can you make the assumption that RTO is not too far off
> >>> from TCP_RTO_MIN?
> >>>
> >>> While you tried to address a problem where the retransmission count
> >>> was high but the actual
> >>> timeout duration was too short, have you considered the other case
> >>> around, i.e., the timeout
> >>> duration is long but the retransmission count is too short? This is
> >>> exactly what's happening
> >>> to us with your patch. We've much reduced TCP_RTO_MIN for our internal
> >>> traffic, but not
> >>> noticing your change has severely shortened the R1 & R2 recommended by
> >>> RFC1122 for our
> >>> long haul traffic until now. In many cases R1 threshold was met upon
> >>> the first retrans timeout.
> >>>
> >>> I think retransmits_timed_out() should check against both time
> >>> duration and retrans count
> >>> (icsk_retransmits).
> >>>
> >>> Thought?
> >>>
> >>> Jerry
> >>>
> >>> >
> >>> > Signed-off-by: Damian Lukowski <damian@tvk.rwth-aachen.de>
> >>> > ---
> >>> >  include/net/tcp.h    |   18 ++++++++++++++++++
> >>> >  net/ipv4/tcp_timer.c |   11 +++++++----
> >>> >  2 files changed, 25 insertions(+), 4 deletions(-)
> >>> >
> >>> > diff --git a/include/net/tcp.h b/include/net/tcp.h
> >>> > index c35b329..17d1a88 100644
> >>> > --- a/include/net/tcp.h
> >>> > +++ b/include/net/tcp.h
> >>> > @@ -1247,6 +1247,24 @@ static inline struct sk_buff
> >>> > *tcp_write_queue_prev(struct sock *sk, struct sk_bu
> >>> >  #define tcp_for_write_queue_from_safe(skb, tmp, sk)                    \
> >>> >        skb_queue_walk_from_safe(&(sk)->sk_write_queue, skb, tmp)
> >>> >
> >>> > +static inline bool retransmits_timed_out(const struct sock *sk,
> >>> > +                                        unsigned int boundary)
> >>> > +{
> >>> > +       int limit, K;
> >>> > +       if (!inet_csk(sk)->icsk_retransmits)
> >>> > +               return false;
> >>> > +
> >>> > +       K = ilog2(TCP_RTO_MAX/TCP_RTO_MIN);
> >>> > +
> >>> > +       if (boundary <= K)
> >>> > +               limit = ((2 << boundary) - 1) * TCP_RTO_MIN;
> >>> > +       else
> >>> > +               limit = ((2 << K) - 1) * TCP_RTO_MIN +
> >>> > +                       (boundary - K) * TCP_RTO_MAX;
> >>> > +
> >>> > +       return (tcp_time_stamp - tcp_sk(sk)->retrans_stamp) >= limit;
> >>> > +}
> >>> > +
> >>> >  static inline struct sk_buff *tcp_send_head(struct sock *sk)
> >>> >  {
> >>> >        return sk->sk_send_head;
> >>> > diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
> >>> > index a3ba494..2972d7b 100644
> >>> > --- a/net/ipv4/tcp_timer.c
> >>> > +++ b/net/ipv4/tcp_timer.c
> >>> > @@ -137,13 +137,14 @@ static int tcp_write_timeout(struct sock *sk)
> >>> >  {
> >>> >        struct inet_connection_sock *icsk = inet_csk(sk);
> >>> >        int retry_until;
> >>> > +       bool do_reset;
> >>> >
> >>> >        if ((1 << sk->sk_state) & (TCPF_SYN_SENT | TCPF_SYN_RECV)) {
> >>> >                if (icsk->icsk_retransmits)
> >>> >                        dst_negative_advice(&sk->sk_dst_cache);
> >>> >                retry_until = icsk->icsk_syn_retries ? :
> >>> > sysctl_tcp_syn_retries;
> >>> >        } else {
> >>> > -               if (icsk->icsk_retransmits >= sysctl_tcp_retries1) {
> >>> > +               if (retransmits_timed_out(sk, sysctl_tcp_retries1)) {
> >>> >                        /* Black hole detection */
> >>> >                        tcp_mtu_probing(icsk, sk);
> >>> >
> >>> > @@ -155,13 +156,15 @@ static int tcp_write_timeout(struct sock *sk)
> >>> >                        const int alive = (icsk->icsk_rto < TCP_RTO_MAX);
> >>> >
> >>> >                        retry_until = tcp_orphan_retries(sk, alive);
> >>> > +                       do_reset = alive ||
> >>> > +                                  !retransmits_timed_out(sk, retry_until);
> >>> >
> >>> > -                       if (tcp_out_of_resources(sk, alive ||
> >>> > icsk->icsk_retransmits < retry_until))
> >>> > +                       if (tcp_out_of_resources(sk, do_reset))
> >>> >                                return 1;
> >>> >                }
> >>> >        }
> >>> >
> >>> > -       if (icsk->icsk_retransmits >= retry_until) {
> >>> > +       if (retransmits_timed_out(sk, retry_until)) {
> >>> >                /* Has it gone just too far? */
> >>> >                tcp_write_err(sk);
> >>> >                return 1;
> >>> > @@ -385,7 +388,7 @@ void tcp_retransmit_timer(struct sock *sk)
> >>> >  out_reset_timer:
> >>> >        icsk->icsk_rto = min(icsk->icsk_rto << 1, TCP_RTO_MAX);
> >>> >        inet_csk_reset_xmit_timer(sk, ICSK_TIME_RETRANS, icsk->icsk_rto,
> >>> > TCP_RTO_MAX);
> >>> > -       if (icsk->icsk_retransmits > sysctl_tcp_retries1)
> >>> > +       if (retransmits_timed_out(sk, sysctl_tcp_retries1 + 1))
> >>> >                __sk_dst_reset(sk);
> >>> >
> >>> >  out:;
> >>> > --
> >>> > 1.6.3.3
> >>> >
> >>> > --
> >>> > 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
> >>> >
> >>
> >>

^ permalink raw reply

* Re: [net-next PATCH 1/3] Added kernel support in EEE Ethtool commands
From: Ben Hutchings @ 2012-06-05 19:01 UTC (permalink / raw)
  To: Yuval Mintz; +Cc: davem, netdev, eilong, peppe.cavallaro
In-Reply-To: <1338878342-24586-2-git-send-email-yuvalmin@broadcom.com>

On Tue, 2012-06-05 at 09:39 +0300, Yuval Mintz wrote:
> This patch extends the kernel's ethtool interface by adding support
> for 2 new EEE commands - get_eee and set_eee.
> 
> Thanks goes to Giuseppe Cavallaro for his original patch adding this support.
> 
> Signed-off-by: Yuval Mintz <yuvalmin@broadcom.com>
> Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
> ---
>  include/linux/ethtool.h |   19 +++++++++++++++++++
>  net/core/ethtool.c      |   34 ++++++++++++++++++++++++++++++++++
>  2 files changed, 53 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
> index e17fa71..527de4c 100644
> --- a/include/linux/ethtool.h
> +++ b/include/linux/ethtool.h
> @@ -136,6 +136,19 @@ struct ethtool_eeprom {
>  	__u8	data[0];
>  };
>  
> +/* EEE settings */
> +struct ethtool_eee {
> +	__u32	cmd;
> +	__u32	supported;
> +	__u32	advertised;
> +	__u32	lp_advertised;
> +	__u32	eee_active;
> +	__u32	eee_enabled;
> +	__u32	tx_lpi_enabled;
> +	__u32	tx_lpi_timer;
> +	__u32	reserved[2];
> +};

This needs a kernel-doc comment explaining exactly what each of the
fields means.

[...]
> --- a/net/core/ethtool.c
> +++ b/net/core/ethtool.c
> @@ -729,6 +729,34 @@ static int ethtool_set_wol(struct net_device *dev, char __user *useraddr)
>  	return dev->ethtool_ops->set_wol(dev, &wol);
>  }
>  
> +static int ethtool_get_eee(struct net_device *dev, char __user *useraddr)
> +{
> +	struct ethtool_eee edata;
> +
> +	if (!dev->ethtool_ops->get_eee)
> +		return -EOPNOTSUPP;

We *must* initialise all of edata and we should not leave it to the
driver to do.  Otherwise we will be copying back uninitialised data to
userland which (1) might reveal sensitive information stored on the
stack by previous function calls (2) sets the cmd field to the wrong
value (3) sets the reserved fields to random values, making them
unusable for expansion.

> +	dev->ethtool_ops->get_eee(dev, &edata);

Missing error check.

> +	if (copy_to_user(useraddr, &edata, sizeof(edata)))
> +		return -EFAULT;
> +
> +	return 0;
> +}
[...]

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox