Christoph Hellwig <hch@xxxxxx> wrote: > > On Wed, Apr 19, 2006 at 03:53:00PM -0700, Andrew Morton wrote: > > > > Gents, Erich thinks that the Areca driver is pretty much ready to go. > > > > We still have the weird read-past-EOF problem if MAX_XFER_SECTORS=4096, but > > this version of the driver has it set to 512, which makes the problem go > > away. I've suggested that we can set that problem aside for now. > > > > I've put a copy of the latest diff at > > http://www.zip.com.au/~akpm/linux/patches/stuff/areca-raid-linux-scsi-driver.patch. > > Could you please take a look sometime, decide how we should proceed? > > It still doesn't address lots of the month-old comments. > > Serious stuff: > - doesn't follow one value per file in sysfs rule > - arcmsr_device_probe (which should really be called arcmsr_probe!) > doesn't propagate the pci_enable_device return value > - ditto for request_irq > - please do proper goto unwinding in arcmsr_device_probe instead of > calling arcmsr_pcidev_disattach in various places > - arcmsr_report_sense_info is buggy. all scsi-commands are S/G lists > now. > - pleaase don't put that odd "linux bug" workaround into > arcmsr_queue_command. If you can reproduce that bug please report it > and we'll fix it. > - the target 16 inquiry emulation is broken for the same reasons as the > report_luns one above. Care to explain why you need it at all? > - I can't see any endianess handling in the whole driver. Was this > tested on a BE platform like powerpc? > - the probe path is doing mmio access before pci_request_regions. > - scsi_remove_host is called far too late > - dma_addr_ts are casted to unsigned longs which doesn't work > with 64bit dma and 32bit plattforms > > Cosmetical stuff: > > - odd formated comments instead of kerneldoc / per CondingStyle > - very odd variable naming, lots of uppercase and p prefixes > - please kill all these useless forward-declarations and format > the code in the natural call flow > - the consistent_dma_mask is 32bits by default, not need to call > pci_set_consistent_dma_mask > Thanks again, Christoph. I'll add the above to the changelog (along with Matthew's observations) > And there's probably a lot more issues. Below is a patch that tries to > cleanup the pci driver entry points according to the above list. Due > to all this moving things around it's pretty huge. Erich, could you please test and review this patch, and send the result back to me for integration? - : send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
