Patchwork [isci,v2,09/18] isci: fix 'link-up' events occur after 'start-complete'

login
register
mail settings
Submitter Dan Williams
Date March 11, 2012, 7:28 a.m.
Message ID <20120311072830.6320.51979.stgit@dwillia2-linux.jf.intel.com>
Download mbox | patch
Permalink /patch/145954/
State Not Applicable
Delegated to: David Miller
Headers show

Comments

Dan Williams - March 11, 2012, 7:28 a.m.
The call to wait_for_start() is meant to ensure that all links have been
given a chance to come up before letting the kernel proceed with
probing.  However, the implementation is not correctly syncing with the
port configuration agent.  In the MPC case the ports are hard-coded, in
the APC case we need to wait for the port-configuration to form ports
from the started phys.

Towards that end increase the timeout for the APC agent to form ports,
and delay start complete until all phys are out of link-training.

Cc: <stable@vger.kernel.org>
Cc: Richard Boyd <richard.g.boyd@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/scsi/isci/host.c        |   67 +++++++++++++++++++++------------------
 drivers/scsi/isci/host.h        |    3 ++
 drivers/scsi/isci/port_config.c |   18 ++++++----
 3 files changed, 50 insertions(+), 38 deletions(-)


--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/scsi/isci/host.c b/drivers/scsi/isci/host.c
index 95c3da6..577a836 100644
--- a/drivers/scsi/isci/host.c
+++ b/drivers/scsi/isci/host.c
@@ -816,7 +816,7 @@  static void sci_controller_initialize_unsolicited_frame_queue(struct isci_host *
 	       &ihost->scu_registers->sdma.unsolicited_frame_put_pointer);
 }
 
