Netdev List
 help / color / mirror / Atom feed
* [PATCH 0/9] ax88796: cleanups and convert to phylib and mdio_bitbang
From: Marc Kleine-Budde @ 2011-02-21 13:03 UTC (permalink / raw)
  To: netdev; +Cc: Ben Dooks, Daniel Mack

Hello,

this patch series fixes the phy-read/write problems of the ax88796
(see http://www.spinics.net/lists/arm-kernel/msg98982.html).

Patches 1-8 clean verious aspects of the driver. The 9th patch replaces the
handcrafted mdio bitbang loop with the generic mdio_bitbang driver.

This patch series has been tested on the Toradex colibri-320. With the patch
mii-diag gives sound data:

root@grabowski:~ mii-diag
Using the default interface 'eth0'.
Basic registers of MII PHY #16:  3100 782d 003b 1841 01e1 45e1 0003 0000.
 The autonegotiated capability is 01e0.
The autonegotiated media type is 100baseTx-FD.
 Basic mode control register 0x3100: Auto-negotiation enabled.
 You have link beat, and everything is working OK.
 Your link partner advertised 45e1: Flow-control 100baseTx-FD 100baseTx 10baseT-FD 10baseT, w/ 802.3X flow control.
   End of basic transceiver information.

please review and consider to appply.

regards, Marc

---

The series applies to net-next-2.6/master and can be pulled:

The following changes since commit 59ed5aba9ca1c799e272b352d5d2d7fe12bd32e8:

  sctp: fix compile warnings in sctp_tsnmap_num_gabs (2011-02-20 11:10:15 -0800)

are available in the git repository at:
  git://git.pengutronix.de/git/mkl/linux-2.6.git net/ax88796

Marc Kleine-Budde (9):
      ax88796: fix codingstyle and checkpatch warnings
      ax88796: don't use magic ei_status to acces private data
      ax88796: remove memset of private data
      ax88796: remove first_init parameter from ax_init_dev()
      ax88796: use netdev_<LEVEL> instead of dev_<LEVEL> and pr_<LEVEL>
      ax88796: remove platform_device member from struct ax_device
      ax88796: make pointer to platform data const
      ax88796: clean up probe and remove function
      ax88796: use generic mdio_bitbang driver

 drivers/net/Kconfig   |    4 +-
 drivers/net/ax88796.c |  810 ++++++++++++++++++++++++-------------------------
 2 files changed, 395 insertions(+), 419 deletions(-)




^ permalink raw reply

* [PATCH 4/9] ax88796: remove first_init parameter from ax_init_dev()
From: Marc Kleine-Budde @ 2011-02-21 13:03 UTC (permalink / raw)
  To: netdev; +Cc: Ben Dooks, Daniel Mack, Marc Kleine-Budde
In-Reply-To: <1298293400-21570-1-git-send-email-mkl@pengutronix.de>

ax_init_dev() is always called with first_init=1.

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/ax88796.c |   44 +++++++++++++++++++-------------------------
 1 files changed, 19 insertions(+), 25 deletions(-)

diff --git a/drivers/net/ax88796.c b/drivers/net/ax88796.c
index 885f04e..eac5b10 100644
--- a/drivers/net/ax88796.c
+++ b/drivers/net/ax88796.c
@@ -675,7 +675,7 @@ static void ax_initial_setup(struct net_device *dev, struct ei_device *ei_local)
  * the device is ready to be used by lib8390.c and registerd with
  * the network layer.
  */
-static int ax_init_dev(struct net_device *dev, int first_init)
+static int ax_init_dev(struct net_device *dev)
 {
 	struct ei_device *ei_local = netdev_priv(dev);
 	struct ax_device *ax = to_ax_dev(dev);
@@ -695,7 +695,7 @@ static int ax_init_dev(struct net_device *dev, int first_init)
 
 	/* read the mac from the card prom if we need it */
 
-	if (first_init && ax->plat->flags & AXFLG_HAS_EEPROM) {
+	if (ax->plat->flags & AXFLG_HAS_EEPROM) {
 		unsigned char SA_prom[32];
 
 		for (i = 0; i < sizeof(SA_prom); i += 2) {
@@ -711,7 +711,7 @@ static int ax_init_dev(struct net_device *dev, int first_init)
 	}
 
 #ifdef CONFIG_AX88796_93CX6
-	if (first_init && ax->plat->flags & AXFLG_HAS_93CX6) {
+	if (ax->plat->flags & AXFLG_HAS_93CX6) {
 		unsigned char mac_addr[6];
 		struct eeprom_93cx6 eeprom;
 
@@ -737,25 +737,20 @@ static int ax_init_dev(struct net_device *dev, int first_init)
 		stop_page = NE1SM_STOP_PG;
 	}
 
-	/*
-	 * load the mac-address from the device if this is the first
-	 * time we've initialised
-	 */
-	if (first_init) {
-		if (ax->plat->flags & AXFLG_MAC_FROMDEV) {
-			ei_outb(E8390_NODMA + E8390_PAGE1 + E8390_STOP,
-				ei_local->mem + E8390_CMD); /* 0x61 */
-			for (i = 0; i < ETHER_ADDR_LEN; i++)
-				dev->dev_addr[i] =
-					ei_inb(ioaddr + EN1_PHYS_SHIFT(i));
-		}
-
-		if ((ax->plat->flags & AXFLG_MAC_FROMPLATFORM) &&
-		     ax->plat->mac_addr)
-			memcpy(dev->dev_addr, ax->plat->mac_addr,
-				ETHER_ADDR_LEN);
+	/* load the mac-address from the device */
+	if (ax->plat->flags & AXFLG_MAC_FROMDEV) {
+		ei_outb(E8390_NODMA + E8390_PAGE1 + E8390_STOP,
+			ei_local->mem + E8390_CMD); /* 0x61 */
+		for (i = 0; i < ETHER_ADDR_LEN; i++)
+			dev->dev_addr[i] =
+				ei_inb(ioaddr + EN1_PHYS_SHIFT(i));
 	}
 
+	if ((ax->plat->flags & AXFLG_MAC_FROMPLATFORM) &&
+	    ax->plat->mac_addr)
+		memcpy(dev->dev_addr, ax->plat->mac_addr,
+		       ETHER_ADDR_LEN);
+
 	ax_reset_8390(dev);
 
 	ei_local->name = "AX88796";
@@ -790,10 +785,9 @@ static int ax_init_dev(struct net_device *dev, int first_init)
 
 	ax_NS8390_init(dev, 0);
 
-	if (first_init)
-		dev_info(&ax->dev->dev, "%dbit, irq %d, %lx, MAC: %pM\n",
-			 ei_local->word16 ? 16 : 8, dev->irq, dev->base_addr,
-			 dev->dev_addr);
+	dev_info(&ax->dev->dev, "%dbit, irq %d, %lx, MAC: %pM\n",
+		 ei_local->word16 ? 16 : 8, dev->irq, dev->base_addr,
+		 dev->dev_addr);
 
 	ret = register_netdev(dev);
 	if (ret)
@@ -949,7 +943,7 @@ static int ax_probe(struct platform_device *pdev)
 
 	/* got resources, now initialise and register device */
 
-	ret = ax_init_dev(dev, 1);
+	ret = ax_init_dev(dev);
 	if (!ret)
 		return 0;
 
-- 
1.7.2.3


^ permalink raw reply related

* [PATCH 3/9] ax88796: remove memset of private data
From: Marc Kleine-Budde @ 2011-02-21 13:03 UTC (permalink / raw)
  To: netdev; +Cc: Ben Dooks, Daniel Mack, Marc Kleine-Budde
In-Reply-To: <1298293400-21570-1-git-send-email-mkl@pengutronix.de>

It's allocated via alloc_netdev, thus already zeroed.

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/ax88796.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ax88796.c b/drivers/net/ax88796.c
index c49c3b1..885f04e 100644
--- a/drivers/net/ax88796.c
+++ b/drivers/net/ax88796.c
@@ -858,8 +858,6 @@ static int ax_probe(struct platform_device *pdev)
 	ei_local = netdev_priv(dev);
 	ax = to_ax_dev(dev);
 
-	memset(ax, 0, sizeof(struct ax_device));
-
 	spin_lock_init(&ax->mii_lock);
 
 	ax->dev = pdev;
-- 
1.7.2.3


^ permalink raw reply related

* [PATCH 1/9] ax88796: fix codingstyle and checkpatch warnings
From: Marc Kleine-Budde @ 2011-02-21 13:03 UTC (permalink / raw)
  To: netdev; +Cc: Ben Dooks, Daniel Mack, Marc Kleine-Budde
In-Reply-To: <1298293400-21570-1-git-send-email-mkl@pengutronix.de>

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/ax88796.c |  262 +++++++++++++++++++++++++------------------------
 1 files changed, 134 insertions(+), 128 deletions(-)

diff --git a/drivers/net/ax88796.c b/drivers/net/ax88796.c
index 4bebff3..6d1d5ca 100644
--- a/drivers/net/ax88796.c
+++ b/drivers/net/ax88796.c
@@ -9,7 +9,7 @@
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2 as
  * published by the Free Software Foundation.
-*/
+ */
 
 #include <linux/module.h>
 #include <linux/kernel.h>
@@ -17,6 +17,7 @@
 #include <linux/isapnp.h>
 #include <linux/init.h>
 #include <linux/interrupt.h>
+#include <linux/io.h>
 #include <linux/platform_device.h>
 #include <linux/delay.h>
 #include <linux/timer.h>
@@ -30,33 +31,32 @@
 #include <net/ax88796.h>
 
 #include <asm/system.h>
-#include <asm/io.h>
 
-static int phy_debug = 0;
+static int phy_debug;
 
 /* Rename the lib8390.c functions to show that they are in this driver */
-#define __ei_open       ax_ei_open
-#define __ei_close      ax_ei_close
-#define __ei_poll	ax_ei_poll
+#define __ei_open ax_ei_open
+#define __ei_close ax_ei_close
+#define __ei_poll ax_ei_poll
 #define __ei_start_xmit ax_ei_start_xmit
 #define __ei_tx_timeout ax_ei_tx_timeout
-#define __ei_get_stats  ax_ei_get_stats
+#define __ei_get_stats ax_ei_get_stats
 #define __ei_set_multicast_list ax_ei_set_multicast_list
-#define __ei_interrupt  ax_ei_interrupt
+#define __ei_interrupt ax_ei_interrupt
 #define ____alloc_ei_netdev ax__alloc_ei_netdev
-#define __NS8390_init   ax_NS8390_init
+#define __NS8390_init ax_NS8390_init
 
 /* force unsigned long back to 'void __iomem *' */
 #define ax_convert_addr(_a) ((void __force __iomem *)(_a))
 
-#define ei_inb(_a)	readb(ax_convert_addr(_a))
+#define ei_inb(_a) readb(ax_convert_addr(_a))
 #define ei_outb(_v, _a) writeb(_v, ax_convert_addr(_a))
 
-#define ei_inb_p(_a)	ei_inb(_a)
+#define ei_inb_p(_a) ei_inb(_a)
 #define ei_outb_p(_v, _a) ei_outb(_v, _a)
 
 /* define EI_SHIFT() to take into account our register offsets */
-#define EI_SHIFT(x)     (ei_local->reg_offset[(x)])
+#define EI_SHIFT(x) (ei_local->reg_offset[(x)])
 
 /* Ensure we have our RCR base value */
 #define AX88796_PLATFORM
@@ -74,43 +74,43 @@ static unsigned char version[] = "ax88796.c: Copyright 2005,2007 Simtec Electron
 #define NE_DATAPORT	EI_SHIFT(0x10)
 
 #define NE1SM_START_PG	0x20	/* First page of TX buffer */
-#define NE1SM_STOP_PG 	0x40	/* Last page +1 of RX ring */
+#define NE1SM_STOP_PG	0x40	/* Last page +1 of RX ring */
 #define NESM_START_PG	0x40	/* First page of TX buffer */
 #define NESM_STOP_PG	0x80	/* Last page +1 of RX ring */
 
 /* device private data */
 
 struct ax_device {
-	struct timer_list	 mii_timer;
-	spinlock_t		 mii_lock;
-	struct mii_if_info	 mii;
-
-	u32			 msg_enable;
-	void __iomem		*map2;
-	struct platform_device	*dev;
-	struct resource		*mem;
-	struct resource		*mem2;
-	struct ax_plat_data	*plat;
-
-	unsigned char		 running;
-	unsigned char		 resume_open;
-	unsigned int		 irqflags;
-
-	u32			 reg_offsets[0x20];
+	struct timer_list mii_timer;
+	spinlock_t mii_lock;
+	struct mii_if_info mii;
+
+	u32 msg_enable;
+	void __iomem *map2;
+	struct platform_device *dev;
+	struct resource *mem;
+	struct resource *mem2;
+	struct ax_plat_data *plat;
+
+	unsigned char running;
+	unsigned char resume_open;
+	unsigned int irqflags;
+
+	u32 reg_offsets[0x20];
 };
 
 static inline struct ax_device *to_ax_dev(struct net_device *dev)
 {
 	struct ei_device *ei_local = netdev_priv(dev);
-	return (struct ax_device *)(ei_local+1);
+	return (struct ax_device *)(ei_local + 1);
 }
 
-/* ax_initial_check
+/*
+ * ax_initial_check
  *
  * do an initial probe for the card to check wether it exists
  * and is functional
  */
-
 static int ax_initial_check(struct net_device *dev)
 {
 	struct ei_device *ei_local = netdev_priv(dev);
@@ -122,10 +122,10 @@ static int ax_initial_check(struct net_device *dev)
 	if (reg0 == 0xFF)
 		return -ENODEV;
 
-	ei_outb(E8390_NODMA+E8390_PAGE1+E8390_STOP, ioaddr + E8390_CMD);
+	ei_outb(E8390_NODMA + E8390_PAGE1 + E8390_STOP, ioaddr + E8390_CMD);
 	regd = ei_inb(ioaddr + 0x0d);
 	ei_outb(0xff, ioaddr + 0x0d);
-	ei_outb(E8390_NODMA+E8390_PAGE0, ioaddr + E8390_CMD);
+	ei_outb(E8390_NODMA + E8390_PAGE0, ioaddr + E8390_CMD);
 	ei_inb(ioaddr + EN0_COUNTER0); /* Clear the counter by reading. */
 	if (ei_inb(ioaddr + EN0_COUNTER0) != 0) {
 		ei_outb(reg0, ioaddr);
@@ -136,13 +136,14 @@ static int ax_initial_check(struct net_device *dev)
 	return 0;
 }
 
-/* Hard reset the card.  This used to pause for the same period that a
-   8390 reset command required, but that shouldn't be necessary. */
-
+/*
+ * Hard reset the card. This used to pause for the same period that a
+ * 8390 reset command required, but that shouldn't be necessary.
+ */
 static void ax_reset_8390(struct net_device *dev)
 {
 	struct ei_device *ei_local = netdev_priv(dev);
-	struct ax_device  *ax = to_ax_dev(dev);
+	struct ax_device *ax = to_ax_dev(dev);
 	unsigned long reset_start_time = jiffies;
 	void __iomem *addr = (void __iomem *)dev->base_addr;
 
@@ -156,7 +157,7 @@ static void ax_reset_8390(struct net_device *dev)
 
 	/* This check _should_not_ be necessary, omit eventually. */
 	while ((ei_inb(addr + EN0_ISR) & ENISR_RESET) == 0) {
-		if (jiffies - reset_start_time > 2*HZ/100) {
+		if (jiffies - reset_start_time > 2 * HZ / 100) {
 			dev_warn(&ax->dev->dev, "%s: %s did not complete.\n",
 			       __func__, dev->name);
 			break;
@@ -171,7 +172,7 @@ static void ax_get_8390_hdr(struct net_device *dev, struct e8390_pkt_hdr *hdr,
 			    int ring_page)
 {
 	struct ei_device *ei_local = netdev_priv(dev);
-	struct ax_device  *ax = to_ax_dev(dev);
+	struct ax_device *ax = to_ax_dev(dev);
 	void __iomem *nic_base = ei_local->mem;
 
 	/* This *shouldn't* happen. If it does, it's the last thing you'll see */
@@ -184,7 +185,7 @@ static void ax_get_8390_hdr(struct net_device *dev, struct e8390_pkt_hdr *hdr,
 	}
 
 	ei_status.dmaing |= 0x01;
-	ei_outb(E8390_NODMA+E8390_PAGE0+E8390_START, nic_base+ NE_CMD);
+	ei_outb(E8390_NODMA + E8390_PAGE0 + E8390_START, nic_base + NE_CMD);
 	ei_outb(sizeof(struct e8390_pkt_hdr), nic_base + EN0_RCNTLO);
 	ei_outb(0, nic_base + EN0_RCNTHI);
 	ei_outb(0, nic_base + EN0_RSARLO);		/* On page boundary */
@@ -192,9 +193,11 @@ static void ax_get_8390_hdr(struct net_device *dev, struct e8390_pkt_hdr *hdr,
 	ei_outb(E8390_RREAD+E8390_START, nic_base + NE_CMD);
 
 	if (ei_status.word16)
-		readsw(nic_base + NE_DATAPORT, hdr, sizeof(struct e8390_pkt_hdr)>>1);
+		readsw(nic_base + NE_DATAPORT, hdr,
+		       sizeof(struct e8390_pkt_hdr) >> 1);
 	else
-		readsb(nic_base + NE_DATAPORT, hdr, sizeof(struct e8390_pkt_hdr));
+		readsb(nic_base + NE_DATAPORT, hdr,
+		       sizeof(struct e8390_pkt_hdr));
 
 	ei_outb(ENISR_RDC, nic_base + EN0_ISR);	/* Ack intr. */
 	ei_status.dmaing &= ~0x01;
@@ -203,16 +206,18 @@ static void ax_get_8390_hdr(struct net_device *dev, struct e8390_pkt_hdr *hdr,
 }
 
 
-/* Block input and output, similar to the Crynwr packet driver.  If you
-   are porting to a new ethercard, look at the packet driver source for hints.
-   The NEx000 doesn't share the on-board packet memory -- you have to put
-   the packet out through the "remote DMA" dataport using ei_outb. */
-
+/*
+ * Block input and output, similar to the Crynwr packet driver. If
+ * you are porting to a new ethercard, look at the packet driver
+ * source for hints. The NEx000 doesn't share the on-board packet
+ * memory -- you have to put the packet out through the "remote DMA"
+ * dataport using ei_outb.
+ */
 static void ax_block_input(struct net_device *dev, int count,
 			   struct sk_buff *skb, int ring_offset)
 {
 	struct ei_device *ei_local = netdev_priv(dev);
-	struct ax_device  *ax = to_ax_dev(dev);
+	struct ax_device *ax = to_ax_dev(dev);
 	void __iomem *nic_base = ei_local->mem;
 	char *buf = skb->data;
 
@@ -227,7 +232,7 @@ static void ax_block_input(struct net_device *dev, int count,
 
 	ei_status.dmaing |= 0x01;
 
-	ei_outb(E8390_NODMA+E8390_PAGE0+E8390_START, nic_base+ NE_CMD);
+	ei_outb(E8390_NODMA+E8390_PAGE0+E8390_START, nic_base + NE_CMD);
 	ei_outb(count & 0xff, nic_base + EN0_RCNTLO);
 	ei_outb(count >> 8, nic_base + EN0_RCNTHI);
 	ei_outb(ring_offset & 0xff, nic_base + EN0_RSARLO);
@@ -250,14 +255,15 @@ static void ax_block_output(struct net_device *dev, int count,
 			    const unsigned char *buf, const int start_page)
 {
 	struct ei_device *ei_local = netdev_priv(dev);
-	struct ax_device  *ax = to_ax_dev(dev);
+	struct ax_device *ax = to_ax_dev(dev);
 	void __iomem *nic_base = ei_local->mem;
 	unsigned long dma_start;
 
-	/* Round the count up for word writes.  Do we need to do this?
-	   What effect will an odd byte count have on the 8390?
-	   I should check someday. */
-
+	/*
+	 * Round the count up for word writes. Do we need to do this?
+	 * What effect will an odd byte count have on the 8390?  I
+	 * should check someday.
+	 */
 	if (ei_status.word16 && (count & 0x01))
 		count++;
 
@@ -278,25 +284,24 @@ static void ax_block_output(struct net_device *dev, int count,
 
 	/* Now the normal output. */
 	ei_outb(count & 0xff, nic_base + EN0_RCNTLO);
-	ei_outb(count >> 8,   nic_base + EN0_RCNTHI);
+	ei_outb(count >> 8, nic_base + EN0_RCNTHI);
 	ei_outb(0x00, nic_base + EN0_RSARLO);
 	ei_outb(start_page, nic_base + EN0_RSARHI);
 
 	ei_outb(E8390_RWRITE+E8390_START, nic_base + NE_CMD);
-	if (ei_status.word16) {
-		writesw(nic_base + NE_DATAPORT, buf, count>>1);
-	} else {
+	if (ei_status.word16)
+		writesw(nic_base + NE_DATAPORT, buf, count >> 1);
+	else
 		writesb(nic_base + NE_DATAPORT, buf, count);
-	}
 
 	dma_start = jiffies;
 
 	while ((ei_inb(nic_base + EN0_ISR) & ENISR_RDC) == 0) {
-		if (jiffies - dma_start > 2*HZ/100) {		/* 20ms */
+		if (jiffies - dma_start > 2 * HZ / 100) {		/* 20ms */
 			dev_warn(&ax->dev->dev,
 				 "%s: timeout waiting for Tx RDC.\n", dev->name);
 			ax_reset_8390(dev);
-			ax_NS8390_init(dev,1);
+			ax_NS8390_init(dev, 1);
 			break;
 		}
 	}
@@ -308,20 +313,20 @@ static void ax_block_output(struct net_device *dev, int count,
 /* definitions for accessing MII/EEPROM interface */
 
 #define AX_MEMR			EI_SHIFT(0x14)
-#define AX_MEMR_MDC		(1<<0)
-#define AX_MEMR_MDIR		(1<<1)
-#define AX_MEMR_MDI		(1<<2)
-#define AX_MEMR_MDO		(1<<3)
-#define AX_MEMR_EECS		(1<<4)
-#define AX_MEMR_EEI		(1<<5)
-#define AX_MEMR_EEO		(1<<6)
-#define AX_MEMR_EECLK		(1<<7)
-
-/* ax_mii_ei_outbits
+#define AX_MEMR_MDC		BIT(0)
+#define AX_MEMR_MDIR		BIT(1)
+#define AX_MEMR_MDI		BIT(2)
+#define AX_MEMR_MDO		BIT(3)
+#define AX_MEMR_EECS		BIT(4)
+#define AX_MEMR_EEI		BIT(5)
+#define AX_MEMR_EEO		BIT(6)
+#define AX_MEMR_EECLK		BIT(7)
+
+/*
+ * ax_mii_ei_outbits
  *
  * write the specified set of bits to the phy
-*/
-
+ */
 static void
 ax_mii_ei_outbits(struct net_device *dev, unsigned int bits, int len)
 {
@@ -356,11 +361,11 @@ ax_mii_ei_outbits(struct net_device *dev, unsigned int bits, int len)
 	ei_outb(memr, (void __iomem *)dev->base_addr + AX_MEMR);
 }
 
-/* ax_phy_ei_inbits
+/*
+ * ax_phy_ei_inbits
  *
  * read a specified number of bits from the phy
-*/
-
+ */
 static unsigned int
 ax_phy_ei_inbits(struct net_device *dev, int no)
 {
@@ -381,7 +386,7 @@ ax_phy_ei_inbits(struct net_device *dev, int no)
 		udelay(1);
 
 		if (ei_inb(memr_addr) & AX_MEMR_MDI)
-			result |= (1<<no);
+			result |= (1 << no);
 
 		ei_outb(memr, memr_addr);
 	}
@@ -389,12 +394,12 @@ ax_phy_ei_inbits(struct net_device *dev, int no)
 	return result;
 }
 
-/* ax_phy_issueaddr
+/*
+ * ax_phy_issueaddr
  *
  * use the low level bit shifting routines to send the address
  * and command to the specified phy
-*/
-
+ */
 static void
 ax_phy_issueaddr(struct net_device *dev, int phy_addr, int reg, int opc)
 {
@@ -414,16 +419,16 @@ ax_phy_read(struct net_device *dev, int phy_addr, int reg)
 {
 	struct ei_device *ei_local = netdev_priv(dev);
 	unsigned long flags;
- 	unsigned int result;
+	unsigned int result;
 
-      	spin_lock_irqsave(&ei_local->page_lock, flags);
+	spin_lock_irqsave(&ei_local->page_lock, flags);
 
 	ax_phy_issueaddr(dev, phy_addr, reg, 2);
 
 	result = ax_phy_ei_inbits(dev, 17);
-	result &= ~(3<<16);
+	result &= ~(3 << 16);
 
-      	spin_unlock_irqrestore(&ei_local->page_lock, flags);
+	spin_unlock_irqrestore(&ei_local->page_lock, flags);
 
 	if (phy_debug)
 		pr_debug("%s: %04x.%04x => read %04x\n", __func__,
@@ -436,25 +441,25 @@ static void
 ax_phy_write(struct net_device *dev, int phy_addr, int reg, int value)
 {
 	struct ei_device *ei = netdev_priv(dev);
-	struct ax_device  *ax = to_ax_dev(dev);
+	struct ax_device *ax = to_ax_dev(dev);
 	unsigned long flags;
 
 	dev_dbg(&ax->dev->dev, "%s: %p, %04x, %04x %04x\n",
 		__func__, dev, phy_addr, reg, value);
 
-      	spin_lock_irqsave(&ei->page_lock, flags);
+	spin_lock_irqsave(&ei->page_lock, flags);
 
 	ax_phy_issueaddr(dev, phy_addr, reg, 1);
 	ax_mii_ei_outbits(dev, 2, 2);		/* send TA */
 	ax_mii_ei_outbits(dev, value, 16);
 
-      	spin_unlock_irqrestore(&ei->page_lock, flags);
+	spin_unlock_irqrestore(&ei->page_lock, flags);
 }
 
 static void ax_mii_expiry(unsigned long data)
 {
 	struct net_device *dev = (struct net_device *)data;
-	struct ax_device  *ax = to_ax_dev(dev);
+	struct ax_device *ax = to_ax_dev(dev);
 	unsigned long flags;
 
 	spin_lock_irqsave(&ax->mii_lock, flags);
@@ -469,7 +474,7 @@ static void ax_mii_expiry(unsigned long data)
 
 static int ax_open(struct net_device *dev)
 {
-	struct ax_device  *ax = to_ax_dev(dev);
+	struct ax_device *ax = to_ax_dev(dev);
 	struct ei_device *ei_local = netdev_priv(dev);
 	int ret;
 
@@ -495,8 +500,8 @@ static int ax_open(struct net_device *dev)
 
 	init_timer(&ax->mii_timer);
 
-	ax->mii_timer.expires  = jiffies+1;
-	ax->mii_timer.data     = (unsigned long) dev;
+	ax->mii_timer.expires = jiffies + 1;
+	ax->mii_timer.data = (unsigned long) dev;
 	ax->mii_timer.function = ax_mii_expiry;
 
 	add_timer(&ax->mii_timer);
@@ -513,7 +518,7 @@ static int ax_close(struct net_device *dev)
 
 	/* turn the phy off */
 
-	ei_outb(ax->plat->gpoc_val | (1<<6),
+	ei_outb(ax->plat->gpoc_val | (1 << 6),
 	       ei_local->mem + EI_SHIFT(0x17));
 
 	ax->running = 0;
@@ -640,7 +645,7 @@ static const struct net_device_ops ax_netdev_ops = {
 	.ndo_get_stats		= ax_ei_get_stats,
 	.ndo_set_multicast_list = ax_ei_set_multicast_list,
 	.ndo_validate_addr	= eth_validate_addr,
-	.ndo_set_mac_address 	= eth_mac_addr,
+	.ndo_set_mac_address	= eth_mac_addr,
 	.ndo_change_mtu		= eth_change_mtu,
 #ifdef CONFIG_NET_POLL_CONTROLLER
 	.ndo_poll_controller	= ax_ei_poll,
@@ -654,22 +659,22 @@ static void ax_initial_setup(struct net_device *dev, struct ei_device *ei_local)
 	void __iomem *ioaddr = ei_local->mem;
 	struct ax_device *ax = to_ax_dev(dev);
 
-	/* Select page 0*/
-	ei_outb(E8390_NODMA+E8390_PAGE0+E8390_STOP, ioaddr + E8390_CMD);
+	/* Select page 0 */
+	ei_outb(E8390_NODMA + E8390_PAGE0 + E8390_STOP, ioaddr + E8390_CMD);
 
 	/* set to byte access */
 	ei_outb(ax->plat->dcr_val & ~1, ioaddr + EN0_DCFG);
 	ei_outb(ax->plat->gpoc_val, ioaddr + EI_SHIFT(0x17));
 }
 
-/* ax_init_dev
+/*
+ * ax_init_dev
  *
  * initialise the specified device, taking care to note the MAC
  * address it may already have (if configured), ensure
  * the device is ready to be used by lib8390.c and registerd with
  * the network layer.
  */
-
 static int ax_init_dev(struct net_device *dev, int first_init)
 {
 	struct ei_device *ei_local = netdev_priv(dev);
@@ -693,16 +698,16 @@ static int ax_init_dev(struct net_device *dev, int first_init)
 	if (first_init && ax->plat->flags & AXFLG_HAS_EEPROM) {
 		unsigned char SA_prom[32];
 
-		for(i = 0; i < sizeof(SA_prom); i+=2) {
+		for (i = 0; i < sizeof(SA_prom); i += 2) {
 			SA_prom[i] = ei_inb(ioaddr + NE_DATAPORT);
-			SA_prom[i+1] = ei_inb(ioaddr + NE_DATAPORT);
+			SA_prom[i + 1] = ei_inb(ioaddr + NE_DATAPORT);
 		}
 
 		if (ax->plat->wordlength == 2)
 			for (i = 0; i < 16; i++)
 				SA_prom[i] = SA_prom[i+i];
 
-		memcpy(dev->dev_addr,  SA_prom, 6);
+		memcpy(dev->dev_addr, SA_prom, 6);
 	}
 
 #ifdef CONFIG_AX88796_93CX6
@@ -719,7 +724,7 @@ static int ax_init_dev(struct net_device *dev, int first_init)
 				       (__le16 __force *)mac_addr,
 				       sizeof(mac_addr) >> 1);
 
-		memcpy(dev->dev_addr,  mac_addr, 6);
+		memcpy(dev->dev_addr, mac_addr, 6);
 	}
 #endif
 	if (ax->plat->wordlength == 2) {
@@ -732,9 +737,10 @@ static int ax_init_dev(struct net_device *dev, int first_init)
 		stop_page = NE1SM_STOP_PG;
 	}
 
-	/* load the mac-address from the device if this is the
-	 * first time we've initialised */
-
+	/*
+	 * load the mac-address from the device if this is the first
+	 * time we've initialised
+	 */
 	if (first_init) {
 		if (ax->plat->flags & AXFLG_MAC_FROMDEV) {
 			ei_outb(E8390_NODMA + E8390_PAGE1 + E8390_STOP,
@@ -759,7 +765,7 @@ static int ax_init_dev(struct net_device *dev, int first_init)
 	ei_status.rx_start_page = start_page + TX_PAGES;
 
 #ifdef PACKETBUF_MEMSIZE
-	 /* Allow the packet buffer size to be overridden by know-it-alls. */
+	/* Allow the packet buffer size to be overridden by know-it-alls. */
 	ei_status.stop_page = ei_status.tx_start_page + PACKETBUF_MEMSIZE;
 #endif
 
@@ -769,8 +775,8 @@ static int ax_init_dev(struct net_device *dev, int first_init)
 	ei_status.get_8390_hdr	= &ax_get_8390_hdr;
 	ei_status.priv = 0;
 
-	dev->netdev_ops		= &ax_netdev_ops;
-	dev->ethtool_ops	= &ax_ethtool_ops;
+	dev->netdev_ops = &ax_netdev_ops;
+	dev->ethtool_ops = &ax_ethtool_ops;
 
 	ax->msg_enable		= NETIF_MSG_LINK;
 	ax->mii.phy_id_mask	= 0x1f;
@@ -786,7 +792,7 @@ static int ax_init_dev(struct net_device *dev, int first_init)
 
 	if (first_init)
 		dev_info(&ax->dev->dev, "%dbit, irq %d, %lx, MAC: %pM\n",
-			 ei_status.word16 ? 16:8, dev->irq, dev->base_addr,
+			 ei_status.word16 ? 16 : 8, dev->irq, dev->base_addr,
 			 dev->dev_addr);
 
 	ret = register_netdev(dev);
@@ -805,7 +811,7 @@ static int ax_init_dev(struct net_device *dev, int first_init)
 static int ax_remove(struct platform_device *_dev)
 {
 	struct net_device *dev = platform_get_drvdata(_dev);
-	struct ax_device  *ax;
+	struct ax_device *ax;
 
 	ax = to_ax_dev(dev);
 
@@ -827,18 +833,18 @@ static int ax_remove(struct platform_device *_dev)
 	return 0;
 }
 
-/* ax_probe
+/*
+ * ax_probe
  *
  * This is the entry point when the platform device system uses to
- * notify us of a new device to attach to. Allocate memory, find
- * the resources and information passed, and map the necessary registers.
-*/
-
+ * notify us of a new device to attach to. Allocate memory, find the
+ * resources and information passed, and map the necessary registers.
+ */
 static int ax_probe(struct platform_device *pdev)
 {
 	struct net_device *dev;
-	struct ax_device  *ax;
-	struct resource   *res;
+	struct ax_device *ax;
+	struct resource *res;
 	size_t size;
 	int ret = 0;
 
@@ -857,10 +863,9 @@ static int ax_probe(struct platform_device *pdev)
 	ax->plat = pdev->dev.platform_data;
 	platform_set_drvdata(pdev, dev);
 
-	ei_status.rxcr_base  = ax->plat->rcr_val;
+	ei_status.rxcr_base = ax->plat->rcr_val;
 
 	/* find the platform resources */
-
 	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
 	if (res == NULL) {
 		dev_err(&pdev->dev, "no IRQ specified\n");
@@ -880,9 +885,10 @@ static int ax_probe(struct platform_device *pdev)
 
 	size = (res->end - res->start) + 1;
 
-	/* setup the register offsets from either the platform data
-	 * or by using the size of the resource provided */
-
+	/*
+	 * setup the register offsets from either the platform data or
+	 * by using the size of the resource provided
+	 */
 	if (ax->plat->reg_offsets)
 		ei_status.reg_offset = ax->plat->reg_offsets;
 	else {
@@ -894,7 +900,7 @@ static int ax_probe(struct platform_device *pdev)
 	ax->mem = request_mem_region(res->start, size, pdev->name);
 	if (ax->mem == NULL) {
 		dev_err(&pdev->dev, "cannot reserve registers\n");
- 		ret = -ENXIO;
+		ret = -ENXIO;
 		goto exit_mem;
 	}
 
@@ -906,7 +912,7 @@ static int ax_probe(struct platform_device *pdev)
 			(unsigned long long)res->start,
 			(unsigned long long)res->end);
 
- 		ret = -ENXIO;
+		ret = -ENXIO;
 		goto exit_req;
 	}
 
@@ -921,7 +927,7 @@ static int ax_probe(struct platform_device *pdev)
 
 		ax->map2 = NULL;
 	} else {
- 		size = (res->end - res->start) + 1;
+		size = (res->end - res->start) + 1;
 
 		ax->mem2 = request_mem_region(res->start, size, pdev->name);
 		if (ax->mem2 == NULL) {
@@ -974,7 +980,7 @@ static int ax_probe(struct platform_device *pdev)
 static int ax_suspend(struct platform_device *dev, pm_message_t state)
 {
 	struct net_device *ndev = platform_get_drvdata(dev);
-	struct ax_device  *ax = to_ax_dev(ndev);
+	struct ax_device *ax = to_ax_dev(ndev);
 
 	ax->resume_open = ax->running;
 
@@ -987,7 +993,7 @@ static int ax_suspend(struct platform_device *dev, pm_message_t state)
 static int ax_resume(struct platform_device *pdev)
 {
 	struct net_device *ndev = platform_get_drvdata(pdev);
-	struct ax_device  *ax = to_ax_dev(ndev);
+	struct ax_device *ax = to_ax_dev(ndev);
 
 	ax_initial_setup(ndev, netdev_priv(ndev));
 	ax_NS8390_init(ndev, ax->resume_open);
@@ -1001,7 +1007,7 @@ static int ax_resume(struct platform_device *pdev)
 
 #else
 #define ax_suspend NULL
-#define ax_resume  NULL
+#define ax_resume NULL
 #endif
 
 static struct platform_driver axdrv = {
-- 
1.7.2.3


^ permalink raw reply related

* [PATCH 2/9] ax88796: don't use magic ei_status to acces private data
From: Marc Kleine-Budde @ 2011-02-21 13:03 UTC (permalink / raw)
  To: netdev; +Cc: Ben Dooks, Daniel Mack, Marc Kleine-Budde
In-Reply-To: <1298293400-21570-1-git-send-email-mkl@pengutronix.de>

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/ax88796.c |   81 +++++++++++++++++++++++++-----------------------
 1 files changed, 42 insertions(+), 39 deletions(-)

diff --git a/drivers/net/ax88796.c b/drivers/net/ax88796.c
index 6d1d5ca..c49c3b1 100644
--- a/drivers/net/ax88796.c
+++ b/drivers/net/ax88796.c
@@ -152,8 +152,8 @@ static void ax_reset_8390(struct net_device *dev)
 
 	ei_outb(ei_inb(addr + NE_RESET), addr + NE_RESET);
 
-	ei_status.txing = 0;
-	ei_status.dmaing = 0;
+	ei_local->txing = 0;
+	ei_local->dmaing = 0;
 
 	/* This check _should_not_ be necessary, omit eventually. */
 	while ((ei_inb(addr + EN0_ISR) & ENISR_RESET) == 0) {
@@ -176,15 +176,15 @@ static void ax_get_8390_hdr(struct net_device *dev, struct e8390_pkt_hdr *hdr,
 	void __iomem *nic_base = ei_local->mem;
 
 	/* This *shouldn't* happen. If it does, it's the last thing you'll see */
-	if (ei_status.dmaing) {
+	if (ei_local->dmaing) {
 		dev_err(&ax->dev->dev, "%s: DMAing conflict in %s "
 			"[DMAstat:%d][irqlock:%d].\n",
 			dev->name, __func__,
-			ei_status.dmaing, ei_status.irqlock);
+			ei_local->dmaing, ei_local->irqlock);
 		return;
 	}
 
-	ei_status.dmaing |= 0x01;
+	ei_local->dmaing |= 0x01;
 	ei_outb(E8390_NODMA + E8390_PAGE0 + E8390_START, nic_base + NE_CMD);
 	ei_outb(sizeof(struct e8390_pkt_hdr), nic_base + EN0_RCNTLO);
 	ei_outb(0, nic_base + EN0_RCNTHI);
@@ -192,7 +192,7 @@ static void ax_get_8390_hdr(struct net_device *dev, struct e8390_pkt_hdr *hdr,
 	ei_outb(ring_page, nic_base + EN0_RSARHI);
 	ei_outb(E8390_RREAD+E8390_START, nic_base + NE_CMD);
 
-	if (ei_status.word16)
+	if (ei_local->word16)
 		readsw(nic_base + NE_DATAPORT, hdr,
 		       sizeof(struct e8390_pkt_hdr) >> 1);
 	else
@@ -200,7 +200,7 @@ static void ax_get_8390_hdr(struct net_device *dev, struct e8390_pkt_hdr *hdr,
 		       sizeof(struct e8390_pkt_hdr));
 
 	ei_outb(ENISR_RDC, nic_base + EN0_ISR);	/* Ack intr. */
-	ei_status.dmaing &= ~0x01;
+	ei_local->dmaing &= ~0x01;
 
 	le16_to_cpus(&hdr->count);
 }
@@ -221,16 +221,16 @@ static void ax_block_input(struct net_device *dev, int count,
 	void __iomem *nic_base = ei_local->mem;
 	char *buf = skb->data;
 
-	if (ei_status.dmaing) {
+	if (ei_local->dmaing) {
 		dev_err(&ax->dev->dev,
 			"%s: DMAing conflict in %s "
 			"[DMAstat:%d][irqlock:%d].\n",
 			dev->name, __func__,
-			ei_status.dmaing, ei_status.irqlock);
+			ei_local->dmaing, ei_local->irqlock);
 		return;
 	}
 
-	ei_status.dmaing |= 0x01;
+	ei_local->dmaing |= 0x01;
 
 	ei_outb(E8390_NODMA+E8390_PAGE0+E8390_START, nic_base + NE_CMD);
 	ei_outb(count & 0xff, nic_base + EN0_RCNTLO);
@@ -239,7 +239,7 @@ static void ax_block_input(struct net_device *dev, int count,
 	ei_outb(ring_offset >> 8, nic_base + EN0_RSARHI);
 	ei_outb(E8390_RREAD+E8390_START, nic_base + NE_CMD);
 
-	if (ei_status.word16) {
+	if (ei_local->word16) {
 		readsw(nic_base + NE_DATAPORT, buf, count >> 1);
 		if (count & 0x01)
 			buf[count-1] = ei_inb(nic_base + NE_DATAPORT);
@@ -248,7 +248,7 @@ static void ax_block_input(struct net_device *dev, int count,
 		readsb(nic_base + NE_DATAPORT, buf, count);
 	}
 
-	ei_status.dmaing &= ~1;
+	ei_local->dmaing &= ~1;
 }
 
 static void ax_block_output(struct net_device *dev, int count,
@@ -264,19 +264,19 @@ static void ax_block_output(struct net_device *dev, int count,
 	 * What effect will an odd byte count have on the 8390?  I
 	 * should check someday.
 	 */
-	if (ei_status.word16 && (count & 0x01))
+	if (ei_local->word16 && (count & 0x01))
 		count++;
 
 	/* This *shouldn't* happen. If it does, it's the last thing you'll see */
-	if (ei_status.dmaing) {
+	if (ei_local->dmaing) {
 		dev_err(&ax->dev->dev, "%s: DMAing conflict in %s."
 			"[DMAstat:%d][irqlock:%d]\n",
 			dev->name, __func__,
-		       ei_status.dmaing, ei_status.irqlock);
+		       ei_local->dmaing, ei_local->irqlock);
 		return;
 	}
 
-	ei_status.dmaing |= 0x01;
+	ei_local->dmaing |= 0x01;
 	/* We should already be in page 0, but to be safe... */
 	ei_outb(E8390_PAGE0+E8390_START+E8390_NODMA, nic_base + NE_CMD);
 
@@ -289,7 +289,7 @@ static void ax_block_output(struct net_device *dev, int count,
 	ei_outb(start_page, nic_base + EN0_RSARHI);
 
 	ei_outb(E8390_RWRITE+E8390_START, nic_base + NE_CMD);
-	if (ei_status.word16)
+	if (ei_local->word16)
 		writesw(nic_base + NE_DATAPORT, buf, count >> 1);
 	else
 		writesb(nic_base + NE_DATAPORT, buf, count);
@@ -307,7 +307,7 @@ static void ax_block_output(struct net_device *dev, int count,
 	}
 
 	ei_outb(ENISR_RDC, nic_base + EN0_ISR);	/* Ack intr. */
-	ei_status.dmaing &= ~0x01;
+	ei_local->dmaing &= ~0x01;
 }
 
 /* definitions for accessing MII/EEPROM interface */
@@ -758,22 +758,22 @@ static int ax_init_dev(struct net_device *dev, int first_init)
 
 	ax_reset_8390(dev);
 
-	ei_status.name = "AX88796";
-	ei_status.tx_start_page = start_page;
-	ei_status.stop_page = stop_page;
-	ei_status.word16 = (ax->plat->wordlength == 2);
-	ei_status.rx_start_page = start_page + TX_PAGES;
+	ei_local->name = "AX88796";
+	ei_local->tx_start_page = start_page;
+	ei_local->stop_page = stop_page;
+	ei_local->word16 = (ax->plat->wordlength == 2);
+	ei_local->rx_start_page = start_page + TX_PAGES;
 
 #ifdef PACKETBUF_MEMSIZE
 	/* Allow the packet buffer size to be overridden by know-it-alls. */
-	ei_status.stop_page = ei_status.tx_start_page + PACKETBUF_MEMSIZE;
+	ei_local->stop_page = ei_local->tx_start_page + PACKETBUF_MEMSIZE;
 #endif
 
-	ei_status.reset_8390	= &ax_reset_8390;
-	ei_status.block_input	= &ax_block_input;
-	ei_status.block_output	= &ax_block_output;
-	ei_status.get_8390_hdr	= &ax_get_8390_hdr;
-	ei_status.priv = 0;
+	ei_local->reset_8390 = &ax_reset_8390;
+	ei_local->block_input = &ax_block_input;
+	ei_local->block_output = &ax_block_output;
+	ei_local->get_8390_hdr = &ax_get_8390_hdr;
+	ei_local->priv = 0;
 
 	dev->netdev_ops = &ax_netdev_ops;
 	dev->ethtool_ops = &ax_ethtool_ops;
@@ -792,7 +792,7 @@ static int ax_init_dev(struct net_device *dev, int first_init)
 
 	if (first_init)
 		dev_info(&ax->dev->dev, "%dbit, irq %d, %lx, MAC: %pM\n",
-			 ei_status.word16 ? 16 : 8, dev->irq, dev->base_addr,
+			 ei_local->word16 ? 16 : 8, dev->irq, dev->base_addr,
 			 dev->dev_addr);
 
 	ret = register_netdev(dev);
@@ -811,6 +811,7 @@ static int ax_init_dev(struct net_device *dev, int first_init)
 static int ax_remove(struct platform_device *_dev)
 {
 	struct net_device *dev = platform_get_drvdata(_dev);
+	struct ei_device *ei_local = netdev_priv(dev);
 	struct ax_device *ax;
 
 	ax = to_ax_dev(dev);
@@ -818,7 +819,7 @@ static int ax_remove(struct platform_device *_dev)
 	unregister_netdev(dev);
 	free_irq(dev->irq, dev);
 
-	iounmap(ei_status.mem);
+	iounmap(ei_local->mem);
 	release_resource(ax->mem);
 	kfree(ax->mem);
 
@@ -843,6 +844,7 @@ static int ax_remove(struct platform_device *_dev)
 static int ax_probe(struct platform_device *pdev)
 {
 	struct net_device *dev;
+	struct ei_device *ei_local;
 	struct ax_device *ax;
 	struct resource *res;
 	size_t size;
@@ -853,6 +855,7 @@ static int ax_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	/* ok, let's setup our device */
+	ei_local = netdev_priv(dev);
 	ax = to_ax_dev(dev);
 
 	memset(ax, 0, sizeof(struct ax_device));
@@ -863,7 +866,7 @@ static int ax_probe(struct platform_device *pdev)
 	ax->plat = pdev->dev.platform_data;
 	platform_set_drvdata(pdev, dev);
 
-	ei_status.rxcr_base = ax->plat->rcr_val;
+	ei_local->rxcr_base = ax->plat->rcr_val;
 
 	/* find the platform resources */
 	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
@@ -890,9 +893,9 @@ static int ax_probe(struct platform_device *pdev)
 	 * by using the size of the resource provided
 	 */
 	if (ax->plat->reg_offsets)
-		ei_status.reg_offset = ax->plat->reg_offsets;
+		ei_local->reg_offset = ax->plat->reg_offsets;
 	else {
-		ei_status.reg_offset = ax->reg_offsets;
+		ei_local->reg_offset = ax->reg_offsets;
 		for (ret = 0; ret < 0x18; ret++)
 			ax->reg_offsets[ret] = (size / 0x18) * ret;
 	}
@@ -904,10 +907,10 @@ static int ax_probe(struct platform_device *pdev)
 		goto exit_mem;
 	}
 
-	ei_status.mem = ioremap(res->start, size);
-	dev->base_addr = (unsigned long)ei_status.mem;
+	ei_local->mem = ioremap(res->start, size);
+	dev->base_addr = (unsigned long)ei_local->mem;
 
-	if (ei_status.mem == NULL) {
+	if (ei_local->mem == NULL) {
 		dev_err(&pdev->dev, "Cannot ioremap area (%08llx,%08llx)\n",
 			(unsigned long long)res->start,
 			(unsigned long long)res->end);
@@ -943,7 +946,7 @@ static int ax_probe(struct platform_device *pdev)
 			goto exit_mem2;
 		}
 
-		ei_status.reg_offset[0x1f] = ax->map2 - ei_status.mem;
+		ei_local->reg_offset[0x1f] = ax->map2 - ei_local->mem;
 	}
 
 	/* got resources, now initialise and register device */
@@ -962,7 +965,7 @@ static int ax_probe(struct platform_device *pdev)
 	kfree(ax->mem2);
 
  exit_mem1:
-	iounmap(ei_status.mem);
+	iounmap(ei_local->mem);
 
  exit_req:
 	release_resource(ax->mem);
-- 
1.7.2.3


^ permalink raw reply related

* [PATCH 5/9] ax88796: use netdev_<LEVEL> instead of dev_<LEVEL> and pr_<LEVEL>
From: Marc Kleine-Budde @ 2011-02-21 13:03 UTC (permalink / raw)
  To: netdev; +Cc: Ben Dooks, Daniel Mack, Marc Kleine-Budde
In-Reply-To: <1298293400-21570-1-git-send-email-mkl@pengutronix.de>

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/ax88796.c |   46 ++++++++++++++++++++--------------------------
 1 files changed, 20 insertions(+), 26 deletions(-)

diff --git a/drivers/net/ax88796.c b/drivers/net/ax88796.c
index eac5b10..1370cac 100644
--- a/drivers/net/ax88796.c
+++ b/drivers/net/ax88796.c
@@ -143,12 +143,11 @@ static int ax_initial_check(struct net_device *dev)
 static void ax_reset_8390(struct net_device *dev)
 {
 	struct ei_device *ei_local = netdev_priv(dev);
-	struct ax_device *ax = to_ax_dev(dev);
 	unsigned long reset_start_time = jiffies;
 	void __iomem *addr = (void __iomem *)dev->base_addr;
 
 	if (ei_debug > 1)
-		dev_dbg(&ax->dev->dev, "resetting the 8390 t=%ld\n", jiffies);
+		netdev_dbg(dev, "resetting the 8390 t=%ld\n", jiffies);
 
 	ei_outb(ei_inb(addr + NE_RESET), addr + NE_RESET);
 
@@ -158,8 +157,7 @@ static void ax_reset_8390(struct net_device *dev)
 	/* This check _should_not_ be necessary, omit eventually. */
 	while ((ei_inb(addr + EN0_ISR) & ENISR_RESET) == 0) {
 		if (jiffies - reset_start_time > 2 * HZ / 100) {
-			dev_warn(&ax->dev->dev, "%s: %s did not complete.\n",
-			       __func__, dev->name);
+			netdev_warn(dev, "%s: did not complete.\n", __func__);
 			break;
 		}
 	}
@@ -172,14 +170,13 @@ static void ax_get_8390_hdr(struct net_device *dev, struct e8390_pkt_hdr *hdr,
 			    int ring_page)
 {
 	struct ei_device *ei_local = netdev_priv(dev);
-	struct ax_device *ax = to_ax_dev(dev);
 	void __iomem *nic_base = ei_local->mem;
 
 	/* This *shouldn't* happen. If it does, it's the last thing you'll see */
 	if (ei_local->dmaing) {
-		dev_err(&ax->dev->dev, "%s: DMAing conflict in %s "
+		netdev_err(dev, "DMAing conflict in %s "
 			"[DMAstat:%d][irqlock:%d].\n",
-			dev->name, __func__,
+			__func__,
 			ei_local->dmaing, ei_local->irqlock);
 		return;
 	}
@@ -217,15 +214,14 @@ static void ax_block_input(struct net_device *dev, int count,
 			   struct sk_buff *skb, int ring_offset)
 {
 	struct ei_device *ei_local = netdev_priv(dev);
-	struct ax_device *ax = to_ax_dev(dev);
 	void __iomem *nic_base = ei_local->mem;
 	char *buf = skb->data;
 
 	if (ei_local->dmaing) {
-		dev_err(&ax->dev->dev,
-			"%s: DMAing conflict in %s "
+		netdev_err(dev,
+			"DMAing conflict in %s "
 			"[DMAstat:%d][irqlock:%d].\n",
-			dev->name, __func__,
+			__func__,
 			ei_local->dmaing, ei_local->irqlock);
 		return;
 	}
@@ -255,7 +251,6 @@ static void ax_block_output(struct net_device *dev, int count,
 			    const unsigned char *buf, const int start_page)
 {
 	struct ei_device *ei_local = netdev_priv(dev);
-	struct ax_device *ax = to_ax_dev(dev);
 	void __iomem *nic_base = ei_local->mem;
 	unsigned long dma_start;
 
@@ -269,9 +264,9 @@ static void ax_block_output(struct net_device *dev, int count,
 
 	/* This *shouldn't* happen. If it does, it's the last thing you'll see */
 	if (ei_local->dmaing) {
-		dev_err(&ax->dev->dev, "%s: DMAing conflict in %s."
+		netdev_err(dev, "DMAing conflict in %s."
 			"[DMAstat:%d][irqlock:%d]\n",
-			dev->name, __func__,
+			__func__,
 		       ei_local->dmaing, ei_local->irqlock);
 		return;
 	}
@@ -298,8 +293,7 @@ static void ax_block_output(struct net_device *dev, int count,
 
 	while ((ei_inb(nic_base + EN0_ISR) & ENISR_RDC) == 0) {
 		if (jiffies - dma_start > 2 * HZ / 100) {		/* 20ms */
-			dev_warn(&ax->dev->dev,
-				 "%s: timeout waiting for Tx RDC.\n", dev->name);
+			netdev_warn(dev, "timeout waiting for Tx RDC.\n");
 			ax_reset_8390(dev);
 			ax_NS8390_init(dev, 1);
 			break;
@@ -404,7 +398,7 @@ static void
 ax_phy_issueaddr(struct net_device *dev, int phy_addr, int reg, int opc)
 {
 	if (phy_debug)
-		pr_debug("%s: dev %p, %04x, %04x, %d\n",
+		netdev_dbg(dev, "%s: dev %p, %04x, %04x, %d\n",
 			__func__, dev, phy_addr, reg, opc);
 
 	ax_mii_ei_outbits(dev, 0x3f, 6);	/* pre-amble */
@@ -431,7 +425,7 @@ ax_phy_read(struct net_device *dev, int phy_addr, int reg)
 	spin_unlock_irqrestore(&ei_local->page_lock, flags);
 
 	if (phy_debug)
-		pr_debug("%s: %04x.%04x => read %04x\n", __func__,
+		netdev_dbg(dev, "%s: %04x.%04x => read %04x\n", __func__,
 			 phy_addr, reg, result);
 
 	return result;
@@ -441,10 +435,9 @@ static void
 ax_phy_write(struct net_device *dev, int phy_addr, int reg, int value)
 {
 	struct ei_device *ei = netdev_priv(dev);
-	struct ax_device *ax = to_ax_dev(dev);
 	unsigned long flags;
 
-	dev_dbg(&ax->dev->dev, "%s: %p, %04x, %04x %04x\n",
+	netdev_dbg(dev, "%s: %p, %04x, %04x %04x\n",
 		__func__, dev, phy_addr, reg, value);
 
 	spin_lock_irqsave(&ei->page_lock, flags);
@@ -478,7 +471,7 @@ static int ax_open(struct net_device *dev)
 	struct ei_device *ei_local = netdev_priv(dev);
 	int ret;
 
-	dev_dbg(&ax->dev->dev, "%s: open\n", dev->name);
+	netdev_dbg(dev, "open\n");
 
 	ret = request_irq(dev->irq, ax_ei_interrupt, ax->irqflags,
 			  dev->name, dev);
@@ -514,7 +507,7 @@ static int ax_close(struct net_device *dev)
 	struct ax_device *ax = to_ax_dev(dev);
 	struct ei_device *ei_local = netdev_priv(dev);
 
-	dev_dbg(&ax->dev->dev, "%s: close\n", dev->name);
+	netdev_dbg(dev, "close\n");
 
 	/* turn the phy off */
 
@@ -785,14 +778,14 @@ static int ax_init_dev(struct net_device *dev)
 
 	ax_NS8390_init(dev, 0);
 
-	dev_info(&ax->dev->dev, "%dbit, irq %d, %lx, MAC: %pM\n",
-		 ei_local->word16 ? 16 : 8, dev->irq, dev->base_addr,
-		 dev->dev_addr);
-
 	ret = register_netdev(dev);
 	if (ret)
 		goto out_irq;
 
+	netdev_info(dev, "%dbit, irq %d, %lx, MAC: %pM\n",
+		    ei_local->word16 ? 16 : 8, dev->irq, dev->base_addr,
+		    dev->dev_addr);
+
 	return 0;
 
  out_irq:
@@ -849,6 +842,7 @@ static int ax_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	/* ok, let's setup our device */
+	SET_NETDEV_DEV(dev, &pdev->dev);
 	ei_local = netdev_priv(dev);
 	ax = to_ax_dev(dev);
 
-- 
1.7.2.3


^ permalink raw reply related

* [PATCH 8/9] ax88796: clean up probe and remove function
From: Marc Kleine-Budde @ 2011-02-21 13:03 UTC (permalink / raw)
  To: netdev; +Cc: Ben Dooks, Daniel Mack, Marc Kleine-Budde
In-Reply-To: <1298293400-21570-1-git-send-email-mkl@pengutronix.de>

This way we can remove the struct resource pointers from the private data.

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/ax88796.c |   75 ++++++++++++++++++++----------------------------
 1 files changed, 31 insertions(+), 44 deletions(-)

diff --git a/drivers/net/ax88796.c b/drivers/net/ax88796.c
index fd289e5..e62d0ba 100644
--- a/drivers/net/ax88796.c
+++ b/drivers/net/ax88796.c
@@ -87,8 +87,6 @@ struct ax_device {
 
 	u32 msg_enable;
 	void __iomem *map2;
-	struct resource *mem;
-	struct resource *mem2;
 	const struct ax_plat_data *plat;
 
 	unsigned char running;
@@ -794,25 +792,24 @@ static int ax_init_dev(struct net_device *dev)
 	return ret;
 }
 
-static int ax_remove(struct platform_device *_dev)
+static int ax_remove(struct platform_device *pdev)
 {
-	struct net_device *dev = platform_get_drvdata(_dev);
+	struct net_device *dev = platform_get_drvdata(pdev);
 	struct ei_device *ei_local = netdev_priv(dev);
-	struct ax_device *ax;
-
-	ax = to_ax_dev(dev);
+	struct ax_device *ax = to_ax_dev(dev);
+	struct resource *mem;
 
 	unregister_netdev(dev);
 	free_irq(dev->irq, dev);
 
 	iounmap(ei_local->mem);
-	release_resource(ax->mem);
-	kfree(ax->mem);
+	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	release_mem_region(mem->start, resource_size(mem));
 
 	if (ax->map2) {
 		iounmap(ax->map2);
-		release_resource(ax->mem2);
-		kfree(ax->mem2);
+		mem = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+		release_mem_region(mem->start, resource_size(mem));
 	}
 
 	free_netdev(dev);
@@ -832,8 +829,8 @@ static int ax_probe(struct platform_device *pdev)
 	struct net_device *dev;
 	struct ei_device *ei_local;
 	struct ax_device *ax;
-	struct resource *res;
-	size_t size;
+	struct resource *irq, *mem, *mem2;
+	resource_size_t mem_size, mem2_size = 0;
 	int ret = 0;
 
 	dev = ax__alloc_ei_netdev(sizeof(struct ax_device));
@@ -853,24 +850,24 @@ static int ax_probe(struct platform_device *pdev)
 	ei_local->rxcr_base = ax->plat->rcr_val;
 
 	/* find the platform resources */
-	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
-	if (res == NULL) {
+	irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+	if (!irq) {
 		dev_err(&pdev->dev, "no IRQ specified\n");
 		ret = -ENXIO;
 		goto exit_mem;
 	}
 
-	dev->irq = res->start;
-	ax->irqflags = res->flags & IRQF_TRIGGER_MASK;
+	dev->irq = irq->start;
+	ax->irqflags = irq->flags & IRQF_TRIGGER_MASK;
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (res == NULL) {
+	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!mem) {
 		dev_err(&pdev->dev, "no MEM specified\n");
 		ret = -ENXIO;
 		goto exit_mem;
 	}
 
-	size = (res->end - res->start) + 1;
+	mem_size = resource_size(mem);
 
 	/*
 	 * setup the register offsets from either the platform data or
@@ -881,50 +878,43 @@ static int ax_probe(struct platform_device *pdev)
 	else {
 		ei_local->reg_offset = ax->reg_offsets;
 		for (ret = 0; ret < 0x18; ret++)
-			ax->reg_offsets[ret] = (size / 0x18) * ret;
+			ax->reg_offsets[ret] = (mem_size / 0x18) * ret;
 	}
 
-	ax->mem = request_mem_region(res->start, size, pdev->name);
-	if (ax->mem == NULL) {
+	if (!request_mem_region(mem->start, mem_size, pdev->name)) {
 		dev_err(&pdev->dev, "cannot reserve registers\n");
 		ret = -ENXIO;
 		goto exit_mem;
 	}
 
-	ei_local->mem = ioremap(res->start, size);
+	ei_local->mem = ioremap(mem->start, mem_size);
 	dev->base_addr = (unsigned long)ei_local->mem;
 
 	if (ei_local->mem == NULL) {
-		dev_err(&pdev->dev, "Cannot ioremap area (%08llx,%08llx)\n",
-			(unsigned long long)res->start,
-			(unsigned long long)res->end);
+		dev_err(&pdev->dev, "Cannot ioremap area %pR\n", mem);
 
 		ret = -ENXIO;
 		goto exit_req;
 	}
 
 	/* look for reset area */
-
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
-	if (res == NULL) {
+	mem2 = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	if (!mem2) {
 		if (!ax->plat->reg_offsets) {
 			for (ret = 0; ret < 0x20; ret++)
-				ax->reg_offsets[ret] = (size / 0x20) * ret;
+				ax->reg_offsets[ret] = (mem_size / 0x20) * ret;
 		}
-
-		ax->map2 = NULL;
 	} else {
-		size = (res->end - res->start) + 1;
+		mem2_size = resource_size(mem2);
 
-		ax->mem2 = request_mem_region(res->start, size, pdev->name);
-		if (ax->mem2 == NULL) {
+		if (!request_mem_region(mem2->start, mem2_size, pdev->name)) {
 			dev_err(&pdev->dev, "cannot reserve registers\n");
 			ret = -ENXIO;
 			goto exit_mem1;
 		}
 
-		ax->map2 = ioremap(res->start, size);
-		if (ax->map2 == NULL) {
+		ax->map2 = ioremap(mem2->start, mem2_size);
+		if (!ax->map2) {
 			dev_err(&pdev->dev, "cannot map reset register\n");
 			ret = -ENXIO;
 			goto exit_mem2;
@@ -934,26 +924,23 @@ static int ax_probe(struct platform_device *pdev)
 	}
 
 	/* got resources, now initialise and register device */
-
 	ret = ax_init_dev(dev);
 	if (!ret)
 		return 0;
 
-	if (ax->map2 == NULL)
+	if (!ax->map2)
 		goto exit_mem1;
 
 	iounmap(ax->map2);
 
  exit_mem2:
-	release_resource(ax->mem2);
-	kfree(ax->mem2);
+	release_mem_region(mem2->start, mem2_size);
 
  exit_mem1:
 	iounmap(ei_local->mem);
 
  exit_req:
-	release_resource(ax->mem);
-	kfree(ax->mem);
+	release_mem_region(mem->start, mem_size);
 
  exit_mem:
 	free_netdev(dev);
-- 
1.7.2.3


^ permalink raw reply related

* [PATCH 6/9] ax88796: remove platform_device member from struct ax_device
From: Marc Kleine-Budde @ 2011-02-21 13:03 UTC (permalink / raw)
  To: netdev; +Cc: Ben Dooks, Daniel Mack, Marc Kleine-Budde
In-Reply-To: <1298293400-21570-1-git-send-email-mkl@pengutronix.de>

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/ax88796.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ax88796.c b/drivers/net/ax88796.c
index 1370cac..782d73e 100644
--- a/drivers/net/ax88796.c
+++ b/drivers/net/ax88796.c
@@ -87,7 +87,6 @@ struct ax_device {
 
 	u32 msg_enable;
 	void __iomem *map2;
-	struct platform_device *dev;
 	struct resource *mem;
 	struct resource *mem2;
 	struct ax_plat_data *plat;
@@ -545,11 +544,11 @@ static int ax_ioctl(struct net_device *dev, struct ifreq *req, int cmd)
 static void ax_get_drvinfo(struct net_device *dev,
 			   struct ethtool_drvinfo *info)
 {
-	struct ax_device *ax = to_ax_dev(dev);
+	struct platform_device *pdev = to_platform_device(dev->dev.parent);
 
 	strcpy(info->driver, DRV_NAME);
 	strcpy(info->version, DRV_VERSION);
-	strcpy(info->bus_info, ax->dev->name);
+	strcpy(info->bus_info, pdev->name);
 }
 
 static int ax_get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
@@ -848,7 +847,6 @@ static int ax_probe(struct platform_device *pdev)
 
 	spin_lock_init(&ax->mii_lock);
 
-	ax->dev = pdev;
 	ax->plat = pdev->dev.platform_data;
 	platform_set_drvdata(pdev, dev);
 
-- 
1.7.2.3


^ permalink raw reply related

* [PATCH 7/9] ax88796: make pointer to platform data const
From: Marc Kleine-Budde @ 2011-02-21 13:03 UTC (permalink / raw)
  To: netdev; +Cc: Ben Dooks, Daniel Mack, Marc Kleine-Budde
In-Reply-To: <1298293400-21570-1-git-send-email-mkl@pengutronix.de>

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/ax88796.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ax88796.c b/drivers/net/ax88796.c
index 782d73e..fd289e5 100644
--- a/drivers/net/ax88796.c
+++ b/drivers/net/ax88796.c
@@ -89,7 +89,7 @@ struct ax_device {
 	void __iomem *map2;
 	struct resource *mem;
 	struct resource *mem2;
-	struct ax_plat_data *plat;
+	const struct ax_plat_data *plat;
 
 	unsigned char running;
 	unsigned char resume_open;
-- 
1.7.2.3


^ permalink raw reply related

* [PATCH 9/9] ax88796: use generic mdio_bitbang driver
From: Marc Kleine-Budde @ 2011-02-21 13:03 UTC (permalink / raw)
  To: netdev; +Cc: Ben Dooks, Daniel Mack, Marc Kleine-Budde
In-Reply-To: <1298293400-21570-1-git-send-email-mkl@pengutronix.de>

..instead of using hand-crafted and not proper working version.

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/Kconfig   |    4 +-
 drivers/net/ax88796.c |  392 ++++++++++++++++++++++++-------------------------
 2 files changed, 196 insertions(+), 200 deletions(-)

diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 65027a7..f4b3927 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -238,8 +238,8 @@ source "drivers/net/arm/Kconfig"
 config AX88796
 	tristate "ASIX AX88796 NE2000 clone support"
 	depends on ARM || MIPS || SUPERH
-	select CRC32
-	select MII
+	select PHYLIB
+	select MDIO_BITBANG
 	help
 	  AX88796 driver, using platform bus to provide
 	  chip detection and resources
diff --git a/drivers/net/ax88796.c b/drivers/net/ax88796.c
index e62d0ba..e7cb8c8 100644
--- a/drivers/net/ax88796.c
+++ b/drivers/net/ax88796.c
@@ -24,7 +24,8 @@
 #include <linux/netdevice.h>
 #include <linux/etherdevice.h>
 #include <linux/ethtool.h>
-#include <linux/mii.h>
+#include <linux/mdio-bitbang.h>
+#include <linux/phy.h>
 #include <linux/eeprom_93cx6.h>
 #include <linux/slab.h>
 
@@ -32,8 +33,6 @@
 
 #include <asm/system.h>
 
-static int phy_debug;
-
 /* Rename the lib8390.c functions to show that they are in this driver */
 #define __ei_open ax_ei_open
 #define __ei_close ax_ei_close
@@ -78,14 +77,20 @@ static unsigned char version[] = "ax88796.c: Copyright 2005,2007 Simtec Electron
 #define NESM_START_PG	0x40	/* First page of TX buffer */
 #define NESM_STOP_PG	0x80	/* Last page +1 of RX ring */
 
+#define AX_GPOC_PPDSET	BIT(6)
+
 /* device private data */
 
 struct ax_device {
-	struct timer_list mii_timer;
-	spinlock_t mii_lock;
-	struct mii_if_info mii;
+	struct mii_bus *mii_bus;
+	struct mdiobb_ctrl bb_ctrl;
+	struct phy_device *phy_dev;
+	void __iomem *addr_memr;
+	u8 reg_memr;
+	int link;
+	int speed;
+	int duplex;
 
-	u32 msg_enable;
 	void __iomem *map2;
 	const struct ax_plat_data *plat;
 
@@ -313,159 +318,84 @@ static void ax_block_output(struct net_device *dev, int count,
 #define AX_MEMR_EEO		BIT(6)
 #define AX_MEMR_EECLK		BIT(7)
 
-/*
- * ax_mii_ei_outbits
- *
- * write the specified set of bits to the phy
- */
-static void
-ax_mii_ei_outbits(struct net_device *dev, unsigned int bits, int len)
+static void ax_handle_link_change(struct net_device *dev)
 {
-	struct ei_device *ei_local = netdev_priv(dev);
-	void __iomem *memr_addr = (void __iomem *)dev->base_addr + AX_MEMR;
-	unsigned int memr;
-
-	/* clock low, data to output mode */
-	memr = ei_inb(memr_addr);
-	memr &= ~(AX_MEMR_MDC | AX_MEMR_MDIR);
-	ei_outb(memr, memr_addr);
-
-	for (len--; len >= 0; len--) {
-		if (bits & (1 << len))
-			memr |= AX_MEMR_MDO;
-		else
-			memr &= ~AX_MEMR_MDO;
+	struct ax_device  *ax = to_ax_dev(dev);
+	struct phy_device *phy_dev = ax->phy_dev;
+	int status_change = 0;
 
-		ei_outb(memr, memr_addr);
+	if (phy_dev->link && ((ax->speed != phy_dev->speed) ||
+			     (ax->duplex != phy_dev->duplex))) {
 
-		/* clock high */
-
-		ei_outb(memr | AX_MEMR_MDC, memr_addr);
-		udelay(1);
-
-		/* clock low */
-		ei_outb(memr, memr_addr);
+		ax->speed = phy_dev->speed;
+		ax->duplex = phy_dev->duplex;
+		status_change = 1;
 	}
 
-	/* leaves the clock line low, mdir input */
-	memr |= AX_MEMR_MDIR;
-	ei_outb(memr, (void __iomem *)dev->base_addr + AX_MEMR);
-}
-
-/*
- * ax_phy_ei_inbits
- *
- * read a specified number of bits from the phy
- */
-static unsigned int
-ax_phy_ei_inbits(struct net_device *dev, int no)
-{
-	struct ei_device *ei_local = netdev_priv(dev);
-	void __iomem *memr_addr = (void __iomem *)dev->base_addr + AX_MEMR;
-	unsigned int memr;
-	unsigned int result = 0;
-
-	/* clock low, data to input mode */
-	memr = ei_inb(memr_addr);
-	memr &= ~AX_MEMR_MDC;
-	memr |= AX_MEMR_MDIR;
-	ei_outb(memr, memr_addr);
-
-	for (no--; no >= 0; no--) {
-		ei_outb(memr | AX_MEMR_MDC, memr_addr);
-
-		udelay(1);
-
-		if (ei_inb(memr_addr) & AX_MEMR_MDI)
-			result |= (1 << no);
+	if (phy_dev->link != ax->link) {
+		if (!phy_dev->link) {
+			ax->speed = 0;
+			ax->duplex = -1;
+		}
+		ax->link = phy_dev->link;
 
-		ei_outb(memr, memr_addr);
+		status_change = 1;
 	}
 
-	return result;
+	if (status_change)
+		phy_print_status(phy_dev);
 }
 
-/*
- * ax_phy_issueaddr
- *
- * use the low level bit shifting routines to send the address
- * and command to the specified phy
- */
-static void
-ax_phy_issueaddr(struct net_device *dev, int phy_addr, int reg, int opc)
-{
-	if (phy_debug)
-		netdev_dbg(dev, "%s: dev %p, %04x, %04x, %d\n",
-			__func__, dev, phy_addr, reg, opc);
-
-	ax_mii_ei_outbits(dev, 0x3f, 6);	/* pre-amble */
-	ax_mii_ei_outbits(dev, 1, 2);		/* frame-start */
-	ax_mii_ei_outbits(dev, opc, 2);		/* op code */
-	ax_mii_ei_outbits(dev, phy_addr, 5);	/* phy address */
-	ax_mii_ei_outbits(dev, reg, 5);		/* reg address */
-}
-
-static int
-ax_phy_read(struct net_device *dev, int phy_addr, int reg)
+static int ax_mii_probe(struct net_device *dev)
 {
-	struct ei_device *ei_local = netdev_priv(dev);
-	unsigned long flags;
-	unsigned int result;
-
-	spin_lock_irqsave(&ei_local->page_lock, flags);
-
-	ax_phy_issueaddr(dev, phy_addr, reg, 2);
-
-	result = ax_phy_ei_inbits(dev, 17);
-	result &= ~(3 << 16);
-
-	spin_unlock_irqrestore(&ei_local->page_lock, flags);
-
-	if (phy_debug)
-		netdev_dbg(dev, "%s: %04x.%04x => read %04x\n", __func__,
-			 phy_addr, reg, result);
+	struct ax_device  *ax = to_ax_dev(dev);
+	struct phy_device *phy_dev = NULL;
+	int ret;
 
-	return result;
-}
+	/* find the first phy */
+	phy_dev = phy_find_first(ax->mii_bus);
+	if (!phy_dev) {
+		netdev_err(dev, "no PHY found\n");
+		return -ENODEV;
+	}
 
-static void
-ax_phy_write(struct net_device *dev, int phy_addr, int reg, int value)
-{
-	struct ei_device *ei = netdev_priv(dev);
-	unsigned long flags;
+	ret = phy_connect_direct(dev, phy_dev, ax_handle_link_change, 0,
+				 PHY_INTERFACE_MODE_MII);
+	if (ret) {
+		netdev_err(dev, "Could not attach to PHY\n");
+		return ret;
+	}
 
-	netdev_dbg(dev, "%s: %p, %04x, %04x %04x\n",
-		__func__, dev, phy_addr, reg, value);
+	/* mask with MAC supported features */
+	phy_dev->supported &= PHY_BASIC_FEATURES;
+	phy_dev->advertising = phy_dev->supported;
 
-	spin_lock_irqsave(&ei->page_lock, flags);
+	ax->phy_dev = phy_dev;
 
-	ax_phy_issueaddr(dev, phy_addr, reg, 1);
-	ax_mii_ei_outbits(dev, 2, 2);		/* send TA */
-	ax_mii_ei_outbits(dev, value, 16);
+	netdev_info(dev, "PHY driver [%s] (mii_bus:phy_addr=%s, irq=%d)\n",
+		    phy_dev->drv->name, dev_name(&phy_dev->dev), phy_dev->irq);
 
-	spin_unlock_irqrestore(&ei->page_lock, flags);
+	return 0;
 }
 
-static void ax_mii_expiry(unsigned long data)
+static void ax_phy_switch(struct net_device *dev, int on)
 {
-	struct net_device *dev = (struct net_device *)data;
+	struct ei_device *ei_local = netdev_priv(dev);
 	struct ax_device *ax = to_ax_dev(dev);
-	unsigned long flags;
 
-	spin_lock_irqsave(&ax->mii_lock, flags);
-	mii_check_media(&ax->mii, netif_msg_link(ax), 0);
-	spin_unlock_irqrestore(&ax->mii_lock, flags);
+	u8 reg_gpoc =  ax->plat->gpoc_val;
 
-	if (ax->running) {
-		ax->mii_timer.expires = jiffies + HZ*2;
-		add_timer(&ax->mii_timer);
-	}
+	if (!!on)
+		reg_gpoc &= ~AX_GPOC_PPDSET;
+	else
+		reg_gpoc |= AX_GPOC_PPDSET;
+
+	ei_outb(reg_gpoc, ei_local->mem + EI_SHIFT(0x17));
 }
 
 static int ax_open(struct net_device *dev)
 {
 	struct ax_device *ax = to_ax_dev(dev);
-	struct ei_device *ei_local = netdev_priv(dev);
 	int ret;
 
 	netdev_dbg(dev, "open\n");
@@ -473,50 +403,48 @@ static int ax_open(struct net_device *dev)
 	ret = request_irq(dev->irq, ax_ei_interrupt, ax->irqflags,
 			  dev->name, dev);
 	if (ret)
-		return ret;
-
-	ret = ax_ei_open(dev);
-	if (ret) {
-		free_irq(dev->irq, dev);
-		return ret;
-	}
+		goto failed_request_irq;
 
 	/* turn the phy on (if turned off) */
+	ax_phy_switch(dev, 1);
 
-	ei_outb(ax->plat->gpoc_val, ei_local->mem + EI_SHIFT(0x17));
-	ax->running = 1;
-
-	/* start the MII timer */
-
-	init_timer(&ax->mii_timer);
+	ret = ax_mii_probe(dev);
+	if (ret)
+		goto failed_mii_probe;
+	phy_start(ax->phy_dev);
 
-	ax->mii_timer.expires = jiffies + 1;
-	ax->mii_timer.data = (unsigned long) dev;
-	ax->mii_timer.function = ax_mii_expiry;
+	ret = ax_ei_open(dev);
+	if (ret)
+		goto failed_ax_ei_open;
 
-	add_timer(&ax->mii_timer);
+	ax->running = 1;
 
 	return 0;
+
+ failed_ax_ei_open:
+	phy_disconnect(ax->phy_dev);
+ failed_mii_probe:
+	ax_phy_switch(dev, 0);
+	free_irq(dev->irq, dev);
+ failed_request_irq:
+	return ret;
 }
 
 static int ax_close(struct net_device *dev)
 {
 	struct ax_device *ax = to_ax_dev(dev);
-	struct ei_device *ei_local = netdev_priv(dev);
 
 	netdev_dbg(dev, "close\n");
 
-	/* turn the phy off */
-
-	ei_outb(ax->plat->gpoc_val | (1 << 6),
-	       ei_local->mem + EI_SHIFT(0x17));
-
 	ax->running = 0;
 	wmb();
 
-	del_timer_sync(&ax->mii_timer);
 	ax_ei_close(dev);
 
+	/* turn the phy off */
+	ax_phy_switch(dev, 0);
+	phy_disconnect(ax->phy_dev);
+
 	free_irq(dev->irq, dev);
 	return 0;
 }
@@ -524,17 +452,15 @@ static int ax_close(struct net_device *dev)
 static int ax_ioctl(struct net_device *dev, struct ifreq *req, int cmd)
 {
 	struct ax_device *ax = to_ax_dev(dev);
-	unsigned long flags;
-	int rc;
+	struct phy_device *phy_dev = ax->phy_dev;
 
 	if (!netif_running(dev))
 		return -EINVAL;
 
-	spin_lock_irqsave(&ax->mii_lock, flags);
-	rc = generic_mii_ioctl(&ax->mii, if_mii(req), cmd, NULL);
-	spin_unlock_irqrestore(&ax->mii_lock, flags);
+	if (!phy_dev)
+		return -ENODEV;
 
-	return rc;
+	return phy_mii_ioctl(phy_dev, req, cmd);
 }
 
 /* ethtool ops */
@@ -552,46 +478,30 @@ static void ax_get_drvinfo(struct net_device *dev,
 static int ax_get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
 {
 	struct ax_device *ax = to_ax_dev(dev);
-	unsigned long flags;
+	struct phy_device *phy_dev = ax->phy_dev;
 
-	spin_lock_irqsave(&ax->mii_lock, flags);
-	mii_ethtool_gset(&ax->mii, cmd);
-	spin_unlock_irqrestore(&ax->mii_lock, flags);
+	if (!phy_dev)
+		return -ENODEV;
 
-	return 0;
+	return phy_ethtool_gset(phy_dev, cmd);
 }
 
 static int ax_set_settings(struct net_device *dev, struct ethtool_cmd *cmd)
 {
 	struct ax_device *ax = to_ax_dev(dev);
-	unsigned long flags;
-	int rc;
-
-	spin_lock_irqsave(&ax->mii_lock, flags);
-	rc = mii_ethtool_sset(&ax->mii, cmd);
-	spin_unlock_irqrestore(&ax->mii_lock, flags);
+	struct phy_device *phy_dev = ax->phy_dev;
 
-	return rc;
-}
-
-static int ax_nway_reset(struct net_device *dev)
-{
-	struct ax_device *ax = to_ax_dev(dev);
-	return mii_nway_restart(&ax->mii);
-}
+	if (!phy_dev)
+		return -ENODEV;
 
-static u32 ax_get_link(struct net_device *dev)
-{
-	struct ax_device *ax = to_ax_dev(dev);
-	return mii_link_ok(&ax->mii);
+	return phy_ethtool_sset(phy_dev, cmd);
 }
 
 static const struct ethtool_ops ax_ethtool_ops = {
 	.get_drvinfo		= ax_get_drvinfo,
 	.get_settings		= ax_get_settings,
 	.set_settings		= ax_set_settings,
-	.nway_reset		= ax_nway_reset,
-	.get_link		= ax_get_link,
+	.get_link		= ethtool_op_get_link,
 };
 
 #ifdef CONFIG_AX88796_93CX6
@@ -642,8 +552,102 @@ static const struct net_device_ops ax_netdev_ops = {
 #endif
 };
 
+static void ax_bb_mdc(struct mdiobb_ctrl *ctrl, int level)
+{
+	struct ax_device *ax = container_of(ctrl, struct ax_device, bb_ctrl);
+
+	if (level)
+		ax->reg_memr |= AX_MEMR_MDC;
+	else
+		ax->reg_memr &= ~AX_MEMR_MDC;
+
+	ei_outb(ax->reg_memr, ax->addr_memr);
+}
+
+static void ax_bb_dir(struct mdiobb_ctrl *ctrl, int output)
+{
+	struct ax_device *ax = container_of(ctrl, struct ax_device, bb_ctrl);
+
+	if (output)
+		ax->reg_memr &= ~AX_MEMR_MDIR;
+	else
+		ax->reg_memr |= AX_MEMR_MDIR;
+
+	ei_outb(ax->reg_memr, ax->addr_memr);
+}
+
+static void ax_bb_set_data(struct mdiobb_ctrl *ctrl, int value)
+{
+	struct ax_device *ax = container_of(ctrl, struct ax_device, bb_ctrl);
+
+	if (value)
+		ax->reg_memr |= AX_MEMR_MDO;
+	else
+		ax->reg_memr &= ~AX_MEMR_MDO;
+
+	ei_outb(ax->reg_memr, ax->addr_memr);
+}
+
+static int ax_bb_get_data(struct mdiobb_ctrl *ctrl)
+{
+	struct ax_device *ax = container_of(ctrl, struct ax_device, bb_ctrl);
+	int reg_memr = ei_inb(ax->addr_memr);
+
+	return reg_memr & AX_MEMR_MDI ? 1 : 0;
+}
+
+static struct mdiobb_ops bb_ops = {
+	.owner = THIS_MODULE,
+	.set_mdc = ax_bb_mdc,
+	.set_mdio_dir = ax_bb_dir,
+	.set_mdio_data = ax_bb_set_data,
+	.get_mdio_data = ax_bb_get_data,
+};
+
 /* setup code */
 
+static int ax_mii_init(struct net_device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev->dev.parent);
+	struct ei_device *ei_local = netdev_priv(dev);
+	struct ax_device *ax = to_ax_dev(dev);
+	int err, i;
+
+	ax->bb_ctrl.ops = &bb_ops;
+	ax->addr_memr = ei_local->mem + AX_MEMR;
+	ax->mii_bus = alloc_mdio_bitbang(&ax->bb_ctrl);
+	if (!ax->mii_bus) {
+		err = -ENOMEM;
+		goto out;
+	}
+
+	ax->mii_bus->name = "ax88796_mii_bus";
+	ax->mii_bus->parent = dev->dev.parent;
+	snprintf(ax->mii_bus->id, MII_BUS_ID_SIZE, "%x", pdev->id);
+
+	ax->mii_bus->irq = kmalloc(sizeof(int) * PHY_MAX_ADDR, GFP_KERNEL);
+	if (!ax->mii_bus->irq) {
+		err = -ENOMEM;
+		goto out_free_mdio_bitbang;
+	}
+
+	for (i = 0; i < PHY_MAX_ADDR; i++)
+		ax->mii_bus->irq[i] = PHY_POLL;
+
+	err = mdiobus_register(ax->mii_bus);
+	if (err)
+		goto out_free_irq;
+
+	return 0;
+
+ out_free_irq:
+	kfree(ax->mii_bus->irq);
+ out_free_mdio_bitbang:
+	free_mdio_bitbang(ax->mii_bus);
+ out:
+	return err;
+}
+
 static void ax_initial_setup(struct net_device *dev, struct ei_device *ei_local)
 {
 	void __iomem *ioaddr = ei_local->mem;
@@ -763,15 +767,9 @@ static int ax_init_dev(struct net_device *dev)
 	dev->netdev_ops = &ax_netdev_ops;
 	dev->ethtool_ops = &ax_ethtool_ops;
 
-	ax->msg_enable		= NETIF_MSG_LINK;
-	ax->mii.phy_id_mask	= 0x1f;
-	ax->mii.reg_num_mask	= 0x1f;
-	ax->mii.phy_id		= 0x10;		/* onboard phy */
-	ax->mii.force_media	= 0;
-	ax->mii.full_duplex	= 0;
-	ax->mii.mdio_read	= ax_phy_read;
-	ax->mii.mdio_write	= ax_phy_write;
-	ax->mii.dev		= dev;
+	ret = ax_mii_init(dev);
+	if (ret)
+		goto out_irq;
 
 	ax_NS8390_init(dev, 0);
 
@@ -842,8 +840,6 @@ static int ax_probe(struct platform_device *pdev)
 	ei_local = netdev_priv(dev);
 	ax = to_ax_dev(dev);
 
-	spin_lock_init(&ax->mii_lock);
-
 	ax->plat = pdev->dev.platform_data;
 	platform_set_drvdata(pdev, dev);
 
-- 
1.7.2.3


^ permalink raw reply related

* [PATCH] f_phonet: avoid pskb_pull(), fix OOPS with CONFIG_HIGHMEM
From: Rémi Denis-Courmont @ 2011-02-21 14:16 UTC (permalink / raw)
  To: linux-usb, netdev

This is similar to what we arleady do in cdc-phonet.c in the same
situation.

Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
---
 drivers/usb/gadget/f_phonet.c |   15 ++++++++++-----
 1 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/gadget/f_phonet.c b/drivers/usb/gadget/f_phonet.c
index 3c6e1a0..5e14950 100644
--- a/drivers/usb/gadget/f_phonet.c
+++ b/drivers/usb/gadget/f_phonet.c
@@ -346,14 +346,19 @@ static void pn_rx_complete(struct usb_ep *ep, struct usb_request *req)
 
 		if (unlikely(!skb))
 			break;
-		skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags, page, 0,
-				req->actual);
-		page = NULL;
 
-		if (req->actual < req->length) { /* Last fragment */
+		if (skb->len == 0) { /* First fragment */
 			skb->protocol = htons(ETH_P_PHONET);
 			skb_reset_mac_header(skb);
-			pskb_pull(skb, 1);
+			/* Can't use pskb_pull() on page in IRQ */
+			memcpy(skb_put(skb, 1), page_address(page), 1);
+		}
+
+		skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags, page,
+				skb->len == 0, req->actual);
+		page = NULL;
+
+		if (req->actual < req->length) { /* Last fragment */
 			skb->dev = dev;
 			dev->stats.rx_packets++;
 			dev->stats.rx_bytes += skb->len;
-- 
1.7.1


^ permalink raw reply related

* Re: [ethtool PATCH 1/2] Add macro for displaying [value N] formatting to manpage
From: Ben Hutchings @ 2011-02-21 14:45 UTC (permalink / raw)
  To: Alexander Duyck; +Cc: netdev
In-Reply-To: <20110211011833.23554.32698.stgit@gitlad.jf.intel.com>

On Thu, 2011-02-10 at 17:18 -0800, Alexander Duyck wrote:
> This change adds a macro for displaying optional values that take a numeric
> value to the manpage.
[...]

Applied, thanks.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
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

* Re: [PATCH] f_phonet: avoid pskb_pull(), fix OOPS with CONFIG_HIGHMEM
From: Felipe Balbi @ 2011-02-21 14:50 UTC (permalink / raw)
  To: Rémi Denis-Courmont; +Cc: linux-usb, netdev
In-Reply-To: <1298297813-4795-1-git-send-email-remi.denis-courmont@nokia.com>

On Mon, Feb 21, 2011 at 04:16:53PM +0200, Rémi Denis-Courmont wrote:
> This is similar to what we arleady do in cdc-phonet.c in the same
			      ^^typo

-- 
balbi

^ permalink raw reply

* Re: Ethernet over GRE and vlans
From: Jonathan Thibault @ 2011-02-21 15:01 UTC (permalink / raw)
  To: Herbert Xu; +Cc: netdev
In-Reply-To: <20110221053854.GA26462@gondor.apana.org.au>

On 21/02/11 12:38 AM, Herbert Xu wrote:
> On Sat, Jan 29, 2011 at 12:16:06AM -0500, Jonathan Thibault wrote:
>> Is it wrong on my part to expect such behaviour from gretap devices
>> or is this simply not possible/implemented yet?
> I don't see why this shouldn't work, so it might be a bug or
> misconfiguration.  How did you setup gre1.1 and gre2.2?
>
> Cheers,
I simply ran:

vconfig add rcg0 1
ifconfig rcg0.1 up

(the gretap interface is called rcg0, obviously)

Now that I think of it I did not try to add the vlan using the 'ip link
add' command though I'm not entirely sure it would make much of a
difference.  We sort of bypassed the problem using more hardware so the
test rig is dismantled now but if you feel there really is something to
it, I can set it back up.

Thanks again,

Jonathan

P.S.:  Gretap flies...  The little atom 1.6 boards I tested this on gave
me 877Mbit/sec without breaking a sweat.

^ permalink raw reply

* Re: [ethtool PATCH 2/2] Add RX packet classification interface
From: Ben Hutchings @ 2011-02-21 15:40 UTC (permalink / raw)
  To: Alexander Duyck, Santwona Behera; +Cc: netdev
In-Reply-To: <20110211011838.23554.3735.stgit@gitlad.jf.intel.com>

On Thu, 2011-02-10 at 17:18 -0800, Alexander Duyck wrote:
> From: Santwona Behera <santwona.behera@sun.com>
> 
> This patch was originally introduced as:
>   [PATCH 1/3] [ethtool] Add rx pkt classification interface
>   Signed-off-by: Santwona Behera <santwona.behera@sun.com>
>   http://patchwork.ozlabs.org/patch/23223/
> 
> I have updated it to address a number of issues.  As a result I removed the
> local caching of rules due to the fact that there were memory leaks in this
> code and the rule manager would consume over 1Mb of space for an 8K table
> when all that was needed was 1K in order to store which rules were active
> and which were not.
> 
> In addition I dropped the use of regions as there were multiple issue found
> including the fact that the regions were not properly expanding beyond 2
> and the fact that the regions required reading all of the rules in order to
> correctly expand beyond 2.  By dropping the regions from the rule manager
> it is possible to write a much cleaner interface leaving region management
> to be done by either the driver or by external management scripts.
> 
> I also added an ethtool bitops interface to allow for simple bit set and
> test activities since the rule manager can most efficiently store the list
> of active rules via a bitmap.
[...]
> diff --git a/ethtool-bitops.h b/ethtool-bitops.h
> new file mode 100644
> index 0000000..7101056
> --- /dev/null
> +++ b/ethtool-bitops.h
> @@ -0,0 +1,25 @@
> +#ifndef ETHTOOL_BITOPS_H__
> +#define ETHTOOL_BITOPS_H__
> +
> +#define BITS_PER_LONG		__WORDSIZE

This seems to be a glibc-internal macro and I don't think we should use
it.

> +#define BITS_PER_BYTE		8
> +#define DIV_ROUND_UP(n, d)	(((n) + (d) - 1) / (d))
> +#define BITS_TO_LONGS(nr)	DIV_ROUND_UP(nr, BITS_PER_BYTE * sizeof(long))

In fact I notice you don't use it here...

> +static inline void set_bit(int nr, unsigned long *addr)
> +{
> +	addr[nr / BITS_PER_LONG] |= 1UL << (nr % BITS_PER_LONG);
> +}
> +
> +static inline void clear_bit(int nr, unsigned long *addr)
> +{
> +	addr[nr / BITS_PER_LONG] &= ~(1UL << (nr % BITS_PER_LONG));
> +}
> +
> +static __always_inline int test_bit(unsigned int nr, const unsigned long *addr)
> +{
> +	return ((1UL << (nr % BITS_PER_LONG)) &
> +		(((unsigned long *)addr)[nr / BITS_PER_LONG])) != 0UL;
> +}
> +
> +#endif
> diff --git a/ethtool-util.h b/ethtool-util.h
> index f053028..e9300e2 100644
> --- a/ethtool-util.h
> +++ b/ethtool-util.h
> @@ -103,4 +103,17 @@ int sfc_dump_regs(struct ethtool_drvinfo *info, struct ethtool_regs *regs);
>  int st_mac100_dump_regs(struct ethtool_drvinfo *info,
>  			struct ethtool_regs *regs);
>  int st_gmac_dump_regs(struct ethtool_drvinfo *info, struct ethtool_regs *regs);
> +
> +/* Rx flow classification */
> +#include <sys/ioctl.h>
> +#include <net/if.h>

Put #includes at the top of the file, please.

> +int rxclass_parse_ruleopts(char **optstr, int opt_cnt,
> +			   struct ethtool_rx_flow_spec *fsp, __u8 *loc_valid);
> +int rxclass_rule_getall(int fd, struct ifreq *ifr);
> +int rxclass_rule_get(int fd, struct ifreq *ifr, __u32 loc);
> +int rxclass_rule_ins(int fd, struct ifreq *ifr,
> +		     struct ethtool_rx_flow_spec *fsp, __u8 loc_valid);
> +int rxclass_rule_del(int fd, struct ifreq *ifr, __u32 loc);
> +
>  #endif
> diff --git a/ethtool.8.in b/ethtool.8.in
> index 133825b..c183a3d 100644
> --- a/ethtool.8.in
> +++ b/ethtool.8.in
> @@ -40,21 +40,36 @@
>  [\\fB\\$1\\fP\ \\fIN\\fP]
>  ..
>  .\"
> +.\"	.BM - same as above but has a mask field for format "[value N [m N]]"
> +.\"
> +.de BM
> +[\\fB\\$1\\fP\ \\fIN\\fP\ [\\fBm\\fP\ \\fIN\\fP]]
> +..
> +.\"
>  .\"	\(*MA - mac address
>  .\"
>  .ds MA \fIxx\fP\fB:\fP\fIyy\fP\fB:\fP\fIzz\fP\fB:\fP\fIaa\fP\fB:\fP\fIbb\fP\fB:\fP\fIcc\fP
>  .\"
> +.\"	\(*PA - IP address
> +.\"
> +.ds PA \fIx\fP\fB.\fP\fIx\fP\fB.\fP\fIx\fP\fB.\fP\fIx\fP
> +.\"
>  .\"	\(*WO - wol flags
>  .\"
>  .ds WO \fBp\fP|\fBu\fP|\fBm\fP|\fBb\fP|\fBa\fP|\fBg\fP|\fBs\fP|\fBd\fP...
>  .\"
>  .\"	\(*FL - flow type values
>  .\"
> -.ds FL \fBtcp4\fP|\fBudp4\fP|\fBah4\fP|\fBsctp4\fP|\fBtcp6\fP|\fBudp6\fP|\fBah6\fP|\fBsctp6\fP
> +.ds FL \fBtcp4\fP|\fBudp4\fP|\fBah4\fP||\fBesp4\fP|\fBsctp4\fP|\fBtcp6\fP|\fBudp6\fP|\fBah6\fP|\fBesp6\fP|\fBsctp6\fP

This adds two '|' characters between 'ah4' and 'esp4'.

>  .\"
>  .\"	\(*HO - hash options
>  .\"
>  .ds HO \fBm\fP|\fBv\fP|\fBt\fP|\fBs\fP|\fBd\fP|\fBf\fP|\fBn\fP|\fBr\fP...
> +.\"
> +.\"	\(*L4 - L4 proto options
> +.\"
> +.ds L4 \fBtcp\fP|\fBudp\fP|\fBsctp\fP|\fBah\fP|\fBesp\fP|\fIN\fP
> +.\"
>  .\" Start URL.
>  .de UR
>  .  ds m1 \\$1\"
> @@ -224,11 +239,27 @@ ethtool \- query or control network driver and hardware settings
>  .B ethtool \-n
>  .I ethX
>  .RB [ rx-flow-hash \ \*(FL]
> +.RB [ rx-rings ]
> +.RB [ rx-class-rule-all ]
> +.RB [ rx-class-rule
> +.IR N ]

Should use '.BN'.
 
[...]
> diff --git a/ethtool.c b/ethtool.c
> index 1afdfe4..b624980 100644
> --- a/ethtool.c
> +++ b/ethtool.c
> @@ -6,6 +6,7 @@
>   * Kernel 2.4 update Copyright 2001 Jeff Garzik <jgarzik@mandrakesoft.com>
>   * Wake-on-LAN,natsemi,misc support by Tim Hockin <thockin@sun.com>
>   * Portions Copyright 2002 Intel
> + * Portions Copyright (C) Sun Microsystems 2008
>   * do_test support by Eli Kupermann <eli.kupermann@intel.com>
>   * ETHTOOL_PHYS_ID support by Chris Leech <christopher.leech@intel.com>
>   * e1000 support by Scott Feldman <scott.feldman@intel.com>
> @@ -14,6 +15,7 @@
>   * amd8111e support by Reeja John <reeja.john@amd.com>
>   * long arguments by Andi Kleen.
>   * SMSC LAN911x support by Steve Glendinning <steve.glendinning@smsc.com>
> + * Rx Network Flow Control configuration support <santwona.behera@sun.com>
>   * Various features by Ben Hutchings <bhutchings@solarflare.com>;
>   *	Copyright 2009, 2010 Solarflare Communications
>   *
> @@ -32,7 +34,7 @@
>  #include <sys/ioctl.h>
>  #include <sys/stat.h>
>  #include <stdio.h>
> -#include <string.h>
> +#include <strings.h>

No, <string.h> is standard.

[...]
> @@ -408,6 +425,14 @@ static int msglvl_changed;
>  static u32 msglvl_wanted = 0;
>  static u32 msglvl_mask = 0;
>  
> +static int rx_rings_get = 0;
> +static int rx_class_rule_get = -1;
> +static int rx_class_rule_getall = 0;
> +static int rx_class_rule_del = -1;
> +static int rx_class_rule_added = 0;
> +static struct ethtool_rx_flow_spec rx_rule_fs;
> +static u8 rxclass_loc_valid = 0;
> +
>  static enum {
>  	ONLINE=0,
>  	OFFLINE,
[...]
> @@ -945,6 +974,23 @@ static void parse_cmdline(int argc, char **argp)
>  						rxflow_str_to_type(argp[i]);
>  					if (!rx_fhash_get)
>  						show_usage(1);
> +				} else if (!strcmp(argp[i], "rx-rings")) {
> +					i += 1;
> +					rx_rings_get = 1;

I'm not convinced of the value of a separate rx-rings option/keyword.
However it's probably worth displaying the number of rings/queues when
showing other flow hashing and steering/filtering information (the -x
option does this).

> +				} else if (!strcmp(argp[i],
> +						   "rx-class-rule-all")) {
> +					i += 1;
> +					rx_class_rule_getall = 1;
> +				} else if (!strcmp(argp[i], "rx-class-rule")) {
> +					i += 1;
> +					if (i >= argc) {
> +						show_usage(1);
> +						break;
> +					}
> +					rx_class_rule_get =
> +						strtol(argp[i], NULL, 0);
> +					if (rx_class_rule_get < 0)
> +						show_usage(1);

Use get_uint_range(argp[i], 0, INT_MAX).

>  				} else
>  					show_usage(1);
>  				break;

I don't think the same options (-n, -N) should be used both for flow
hashing and n-tuple flow steering/filtering.  This command-line
interface and the structure used in the ethtool API just seem to reflect
the implementation in the niu driver.

(In fact I would much prefer it if the -u and -U options could be used
for both the rxnfc and rxntuple interfaces.  But I haven't thought about
how the differences in functionality would be exposed to or hidden from
the user.)

> @@ -978,8 +1024,37 @@ static void parse_cmdline(int argc, char **argp)
>  						show_usage(1);
>  					else
>  						rx_fhash_changed = 1;
> -				} else
> +				} else if (!strcmp(argp[i],
> +						   "rx-class-rule-del")) {
> +					i += 1;
> +					if (i >= argc) {
> +						show_usage(1);
> +						break;
> +					}
> +					rx_class_rule_del =
> +						strtol(argp[i], NULL, 0);
> +					if (rx_class_rule_del < 0)
> +						show_usage(1);

Use get_uint_range(argp[i], 0, INT_MAX).

> +				} else if (!strcmp(argp[i],
> +						   "rx-class-rule-add")) {
> +					i += 1;
> +					if (i >= argc) {
> +						show_usage(1);
> +						break;
> +					}
> +					if (rxclass_parse_ruleopts(&argp[i],
> +								   argc - i,
> +								   &rx_rule_fs,
> +								   &rxclass_loc_valid) < 0) {
> +						show_usage(1);
> +					} else {
> +						i = argc;
> +						rx_class_rule_added = 1;
> +					}
> +				} else {
>  					show_usage(1);
> +				}
> +
>  				break;
>  			}
>  			if (mode == MODE_SRXFHINDIR) {
> @@ -1917,9 +1992,12 @@ static int dump_rxfhash(int fhash, u64 val)
>  	case SCTP_V4_FLOW:
>  		fprintf(stdout, "SCTP over IPV4 flows");
>  		break;
> -	case AH_ESP_V4_FLOW:

I believe this is still a valid type for flow hashing.

> +	case AH_V4_FLOW:
>  		fprintf(stdout, "IPSEC AH over IPV4 flows");
>  		break;
> +	case ESP_V4_FLOW:
> +		fprintf(stdout, "IPSEC ESP over IPV4 flows");
> +		break;
>  	case TCP_V6_FLOW:
>  		fprintf(stdout, "TCP over IPV6 flows");
>  		break;
> @@ -1929,9 +2007,12 @@ static int dump_rxfhash(int fhash, u64 val)
>  	case SCTP_V6_FLOW:
>  		fprintf(stdout, "SCTP over IPV6 flows");
>  		break;
> -	case AH_ESP_V6_FLOW:

Same as for AH_ESP_V4_FLOW.

> +	case AH_V6_FLOW:
>  		fprintf(stdout, "IPSEC AH over IPV6 flows");
>  		break;
> +	case ESP_V6_FLOW:
> +		fprintf(stdout, "IPSEC ESP over IPV6 flows");
> +		break;
>  	default:
>  		break;
>  	}
> @@ -2911,14 +2992,12 @@ static int do_gstats(int fd, struct ifreq *ifr)
>  	return 0;
>  }
>  
> -
>  static int do_srxclass(int fd, struct ifreq *ifr)
>  {
>  	int err;
> +	struct ethtool_rxnfc nfccmd;
>  
>  	if (rx_fhash_changed) {
> -		struct ethtool_rxnfc nfccmd;
> -
>  		nfccmd.cmd = ETHTOOL_SRXFH;
>  		nfccmd.flow_type = rx_fhash_set;
>  		nfccmd.data = rx_fhash_val;
> @@ -2930,6 +3009,20 @@ static int do_srxclass(int fd, struct ifreq *ifr)
>  
>  	}
>  
> +	if (rx_class_rule_added) {
> +		err = rxclass_rule_ins(fd, ifr, &rx_rule_fs,
> +				       rxclass_loc_valid);
> +		if (err < 0)
> +			fprintf(stderr, "Cannot insert RX classification rule\n");
> +	}
> +
> +	if (rx_class_rule_del >= 0) {
> +		err = rxclass_rule_del(fd, ifr, rx_class_rule_del);
> +
> +		if (err < 0)
> +			fprintf(stderr, "Cannot delete RX classification rule\n");
> +	}
> +
>  	return 0;
>  }

This needs to return 1 on error (I know that's an existing bug, but
don't compound it).

> @@ -2950,6 +3043,31 @@ static int do_grxclass(int fd, struct ifreq *ifr)
>  			dump_rxfhash(rx_fhash_get, nfccmd.data);
>  	}
>  
> +	if (rx_rings_get) {
> +		struct ethtool_rxnfc nfccmd;
> +
> +		nfccmd.cmd = ETHTOOL_GRXRINGS;
> +		ifr->ifr_data = (caddr_t)&nfccmd;
> +		err = ioctl(fd, SIOCETHTOOL, ifr);
> +		if (err < 0)
> +			perror("Cannot get RX rings");
> +		else
> +			fprintf(stdout, "%d RX rings available\n",
> +				(int)nfccmd.data);
> +	}
> +
> +	if (rx_class_rule_get >= 0) {
> +		err = rxclass_rule_get(fd, ifr, rx_class_rule_get);
> +		if (err < 0)
> +			fprintf(stderr, "Cannot get RX classification rule\n");
> +	}
> +
> +	if (rx_class_rule_getall) {
> +		err = rxclass_rule_getall(fd, ifr);
> +		if (err < 0)
> +			fprintf(stderr, "RX classification rule retrieval failed\n");
> +	}
> +
>  	return 0;
>  }

Ditto for this.

> diff --git a/rxclass.c b/rxclass.c
> new file mode 100644
> index 0000000..fd01a32
> --- /dev/null
> +++ b/rxclass.c
> @@ -0,0 +1,809 @@
> +/*
> + * Copyright (C) 2008 Sun Microsystems, Inc. All rights reserved.
> + */
> +#include <stdio.h>
> +#include <stdint.h>
> +#include <stddef.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <strings.h>
> +
> +#include <linux/sockios.h>
> +#include <arpa/inet.h>
> +#include "ethtool-util.h"
> +#include "ethtool-bitops.h"
> +
> +/*
> + * This is a rule manager implementation for ordering rx flow
> + * classification rules in a longest prefix first match order.
> + * The assumption is that this rule manager is the only one adding rules to
> + * the device's hardware classifier.
> + */
> +
> +struct rmgr_ctrl {
> +	/* slot contains a bitmap indicating which filters are valid */
> +	unsigned long		*slot;
> +	__u32			n_rules;
> +	__u32			size;
> +};
> +
> +static struct rmgr_ctrl rmgr;
> +static int rmgr_init_done = 0;
> +
> +#ifndef SIOCETHTOOL
> +#define SIOCETHTOOL     0x8946
> +#endif

This definition ought to be moved to ethtool-util.h rather than
duplicated.

> +static void rmgr_print_ipv4_rule(struct ethtool_rx_flow_spec *fsp)
> +{
> +	char		chan[16];
> +	char		l4_proto[16];
> +	__u32		sip, dip, sipm, dipm;
> +
> +	sip = ntohl(fsp->h_u.tcp_ip4_spec.ip4src);
> +	dip = ntohl(fsp->h_u.tcp_ip4_spec.ip4dst);
> +	sipm = ntohl(fsp->m_u.tcp_ip4_spec.ip4src);
> +	dipm = ntohl(fsp->m_u.tcp_ip4_spec.ip4dst);
> +
> +	if (fsp->ring_cookie != RX_CLS_FLOW_DISC)
> +		sprintf(chan, "Rx Ring [%d]", (int)fsp->ring_cookie);
> +	else
> +		sprintf(chan, "Discard");
> +
> +	switch (fsp->flow_type) {
> +	case TCP_V4_FLOW:
> +	case UDP_V4_FLOW:
> +	case SCTP_V4_FLOW:
> +	case AH_V4_FLOW:
> +	case ESP_V4_FLOW:
> +	case IP_USER_FLOW:
> +		fprintf(stdout,
> +			"      IPv4 Rule:  ID[%d] Target[%s]\n"
> +			"      IP src addr[%d.%d.%d.%d] mask[%d.%d.%d.%d]\n"
> +			"      IP dst addr[%d.%d.%d.%d] mask[%d.%d.%d.%d]\n"
> +			"      IP TOS[0x%x] mask[0x%x]\n",

To be consistent with other ethtool output, this should use colons
rather than square brackets to separate field names and values.

[...]
> +int rxclass_parse_ruleopts(char **optstr, int opt_cnt,
> +			   struct ethtool_rx_flow_spec *fsp,
> +			   u_int8_t *loc_valid)
> +{
> +	int i = 0;
> +
> +	u_int8_t discard, ring_set;
> +	u_int32_t ipsa, ipsm, ipda, ipdm, spi, spim;
> +	u_int16_t sp, spm, dp, dpm;
> +	u_int8_t ip_ver, proto, tos, tm;
> +	struct in_addr in_addr;
> +
> +	if (*optstr == NULL || **optstr == '\0' || opt_cnt < 2) {
> +		fprintf(stdout, "Add rule, invalid syntax\n");
> +		return -1;
> +	}
> +
> +	bzero(fsp, sizeof(struct ethtool_rx_flow_spec));
> +	ipsa = ipda = ipsm = ipdm = spi = spim = 0x0;
> +	sp = dp = spm = dpm = 0x0;
> +	ip_ver = proto = tos = tm = 0x0;
> +	discard = ring_set = 0;
> +
> +	if (!strcmp(optstr[i], "ip4")) {
> +		ip_ver = ETH_RX_NFC_IP4;
> +	} else if (!strcmp(optstr[i], "ip6")) {
> +		fprintf(stdout, "IPv6 not yet implemented\n");
> +		return -1;
> +	} else {
> +		fprintf(stdout, "Add rule, invalid syntax for IP version\n");
> +		return -1;
> +	}
> +
> +	i++;
> +
> +	switch (ip_ver) {
> +	case ETH_RX_NFC_IP4:
> +		if (!strcmp(optstr[i], "tcp"))
> +			fsp->flow_type = TCP_V4_FLOW;
> +		else if (!strcmp(optstr[i], "udp"))
> +			fsp->flow_type = UDP_V4_FLOW;
> +		else if (!strcmp(optstr[i], "sctp"))
> +			fsp->flow_type = SCTP_V4_FLOW;
> +		else if (!strcmp(optstr[i], "ah"))
> +			fsp->flow_type = AH_V4_FLOW;
> +		else if (!strcmp(optstr[i], "esp"))
> +			fsp->flow_type = ESP_V4_FLOW;
> +		break;
> +	default:
> +		fprintf(stdout, "Add rule, Invalid IP version %d\n", ip_ver);
> +			return -1;
> +	}
> +
> +	if (fsp->flow_type == 0) {
> +		proto = (u_int8_t)strtoul(optstr[i], (char **)NULL, 0);
> +		if (proto != 0) {
> +			fprintf(stdout, "Add rule, user defined proto %d\n",
> +				proto);
> +			fsp->flow_type = IP_USER_FLOW;
> +			fsp->h_u.usr_ip4_spec.proto = proto;
> +			fsp->h_u.usr_ip4_spec.ip_ver = ip_ver;
> +		} else {
> +			fprintf(stdout, "Add rule, invalid IP proto %s\n",
> +				optstr[i]);
> +			return -1;
> +		}
> +	}
> +
> +	for (i = 2; i < opt_cnt;) {
> +		if (!strcmp(optstr[i], "tos")) {
> +			tos = (u_int8_t)strtoul(optstr[i+1], (char **)NULL,
> +						 0);
> +			tm = 0xff;
> +			fsp->h_u.tcp_ip4_spec.tos = tos;
> +
> +			i += 2;
> +			if (opt_cnt > (i+1)) {
> +				if (!strcmp(optstr[i], "m")) {
> +					tm = (u_int8_t)strtoul(optstr[i+1],
> +							       (char **)NULL,
> +							       16);
> +					i += 2;
> +				}
> +			}
> +			fsp->m_u.tcp_ip4_spec.tos = tm;
> +		} else if (!strcmp(optstr[i], "sip")) {
[...]

These keyword names must be made consistent with those used for the -U
(--config-ntuple) option.

Also, they can be parsed much more concisely using the new option types
I defined a while back for struct cmdline_info.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
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

* Re: [PATCH v2] ethtool : Add option -L | --set-common to set common flags.
From: Ben Hutchings @ 2011-02-21 16:00 UTC (permalink / raw)
  To: Mahesh Bandewar; +Cc: David Miller, Tom Herbert, Laurent Chavey, netdev
In-Reply-To: <1295390237-27381-1-git-send-email-maheshb@google.com>

On Tue, 2011-01-18 at 14:37 -0800, Mahesh Bandewar wrote:
> This patch adds -L | --set-common option to add / remove common flags which
> includes loopback flag. The -l | --show-common displays the current values
> for these common flags.
[...]

I haven't forgotten this, but I won't apply it until the corresponding
kernel change is applied.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
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

* Re: IGMPMSG_WRONGVIF only happens when true VIF is an OIF of correct IIF?
From: Dan Langlois @ 2011-02-21 16:03 UTC (permalink / raw)
  To: David Lamparter; +Cc: netdev
In-Reply-To: <20110218171225.GC1739639@jupiter.n2.diac24.net>

On Fri, 2011-02-18 at 18:12 +0100, David Lamparter wrote:
> On Fri, Feb 18, 2011 at 04:46:56PM +0100, Dan Langlois wrote:
> > Hi, I have a question about the IGMPMSG_WRONGVIF message.
> > I have MRT_ASSERT enabled but do not receive notifications.
> > However, if I enable MRT_PIM, I do get them.  I'm curious
> > why MRT_ASSERT alone isn't sufficient.
> > 
> > This if-statement appears in ip_mr_forward() in net/ipv4/ipmr.c
> > 
> >        if (true_vifi >= 0 && net->ipv4.mroute_do_assert &&
> >             /* pimsm uses asserts, when switching from RPT to SPT, 
> >                so that we cannot check that packet arrived on an oif. 
> >                It is bad, but otherwise we would need to move pretty
> >                large chunk of pimd to kernel. Ough... --ANK
> >              */      
> >             (net->ipv4.mroute_do_pim ||
> >              cache->mfc_un.res.ttls[true_vifi] < 255) && 
> >             time_after(jiffies,
> >                        cache->mfc_un.res.last_assert +
> > MFC_ASSERT_THRESH)) {
> >                 cache->mfc_un.res.last_assert = jiffies;
> >                 ipmr_cache_report(net, skb, true_vifi,
> > IGMPMSG_WRONGVIF);
> >         }       
> > 
> > 
> > By the time this statement is reached, it is known that the packet
> > did not arrive on the expected incoming interface (IIF) and thus a
> > "WRONGVIF" condition.  This if-statement decides whether a PIM-assert
> > notification needs to be sent to the multicast routing daemon.
> > 
> > The part of this if-statement I'm questioning is:
> >     (net->ipv4.mroute_do_pim ||
> >      cache->mfc_un.res.ttls[true_vifi] < 255) &&
> > 
> > I read this as:
> > "send WRONGVIF if PIM is enabled -OR-
> > the packet arrived on an outgoing interface OIF of the correct IIF"
> > 
> > So this statement is always true when PIM is enabled.
> > However, if only ASSERT is enabled, this statement is only true when
> > a packet is reflected back on an OIF.
> > 
> > Why not always send the assert for any WRONGVIF condition regardless
> > of the interface it came in on?  I mean, isn't that the point of
> > enabling MRT_ASSERT in the first place?  If the assertions weren't
> > wanted, just don't enable that feature, right?
> 
> The point of MRT_ASSERT is to aid in resolving duplication issues where
> you end up with multiple mrouters thinking they're responsible for the
> same subnet. This condition is indicated by more than 1 router on the
> subnet having the subnet as OIF on the (*,G) or (S,G). Therefore, your
> daemon receives the assertion if it is one of the forwarding mrouters,
> which it will only be if it's an OIF.
> 
> If your daemon doesn't have the target subnet listed as oif, the
> assumption is that a different mrouter has been elected to forward
> packets for this G/S,G to this subnet. The kernel knows that your daemon
> decided to not forward things to this subnet, so you are not involved in
> duplicates, so you don't get an assert.


Hmmm, well thanks for the explanation.  At least that explains why the
notification only applies to OIFs.  I suppose it would actually be wrong
to generate an ASSERT under any other condition?  


> The "PIM or" is a kludge to make PIM work, as indicated by the comment
> above the if. I'd have to refreshen my PIM knowledge to fully understand
> it or explain it ;)
> 
> > In our system, multicast streams may get rerouted through a different
> > VIF than what was first installed in the MFC cache.  I would very much
> > like to get these WRONGVIF assertions in this case, which is NOT the
> > case that a packet is seen on an OIF.
> > 
> > Of course I can simply enable MRT_PIM to get the messages, but in that
> > case I don't understand the reason for having two separate toggles
> > (MRT_ASSERT versus MRT_PIM).
> 
> I don't really understand you use case -- is this a case of another
> router showing up on the subnet and directing traffic to it? If so, why
> wasn't the first router directing traffic to it? Do you have a primary
> multicast forwarder election system in place?
> 

It is a bit of an odd setup:  We have multicast traffic aboard various
vehicles and we need to tunnel that back to a central server, to then be
forwarded on its various backend interfaces.  There is also traffic
coming in from those interfaces which needs to be directed back out to
one or more of these vehicles, depending on group membership.  We use
IGMP-based multicast forwarding daemons on the vehicles and on the
server (something like igmpproxyd, but bidirectional) and IP-IP tunnels
between the server and each vehicle.

These vehicles can be coupled together.  When this happens, we designate
one of the coupled vehicles as "master" and all traffic is sent through
the master tunnel, rather than each of the individual tunnels.  The
vehicles can of course be decoupled again, such that the multicast
streams need to be separated to individual tunnels again.  

Since the onboard equipment maintains its IP address despite coupling,
the (S,G) pairing remains the same, but now it is being forwarded
through the "master" rather than its own IP-IP tunnel.  This corresponds
to a different VIF on the server.  So this (de)coupling activity is why
traffic can shift between VIFs.  I noticed the "Wrong" statistic
incrementing, but didn't get an ASSERT.  Once I started digging I found
this if-statement and decided to ask about it.

Now I'm not too sure how to proceed.  In our system, the IIF for a (S,G)
can change due to these couplings.  Receiving the IGMPMSG_WRONGVIF
notification seems to be the most ideal fix, but maybe it isn't.  Can
you suggest something else?  Is it "dangerous" to enable PIM in an
IGMP-based multicast forwarding daemon? (e.g. a daemon that doesn't
implement PIM-SM or -DM, but is still protocol independent)  Perhaps
enabling MRT_PIM is actually the right solution?  Advice appreciated. 

cheers,
Dan


^ permalink raw reply

* Re: [Lxc-users] Huge ammount of invalid checksum packets on macvlan
From: Daniel Lezcano @ 2011-02-21 16:07 UTC (permalink / raw)
  To: Andrian Nord; +Cc: lxc-users, Patrick McHardy, Linux Netdev List
In-Reply-To: <20110221153421.GA6602@nord.niifaq.ru>

On 02/21/2011 04:34 PM, Andrian Nord wrote:
> Greetings, Daniel.
>
> On Mon, Feb 21, 2011 at 04:20:59PM +0100, Daniel Lezcano wrote:
>
> 2.6.37 kernel with gentoo linux patches (doesn't affect any low-system
> stuff, AFAIK).
> lxc-0.7.2 is used.
>
> Reproducable on two different machines.
> I'm using tcpdump -vvv for bad checksum detection. This also affects
> traffic from container to hardware node (as it's using macvlan to
> communicate with containers by itself).
>
> Also, I've got same problem on UDP packets coming from lan on other
> server and it was worked around by disabeling tx and rx checksum offload
> via ethtool. But dummy devices doesn't allow this.

I am not sure it is a bug. If we go outside of the container context and 
we do the following:

ssh 127.0.0.1
tcpdump -vvv -i lo

We will get the same errors AFAICS.

There is also in the man page the following option:

-K Don't attempt to verify IP, TCP, or UDP checksums. This is useful for 
inter‐
faces that perform some or all of those checksum calculation in 
hardware; other‐
wise, all outgoing TCP checksums will be flagged as bad.

IMO, the checksum is not needed for the virtual macvlan devices, hence 
the checksum is not computed and the checksum tcp packet is not filled. 
As the skb's are flagged as 'checksum not necessary' the packets are not 
dropped by the kernel and are delivered to the network stack. tcpdump 
intercept the raw packet and analyse the header. It will see a bad value 
as this one is a default value.

I Cc'ed the netdev mailing list and Patrick in case my analysis is wrong 
or incomplete.

Thanks
-- Daniel


^ permalink raw reply

* Re: [PATCH] sfc: lower stack usage in efx_ethtool_self_test
From: Ben Hutchings @ 2011-02-21 16:09 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, netdev
In-Reply-To: <1297864118.3201.354.camel@edumazet-laptop>

On Wed, 2011-02-16 at 14:48 +0100, Eric Dumazet wrote:
> drivers/net/sfc/ethtool.c: In function ‘efx_ethtool_self_test’:
> drivers/net/sfc/ethtool.c:613: warning: the frame size of 1200 bytes
> is larger than 1024 bytes

This also fixes a whopping information leak in case the device is in a
broken state.

> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Acked-by: Ben Hutchings <bhutchings@solarflare.com>

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
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

* Re: Advice on network driver design
From: Felix Radensky @ 2011-02-21 16:25 UTC (permalink / raw)
  To: arnd; +Cc: netdev@vger.kernel.org
In-Reply-To: <201102202013.34994.arnd@arndb.de>

Hi Anrd,

On 02/20/2011 09:13 PM, arnd@arndb.de wrote:
> On Saturday 19 February 2011 14:37:43 Felix Radensky wrote:
>> Hi,
>>
>> I'm in the process of designing a network driver for a custom
>> hardware and would like to get some advice from linux network
>> gurus.
>>
>> The host platform is Freescale P2020. The custom hardware is
>> FPGA with several TX FIFOs, single RX FIFO and a set of registers.
>> FPGA is connected to CPU via PCI-E. Host CPU DMA controller is used
>> to get packets to/from FIFOs. Each FIFO has its set of events,
>> generating interrupts, which can be enabled and disabled. Status
>> register reflects the current state of events, the bit in status
>> register is cleared by FPGA when event is handled. Reads or writes to
>> status register have no impact on its contents.
>>
>> The device driver should support 80Mbit/sec of traffic in each direction.
>>
>> So far I have TX side working. I'm using Linux dmaengine APIs to
>> transfer packets to FIFOs. The DMA completion interrupt is handled
>> by per-fifo work queue.
>>
>> My question is about RX. Would such design benefit from NAPI ?
>> If my understanding of NAPI is correct, it runs in softirq context,
>> so I cannot do any DMA work in dev->poll(). If I were to use NAPI,
>> I should probably disable RX interrupts, do all DMA work in some
>> work queue, keep RX packets in a list and only then call dev->poll().
>> Is that correct ?
>>
>> Any other advice and how to write an efficient driver for this
>> hardware is most welcome. I can influence FPGA design to some degree,
>> so if you think FPGA should be changed to improve things, please let
>> me know.
> There are currently discussions ongoing about using virtio for this
> kind of connection. See http://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg49294.html
> for an archive.
>
> When you use virtio as the base, you can use the regular virtio-net
> driver or any other virtio high-level driver on top.
>
> 	Arnd
>
Thanks, I'll take a look at virtio. Some people seem to think it's an 
overkill for this task.

Felix.

^ permalink raw reply

* [PATCH net-next] bnx2x: use dcb_setapp to manage negotiated application tlvs
From: Shmulik Ravid @ 2011-02-21 18:38 UTC (permalink / raw)
  To: davem; +Cc: eilong, netdev

With this patch the bnx2x uses the generic dcbnl application tlv list
instead of implementing its own get-app handler. When the driver is
alerted to a change in the DCB negotiated parameters, it calls
dcb_setapp to update the dcbnl application tlvs list making it available
to user mode applications and registered notifiers.   

Signed-off-by: Shmulik Ravid <shmulikr@broadcom.com>
Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
---
 drivers/net/bnx2x/bnx2x.h      |    2 +-
 drivers/net/bnx2x/bnx2x_dcb.c  |  137 ++++++++++++++++++++++++----------------
 drivers/net/bnx2x/bnx2x_dcb.h  |    5 +-
 drivers/net/bnx2x/bnx2x_main.c |    7 ++-
 4 files changed, 92 insertions(+), 59 deletions(-)

diff --git a/drivers/net/bnx2x/bnx2x.h b/drivers/net/bnx2x/bnx2x.h
index c0dd30d..1914026 100644
--- a/drivers/net/bnx2x/bnx2x.h
+++ b/drivers/net/bnx2x/bnx2x.h
@@ -31,7 +31,7 @@
 #define BNX2X_NEW_NAPI
 
 #if defined(CONFIG_DCB)
-#define BCM_DCB
+#define BCM_DCBNL
 #endif
 #if defined(CONFIG_CNIC) || defined(CONFIG_CNIC_MODULE)
 #define BCM_CNIC 1
diff --git a/drivers/net/bnx2x/bnx2x_dcb.c b/drivers/net/bnx2x/bnx2x_dcb.c
index fb60021..9a24d79 100644
--- a/drivers/net/bnx2x/bnx2x_dcb.c
+++ b/drivers/net/bnx2x/bnx2x_dcb.c
@@ -19,6 +19,9 @@
 #include <linux/netdevice.h>
 #include <linux/types.h>
 #include <linux/errno.h>
+#ifdef BCM_DCBNL
+#include <linux/dcbnl.h>
+#endif
 
 #include "bnx2x.h"
 #include "bnx2x_cmn.h"
@@ -508,13 +511,75 @@ static int bnx2x_dcbx_read_shmem_neg_results(struct bnx2x *bp)
 	return 0;
 }
 
+
+#ifdef BCM_DCBNL
+static inline
+u8 bnx2x_dcbx_dcbnl_app_up(struct dcbx_app_priority_entry *ent)
+{
+	u8 pri;
+
+	/* Choose the highest priority */
+	for (pri = MAX_PFC_PRIORITIES - 1; pri > 0; pri--)
+		if (ent->pri_bitmap & (1 << pri))
+			break;
+	return pri;
+}
+
+static inline
+u8 bnx2x_dcbx_dcbnl_app_idtype(struct dcbx_app_priority_entry *ent)
+{
+	return ((ent->appBitfield & DCBX_APP_ENTRY_SF_MASK) ==
+		DCBX_APP_SF_PORT) ? DCB_APP_IDTYPE_PORTNUM :
+		DCB_APP_IDTYPE_ETHTYPE;
+}
+
+static inline
+void bnx2x_dcbx_invalidate_local_apps(struct bnx2x *bp)
+{
+	int i;
+	for (i = 0; i < DCBX_MAX_APP_PROTOCOL; i++)
+		bp->dcbx_local_feat.app.app_pri_tbl[i].appBitfield &=
+							~DCBX_APP_ENTRY_VALID;
+}
+
+int bnx2x_dcbnl_update_applist(struct bnx2x *bp, bool delall)
+{
+	int i, err = 0;
+
+	for (i = 0; i < DCBX_MAX_APP_PROTOCOL && err == 0; i++) {
+		struct dcbx_app_priority_entry *ent =
+			&bp->dcbx_local_feat.app.app_pri_tbl[i];
+
+		if (ent->appBitfield & DCBX_APP_ENTRY_VALID) {
+			u8 up = bnx2x_dcbx_dcbnl_app_up(ent);
+
+			/* avoid invalid user-priority */
+			if (up) {
+				struct dcb_app app;
+				app.selector = bnx2x_dcbx_dcbnl_app_idtype(ent);
+				app.protocol = ent->app_id;
+				app.priority = delall ? 0 : up;
+				err = dcb_setapp(bp->dev, &app);
+			}
+		}
+	}
+	return err;
+}
+#endif
+
 void bnx2x_dcbx_set_params(struct bnx2x *bp, u32 state)
 {
 	switch (state) {
 	case BNX2X_DCBX_STATE_NEG_RECEIVED:
 		{
 			DP(NETIF_MSG_LINK, "BNX2X_DCBX_STATE_NEG_RECEIVED\n");
-
+#ifdef BCM_DCBNL
+			/**
+			 * Delete app tlvs from dcbnl before reading new
+			 * negotiation results
+			 */
+			bnx2x_dcbnl_update_applist(bp, true);
+#endif
 			/* Read neg results if dcbx is in the FW */
 			if (bnx2x_dcbx_read_shmem_neg_results(bp))
 				return;
@@ -526,10 +591,24 @@ void bnx2x_dcbx_set_params(struct bnx2x *bp, u32 state)
 						 bp->dcbx_error);
 
 			if (bp->state != BNX2X_STATE_OPENING_WAIT4_LOAD) {
+#ifdef BCM_DCBNL
+				/**
+				 * Add new app tlvs to dcbnl
+				 */
+				bnx2x_dcbnl_update_applist(bp, false);
+#endif
 				bnx2x_dcbx_stop_hw_tx(bp);
 				return;
 			}
 			/* fall through */
+#ifdef BCM_DCBNL
+			/**
+			 * Invalidate the local app tlvs if they are not added
+			 * to the dcbnl app list to avoid deleting them from
+			 * the list later on
+			 */
+			bnx2x_dcbx_invalidate_local_apps(bp);
+#endif
 		}
 	case BNX2X_DCBX_STATE_TX_PAUSED:
 		DP(NETIF_MSG_LINK, "BNX2X_DCBX_STATE_TX_PAUSED\n");
@@ -1505,8 +1584,7 @@ static void bnx2x_pfc_fw_struct_e2(struct bnx2x *bp)
 	bnx2x_dcbx_print_cos_params(bp,	pfc_fw_cfg);
 }
 /* DCB netlink */
-#ifdef BCM_DCB
-#include <linux/dcbnl.h>
+#ifdef BCM_DCBNL
 
 #define BNX2X_DCBX_CAPS		(DCB_CAP_DCBX_LLD_MANAGED | \
 				DCB_CAP_DCBX_VER_CEE | DCB_CAP_DCBX_STATIC)
@@ -1816,32 +1894,6 @@ static void bnx2x_dcbnl_set_pfc_state(struct net_device *netdev, u8 state)
 	bp->dcbx_config_params.admin_pfc_enable = (state ? 1 : 0);
 }
 
-static bool bnx2x_app_is_equal(struct dcbx_app_priority_entry *app_ent,
-			       u8 idtype, u16 idval)
-{
-	if (!(app_ent->appBitfield & DCBX_APP_ENTRY_VALID))
-		return false;
-
-	switch (idtype) {
-	case DCB_APP_IDTYPE_ETHTYPE:
-		if ((app_ent->appBitfield & DCBX_APP_ENTRY_SF_MASK) !=
-			DCBX_APP_SF_ETH_TYPE)
-			return false;
-		break;
-	case DCB_APP_IDTYPE_PORTNUM:
-		if ((app_ent->appBitfield & DCBX_APP_ENTRY_SF_MASK) !=
-			DCBX_APP_SF_PORT)
-			return false;
-		break;
-	default:
-		return false;
-	}
-	if (app_ent->app_id != idval)
-		return false;
-
-	return true;
-}
-
 static void bnx2x_admin_app_set_ent(
 	struct bnx2x_admin_priority_app_table *app_ent,
 	u8 idtype, u16 idval, u8 up)
@@ -1943,30 +1995,6 @@ static u8 bnx2x_dcbnl_set_app_up(struct net_device *netdev, u8 idtype,
 	return bnx2x_set_admin_app_up(bp, idtype, idval, up);
 }
 
-static u8 bnx2x_dcbnl_get_app_up(struct net_device *netdev, u8 idtype,
-				 u16 idval)
-{
-	int i;
-	u8 up = 0;
-
-	struct bnx2x *bp = netdev_priv(netdev);
-	DP(NETIF_MSG_LINK, "app_type %d, app_id 0x%x\n", idtype, idval);
-
-	/* iterate over the app entries looking for idtype and idval */
-	for (i = 0; i < DCBX_MAX_APP_PROTOCOL; i++)
-		if (bnx2x_app_is_equal(&bp->dcbx_local_feat.app.app_pri_tbl[i],
-				       idtype, idval))
-			break;
-
-	if (i < DCBX_MAX_APP_PROTOCOL)
-		/* if found return up */
-		up = bp->dcbx_local_feat.app.app_pri_tbl[i].pri_bitmap;
-	else
-		DP(NETIF_MSG_LINK, "app not found\n");
-
-	return up;
-}
-
 static u8 bnx2x_dcbnl_get_dcbx(struct net_device *netdev)
 {
 	struct bnx2x *bp = netdev_priv(netdev);
@@ -2107,7 +2135,6 @@ const struct dcbnl_rtnl_ops bnx2x_dcbnl_ops = {
 	.setnumtcs      = bnx2x_dcbnl_set_numtcs,
 	.getpfcstate    = bnx2x_dcbnl_get_pfc_state,
 	.setpfcstate    = bnx2x_dcbnl_set_pfc_state,
-	.getapp         = bnx2x_dcbnl_get_app_up,
 	.setapp         = bnx2x_dcbnl_set_app_up,
 	.getdcbx        = bnx2x_dcbnl_get_dcbx,
 	.setdcbx        = bnx2x_dcbnl_set_dcbx,
@@ -2115,4 +2142,4 @@ const struct dcbnl_rtnl_ops bnx2x_dcbnl_ops = {
 	.setfeatcfg     = bnx2x_dcbnl_set_featcfg,
 };
 
-#endif /* BCM_DCB */
+#endif /* BCM_DCBNL */
diff --git a/drivers/net/bnx2x/bnx2x_dcb.h b/drivers/net/bnx2x/bnx2x_dcb.h
index f650f98..71b8eda 100644
--- a/drivers/net/bnx2x/bnx2x_dcb.h
+++ b/drivers/net/bnx2x/bnx2x_dcb.h
@@ -189,8 +189,9 @@ enum {
 void bnx2x_dcbx_set_params(struct bnx2x *bp, u32 state);
 
 /* DCB netlink */
-#ifdef BCM_DCB
+#ifdef BCM_DCBNL
 extern const struct dcbnl_rtnl_ops bnx2x_dcbnl_ops;
-#endif /* BCM_DCB */
+int bnx2x_dcbnl_update_applist(struct bnx2x *bp, bool delall);
+#endif /* BCM_DCBNL */
 
 #endif /* BNX2X_DCB_H */
diff --git a/drivers/net/bnx2x/bnx2x_main.c b/drivers/net/bnx2x/bnx2x_main.c
index 6c7745e..061733e 100644
--- a/drivers/net/bnx2x/bnx2x_main.c
+++ b/drivers/net/bnx2x/bnx2x_main.c
@@ -9441,7 +9441,7 @@ static int __devinit bnx2x_init_dev(struct pci_dev *pdev,
 	dev->vlan_features |= (NETIF_F_TSO | NETIF_F_TSO_ECN);
 	dev->vlan_features |= NETIF_F_TSO6;
 
-#ifdef BCM_DCB
+#ifdef BCM_DCBNL
 	dev->dcbnl_ops = &bnx2x_dcbnl_ops;
 #endif
 
@@ -9848,6 +9848,11 @@ static void __devexit bnx2x_remove_one(struct pci_dev *pdev)
 	}
 #endif
 
+#ifdef BCM_DCBNL
+	/* Delete app tlvs from dcbnl */
+	bnx2x_dcbnl_update_applist(bp, true);
+#endif
+
 	unregister_netdev(dev);
 
 	/* Delete all NAPI objects */
-- 
1.7.3.5





^ permalink raw reply related

* [PATCH ethtool 0/3] Regularise handling of offload flags
From: Ben Hutchings @ 2011-02-21 16:54 UTC (permalink / raw)
  To: netdev, Michał Mirosław

This patch series is partly inspired by the recent changes to regularise
the relationship between ethtool offload control and net_device feature
flags, though I started on it earlier.

I don't intend to apply these changes until after the kernel and ethtool
v2.6.38 releases.  In the mean time, I would appreciate testing and
feedback.

Ben.

Ben Hutchings (3):
  ethtool-copy.h: sync with net-next-2.6
  ethtool: Regularise handling of offload flags
  ethtool: Report additional offload flag changes made automatically

 ethtool-copy.h |   89 +++++++++++-
 ethtool-util.h |   30 ++++
 ethtool.c      |  465 ++++++++++++++++++++++++++++----------------------------
 3 files changed, 352 insertions(+), 232 deletions(-)

-- 
1.7.3.4


-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
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

* [PATCH ethtool 1/3] ethtool-copy.h: sync with net-next-2.6
From: Ben Hutchings @ 2011-02-21 16:57 UTC (permalink / raw)
  To: netdev, Michał Mirosław
In-Reply-To: <1298307282.2608.47.camel@bwh-desktop>

This covers kernel changes up to:

commit e83d360d9a7e5d71d55c13e96b19109a2ea23bf0
Author: Michał Mirosław <mirq-linux@rere.qmqm.pl>
Date:   Tue Feb 15 16:59:18 2011 +0000

    net: introduce NETIF_F_RXCSUM

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
 ethtool-copy.h |   89 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 88 insertions(+), 1 deletions(-)

diff --git a/ethtool-copy.h b/ethtool-copy.h
index 75c3ae7..6430dbd 100644
--- a/ethtool-copy.h
+++ b/ethtool-copy.h
@@ -251,6 +251,7 @@ enum ethtool_stringset {
 	ETH_SS_STATS,
 	ETH_SS_PRIV_FLAGS,
 	ETH_SS_NTUPLE_FILTERS,
+	ETH_SS_FEATURES,
 };
 
 /* for passing string sets for data tagging */
@@ -523,6 +524,87 @@ struct ethtool_flash {
 	char	data[ETHTOOL_FLASH_MAX_FILENAME];
 };
 
+/* for returning and changing feature sets */
+
+/**
+ * struct ethtool_get_features_block - block with state of 32 features
+ * @available: mask of changeable features
+ * @requested: mask of features requested to be enabled if possible
+ * @active: mask of currently enabled features
+ * @never_changed: mask of features not changeable for any device
+ */
+struct ethtool_get_features_block {
+	__u32	available;
+	__u32	requested;
+	__u32	active;
+	__u32	never_changed;
+};
+
+/**
+ * struct ethtool_gfeatures - command to get state of device's features
+ * @cmd: command number = %ETHTOOL_GFEATURES
+ * @size: in: number of elements in the features[] array;
+ *       out: number of elements in features[] needed to hold all features
+ * @features: state of features
+ */
+struct ethtool_gfeatures {
+	__u32	cmd;
+	__u32	size;
+	struct ethtool_get_features_block features[0];
+};
+
+/**
+ * struct ethtool_set_features_block - block with request for 32 features
+ * @valid: mask of features to be changed
+ * @requested: values of features to be changed
+ */
+struct ethtool_set_features_block {
+	__u32	valid;
+	__u32	requested;
+};
+
+/**
+ * struct ethtool_sfeatures - command to request change in device's features
+ * @cmd: command number = %ETHTOOL_SFEATURES
+ * @size: array size of the features[] array
+ * @features: feature change masks
+ */
+struct ethtool_sfeatures {
+	__u32	cmd;
+	__u32	size;
+	struct ethtool_set_features_block features[0];
+};
+
+/*
+ * %ETHTOOL_SFEATURES changes features present in features[].valid to the
+ * values of corresponding bits in features[].requested. Bits in .requested
+ * not set in .valid or not changeable are ignored.
+ *
+ * Returns %EINVAL when .valid contains undefined or never-changable bits
+ * or size is not equal to required number of features words (32-bit blocks).
+ * Returns >= 0 if request was completed; bits set in the value mean:
+ *   %ETHTOOL_F_UNSUPPORTED - there were bits set in .valid that are not
+ *	changeable (not present in %ETHTOOL_GFEATURES' features[].available)
+ *	those bits were ignored.
+ *   %ETHTOOL_F_WISH - some or all changes requested were recorded but the
+ *      resulting state of bits masked by .valid is not equal to .requested.
+ *      Probably there are other device-specific constraints on some features
+ *      in the set. When %ETHTOOL_F_UNSUPPORTED is set, .valid is considered
+ *      here as though ignored bits were cleared.
+ *
+ * Meaning of bits in the masks are obtained by %ETHTOOL_GSSET_INFO (number of
+ * bits in the arrays - always multiple of 32) and %ETHTOOL_GSTRINGS commands
+ * for ETH_SS_FEATURES string set. First entry in the table corresponds to least
+ * significant bit in features[0] fields. Empty strings mark undefined features.
+ */
+enum ethtool_sfeatures_retval_bits {
+	ETHTOOL_F_UNSUPPORTED__BIT,
+	ETHTOOL_F_WISH__BIT,
+};
+
+#define ETHTOOL_F_UNSUPPORTED   (1 << ETHTOOL_F_UNSUPPORTED__BIT)
+#define ETHTOOL_F_WISH          (1 << ETHTOOL_F_WISH__BIT)
+
 
 /* CMDs currently supported */
 #define ETHTOOL_GSET		0x00000001 /* Get settings. */
@@ -534,7 +616,9 @@ struct ethtool_flash {
 #define ETHTOOL_GMSGLVL		0x00000007 /* Get driver message level */
 #define ETHTOOL_SMSGLVL		0x00000008 /* Set driver msg level. */
 #define ETHTOOL_NWAY_RST	0x00000009 /* Restart autonegotiation. */
-#define ETHTOOL_GLINK		0x0000000a /* Get link status (ethtool_value) */
+/* Get link status for host, i.e. whether the interface *and* the
+ * physical port (if there is one) are up (ethtool_value). */
+#define ETHTOOL_GLINK		0x0000000a
 #define ETHTOOL_GEEPROM		0x0000000b /* Get EEPROM data */
 #define ETHTOOL_SEEPROM		0x0000000c /* Set EEPROM data. */
 #define ETHTOOL_GCOALESCE	0x0000000e /* Get coalesce config */
@@ -585,6 +669,9 @@ struct ethtool_flash {
 #define ETHTOOL_GRXFHINDIR	0x00000038 /* Get RX flow hash indir'n table */
 #define ETHTOOL_SRXFHINDIR	0x00000039 /* Set RX flow hash indir'n table */
 
+#define ETHTOOL_GFEATURES	0x0000003a /* Get device offload settings */
+#define ETHTOOL_SFEATURES	0x0000003b /* Change device offload settings */
+
 /* compatibility with older code */
 #define SPARC_ETH_GSET		ETHTOOL_GSET
 #define SPARC_ETH_SSET		ETHTOOL_SSET
-- 
1.7.3.4



-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
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 related

* [PATCH ethtool 2/3] ethtool: Regularise handling of offload flags
From: Ben Hutchings @ 2011-02-21 16:59 UTC (permalink / raw)
  To: netdev, Michał Mirosław
In-Reply-To: <1298307282.2608.47.camel@bwh-desktop>

Use the new ETHTOOL_{G,S}FEATURES operations where available, and
use the new structure and netif feature flags in any case.

Replace repetitive code for getting/setting offload flags with data-
driven loops.

This changes error messages to use the same long names for offload
flags as in dump_offload(), and changes various exit codes to 1.

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
NEITF_F_* flags are copied into ethtool-util.h for now.  I think in
future they should be exposed from <linux/netdevice.h> (hence the
#ifndef).

Ben.

 ethtool-util.h |   30 ++++
 ethtool.c      |  441 +++++++++++++++++++++++++++-----------------------------
 2 files changed, 243 insertions(+), 228 deletions(-)

diff --git a/ethtool-util.h b/ethtool-util.h
index f053028..a118202 100644
--- a/ethtool-util.h
+++ b/ethtool-util.h
@@ -40,6 +40,36 @@ static inline u32 cpu_to_be32(u32 value)
 }
 #endif
 
+#ifndef NETIF_F_SG
+#define NETIF_F_SG		(1 << 0)
+#define NETIF_F_IP_CSUM		(1 << 1)
+#define NETIF_F_NO_CSUM		(1 << 2)
+#define NETIF_F_HW_CSUM		(1 << 3)
+#define NETIF_F_IPV6_CSUM	(1 << 4)
+#define NETIF_F_FRAGLIST	(1 << 6)
+#define NETIF_F_HW_VLAN_TX	(1 << 7)
+#define NETIF_F_HW_VLAN_RX	(1 << 8)
+#define NETIF_F_HW_VLAN_FILTER	(1 << 9)
+#define NETIF_F_GSO		(1 << 11)
+#define NETIF_F_GRO		(1 << 14)
+#define NETIF_F_LRO		(1 << 15)
+#define NETIF_F_TSO		(1 << 16)
+#define NETIF_F_UFO		(1 << 17)
+#define NETIF_F_GSO_ROBUST	(1 << 18)
+#define NETIF_F_TSO_ECN		(1 << 19)
+#define NETIF_F_TSO6		(1 << 20)
+#define NETIF_F_FSO		(1 << 21)
+#define NETIF_F_FCOE_CRC	(1 << 24)
+#define NETIF_F_SCTP_CSUM	(1 << 25)
+#define NETIF_F_FCOE_MTU	(1 << 26)
+#define NETIF_F_NTUPLE		(1 << 27)
+#define NETIF_F_RXHASH		(1 << 28)
+#define NETIF_F_RXCSUM		(1 << 29)
+#define NETIF_F_ALL_CSUM	(NETIF_F_NO_CSUM | NETIF_F_HW_CSUM | \
+				 NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM)
+#define NETIF_F_ALL_TSO 	(NETIF_F_TSO | NETIF_F_TSO6 | NETIF_F_TSO_ECN)
+#endif
+
 /* National Semiconductor DP83815, DP83816 */
 int natsemi_dump_regs(struct ethtool_drvinfo *info, struct ethtool_regs *regs);
 int natsemi_dump_eeprom(struct ethtool_drvinfo *info,
diff --git a/ethtool.c b/ethtool.c
index f680b6d..25601a5 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -299,15 +299,7 @@ static void show_usage(int badarg)
 static char *devname = NULL;
 
 static int goffload_changed = 0;
-static int off_csum_rx_wanted = -1;
-static int off_csum_tx_wanted = -1;
-static int off_sg_wanted = -1;
-static int off_tso_wanted = -1;
-static int off_ufo_wanted = -1;
-static int off_gso_wanted = -1;
-static u32 off_flags_wanted = 0;
-static u32 off_flags_mask = 0;
-static int off_gro_wanted = -1;
+struct ethtool_set_features_block off_features;
 
 static struct ethtool_pauseparam epause;
 static int gpause_changed = 0;
@@ -463,23 +455,30 @@ static struct cmdline_info cmdline_seeprom[] = {
 };
 
 static struct cmdline_info cmdline_offload[] = {
-	{ "rx", CMDL_BOOL, &off_csum_rx_wanted, NULL },
-	{ "tx", CMDL_BOOL, &off_csum_tx_wanted, NULL },
-	{ "sg", CMDL_BOOL, &off_sg_wanted, NULL },
-	{ "tso", CMDL_BOOL, &off_tso_wanted, NULL },
-	{ "ufo", CMDL_BOOL, &off_ufo_wanted, NULL },
-	{ "gso", CMDL_BOOL, &off_gso_wanted, NULL },
-	{ "lro", CMDL_FLAG, &off_flags_wanted, NULL,
-	  ETH_FLAG_LRO, &off_flags_mask },
-	{ "gro", CMDL_BOOL, &off_gro_wanted, NULL },
-	{ "rxvlan", CMDL_FLAG, &off_flags_wanted, NULL,
-	  ETH_FLAG_RXVLAN, &off_flags_mask },
-	{ "txvlan", CMDL_FLAG, &off_flags_wanted, NULL,
-	  ETH_FLAG_TXVLAN, &off_flags_mask },
-	{ "ntuple", CMDL_FLAG, &off_flags_wanted, NULL,
-	  ETH_FLAG_NTUPLE, &off_flags_mask },
-	{ "rxhash", CMDL_FLAG, &off_flags_wanted, NULL,
-	  ETH_FLAG_RXHASH, &off_flags_mask },
+	{ "rx", CMDL_FLAG, &off_features.requested, NULL,
+	  NETIF_F_RXCSUM, &off_features.valid },
+	{ "tx", CMDL_FLAG, &off_features.requested, NULL,
+	  NETIF_F_ALL_CSUM, &off_features.valid },
+	{ "sg", CMDL_FLAG, &off_features.requested, NULL,
+	  NETIF_F_SG, &off_features.valid },
+	{ "tso", CMDL_FLAG, &off_features.requested, NULL,
+	  NETIF_F_ALL_TSO, &off_features.valid },
+	{ "ufo", CMDL_FLAG, &off_features.requested, NULL,
+	  NETIF_F_UFO, &off_features.valid },
+	{ "gso", CMDL_FLAG, &off_features.requested, NULL,
+	  NETIF_F_GSO, &off_features.valid },
+	{ "lro", CMDL_FLAG, &off_features.requested, NULL,
+	  NETIF_F_LRO, &off_features.valid },
+	{ "gro", CMDL_FLAG, &off_features.requested, NULL,
+	  NETIF_F_GRO, &off_features.valid },
+	{ "rxvlan", CMDL_FLAG, &off_features.requested, NULL,
+	  NETIF_F_HW_VLAN_TX, &off_features.valid },
+	{ "txvlan", CMDL_FLAG, &off_features.requested, NULL,
+	  NETIF_F_HW_VLAN_TX, &off_features.valid },
+	{ "ntuple", CMDL_FLAG, &off_features.requested, NULL,
+	  NETIF_F_NTUPLE, &off_features.valid },
+	{ "rxhash", CMDL_FLAG, &off_features.requested, NULL,
+	  NETIF_F_RXHASH, &off_features.valid },
 };
 
 static struct cmdline_info cmdline_pause[] = {
@@ -1872,35 +1871,39 @@ static int dump_coalesce(void)
 	return 0;
 }
 
-static int dump_offload(int rx, int tx, int sg, int tso, int ufo, int gso,
-			int gro, int lro, int rxvlan, int txvlan, int ntuple,
-			int rxhash)
+static const struct {
+	const char *long_name;
+	u32 cmd;
+	u32 value;
+} off_feature_def[] = {
+	{ "rx-checksumming",		  ETHTOOL_GRXCSUM, NETIF_F_RXCSUM },
+	{ "tx-checksumming",		  ETHTOOL_GTXCSUM, NETIF_F_ALL_CSUM },
+	{ "scatter-gather",		  ETHTOOL_GSG,	   NETIF_F_SG },
+	{ "tcp-segmentation-offload",	  ETHTOOL_GTSO,	   NETIF_F_ALL_TSO },
+	{ "udp-fragmentation-offload",	  ETHTOOL_GUFO,	   NETIF_F_UFO },
+	{ "generic-segmentation-offload", ETHTOOL_GGSO,	   NETIF_F_GSO },
+	{ "generic-receive-offload",	  ETHTOOL_GGRO,	   NETIF_F_GRO },
+	{ "large-receive-offload",	  0,		   NETIF_F_LRO },
+	{ "rx-vlan-offload",		  0,		   NETIF_F_HW_VLAN_RX },
+	{ "tx-vlan-offload",		  0,		   NETIF_F_HW_VLAN_TX },
+	{ "ntuple-filters",		  0,		   NETIF_F_NTUPLE },
+	{ "receive-hashing",		  0,		   NETIF_F_RXHASH },
+};
+
+static int dump_offload(const struct ethtool_get_features_block *features)
 {
-	fprintf(stdout,
-		"rx-checksumming: %s\n"
-		"tx-checksumming: %s\n"
-		"scatter-gather: %s\n"
-		"tcp-segmentation-offload: %s\n"
-		"udp-fragmentation-offload: %s\n"
-		"generic-segmentation-offload: %s\n"
-		"generic-receive-offload: %s\n"
-		"large-receive-offload: %s\n"
-		"rx-vlan-offload: %s\n"
-		"tx-vlan-offload: %s\n"
-		"ntuple-filters: %s\n"
-		"receive-hashing: %s\n",
-		rx ? "on" : "off",
-		tx ? "on" : "off",
-		sg ? "on" : "off",
-		tso ? "on" : "off",
-		ufo ? "on" : "off",
-		gso ? "on" : "off",
-		gro ? "on" : "off",
-		lro ? "on" : "off",
-		rxvlan ? "on" : "off",
-		txvlan ? "on" : "off",
-		ntuple ? "on" : "off",
-		rxhash ? "on" : "off");
+	u32 value;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(off_feature_def); i++) {
+		value = off_feature_def[i].value;
+		printf("%s: %s%s%s\n",
+		       off_feature_def[i].long_name,
+		       (features->active & value) ? "on" : "off",
+		       (features->requested & ~features->active & value) ?
+		       " [requested on]" : "",
+		       (~features->available & value) ? " [unchangeable]" : "");
+	}
 
 	return 0;
 }
@@ -2219,97 +2222,72 @@ static int do_scoalesce(int fd, struct ifreq *ifr)
 	return 0;
 }
 
+/* the following list of flags are the same as their associated
+ * NETIF_F_xxx values in include/linux/netdevice.h
+ */
+static const u32 flags_dup_features =
+	(ETH_FLAG_LRO | ETH_FLAG_RXVLAN | ETH_FLAG_TXVLAN | ETH_FLAG_NTUPLE |
+	 ETH_FLAG_RXHASH);
+
 static int do_goffload(int fd, struct ifreq *ifr)
 {
+	struct {
+		struct ethtool_gfeatures cmd;
+		struct ethtool_get_features_block data[1];
+	} features;
 	struct ethtool_value eval;
-	int err, allfail = 1, rx = 0, tx = 0, sg = 0;
-	int tso = 0, ufo = 0, gso = 0, gro = 0, lro = 0, rxvlan = 0, txvlan = 0,
-	    ntuple = 0, rxhash = 0;
+	int err, allfail = 1;
+	u32 value;
+	int i;
 
 	fprintf(stdout, "Offload parameters for %s:\n", devname);
 
-	eval.cmd = ETHTOOL_GRXCSUM;
-	ifr->ifr_data = (caddr_t)&eval;
-	err = send_ioctl(fd, ifr);
-	if (err)
-		perror("Cannot get device rx csum settings");
-	else {
-		rx = eval.data;
+	features.cmd.cmd = ETHTOOL_GFEATURES;
+	features.cmd.size = ARRAY_SIZE(features.data);
+	ifr->ifr_data = (caddr_t)&features;
+	err = ioctl(fd, SIOCETHTOOL, ifr);
+	if (err == 0) {
 		allfail = 0;
-	}
+	} else if (errno != EOPNOTSUPP && errno != EPERM) {
+		perror("Cannot get device offload settings");
+	} else {
+		memset(&features.data, 0, sizeof(features.data));
 
-	eval.cmd = ETHTOOL_GTXCSUM;
-	ifr->ifr_data = (caddr_t)&eval;
-	err = send_ioctl(fd, ifr);
-	if (err)
-		perror("Cannot get device tx csum settings");
-	else {
-		tx = eval.data;
-		allfail = 0;
-	}
+		for (i = 0; i < ARRAY_SIZE(off_feature_def); i++) {
+			value = off_feature_def[i].value;
 
-	eval.cmd = ETHTOOL_GSG;
-	ifr->ifr_data = (caddr_t)&eval;
-	err = send_ioctl(fd, ifr);
-	if (err)
-		perror("Cannot get device scatter-gather settings");
-	else {
-		sg = eval.data;
-		allfail = 0;
-	}
+			/* Assume that anything we can get is changeable */
+			features.data[0].available |= value;
 
-	eval.cmd = ETHTOOL_GTSO;
-	ifr->ifr_data = (caddr_t)&eval;
-	err = send_ioctl(fd, ifr);
-	if (err)
-		perror("Cannot get device tcp segmentation offload settings");
-	else {
-		tso = eval.data;
-		allfail = 0;
-	}
+			if (!off_feature_def[i].cmd)
+				continue;
 
-	eval.cmd = ETHTOOL_GUFO;
-	ifr->ifr_data = (caddr_t)&eval;
-	err = ioctl(fd, SIOCETHTOOL, ifr);
-	if (err)
-		perror("Cannot get device udp large send offload settings");
-	else {
-		ufo = eval.data;
-		allfail = 0;
-	}
-
-	eval.cmd = ETHTOOL_GGSO;
-	ifr->ifr_data = (caddr_t)&eval;
-	err = ioctl(fd, SIOCETHTOOL, ifr);
-	if (err)
-		perror("Cannot get device generic segmentation offload settings");
-	else {
-		gso = eval.data;
-		allfail = 0;
-	}
+			eval.cmd = off_feature_def[i].cmd;
+			ifr->ifr_data = (caddr_t)&eval;
+			err = send_ioctl(fd, ifr);
+			if (err) {
+				fprintf(stderr,
+					"Cannot get device %s settings: %m\n",
+					off_feature_def[i].long_name);
+			} else {
+				if (eval.data)
+					features.data[0].active |= value;
+				allfail = 0;
+			}
+		}
 
-	eval.cmd = ETHTOOL_GFLAGS;
-	ifr->ifr_data = (caddr_t)&eval;
-	err = ioctl(fd, SIOCETHTOOL, ifr);
-	if (err) {
-		perror("Cannot get device flags");
-	} else {
-		lro = (eval.data & ETH_FLAG_LRO) != 0;
-		rxvlan = (eval.data & ETH_FLAG_RXVLAN) != 0;
-		txvlan = (eval.data & ETH_FLAG_TXVLAN) != 0;
-		ntuple = (eval.data & ETH_FLAG_NTUPLE) != 0;
-		rxhash = (eval.data & ETH_FLAG_RXHASH) != 0;
-		allfail = 0;
-	}
+		eval.cmd = ETHTOOL_GFLAGS;
+		ifr->ifr_data = (caddr_t)&eval;
+		err = ioctl(fd, SIOCETHTOOL, ifr);
+		if (err) {
+			perror("Cannot get device flags");
+		} else {
+			features.data[0].active |=
+				eval.data & flags_dup_features;
+			allfail = 0;
+		}
 
-	eval.cmd = ETHTOOL_GGRO;
-	ifr->ifr_data = (caddr_t)&eval;
-	err = ioctl(fd, SIOCETHTOOL, ifr);
-	if (err)
-		perror("Cannot get device GRO settings");
-	else {
-		gro = eval.data;
-		allfail = 0;
+		features.data[0].requested = features.data[0].active;
 	}
 
 	if (allfail) {
@@ -2317,114 +2295,121 @@ static int do_goffload(int fd, struct ifreq *ifr)
 		return 83;
 	}
 
-	return dump_offload(rx, tx, sg, tso, ufo, gso, gro, lro, rxvlan, txvlan,
-			    ntuple, rxhash);
+	return dump_offload(features.data);
 }
 
 static int do_soffload(int fd, struct ifreq *ifr)
 {
+	struct {
+		struct ethtool_gfeatures cmd;
+		struct ethtool_get_features_block data[1];
+	} get_features;
+	struct {
+		struct ethtool_sfeatures cmd;
+		struct ethtool_set_features_block data[1];
+	} set_features;
 	struct ethtool_value eval;
 	int err, changed = 0;
+	u32 value;
+	int i;
 
-	if (off_csum_rx_wanted >= 0) {
-		changed = 1;
-		eval.cmd = ETHTOOL_SRXCSUM;
-		eval.data = (off_csum_rx_wanted == 1);
-		ifr->ifr_data = (caddr_t)&eval;
-		err = send_ioctl(fd, ifr);
-		if (err) {
-			perror("Cannot set device rx csum settings");
-			return 84;
+	get_features.cmd.cmd = ETHTOOL_GFEATURES;
+	get_features.cmd.size = ARRAY_SIZE(get_features.data);
+	ifr->ifr_data = (caddr_t)&get_features;
+	err = ioctl(fd, SIOCETHTOOL, ifr);
+	if (err == 0) {
+		set_features.cmd.cmd = ETHTOOL_SFEATURES;
+		set_features.cmd.size = ARRAY_SIZE(set_features.data);
+		set_features.data[0] = off_features;
+
+		for (i = 0; i < ARRAY_SIZE(off_feature_def); i++) {
+			value = off_feature_def[i].value;
+			if (!(off_features.valid & value))
+				continue;
+			if (!(get_features.data[0].available & value)) {
+				/* None of these features can be changed */
+				fprintf(stderr,
+					"Cannot set device %s settings: "
+					"Operation not supported\n",
+					off_feature_def[i].long_name);
+			} else if (off_features.requested & value) {
+				/* Some of these features can be
+				 * enabled; mask out any that cannot
+				 */
+				set_features.data[0].requested &=
+					~(value &
+					  ~get_features.data[0].available);
+			}
 		}
-	}
 
-	if (off_csum_tx_wanted >= 0) {
-		changed = 1;
-		eval.cmd = ETHTOOL_STXCSUM;
-		eval.data = (off_csum_tx_wanted == 1);
-		ifr->ifr_data = (caddr_t)&eval;
-		err = send_ioctl(fd, ifr);
-		if (err) {
-			perror("Cannot set device tx csum settings");
-			return 85;
+		ifr->ifr_data = (caddr_t)&set_features;
+		err = ioctl(fd, SIOCETHTOOL, ifr);
+		if (err < 0) {
+			perror("Cannot set device offload settings");
+			return 1;
 		}
-	}
 
-	if (off_sg_wanted >= 0) {
-		changed = 1;
-		eval.cmd = ETHTOOL_SSG;
-		eval.data = (off_sg_wanted == 1);
-		ifr->ifr_data = (caddr_t)&eval;
-		err = send_ioctl(fd, ifr);
-		if (err) {
-			perror("Cannot set device scatter-gather settings");
-			return 86;
-		}
-	}
+		changed = !!set_features.data[0].valid;
 
-	if (off_tso_wanted >= 0) {
-		changed = 1;
-		eval.cmd = ETHTOOL_STSO;
-		eval.data = (off_tso_wanted == 1);
-		ifr->ifr_data = (caddr_t)&eval;
-		err = send_ioctl(fd, ifr);
-		if (err) {
-			perror("Cannot set device tcp segmentation offload settings");
-			return 88;
-		}
-	}
-	if (off_ufo_wanted >= 0) {
-		changed = 1;
-		eval.cmd = ETHTOOL_SUFO;
-		eval.data = (off_ufo_wanted == 1);
-		ifr->ifr_data = (caddr_t)&eval;
-		err = ioctl(fd, SIOCETHTOOL, ifr);
-		if (err) {
-			perror("Cannot set device udp large send offload settings");
-			return 89;
-		}
-	}
-	if (off_gso_wanted >= 0) {
-		changed = 1;
-		eval.cmd = ETHTOOL_SGSO;
-		eval.data = (off_gso_wanted == 1);
-		ifr->ifr_data = (caddr_t)&eval;
-		err = ioctl(fd, SIOCETHTOOL, ifr);
-		if (err) {
-			perror("Cannot set device generic segmentation offload settings");
-			return 90;
-		}
-	}
-	if (off_flags_mask) {
-		changed = 1;
-		eval.cmd = ETHTOOL_GFLAGS;
-		eval.data = 0;
-		ifr->ifr_data = (caddr_t)&eval;
-		err = ioctl(fd, SIOCETHTOOL, ifr);
-		if (err) {
-			perror("Cannot get device flag settings");
-			return 91;
+		if (err & ETHTOOL_F_WISH)
+			fprintf(stderr,
+				"Cannot set device offload settings: "
+				"Some requested features depend on others "
+				"that are not currently enabled\n");
+
+		/* ETHTOOL_F_UNSUPPORTED should never be set as we
+		 * checked for unsupported flags above.  Treat any
+		 * other warning flags as unknown.
+		 */
+		if (err & ~ETHTOOL_F_WISH)
+			fprintf(stderr,
+				"Cannot set device offload settings: "
+				"warning flags %#x",
+				err & ~ETHTOOL_F_WISH);
+	} else if (errno != EOPNOTSUPP && errno != EPERM) {
+		perror("Cannot get device offload settings");
+		return 1;
+	} else {
+		for (i = 0; i < ARRAY_SIZE(off_feature_def); i++) {
+			if (!off_feature_def[i].cmd)
+				continue;
+			if (off_features.valid & off_feature_def[i].value) {
+				changed = 1;
+				eval.cmd = off_feature_def[i].cmd + 1;
+				eval.data = !!(off_features.requested &
+					       off_feature_def[i].value);
+				ifr->ifr_data = (caddr_t)&eval;
+				err = send_ioctl(fd, ifr);
+				if (err) {
+					fprintf(stderr,
+						"Cannot set device %s settings: %m\n",
+						off_feature_def[i].long_name);
+					return 1;
+				}
+			}
 		}
 
-		eval.cmd = ETHTOOL_SFLAGS;
-		eval.data = ((eval.data & ~off_flags_mask) |
-			     off_flags_wanted);
+		if (off_features.valid & flags_dup_features) {
+			changed = 1;
+			eval.cmd = ETHTOOL_GFLAGS;
+			eval.data = 0;
+			ifr->ifr_data = (caddr_t)&eval;
+			err = ioctl(fd, SIOCETHTOOL, ifr);
+			if (err) {
+				perror("Cannot get device flag settings");
+				return 91;
+			}
 
-		err = ioctl(fd, SIOCETHTOOL, ifr);
-		if (err) {
-			perror("Cannot set device flag settings");
-			return 92;
-		}
-	}
-	if (off_gro_wanted >= 0) {
-		changed = 1;
-		eval.cmd = ETHTOOL_SGRO;
-		eval.data = (off_gro_wanted == 1);
-		ifr->ifr_data = (caddr_t)&eval;
-		err = ioctl(fd, SIOCETHTOOL, ifr);
-		if (err) {
-			perror("Cannot set device GRO settings");
-			return 93;
+			eval.cmd = ETHTOOL_SFLAGS;
+			eval.data &= ~(off_features.valid & flags_dup_features);
+			eval.data |= (off_features.requested &
+				      flags_dup_features);
+
+			err = ioctl(fd, SIOCETHTOOL, ifr);
+			if (err) {
+				perror("Cannot set device flag settings");
+				return 92;
+			}
 		}
 	}
 
-- 
1.7.3.4



-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
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 related

* [PATCH ethtool 3/3] ethtool: Report additional offload flag changes made automatically
From: Ben Hutchings @ 2011-02-21 16:59 UTC (permalink / raw)
  To: netdev, Michał Mirosław
In-Reply-To: <1298307282.2608.47.camel@bwh-desktop>

Turning off checksum offloads or SG will cause the kernel to turn off
other offloads that depend on them.  On some hardware, other offload
features may have to be turned on or off together.  Therefore, check
the offload flags before and after making changes and report any
additional changes beyond those requested.

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
 ethtool.c |  114 +++++++++++++++++++++++++++++++++++-------------------------
 1 files changed, 66 insertions(+), 48 deletions(-)

diff --git a/ethtool.c b/ethtool.c
index 25601a5..4e6bd41 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -1890,13 +1890,16 @@ static const struct {
 	{ "receive-hashing",		  0,		   NETIF_F_RXHASH },
 };
 
-static int dump_offload(const struct ethtool_get_features_block *features)
+static int
+dump_offload(const struct ethtool_get_features_block *features, u32 mask)
 {
 	u32 value;
 	int i;
 
 	for (i = 0; i < ARRAY_SIZE(off_feature_def); i++) {
 		value = off_feature_def[i].value;
+		if (!(mask & value))
+			continue;
 		printf("%s: %s%s%s\n",
 		       off_feature_def[i].long_name,
 		       (features->active & value) ? "on" : "off",
@@ -2229,7 +2232,8 @@ static const u32 flags_dup_features =
 	(ETH_FLAG_LRO | ETH_FLAG_RXVLAN | ETH_FLAG_TXVLAN | ETH_FLAG_NTUPLE |
 	 ETH_FLAG_RXHASH);
 
-static int do_goffload(int fd, struct ifreq *ifr)
+static int get_offload(int fd, struct ifreq *ifr,
+		       struct ethtool_get_features_block *pfeatures)
 {
 	struct {
 		struct ethtool_gfeatures cmd;
@@ -2240,24 +2244,23 @@ static int do_goffload(int fd, struct ifreq *ifr)
 	u32 value;
 	int i;
 
-	fprintf(stdout, "Offload parameters for %s:\n", devname);
-
 	features.cmd.cmd = ETHTOOL_GFEATURES;
 	features.cmd.size = ARRAY_SIZE(features.data);
 	ifr->ifr_data = (caddr_t)&features;
 	err = ioctl(fd, SIOCETHTOOL, ifr);
 	if (err == 0) {
 		allfail = 0;
+		pfeatures[0] = features.data[0];
 	} else if (errno != EOPNOTSUPP && errno != EPERM) {
 		perror("Cannot get device offload settings");
 	} else {
-		memset(&features.data, 0, sizeof(features.data));
+		memset(pfeatures, 0, sizeof(*pfeatures));
 
 		for (i = 0; i < ARRAY_SIZE(off_feature_def); i++) {
 			value = off_feature_def[i].value;
 
 			/* Assume that anything we can get is changeable */
-			features.data[0].available |= value;
+			pfeatures[0].available |= value;
 
 			if (!off_feature_def[i].cmd)
 				continue;
@@ -2271,7 +2274,7 @@ static int do_goffload(int fd, struct ifreq *ifr)
 					off_feature_def[i].long_name);
 			} else {
 				if (eval.data)
-					features.data[0].active |= value;
+					pfeatures[0].active |= value;
 				allfail = 0;
 			}
 		}
@@ -2282,28 +2285,34 @@ static int do_goffload(int fd, struct ifreq *ifr)
 		if (err) {
 			perror("Cannot get device flags");
 		} else {
-			features.data[0].active |=
+			pfeatures[0].active |=
 				eval.data & flags_dup_features;
 			allfail = 0;
 		}
 
-		features.data[0].requested = features.data[0].active;
+		pfeatures[0].requested = pfeatures[0].active;
 	}
 
-	if (allfail) {
+	return allfail;
+}
+
+static int do_goffload(int fd, struct ifreq *ifr)
+{
+	struct ethtool_get_features_block features;
+
+	fprintf(stdout, "Offload parameters for %s:\n", devname);
+
+	if (get_offload(fd, ifr, &features)) {
 		fprintf(stdout, "no offload info available\n");
 		return 83;
 	}
 
-	return dump_offload(features.data);
+	return dump_offload(&features, ~(u32)0);
 }
 
 static int do_soffload(int fd, struct ifreq *ifr)
 {
-	struct {
-		struct ethtool_gfeatures cmd;
-		struct ethtool_get_features_block data[1];
-	} get_features;
+	struct ethtool_get_features_block old_features, new_features;
 	struct {
 		struct ethtool_sfeatures cmd;
 		struct ethtool_set_features_block data[1];
@@ -2313,42 +2322,37 @@ static int do_soffload(int fd, struct ifreq *ifr)
 	u32 value;
 	int i;
 
-	get_features.cmd.cmd = ETHTOOL_GFEATURES;
-	get_features.cmd.size = ARRAY_SIZE(get_features.data);
-	ifr->ifr_data = (caddr_t)&get_features;
-	err = ioctl(fd, SIOCETHTOOL, ifr);
-	if (err == 0) {
-		set_features.cmd.cmd = ETHTOOL_SFEATURES;
-		set_features.cmd.size = ARRAY_SIZE(set_features.data);
-		set_features.data[0] = off_features;
+	if (get_offload(fd, ifr, &old_features)) {
+		fprintf(stderr, "no offload info available\n");
+		return 1;
+	}
 
-		for (i = 0; i < ARRAY_SIZE(off_feature_def); i++) {
-			value = off_feature_def[i].value;
-			if (!(off_features.valid & value))
-				continue;
-			if (!(get_features.data[0].available & value)) {
-				/* None of these features can be changed */
-				fprintf(stderr,
-					"Cannot set device %s settings: "
-					"Operation not supported\n",
-					off_feature_def[i].long_name);
-			} else if (off_features.requested & value) {
-				/* Some of these features can be
-				 * enabled; mask out any that cannot
-				 */
-				set_features.data[0].requested &=
-					~(value &
-					  ~get_features.data[0].available);
-			}
-		}
+	set_features.cmd.cmd = ETHTOOL_SFEATURES;
+	set_features.cmd.size = ARRAY_SIZE(set_features.data);
+	set_features.data[0] = off_features;
 
-		ifr->ifr_data = (caddr_t)&set_features;
-		err = ioctl(fd, SIOCETHTOOL, ifr);
-		if (err < 0) {
-			perror("Cannot set device offload settings");
-			return 1;
+	for (i = 0; i < ARRAY_SIZE(off_feature_def); i++) {
+		value = off_feature_def[i].value;
+		if (!(off_features.valid & value))
+			continue;
+		if (!(old_features.available & value)) {
+			/* None of these features can be changed */
+			fprintf(stderr,
+				"Cannot set device %s settings: "
+				"Operation not supported\n",
+				off_feature_def[i].long_name);
+		} else if (off_features.requested & value) {
+			/* Some of these features can be
+			 * enabled; mask out any that cannot
+			 */
+			set_features.data[0].requested &=
+				~(value & ~old_features.available);
 		}
+	}
 
+	ifr->ifr_data = (caddr_t)&set_features;
+	err = ioctl(fd, SIOCETHTOOL, ifr);
+	if (err >= 0) {
 		changed = !!set_features.data[0].valid;
 
 		if (err & ETHTOOL_F_WISH)
@@ -2367,7 +2371,7 @@ static int do_soffload(int fd, struct ifreq *ifr)
 				"warning flags %#x",
 				err & ~ETHTOOL_F_WISH);
 	} else if (errno != EOPNOTSUPP && errno != EPERM) {
-		perror("Cannot get device offload settings");
+		perror("Cannot set device offload settings");
 		return 1;
 	} else {
 		for (i = 0; i < ARRAY_SIZE(off_feature_def); i++) {
@@ -2415,6 +2419,20 @@ static int do_soffload(int fd, struct ifreq *ifr)
 
 	if (!changed) {
 		fprintf(stdout, "no offload settings changed\n");
+		return 0;
+	}
+
+	/* Were any additional changes made automatically? */
+	if (get_offload(fd, ifr, &new_features)) {
+		fprintf(stderr, "no offload info available\n");
+		return 1;
+	}
+	value = ((old_features.active & ~off_features.valid) |
+		 off_features.requested) ^
+		new_features.active;
+	if (value) {
+		printf("Additional changes:\n");
+		dump_offload(&new_features, value);
 	}
 
 	return 0;
-- 
1.7.3.4


-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
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 related


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