et131x: Promote staging et131x driver to drivers/net

RFC PATCH Superseded Mark Einon 4 participants 15 messages Jan 18, 2013 – Jan 29, 2013

Subsystems (4)

Networking Network drivers Staging ET131X network driver

lore

Share & tools

lore

Mark Einon

author maintainer

2013-01-18 20:40 UTC #0 lore raw

RFC PATCH 1/1 et131x: Promote staging et131x driver to drivers/net

This patch moves the et131x gigabit ethernet driver from drivers/staging to drivers/net/ethernet/agere.

All the existing issues noted for this driver have been resolved, apart from one performance issue where some fragmented packets suffer from frame receive errors. As I've only tested the code on one piece of hardware to date, it would be useful to get feedback on this issue from any other users of the device (my device has been stepped on several times now, and is known to need a few wiggles before starting up correctly).

Signed-off-by Mark Einon <[email protected]>

---
 drivers/net/ethernet/Kconfig        |    1 +
 drivers/net/ethernet/Makefile       |    1 +
 drivers/net/ethernet/agere/Kconfig  |   31 +
 drivers/net/ethernet/agere/Makefile |    5 +
 drivers/net/ethernet/agere/et131x.c | 5060 +++++++++++++++++++++++++++++++++++
 drivers/net/ethernet/agere/et131x.h | 1710 ++++++++++++
 drivers/staging/Kconfig             |    2 -
 drivers/staging/Makefile            |    1 -
 drivers/staging/et131x/Kconfig      |   10 -
 drivers/staging/et131x/Makefile     |    5 -
 drivers/staging/et131x/README       |   16 -
 drivers/staging/et131x/et131x.c     | 5060 -----------------------------------
 drivers/staging/et131x/et131x.h     | 1710 ------------
 13 files changed, 6808 insertions(+), 6804 deletions(-)
 create mode 100644 drivers/net/ethernet/agere/Kconfig
 create mode 100644 drivers/net/ethernet/agere/Makefile
 create mode 100644 drivers/net/ethernet/agere/et131x.c
 create mode 100644 drivers/net/ethernet/agere/et131x.h
 delete mode 100644 drivers/staging/et131x/Kconfig
 delete mode 100644 drivers/staging/et131x/Makefile
 delete mode 100644 drivers/staging/et131x/README
 delete mode 100644 drivers/staging/et131x/et131x.c
 delete mode 100644 drivers/staging/et131x/et131x.h

drivers/net/ethernet/Kconfig +1 -0

index e4ff389..aa954f3 100644
@@ -20,6 +20,7 @@ config SUNGEM_PHY
 source "drivers/net/ethernet/3com/Kconfig"
 source "drivers/net/ethernet/adaptec/Kconfig"
 source "drivers/net/ethernet/aeroflex/Kconfig"
+source "drivers/net/ethernet/agere/Kconfig"
 source "drivers/net/ethernet/alteon/Kconfig"
 source "drivers/net/ethernet/amd/Kconfig"
 source "drivers/net/ethernet/apple/Kconfig"

drivers/net/ethernet/Makefile +1 -0

index d447307..9b1d129 100644
@@ -6,6 +6,7 @@ obj-$(CONFIG_NET_VENDOR_3COM) += 3com/
 obj-$(CONFIG_NET_VENDOR_8390) += 8390/
 obj-$(CONFIG_NET_VENDOR_ADAPTEC) += adaptec/
 obj-$(CONFIG_GRETH) += aeroflex/
+obj-$(CONFIG_NET_VENDOR_AGERE) += agere/
 obj-$(CONFIG_NET_VENDOR_ALTEON) += alteon/
 obj-$(CONFIG_NET_VENDOR_AMD) += amd/
 obj-$(CONFIG_NET_VENDOR_APPLE) += apple/

drivers/net/ethernet/agere/Kconfig new file +31 -0

new file mode 100644
index 0000000..b26b185a
@@ -0,0 +1,31 @@
+#
+# Agere device configuration
+#
+
+config NET_VENDOR_AGERE
+	bool "Agere devices"
+	default y
+	depends on PCI
+	---help---
+	  If you have a network (Ethernet) card belonging to this class, say Y
+	  and read the Ethernet-HOWTO, available from
+	  <http://www.tldp.org/docs.html#howto>.
+
+	  Note that the answer to this question doesn't directly affect the
+	  kernel: saying N will just cause the configurator to skip all
+	  the questions about Atheros devices. If you say Y, you will be asked
+	  for your specific card in the following questions.
+
+if NET_VENDOR_AGERE
+
+config ET131X
+	tristate "Agere ET-1310 Gigabit Ethernet support"
+	depends on PCI
+	select PHYLIB
+	---help---
+	  This driver supports Agere ET-1310 ethernet adapters.
+
+	  To compile this driver as a module, choose M here. The module
+	  will be called et131x.
+
+endif # NET_VENDOR_AGERE

drivers/net/ethernet/agere/Makefile new file +5 -0

new file mode 100644
index 0000000..027ff94
@@ -0,0 +1,5 @@
+#
+# Makefile for the Agere ET-131x ethernet driver
+#
+
+obj-$(CONFIG_ET131X) += et131x.o

drivers/net/ethernet/agere/et131x.c new file +5060 -0

2,950 lines not on this page · raw message

drivers/net/ethernet/agere/et131x.h new file +1710 -0

1,710 lines not on this page · raw message

drivers/staging/Kconfig +0 -2

index db8a512..2bc8f78 100644

Lines of this file are not shown. View the raw message.

drivers/staging/Makefile +0 -1

index a9d5479..ad02341 100644

Lines of this file are not shown. View the raw message.

drivers/staging/et131x/Kconfig deleted +0 ~~-10~~

deleted file mode 100644
index 8190f2a..0000000

Lines of this file are not shown. View the raw message.

drivers/staging/et131x/Makefile deleted +0 -5

deleted file mode 100644
index 027ff94..0000000

Lines of this file are not shown. View the raw message.

drivers/staging/et131x/README deleted +0 ~~-16~~

deleted file mode 100644
index 38537d4..0000000

Lines of this file are not shown. View the raw message.

drivers/staging/et131x/et131x.c deleted +0 ~~-5060~~

5,060 lines not on this page · raw message

drivers/staging/et131x/et131x.h deleted +0 ~~-1710~~

1,710 lines not on this page · raw message

The rest of this diff is not shown. View the raw message.

Signature · 1 line

1.7.9.5

Part of the diff is not shown here. View the raw message.

Greg KH

maintainer

2013-01-18 22:57 UTC #1 lore raw

Quoted 11 lines · Mark Einon

This patch moves the et131x gigabit ethernet driver from drivers/staging to drivers/net/ethernet/agere.
All the existing issues noted for this driver have been resolved, apart from one performance issue where some fragmented packets suffer from frame receive errors. As I've only tested the code on one piece of hardware to date, it would be useful to get feedback on this issue from any other users of the device (my device has been stepped on several times now, and is known to need a few wiggles before starting up correctly).
Signed-off-by Mark Einon <[email protected]>

No objection from me for this. It's up to the network driver maintainers if this is ok or not.

greg k-h

Dan Carpenter

2013-01-19 11:03 UTC #2 lore raw

Quoted 11 lines · Greg KH

