Messages in this thread Patch in this message | | | Date | Mon, 22 Nov 2004 14:02:02 +0100 | From | Jens Axboe <> | Subject | Re: ide-cd problem |
| |
On Mon, Nov 22 2004, Alan Chandler wrote: > Jens Axboe writes: > > >On Mon, Nov 22 2004, Alan Chandler wrote: > >>Jens Axboe writes: > > >>>400ns is the correctl value. Your writing is a little unclear to me - > >>>did it work or not, with that change alone? > >>> > >> > >>To be clear ... > >> > >> > >>I have modified ide-cd.c with > >> > >>1) ndelay(400) at the head of cdrom_newpc_intr() > >> > >>2) Alan Cox's patch in the place he originally identified for it to go > >> > >>3) Some printk's in cdrom_newpc_intr() after the point where it reads the > >>status and IREASON and length registers and just for the purposes of > >>diagnostics. > >> > >>With only those changes it now works. > > > >You are not answering my question :-) > > > >Here's is Alans patch as I posted some mails ago. Does it work with that > >alone?? I'm curious of it is enough. It should not be necessary to incur > >extra delay in the interrupt handler, if it is invoked from a real irq. > > Sorry, I misunderstood what you meant. I presume you think that the > interrupt may be triggered immediately the command packet has been sent but > before 400ns delay had occurred. > > NO - with Alan's patch alone, this did not work. > > The delay seesm to be needed in the path between the interrupt occuring and > the IDE_STATUS_REG being read. > > I had seen an note on a web site that said that there was two delays > required in the ATA/ATAPI spec - the 400ns which Alan's patch deals with > and a shorter delay (one PIO cycle) between busy being cleared and DRQ > reaching the correct state where the technique had been to read the > ALTSTATUS register. That was why I had tried that approach but found it > not to work. > (I have subsequently downloaded a copy of the full spec and haven't been > able to find this - but then its just short of 500 pages of dense text:-)). > > Thinking about it now, I tried the ALTSTATUS delay before applying Alan's > patch, so maybe its the some of the two delays that maybe necessary. If > you think its appropriate I will try that again this evening.
I think the more correct patch is the following. It seems I was wrong in assuming that the ide_intr() path already waited 400ns for us, I think this should work for you. Can you test it?
===== drivers/ide/ide-iops.c 1.31 vs edited ===== --- 1.31/drivers/ide/ide-iops.c 2004-11-01 18:06:50 +01:00 +++ edited/drivers/ide/ide-iops.c 2004-11-22 13:59:27 +01:00 @@ -476,10 +476,8 @@ if (drive->waiting_for_dma) return hwif->ide_dma_test_irq(drive); -#if 0 /* need to guarantee 400ns since last command was issued */ - udelay(1); -#endif + ndelay(400); #ifdef CONFIG_IDEPCI_SHARE_IRQ /* -- Jens Axboe
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |