Attempt to fix and document interactions between suspend/resume and pccardc

power x 0.

pccardc power x 0 used to disable the slot.  But a suspend/resume
would reactivate the pccard.  It no longer does that.  Now the
disabling of the slot is sticy until it is reset with power x 1 or the
card is ejected.  This seems closer to correct behavior to me.

o Process all card state changes the same using pccard_do_stat_change().
o Cleanup disabling the card so that we can preserve the state after
  the change.  Basically, don't set it to empty as often as we do.
o On suspend, the new state is "empty" and the laststate is "suspend"
o Document state machine with a diagram of states and edges.  The
  edges are labeld to tell the reader what event causes the external
  state changes.
o "machdep.pccard.pcic_resume_reset" may be obsolete now.  We always
  call the bridge driver's resume method on resume now.  Otherwise cards
  won't automatically show up.  If it needs to stay, I'll add it back.

6 files changed, 66 insertions, 50 deletions

diff --git a/sys/pccard/cardinfo.h b/sys/pccard/cardinfo.h
index b4926e7..0c48451 100644
--- a/sys/pccard/cardinfo.h
+++ b/sys/pccard/cardinfo.h
@@ -58,7 +58,35 @@
 
 /*
  *	Slot states for PIOCGSTATE
+ *
+ *	Here's a state diagram of all the possible states:
+ *
+ *			      power x 1
+ *			 -------------------
+ *			/		    \
+ *		       /       		     v
+ *    resume  	+----------+   power x 0   +----------+
+ *	------->| inactive |<--------------| filled   |
+ *     /       	+----------+         	   +----------+
+ *    /	    	  / 	\    	      	    ^  	|
+ *  nil <---------  	 \   	  insert or |  	| suspend or
+ *	     suspend	  \   	  power x 1 |  	| eject
+ *			   \   	       	    |  	v
+ *	    		    \	    	   +----------+
+ *     	       	       	     ------------->|  empty   |
+ *     	       	       	       eject	   +----------+
+ *
+ *	Note, the above diagram is for the state.  On suspend, the laststate
+ * gets set to suspend to tell pccardd what happened.  Also the nil state
+ * means that when the no state change has happened.
+ *
+ * Some might argue that inactive should be sticky forever and
+ * eject/insert shouldn't take it out of that state.  They might be
+ * right.  On the other hand, some would argue that eject resets all
+ * state.  They might be right.  They both can't be right.  The above
+ * represents a reasonable compromise between the two.
  */
