From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60453) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UpGpu-0002TT-6H for qemu-devel@nongnu.org; Wed, 19 Jun 2013 07:46:07 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UpGps-0001xb-Ph for qemu-devel@nongnu.org; Wed, 19 Jun 2013 07:46:02 -0400 Received: from mail-we0-x232.google.com ([2a00:1450:400c:c03::232]:63867) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UpGps-0001xF-JU for qemu-devel@nongnu.org; Wed, 19 Jun 2013 07:46:00 -0400 Received: by mail-we0-f178.google.com with SMTP id u53so4418098wes.9 for ; Wed, 19 Jun 2013 04:45:59 -0700 (PDT) Date: Wed, 19 Jun 2013 13:45:47 +0200 From: Stefan Hajnoczi Message-ID: <20130619114547.GB31475@stefanha-thinkpad.muc.redhat.com> References: <1371543971-23241-1-git-send-email-kwolf@redhat.com> <1371543971-23241-2-git-send-email-kwolf@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1371543971-23241-2-git-send-email-kwolf@redhat.com> Subject: Re: [Qemu-devel] [PATCH 01/17] ide: Add handler to ide_cmd_table List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: qemu-devel@nongnu.org, stefanha@redhat.com On Tue, Jun 18, 2013 at 10:25:55AM +0200, Kevin Wolf wrote: > As a preparation for moving all IDE commands into their own function > like in the ATAPI code, introduce a 'handler' callback to ide_cmd_table. > > Commands using this new infrastructure get some things handled > automatically: > > * The BSY flag is set before calling the handler (in order to avoid bugs > like the one fixed in f68ec837) and reset on completion. > > * The (obsolete) DSC flag in the status register is set on completion if > the command is flagged with SET_DSC in the command table > > * An IRQ is triggered on completion. > > * The error register and the ERR flag in the status register are cleared > before calling the handler and on completion it is asserted that > either none or both of them are set. > > No commands are converted at this point. > > Signed-off-by: Kevin Wolf > --- > hw/ide/core.c | 144 +++++++++++++++++++++++++++++++++++----------------------- > 1 file changed, 86 insertions(+), 58 deletions(-) > > diff --git a/hw/ide/core.c b/hw/ide/core.c > index 9926d92..cd9de14 100644 > --- a/hw/ide/core.c > +++ b/hw/ide/core.c > @@ -1010,71 +1010,78 @@ void ide_ioport_write(void *opaque, uint32_t addr, uint32_t val) > #define HD_CFA_OK (HD_OK | CFA_OK) > #define ALL_OK (HD_OK | CD_OK | CFA_OK) > > +/* Set the Disk Seek Completed status bit during completion */ > +#define SET_DSC (1u << 8) If DriveKind changes there could be unnoticed collisions and in the meantime we have empty flags bits... Creating a single IDEHandlerFlags enum would still depend on IDEDriveKind constants so I don't see a nicer solution than what you've done. We can live with this.