Is the state machine guaranteed to work?

1 files changed, 82 insertions, 41 deletions

diff --git a/xhfc/xhfc_st_state.c b/xhfc/xhfc_st_state.c
index 58b24b2..537b8d9 100644
--- a/xhfc/xhfc_st_state.c
+++ b/xhfc/xhfc_st_state.c
@@ -27,8 +27,7 @@
 enum te_state { F0, F1, F2, F3, F4, F5, F6, F7, F8 };
 enum nt_state { G0, G1, G2, G3, G4 };
 
-static void expiry_T1(struct xhfc_span* s);
-static void expiry_T3(struct xhfc_span* s);
+static void activation_timer_expiry(struct xhfc_span* s)
 static void signal_f7_transition(struct xhfc_span* s);
 
 static void handle_state_change_nt(struct xhfc_span* s);
@@ -52,8 +51,7 @@ void activate_request(struct xhfc_span* s)
 	write_xhfc(s->xhfc, R_SU_SEL, s->port);
 	write_xhfc(s->xhfc, A_SU_WR_STA, a_su_wr_sta);
 
-	s->t1 = s->t3 = 0;
-
+	s->activation_timer = NOT_RUNNING;
 	/* timer activation done by state change handler (xref3) */
 }
 
@@ -65,18 +63,44 @@ void deactivate_request(struct xhfc_span* s) /* XXX not called */
 	if(DBG_ST && DBG_SPAN(s))
 		printk(KERN_DEBUG DRIVER_NAME
 				": deactivate on port %d\n",
-				s->port);
+				s->port);xhfc_interrupt
 
 	if(s->nt && (sta == G2 || sta == G3))
 		set_st_state(s, G4);
 }
 
+/* This function is meant to be ran each time an interface changes state;
+ * however, if the state change is polled instead of generating an interrupt
+ * (what is done currently during the timer interrupt; see enable_interrupts
+ * and xhfc_interrupt). In this case, it is possible that two or more state
+ * changes happen on the same interface between two interruptions.
+ * Additionally, even when using the ST state change interruptions, an
+ * interruption can still be overlooked and lead to one of these conditions.
+ *
+ * However, there is no notable consequence because the actions are either:
+ *
+ * - Starting timers:
+ *   there is a problem iff a timer that should be started is not started and
+ *   cannot expire when it should
+ *   - T3 is only started after an activation request, therefore we always now
+ *     when T3 has to start
+ *   - T1 is only started when going to state G2, which can only yield G3
+ *     automatically. This can be disallowed with V_G2_G3_EN so that the timer
+ *     can be started before allowing the transition with V_SU_SET_G2_G3.
+ * - Stopping timers:
+ *   - T1 is only stopped on G2->G3.
+ *     From G3:
+ *      - G3->G2: if the timer is already running, stop it
+ *      - on deactivate requests stop the timer
+ *   - T3 is stopped on going to state F7. I don't see how it is possible to
+ *     guarantee this.
+ * - Changing alarms in DAHDI, which will be done anyway. No point notifying
+ *   something that lasts less that 1ms.
+ */
 void handle_state_change(struct xhfc_span* s)
 {
-	struct xhfc* x;
-	
-	x = s->xhfc;
-	
+	struct xhfc* x = s->xhfc;
+
 	s->prev_state = s->state;
 
 	write_xhfc(x, R_SU_SEL, s->port);
@@ -88,7 +112,7 @@ void handle_state_change(struct xhfc_span* s)
 	if(unlikely(s->prev_state == s->state)) {
 		printk(KERN_WARNING DRIVER_NAME ": a state change has been "
 				"reported on port %d but it looks like the "
-				"state has not changed?", s->port + 1);
+				"state has not changed?\n", s->port + 1);
 		return;
 	}
 
@@ -109,19 +133,13 @@ static void handle_state_change_nt(struct xhfc_span* s)
 		case G2:
 			s->span.alarms = DAHDI_ALARM_YELLOW;
 			if(s->prev_state != G3)
-				/* When setting the state to G4 by writing
-				 * A_SU_WR_STA, is the T2 timer automatically
-				 * started? I suppose so because I don't see
-				 * any other way to start it and it is
-				 * implemented in hardware.
-				 */
-
-				s->t1++; /* xref3 */
+				/* xref3 */
+				s->activation_timer = s->xhfc->ticks + T1_MS;
 			break;
 		case G3:
 			s->span.alarms = DAHDI_ALARM_NONE;
 
-			s->t1 = NOT_RUNNING;
+			s->activation_timer = NOT_RUNNING;
 			break;
 	}
 }
