Alan Cox wrote: > On Tue, 27 Nov 2007 19:43:41 +0900 > Tejun Heo <htejun@xxxxxxxxx> wrote: > >> ata_id_to_dma_mode() isn't quite generic. The function is basically >> privately implemented ata_id_xfermask() combined with hardcoded mode >> printing and configuration which are specific to ata_generic. >> >> Kill the function and open code it in generic_set_mode() using generic >> xfermode handling functions. > > That one I still think is a bad idea. This sort of internal knowledge > belongs in library routines. It is a pretty generic function for the case > where the device is using existing modes and wants to display them. The thing that bothers me is that the code assumes SWDMA and reports "DMA" and sets xfer_mask for MWDMA0. This is specific to ata_generic and with that part removed, what can be factored out is... ata_dev_printk(dev, KERN_INFO, "configured for %s\n", ata_mode_string(xfer_mask)); dev->xfer_mode = ata_xfer_mask2mode(xfer_mask); dev->xfer_shift = ata_xfer_mode2shift(dev->xfer_mode); Unfortunately, ata_generic won't be able to use such generic helper because it uses unspecified DMA and PIO modes which generic helper wouldn't support. Unless we are gonna have other drivers which would blindly trust device setting regarding PIO/DMA modes, I don't think ata_id_to_dma_mode() would be useful, and if we are going to have more such drivers, we at least need to rename ata_id_to_dma_mode() to note that the function blindly trusts device reported configuration. Thanks. -- tejun - 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
