From owner-freebsd-hackers Tue Aug 29 04:28:53 1995 Return-Path: hackers-owner Received: (from majordom@localhost) by freefall.FreeBSD.org (8.6.11/8.6.6) id EAA24487 for hackers-outgoing; Tue, 29 Aug 1995 04:28:53 -0700 Received: from crox.net.kiae.su (crox.net.kiae.su [144.206.130.72]) by freefall.FreeBSD.org (8.6.11/8.6.6) with ESMTP id EAA24419 for ; Tue, 29 Aug 1995 04:28:41 -0700 Received: by crox.net.kiae.su id PAA00478; (8.6.9/vak/1.8a) Tue, 29 Aug 1995 15:24:00 +0400 To: bde@zeta.org.au Cc: hackers@freebsd.org References: <199508271752.DAA24683@godzilla.zeta.org.au> Message-ID: Organization: Cronyx Ltd. Date: Tue, 29 Aug 95 15:24:00 +0400 X-Mailer: BML [UNIX Beauty Mail v.1.39] From: vak@cronyx.ru Subject: Re: wd0 detect fails Sender: hackers-owner@freebsd.org Precedence: bulk > From: Bruce Evans > > Cdrom support was added. The following change looks wrong (but works > here): > > diff -c -2 -r1.81 wd.c > *** 1.81 1995/05/16 07:52:04 > --- wd.c 1995/08/19 19:40:54 > *************** > *** 1863,1868 **** > DELAY(10 * 1000); > outb(wdc + wd_ctlr, WDCTL_IDS); > ! if (wdwait(du, WDCS_READY | WDCS_SEEKCMPLT, TIMEOUT) != 0 > ! || (du->dk_error = inb(wdc + wd_error)) != 0x01) > return (1); > outb(wdc + wd_ctlr, WDCTL_4BIT); > --- 1903,1912 ---- > DELAY(10 * 1000); > outb(wdc + wd_ctlr, WDCTL_IDS); > ! if (wdwait(du, 0, TIMEOUT) != 0) > ! return (1); > ! du->dk_status = inb(wdc + wd_status); > ! du->dk_error = inb(wdc + wd_error); > ! if ((du->dk_status & ~(WDCS_READY | WDCS_SEEKCMPLT)) != 0 || > ! du->dk_error != 0x01) > return (1); > outb(wdc + wd_ctlr, WDCTL_4BIT); > > The ATA spec (revision 4c, section B.6) says that the hardware shall set > the status to 0x50 (WDCS_READY | WDCS_SEEKCMPLT) within 31 seconds of > reset. This is what the old code tested for, except for the follow bugs: > > oldbug1) The other status bits (except WDCS_BUSY) aren't required to be > zero. > oldbug2) TIMEOUT is to small. It is 10000 ms but should be 31000 ms. > > The new code only waits for the WDCS_BUSY bit to go low; then it tests > the other bits. This may be OK - the spec seems to say that the status > register is set atomically, and reading the status register again might > fix any timing problems (normally the status register shouldn't be read > twice because of side effects. Reading it twice is is probably a > harmless no-op here). The new code then tests the bits sloppily: > > newbug1) The (WDCS_READY | WDCS_SEEKCMPLT) bits aren't required to be one. > It is being fussier. than the old code about the bits that should be > zero and sloppier about the bits that should be one. I wouldn't have > expected this to matter for drives that meet the spec. I'm not sure that this code is 100% correct. Old version did not work for some CD-ROMs, current one seems to fail for some disks. :-( Unfortunately, ATA specs cannot help much here, just as ATAPI specs cannot help in CD-ROM probing. Manufacturers never read specifications. :-) > The new code also has some changes involving the b_active state. These > seemed OK. If so, what about removing the ATAPI option and enable ATAPI support by default? Serge --- Serge Vakulenko Cronyx Ltd., Moscow Unix consulting and custom programming phone: +7 (095) 939-23-23 FreeBSD support fax: +7 (095) 939-03-00 Relcom network development