Subject: PCMCIA changes
To: None <tech-kern@netbsd.org>
From: Charles M. Hannum <abuse@spamalicious.com>
List: tech-kern
Date: 09/03/2003 18:53:09
I've made some changes to the YENTA-compatible PCMCIA code to help
eliminate bogus controllers and sockets being probed.  I'd like some
other people to test it -- particularly on weirdass platforms like
hpcmips -- before I commit.


PCIC_VENDOR_CIRRUS_*: Collapse the 2 chips into one vendor ID.
PCIC_VENDOR_NONE: New.

pcic_ident_ok(): Check the ID revision field -- if it's 0, punt.

pcic_vendor(): Check the ID revision field -- if it's 0, or the ID register
is all-1s, assume there is no chip present.  (Previously this would return
"Unknown controller" -- which, AFAICT, *never* resulted in a working device.)
Do the Cirrus check after verifying that we got the Intel ID.

pcic_attach(): Use apriori knowledge of the Cirrus chips to determine the
number of sockets rather than trying (unsuccessfully) to probe.  Just blast
all of PCIC_INTR -- we do this in pcic_deactivate_card() anyway.


Index: ic/i82365.c
===================================================================
RCS file: /cvsroot/src/sys/dev/ic/i82365.c,v
retrieving revision 1.74
diff -u -r1.74 i82365.c
--- ic/i82365.c	2003/09/03 01:33:23	1.74
+++ ic/i82365.c	2003/09/03 18:50:58
@@ -104,6 +104,9 @@
 	if ((ident == 0) || (ident == 0xff) || (ident & PCIC_IDENT_ZERO))
 		return (0);
 
+	if ((ident & PCIC_IDENT_REV_MASK) == 0)
+		return (0);
+
 	if ((ident & PCIC_IDENT_IFTYPE_MASK) != PCIC_IDENT_IFTYPE_MEM_AND_IO) {
 #ifdef DIAGNOSTIC
 		printf("pcic: does not support memory and I/O cards, "
@@ -111,6 +114,7 @@
 #endif
 		return (0);
 	}
+
 	return (1);
 }
 
@@ -121,28 +125,15 @@
 	int reg;
 	int vendor;
 
-	/*
-	 * the chip_id of the cirrus toggles between 11 and 00 after a write.
-	 * weird.
-	 */
-
-	pcic_write(h, PCIC_CIRRUS_CHIP_INFO, 0);
-	reg = pcic_read(h, -1);
-
-	if ((reg & PCIC_CIRRUS_CHIP_INFO_CHIP_ID) ==
-	    PCIC_CIRRUS_CHIP_INFO_CHIP_ID) {
-		reg = pcic_read(h, -1);
-		if ((reg & PCIC_CIRRUS_CHIP_INFO_CHIP_ID) == 0) {
-			if (reg & PCIC_CIRRUS_CHIP_INFO_SLOTS)
-				return (PCIC_VENDOR_CIRRUS_PD672X);
-			else
-				return (PCIC_VENDOR_CIRRUS_PD6710);
-		}
-	}
-
 	reg = pcic_read(h, PCIC_IDENT);
 
+	if ((reg & PCIC_IDENT_REV_MASK) == 0)
+		return (PCIC_VENDOR_NONE);
+
 	switch (reg) {
+	case 0x00:
+	case 0xff:
+		return (PCIC_VENDOR_NONE);
 	case PCIC_IDENT_ID_INTEL0:
 		vendor = PCIC_VENDOR_I82365SLR0;
 		break;
@@ -167,22 +158,32 @@
 	if (vendor == PCIC_VENDOR_I82365SLR0 ||
 	    vendor == PCIC_VENDOR_I82365SLR1) {
 		/*
+		 * Check for Cirrus PD67xx.
+		 * the chip_id of the cirrus toggles between 11 and 00 after a
+		 * write.  weird.
+		 */
+		pcic_write(h, PCIC_CIRRUS_CHIP_INFO, 0);
+		reg = pcic_read(h, -1);
+		if ((reg & PCIC_CIRRUS_CHIP_INFO_CHIP_ID) ==
+		    PCIC_CIRRUS_CHIP_INFO_CHIP_ID) {
+			reg = pcic_read(h, -1);
+			if ((reg & PCIC_CIRRUS_CHIP_INFO_CHIP_ID) == 0)
+				return (PCIC_VENDOR_CIRRUS_PD67XX);
+		}
+
+		/*
 		 * check for Ricoh RF5C[23]96
 		 */
 		reg = pcic_read(h, PCIC_RICOH_REG_CHIP_ID);
 		switch (reg) {
 		case PCIC_RICOH_CHIP_ID_5C296:
-			vendor = PCIC_VENDOR_RICOH_5C296;
-			break;
+			return (PCIC_VENDOR_RICOH_5C296);
 		case PCIC_RICOH_CHIP_ID_5C396:
-			vendor = PCIC_VENDOR_RICOH_5C396;
-			break;
-		default:
-			break;
+			return (PCIC_VENDOR_RICOH_5C396);
 		}
 	}
 
-	return ( vendor );
+	return (vendor);
 }
 
 char *
