From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:44985) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SfAgf-0001Cp-M7 for qemu-devel@nongnu.org; Thu, 14 Jun 2012 10:06:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SfAgU-0005DK-TZ for qemu-devel@nongnu.org; Thu, 14 Jun 2012 10:06:13 -0400 Received: from mx1.redhat.com ([209.132.183.28]:47148) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SfAgU-0005DD-LR for qemu-devel@nongnu.org; Thu, 14 Jun 2012 10:06:02 -0400 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q5EE602D010044 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Thu, 14 Jun 2012 10:06:01 -0400 Message-ID: <4FD9EFC5.4030603@redhat.com> Date: Thu, 14 Jun 2012 16:05:57 +0200 From: Kevin Wolf MIME-Version: 1.0 References: In-Reply-To: Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v5 3/4] fdc: rewrite seek and DSKCHG bit handling List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Pavel Hrdina Cc: qemu-devel@nongnu.org Am 13.06.2012 15:43, schrieb Pavel Hrdina: > This bit is cleared on every successful seek to a different track (cylinder). > The seek is also called on revalidate or on read/write/format commands which > also clear the DSKCHG bit. > > Signed-off-by: Pavel Hrdina > --- > hw/fdc.c | 68 +++++++++++++++++++++++++++++++------------------------------- > 1 files changed, 34 insertions(+), 34 deletions(-) Nice cleanup! Looks good generally, but this and patch 4 need test cases as well before I can apply them. > @@ -1004,30 +998,39 @@ static int fdctrl_seek_to_next_sect(FDCtrl *fdctrl, FDrive *cur_drv) > fd_sector(cur_drv)); > /* XXX: cur_drv->sect >= cur_drv->last_sect should be an > error in fact */ > - if (cur_drv->sect >= cur_drv->last_sect || > - cur_drv->sect == fdctrl->eot) { > - cur_drv->sect = 1; > + uint8_t new_head = cur_drv->head; > + uint8_t new_track = cur_drv->track; > + uint8_t new_sect = cur_drv->sect; > + > + int ret = 1; > + > + if (new_sect >= cur_drv->last_sect || > + new_sect == fdctrl->eot) { > + new_sect = 1; > if (FD_MULTI_TRACK(fdctrl->data_state)) { > - if (cur_drv->head == 0 && > + if (new_head == 0 && > (cur_drv->flags & FDISK_DBL_SIDES) != 0) { > - cur_drv->head = 1; > + new_head = 1; > } else { > - cur_drv->head = 0; > - cur_drv->track++; > + new_head = 0; > + new_track++; > if ((cur_drv->flags & FDISK_DBL_SIDES) == 0) > - return 0; > + ret = 0; Please use the chance to add braces here. Can you also add a small header comment to the function that explains what the return value means? Initially I read 0 as error, but in fact it seems only to mean "request completed". Kevin