From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41169) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XSmqp-0000NF-BR for qemu-devel@nongnu.org; Sat, 13 Sep 2014 08:55:00 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XSmqg-0008QY-8x for qemu-devel@nongnu.org; Sat, 13 Sep 2014 08:54:51 -0400 Received: from mail-wi0-x230.google.com ([2a00:1450:400c:c05::230]:65291) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XSmqg-0008QS-25 for qemu-devel@nongnu.org; Sat, 13 Sep 2014 08:54:42 -0400 Received: by mail-wi0-f176.google.com with SMTP id ex7so1979503wid.9 for ; Sat, 13 Sep 2014 05:54:41 -0700 (PDT) Sender: Paolo Bonzini Message-ID: <54143E8D.3010704@redhat.com> Date: Sat, 13 Sep 2014 14:54:37 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1410582855-21870-1-git-send-email-jsnow@redhat.com> <1410582855-21870-2-git-send-email-jsnow@redhat.com> In-Reply-To: <1410582855-21870-2-git-send-email-jsnow@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC 01/10] ide: add is_write() macro for semantic consistency List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: John Snow , qemu-devel@nongnu.org Cc: stefanha@redhat.com, mst@redhat.com Il 13/09/2014 06:34, John Snow ha scritto: > The prepare_buf callback takes an argument named /is_write/, > however in core.c we are checking to see if this DMA command > is /is_read/. I am adding a small macro to correct this oversight. > > Impact: Nothing, yet. > -The prepare_buf callback is only used in ahci and pci, and both > versions of this callback name the incoming argument is_write. > -Both functions ignore this hint currently, anyway. > > This is therefore a simple patch to avoid future mistakes. > > Signed-off-by: John Snow > --- > hw/ide/core.c | 2 +- > hw/ide/internal.h | 2 ++ > 2 files changed, 3 insertions(+), 1 deletion(-) > > diff --git a/hw/ide/core.c b/hw/ide/core.c > index 191f893..3d682e2 100644 > --- a/hw/ide/core.c > +++ b/hw/ide/core.c > @@ -718,7 +718,7 @@ void ide_dma_cb(void *opaque, int ret) > n = s->nsector; > s->io_buffer_index = 0; > s->io_buffer_size = n * 512; > - if (s->bus->dma->ops->prepare_buf(s->bus->dma, ide_cmd_is_read(s)) == 0) { > + if (s->bus->dma->ops->prepare_buf(s->bus->dma, ide_cmd_is_write(s)) == 0) { > /* The PRDs were too short. Reset the Active bit, but don't raise an > * interrupt. */ > s->status = READY_STAT | SEEK_STAT; > diff --git a/hw/ide/internal.h b/hw/ide/internal.h > index 5c19f79..72d0147 100644 > --- a/hw/ide/internal.h > +++ b/hw/ide/internal.h > @@ -338,6 +338,8 @@ enum ide_dma_cmd { > > #define ide_cmd_is_read(s) \ > ((s)->dma_cmd == IDE_DMA_READ) > +#define ide_cmd_is_write(s) \ > + ((s)->dma_cmd == IDE_DMA_WRITE) > > /* NOTE: IDEState represents in fact one drive */ > struct IDEState { > Actually the code is right (!). A read command corresponds to a DMA write. A write or trim will read data from memory, so it is a DMA read. See for example how rw_buf is only used from the READ CD command, but passes 1 for the second argument of prepare_buf. Paolo