From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1KBmI6-0008Mm-FN for qemu-devel@nongnu.org; Thu, 26 Jun 2008 03:53:14 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1KBmI4-0008MR-Ek for qemu-devel@nongnu.org; Thu, 26 Jun 2008 03:53:13 -0400 Received: from [199.232.76.173] (port=60839 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1KBmI4-0008MO-7z for qemu-devel@nongnu.org; Thu, 26 Jun 2008 03:53:12 -0400 Received: from mx20.gnu.org ([199.232.41.8]:9153) by monty-python.gnu.org with esmtps (TLS-1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1KBmI3-0005rf-Q8 for qemu-devel@nongnu.org; Thu, 26 Jun 2008 03:53:11 -0400 Received: from honiara.magic.fr ([195.154.193.36]) by mx20.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1KBmI1-0007EU-W6 for qemu-devel@nongnu.org; Thu, 26 Jun 2008 03:53:10 -0400 Received: from [192.168.0.2] (ppp-36.net-123.static.magiconline.fr [80.118.184.36]) by honiara.magic.fr (8.13.1/8.13.1) with ESMTP id m5Q7qxWq019228 for ; Thu, 26 Jun 2008 09:52:59 +0200 Subject: Re: [Qemu-devel] [PATCH] FDC: rework status0, status1, status2 handling From: "J. Mayer" In-Reply-To: <4862B7CB.8040407@reactos.org> References: <4862B7CB.8040407@reactos.org> Content-Type: text/plain; charset=ISO-8859-15 Date: Thu, 26 Jun 2008 09:53:01 +0200 Message-Id: <1214466781.29120.14.camel@localhost> Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org On Wed, 2008-06-25 at 23:25 +0200, Herv=E9 Poussineau wrote: > Hi, >=20 > Attached patch fixes status0, status1 and status2 handling, which were=20 > broken in read/write commands and in some subtle ways. Status values ar= e=20 > now calculated during command execution and not at the very end. > This allows removing of one hack in fdctrl_handle_sense_interrupt_stat= us(). Hi, this way of proceeding seem very strange, as actual hardware do compute the status bytes in the FIFO when entering the status phase. This is quite logical as those bytes are supposed to reflect the state of the hardware at the end of the command completion and cannot be computed before, apart from a few bits, like the DID_SEEK bit. Furthermore, doing this way, you have to introduce weird hack in the main access routine : @@ -1811,6 +1781,15 @@ > return; > } > =20 > + if (fdctrl->fifo[0] !=3D FD_CMD_SENSE_INTERRUPT_STATUS) > + { > + /* Reset status0, except for SENSE_INTERRUPT_STATUS > + * which returns it */ > + fdctrl->status0 =3D 0; > + } > + fdctrl->status1 =3D 0; > + fdctrl->status2 =3D 0; > + what is the exact bug and why isn't it possible to try to going on doing things like actual hardware do ? Another remark: moving code, like you do in this patch make very difficult to figure what the real changes are and to track the regressions if any. Could you please avoid introducing such artificial diffs ? --=20 J. Mayer Never organized