-static void sci_controller_transition_to_ready(struct isci_host *ihost, enum sci_status status)
+void sci_controller_transition_to_ready(struct isci_host *ihost, enum sci_status status)
 {
 	if (ihost->sm.current_state_id == SCIC_STARTING) {
 		/*
@@ -843,6 +843,7 @@  static bool is_phy_starting(struct isci_phy *iphy)
 	case SCI_PHY_SUB_AWAIT_SATA_POWER:
 	case SCI_PHY_SUB_AWAIT_SATA_PHY_EN:
 	case SCI_PHY_SUB_AWAIT_SATA_SPEED_EN:
+	case SCI_PHY_SUB_AWAIT_OSSP_EN:
 	case SCI_PHY_SUB_AWAIT_SIG_FIS_UF:
 	case SCI_PHY_SUB_FINAL:
 		return true;
@@ -851,6 +852,39 @@  static bool is_phy_starting(struct isci_phy *iphy)
 	}
 }
 
+bool is_controller_start_complete(struct isci_host *ihost)
+{
+	int i;
+
+	for (i = 0; i < SCI_MAX_PHYS; i++) {
+		struct isci_phy *iphy = &ihost->phys[i];
+		u32 state = iphy->sm.current_state_id;
+
+		/* in apc mode we need to check every phy, in
+		 * mpc mode we only need to check phys that have
+		 * been configured into a port
+		 */
+		if (is_port_config_apc(ihost))
+			/* pass */;
+		else if (!phy_get_non_dummy_port(iphy))
+			continue;
+
+		/* The controller start operation is complete iff:
+		 * - all links have been given an opportunity to start
+		 * - have no indication of a connected device
+		 * - have an indication of a connected device and it has
+		 *   finished the link training process.
+		 */
+		if ((iphy->is_in_link_training == false && state == SCI_PHY_INITIAL) ||
+		    (iphy->is_in_link_training == false && state == SCI_PHY_STOPPED) ||
+		    (iphy->is_in_link_training == true && is_phy_starting(iphy)) ||
+		    (ihost->port_agent.phy_ready_mask != ihost->port_agent.phy_configured_mask))
+			return false;
+	}
+
+	return true;
+}
+
 /**
  * sci_controller_start_next_phy - start phy
  * @scic: controller
@@ -871,36 +905,7 @@  static enum sci_status sci_controller_start_next_phy(struct isci_host *ihost)
 		return status;
 
 	if (ihost->next_phy_to_start >= SCI_MAX_PHYS) {
-		bool is_controller_start_complete = true;
-		u32 state;
-		u8 index;
-
-		for (index = 0; index < SCI_MAX_PHYS; index++) {
-			iphy = &ihost->phys[index];
-			state = iphy->sm.current_state_id;
-
-			if (!phy_get_non_dummy_port(iphy))
-				continue;
-
-			/* The controller start operation is complete iff:
-			 * - all links have been given an opportunity to start
-			 * - have no indication of a connected device
-			 * - have an indication of a connected device and it has
-			 *   finished the link training process.
-			 */
-			if ((iphy->is_in_link_training == false && state == SCI_PHY_INITIAL) ||
-			    (iphy->is_in_link_training == false && state == SCI_PHY_STOPPED) ||
-			    (iphy->is_in_link_training == true && is_phy_starting(iphy)) ||
-			    (ihost->port_agent.phy_ready_mask != ihost->port_agent.phy_configured_mask)) {
-				is_controller_start_complete = false;
-				break;
-			}
-		}
-
-		/*
-		 * The controller has successfully finished the start process.
-		 * Inform the SCI Core user and transition to the READY state. */
-		if (is_controller_start_complete == true) {
+		if (is_controller_start_complete(ihost)) {
 			sci_controller_transition_to_ready(ihost, SCI_SUCCESS);
 			sci_del_timer(&ihost->phy_timer);
 			ihost->phy_startup_timer_pending = false;
diff --git a/drivers/scsi/isci/host.h b/drivers/scsi/isci/host.h
index a89c0e3..9dc910b 100644
--- a/drivers/scsi/isci/host.h
+++ b/drivers/scsi/isci/host.h
@@ -109,6 +109,8 @@  struct sci_port_configuration_agent;
 typedef void (*port_config_fn)(struct isci_host *,
 			       struct sci_port_configuration_agent *,
 			       struct isci_port *, struct isci_phy *);
+bool is_port_config_apc(struct isci_host *ihost);
+bool is_controller_start_complete(struct isci_host *ihost);
 
 struct sci_port_configuration_agent {
 	u16 phy_configured_mask;
@@ -473,6 +475,7 @@  void isci_host_completion_routine(unsigned long data);
 void isci_host_deinit(struct isci_host *);
 void sci_controller_disable_interrupts(struct isci_host *ihost);
 bool sci_controller_has_remote_devices_stopping(struct isci_host *ihost);
+void sci_controller_transition_to_ready(struct isci_host *ihost, enum sci_status status);
 
 enum sci_status sci_controller_start_io(
 	struct isci_host *ihost,
diff --git a/drivers/scsi/isci/port_config.c b/drivers/scsi/isci/port_config.c
index 6d1e954..cd962da 100644
--- a/drivers/scsi/isci/port_config.c
+++ b/drivers/scsi/isci/port_config.c
@@ -57,7 +57,7 @@ 
 
 #define SCIC_SDS_MPC_RECONFIGURATION_TIMEOUT    (10)
 #define SCIC_SDS_APC_RECONFIGURATION_TIMEOUT    (10)
-#define SCIC_SDS_APC_WAIT_LINK_UP_NOTIFICATION  (250)
+#define SCIC_SDS_APC_WAIT_LINK_UP_NOTIFICATION  (1000)
 
 enum SCIC_SDS_APC_ACTIVITY {
 	SCIC_SDS_APC_SKIP_PHY,
@@ -472,13 +472,9 @@  sci_apc_agent_validate_phy_configuration(struct isci_host *ihost,
  * down event or a link up event where we can not yet tell to which a phy
  * belongs.
  */
-static void sci_apc_agent_start_timer(
-	struct sci_port_configuration_agent *port_agent,
-	u32 timeout)
+static void sci_apc_agent_start_timer(struct sci_port_configuration_agent *port_agent,
+				      u32 timeout)
 {
-	if (port_agent->timer_pending)
-		sci_del_timer(&port_agent->timer);
-
 	port_agent->timer_pending = true;
 	sci_mod_timer(&port_agent->timer, timeout);
 }
@@ -697,6 +693,9 @@  static void apc_agent_timeout(unsigned long data)
 						   &ihost->phys[index], false);
 	}
 
+	if (is_controller_start_complete(ihost))
+		sci_controller_transition_to_ready(ihost, SCI_SUCCESS);
+
 done:
 	spin_unlock_irqrestore(&ihost->scic_lock, flags);
 }
@@ -732,6 +731,11 @@  void sci_port_configuration_agent_construct(
 	}
 }
 
+bool is_port_config_apc(struct isci_host *ihost)
+{
+	return ihost->port_agent.link_up_handler == sci_apc_agent_link_up;
+}
+
 enum sci_status sci_port_configuration_agent_initialize(
 	struct isci_host *ihost,
 	struct sci_port_configuration_agent *port_agent)