Quoted 10 lines · Mark Einon
This patch moves the et131x gigabit ethernet driver from drivers/staging to drivers/net/ethernet/agere.
All the existing issues noted for this driver have been resolved, apart from one performance issue where some fragmented packets suffer from frame receive errors. As I've only tested the code on one piece of hardware to date, it would be useful to get feedback on this issue from any other users of the device (my device has been stepped on several times now, and is known to need a few wiggles before starting up correctly).

Quoted 5 lines

Quoted 1 line
Signed-off-by Mark Einon <[email protected]>
No objection from me for this. It's up to the network driver maintainers if this is ok or not.

et131x_get_regs() has endian bugs calling et131x_mii_read().

There are a bunch of sparse warnings as well. I haven't looked at them because it's the weekend.

drivers/staging/et131x/et131x.c:1870:33: warning: incorrect type in assignment (different address spaces)
drivers/staging/et131x/et131x.c:1870:33: expected unsigned int [usertype] *num_des
drivers/staging/et131x/et131x.c:1870:33: got unsigned int [noderef] <asn:2>*<noident>
drivers/staging/et131x/et131x.c:1871:37: warning: incorrect type in assignment (different address spaces)
drivers/staging/et131x/et131x.c:1871:37: expected unsigned int [usertype] *full_offset
drivers/staging/et131x/et131x.c:1871:37: got unsigned int [noderef] <asn:2>*<noident>
drivers/staging/et131x/et131x.c:1872:33: warning: incorrect type in assignment (different address spaces)
drivers/staging/et131x/et131x.c:1872:33: expected unsigned int [usertype] *min_des
drivers/staging/et131x/et131x.c:1872:33: got unsigned int [noderef] <asn:2>*<noident>
drivers/staging/et131x/et131x.c:1873:33: warning: incorrect type in assignment (different address spaces)
drivers/staging/et131x/et131x.c:1873:33: expected unsigned int [usertype] *base_hi
drivers/staging/et131x/et131x.c:1873:33: got unsigned int [noderef] <asn:2>*<noident>
drivers/staging/et131x/et131x.c:1874:33: warning: incorrect type in assignment (different address spaces)
drivers/staging/et131x/et131x.c:1874:33: expected unsigned int [usertype] *base_lo
drivers/staging/et131x/et131x.c:1874:33: got unsigned int [noderef] <asn:2>*<noident>
drivers/staging/et131x/et131x.c:1876:33: warning: incorrect type in assignment (different address spaces)
drivers/staging/et131x/et131x.c:1876:33: expected unsigned int [usertype] *num_des
drivers/staging/et131x/et131x.c:1876:33: got unsigned int [noderef] <asn:2>*<noident>
drivers/staging/et131x/et131x.c:1877:37: warning: incorrect type in assignment (different address spaces)
drivers/staging/et131x/et131x.c:1877:37: expected unsigned int [usertype] *full_offset
drivers/staging/et131x/et131x.c:1877:37: got unsigned int [noderef] <asn:2>*<noident>
drivers/staging/et131x/et131x.c:1878:33: warning: incorrect type in assignment (different address spaces)
drivers/staging/et131x/et131x.c:1878:33: expected unsigned int [usertype] *min_des
drivers/staging/et131x/et131x.c:1878:33: got unsigned int [noderef] <asn:2>*<noident>
drivers/staging/et131x/et131x.c:1879:33: warning: incorrect type in assignment (different address spaces)
drivers/staging/et131x/et131x.c:1879:33: expected unsigned int [usertype] *base_hi
drivers/staging/et131x/et131x.c:1879:33: got unsigned int [noderef] <asn:2>*<noident>
drivers/staging/et131x/et131x.c:1880:33: warning: incorrect type in assignment (different address spaces)
drivers/staging/et131x/et131x.c:1880:33: expected unsigned int [usertype] *base_lo
drivers/staging/et131x/et131x.c:1880:33: got unsigned int [noderef] <asn:2>*<noident>
drivers/staging/et131x/et131x.c:1898:24: warning: incorrect type in argument 2 (different address spaces)
drivers/staging/et131x/et131x.c:1898:24: expected void volatile [noderef] <asn:2>*addr
drivers/staging/et131x/et131x.c:1898:24: got unsigned int [usertype] *base_hi
drivers/staging/et131x/et131x.c:1900:24: warning: incorrect type in argument 2 (different address spaces)
drivers/staging/et131x/et131x.c:1900:24: expected void volatile [noderef] <asn:2>*addr
drivers/staging/et131x/et131x.c:1900:24: got unsigned int [usertype] *base_lo
drivers/staging/et131x/et131x.c:1901:60: warning: incorrect type in argument 2 (different address spaces)
drivers/staging/et131x/et131x.c:1901:60: expected void volatile [noderef] <asn:2>*addr
drivers/staging/et131x/et131x.c:1901:60: got unsigned int [usertype] *num_des
drivers/staging/et131x/et131x.c:1902:39: warning: incorrect type in argument 2 (different address spaces)
drivers/staging/et131x/et131x.c:1902:39: expected void volatile [noderef] <asn:2>*addr
drivers/staging/et131x/et131x.c:1902:39: got unsigned int [usertype] *full_offset
drivers/staging/et131x/et131x.c:1910:24: warning: incorrect type in argument 2 (different address spaces)
drivers/staging/et131x/et131x.c:1910:24: expected void volatile [noderef] <asn:2>*addr
drivers/staging/et131x/et131x.c:1910:24: got unsigned int [usertype] *min_des
drivers/staging/et131x/et131x.c:2583:32: warning: incorrect type in assignment (different address spaces)
drivers/staging/et131x/et131x.c:2583:32: expected unsigned int [usertype] *offset
drivers/staging/et131x/et131x.c:2583:32: got unsigned int [noderef] <asn:2>*<noident>
drivers/staging/et131x/et131x.c:2585:32: warning: incorrect type in assignment (different address spaces)
drivers/staging/et131x/et131x.c:2585:32: expected unsigned int [usertype] *offset
drivers/staging/et131x/et131x.c:2585:32: got unsigned int [noderef] <asn:2>*<noident>
drivers/staging/et131x/et131x.c:2602:24: warning: incorrect type in argument 2 (different address spaces)
drivers/staging/et131x/et131x.c:2602:24: expected void volatile [noderef] <asn:2>*addr
drivers/staging/et131x/et131x.c:2602:24: got unsigned int [usertype] *offset
drivers/staging/et131x/et131x.c:4093:13: warning: symbol 'et131x_isr' was not declared. Should it be static?
CC [M] drivers/staging/et131x/et131x.o

regards,
dan carpenter

Mark Einon

author maintainer

2013-01-19 11:48 UTC #3 lore raw

PATCH 1/1 staging: et131x: Fix all sparse warnings

Dan Carpenter has pointed out that there are several sparse warnings from et131x.c, listed below. This patch fixes all these errors.