@@ -129,10 +147,13 @@ static void handle_state_change_nt(struct xhfc_span* s)
 static void handle_state_change_te(struct xhfc_span* s)
 {
 	switch(GET_V_SU_STA(s->state)) {
+		case F3:
+			s->activation_timer = NOT_RUNNING;
+
+			/* Fall through */
 		case F0:
 		case F1:
 		case F2:
-		case F3:
 		case F8:
 			s->span.alarms = DAHDI_ALARM_RED;
 			break;
@@ -153,7 +174,7 @@ static void handle_state_change_te(struct xhfc_span* s)
 			 * hard) is assumed. 
 			 */
 
-			s->t3++; /* xref3 */
+			s->activation_timer = s->xhfc->ticks + T3_MS; /* xref3 */
 
 			/* Fall through */
 		case F5:
@@ -161,11 +182,11 @@ static void handle_state_change_te(struct xhfc_span* s)
 			s->span.alarms = DAHDI_ALARM_YELLOW;
 			break;
 		case F7:
-			s->t3 = NOT_RUNNING;
+			s->activation_timer = NOT_RUNNING;
 
 			if (		GET_V_SU_STA(s->prev_state) == F6 && 
 					GET_V_SU_INFO0(s->state)) {
-				s->f6_f7_transition++;
+				s->f6_f7_transition = s->xhfc->ticks + T1_MS;
 				break;
 			}
 
@@ -177,9 +198,11 @@ static void handle_state_change_te(struct xhfc_span* s)
 
 void handle_st_timers(struct xhfc_span* s)
 {
-	if(unlikely(s->t1 && ++s->t1 > T1_MS)) expiry_T1(s);
-	if(unlikely(s->t3 && ++s->t3 > T3_MS)) expiry_T3(s);
-	if(unlikely(s->f6_f7_transition && ++s->f6_f7_transition > F6_F7_MS))
+	struct xhfc* x = s->xhfc;
+
+	if(unlikely(s->activation_timer && s->activation_timer <= x->ticks))
+		activation_timer_expiry(s);
+	if(unlikely(s->f6_f7_transition && s->f6_f7_transition <= x->ticks))
 		signal_f7_transition(s);
 }
 
@@ -187,30 +210,48 @@ static void signal_f7_transition(struct xhfc_span* s)
 {
 	if(DBG_ST && DBG_SPAN(s))
 		printk(KERN_DEBUG DRIVER_NAME ": signalling transition to F7 "
-				"on port %d", s->port + 1);
+				"on port %d\n", s->port + 1);
 
 	s->span.alarms = DAHDI_ALARM_NONE;
 	s->f6_f7_transition = NOT_RUNNING;
 }
 
-static void expiry_T1(struct xhfc_span* s)
+static void activation_timer_expiry(struct xhfc_span* s)
 {
-	if(DBG_ST && DBG_SPAN(s))
-		printk(KERN_INFO DRIVER_NAME ": NT activation timeout"
-			       " on port %d", s->port + 1);
+	int sta = GET_V_SU_STA(s->state);
 
-	set_st_state(s, G4);
-	s->t1 = NOT_RUNNING;
-}
+	bool nothing_done = false;
 
-static void expiry_T3(struct xhfc_span* s)
-{
-	if(DBG_ST && DBG_SPAN(s))
-		printk(KERN_INFO DRIVER_NAME ": TE activation timeout"
-			       " on port %d", s->port + 1);
+	s->activation_timer = NOT_RUNNING;
 
-	set_st_state(s, F3);
-	s->t3 = NOT_RUNNING;
+	if(s->nt /* T1 expiry */
+			&& sta == G2) {
+
+		/* When setting the state to G4 by writing
+		 * A_SU_WR_STA, is the T2 timer automatically
+		 * started? I suppose so because I don't see
+		 * any other way to start it and it is
+		 * implemented in hardware.
+		 */
+
+		set_st_state(s, G4);
+
+	} else if(!s->nt /* T2 expiry */
+			&& (sta == F4 || sta == F5 || sta == F8)) {
+
+		set_st_state(s, F3);
+
+	} else
+		nothing_done = true;
+	
+	if(DBG_ST && DBG_SPAN(s))
+		printk(KERN_INFO DRIVER_NAME 
+				": %s%s activation timeout"
+				" in state %c%d on port %d\n",
+				(nothing_done ? "no action for " : ""),
+				(s->nt ? "NT" : "TE"),
+				(s->nt ? 'G' : 'F'),
+				sta, s->port + 1);
 }
 
 static void set_st_state(struct xhfc_span* s, int state)