@@ -194,10 +195,8 @@
 		return ("Intel 82365SL Revision 0");
 	case PCIC_VENDOR_I82365SLR1:
 		return ("Intel 82365SL Revision 1");
-	case PCIC_VENDOR_CIRRUS_PD6710:
-		return ("Cirrus PD6710");
-	case PCIC_VENDOR_CIRRUS_PD672X:
-		return ("Cirrus PD672X");
+	case PCIC_VENDOR_CIRRUS_PD67XX:
+		return ("Cirrus PD6710/2X");
 	case PCIC_VENDOR_I82365SL_DF:
 		return ("Intel 82365SL-DF");
 	case PCIC_VENDOR_RICOH_5C296:
@@ -217,7 +216,7 @@
 pcic_attach(sc)
 	struct pcic_softc *sc;
 {
-	int i, reg, chip, socket, intr;
+	int i, reg, chip, socket;
 	struct pcic_handle *h;
 
 	DPRINTF(("pcic ident regs:"));
@@ -239,23 +238,37 @@
 		h->ph_write = st_pcic_write;
 		h->ph_bus_t = sc->iot;
 		h->ph_bus_h = sc->ioh;
+		h->flags = 0;
 
 		/* need to read vendor -- for cirrus to report no xtra chip */
 		if (socket == 0)
 			h->vendor = (h+1)->vendor = pcic_vendor(h);
 
-		/*
-		 * During the socket probe, read the ident register twice.
-		 * I don't understand why, but sometimes the clone chips
-		 * in hpcmips boxes read all-0s the first time. -- mycroft
-		 */
-		reg = pcic_read(h, PCIC_IDENT);
-		reg = pcic_read(h, PCIC_IDENT);
-		DPRINTF(("ident reg 0x%02x\n", reg));
-		if (pcic_ident_ok(reg))
-			h->flags = PCIC_FLAG_SOCKETP;
-		else
-			h->flags = 0;
+		switch (h->vendor) {
+		case PCIC_VENDOR_NONE:
+			/* no chip */
+			continue;
+		case PCIC_VENDOR_CIRRUS_PD67XX:
+			reg = pcic_read(h, PCIC_CIRRUS_CHIP_INFO);
+			if (socket == 0 ||
+			    (reg & PCIC_CIRRUS_CHIP_INFO_SLOTS))
+				h->flags = PCIC_FLAG_SOCKETP;
+			break;
+		default: 
+			/*
+			 * During the socket probe, read the ident register
+			 * twice.  I don't understand why, but sometimes the
+			 * clone chips in hpcmips boxes read all-0s the first
+			 * time. -- mycroft
+			 */
+			reg = pcic_read(h, PCIC_IDENT);
+			DPRINTF(("socket %d ident reg 0x%02x\n", i, reg));
+			reg = pcic_read(h, PCIC_IDENT);
+			DPRINTF(("socket %d ident reg 0x%02x\n", i, reg));
+			if (pcic_ident_ok(reg))
+				h->flags = PCIC_FLAG_SOCKETP;
+			break;
+		}
 	}
 
 	for (i = 0; i < PCIC_NSLOTS; i++) {
@@ -264,13 +277,9 @@
 		if (h->flags & PCIC_FLAG_SOCKETP) {
 			SIMPLEQ_INIT(&h->events);
 
-			/* disable interrupts -- for now */
+			/* disable interrupts and leave socket in reset */
 			pcic_write(h, PCIC_CSC_INTR, 0);
-			intr = pcic_read(h, PCIC_INTR);
-			DPRINTF(("intr was 0x%02x\n", intr));
-			intr &= ~(PCIC_INTR_RI_ENABLE | PCIC_INTR_ENABLE |
-			    PCIC_INTR_IRQ_MASK);
-			pcic_write(h, PCIC_INTR, intr);
+			pcic_write(h, PCIC_INTR, 0);
 			(void) pcic_read(h, PCIC_CSC);
 		}
 	}
@@ -280,6 +289,9 @@
 		h = &sc->handle[i];
 		chip = i / 2;
 
+		if (h->vendor == PCIC_VENDOR_NONE)
+			continue;
+
 		aprint_normal("%s: controller %d (%s) has ", sc->dev.dv_xname,
 		    chip, pcic_vendor_to_string(sc->handle[i].vendor));
 
@@ -459,8 +471,7 @@
 	    h->vendor));
 
 	/* unsleep the cirrus controller */
-	if ((h->vendor == PCIC_VENDOR_CIRRUS_PD6710) ||
-	    (h->vendor == PCIC_VENDOR_CIRRUS_PD672X)) {
+	if (h->vendor == PCIC_VENDOR_CIRRUS_PD67XX) {
 		reg = pcic_read(h, PCIC_CIRRUS_MISC_CTL_2);
 		if (reg & PCIC_CIRRUS_MISC_CTL_2_SUSPEND) {
 			DPRINTF(("%s: socket %02x was suspended\n",
Index: ic/i82365var.h
===================================================================
RCS file: /cvsroot/src/sys/dev/ic/i82365var.h,v
retrieving revision 1.17
diff -u -r1.17 i82365var.h
--- ic/i82365var.h	2002/11/24 02:46:55	1.17
+++ ic/i82365var.h	2003/09/03 18:50:58
@@ -95,16 +95,16 @@
 #define	C1SA	PCIC_CHIP_OFFSET
 #define	C1SB	PCIC_CHIP_OFFSET + PCIC_SOCKET_OFFSET
 
+#define	PCIC_VENDOR_NONE		-1
 #define	PCIC_VENDOR_UNKNOWN		0
 #define	PCIC_VENDOR_I82365SLR0		1
 #define	PCIC_VENDOR_I82365SLR1		2
-#define	PCIC_VENDOR_CIRRUS_PD6710	3
-#define	PCIC_VENDOR_CIRRUS_PD672X	4
-#define PCIC_VENDOR_I82365SL_DF		5
-#define PCIC_VENDOR_IBM			6
-#define PCIC_VENDOR_IBM_KING		7
-#define PCIC_VENDOR_RICOH_5C296		8
-#define PCIC_VENDOR_RICOH_5C396		9
+#define	PCIC_VENDOR_CIRRUS_PD67XX	3
+#define PCIC_VENDOR_I82365SL_DF		4
+#define PCIC_VENDOR_IBM			5
+#define PCIC_VENDOR_IBM_KING		6
+#define PCIC_VENDOR_RICOH_5C296		7
+#define PCIC_VENDOR_RICOH_5C396		8
 
 /*
  * This is sort of arbitrary.  It merely needs to be "enough". It can be
Index: isa/i82365_isasubr.c
===================================================================
RCS file: /cvsroot/src/sys/dev/isa/i82365_isasubr.c,v
retrieving revision 1.32
diff -u -r1.32 i82365_isasubr.c
--- isa/i82365_isasubr.c	2003/09/02 22:44:09	1.32
+++ isa/i82365_isasubr.c	2003/09/03 18:50:59
@@ -300,8 +300,7 @@
 
 		/* the cirrus chips lack support for the soft interrupt */
 		if (pcic_irq_probe != 0 &&
-		    h->vendor != PCIC_VENDOR_CIRRUS_PD6710 &&
-		    h->vendor != PCIC_VENDOR_CIRRUS_PD672X)
+		    h->vendor != PCIC_VENDOR_CIRRUS_PD67XX)
 			pcic_isa_probe_interrupts(sc, h);
 
 		chipmask &= sc->intr_mask[h->chip];