drivers/staging/et131x/et131x.c:1870:33: warning: incorrect type in assignment (different address spaces) drivers/staging/et131x/et131x.c:1870:33: expected unsigned int [usertype] *num_des drivers/staging/et131x/et131x.c:1870:33: got unsigned int [noderef] <asn:2>*<noident> drivers/staging/et131x/et131x.c:1871:37: warning: incorrect type in assignment (different address spaces) drivers/staging/et131x/et131x.c:1871:37: expected unsigned int [usertype] *full_offset drivers/staging/et131x/et131x.c:1871:37: got unsigned int [noderef] <asn:2>*<noident> drivers/staging/et131x/et131x.c:1872:33: warning: incorrect type in assignment (different address spaces) drivers/staging/et131x/et131x.c:1872:33: expected unsigned int [usertype] *min_des drivers/staging/et131x/et131x.c:1872:33: got unsigned int [noderef] <asn:2>*<noident> drivers/staging/et131x/et131x.c:1873:33: warning: incorrect type in assignment (different address spaces) drivers/staging/et131x/et131x.c:1873:33: expected unsigned int [usertype] *base_hi drivers/staging/et131x/et131x.c:1873:33: got unsigned int [noderef] <asn:2>*<noident> drivers/staging/et131x/et131x.c:1874:33: warning: incorrect type in assignment (different address spaces) drivers/staging/et131x/et131x.c:1874:33: expected unsigned int [usertype] *base_lo drivers/staging/et131x/et131x.c:1874:33: got unsigned int [noderef] <asn:2>*<noident> drivers/staging/et131x/et131x.c:1876:33: warning: incorrect type in assignment (different address spaces) drivers/staging/et131x/et131x.c:1876:33: expected unsigned int [usertype] *num_des drivers/staging/et131x/et131x.c:1876:33: got unsigned int [noderef] <asn:2>*<noident> drivers/staging/et131x/et131x.c:1877:37: warning: incorrect type in assignment (different address spaces) drivers/staging/et131x/et131x.c:1877:37: expected unsigned int [usertype] *full_offset drivers/staging/et131x/et131x.c:1877:37: got unsigned int [noderef] <asn:2>*<noident> drivers/staging/et131x/et131x.c:1878:33: warning: incorrect type in assignment (different address spaces) drivers/staging/et131x/et131x.c:1878:33: expected unsigned int [usertype] *min_des drivers/staging/et131x/et131x.c:1878:33: got unsigned int [noderef] <asn:2>*<noident> drivers/staging/et131x/et131x.c:1879:33: warning: incorrect type in assignment (different address spaces) drivers/staging/et131x/et131x.c:1879:33: expected unsigned int [usertype] *base_hi drivers/staging/et131x/et131x.c:1879:33: got unsigned int [noderef] <asn:2>*<noident> drivers/staging/et131x/et131x.c:1880:33: warning: incorrect type in assignment (different address spaces) drivers/staging/et131x/et131x.c:1880:33: expected unsigned int [usertype] *base_lo drivers/staging/et131x/et131x.c:1880:33: got unsigned int [noderef] <asn:2>*<noident> drivers/staging/et131x/et131x.c:1898:24: warning: incorrect type in argument 2 (different address spaces) drivers/staging/et131x/et131x.c:1898:24: expected void volatile [noderef] <asn:2>*addr drivers/staging/et131x/et131x.c:1898:24: got unsigned int [usertype] *base_hi drivers/staging/et131x/et131x.c:1900:24: warning: incorrect type in argument 2 (different address spaces) drivers/staging/et131x/et131x.c:1900:24: expected void volatile [noderef] <asn:2>*addr drivers/staging/et131x/et131x.c:1900:24: got unsigned int [usertype] *base_lo drivers/staging/et131x/et131x.c:1901:60: warning: incorrect type in argument 2 (different address spaces) drivers/staging/et131x/et131x.c:1901:60: expected void volatile [noderef] <asn:2>*addr drivers/staging/et131x/et131x.c:1901:60: got unsigned int [usertype] *num_des drivers/staging/et131x/et131x.c:1902:39: warning: incorrect type in argument 2 (different address spaces) drivers/staging/et131x/et131x.c:1902:39: expected void volatile [noderef] <asn:2>*addr drivers/staging/et131x/et131x.c:1902:39: got unsigned int [usertype] *full_offset drivers/staging/et131x/et131x.c:1910:24: warning: incorrect type in argument 2 (different address spaces) drivers/staging/et131x/et131x.c:1910:24: expected void volatile [noderef] <asn:2>*addr drivers/staging/et131x/et131x.c:1910:24: got unsigned int [usertype] *min_des drivers/staging/et131x/et131x.c:2583:32: warning: incorrect type in assignment (different address spaces) drivers/staging/et131x/et131x.c:2583:32: expected unsigned int [usertype] *offset drivers/staging/et131x/et131x.c:2583:32: got unsigned int [noderef] <asn:2>*<noident> drivers/staging/et131x/et131x.c:2585:32: warning: incorrect type in assignment (different address spaces) drivers/staging/et131x/et131x.c:2585:32: expected unsigned int [usertype] *offset drivers/staging/et131x/et131x.c:2585:32: got unsigned int [noderef] <asn:2>*<noident> drivers/staging/et131x/et131x.c:2602:24: warning: incorrect type in argument 2 (different address spaces) drivers/staging/et131x/et131x.c:2602:24: expected void volatile [noderef] <asn:2>*addr drivers/staging/et131x/et131x.c:2602:24: got unsigned int [usertype] *offset drivers/staging/et131x/et131x.c:4093:13: warning: symbol 'et131x_isr' was not declared. Should it be static?

Signed-off-by Mark Einon <[email protected]>

---
 drivers/staging/et131x/et131x.c |   14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

drivers/staging/et131x/et131x.c +7 -7

