Re: [PATCH 04/12] libata: kill ata_id_to_dma_mode()

[Tejun Heo <htejun@gmail.com>]

Alan Cox wrote:
> On Tue, 27 Nov 2007 19:43:41 +0900
> Tejun Heo <htejun@gmail.com> 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