Re: [PATCH] QStor SATA/RAID driver for 2.6.9-rc3

[Christoph Hellwig <hch@infradead.org>]

> So, skipping the EXPORTs for now, how do you guys
> feel about the driver ?

 - please kill the #ifndef SERVICE_ACTION_IN section
 - please use <scsi/scsi.h> sense data ifdefs instead of adding your own
 - please kill your scsi_to_pci_dma_dirusage, it's been obsoleted for a reason
 - please remove DRPINTK in favour of pr_debug from <linux/kernel.h>
 - please removæ the global host list, not just the exports for it
 - dito for qs_notify
 - code like
 +       if (dev->raid) {
 +               memset(dev->raid, 0, sizeof(struct qs_raid));
 +               kfree(dev->raid);
 +       }
   is totally bogus, you defeat the slab poisoning with that
 - qs_alloc duplicates kcalloc
 - the !dev case in qs_scsi_queuecomman can't happen
 - no need for the case in sc->host_scribble = (void *)(dev->uhba);
 - never mess with eh_timeout from inside a driver
 - please don't implemente the HDIO_ ioctls, Jeff said this can
   be done via SG_IO
 - if ->info return a static string you can just store it into ->name
 - please don't discard the exact error returned from
   pci_enable_device(pdev)
 - never call scsi_set_device() in a new driver, scsi_add_host does that for
   you
 - never use scsi_assign_lock in a new driver, the midlayer already has
   a per-host lock for you
 - please use the kernel/kthread.c interface for your kernel thread
 - never cast your ioregs before iounmap.
 - in fact it looks like you want to run sparse over the driver and
   fix up everything it found - at least __iomem annotations are missing
 - please don't use set_fs(KERNEL_DS), split up the underlying code for
   user and kernel buffers

This was just a quick 5 minute review, I'll give it a deeper look once I get a
little bit more time.