This patch takes care of Initialization and configuration steps of Neterion Inc's X3100 Series 10GbE PCIe I/O Virtualized Server Adapter. - Device Initialization. - Verification and setting of device config parameters. - Allocation of Tx FIFO and Rx Ring descriptors (DTR). - APIs to get various type of hw stats - APIs to configure RTS (Receive Traffic Steering) Signed-off-by: Sivakumar Subramani <sivakumar.subramani@neterion.com> Signed-off-by: Rastapur Santosh <santosh.rastapur@neterion.com> Signed-off-by: Ramkrishna Vepa <ram.vepa@neterion.com> --- diff -urpN patch_5/drivers/net/vxge/vxge-config.c patch_6/drivers/net/vxge/vxge-config.c --- patch_5/drivers/net/vxge/vxge-config.c 1969-12-31 16:00:00.000000000 -0800 +++ patch_6/drivers/net/vxge/vxge-config.c 2009-03-13 00:11:27.000000000 -0700 @@ -0,0 +1,7576 @@ +/****************************************************************************** + * This software may be used and distributed according to the terms of + * the GNU General Public License (GPL), incorporated herein by reference. + * Drivers based on or derived from this code fall under the GPL and must + * retain the authorship, copyright and license notice. This file is not + * a complete program and may only be used when the entire operating + * system is licensed under the GPL. + * See the file COPYING in this distribution for more information. + ******************************************************************************/ +/******************************************************************************* + * vxge-config.c: Driver for Neterion Inc's X3100 Series 10GbE PCIe I/O + * Virtualized Server Adapter. + * Copyright(c) 2002-2009 Neterion Inc. + ******************************************************************************/ +#include <linux/version.h> +#include <linux/netdevice.h> +#include <linux/etherdevice.h> +#include <linux/pci.h> +#include <linux/delay.h> + +#include "vxge-traffic.h" +#include "vxge-config.h" + +/* + * ...
Drivers in the kernel tree are not necessarily compiled as modules, so their external symbols must be globally unique. You should use a driver- specific prefix, presumably "vxge" (possibly with "__" in front of that). Why bother clearing members of *channel just before you free it? Why are you using backslashes in multi-line statements? This is only necessary in macro definitions. This is redundant with pci_find_capability(). Since you only change two bits in the command register, maybe you should You should use pci_save_state() and pci_restore_state() around resets You need to reset i to 0 or 1 after this, since you've waited 910 us not Based on the function call below I'm guessing "sapper" should be This function might have too much vertical whitespace. Use pci_read_vpd() instead of this loop. [...] Why 176? This value should have at least a comment, but preferably a name or explicit formula. Also should this variable be called "calendar"? This is as far as I got. 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. --
Having so many arguments in a function is usually a clear sign that it needs You seem to use vmalloc nearly everywhere. First that has a lot of overhead (rounds up to pages, flushes TLBs) and also will cause more That assert is pretyt pointless because you'll just get a NULL pointer reference next. Same true all over. asserts (or rather BUG_ON) only make sense when they check for something that's not nearly obvious Such magic numbers are frowned upon. Also in general you should use the pci capabilities accessor functions Especially the patch orgies are scary. Does that really use readl() et.al. Such wrappers are not encouraged in Linux code, which aims to do with Is that readl/writel clean again? In general your driver looks like it could use a pass with sparse's Again far too many arguments. Stopped reading for now, but that file needs a lot of work in general. -Andi --
Andi, Thanks for the comments. We are looking into them. How do I enable this checking? --
What Andi said, only without the typo: run "make C=1 drivers/net/vxge/" 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. --
Install sparse and then make V=1 and add __iomem annotations as needed. There will be a lot of warnings, it's likely enough to concentrate on iomem first pass. -Andi --
