Messages in this thread | | | From | Bartlomiej Zolnierkiewicz <> | Subject | Re: [PATCH 2/5] ide: ->ide_dma_clear_irq() -> ->clear_irq() | Date | Mon, 15 Sep 2008 15:29:50 -0700 |
| |
On Monday 15 September 2008 15:11:21 Sergei Shtylyov wrote: > Hello, I wrote: > > >> * Rename ->ide_dma_clear_irq method to ->clear_irq > >> and move it from ide_hwif_t to struct ide_port_ops. > >> > >> * Move ->waiting_for_dma check inside ->clear_irq method. > >> > >> * Move ->dma_base check inside ->clear_irq method. > >> > >> piix.c: > >> * Add ich_port_ops and remove init_hwif_ich() wrapper. > >> > >> There should be no functional changes caused by this patch. > >> > > > > Good. I think it's worth implementing this method in at least > > cmd64x.c which actually reads the IDE interrupt latch bits > > (independent from the DMA interrupt status) in the dma_test_irq() > > methods but never clears them, so the latches may reflect a > > non-current state of the IDE interrupt... > > I forgot that it does clear them in its dma_end() methods (which I > myself have reworked :-). > It seems however that at least for SFF-8038 compatibles, it makes > sense to leave it that way since INTRQ might be asserted while BMIDE > interrupt bit is not, so the interrupt latch would need clearing even on > DMA timeout... > > > It may also be worth considering turning this method into > > test-and-clear, so that we can get the actual IDE interrupt state on > > the chips that implement this... > > Probably might add the test_irq() method to be called on > !hwif->waiting_for_dma. Cleraing the status at once seems impractical...
Or we can test for ->waiting_for_dma inside ->test_irq.
> >> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com> > > > > Acked-by: Sergei Shtylyov <sshtylyov@ru.mvista.com> > > Not feeling sure about this patch -- ->waiting_for_dma probably > should've been left where it was...
Well, it doesn't change behavior and I think having ->clear_irq method independent from the transfer mode is a preffered approach.
Thanks, Bart
| |