From owner-freebsd-stable@FreeBSD.ORG Tue Oct 21 12:44:28 2003 Return-Path: Delivered-To: freebsd-stable@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 7259C16A4B3 for ; Tue, 21 Oct 2003 12:44:28 -0700 (PDT) Received: from mail.speakeasy.net (mail7.speakeasy.net [216.254.0.207]) by mx1.FreeBSD.org (Postfix) with ESMTP id DE0C243FCB for ; Tue, 21 Oct 2003 12:44:24 -0700 (PDT) (envelope-from jhb@FreeBSD.org) Received: (qmail 14508 invoked from network); 21 Oct 2003 19:44:24 -0000 Received: from unknown (HELO server.baldwin.cx) ([216.27.160.63]) (envelope-sender )encrypted SMTP for ; 21 Oct 2003 19:44:24 -0000 Received: from laptop.baldwin.cx (gw1.twc.weather.com [216.133.140.1]) by server.baldwin.cx (8.12.9/8.12.9) with ESMTP id h9LJiKce088250; Tue, 21 Oct 2003 15:44:20 -0400 (EDT) (envelope-from jhb@FreeBSD.org) Message-ID: X-Mailer: XFMail 1.5.4 on FreeBSD X-Priority: 3 (Normal) Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 8bit MIME-Version: 1.0 In-Reply-To: <200310210738.h9L7ckYQ000811@ice.nodomain> Date: Tue, 21 Oct 2003 15:44:33 -0400 (EDT) From: John Baldwin To: Dan Strick X-Spam-Checker-Version: SpamAssassin 2.55 (1.174.2.19-2003-05-19-exp) cc: dan@ice.nodomain cc: freebsd-stable@freebsd.org cc: jhb@freefall.freebsd.org cc: sos@FreeBSD.org Subject: RE: FreeBSD 4.9-RC3 and the ICH5 ATA controllers X-BeenThere: freebsd-stable@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Production branch of FreeBSD source code List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 21 Oct 2003 19:44:28 -0000 On 21-Oct-2003 Dan Strick wrote: > On Sat, 18 Oct 2003, Murray Stokely wrote in freebsd-stable: >> ... >> release candidate (RC3) is now available for i386 >> ... >> The firewire module loading issue has been resolved, but the >> SATA issue has not. John Baldwin has a patch posted at >> http://people.freebsd.org/~jhb/patches/sata.patch >> ... > > I fetched the SATA patch and examined it. It seems to be identical > to the changes I suggested in my Oct 13 post to freebsd-stable (modulo > the order of cases in switch statements) with one minor exception. > > I was wondering if the patch is based on my suggested changes. > (I have received no feedback on my email to freebsd-stable.) > If so, did someone test them or did you just "take my word" for > the correctness of the changes? I did use your suggested change to the interrupt handler. You should have been cc'd on my reply to re@ that said as much. The one exception I noticed is that I didn't and dmastat with 0x7f but or'd it with INTERRUPT like other code in that function does. > Is the patch likely to make it into release 4.9? I have discovered > that on my system with the SATA controller configured for "legacy" > mode and *without* the patch, RC3 fails to configure my CD drive for > DMA even though I set hw.ata.atapi_dma=1 in /boot/loader.conf.local. > > The problem is that when the ICH5 SATA controller is configured in > legacy mode, the ICH5 IDE controller disappears from PCI configuration > space. Then the IDE devices appear to be connected to the SATA controller, > which is an unknown controller if the patch is not installed. Thus some > sort of patch is needed even if the SATA controller is configured in > legacy mode. Yes. > I was also wondering about the minor difference between the patch and > my suggested changes. In function ata_pci_match() in ata-pci.c, the > patch does: > ATA_OUTB(ch->r_bmio, ATA_BMSTAT_PORT, dmastat | ATA_BMSTAT_INTERRUPT); > where my code does: > ATA_OUTB(ch->r_bmio, ATA_BMSTAT_PORT, dmastat & 0x7c); > Setting the ATA_BMSTAT_INTERRUPT bit in dmastat is non functional since > we know from context that the bit is already set. True. It's what other code in that function does though, so I figured Søren had a reason for doing it that way. > The ICH5 SATA Bus Master Status Register, offset ATA_BMSTAT_PORT in the > bus master i/o control block, is described in section 11.2.2 of the Intel > ICH5 datasheet. Bits 1, 2, 7 are R/WC. In order to clear just the > ATA_BMSTAT_INTERRUPT (bit 2) and leave the other bits alone we should > clear the other R/WC bits in the status register value before writing > it back into the register (i.e. write back the value (dmastat & 0x7d)). > Bit 0 happens to be RO for the ICH5 ATA controllers, so (dmastat & 0x7c) > does the same thing, but on reconsidering the code I favor writing the > value (dmastat & 0x7d). Using cpp symbols, that would be: > (dmastat & ~(ATA_BMSTAT_DMA_SIMPLEX|ATA_BMSTAT_ERROR)) > > Just to add to the confusion, different PCI ATA controllers treat bit 7 > differently. The ICH5 SATA controller uses it to say something obscure > about the current/previous DMA operation. The bit is reserved for the > ICH5 IDE/PATA controller. The so far unpublished INCITS T13 ATA/ATAPI > Host Adapters Standard says the bit is normally RO and means two ATA > channels cannot be simultaneously active (hence the cpp macro name > ATA_BMSTAT_DMA_SIMPLEX) but some ATA vendors use it differently. I read > somewhere that PCI-SIG also has a document that expresses an opinion on > this bit, but I don't have access to PCI-SIG standards so I can't check > that out. In any case, the above code would only be executed for the > ICH5 ATA controllers, so only the ICH5 usage really matters here. > > The upshot is that the current version of the patch might clear the > ATA_BMSTAT_DMA_SIMPLEX and ATA_BMSTAT_ERROR error bits inappropriately. > The ATA_BMSTAT_DMA_SIMPLEX bit (which actually means something different > for the ICH5) does not seem to be used by the ATA driver in a way that > would make a difference. The driver might be using the ATA_BMSTAT_ERROR > bit but I don't know if the patch interferes with this usage. It may be > a waste to spend too much effort beating this to death since there may > be a whole new ATA driver in FreeBSD CURRENT. > Have I thoroughly confused you? Sorry about that... Whatever Søren says to use for the OUTB value is fine. I'd like his input though. -- John Baldwin <>< http://www.FreeBSD.org/~jhb/ "Power Users Use the Power to Serve!" - http://www.FreeBSD.org/