+
 enum cardstate { noslot, empty, suspend, filled, inactive };
 
 /*
diff --git a/sys/pccard/pccard.c b/sys/pccard/pccard.c
index 49c492c..274918d 100644
--- a/sys/pccard/pccard.c
+++ b/sys/pccard/pccard.c
@@ -55,6 +55,7 @@
 
 SYSCTL_NODE(_machdep, OID_AUTO, pccard, CTLFLAG_RW, 0, "pccard");
 
+/* The following might now be obsolete */
 static int pcic_resume_reset = 1;
 
 SYSCTL_INT(_machdep_pccard, OID_AUTO, pcic_resume_reset, CTLFLAG_RW,
@@ -165,9 +166,13 @@ disable_slot(struct slot *slt)
 static void
 disable_slot_to(struct slot *slt)
 {
-	slt->state = empty;
+	int wasinactive;
+
 	disable_slot(slt);
-	printf("pccard: card removed, slot %d\n", slt->slotnum);
+	if (slt->state == empty)
+		printf("pccard: card removed, slot %d\n", slt->slotnum);
+	else
+		printf("pccard: card deactivated, slot %d\n", slt->slotnum);
 	pccard_remove_beep();
 	selwakeup(&slt->selp);
 }
@@ -300,12 +305,12 @@ pccard_event(struct slot *slt, enum card_event event)
 
 	switch(event) {
 	case card_removed:
-		/*
-		 *	The slot and devices are disabled, but the
-		 *	data structures are not unlinked.
-		 */
+	case card_deactivated:
 		if (slt->state == filled || slt->state == inactive) {
-			slt->state = empty;
+			if (event == card_removed)
+				slt->state = empty;
+			else
+				slt->state = inactive;
 			disable_slot_to(slt);
 		}
 		break;
@@ -616,8 +621,7 @@ crdioctl(dev_t dev, u_long cmd, caddr_t data, int fflag, struct proc *p)
 		if (!pwval) {
 			if (slt->state != filled)
 				return (EINVAL);
-			pccard_event(slt, card_removed);
-			slt->state = inactive;
+			pccard_event(slt, card_deactivated);
 		} else {
 			if (slt->state != empty && slt->state != inactive)
 				return (EINVAL);
@@ -678,10 +682,10 @@ pccard_suspend(device_t dev)
 
 	/* This code stolen from pccard_event:card_removed */
 	if (slt->state == filled) {
-		int s = splhigh();
+		int s = splhigh();		/* nop on current */
 		disable_slot(slt);
-		slt->laststate = filled;
-		slt->state = suspend;
+		slt->laststate = suspend;	/* for pccardd */
+		slt->state = empty;
 		splx(s);
 		printf("pccard: card disabled, slot %d\n", slt->slotnum);
 	}
@@ -699,16 +703,5 @@ pccard_resume(device_t dev)
 {
 	struct slot *slt = PCCARD_DEVICE2SOFTC(dev);
 
-	if (pcic_resume_reset)
-		slt->ctrl->resume(slt);
-	/* This code stolen from pccard_event:card_inserted */
-	if (slt->state == suspend) {
-		slt->laststate = suspend;
-		slt->state = empty;
-		slt->insert_seq = 1;
-		untimeout(inserted, (void *)slt, slt->insert_ch);
-		inserted((void *) slt);
-		selwakeup(&slt->selp);
-	}
-	return (0);
+	slt->ctrl->resume(slt);
 }
diff --git a/sys/pccard/pcic.c b/sys/pccard/pcic.c
index 2ee54df6..460f123 100644
--- a/sys/pccard/pcic.c
+++ b/sys/pccard/pcic.c
@@ -314,7 +314,6 @@ pcic_attach(device_t dev)
 	struct pcic_softc *sc;
 	struct slot	*slt;
 	struct pcic_slot *sp;
-	u_int8_t	stat;
 	
 	sc = (struct pcic_softc *) device_get_softc(dev);
 	callout_handle_init(&sc->timeout_ch);
@@ -350,15 +349,7 @@ pcic_attach(device_t dev)
 
 		/* Check for changes */
 		pcic_setb(sp, PCIC_POWER, PCIC_PCPWRE | PCIC_DISRST);
-		stat = sp->getb(sp, PCIC_STATUS);
-		if (bootverbose)
-			printf("stat is %x\n", stat);
-		if ((stat & PCIC_CD) != PCIC_CD) {
-			sp->slt->laststate = sp->slt->state = empty;
-		} else {
-			sp->slt->laststate = sp->slt->state = filled;
-			pccard_event(sp->slt, card_inserted);
-		}
+		pcic_do_stat_delta(sp);
 	}
 
 	return (bus_generic_attach(dev));
@@ -551,19 +542,19 @@ pcic_reset(void *chan)
 	struct pcic_slot *sp = slt->cdata;
 
 	switch (slt->insert_seq) {
-	    case 0: /* Something funny happended on the way to the pub... */
+	case 0: /* Something funny happended on the way to the pub... */
 		return;
-	    case 1: /* Assert reset */
+	case 1: /* Assert reset */
 		pcic_clrb(sp, PCIC_INT_GEN, PCIC_CARDRESET);
 		slt->insert_seq = 2;
 		timeout(pcic_reset, (void *)slt, hz/4);
 		return;
-	    case 2: /* Deassert it again */
+	case 2: /* Deassert it again */
 		pcic_setb(sp, PCIC_INT_GEN, PCIC_CARDRESET | PCIC_IOCARD);
 		slt->insert_seq = 3;
 		timeout(pcic_reset, (void *)slt, hz/4);
 		return;
-	    case 3: /* Wait if card needs more time */
+	case 3: /* Wait if card needs more time */
 		if (!sp->getb(sp, PCIC_STATUS) & PCIC_READY) {
 			timeout(pcic_reset, (void *)slt, hz/10);
 			return;
@@ -600,16 +591,14 @@ static void
 pcic_resume(struct slot *slt)
 {
 	struct pcic_slot *sp = slt->cdata;
-	u_int8_t stat;
 
 	pcic_do_mgt_irq(sp, slt->irq);
 	if (sp->controller == PCIC_PD672X) {
 		pcic_setb(sp, PCIC_MISC1, PCIC_MISC1_SPEAKER);
 		pcic_setb(sp, PCIC_MISC2, PCIC_LPDM_EN);
 	}
-	stat = sp->getb(sp, PCIC_STATUS);
-	if ((stat & PCIC_CD) != PCIC_CD)
-		sp->slt->laststate = sp->slt->state = empty;
+	if (sp->slt->state != inactive)
+		pcic_do_stat_delta(sp);
 }
 
 int
@@ -848,3 +837,13 @@ pcic_alloc_resource(device_t dev, device_t child, int type, int *rid,
 	return (bus_generic_alloc_resource(dev, child, type, rid, start, end,
 	    count, flags));
 }
+
+void
+pcic_do_stat_delta(struct pcic_slot *sp)
+{
+	if ((sp->getb(sp, PCIC_STATUS) & PCIC_CD) != PCIC_CD)
+		pccard_event(sp->slt, card_removed);
+	else
+		pccard_event(sp->slt, card_inserted);
+}
+
diff --git a/sys/pccard/pcic_isa.c b/sys/pccard/pcic_isa.c
index f90e76f..4383e25 100644
--- a/sys/pccard/pcic_isa.c
+++ b/sys/pccard/pcic_isa.c
@@ -396,13 +396,8 @@ pcicintr1(void *arg)
 				splx(s);
 				return (EIO);
 			}
-			if (chg & PCIC_CDTCH) {
-				if ((sp->getb(sp, PCIC_STATUS) & PCIC_CD) ==
-				    PCIC_CD)
-					pccard_event(sp->slt, card_inserted);
-				else
-					pccard_event(sp->slt, card_removed);
-			}
+			if (chg & PCIC_CDTCH)
+				pcic_do_stat_delta(sp);
 		}
 	}
 	splx(s);
diff --git a/sys/pccard/pcicvar.h b/sys/pccard/pcicvar.h
index bd780c2..75481b5 100644
--- a/sys/pccard/pcicvar.h
+++ b/sys/pccard/pcicvar.h
@@ -83,6 +83,7 @@ void pcic_clrb(struct pcic_slot *sp, int reg, unsigned char mask);
 int pcic_deactivate_resource(device_t dev, device_t child, int type, int rid,
     struct resource *r);
 void pcic_dealloc(device_t dev);
+void pcic_do_stat_delta(struct pcic_slot *sp);
 int pcic_get_memory_offset(device_t bus, device_t child, int rid,
     u_int32_t *offset);
 int pcic_get_res_flags(device_t bus, device_t child, int restype, int rid,
diff --git a/sys/pccard/slot.h b/sys/pccard/slot.h
index 886110e..779fdbc 100644
--- a/sys/pccard/slot.h
+++ b/sys/pccard/slot.h
@@ -121,7 +121,7 @@ struct slot {
 #define PCCARD_DEVICE2SOFTC(d)	((struct slot *) device_get_softc(d))
 #define PCCARD_DEV2SOFTC(d)	((struct slot *) (d)->si_drv1)
 
-enum card_event { card_removed, card_inserted };
+enum card_event { card_removed, card_inserted, card_deactivated };
 
 struct slot	*pccard_init_slot(device_t, struct slot_ctrl *);
 void		 pccard_event(struct slot *, enum card_event);