| Message ID | 1228832471.6435.104.camel@sperla-laptop |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
Ok I looked at this driver some more and I have many comments. The hardware library abstraction has to go. You aren't writing a native linux driver if you split things up like this. I know you want code sharing with your other supported platforms, but that's too bad. If we let every vendor do this the whole tree would be one big unmaintainable mess. Many structures have all-caps or capitalized names. Good coding style indicates that only CPP macros are to be named with capital letters. (capital letterd symbols say to the reader "I'm a CPP macro and probably have side-effects, beware") What's happening here looks ugly and is inconsistent with the rest of the linux kernel. The definition of the access to the chip registers is overly obfuscated. All of this structure stuff and offsetof() business adds complexity to the driver and makes it harder to understand. The bit twiddling is difficult to understand and makes the compiler work too hard. I would suggest to fix all of this by simply using macros which define chip register offsets, and next to those offset define macros which define the bit values within the register. Endianness is not an issue, and all read*()/write*() calls will write out to the chip in little endian regardless of cpu endianness. See other drivers such as drivers/net/tg3.[ch] or drivers/net/niu.[ch] As you'll notice even such huge drivers as those can be done cleanly in a single source file with no hardware abstraction library layer and no funny register access structures and bit twiddling, so you can strive for that as well. So, this driver still needs a lot of work. :) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig index 231eeaf..85992af 100644 --- a/drivers/net/Kconfig +++ b/drivers/net/Kconfig @@ -2579,6 +2579,8 @@ config QLGE source "drivers/net/sfc/Kconfig" +source "drivers/net/benet/Kconfig" + endif # NETDEV_10000 source "drivers/net/tokenring/Kconfig" diff --git a/drivers/net/Makefile b/drivers/net/Makefile index 017383a..0d48432 100644 --- a/drivers/net/Makefile +++ b/drivers/net/Makefile @@ -21,6 +21,7 @@ obj-$(CONFIG_GIANFAR) += gianfar_driver.o obj-$(CONFIG_TEHUTI) += tehuti.o obj-$(CONFIG_ENIC) += enic/ obj-$(CONFIG_JME) += jme.o +obj-$(CONFIG_BENET) += benet/ gianfar_driver-objs := gianfar.o \ gianfar_ethtool.o \ diff --git a/drivers/net/benet/Kconfig b/drivers/net/benet/Kconfig new file mode 100644 index 0000000..f680607 --- /dev/null +++ b/drivers/net/benet/Kconfig @@ -0,0 +1,7 @@ +config BENET + tristate "ServerEngines 10Gb NIC - BladeEngine" + depends on PCI && INET + select INET_LRO + help + This driver implements the NIC functionality for ServerEngines + 10Gb network adapter BladeEngine (EC 3210). diff --git a/drivers/net/benet/MAINTAINERS b/drivers/net/benet/MAINTAINERS new file mode 100644 index 0000000..60f3144 --- /dev/null +++ b/drivers/net/benet/MAINTAINERS @@ -0,0 +1,8 @@ +SERVER ENGINES 10Gbe NIC - BLADE-ENGINE +P: Subbu Seetharaman +M: subbus@serverengines.com +P: Sathya Perla +M: sathyap@serverengines.com +L: netdev@vger.kernel.org +W: http://www.serverengines.com +S: Supported
Signed-off-by: Sathya Perla <sathyap@serverengines.com> --- drivers/net/Kconfig | 2 ++ drivers/net/Makefile | 1 + drivers/net/benet/Kconfig | 7 +++++++ drivers/net/benet/MAINTAINERS | 8 ++++++++ 4 files changed, 18 insertions(+), 0 deletions(-) create mode 100644 drivers/net/benet/Kconfig create mode 100644 drivers/net/benet/MAINTAINERS