lkml.org 
[lkml]   [2004]   [Oct]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] QStor SATA/RAID driver for 2.6.9-rc3
> 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.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-03-22 14:06    [W:0.397 / U:0.156 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site