resubmit as attachments: add bnx2x to Kconfig and Makefile

Add bnx2x to Kconfig and Makefile

From 94bf385550356534e6dd41ce61879608a63ed972 Mon Sep 17 00:00:00 2001
From: Eliezer Tamir <[email protected]>
Date: Mon, 8 Oct 2007 11:48:10 +0200
Subject: [PATCH 1/8] add bnx2x to Kconfig and Makefile

---
 drivers/net/Kconfig  |    9 +++++++++
 drivers/net/Makefile |    1 +
 2 files changed, 10 insertions(+), 0 deletions(-)

index 9c635a2..1d44a5e 100644
@@ -2630,6 +2630,15 @@ config TEHUTI
 	help
 	  Tehuti Networks 10G Ethernet NIC
 
+config BNX2X
+	tristate "Broadcom NetXtremeII XGb support"
+	depends on PCI
+	help
+	  This driver supports Broadcom NetXtremeII 10 gigabit Ethernet cards.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called bnx2x.  This is recommended.
+
 endif # NETDEV_10000
 
 source "drivers/net/tokenring/Kconfig"

index d2e0f35..2f11a2c 100644
@@ -63,6 +63,7 @@ obj-$(CONFIG_STNIC) += stnic.o 8390.o
 obj-$(CONFIG_FEALNX) += fealnx.o
 obj-$(CONFIG_TIGON3) += tg3.o
 obj-$(CONFIG_BNX2) += bnx2.o
+obj-$(CONFIG_BNX2X) += bnx2x.o
 spidernet-y += spider_net.o spider_net_ethtool.o
 obj-$(CONFIG_SPIDER_NET) += spidernet.o sungem_phy.o
 obj-$(CONFIG_GELIC_NET) += ps3_gelic.o

1.4.2

From: "Eliezer Tamir" <[email protected]>
Date: Mon, 08 Oct 2007 18:15:17 +0200

In a patch submission, the tree should build on every step along the way in applying your patches, with any given configuration.

Here, the user (or something automated like "make allmodconfig") can select the new config option, but because the driver source hasn't been added, the compile will fail.

The idea is that if your patch set really is composed of logically seperate changes, you submit them one logical compilable piece at a time, and if the first few patches are ok, they could go right in whilst we work out issues in later patches. But that's not how a new driver is, it is logically one change only.

You go on next to add a foo.c file, and then a foo.h file. This makes things even more difficult to review.

Really, for a new driver that doesn't make any generic kernel code changes, just submit the whole thing in one shot. It's the only reasonable way.

Thank you.

Due to the size of the patch I can not post it to the list.

Here is an FTP link. ftp://[email protected]/bnx2x-0.40.10-net-2.6.24-one.patch.txt

Or if you prefer, I can post it gzipped.

Thanks
Eliezer

From: "Eliezer Tamir" <[email protected]>
Date: Tue, 09 Oct 2007 06:13:23 +0200

Understood

I took a look at what takes up so much space and it's the firmware and this register init stuff.

The firmware is unavoidable, but wrt. the register init tables there appears to be a ton of superfluous information in that header file.

It seem extreme overkill and I would guess it exists purely to simplify your in-house chip validation. I would really wish you wouldn't do this as these tables take up unswappable kernel memory.

No other driver goes about initializing the chip registers using tables like this. Many of them are sequences of nothing but zeros or the same value repeated over and over! The non-zero values that are specific are "magic constants" with no macros to describe up the meaning of the bits being set in these registers.

Huge functions like bnx2x_idle_chk() that just validate register values with hundreds of lines of C code are not appropriate for submission into a Linux kernel network driver. Again, this looks like code that assists you with in-house chip validation.

I'm open to putting self tests into the driver, but this adds so much bloat it really takes things beyond reasonable.

Please remove this self-test code and compress the register initialization information, preferrably into foo_hw_init() C functions as is done traditionally in drivers. It will take up less space, remove the magic values, and make the hardware easier to understand for other developers.

This should get the driver under the posting limit of 400K, which to be honest no driver's should be larger than. Unfortunately that firmware file is 350K so you won't have much left to work with :-)

What we could do is post the driver without the firmware, so that people can review the actual C-code by just replying to this posting and it won't go over the posting size limit.

Thanks!

First, I must admit that looking over the file again I noticed an error on my part in generating the file, all the data that initializes FPGA or EMULATION should not be there. fixed.
(This reduced about half of the lines but only a third in size.)

Almost all of the zero-filled tables will be removed. The rest of the registers do need to be initialized.

I agree that the number of registers that needs to be initialized is huge, but that is caused by the way the hardware was designed.

The values for the initialization come from several sources:
Some are derived from HW code (the XML files used to derive the verilog code),
Others (along with much of the machine generated .h files) are generated at microcode build time, adding a microcode routine will cause the init values to change, using a new variable can cause an .h file to change. In the last group which is very small, are registers that are controlled by the driver.

The values in this file really are machine generated, they really are not meant to be modified directly by editing the file.

The registers that are under the driver's control are in the main .c and .h files.

The idle check code is not a manufacturing test, it is meant to help debug the driver and microcode.
If the driver sends an invalid command to one of the CPUs which then chokes on it, this will tell you which one of them died and the general whereabouts of the problem. (ingress CPU X is stuck because output queue Y is full)

I agree that it is not a clean implementation.
I will remove it and it will be re-written.

( Michael has showed me the trick of how to post with evolution, so I hope that the mangled patch problem is behind us and I think that I can now post everything without a problem, Hallelujah!)

From: "Eliezer Tamir" <[email protected]>
Date: Tue, 09 Oct 2007 18:20:01 +0200

...

...

Thanks for the explanations, I look forward to your next submission.