Alan Cox wrote:
+ * Called to enable the use of DMA based on kernel command line.
+ */
+void octeon_cf_enable_dma(void)
+{
+ use_cf_dma = 1;
+}
Why not use the standard module parameter interface ?
I will do that.
+ /*
+ * PIO modes 0-4 all allow the device to deassert IORDY to slow down
+ * the host.
+ */
See ata_timing_compute() which also knows about master/slave timing,
PIO5/6 rules and timing adjustments as well as doing bus clock
quantisations and lengthenings for you.
OK.
+ use_iordy = 1;
This depends on the device as well and gets quite complicated. We have
ata_pio_need_iordy() to do the work for you.
+ t1 = (t1 * clocks_us) / 1000 / 2;
+ if (t1)
+ t1--;
Even if you wanted to do it this way you could just use arrays and lookup
tables as many other drivers do - ie
pio = dev->pio_mode - XFER_PIO_0;
t1 = data[pio];
The timing calculations are based on the CPU clock rate, It is difficult
to encapsulate that in a table.
[...]
+ /*
+ * Odd lengths are not supported. We should always be a
+ * multiple of 512.
+ */
+ BUG_ON(buflen & 1);
If you get a request for an odd length you should write an extra word
containing the last byte and one other. See how the standard methods
handle this.
OK.
+ if (ocd->is16bit) {
Or you could have two methods and two transfer routines defined
Good idea.
+ while (words--) {
+ *(uint16_t *)buffer = ioread16(data_addr);
+ buffer += sizeof(uint16_t);
By definition tht is 2. Do you have an ioread16_rep ?
It appears to be broken. One would expect ioread16 and ioread16_rep to
do endian swapping in the same manner. On MIPS they do not. Perhaps it
would be better to fix the problem at the source.
+
+/**
+ * Get ready for a dma operation. We do nothing, as all DMA
+ * operations are taken care of in octeon_cf_bmdma_start.
+ */
+void octeon_cf_qc_prep(struct ata_queued_cmd *qc)
+{
+}
ata_noop_qc_prep
OK.
+/**
+ * Check if any queued commands have more DMAs, if so start the next
+ * transfer, else do standard handling.
+ */
+irqreturn_t octeon_cf_interrupt(int irq, void *dev_instance)
A lot of these functions could be static it seems
An oversight on my part. I will make them all static.
+static void octeon_cf_delayed_irq(unsigned long data)
+{
What stops the following occuring
ATA irq
BUSY still set
Queue tasklet
Other irq on same line
ATA busy clear
Handle command
Tasklet runs but command was sorted out
(or a reset of the ata controller in the gap)
Probably nothing. I will try to sort it out.
+ base = cs0 + ocd->base_region_bias;
+ if (!ocd->is16bit) {
ata_std_
+ ap->ioaddr.cmd_addr = base + ATA_REG_CMD;
ata_sff_std_ports ? (at least for the 8bit case)
OK.
Thanks,
David Daney
