Sorry this has taken a while - I've been away and also dealing with various bits of graphics security stuff. I've now been through the errata, the timing data and the driver code in somewhat more detail Overall: The hardware deviates a bit from AHCI. The AHCI driver could be taught to support it but even with the longer queue supported it's not clear this is the right path, and some of the error handling needs deviate a bit. The performance numbers are pretty definitive, and the data shows that is mostly higher up in the queue handling. That's awkward in some ways because it means there isn't an obvious way to fix it, and we still want the queue stuff for 'normal' disks. Looking at other vendors there don't seem to be a pile of them also planning to do AHCI with extras instead most seem focussed on NVHMCI so it doesn't look like a pile of near identikit drivers will appear. Also if they do we would probably want them all to be related to this driver not to the general AHCI driver. So having gone over it all I think the case is rather well made for this being added as its own driver matching their specific PCI idents, but with some code clean up, and possibly some further compatibility code if it turns out some general ide/scsi tools don't work on it as expected. Comments on the driver code Questions: Should there be security checks on the ioctl interfaces ? Code: Use k[mz]alloc/kfree for small objects like structs, vmalloc has a lot of ovherad you don't need - Lots of global function names with general naming. This causes problems in Linux because all the compiled in drivers share a common namespace. So they really ought to be something like mtip_ahci_write() and so on - Semaphores. Unless you need the counting properties please use mutexes. Sempahores really make for problems in hard real time environments if using the -rt kernel additions Style: - Confuses our kernel-doc tools as it has its own different comment extraction format. That wants pulling into line (it looks like all the info is there and its a 'perl script from hell' sort of conversion) - Various struct names in capitals - please search/replace those as for style we keep capitals for defines - Various ifdefs and a lot of printk stuff. Some of this is clearly because its a development driver, but it ought to be tidied for a final submission. Also use of dev_info/dev_err etc is strongly preferred as it means a user and tools can clearly identify which device generated the message (dev_dbg() supports runtime debug switching so may also deal with stuff you'd otherwise remove later) - for ata_swap_string look at bswap() Alan -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
