From: Christoph Hellwig <hch@infradead.org>
To: Andrew Vasquez <andrew.vasquez@qlogic.com>
Cc: Linux-Kernel <linux-kernel@vger.kernel.org>,
Linux-SCSI <linux-scsi@vger.kernel.org>
Subject: Re: [ANNOUNCE] QLogic qla2xxx driver update available (v8.00.00b4).
Date: Fri, 18 Jul 2003 12:23:04 +0100 [thread overview]
Message-ID: <20030718122304.A23013@infradead.org> (raw)
In-Reply-To: <B179AE41C1147041AA1121F44614F0B0598B10@AVEXCH02.qlogic.org>; from andrew.vasquez@qlogic.com on Thu, Jul 17, 2003 at 04:40:00PM -0700
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.
- what is CONFIG_MD_MULTIHOST_FC?
- 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
next prev parent reply other threads:[~2003-07-18 11:08 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2003-07-17 23:40 [ANNOUNCE] QLogic qla2xxx driver update available (v8.00.00b4) Andrew Vasquez
2003-07-18 10:48 ` Christoph Hellwig
2003-07-18 11:23 ` Christoph Hellwig [this message]
2003-07-18 12:12 ` Lars Marowsky-Bree
2003-07-18 12:13 ` Christoph Hellwig
2003-07-18 12:26 ` Lars Marowsky-Bree
2003-07-18 12:26 ` Lars Marowsky-Bree
2003-07-18 12:34 ` Christoph Hellwig
2003-07-18 12:41 ` Lars Marowsky-Bree
2003-07-18 12:41 ` Lars Marowsky-Bree
2003-07-18 12:46 ` Christoph Hellwig
2003-07-18 13:46 ` Arjan van de Ven
2003-07-18 14:03 ` [ANNOUNCE] QLogic qla2xxx driver update available (v8.00.00b4 ) Jamie Wellnitz
2003-07-18 19:12 ` [ANNOUNCE] QLogic qla2xxx driver update available (v8.00.00b4) Christoph Hellwig
2003-07-18 19:12 ` Christoph Hellwig
2003-07-18 19:10 ` Christoph Hellwig
2003-07-18 21:15 Andrew Vasquez
2003-07-18 21:46 Andrew Vasquez
2003-07-18 21:53 Andrew Vasquez
2003-07-19 1:05 Manfred Spraul
2005-01-14 20:24 Eric Pesciotta
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20030718122304.A23013@infradead.org \
--to=hch@infradead.org \
--cc=andrew.vasquez@qlogic.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-scsi@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.