Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 21 Apr 2004 08:31:31 -0600 (MDT)
From:      "M. Warner Losh" <imp@bsdimp.com>
To:        nike_d@cytexbg.com
Cc:        freebsd-current@freebsd.org
Subject:   Re: Failed to detect hard disk on TP X31
Message-ID:  <20040421.083131.89422632.imp@bsdimp.com>
In-Reply-To: <cone.1082541195.694771.566.1001@phobos.totalterror.net>
References:  <cone.1082540578.23288.566.1001@phobos.totalterror.net> <408642A0.3030305@netlab.nec.de> <cone.1082541195.694771.566.1001@phobos.totalterror.net>

next in thread | previous in thread | raw e-mail | index | archive | help
----Next_Part(Wed_Apr_21_08:31:31_2004_853)--
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit

In message: <cone.1082541195.694771.566.1001@phobos.totalterror.net>
            Niki Denev <nike_d@cytexbg.com> writes:
: yes, but i was not getting this before, but now, after the last commits to 
: pci/ata-pci.
: maybe i'll try to find the patch and apply it to the latest sources, to see 
: what will happen.

Try to apply this patch to latest sources.

Warner

----Next_Part(Wed_Apr_21_08:31:31_2004_853)--
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline; filename="pciata.diff"

--- FreeBSD/src/sys/dev/pci/pci.c	Wed Apr 21 05:01:45 2004
+++ p4/newcard/src/sys/dev/pci/pci.c	Wed Apr 21 04:52:17 2004
@@ -825,14 +825,6 @@
 	if (base == 0)
 		return 1;
 
-	/* if this is an ATA MASTERDEV on std addresses, resources are bogus */
-	if ((pci_get_class(dev) == PCIC_STORAGE) &&
-	    (pci_get_subclass(dev) == PCIS_STORAGE_IDE) &&
-	    (pci_get_progif(dev) & PCIP_STORAGE_IDE_MASTERDEV) &&
-	    !(pci_get_progif(dev) &
-	      (PCIP_STORAGE_IDE_MODEPRIM | PCIP_STORAGE_IDE_MODESEC)))
-		return 1;
-
 	start = base;
 	end = base + (1 << ln2size) - 1;
 	count = 1 << ln2size;
@@ -846,6 +838,59 @@
 	return ((ln2range == 64) ? 2 : 1);
 }
 
