[openib-general] Initial ipath review brain dump

[Roland Dreier]

Now that I got through reviewing the generic parts of the PathScale
merge and the low-level driver is on the trunk, I started looking
through the real driver.  I'm only about a third of the way through
infinipath_core.c, but here's a quick dump of what I see as needing
work so far:

  flatten source from four dirs into a single directory?

  Makefiles/Kconfig shouldn't have huge copyright notices (or any
    copyright notices at all for that matter)

  You need better Kconfig help text.

  Consistent naming convention ipath vs infinipath?

  get rid of "openib" uses -- these are just Linux drivers and I don't
    think OpenIB nomenclature is appropriate or helpful.

  You already depend on PCI_MSI in Kconfig -- no need to test in source file

  this must be a bug (since various places do ipath_sma_first++ etc):
        static volatile unsigned ipath_sma_first;	/* oldest sma packet index */
  use lock around it instead
  In general "volatile" is a pretty good marker for bugs and is almost
    never correct in a declaration.

  get rid of infinipath_stats typedef -- just use struct infinipath_stats

  no need to have a sysctl for debug level -- just use module param

  move /proc files to sysfs/debugfs as appropriate

  no need to test #ifndef HAVE_COMPAT_IOCTL -- new kernels have it.
    same for HAVE_UNLOCKED_IOCTL
  It's OK to maintain backport patches separately but the driver that goes
    upstream shouldn't have this obsolete code.

  Don't hard-code limit of 4 devices
	dev = ++chip_idx;
    is a bug if PCI probing becomes multi-threaded.
    just allocate dev structs as needed rather than having a static table

  Add required PCI cap/HT stuff to drivers/pci and linux/pci_regs.h
    instead of hiding in ipath_setup_htconfig().  It does seem an
    extension to pci_find_capability() is required to handle multiple
    capabilities with the same ID.

  make debugging code simpler (consider relayfs or just printk -- if
    you temporarily turn off messages going to the console with dmesg -n,
    and have CONFIG_PRINTK_TIME=y, I think printk does everything you want)

  put .owner = THIS_MODULE in ipath_fops instead of fooling with
    try_module_get()/module_put() -- that's racy.