index 82508fb..f0bc7ba 100644
@@ -1860,11 +1860,11 @@ static void et131x_config_rx_dma_regs(struct et131x_adapter *adapter)
 	rx_local->local_psr_full = 0;
 
 	for (id = 0; id < NUM_FBRS; id++) {
-		u32 *num_des;
-		u32 *full_offset;
-		u32 *min_des;
-		u32 *base_hi;
-		u32 *base_lo;
+		u32 __iomem *num_des;
+		u32 __iomem *full_offset;
+		u32 __iomem *min_des;
+		u32 __iomem *base_hi;
+		u32 __iomem *base_lo;
 
 		if (id == 0) {
 			num_des = &rx_dma->fbr0_num_des;

@@ -2574,7 +2574,7 @@ static void nic_return_rfd(struct et131x_adapter *adapter, struct rfd *rfd)
 	 * need to clean up OOB data
 	 */
 	if (buff_index < rx_local->fbr[ring_index]->num_entries) {
-		u32 *offset;
+		u32 __iomem *offset;
 		struct fbr_desc *next;
 
 		spin_lock_irqsave(&adapter->fbr_lock, flags);

@@ -4090,7 +4090,7 @@ static SIMPLE_DEV_PM_OPS(et131x_pm_ops, et131x_suspend, et131x_resume);
  *
  * Returns a value indicating if the interrupt was handled.
  */
-irqreturn_t et131x_isr(int irq, void *dev_id)
+static irqreturn_t et131x_isr(int irq, void *dev_id)
 {
 	bool handled = true;
 	struct net_device *netdev = (struct net_device *)dev_id;

Signature · 1 line

1.7.9.5

Mark Einon

author maintainer

2013-01-21 23:44 UTC #4 lore raw

In reply to Dan Carpenter

On 19 January 2013 11:03, Dan Carpenter <[email protected]> wrote:
<snip>

Quoted 3 lines

et131x_get_regs() has endian bugs calling et131x_mii_read().

</snip>

Hi Dan,

Could you be a bit more descriptive about the issues you think there are with these calls?

cheers,

Mark

Dan Carpenter

2013-01-22 06:20 UTC #5 lore raw

Quoted 12 lines · Mark Einon

On 19 January 2013 11:03, Dan Carpenter <[email protected]> wrote:
<snip>
Quoted 3 lines
et131x_get_regs() has endian bugs calling et131x_mii_read().
</snip>
Hi Dan,
Could you be a bit more descriptive about the issues you think there are with these calls?

Sorry, that was sloppy on my part. Here's what I meant:

	et131x_mii_read(adapter, MII_BMCR, (u16 *)&regs_buff[num++]);

That puts the number in the 2 high bits which works for little endian systems but not for big endian. It should be something like:

	u16 tmp;

	et131x_mii_read(adapter, MII_BMCR, &tmp);
	regs_buff[num++] = tmp;

regards,
dan carpenter

Mark Einon

author maintainer

2013-01-23 10:10 UTC #6 lore raw

RFC PATCH v2 1/1 et131x: Promote staging et131x driver to drivers/net

In reply to Mark Einon

This patch moves the et131x gigabit ethernet driver from drivers/staging to drivers/net/ethernet/agere.

All the existing issues noted for this driver have been resolved, apart from one performance issue where some fragmented packets suffer from frame receive errors.

Signed-off-by Mark Einon <[email protected]>

---
 MAINTAINERS                                        |   11 ++++---
 drivers/net/ethernet/Kconfig                       |    1 +
 drivers/net/ethernet/Makefile                      |    1 +
 drivers/net/ethernet/agere/Kconfig                 |   32 ++++++++++++++++++++
 .../et131x => net/ethernet/agere}/Makefile         |    1 +
 .../et131x => net/ethernet/agere}/et131x.c         |    0
 .../et131x => net/ethernet/agere}/et131x.h         |    0
 drivers/staging/Kconfig                            |    2 --
 drivers/staging/Makefile                           |    1 -
 drivers/staging/et131x/Kconfig                     |   10 ------
 drivers/staging/et131x/README                      |   16 ----------
 11 files changed, 41 insertions(+), 34 deletions(-)
 create mode 100644 drivers/net/ethernet/agere/Kconfig
 rename drivers/{staging/et131x => net/ethernet/agere}/Makefile (98%)
 rename drivers/{staging/et131x => net/ethernet/agere}/et131x.c (100%)
 rename drivers/{staging/et131x => net/ethernet/agere}/et131x.h (100%)
 delete mode 100644 drivers/staging/et131x/Kconfig
 delete mode 100644 drivers/staging/et131x/README

MAINTAINERS +6 -5

index e80447a..cb09e9e 100644
@@ -3029,6 +3029,12 @@ T:	git git://git.kernel.org/pub/scm/linux/kernel/git/kristoffer/linux-hpc.git
 F:	drivers/video/s1d13xxxfb.c
 F:	include/video/s1d13xxxfb.h
 
+ET131X NETWORK DRIVER
+M:	Mark Einon <[email protected]>
+L:	[email protected]
+S:	Maintained
+F:	drivers/net/ethernet/agere
+
 ETHEREXPRESS-16 NETWORK DRIVER
 M:	Philip Blundell <[email protected]>
 L:	[email protected]

@@ -7473,11 +7479,6 @@ M:	David Rowe <[email protected]>
 S:	Odd Fixes
 F:	drivers/staging/echo/
 
-STAGING - ET131X NETWORK DRIVER
-M:	Mark Einon <[email protected]>
-S:	Odd Fixes
-F:	drivers/staging/et131x/
-
 STAGING - FLARION FT1000 DRIVERS
 M:	Marek Belisko <[email protected]>
 S:	Odd Fixes

drivers/net/ethernet/Kconfig +1 -0

index ed956e0..4b4edbb 100644
@@ -20,6 +20,7 @@ config SUNGEM_PHY
 source "drivers/net/ethernet/3com/Kconfig"
 source "drivers/net/ethernet/adaptec/Kconfig"
 source "drivers/net/ethernet/aeroflex/Kconfig"
+source "drivers/net/ethernet/agere/Kconfig"
 source "drivers/net/ethernet/alteon/Kconfig"
 source "drivers/net/ethernet/amd/Kconfig"
 source "drivers/net/ethernet/apple/Kconfig"

drivers/net/ethernet/Makefile +1 -0

index 8268d85..7a96adb 100644
@@ -6,6 +6,7 @@ obj-$(CONFIG_NET_VENDOR_3COM) += 3com/
 obj-$(CONFIG_NET_VENDOR_8390) += 8390/
 obj-$(CONFIG_NET_VENDOR_ADAPTEC) += adaptec/
 obj-$(CONFIG_GRETH) += aeroflex/
+obj-$(CONFIG_NET_VENDOR_AGERE) += agere/
 obj-$(CONFIG_NET_VENDOR_ALTEON) += alteon/
 obj-$(CONFIG_NET_VENDOR_AMD) += amd/
 obj-$(CONFIG_NET_VENDOR_APPLE) += apple/

drivers/net/ethernet/agere/Kconfig new file +32 -0

new file mode 100644
index 0000000..6854966
@@ -0,0 +1,32 @@
+#
+# Agere device configuration
+#
+
+config NET_VENDOR_AGERE
+       bool "Agere devices"
+       default y
+       depends on PCI
+       ---help---
+         If you have a network (Ethernet) card belonging to this class, say Y
+         and read the Ethernet-HOWTO, available from
+         <http://www.tldp.org/docs.html#howto>.
+
+         Note that the answer to this question doesn't directly affect the
+         kernel: saying N will just cause the configurator to skip all
+         the questions about Atheros devices. If you say Y, you will be asked
+         for your specific card in the following questions.
+
+if NET_VENDOR_AGERE
+
+config ET131X
+       tristate "Agere ET-1310 Gigabit Ethernet support"
+       depends on PCI
+       select PHYLIB
+       ---help---
+         This driver supports Agere ET-1310 ethernet adapters.
+
+         To compile this driver as a module, choose M here. The module
+         will be called et131x.
+
+endif # NET_VENDOR_AGERE
+

drivers/net/ethernet/agere/Makefile renamed from drivers/staging/et131x/Makefile +1 -0

similarity index 98%
rename from drivers/staging/et131x/Makefile
rename to drivers/net/ethernet/agere/Makefile
index 027ff94..740f413 100644
@@ -3,3 +3,4 @@
 #
 
 obj-$(CONFIG_ET131X) += et131x.o
+

drivers/net/ethernet/agere/et131x.c renamed from drivers/staging/et131x/et131x.c +0 -0

similarity index 100%
rename from drivers/staging/et131x/et131x.c
rename to drivers/net/ethernet/agere/et131x.c

drivers/net/ethernet/agere/et131x.h renamed from drivers/staging/et131x/et131x.h +0 -0

similarity index 100%
rename from drivers/staging/et131x/et131x.h
rename to drivers/net/ethernet/agere/et131x.h

drivers/staging/Kconfig +0 -2

index 0b47a06..5dee726 100644
@@ -24,8 +24,6 @@ menuconfig STAGING
 
 if STAGING
 
-source "drivers/staging/et131x/Kconfig"
-
 source "drivers/staging/slicoss/Kconfig"
 
 source "drivers/staging/usbip/Kconfig"

drivers/staging/Makefile +0 -1

index b026ea3..6983d77 100644
@@ -5,7 +5,6 @@ obj-$(CONFIG_STAGING)		+= staging.o
 
 obj-y				+= media/
 obj-y				+= net/
-obj-$(CONFIG_ET131X)		+= et131x/
 obj-$(CONFIG_SLICOSS)		+= slicoss/
 obj-$(CONFIG_USBIP_CORE)	+= usbip/
 obj-$(CONFIG_W35UND)		+= winbond/

drivers/staging/et131x/Kconfig deleted +0 ~~-10~~

deleted file mode 100644
index 8190f2a..0000000
@@ -1,10 +0,0 @@
-config ET131X
-	tristate "Agere ET-1310 Gigabit Ethernet support"
-	depends on PCI && NET && NETDEVICES
-	select PHYLIB
-	default n
-	---help---
-	  This driver supports Agere ET-1310 ethernet adapters.
-
-	  To compile this driver as a module, choose M here. The module
-	  will be called et131x.

drivers/staging/et131x/README deleted +0 ~~-16~~

deleted file mode 100644
index 38537d4..0000000
@@ -1,16 +0,0 @@
-This is a driver for the ET1310 network device.
-
-Based on the driver found at https://sourceforge.net/projects/et131x/
-
-Cleaned up immensely by Olaf Hartman and Christoph Hellwig <[email protected]>
-
-Note, the powermanagement options were removed from the vendor provided
-driver as they did not build properly at the time.
-
-TODO:
-	- some rx packets have CRC/code/frame errors
-
-Please send patches to:
-	Greg Kroah-Hartman <[email protected]>
-	Mark Einon <[email protected]>
-

Signature · 1 line

1.7.10.4

Dan Carpenter

2013-01-23 10:31 UTC #7 lore raw

Quoted 12 lines · Mark Einon

drivers/net/ethernet/agere/Kconfig new file +6 -0

new file mode 100644
index 0000000..6854966
@@ -0,0 +1,32 @@
+#
+# Agere device configuration
+#
+
+config NET_VENDOR_AGERE
+       bool "Agere devices"

drivers/net/ethernet/agere/Kconfig line 16

Quoted 10 lines

drivers/net/ethernet/agere/Kconfig new file +10 -0

+       default y
+       depends on PCI
+       ---help---
+         If you have a network (Ethernet) card belonging to this class, say Y
+         and read the Ethernet-HOWTO, available from
+         <http://www.tldp.org/docs.html#howto>.
+
+         Note that the answer to this question doesn't directly affect the
+         kernel: saying N will just cause the configurator to skip all
+         the questions about Atheros devices. If you say Y, you will be asked

                                ^^^^^^^

Agere?

drivers/net/ethernet/agere/Kconfig line 18

Quoted 2 lines

drivers/net/ethernet/agere/Kconfig new file +2 -0
+         for your specific card in the following questions.
+

I think you should just leave this block out entirely. It doesn't save time for anyone.

regards,
dan carpenter

Mark Einon

author maintainer

2013-01-23 11:31 UTC #8 lore raw

Quoted 24 lines · Dan Carpenter

Quoted 12 lines · Mark Einon

drivers/net/ethernet/agere/Kconfig new file +6 -0

new file mode 100644
index 0000000..6854966
@@ -0,0 +1,32 @@
+#
+# Agere device configuration
+#
+
+config NET_VENDOR_AGERE
+       bool "Agere devices"

drivers/net/ethernet/agere/Kconfig line 16

Quoted 10 lines

drivers/net/ethernet/agere/Kconfig new file +10 -0

+       default y
+       depends on PCI
+       ---help---
+         If you have a network (Ethernet) card belonging to this class, say Y
+         and read the Ethernet-HOWTO, available from
+         <http://www.tldp.org/docs.html#howto>.
+
+         Note that the answer to this question doesn't directly affect the
+         kernel: saying N will just cause the configurator to skip all
+         the questions about Atheros devices. If you say Y, you will be asked

                                ^^^^^^^

Quoted 1 line

Agere?

Well spotted, thanks. I'll fix it asap.

Quoted 6 lines

drivers/net/ethernet/agere/Kconfig line 18
Quoted 3 lines
drivers/net/ethernet/agere/Kconfig new file +2 -0
+         for your specific card in the following questions.
+
I think you should just leave this block out entirely. It doesn't save time for anyone.

It looks to be standard boilerplate for all the net/ethernet drivers having their own directory. I'm going to leave it in for now. If that's an issue that needs a change, it would also affect the other drivers and I think should be tackled as a separate thing.

Cheers,

Mark

Mark Einon

author maintainer

2013-01-23 12:45 UTC #9 lore raw

RFC PATCH v3 1/1 et131x: Promote staging et131x driver to drivers/net

In reply to Mark Einon

This patch moves the et131x gigabit ethernet driver from drivers/staging to drivers/net/ethernet/agere.

All the existing issues noted for this driver have been resolved, apart from one performance issue where some fragmented packets suffer from frame receive errors.

Signed-off-by Mark Einon <[email protected]>

---

Changes from V2 - vendor name changed from Atheros to Agere in drivers/net/ethernet/agere/Kconfig

 MAINTAINERS                                        |   11 ++++---
 drivers/net/ethernet/Kconfig                       |    1 +
 drivers/net/ethernet/Makefile                      |    1 +
 drivers/net/ethernet/agere/Kconfig                 |   32 ++++++++++++++++++++
 .../et131x => net/ethernet/agere}/Makefile         |    1 +
 .../et131x => net/ethernet/agere}/et131x.c         |    0
 .../et131x => net/ethernet/agere}/et131x.h         |    0
 drivers/staging/Kconfig                            |    2 --
 drivers/staging/Makefile                           |    1 -
 drivers/staging/et131x/Kconfig                     |   10 ------
 drivers/staging/et131x/README                      |   16 ----------
 11 files changed, 41 insertions(+), 34 deletions(-)
 create mode 100644 drivers/net/ethernet/agere/Kconfig
 rename drivers/{staging/et131x => net/ethernet/agere}/Makefile (98%)
 rename drivers/{staging/et131x => net/ethernet/agere}/et131x.c (100%)
 rename drivers/{staging/et131x => net/ethernet/agere}/et131x.h (100%)
 delete mode 100644 drivers/staging/et131x/Kconfig
 delete mode 100644 drivers/staging/et131x/README