+static int
+pci_is_ata_legacy(device_t dev)
+{
+	/*
+	 * ATA PCI in compatibility mode are hard wired to certain
+	 * compatibility addresses.  Such entries does not contain
+	 * valid resources as they are at fixed positions to be
+	 * compatible with old ISA requirements.
+	 */
+	if ((pci_get_class(dev) == PCIC_STORAGE) &&
+	    (pci_get_subclass(dev) == PCIS_STORAGE_IDE) &&
+	    (pci_get_progif(dev) & PCIP_STORAGE_IDE_MASTERDEV) &&
+	    !(pci_get_progif(dev) &
+	      (PCIP_STORAGE_IDE_MODEPRIM | PCIP_STORAGE_IDE_MODESEC)))
+		return 1;
+	return 0;
+}
+
+/*
+ * The ATA PCI spec specifies that in legacy mode, the device shall
+ * decode the resources listed below.  The ata driver allocates
+ * resources in this order, and many atapci devices actually have
+ * values similar to these in the actual underlying bars.  Part of the
+ * problem is that the floppy controller and ata overlap for 1 byte,
+ * which makes it difficult to properly allocate things.
+ *
+ * My reading of the pci spec is such that this appears to be the only
+ * allowed exception to the rule that devices only decode the addresses
+ * presented in their BARs.
+ */
+static void
+pci_add_ata_legacy_maps(device_t pcib, device_t bus, device_t dev, int b,
+    int s, int f, struct resource_list *rl)
+{
+	int rid;
+	int type;
+
+	type = SYS_RES_IOPORT;
+	rid = PCIR_BAR(0);
+	resource_list_add(rl, type, rid, 0x1f0, 0x1f7, 8);
+	resource_list_alloc(rl, bus, dev, type, &rid, 0x1f0, 0x1f7, 8, 0);
+	rid = PCIR_BAR(1);
+	resource_list_add(rl, type, rid, 0x3f6, 0x3f6, 1);
+	resource_list_alloc(rl, bus, dev, type, &rid, 0x3f6, 0x3f6, 1, 0);
+	rid = PCIR_BAR(2);
+	resource_list_add(rl, type, rid, 0x170, 0x177, 8);
+	resource_list_alloc(rl, bus, dev, type, &rid, 0x170, 0x177, 8, 0);
+	rid = PCIR_BAR(3);
+	resource_list_add(rl, type, rid, 0x376, 0x376, 1);
+	resource_list_alloc(rl, bus, dev, type, &rid, 0x376, 0x376, 1, 0);
+	pci_add_map(pcib, bus, dev, b, s, f, PCIR_BAR(4), rl);
+}
+
 static void
 pci_add_resources(device_t pcib, device_t bus, device_t dev)
 {
@@ -858,8 +903,13 @@
 	b = cfg->bus;
 	s = cfg->slot;
 	f = cfg->func;
-	for (i = 0; i < cfg->nummaps;)
-		i += pci_add_map(pcib, bus, dev, b, s, f, PCIR_BAR(i), rl);
+
+	if (pci_is_ata_legacy(dev))
+		pci_add_ata_legacy_maps(pcib, bus, dev, b, s, f, rl);
+	else
+		for (i = 0; i < cfg->nummaps;)
+			i += pci_add_map(pcib, bus, dev, b, s, f, PCIR_BAR(i),
+			    rl);
 
 	for (q = &pci_quirks[0]; q->devid; q++) {
 		if (q->devid == ((cfg->device << 16) | cfg->vendor)
@@ -1468,49 +1518,43 @@
 
 	/*
 	 * Weed out the bogons, and figure out how large the BAR/map
-	 * is.  Note: some devices have been found that are '0' after
-	 * a write of 0xffffffff.  We view these as 'special' and
-	 * allow drivers to allocate whatever they want with them.  So
-	 * far, these BARs have only appeared in certain south bridges
-	 * and ata controllers made by VIA, nVidia and AMD.
+	 * is.  Bars that read back 0 here are bogus and unimplemented.
+	 * Note: atapci in legacy mode are special and handled elsewhere
+	 * in the code.  If you have a atapci device in legacy mode and
+	 * it fails here, that other code is broken.
 	 */
 	res = NULL;
 	map = pci_read_config(child, *rid, 4);
 	pci_write_config(child, *rid, 0xffffffff, 4);
 	testval = pci_read_config(child, *rid, 4);
-	if (testval != 0) {
-		if (pci_maptype(testval) & PCI_MAPMEM) {
-			if (type != SYS_RES_MEMORY) {
-				device_printf(child,
-				    "failed: rid %#x is memory, requested %d\n",
-				    *rid, type);
-				goto out;
-			}
-		} else {
-			if (type != SYS_RES_IOPORT) {
-				device_printf(child,
-				    "failed: rid %#x is ioport, requested %d\n",
-				    *rid, type);
-				goto out;
-			}
+	if (testval == 0)
+		return (NULL);
+	if (pci_maptype(testval) & PCI_MAPMEM) {
+		if (type != SYS_RES_MEMORY) {
+			device_printf(child,
+			    "failed: rid %#x is memory, requested %d\n",
+			    *rid, type);
+			goto out;
 		}
-		/*
-		 * For real BARs, we need to override the size that
-		 * the driver requests, because that's what the BAR
-		 * actually uses and we would otherwise have a
-		 * situation where we might allocate the excess to
-		 * another driver, which won't work.
-		 */
-		mapsize = pci_mapsize(testval);
-		count = 1 << mapsize;
-		if (RF_ALIGNMENT(flags) < mapsize)
-	    		flags = (flags & ~RF_ALIGNMENT_MASK) | RF_ALIGNMENT_LOG2(mapsize);
-	}
-	else {
-		if (bootverbose)
+	} else {
+		if (type != SYS_RES_IOPORT) {
 			device_printf(child,
-			    "ZERO BAR: resource checks suppressed.\n");
+			    "failed: rid %#x is ioport, requested %d\n",
+			    *rid, type);
+			goto out;
+		}
 	}
+	/*
+	 * For real BARs, we need to override the size that
+	 * the driver requests, because that's what the BAR
+	 * actually uses and we would otherwise have a
+	 * situation where we might allocate the excess to
+	 * another driver, which won't work.
+	 */
+	mapsize = pci_mapsize(testval);
+	count = 1 << mapsize;
+	if (RF_ALIGNMENT(flags) < mapsize)
+		flags = (flags & ~RF_ALIGNMENT_MASK) | RF_ALIGNMENT_LOG2(mapsize);
 	
 	/*
 	 * Allocate enough resource, and then write back the
--- FreeBSD/src/sys/dev/ata/ata-pci.c	Wed Apr 21 05:01:05 2004
+++ p4/newcard/src/sys/dev/ata/ata-pci.c	Wed Apr 21 04:44:58 2004
@@ -247,6 +247,15 @@
     if (type == SYS_RES_IOPORT) {
 	switch (*rid) {
 	case ATA_IOADDR_RID:
+	    /*
+	     * ATA master devices are hard wired to the traditional ata
+	     * I/O addresses.  Some devices have these resources wired to
+	     * their BARs, while others do not, hence the need to hardwire
+	     * the values here.  This will fail on those machines that
+	     * claim to be master but are not wired to these address.
+	     * But claiming to be master defines that they are at the
+	     * traditional addresses.
+	     */
 	    if (ATA_MASTERDEV(dev)) {
 		start = (unit ? ATA_SECONDARY : ATA_PRIMARY);
 		count = ATA_IOSIZE;
@@ -259,6 +268,15 @@
 	    break;
 
 	case ATA_ALTADDR_RID:
+	    /*
+	     * For non-master devices, the ALTADDR is offset by 2
+	     * into the rid that's returned.  Since ALTADDR is also
+	     * offset by 2 for master devices (hence the need to round
+	     * down to the nearest block of 4), both cases have an offset
+	     * of 2.  This means that the initial allocation for both
+	     * of these cases, as well as the offset of 2 elsewhere in
+	     * the code should be sufficient.
+	     */
 	    if (ATA_MASTERDEV(dev)) {
 		start = (unit ? ATA_SECONDARY : ATA_PRIMARY) + ATA_ALTOFFSET;
 		count = ATA_ALTIOSIZE;
@@ -402,7 +420,7 @@
 	ch->r_io[i].offset = i;
     }
     ch->r_io[ATA_ALTSTAT].res = altio;
-    ch->r_io[ATA_ALTSTAT].offset = 0;
+    ch->r_io[ATA_ALTSTAT].offset = ATA_MASTERDEV(device_get_parent(dev)) ? 0:2;
     ch->r_io[ATA_IDX_ADDR].res = io;
 
     if (ctlr->r_res1) {

----Next_Part(Wed_Apr_21_08:31:31_2004_853)----



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20040421.083131.89422632.imp>