All of lore.kernel.org
 help / color / mirror / Atom feed
From: Fabien Salvi <fabien@cri74.org>
To: brian.auld@adic.com
Cc: linux-scsi@vger.kernel.org
Subject: Re: Qlogic FC driver(s)
Date: Wed, 07 May 2003 18:24:43 +0200	[thread overview]
Message-ID: <3EB9334B.7020409@cri74.org> (raw)
In-Reply-To: 995FF289C9D69747A09E42992644595405B2372F@penguin.adic.com



brian.auld@adic.com a écrit:
> Hello,
> 
> Can someone in the 'know' comment on the differences/background as to why there is a Qlogic Fibre Channel driver in the kernel tree and a separate driver released by qlogic. 
> 
> Is one preferable over the other?
> 
> I'm doing some iSCSI target driver work. I have an FC target driver as a reference that was superimposed on top of the qlogic intitiator driver found in the kernel tree (as opposed to glogic). I want to make sure this base driver (kernel tree vs. qlogic) is the preferred choice before proceeding. 
> 

Hello Brian,

There are a lot of differences between the kernel one and the Qlogic 
official driver.
The first one is based on an old driver, it's not really maintained.
I can't use it when I use FC switch in F_port.

The official Qlogic driver works betten in my opinion, but it's not 
integrated because of specific coding not accepted by scsi maintainer :

Here is what Christoph Hellwig said :


First thing is of course to actually port the driver to 2.5-CURRENT, there's
no way it'll get into 2.4 before 2.5.

The it needs some massaging into an actual linux driver:

* remove typedef abuse
* remove #ifdef abuse.  The current driver is ifdef hell at it's best.
   examples:

#ifdef UNIQUE_FW_NAME
unsigned short fw2300ip_version = 3*1024+1;
#else
unsigned short risc_code_version = 3*1024+1;
#endif

   just always use the unique name, it can't harm

#if QLA2X_PERFORMANCE

   WTF?  Either your performance changes work and can always be enabled
   or you should remove them from a submitted driver.
   The useful remaining options should be turned into CONFIG_* symbols
   for use with the kernel config tools.

* the source file organization is a mess.  Don't ever include c files
   in other c files.  Reorganize the code into qla2100/qla2200/qla2300
   specific sources and common sources instead of the ifdef mess and
   turn the shared code into a library module (e.g. qla2x00.ko)

* don't delcare the host template in a header but as normal struct,
   see the many 2.5 driver that underwent that change (i.e. aic7xxx)

Second there's a bunch of functional changes that need to be done:

* Convert to the new pci API and scsi_add_host/scsi_remove_host for
   proper PCI hotplug support.
* Remove failover/multipath support.  It has been frequently stated
   multipathing for linux schould be handle in the driver-independant
   later, like the merged md-level support or IBM's scsi midlayer
   MP patches.
* Don't use deprecated functions like check_region :)
* Use proper abstraction instead of version checking hell in the
   source files.  I.e. the many occurances of

#if LINUX_VERSION_CODE < KERNEL_VERSION(2,5,0)
         spin_lock_irq(&io_request_lock);
#else
	spin_lock_irq(ha->host->host_lock);
#endif

   should be hidden behing a proper wrapper.
* scsi_block_requests/scsi_unblock_requests instead of queueing commands
   yourself in ->queuecommand if the port stated is "dead" and remove
   much of the then obsolete queueing code.


Janitoral stuff:

* there seems to be lots of missing error return checks (i.e. 
copy_from_user)
* rename kmem_zalloc or just use kmalloc directly - Linux 2.5 already has
   a kmem_zalloc (in XFS)

I guess there's some more stuff that can be found once the driver is in
a saner shape, but the list is already huge enough 8)




-- 
Fabien SALVI      Centre de Ressources Informatiques
                   Archamps, France -- http://www.cri74.org
                   PingOO GNU/linux distribution : http://www.pingoo.org

-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

  reply	other threads:[~2003-05-07 16:12 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2003-05-05 17:56 Qlogic FC driver(s) brian.auld
2003-05-07 16:24 ` Fabien Salvi [this message]
  -- strict thread matches above, loose matches on Subject: below --
2003-05-07 20:47 Duane Grigsby

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=3EB9334B.7020409@cri74.org \
    --to=fabien@cri74.org \
    --cc=brian.auld@adic.com \
    --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.