On Wed, Dec 8, 2010 at 12:13 PM, Alexander Graf <ag...@suse.de> wrote: > @@ -486,8 +440,8 @@ void ide_dma_error(IDEState *s) > ide_transfer_stop(s); > s->error = ABRT_ERR; > s->status = READY_STAT | ERR_STAT; > - ide_dma_set_inactive(s->bus->bmdma); > - s->bus->bmdma->status |= BM_STATUS_INT; > + ide_set_inactive(s); > + s->bus->dma.ops->set_status(s->bus->dma.opaque, BM_STATUS_INT);
Is BM_STATUS_INT constant naming appropriate for a general DMA abstraction? Perhaps DMA_STATUS_INT. > @@ -2717,6 +2586,29 @@ static void ide_init1(IDEBus *bus, int unit) > ide_sector_write_timer_cb, s); > } > > +static int ide_nop_start_irq(void *opaque) > +{ > + return 1; > +} > + > +static int ide_nop(void *opaque) > +{ > + return 0; > +} > + > +static const IDEDMAOps ide_dma_nop = { > + .start_irq = ide_nop_start_irq, > + .start_dma = (void*)ide_nop, > + .start_transfer = (void*)ide_nop, > + .prepare_buf = (void*)ide_nop, > + .rw_buf = (void*)ide_nop, > + .set_unit = (void*)ide_nop, > + .set_status = (void*)ide_nop, > + .set_inactive = (void*)ide_nop, > + .restart_cb = (void*)ide_nop, > + .reset = (void*)ide_nop, Creative use of void* :). This looks unportable. ppc and other architectures use function descriptors. There, a function pointer is not sizeof(void*) so the (void*) cast is questionable. Also, casting to a function with a different signature is unportable. You're relying on the calling convention to make this work. Instead of fleshing out these functions, how about initializing dma.ops to NULL? The program crashes should anyone try to do DMA before setting a real IDEDMAOps pointer. That's not as robust as limping along with non-working IDE, but should be straightforward to debug if it ever happens. It also requires less code. Stefan