MAINTAINERS +6 -5

index e80447a..cb09e9e 100644
@@ -3029,6 +3029,12 @@ T:	git git://git.kernel.org/pub/scm/linux/kernel/git/kristoffer/linux-hpc.git
 F:	drivers/video/s1d13xxxfb.c
 F:	include/video/s1d13xxxfb.h
 
+ET131X NETWORK DRIVER
+M:	Mark Einon <[email protected]>
+L:	[email protected]
+S:	Maintained
+F:	drivers/net/ethernet/agere
+
 ETHEREXPRESS-16 NETWORK DRIVER
 M:	Philip Blundell <[email protected]>
 L:	[email protected]

@@ -7473,11 +7479,6 @@ M:	David Rowe <[email protected]>
 S:	Odd Fixes
 F:	drivers/staging/echo/
 
-STAGING - ET131X NETWORK DRIVER
-M:	Mark Einon <[email protected]>
-S:	Odd Fixes
-F:	drivers/staging/et131x/
-
 STAGING - FLARION FT1000 DRIVERS
 M:	Marek Belisko <[email protected]>
 S:	Odd Fixes

drivers/net/ethernet/Kconfig +1 -0

index ed956e0..4b4edbb 100644
@@ -20,6 +20,7 @@ config SUNGEM_PHY
 source "drivers/net/ethernet/3com/Kconfig"
 source "drivers/net/ethernet/adaptec/Kconfig"
 source "drivers/net/ethernet/aeroflex/Kconfig"
