Re: [PATCH 2/2] libata: New driver for OCTEON SOC Compact Flash interface (v2).

[David Daney <ddaney@caviumnetworks.com>]

Jeff Garzik wrote:
> David Daney wrote:
>> +
>> +/* Timing multiple used for configuring the boot bus DMA engine */
>> +#define CF_DMA_TIMING_MULT    4
>> +
>> +static struct scsi_host_template octeon_cf_sht = {
>> +    ATA_PIO_SHT(DRV_NAME),
>> +};
>> +
>> +static int mwdmamodes = 0x1f;    /* Support Multiword DMA 0-4 */
>> +module_param(mwdmamodes, int, 0444);
>> +MODULE_PARM_DESC(mwdmamodes,
>> +         "Bitmask controlling which MWDMA modes are supported.  "
>> +         "Default is 0x1f for MWDMA 0-4.");
> 
> Two comments:
> 
> * perhaps I missed this, but why is this module param necessary?  In 
> general we avoid things like this.
> 

The CF interface is on the SOC's Boot Bus.  Under some circumstances a 
  board wide hardware reset can occur while DMA is in progress, this 
results in the CF driving the Boot Bus while the processor is trying to 
read the boot code from the boot ROM which results in an unbootable 
system.  The only recourse is to cycle power to the board.  By allowing 
DMA to be disabled, we eliminate this problem.


> * I would avoid pretending to be an SFF DMA engine, and just code it in 
> the style of a custom DMA engine with SFF registers/mode, such as 
> sata_mv or sata_promise.
> 

It is certainly possible to do that, however I think it will result in a 
much larger driver.  If you strongly object, we would have to bite the 
bullet and do as you suggest.

David Daney