FreeBSD 4.9-RC3 and the ICH5 ATA controllers
John Baldwin
jhb at FreeBSD.org
Tue Oct 21 12:44:28 PDT 2003
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 <jhb at FreeBSD.org> <>< http://www.FreeBSD.org/~jhb/
"Power Users Use the Power to Serve!" - http://www.FreeBSD.org/
More information about the freebsd-stable
mailing list