+source "drivers/net/ethernet/agere/Kconfig"
 source "drivers/net/ethernet/alteon/Kconfig"
 source "drivers/net/ethernet/amd/Kconfig"
 source "drivers/net/ethernet/apple/Kconfig"

drivers/net/ethernet/Makefile +1 -0

index 8268d85..7a96adb 100644
@@ -6,6 +6,7 @@ obj-$(CONFIG_NET_VENDOR_3COM) += 3com/
 obj-$(CONFIG_NET_VENDOR_8390) += 8390/
 obj-$(CONFIG_NET_VENDOR_ADAPTEC) += adaptec/
 obj-$(CONFIG_GRETH) += aeroflex/
+obj-$(CONFIG_NET_VENDOR_AGERE) += agere/
 obj-$(CONFIG_NET_VENDOR_ALTEON) += alteon/
 obj-$(CONFIG_NET_VENDOR_AMD) += amd/
 obj-$(CONFIG_NET_VENDOR_APPLE) += apple/

drivers/net/ethernet/agere/Kconfig new file +32 -0

new file mode 100644
index 0000000..efe0b90
@@ -0,0 +1,32 @@
+#
+# Agere device configuration
+#
+
+config NET_VENDOR_AGERE
+       bool "Agere devices"
+       default y
+       depends on PCI
+       ---help---
+         If you have a network (Ethernet) card belonging to this class, say Y
+         and read the Ethernet-HOWTO, available from
+         <http://www.tldp.org/docs.html#howto>.
+
+         Note that the answer to this question doesn't directly affect the
+         kernel: saying N will just cause the configurator to skip all
+         the questions about Agere devices. If you say Y, you will be asked
+         for your specific card in the following questions.
+
+if NET_VENDOR_AGERE
+
+config ET131X
+       tristate "Agere ET-1310 Gigabit Ethernet support"
+       depends on PCI
+       select PHYLIB
+       ---help---
+         This driver supports Agere ET-1310 ethernet adapters.
+
+         To compile this driver as a module, choose M here. The module
+         will be called et131x.
+
+endif # NET_VENDOR_AGERE
+

drivers/net/ethernet/agere/Makefile renamed from drivers/staging/et131x/Makefile +1 -0

similarity index 98%
rename from drivers/staging/et131x/Makefile
rename to drivers/net/ethernet/agere/Makefile
index 027ff94..740f413 100644
@@ -3,3 +3,4 @@
 #
 
 obj-$(CONFIG_ET131X) += et131x.o
+

drivers/net/ethernet/agere/et131x.c renamed from drivers/staging/et131x/et131x.c +0 -0

similarity index 100%
rename from drivers/staging/et131x/et131x.c
rename to drivers/net/ethernet/agere/et131x.c

drivers/net/ethernet/agere/et131x.h renamed from drivers/staging/et131x/et131x.h +0 -0

similarity index 100%
rename from drivers/staging/et131x/et131x.h
rename to drivers/net/ethernet/agere/et131x.h

drivers/staging/Kconfig +0 -2

index 0b47a06..5dee726 100644
@@ -24,8 +24,6 @@ menuconfig STAGING
 
 if STAGING
 
-source "drivers/staging/et131x/Kconfig"
-
 source "drivers/staging/slicoss/Kconfig"
 
 source "drivers/staging/usbip/Kconfig"

drivers/staging/Makefile +0 -1

index b026ea3..6983d77 100644
@@ -5,7 +5,6 @@ obj-$(CONFIG_STAGING)		+= staging.o
 
 obj-y				+= media/
 obj-y				+= net/
-obj-$(CONFIG_ET131X)		+= et131x/
 obj-$(CONFIG_SLICOSS)		+= slicoss/
 obj-$(CONFIG_USBIP_CORE)	+= usbip/
 obj-$(CONFIG_W35UND)		+= winbond/

drivers/staging/et131x/Kconfig deleted +0 ~~-10~~

deleted file mode 100644
index 8190f2a..0000000
@@ -1,10 +0,0 @@
-config ET131X
-	tristate "Agere ET-1310 Gigabit Ethernet support"
-	depends on PCI && NET && NETDEVICES
-	select PHYLIB
-	default n
-	---help---
-	  This driver supports Agere ET-1310 ethernet adapters.
-
-	  To compile this driver as a module, choose M here. The module
-	  will be called et131x.

drivers/staging/et131x/README deleted +0 ~~-16~~

deleted file mode 100644
index 38537d4..0000000
@@ -1,16 +0,0 @@
-This is a driver for the ET1310 network device.
-
-Based on the driver found at https://sourceforge.net/projects/et131x/
-
-Cleaned up immensely by Olaf Hartman and Christoph Hellwig <[email protected]>
-
-Note, the powermanagement options were removed from the vendor provided
-driver as they did not build properly at the time.
-
-TODO:
-	- some rx packets have CRC/code/frame errors
-
-Please send patches to:
-	Greg Kroah-Hartman <[email protected]>
-	Mark Einon <[email protected]>
-

Signature · 1 line

1.7.10.4

Greg KH

maintainer

2013-01-23 14:30 UTC #10 lore raw

Quoted 22 lines · Mark Einon

This patch moves the et131x gigabit ethernet driver from drivers/staging to drivers/net/ethernet/agere.
All the existing issues noted for this driver have been resolved, apart from one performance issue where some fragmented packets suffer from frame receive errors.
Signed-off-by Mark Einon <[email protected]>
---
Changes from V2 - vendor name changed from Atheros to Agere in drivers/net/ethernet/agere/Kconfig
 MAINTAINERS                                        |   11 ++++---
 drivers/net/ethernet/Kconfig                       |    1 +
 drivers/net/ethernet/Makefile                      |    1 +
 drivers/net/ethernet/agere/Kconfig                 |   32 ++++++++++++++++++++
 .../et131x => net/ethernet/agere}/Makefile         |    1 +
 .../et131x => net/ethernet/agere}/et131x.c         |    0
 .../et131x => net/ethernet/agere}/et131x.h         |    0
 drivers/staging/Kconfig                            |    2 --
 drivers/staging/Makefile                           |    1 -
 drivers/staging/et131x/Kconfig                     |   10 ------

Quoted 8 lines

 drivers/staging/et131x/README                      |   16 ----------
 11 files changed, 41 insertions(+), 34 deletions(-)
 create mode 100644 drivers/net/ethernet/agere/Kconfig
 rename drivers/{staging/et131x => net/ethernet/agere}/Makefile (98%)
 rename drivers/{staging/et131x => net/ethernet/agere}/et131x.c (100%)
 rename drivers/{staging/et131x => net/ethernet/agere}/et131x.h (100%)
 delete mode 100644 drivers/staging/et131x/Kconfig
 delete mode 100644 drivers/staging/et131x/README

