Re: [ANNOUNCE] QLogic qla2xxx driver update available (v8.00.00b4).

From: Christoph Hellwig (
Date: Fri Jul 18 2003 - 06:23:04 EST

More comments:

 - Documentation/scsi/qla2xxx.txt seems to document a 2.4ish driver
 - Documentation/scsi/qla2xxx.release.txt looks superflous, can you
   merge it into the (updated) qla2xxx.txt?
 - drivers/scsi/qla2xxx/ has duplicates of the documentation

 - the (v8) comments in drivers/scsi/qla2xxx/Kconfig are superflous
 - drivers/scsi/qla2xxx/Makefile is a mess. If you want to compile
   a single source file multiple times in different ways create a
   new source file that sets the needed defines and #includes the
   source file. But in general this should be avoided in favour of
   a common library module..

 - please document which files are shared with other operating systems
   (some look like they do, don't they?)
 - the driver still has multipath support in the lowlevel driver which
   we don't want in Linux
 - UNIQUE_FW_NAME is still there. shouldn't this be enabled uncdontionally?
 - still tons of typedefs that want cleaning up
 - your include structures is very confusing. Please include the linux
   headers directly i nthe files using them (except of course for
   source files shares with other OSes)
 - qla2x00_intr_handler should use spin_lock, not spin_lock_irqsave
 - you're using down_interruptible without checking the return value
 - please switch to newstyle module/boot parameter handling (module_param)
   (oh, okay, found the TODO comment)
 - what is #if defined (CONFIG_SCSIFCHOTSWAP) || defined(CONFIG_GAMAP)?
   that's never defined in a 2.5 tree and the code looks ugly as hell.
 - .unchecked_isa_dma = 0 in the host template is superflous,
   it's 0 by default.
 - you are using a static buffer in qla2x00_info, there's a lock needed
   to protect it
 - in qla2x00_queuecommand you need to call spin_unlock_irq/spin_lock_irq
   on the hostlock, otherwise your whole ->queuecommand runs with (local)
   interrupts disabled
 - qla2x00_biosparam is superflous, if there's no ->biosparam the midlayer
   calls scsicam_bios_param directly
 - you don't need to call scsi_set_device if you pass the proper struct device
   to scsi_add_host
 - please don't use scsi_assign_lock. In 2.5 we already have a per-HBA
   lock and using a different one than the default one doesn't make sense.
 - you need to check the return value from scsi_add_host
 - please remove the "sanity check" in qla2x00_remove_device. If you
   can't trust the pci layer you have worse problems..
 - ->proc_info is deprecated, please implement shost and sdev sysfs
   attributes instead.
 - all contents of qla_ip.c is surrounded by a big if defined(FC_IP_SUPPORT),
   please compile the file conditionally instead. (also in linux we prefer
   #ifdef over #if defined)
 - where is qla2200_ip_inquiry/qla2300_ip_inquiry called?? also it makes
   more sense to pass the scsi_qla_host_t directrly to it instead of an
   adapter number that requires list walking

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to
More majordomo info at
Please read the FAQ at

This archive was generated by hypermail 2b29 : Wed Jul 23 2003 - 22:00:33 EST