Re: [PATCH] New QStor SATA/RAID Driver for 2.6.9-rc2

[Alan Cox]

On Maw, 2004-09-14 at 16:42, Mark Lord wrote:
> Please email me any errors or corrections you may deem necessary
> for kernel inclusion.

Nothing says its GPL

qstor_printk uses a fixed length buffer yet we have vprintk. It also
appears to be full of magic maybe formatting stuff to optionally insert
things like DRIVER_NAME. Is there a reason for not just using printk ?

qstore_test_logbuf returns the flags value across functions. Not all
platforms can support this (see Rusty's unreliable guide)

What is qstor_alloc about ?

What happens if qstor_scsi_proc_write is passed a length of 
0xFFFFFFFF. You seem to have no upper bound nor any trap on the 
alloc overflow. Seems a horrible way to expose the raid commands too ?

qstor_exec_special_cmd doesn't consider datalen overflow. I'm not sure
what stops a lot of parallel callers here but not sure if thats fixed by
the command queueing ?

Correct ioctl return is -ENOTTY for unknown (thats a mistake still in
many existing drivers so no suprise its still being copied)

The ioremap_nocache should just be ioremap I believe

Announces for the driver should be KERN_INFO IMHO

Reset code seems to be spending a lot of time with irq's off ?

qstor_dealloc_device checks dev->id != NULL - kfree(NULL) is an allowed
no-op btw.


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/