Creating a patch this way is the correct way to do it, however it gives no clue to the network developers as to exactly what is in this driver at the moment :(

I usually recommend just creating a new patch, that adds the driver to the proper place in the kernel, and sending that in. If the subsystem maintainers accept it, then I can just delete the staging driver.

Yes, this looses the history of the code, but usually no one cares about that for staging drivers.

If that's not ok, I have no problem accepting this, if I get an ack from the subsystem maintainers that it's ok, but again, this patch makes it pretty hard for them to know that or not.

thanks,

greg k-h

Mark Einon

author maintainer

2013-01-23 14:51 UTC #11 lore raw

Quoted 10 lines · Greg KH

I usually recommend just creating a new patch, that adds the driver to the proper place in the kernel, and sending that in. If the subsystem maintainers accept it, then I can just delete the staging driver.
Yes, this looses the history of the code, but usually no one cares about that for staging drivers.
If that's not ok, I have no problem accepting this, if I get an ack from the subsystem maintainers that it's ok, but again, this patch makes it pretty hard for them to know that or not.

I can see how that works - I'll send a v4 update shorty.

Cheers,

Mark

Mark Einon

author maintainer

2013-01-23 16:24 UTC #12 lore raw

RFC PATCH v4 1/1 et131x: Promote staging et131x driver to drivers/net

In reply to Mark Einon

This patch moves the et131x gigabit ethernet driver from drivers/staging to drivers/net/ethernet/agere.

All the existing issues noted for this driver have been resolved, apart from one performance issue where some fragmented packets suffer from frame receive errors.

Signed-off-by Mark Einon <[email protected]>

---

Changes from v3 - diff format now shows file contents

 MAINTAINERS                         |   11 +-
 drivers/net/ethernet/Kconfig        |    1 +
 drivers/net/ethernet/Makefile       |    1 +
 drivers/net/ethernet/agere/Kconfig  |   32 +
 drivers/net/ethernet/agere/Makefile |    6 +
 drivers/net/ethernet/agere/et131x.c | 4981 +++++++++++++++++++++++++++++++++++
 drivers/net/ethernet/agere/et131x.h | 1710 ++++++++++++
 7 files changed, 6737 insertions(+), 5 deletions(-)
 create mode 100644 drivers/net/ethernet/agere/Kconfig
 create mode 100644 drivers/net/ethernet/agere/Makefile
 create mode 100644 drivers/net/ethernet/agere/et131x.c
 create mode 100644 drivers/net/ethernet/agere/et131x.h

MAINTAINERS +6 -5

index e80447a..cb09e9e 100644
@@ -3029,6 +3029,12 @@ T:	git git://git.kernel.org/pub/scm/linux/kernel/git/kristoffer/linux-hpc.git
 F:	drivers/video/s1d13xxxfb.c
 F:	include/video/s1d13xxxfb.h
 
+ET131X NETWORK DRIVER
+M:	Mark Einon <[email protected]>
+L:	[email protected]
+S:	Maintained
+F:	drivers/net/ethernet/agere
+
 ETHEREXPRESS-16 NETWORK DRIVER
 M:	Philip Blundell <[email protected]>
 L:	[email protected]

@@ -7473,11 +7479,6 @@ M:	David Rowe <[email protected]>
 S:	Odd Fixes
 F:	drivers/staging/echo/
 
-STAGING - ET131X NETWORK DRIVER
-M:	Mark Einon <[email protected]>
-S:	Odd Fixes
-F:	drivers/staging/et131x/
-
 STAGING - FLARION FT1000 DRIVERS
 M:	Marek Belisko <[email protected]>
 S:	Odd Fixes

drivers/net/ethernet/Kconfig +1 -0

index ed956e0..4b4edbb 100644
@@ -20,6 +20,7 @@ config SUNGEM_PHY
 source "drivers/net/ethernet/3com/Kconfig"
 source "drivers/net/ethernet/adaptec/Kconfig"
 source "drivers/net/ethernet/aeroflex/Kconfig"
+source "drivers/net/ethernet/agere/Kconfig"
 source "drivers/net/ethernet/alteon/Kconfig"
 source "drivers/net/ethernet/amd/Kconfig"
 source "drivers/net/ethernet/apple/Kconfig"

drivers/net/ethernet/Makefile +1 -0

index 8268d85..7a96adb 100644
@@ -6,6 +6,7 @@ obj-$(CONFIG_NET_VENDOR_3COM) += 3com/
 obj-$(CONFIG_NET_VENDOR_8390) += 8390/
 obj-$(CONFIG_NET_VENDOR_ADAPTEC) += adaptec/
 obj-$(CONFIG_GRETH) += aeroflex/
+obj-$(CONFIG_NET_VENDOR_AGERE) += agere/
 obj-$(CONFIG_NET_VENDOR_ALTEON) += alteon/
 obj-$(CONFIG_NET_VENDOR_AMD) += amd/
 obj-$(CONFIG_NET_VENDOR_APPLE) += apple/

drivers/net/ethernet/agere/Kconfig new file +32 -0

new file mode 100644
index 0000000..efe0b90
@@ -0,0 +1,32 @@
+#
+# Agere device configuration
+#
+
+config NET_VENDOR_AGERE
+       bool "Agere devices"
+       default y
+       depends on PCI
+       ---help---
+         If you have a network (Ethernet) card belonging to this class, say Y
+         and read the Ethernet-HOWTO, available from
+         <http://www.tldp.org/docs.html#howto>.
+
+         Note that the answer to this question doesn't directly affect the
+         kernel: saying N will just cause the configurator to skip all
+         the questions about Agere devices. If you say Y, you will be asked
+         for your specific card in the following questions.
+
+if NET_VENDOR_AGERE
+
+config ET131X
+       tristate "Agere ET-1310 Gigabit Ethernet support"
+       depends on PCI
+       select PHYLIB
+       ---help---
+         This driver supports Agere ET-1310 ethernet adapters.
+
+         To compile this driver as a module, choose M here. The module
+         will be called et131x.
+
+endif # NET_VENDOR_AGERE
+

drivers/net/ethernet/agere/Makefile new file +6 -0

new file mode 100644
index 0000000..740f413
@@ -0,0 +1,6 @@
+#
+# Makefile for the Agere ET-131x ethernet driver
+#
+
+obj-$(CONFIG_ET131X) += et131x.o
+

drivers/net/ethernet/agere/et131x.c new file +4981 -0

2,925 lines not on this page · raw message

drivers/net/ethernet/agere/et131x.h new file +1710 -0

1,710 lines not on this page · raw message

The rest of this diff is not shown. View the raw message.

Signature · 1 line

1.7.10.4

Part of the diff is not shown here. View the raw message.

David Miller

maintainer

2013-01-29 04:11 UTC #13 lore raw

From: Mark Einon <[email protected]>
Date: Wed, 23 Jan 2013 16:24:38 +0000

drivers/net/ethernet/agere/Kconfig line 32

Quoted 2 lines

drivers/net/ethernet/agere/Kconfig new file +2 -0
+endif # NET_VENDOR_AGERE
+

Trailing empty line, delete it.

drivers/net/ethernet/agere/Makefile line 6

Quoted 7 lines

drivers/net/ethernet/agere/Makefile new file +6 -0

@@ -0,0 +1,6 @@
+#
+# Makefile for the Agere ET-131x ethernet driver
+#
+
+obj-$(CONFIG_ET131X) += et131x.o
+

Likewise, get rid of this trailing empty line.

drivers/net/ethernet/agere/et131x.c line 499

Quoted 12 lines

drivers/net/ethernet/agere/et131x.c new file +12 -0

+	/* Spinlocks */
+	spinlock_t lock;
+
+	spinlock_t tcb_send_qlock;
+	spinlock_t tcb_ready_qlock;
+	spinlock_t send_hw_lock;
+
+	spinlock_t rcv_lock;
+	spinlock_t rcv_pend_lock;
+	spinlock_t fbr_lock;
+
+	spinlock_t phy_lock;

Do you really need _8_ spinlocks in an ethernet driver? To me that's way over the top and excessive.

The most recent driver I've written, for the 10Gbit Sun NIU parts, has only one spinlock.

drivers/net/ethernet/agere/et131x.c line 2667

Quoted 1 line

drivers/net/ethernet/agere/et131x.c new file +1 -0
+	/* Determine if this is a multicast packet coming in */

What in the world are you doing in this huge code block?

Such much complexity, multicast list iteration, etc. just to get some statistics correct? This stuff is going to run on every multicast packet receive.

No other driver does crap like this, get rid of this stuff.

drivers/net/ethernet/agere/et131x.c line 2737

Quoted 2 lines

drivers/net/ethernet/agere/et131x.c new file +2 -0
+	skb->dev = adapter->netdev;
+	skb->protocol = eth_type_trans(skb, adapter->netdev);

eth_type_trans() sets skb->dev, you don't need to duplicate that.

drivers/net/ethernet/agere/et131x.c line 2739

Quoted 1 line

drivers/net/ethernet/agere/et131x.c new file +1 -0
+	netif_rx_ni(skb);

Why isn't this driver supporting NAPI? If you're going to put this much time into a driver, use the best interfaces for event processing rather than something that might have been appropriate in a new driver two decades ago.

Quoted 15 lines

drivers/net/ethernet/agere/et131x.c new file +15 -0

+static int et131x_tx(struct sk_buff *skb, struct net_device *netdev)
+{
+	int status = 0;
+	struct et131x_adapter *adapter = netdev_priv(netdev);
+
+	/* stop the queue if it's getting full */
+	if (adapter->tx_ring.used >= NUM_TCB - 1 &&
+	    !netif_queue_stopped(netdev))
+		netif_stop_queue(netdev);
+
+	/* Save the timestamp for the TX timeout watchdog */
+	netdev->trans_start = jiffies;
+
+	/* Call the device-specific data Tx routine */
+	status = et131x_send_packets(skb, netdev);

drivers/net/ethernet/agere/et131x.c line 4581

Quoted 10 lines

drivers/net/ethernet/agere/et131x.c new file +10 -0

+
+	/* Check status and manage the netif queue if necessary */
+	if (status != 0) {
+		if (status == -ENOMEM)
+			status = NETDEV_TX_BUSY;
+		else
+			status = NETDEV_TX_OK;
+	}
+	return status;
+}

Returning NETDEV_TX_BUSY is an error, no driver should ever return that status under normal circumstances. Some drivers return it when internal consistency checks fail, but that indicates a driver bug rather than a normal condition.

Memory allocation erorrs do not denote NETDEV_TX_BUSY, simply drop the packet silently with kfree_skb() and return NETDEV_TX_OK.

That's about enough for me, this driver still needs a lot of work and is far from being ready to move out of staging.

Mark Einon

author maintainer

2013-01-29 12:47 UTC #14 lore raw

Quoted 90 lines · David Miller

From: Mark Einon <[email protected]>
Date: Wed, 23 Jan 2013 16:24:38 +0000
drivers/net/ethernet/agere/Kconfig line 32
Quoted 2 lines
drivers/net/ethernet/agere/Kconfig new file +2 -0
+endif # NET_VENDOR_AGERE
+
Trailing empty line, delete it.
drivers/net/ethernet/agere/Makefile line 6
Quoted 7 lines
drivers/net/ethernet/agere/Makefile new file +6 -0
@@ -0,0 +1,6 @@
+#
+# Makefile for the Agere ET-131x ethernet driver
+#
+
+obj-$(CONFIG_ET131X) += et131x.o
+
Likewise, get rid of this trailing empty line.
drivers/net/ethernet/agere/et131x.c line 499
Quoted 12 lines
drivers/net/ethernet/agere/et131x.c new file +12 -0
+     /* Spinlocks */
+     spinlock_t lock;
+
+     spinlock_t tcb_send_qlock;
+     spinlock_t tcb_ready_qlock;
+     spinlock_t send_hw_lock;
+
+     spinlock_t rcv_lock;
+     spinlock_t rcv_pend_lock;
+     spinlock_t fbr_lock;
+
+     spinlock_t phy_lock;
Do you really need _8_ spinlocks in an ethernet driver? To me that's way over the top and excessive.
The most recent driver I've written, for the 10Gbit Sun NIU parts, has only one spinlock.
Quoted 1 line
+ /* Determine if this is a multicast packet coming in */
What in the world are you doing in this huge code block?
Such much complexity, multicast list iteration, etc. just to get some statistics correct? This stuff is going to run on every multicast packet receive.
No other driver does crap like this, get rid of this stuff.
drivers/net/ethernet/agere/et131x.c line 2737
Quoted 2 lines
drivers/net/ethernet/agere/et131x.c new file +2 -0
+     skb->dev = adapter->netdev;
+     skb->protocol = eth_type_trans(skb, adapter->netdev);
eth_type_trans() sets skb->dev, you don't need to duplicate that.
Quoted 1 line
+ netif_rx_ni(skb);
Why isn't this driver supporting NAPI? If you're going to put this much time into a driver, use the best interfaces for event processing rather than something that might have been appropriate in a new driver two decades ago.
Quoted 15 lines
drivers/net/ethernet/agere/et131x.c new file +15 -0
+static int et131x_tx(struct sk_buff *skb, struct net_device *netdev)
+{
+     int status = 0;
+     struct et131x_adapter *adapter = netdev_priv(netdev);
+
+     /* stop the queue if it's getting full */
+     if (adapter->tx_ring.used >= NUM_TCB - 1 &&
+         !netif_queue_stopped(netdev))
+             netif_stop_queue(netdev);
+
+     /* Save the timestamp for the TX timeout watchdog */
+     netdev->trans_start = jiffies;
+
+     /* Call the device-specific data Tx routine */
+     status = et131x_send_packets(skb, netdev);
drivers/net/ethernet/agere/et131x.c line 4581
Quoted 10 lines
drivers/net/ethernet/agere/et131x.c new file +10 -0
+
+     /* Check status and manage the netif queue if necessary */
+     if (status != 0) {
+             if (status == -ENOMEM)
+                     status = NETDEV_TX_BUSY;
+             else
+                     status = NETDEV_TX_OK;
+     }
+     return status;
+}
Returning NETDEV_TX_BUSY is an error, no driver should ever return that status under normal circumstances. Some drivers return it when internal consistency checks fail, but that indicates a driver bug rather than a normal condition.

Quoted 5 lines

Memory allocation erorrs do not denote NETDEV_TX_BUSY, simply drop the packet silently with kfree_skb() and return NETDEV_TX_OK.
That's about enough for me, this driver still needs a lot of work and is far from being ready to move out of staging.

Thanks for taking the time to give feedback. I'll update the TODO list.

Cheers,

Mark