Atsushi Nemoto wrote:
+static void tx4939ide_check_error_ints(ide_hwif_t *hwif, u16 stat)
+{
+ if (stat & TX4939IDE_INT_BUSERR) {
+ unsigned long base = TX4939IDE_BASE(hwif);
+ /* reset FIFO */
+ TX4939IDE_writew(TX4939IDE_readw(base, Sys_Ctl) |
+ 0x4000,
+ base, Sys_Ctl);
Are you sure bit 14 is self-clearing? The datashhet doesn't seem to
say that...
Well, I cannot remember... I thought I checked that bit cleard by
reading it, but actually the bit is write-only. Maybe clearing
explicitly would be a safe bet.
It's also write-only on TC86C001, and the original driver (as well as
mine) cleared it explicitly.
+ rc = __tx4939ide_dma_setup(drive);
+ if (rc == 0) {
+ /* Number of sectors to transfer. */
+ nframes = 0;
+ for (i = 0; i < hwif->sg_nents; i++)
+ nframes += sg_dma_len(&hwif->sg_table[i]);
+ BUG_ON(nframes % sect_size != 0);
+ nframes /= sect_size;
+ BUG_ON(nframes == 0);
+ TX4939IDE_writew(nframes, base, Sec_Cnt);
Ugh, it looks much easier in my TC86C001 driver... doesn't
hwgroup->rq->nr_sectors give you a number of 512 sectors?
Why bother with other (multiple of 512) sizes when you can always
program transfer in 512-byte sectors? Or was I wrong there?
Anyway, the TX3939 datasheet says that sector size must be a multiple of
256 words when transferring more than 1 sector.
Hmm. Good idea. I will try it.
At least it worked with a CD-ROM for me. :-)
+static int tx4939ide_dma_end(ide_drive_t *drive)
+{
+ if ((dma_stat & 7) == 0 &&
+ (ctl & (TX4939IDE_INT_XFEREND | TX4939IDE_INT_HOST)) ==
+ (TX4939IDE_INT_XFEREND | TX4939IDE_INT_HOST))
+ /* INT_IDE lost... bug? */
+ return 0;
You shouldn't fake the BMDMA interrupt. If it's not there, it's not
there. Or does this actually happen?
IIRC, Yes.
Hum, let me think... worth printing a message if this happens.
+ /*
+ * If only one of XFERINT and HOST was asserted, mask
+ * this interrupt and wait for an another one. Note
This comment somewhat contradicts the code which returns 1 if only
HOST interupt is asserted if ERR is set.
Which is not its business to test. I think you should remove that above
check -- if there's INTRQ asserted, then it's asserted. I wonder if BMIDE
interrupt bit gets set in that case (suspecting it's not)...
Indeed. I will make the comment more precise.
+ case TX4939IDE_INT_HOST | TX4939IDE_INT_XFEREND:
+ dma_stat = TX4939IDE_readb(base, DMA_stat);
+ if (!(dma_stat & 4))
+ pr_debug("%s: weired interrupt status. "
Weird.
Sure. But it can happen IIRC...
I meant the typo. :-)
#ifdef __BIG_ENDIAN
+/* custom iops (independent from SWAP_IO_SPACE) */
+static u8 mm_inb(unsigned long port)
+{
+ return (u8)readb((void __iomem *)port);
+}
+static void mm_outb(u8 value, unsigned long port)
+{
+ writeb(value, (void __iomem *)port);
+}
+static void mm_tf_load(ide_drive_t *drive, ide_task_t *task)
+{
[...]
+ if (task->tf_flags & IDE_TFLAG_OUT_DEVICE) {
+ unsigned long base = TX4939IDE_BASE(hwif);
+ mm_outb((tf->device & HIHI) | drive->select,
+ io_ports->device_addr);
I'm seeing no sense in re-defining so far...
+ /* Fix ATA100 CORE System Control Register */
+ TX4939IDE_writew(TX4939IDE_readw(base, Sys_Ctl) & 0x07f0,
+ base, Sys_Ctl);
Ah... you're doing it here (but not in LE mode?). I think to avoid
duplicating ide_tf_load() you need ot use selectproc().
Oh, my fault. LE mode also needs this fix. I still need ide_tf_load
on BE mode to support IDE_TFLAG_OUT_DATA.
Yeah, that totally useless flag...
+static void mm_insw_swap(unsigned long port, void *addr, u32 count)
+{
+ unsigned short *ptr = addr;
+ unsigned long size = count * 2;
+ port &= ~1;
+ while (count--)
+ *ptr++ = le16_to_cpu(__raw_readw((void __iomem *)port));
+ __ide_flush_dcache_range((unsigned long)addr, size);
Why is this needed BTW?
Do you mean __ide_flush_dcache_range? This is needed to avoid cache
inconsistency on PIO drive. PIO transfer only writes to cache but
upper layers expects the data is in main memory.
Hum, then I wonder why it's MIPS specific...
+static const struct ide_tp_ops tx4939ide_tp_ops = {
+ .exec_command = ide_exec_command,
+ .read_status = ide_read_status,
+ .read_altstatus = ide_read_altstatus,
+ .read_sff_dma_status = tx4939ide_read_sff_dma_status,
Hum, it should be re-defined in both LE and BE mode (but actually not
called anyway).
What do you mean? Please elaborate?
I mean that in LE mode you're using ide_read_sff_dma_status() but not
setting hwif->dma_base, so it won't work. But since it shouldn't be called in
this driver's case, this doesn't hurt.
MBR, Sergei
--
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
