Hi,
This afternoon I spent some time trying to get a CompactFlash card to work on ARM (AT91). While looking at the code, I noticed several things in the I/O code that I think are wrong or could be improved, so I figured I'd ask here before writing a patch., as I might be overlooking some things.
To access ide devices, we have a bunch of functions:
ide_inb (overrideable) ide_outb (overrideable)
input_data_shorts input_data
output_data output_data_shorts
input_data/_shorts and output_data/_shorts seem to be just the same functions, the only change is that _data reads twice as much data as _data_shorts (huh). What about getting rid of the _data version and just do this ? #define input_data(p,d,l) input_data_shorts(p,d,l*2)
Also, input/output_data are doing their own read/writes playing with pointers, while we have ide_inb and ide_outb, which are arguably better at doing this job (especially since they can be overriden by board-specific code ...). Would it be a problem to make them use ide_inb and ide_outb ?
The last issue is that when the following condition is not met: #if defined(__PPC__) || defined(CONFIG_PXA_PCMCIA) || defined(CONFIG_SH)
then input_data resp. output_data are defined as follow:
static void input_data(int dev, ulong *sect_buf, int words) { insw(ATA_CURR_BASE(dev)+ATA_DATA_REG, sect_buf, words << 1); }
Now the issue with this is that some more code might be needed for that to work. For example on AT91, it seems that it is required to switch back and forth between 16-bit and 8-bit data bus width for CompactFlash cards to work correctly. This is not possible with the current code, as insw/outws ought to be more generic than that. Would it be possible to do just the same as with ide_inb and ide_outb, ie using ide_outsw and ide_insw as overrideable functions, while using the arch-specific insw and outsw as fallback ?
Thanks in advance for your feedback,