linux-scsi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1 0/5] isci updates for -next
@ 2012-02-10  9:18 Dan Williams
  2012-02-10  9:18 ` [PATCH v1 1/5] isci: T10 DIF support Dan Williams
                   ` (4 more replies)
  0 siblings, 5 replies; 8+ messages in thread
From: Dan Williams @ 2012-02-10  9:18 UTC (permalink / raw)
  To: linux-scsi

This is a resend of the T10 DIF and clock gating patches (checkpatch
fixups), some debug updates, and a fix for driver load/unload testing.

This is based on branch of the pending libsas changes as it grew an
entanglement with the phy reference counting and lldd_port reworks.

---

Andrzej Jakowski (1):
      isci: improvements in driver unloading routine

Dan Williams (2):
      isci: debug, provide state-enum-to-string conversions
      isci: improve phy event warnings

Dave Jiang (1):
      isci: T10 DIF support

Marcin Tomczak (1):
      isci: enable clock gating


 drivers/scsi/isci/host.c                |    9 +
 drivers/scsi/isci/init.c                |   10 +
 drivers/scsi/isci/phy.c                 |  157 +++++++++++++---------
 drivers/scsi/isci/phy.h                 |  154 ++++++++--------------
 drivers/scsi/isci/port.c                |   46 ++++---
 drivers/scsi/isci/port.h                |  103 ++++++---------
 drivers/scsi/isci/registers.h           |   27 ++++
 drivers/scsi/isci/remote_device.c       |   50 ++++---
 drivers/scsi/isci/remote_device.h       |  205 +++++++++++++----------------
 drivers/scsi/isci/remote_node_context.c |   19 +--
 drivers/scsi/isci/remote_node_context.h |   97 ++++++--------
 drivers/scsi/isci/request.c             |  172 +++++++++++++++++++++++-
 drivers/scsi/isci/request.h             |  219 +++++++++++++------------------
 drivers/scsi/isci/scu_task_context.h    |   55 ++++++--
 14 files changed, 732 insertions(+), 591 deletions(-)

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v1 1/5] isci: T10 DIF support
  2012-02-10  9:18 [PATCH v1 0/5] isci updates for -next Dan Williams
@ 2012-02-10  9:18 ` Dan Williams
  2012-02-13 17:14   ` Martin K. Petersen
  2012-02-10  9:18 ` [PATCH v1 2/5] isci: enable clock gating Dan Williams
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 8+ messages in thread
From: Dan Williams @ 2012-02-10  9:18 UTC (permalink / raw)
  To: linux-scsi; +Cc: Dave Jiang, Martin K. Petersen

From: Dave Jiang <dave.jiang@intel.com>

This allows the controller to do WRITE_INSERT and READ_STRIP for SAS
disks that support protection information. SAS disks must be formatted
with protection information to use this feature via sg_format.

  sg3_utils-1.32 -- sg_format version 1.19 20110730
  sg_format usage:
  sg_format --format --verbose --pinfo /dev/sda

Cc: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/scsi/isci/init.c             |    7 ++
 drivers/scsi/isci/request.c          |  147 ++++++++++++++++++++++++++++++++++
 drivers/scsi/isci/scu_task_context.h |   55 +++++++++----
 3 files changed, 193 insertions(+), 16 deletions(-)

diff --git a/drivers/scsi/isci/init.c b/drivers/scsi/isci/init.c
index f682ca1..c3fe39b 100644
--- a/drivers/scsi/isci/init.c
+++ b/drivers/scsi/isci/init.c
@@ -523,6 +523,13 @@ static int __devinit isci_pci_probe(struct pci_dev *pdev, const struct pci_devic
 			goto err_host_alloc;
 		}
 		pci_info->hosts[i] = h;
+
+		/* turn on DIF support */
+		scsi_host_set_prot(h->shost,
+				   SHOST_DIF_TYPE1_PROTECTION |
+				   SHOST_DIF_TYPE2_PROTECTION |
+				   SHOST_DIF_TYPE3_PROTECTION);
+		scsi_host_set_guard(h->shost, SHOST_DIX_GUARD_CRC);
 	}
 
 	err = isci_setup_interrupts(pdev);
diff --git a/drivers/scsi/isci/request.c b/drivers/scsi/isci/request.c
index 0e43efd..1a39ce5 100644
--- a/drivers/scsi/isci/request.c
+++ b/drivers/scsi/isci/request.c
@@ -53,6 +53,7 @@
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
+#include <scsi/scsi_cmnd.h>
 #include "isci.h"
 #include "task.h"
 #include "request.h"
@@ -264,6 +265,141 @@ static void scu_ssp_reqeust_construct_task_context(
 	task_context->response_iu_lower = lower_32_bits(dma_addr);
 }
 
+static u8 scu_bg_blk_size(struct scsi_device *sdp)
+{
+	switch (sdp->sector_size) {
+	case 512:
+		return 0;
+	case 1024:
+		return 1;
+	case 4096:
+		return 3;
+	default:
+		return 0xff;
+	}
+}
+
+static u32 scu_dif_bytes(u32 len, u32 sector_size)
+{
+	return (len >> ilog2(sector_size)) * 8;
+}
+
+static void scu_ssp_ireq_dif_insert(struct isci_request *ireq, u8 type, u8 op)
+{
+	struct scu_task_context *tc = ireq->tc;
+	struct scsi_cmnd *scmd = ireq->ttype_ptr.io_task_ptr->uldd_task;
+	u8 blk_sz = scu_bg_blk_size(scmd->device);
+
+	tc->block_guard_enable = 1;
+	tc->blk_prot_en = 1;
+	tc->blk_sz = blk_sz;
+	/* DIF write insert */
+	tc->blk_prot_func = 0x2;
+
+	tc->transfer_length_bytes += scu_dif_bytes(tc->transfer_length_bytes,
+						   scmd->device->sector_size);
+
+	/* always init to 0, used by hw */
+	tc->interm_crc_val = 0;
+
+	tc->init_crc_seed = 0;
+	tc->app_tag_verify = 0;
+	tc->app_tag_gen = 0;
+	tc->ref_tag_seed_verify = 0;
+
+	/* always init to same as bg_blk_sz */
+	tc->UD_bytes_immed_val = scmd->device->sector_size;
+
+	tc->reserved_DC_0 = 0;
+
+	/* always init to 8 */
+	tc->DIF_bytes_immed_val = 8;
+
+	tc->reserved_DC_1 = 0;
+	tc->bgc_blk_sz = scmd->device->sector_size;
+	tc->reserved_E0_0 = 0;
+	tc->app_tag_gen_mask = 0;
+
+	/** setup block guard control **/
+	tc->bgctl = 0;
+
+	/* DIF write insert */
+	tc->bgctl_f.op = 0x2;
+
+	tc->app_tag_verify_mask = 0;
+
+	/* must init to 0 for hw */
+	tc->blk_guard_err = 0;
+
+	tc->reserved_E8_0 = 0;
+
+	if ((type & SCSI_PROT_DIF_TYPE1) || (type & SCSI_PROT_DIF_TYPE2))
+		tc->ref_tag_seed_gen = scsi_get_lba(scmd) & 0xffffffff;
+	else if (type & SCSI_PROT_DIF_TYPE3)
+		tc->ref_tag_seed_gen = 0;
+}
+
+static void scu_ssp_ireq_dif_strip(struct isci_request *ireq, u8 type, u8 op)
+{
+	struct scu_task_context *tc = ireq->tc;
+	struct scsi_cmnd *scmd = ireq->ttype_ptr.io_task_ptr->uldd_task;
+	u8 blk_sz = scu_bg_blk_size(scmd->device);
+
+	tc->block_guard_enable = 1;
+	tc->blk_prot_en = 1;
+	tc->blk_sz = blk_sz;
+	/* DIF read strip */
+	tc->blk_prot_func = 0x1;
+
+	tc->transfer_length_bytes += scu_dif_bytes(tc->transfer_length_bytes,
+						   scmd->device->sector_size);
+
+	/* always init to 0, used by hw */
+	tc->interm_crc_val = 0;
+
+	tc->init_crc_seed = 0;
+	tc->app_tag_verify = 0;
+	tc->app_tag_gen = 0;
+
+	if ((type & SCSI_PROT_DIF_TYPE1) || (type & SCSI_PROT_DIF_TYPE2))
+		tc->ref_tag_seed_verify = scsi_get_lba(scmd) & 0xffffffff;
+	else if (type & SCSI_PROT_DIF_TYPE3)
+		tc->ref_tag_seed_verify = 0;
+
+	/* always init to same as bg_blk_sz */
+	tc->UD_bytes_immed_val = scmd->device->sector_size;
+
+	tc->reserved_DC_0 = 0;
+
+	/* always init to 8 */
+	tc->DIF_bytes_immed_val = 8;
+
+	tc->reserved_DC_1 = 0;
+	tc->bgc_blk_sz = scmd->device->sector_size;
+	tc->reserved_E0_0 = 0;
+	tc->app_tag_gen_mask = 0;
+
+	/** setup block guard control **/
+	tc->bgctl = 0;
+
+	/* DIF read strip */
+	tc->bgctl_f.crc_verify = 1;
+	tc->bgctl_f.op = 0x1;
+	if ((type & SCSI_PROT_DIF_TYPE1) || (type & SCSI_PROT_DIF_TYPE2)) {
+		tc->bgctl_f.ref_tag_chk = 1;
+		tc->bgctl_f.app_f_detect = 1;
+	} else if (type & SCSI_PROT_DIF_TYPE3)
+		tc->bgctl_f.app_ref_f_detect = 1;
+
+	tc->app_tag_verify_mask = 0;
+
+	/* must init to 0 for hw */
+	tc->blk_guard_err = 0;
+
+	tc->reserved_E8_0 = 0;
+	tc->ref_tag_seed_gen = 0;
+}
+
 /**
  * This method is will fill in the SCU Task Context for a SSP IO request.
  * @sci_req:
@@ -274,6 +410,10 @@ static void scu_ssp_io_request_construct_task_context(struct isci_request *ireq,
 						      u32 len)
 {
 	struct scu_task_context *task_context = ireq->tc;
+	struct sas_task *sas_task = ireq->ttype_ptr.io_task_ptr;
+	struct scsi_cmnd *scmd = sas_task->uldd_task;
+	u8 prot_type = scsi_get_prot_type(scmd);
+	u8 prot_op = scsi_get_prot_op(scmd);
 
 	scu_ssp_reqeust_construct_task_context(ireq, task_context);
 
@@ -296,6 +436,13 @@ static void scu_ssp_io_request_construct_task_context(struct isci_request *ireq,
 
 	if (task_context->transfer_length_bytes > 0)
 		sci_request_build_sgl(ireq);
+
+	if (prot_type != SCSI_PROT_DIF_TYPE0) {
+		if (prot_op == SCSI_PROT_READ_STRIP)
+			scu_ssp_ireq_dif_strip(ireq, prot_type, prot_op);
+		else if (prot_op == SCSI_PROT_WRITE_INSERT)
+			scu_ssp_ireq_dif_insert(ireq, prot_type, prot_op);
+	}
 }
 
 /**
diff --git a/drivers/scsi/isci/scu_task_context.h b/drivers/scsi/isci/scu_task_context.h
index 7df87d9..869a979 100644
--- a/drivers/scsi/isci/scu_task_context.h
+++ b/drivers/scsi/isci/scu_task_context.h
@@ -866,9 +866,9 @@ struct scu_task_context {
 	struct transport_snapshot snapshot; /* read only set to 0 */
 
 	/* OFFSET 0x5C */
-	u32 block_protection_enable:1;
-	u32 block_size:2;
-	u32 block_protection_function:2;
+	u32 blk_prot_en:1;
+	u32 blk_sz:2;
+	u32 blk_prot_func:2;
 	u32 reserved_5C_0:9;
 	u32 active_sgl_element:2;  /* read only set to 0 */
 	u32 sgl_exhausted:1;  /* read only set to 0 */
@@ -896,33 +896,56 @@ struct scu_task_context {
 	u32 reserved_C4_CC[3];
 
 	/* OFFSET 0xD0 */
-	u32 intermediate_crc_value:16;
-	u32 initial_crc_seed:16;
+	u32 interm_crc_val:16;
+	u32 init_crc_seed:16;
 
 	/* OFFSET 0xD4 */
-	u32 application_tag_for_verify:16;
-	u32 application_tag_for_generate:16;
+	u32 app_tag_verify:16;
+	u32 app_tag_gen:16;
 
 	/* OFFSET 0xD8 */
-	u32 reference_tag_seed_for_verify_function;
+	u32 ref_tag_seed_verify;
 
 	/* OFFSET 0xDC */
-	u32 reserved_DC;
+	u32 UD_bytes_immed_val:13;
+	u32 reserved_DC_0:3;
+	u32 DIF_bytes_immed_val:4;
+	u32 reserved_DC_1:12;
 
 	/* OFFSET 0xE0 */
-	u32 reserved_E0_0:16;
-	u32 application_tag_mask_for_generate:16;
+	u32 bgc_blk_sz:13;
+	u32 reserved_E0_0:3;
+	u32 app_tag_gen_mask:16;
 
 	/* OFFSET 0xE4 */
-	u32 block_protection_control:16;
-	u32 application_tag_mask_for_verify:16;
+	union {
+		u16 bgctl;
+		struct {
+			u16 crc_verify:1;
+			u16 app_tag_chk:1;
+			u16 ref_tag_chk:1;
+			u16 op:2;
+			u16 legacy:1;
+			u16 invert_crc_seed:1;
+			u16 ref_tag_gen:1;
+			u16 fixed_ref_tag:1;
+			u16 invert_crc:1;
+			u16 app_ref_f_detect:1;
+			u16 uninit_dif_check_err:1;
+			u16 uninit_dif_bypass:1;
+			u16 app_f_detect:1;
+			u16 reserved_0:2;
+		} bgctl_f;
+	};
+
+	u16 app_tag_verify_mask;
 
 	/* OFFSET 0xE8 */
-	u32 block_protection_error:8;
+	u32 blk_guard_err:8;
 	u32 reserved_E8_0:24;
 
 	/* OFFSET 0xEC */
-	u32 reference_tag_seed_for_verify;
+	u32 ref_tag_seed_gen;
 
 	/* OFFSET 0xF0 */
 	u32 intermediate_crc_valid_snapshot:16;
@@ -937,6 +960,6 @@ struct scu_task_context {
 	/* OFFSET 0xFC */
 	u32 reference_tag_seed_for_generate_function_snapshot;
 
-};
+} __packed;
 
 #endif /* _SCU_TASK_CONTEXT_H_ */


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v1 2/5] isci: enable clock gating
  2012-02-10  9:18 [PATCH v1 0/5] isci updates for -next Dan Williams
  2012-02-10  9:18 ` [PATCH v1 1/5] isci: T10 DIF support Dan Williams
@ 2012-02-10  9:18 ` Dan Williams
  2012-02-10  9:18 ` [PATCH v1 3/5] isci: debug, provide state-enum-to-string conversions Dan Williams
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Dan Williams @ 2012-02-10  9:18 UTC (permalink / raw)
  To: linux-scsi; +Cc: Marcin Tomczak

From: Marcin Tomczak <marcin.tomczak@intel.com>

Enabling clock gating for power savings on entry to controller ready
state. Disable SCU clock gating for power savings on exit from the
controller ready state.

The gating is fully automated by silicon after setting the mode.

Signed-off-by: Marcin Tomczak <marcin.tomczak@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/scsi/isci/host.c      |    9 +++++++++
 drivers/scsi/isci/registers.h |   27 +++++++++++++++++++++++++--
 2 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/isci/host.c b/drivers/scsi/isci/host.c
index 36c443f..2b62af2 100644
--- a/drivers/scsi/isci/host.c
+++ b/drivers/scsi/isci/host.c
@@ -1489,6 +1489,15 @@ sci_controller_set_interrupt_coalescence(struct isci_host *ihost,
 static void sci_controller_ready_state_enter(struct sci_base_state_machine *sm)
 {
 	struct isci_host *ihost = container_of(sm, typeof(*ihost), sm);
+	u32 val;
+
+	/* enable clock gating for power control of the scu unit */
+	val = readl(&ihost->smu_registers->clock_gating_control);
+	val &= ~(SMU_CGUCR_GEN_BIT(REGCLK_ENABLE) |
+		 SMU_CGUCR_GEN_BIT(TXCLK_ENABLE) |
+		 SMU_CGUCR_GEN_BIT(XCLK_ENABLE));
+	val |= SMU_CGUCR_GEN_BIT(IDLE_ENABLE);
+	writel(val, &ihost->smu_registers->clock_gating_control);
 
 	/* set the default interrupt coalescence number and timeout value. */
 	sci_controller_set_interrupt_coalescence(ihost, 0, 0);
diff --git a/drivers/scsi/isci/registers.h b/drivers/scsi/isci/registers.h
index eaa541a..f232f7e 100644
--- a/drivers/scsi/isci/registers.h
+++ b/drivers/scsi/isci/registers.h
@@ -370,6 +370,27 @@ struct scu_iit_entry {
 		>> SMU_DEVICE_CONTEXT_CAPACITY_MAX_RNC_SHIFT \
 	)
 
+/* ************************************************************************** */
+#define SMU_CLOCK_GATING_CONTROL_IDLE_ENABLE_SHIFT    (0)
+#define SMU_CLOCK_GATING_CONTROL_IDLE_ENABLE_MASK     (0x00000001)
+#define SMU_CLOCK_GATING_CONTROL_XCLK_ENABLE_SHIFT    (1)
+#define SMU_CLOCK_GATING_CONTROL_XCLK_ENABLE_MASK     (0x00000002)
+#define SMU_CLOCK_GATING_CONTROL_TXCLK_ENABLE_SHIFT   (2)
+#define SMU_CLOCK_GATING_CONTROL_TXCLK_ENABLE_MASK    (0x00000004)
+#define SMU_CLOCK_GATING_CONTROL_REGCLK_ENABLE_SHIFT  (3)
+#define SMU_CLOCK_GATING_CONTROL_REGCLK_ENABLE_MASK   (0x00000008)
+#define SMU_CLOCK_GATING_CONTROL_IDLE_TIMEOUT_SHIFT   (16)
+#define SMU_CLOCK_GATING_CONTROL_IDLE_TIMEOUT_MASK    (0x000F0000)
+#define SMU_CLOCK_GATING_CONTROL_FORCE_IDLE_SHIFT     (31)
+#define SMU_CLOCK_GATING_CONTROL_FORCE_IDLE_MASK      (0x80000000)
+#define SMU_CLOCK_GATING_CONTROL_RESERVED_MASK        (0x7FF0FFF0)
+
+#define SMU_CGUCR_GEN_VAL(name, value) \
+	SCU_GEN_VALUE(SMU_CLOCK_GATING_CONTROL_##name, value)
+
+#define SMU_CGUCR_GEN_BIT(name) \
+	SCU_GEN_BIT(SMU_CLOCK_GATING_CONTROL_##name)
+
 /* -------------------------------------------------------------------------- */
 
 #define SMU_CONTROL_STATUS_TASK_CONTEXT_RANGE_ENABLE_SHIFT      (0)
@@ -992,8 +1013,10 @@ struct smu_registers {
 	u32 mmr_address_window;
 /* 0x00A4 SMDW */
 	u32 mmr_data_window;
-	u32 reserved_A8;
-	u32 reserved_AC;
+/* 0x00A8 CGUCR */
+	u32 clock_gating_control;
+/* 0x00AC CGUPC */
+	u32 clock_gating_performance;
 /* A whole bunch of reserved space */
 	u32 reserved_Bx[4];
 	u32 reserved_Cx[4];


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v1 3/5] isci: debug, provide state-enum-to-string conversions
  2012-02-10  9:18 [PATCH v1 0/5] isci updates for -next Dan Williams
  2012-02-10  9:18 ` [PATCH v1 1/5] isci: T10 DIF support Dan Williams
  2012-02-10  9:18 ` [PATCH v1 2/5] isci: enable clock gating Dan Williams
@ 2012-02-10  9:18 ` Dan Williams
  2012-02-19 15:17   ` James Bottomley
  2012-02-10  9:18 ` [PATCH v1 4/5] isci: improve phy event warnings Dan Williams
  2012-02-10  9:18 ` [PATCH v1 5/5] isci: improvements in driver unloading routine Dan Williams
  4 siblings, 1 reply; 8+ messages in thread
From: Dan Williams @ 2012-02-10  9:18 UTC (permalink / raw)
  To: linux-scsi

Debugging the driver requires tracing the state transtions and tracing
state names is less work than decoding numbers.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/scsi/isci/phy.c                 |   34 +++--
 drivers/scsi/isci/phy.h                 |  154 ++++++++--------------
 drivers/scsi/isci/port.c                |   46 ++++---
 drivers/scsi/isci/port.h                |  103 ++++++---------
 drivers/scsi/isci/remote_device.c       |   50 ++++---
 drivers/scsi/isci/remote_device.h       |  205 +++++++++++++----------------
 drivers/scsi/isci/remote_node_context.c |   19 +--
 drivers/scsi/isci/remote_node_context.h |   97 ++++++--------
 drivers/scsi/isci/request.c             |   25 ++--
 drivers/scsi/isci/request.h             |  219 +++++++++++++------------------
 10 files changed, 432 insertions(+), 520 deletions(-)

diff --git a/drivers/scsi/isci/phy.c b/drivers/scsi/isci/phy.c
index 8d412ca..87064f1 100644
--- a/drivers/scsi/isci/phy.c
+++ b/drivers/scsi/isci/phy.c
@@ -59,6 +59,16 @@
 #include "scu_event_codes.h"
 #include "probe_roms.h"
 
+#undef C
+#define C(a) (#a)
+static const char *phy_state_name(enum sci_phy_states state)
+{
+	static const char * const strings[] = PHY_STATES;
+
+	return strings[state];
+}
+#undef C
+
 /* Maximum arbitration wait time in micro-seconds */
 #define SCIC_SDS_PHY_MAX_ARBITRATION_WAIT_TIME  (700)
 
@@ -454,8 +464,8 @@ enum sci_status sci_phy_start(struct isci_phy *iphy)
 	enum sci_phy_states state = iphy->sm.current_state_id;
 
 	if (state != SCI_PHY_STOPPED) {
-		dev_dbg(sciphy_to_dev(iphy),
-			 "%s: in wrong state: %d\n", __func__, state);
+		dev_dbg(sciphy_to_dev(iphy), "%s: in wrong state: %s\n",
+			__func__, phy_state_name(state));
 		return SCI_FAILURE_INVALID_STATE;
 	}
 
@@ -480,8 +490,8 @@ enum sci_status sci_phy_stop(struct isci_phy *iphy)
 	case SCI_PHY_READY:
 		break;
 	default:
-		dev_dbg(sciphy_to_dev(iphy),
-			"%s: in wrong state: %d\n", __func__, state);
+		dev_dbg(sciphy_to_dev(iphy), "%s: in wrong state: %s\n",
+			__func__, phy_state_name(state));
 		return SCI_FAILURE_INVALID_STATE;
 	}
 
@@ -494,8 +504,8 @@ enum sci_status sci_phy_reset(struct isci_phy *iphy)
 	enum sci_phy_states state = iphy->sm.current_state_id;
 
 	if (state != SCI_PHY_READY) {
-		dev_dbg(sciphy_to_dev(iphy),
-			"%s: in wrong state: %d\n", __func__, state);
+		dev_dbg(sciphy_to_dev(iphy), "%s: in wrong state: %s\n",
+			__func__, phy_state_name(state));
 		return SCI_FAILURE_INVALID_STATE;
 	}
 
@@ -544,8 +554,8 @@ enum sci_status sci_phy_consume_power_handler(struct isci_phy *iphy)
 		return SCI_SUCCESS;
 	}
 	default:
-		dev_dbg(sciphy_to_dev(iphy),
-			"%s: in wrong state: %d\n", __func__, state);
+		dev_dbg(sciphy_to_dev(iphy), "%s: in wrong state: %s\n",
+			__func__, phy_state_name(state));
 		return SCI_FAILURE_INVALID_STATE;
 	}
 }
@@ -870,8 +880,8 @@ enum sci_status sci_phy_event_handler(struct isci_phy *iphy, u32 event_code)
 		}
 		return SCI_SUCCESS;
 	default:
-		dev_dbg(sciphy_to_dev(iphy),
-			"%s: in wrong state: %d\n", __func__, state);
+		dev_dbg(sciphy_to_dev(iphy), "%s: in wrong state: %s\n",
+			__func__, phy_state_name(state));
 		return SCI_FAILURE_INVALID_STATE;
 	}
 }
@@ -964,8 +974,8 @@ enum sci_status sci_phy_frame_handler(struct isci_phy *iphy, u32 frame_index)
 		return result;
 	}
 	default:
-		dev_dbg(sciphy_to_dev(iphy),
-			"%s: in wrong state: %d\n", __func__, state);
+		dev_dbg(sciphy_to_dev(iphy), "%s: in wrong state: %s\n",
+			__func__, phy_state_name(state));
 		return SCI_FAILURE_INVALID_STATE;
 	}
 
diff --git a/drivers/scsi/isci/phy.h b/drivers/scsi/isci/phy.h
index a5e1a9e..0e45833 100644
--- a/drivers/scsi/isci/phy.h
+++ b/drivers/scsi/isci/phy.h
@@ -343,101 +343,65 @@ enum sci_phy_counter_id {
 	SCIC_PHY_COUNTER_SN_DWORD_SYNC_ERROR
 };
 
-enum sci_phy_states {
-	/**
-	 * Simply the initial state for the base domain state machine.
-	 */
-	SCI_PHY_INITIAL,
-
-	/**
-	 * This state indicates that the phy has successfully been stopped.
-	 * In this state no new IO operations are permitted on this phy.
-	 * This state is entered from the INITIAL state.
-	 * This state is entered from the STARTING state.
-	 * This state is entered from the READY state.
-	 * This state is entered from the RESETTING state.
-	 */
-	SCI_PHY_STOPPED,
-
-	/**
-	 * This state indicates that the phy is in the process of becomming
-	 * ready.  In this state no new IO operations are permitted on this phy.
-	 * This state is entered from the STOPPED state.
-	 * This state is entered from the READY state.
-	 * This state is entered from the RESETTING state.
-	 */
-	SCI_PHY_STARTING,
-
-	/**
-	 * Initial state
-	 */
-	SCI_PHY_SUB_INITIAL,
-
-	/**
-	 * Wait state for the hardware OSSP event type notification
-	 */
-	SCI_PHY_SUB_AWAIT_OSSP_EN,
-
-	/**
-	 * Wait state for the PHY speed notification
-	 */
-	SCI_PHY_SUB_AWAIT_SAS_SPEED_EN,
-
-	/**
-	 * Wait state for the IAF Unsolicited frame notification
-	 */
-	SCI_PHY_SUB_AWAIT_IAF_UF,
-
-	/**
-	 * Wait state for the request to consume power
-	 */
-	SCI_PHY_SUB_AWAIT_SAS_POWER,
-
-	/**
-	 * Wait state for request to consume power
-	 */
-	SCI_PHY_SUB_AWAIT_SATA_POWER,
-
-	/**
-	 * Wait state for the SATA PHY notification
-	 */
-	SCI_PHY_SUB_AWAIT_SATA_PHY_EN,
-
-	/**
-	 * Wait for the SATA PHY speed notification
-	 */
-	SCI_PHY_SUB_AWAIT_SATA_SPEED_EN,
-
-	/**
-	 * Wait state for the SIGNATURE FIS unsolicited frame notification
-	 */
-	SCI_PHY_SUB_AWAIT_SIG_FIS_UF,
-
-	/**
-	 * Exit state for this state machine
-	 */
-	SCI_PHY_SUB_FINAL,
-
-	/**
-	 * This state indicates the the phy is now ready.  Thus, the user
-	 * is able to perform IO operations utilizing this phy as long as it
-	 * is currently part of a valid port.
-	 * This state is entered from the STARTING state.
-	 */
-	SCI_PHY_READY,
-
-	/**
-	 * This state indicates that the phy is in the process of being reset.
-	 * In this state no new IO operations are permitted on this phy.
-	 * This state is entered from the READY state.
-	 */
-	SCI_PHY_RESETTING,
-
-	/**
-	 * Simply the final state for the base phy state machine.
-	 */
-	SCI_PHY_FINAL,
-};
+/**
+ * enum sci_phy_states - phy state machine states
+ * @SCI_PHY_INITIAL: Simply the initial state for the base domain state
+ *		     machine.
+ * @SCI_PHY_STOPPED: phy has successfully been stopped.  In this state
+ *		     no new IO operations are permitted on this phy.
+ * @SCI_PHY_STARTING: the phy is in the process of becomming ready.  In
+ *		      this state no new IO operations are permitted on
+ *		      this phy.
+ * @SCI_PHY_SUB_INITIAL: Initial state
+ * @SCI_PHY_SUB_AWAIT_OSSP_EN: Wait state for the hardware OSSP event
+ *			       type notification
+ * @SCI_PHY_SUB_AWAIT_SAS_SPEED_EN: Wait state for the PHY speed
+ *				    notification
+ * @SCI_PHY_SUB_AWAIT_IAF_UF: Wait state for the IAF Unsolicited frame
+ *			      notification
+ * @SCI_PHY_SUB_AWAIT_SAS_POWER: Wait state for the request to consume
+ *				 power
+ * @SCI_PHY_SUB_AWAIT_SATA_POWER: Wait state for request to consume
+ *				  power
+ * @SCI_PHY_SUB_AWAIT_SATA_PHY_EN: Wait state for the SATA PHY
+ *				   notification
+ * @SCI_PHY_SUB_AWAIT_SATA_SPEED_EN: Wait for the SATA PHY speed
+ *				     notification
+ * @SCI_PHY_SUB_AWAIT_SIG_FIS_UF: Wait state for the SIGNATURE FIS
+ *				  unsolicited frame notification
+ * @SCI_PHY_SUB_FINAL: Exit state for this state machine
+ * @SCI_PHY_READY: phy is now ready.  Thus, the user is able to perform
+ *		   IO operations utilizing this phy as long as it is
+ *		   currently part of a valid port.  This state is
+ *		   entered from the STARTING state.
+ * @SCI_PHY_RESETTING: phy is in the process of being reset.  In this
+ *		       state no new IO operations are permitted on this
+ *		       phy.  This state is entered from the READY state.
+ * @SCI_PHY_FINAL: Simply the final state for the base phy state
+ *		   machine.
+ */
+#define PHY_STATES {\
+	C(PHY_INITIAL),\
+	C(PHY_STOPPED),\
+	C(PHY_STARTING),\
+	C(PHY_SUB_INITIAL),\
+	C(PHY_SUB_AWAIT_OSSP_EN),\
+	C(PHY_SUB_AWAIT_SAS_SPEED_EN),\
+	C(PHY_SUB_AWAIT_IAF_UF),\
+	C(PHY_SUB_AWAIT_SAS_POWER),\
+	C(PHY_SUB_AWAIT_SATA_POWER),\
+	C(PHY_SUB_AWAIT_SATA_PHY_EN),\
+	C(PHY_SUB_AWAIT_SATA_SPEED_EN),\
+	C(PHY_SUB_AWAIT_SIG_FIS_UF),\
+	C(PHY_SUB_FINAL),\
+	C(PHY_READY),\
+	C(PHY_RESETTING),\
+	C(PHY_FINAL),\
+	}
+#undef C
+#define C(a) SCI_##a
+enum sci_phy_states PHY_STATES;
+#undef C
 
 void sci_phy_construct(
 	struct isci_phy *iphy,
diff --git a/drivers/scsi/isci/port.c b/drivers/scsi/isci/port.c
index c5ae94d..5fada73 100644
--- a/drivers/scsi/isci/port.c
+++ b/drivers/scsi/isci/port.c
@@ -60,6 +60,16 @@
 #define SCIC_SDS_PORT_HARD_RESET_TIMEOUT  (1000)
 #define SCU_DUMMY_INDEX    (0xFFFF)
 
+#undef C
+#define C(a) (#a)
+const char *port_state_name(enum sci_port_states state)
+{
+	static const char * const strings[] = PORT_STATES;
+
+	return strings[state];
+}
+#undef C
+
 static struct device *sciport_to_dev(struct isci_port *iport)
 {
 	int i = iport->physical_port_index;
@@ -1054,8 +1064,8 @@ enum sci_status sci_port_start(struct isci_port *iport)
 
 	state = iport->sm.current_state_id;
 	if (state != SCI_PORT_STOPPED) {
-		dev_warn(sciport_to_dev(iport),
-			 "%s: in wrong state: %d\n", __func__, state);
+		dev_warn(sciport_to_dev(iport), "%s: in wrong state: %s\n",
+			 __func__, port_state_name(state));
 		return SCI_FAILURE_INVALID_STATE;
 	}
 
@@ -1129,8 +1139,8 @@ enum sci_status sci_port_stop(struct isci_port *iport)
 					  SCI_PORT_STOPPING);
 		return SCI_SUCCESS;
 	default:
-		dev_warn(sciport_to_dev(iport),
-			 "%s: in wrong state: %d\n", __func__, state);
+		dev_warn(sciport_to_dev(iport), "%s: in wrong state: %s\n",
+			 __func__, port_state_name(state));
 		return SCI_FAILURE_INVALID_STATE;
 	}
 }
@@ -1144,8 +1154,8 @@ static enum sci_status sci_port_hard_reset(struct isci_port *iport, u32 timeout)
 
 	state = iport->sm.current_state_id;
 	if (state != SCI_PORT_SUB_OPERATIONAL) {
-		dev_warn(sciport_to_dev(iport),
-			 "%s: in wrong state: %d\n", __func__, state);
+		dev_warn(sciport_to_dev(iport), "%s: in wrong state: %s\n",
+			 __func__, port_state_name(state));
 		return SCI_FAILURE_INVALID_STATE;
 	}
 
@@ -1239,8 +1249,8 @@ enum sci_status sci_port_add_phy(struct isci_port *iport,
 					  SCI_PORT_SUB_CONFIGURING);
 		return SCI_SUCCESS;
 	default:
-		dev_warn(sciport_to_dev(iport),
-			 "%s: in wrong state: %d\n", __func__, state);
+		dev_warn(sciport_to_dev(iport), "%s: in wrong state: %s\n",
+			 __func__, port_state_name(state));
 		return SCI_FAILURE_INVALID_STATE;
 	}
 }
@@ -1289,8 +1299,8 @@ enum sci_status sci_port_remove_phy(struct isci_port *iport,
 					  SCI_PORT_SUB_CONFIGURING);
 		return SCI_SUCCESS;
 	default:
-		dev_warn(sciport_to_dev(iport),
-			 "%s: in wrong state: %d\n", __func__, state);
+		dev_warn(sciport_to_dev(iport), "%s: in wrong state: %s\n",
+			 __func__, port_state_name(state));
 		return SCI_FAILURE_INVALID_STATE;
 	}
 }
@@ -1332,8 +1342,8 @@ enum sci_status sci_port_link_up(struct isci_port *iport,
 		sci_port_general_link_up_handler(iport, iphy, PF_RESUME);
 		return SCI_SUCCESS;
 	default:
-		dev_warn(sciport_to_dev(iport),
-			 "%s: in wrong state: %d\n", __func__, state);
+		dev_warn(sciport_to_dev(iport), "%s: in wrong state: %s\n",
+			 __func__, port_state_name(state));
 		return SCI_FAILURE_INVALID_STATE;
 	}
 }
@@ -1362,8 +1372,8 @@ enum sci_status sci_port_link_down(struct isci_port *iport,
 		sci_port_deactivate_phy(iport, iphy, false);
 		return SCI_SUCCESS;
 	default:
-		dev_warn(sciport_to_dev(iport),
-			 "%s: in wrong state: %d\n", __func__, state);
+		dev_warn(sciport_to_dev(iport), "%s: in wrong state: %s\n",
+			 __func__, port_state_name(state));
 		return SCI_FAILURE_INVALID_STATE;
 	}
 }
@@ -1382,8 +1392,8 @@ enum sci_status sci_port_start_io(struct isci_port *iport,
 		iport->started_request_count++;
 		return SCI_SUCCESS;
 	default:
-		dev_warn(sciport_to_dev(iport),
-			 "%s: in wrong state: %d\n", __func__, state);
+		dev_warn(sciport_to_dev(iport), "%s: in wrong state: %s\n",
+			 __func__, port_state_name(state));
 		return SCI_FAILURE_INVALID_STATE;
 	}
 }
@@ -1397,8 +1407,8 @@ enum sci_status sci_port_complete_io(struct isci_port *iport,
 	state = iport->sm.current_state_id;
 	switch (state) {
 	case SCI_PORT_STOPPED:
-		dev_warn(sciport_to_dev(iport),
-			 "%s: in wrong state: %d\n", __func__, state);
+		dev_warn(sciport_to_dev(iport), "%s: in wrong state: %s\n",
+			 __func__, port_state_name(state));
 		return SCI_FAILURE_INVALID_STATE;
 	case SCI_PORT_STOPPING:
 		sci_port_decrement_request_count(iport);
diff --git a/drivers/scsi/isci/port.h b/drivers/scsi/isci/port.h
index 321b987..6b56240 100644
--- a/drivers/scsi/isci/port.h
+++ b/drivers/scsi/isci/port.h
@@ -144,70 +144,47 @@ struct sci_port_properties {
 };
 
 /**
- * enum sci_port_states - This enumeration depicts all the states for the
- *    common port state machine.
- *
- *
+ * enum sci_port_states - port state machine states
+ * @SCI_PORT_STOPPED: port has successfully been stopped.  In this state
+ *		      no new IO operations are permitted.  This state is
+ *		      entered from the STOPPING state.
+ * @SCI_PORT_STOPPING: port is in the process of stopping.  In this
+ *		       state no new IO operations are permitted, but
+ *		       existing IO operations are allowed to complete.
+ *		       This state is entered from the READY state.
+ * @SCI_PORT_READY: port is now ready.  Thus, the user is able to
+ *		    perform IO operations on this port. This state is
+ *		    entered from the STARTING state.
+ * @SCI_PORT_SUB_WAITING: port is started and ready but has no active
+ *			  phys.
+ * @SCI_PORT_SUB_OPERATIONAL: port is started and ready and there is at
+ *			      least one phy operational.
+ * @SCI_PORT_SUB_CONFIGURING: port is started and there was an
+ *			      add/remove phy event.  This state is only
+ *			      used in Automatic Port Configuration Mode
+ *			      (APC)
+ * @SCI_PORT_RESETTING: port is in the process of performing a hard
+ *			reset.  Thus, the user is unable to perform IO
+ *			operations on this port.  This state is entered
+ *			from the READY state.
+ * @SCI_PORT_FAILED: port has failed a reset request.  This state is
+ *		     entered when a port reset request times out. This
+ *		     state is entered from the RESETTING state.
  */
-enum sci_port_states {
-	/**
-	 * This state indicates that the port has successfully been stopped.
-	 * In this state no new IO operations are permitted.
-	 * This state is entered from the STOPPING state.
-	 */
-	SCI_PORT_STOPPED,
-
-	/**
-	 * This state indicates that the port is in the process of stopping.
-	 * In this state no new IO operations are permitted, but existing IO
-	 * operations are allowed to complete.
-	 * This state is entered from the READY state.
-	 */
-	SCI_PORT_STOPPING,
-
-	/**
-	 * This state indicates the port is now ready.  Thus, the user is
-	 * able to perform IO operations on this port.
-	 * This state is entered from the STARTING state.
-	 */
-	SCI_PORT_READY,
-
-	/**
-	 * The substate where the port is started and ready but has no
-	 * active phys.
-	 */
-	SCI_PORT_SUB_WAITING,
-
-	/**
-	 * The substate where the port is started and ready and there is
-	 * at least one phy operational.
-	 */
-	SCI_PORT_SUB_OPERATIONAL,
-
-	/**
-	 * The substate where the port is started and there was an
-	 * add/remove phy event.  This state is only used in Automatic
-	 * Port Configuration Mode (APC)
-	 */
-	SCI_PORT_SUB_CONFIGURING,
-
-	/**
-	 * This state indicates the port is in the process of performing a hard
-	 * reset.  Thus, the user is unable to perform IO operations on this
-	 * port.
-	 * This state is entered from the READY state.
-	 */
-	SCI_PORT_RESETTING,
-
-	/**
-	 * This state indicates the port has failed a reset request.  This state
-	 * is entered when a port reset request times out.
-	 * This state is entered from the RESETTING state.
-	 */
-	SCI_PORT_FAILED,
-
-
-};
+#define PORT_STATES {\
+	C(PORT_STOPPED),\
+	C(PORT_STOPPING),\
+	C(PORT_READY),\
+	C(PORT_SUB_WAITING),\
+	C(PORT_SUB_OPERATIONAL),\
+	C(PORT_SUB_CONFIGURING),\
+	C(PORT_RESETTING),\
+	C(PORT_FAILED),\
+	}
+#undef C
+#define C(a) SCI_##a
+enum sci_port_states PORT_STATES;
+#undef C
 
 static inline void sci_port_decrement_request_count(struct isci_port *iport)
 {
diff --git a/drivers/scsi/isci/remote_device.c b/drivers/scsi/isci/remote_device.c
index 934d21e..8f501b0 100644
--- a/drivers/scsi/isci/remote_device.c
+++ b/drivers/scsi/isci/remote_device.c
@@ -62,6 +62,16 @@
 #include "scu_event_codes.h"
 #include "task.h"
 
+#undef C
+#define C(a) (#a)
+const char *dev_state_name(enum sci_remote_device_states state)
+{
+	static const char * const strings[] = REMOTE_DEV_STATES;
+
+	return strings[state];
+}
+#undef C
+
 /**
  * isci_remote_device_not_ready() - This function is called by the ihost when
  *    the remote device is not ready. We mark the isci device as ready (not
@@ -167,8 +177,8 @@ enum sci_status sci_remote_device_stop(struct isci_remote_device *idev,
 	case SCI_DEV_FAILED:
 	case SCI_DEV_FINAL:
 	default:
-		dev_warn(scirdev_to_dev(idev), "%s: in wrong state: %d\n",
-			 __func__, state);
+		dev_warn(scirdev_to_dev(idev), "%s: in wrong state: %s\n",
+			 __func__, dev_state_name(state));
 		return SCI_FAILURE_INVALID_STATE;
 	case SCI_DEV_STOPPED:
 		return SCI_SUCCESS;
@@ -226,8 +236,8 @@ enum sci_status sci_remote_device_reset(struct isci_remote_device *idev)
 	case SCI_DEV_RESETTING:
 	case SCI_DEV_FINAL:
 	default:
-		dev_warn(scirdev_to_dev(idev), "%s: in wrong state: %d\n",
-			 __func__, state);
+		dev_warn(scirdev_to_dev(idev), "%s: in wrong state: %s\n",
+			 __func__, dev_state_name(state));
 		return SCI_FAILURE_INVALID_STATE;
 	case SCI_DEV_READY:
 	case SCI_STP_DEV_IDLE:
@@ -246,8 +256,8 @@ enum sci_status sci_remote_device_reset_complete(struct isci_remote_device *idev
 	enum sci_remote_device_states state = sm->current_state_id;
 
 	if (state != SCI_DEV_RESETTING) {
-		dev_warn(scirdev_to_dev(idev), "%s: in wrong state: %d\n",
-			 __func__, state);
+		dev_warn(scirdev_to_dev(idev), "%s: in wrong state: %s\n",
+			 __func__, dev_state_name(state));
 		return SCI_FAILURE_INVALID_STATE;
 	}
 
@@ -262,8 +272,8 @@ enum sci_status sci_remote_device_suspend(struct isci_remote_device *idev,
 	enum sci_remote_device_states state = sm->current_state_id;
 
 	if (state != SCI_STP_DEV_CMD) {
-		dev_warn(scirdev_to_dev(idev), "%s: in wrong state: %d\n",
-			 __func__, state);
+		dev_warn(scirdev_to_dev(idev), "%s: in wrong state: %s\n",
+			 __func__, dev_state_name(state));
 		return SCI_FAILURE_INVALID_STATE;
 	}
 
@@ -287,8 +297,8 @@ enum sci_status sci_remote_device_frame_handler(struct isci_remote_device *idev,
 	case SCI_SMP_DEV_IDLE:
 	case SCI_DEV_FINAL:
 	default:
-		dev_warn(scirdev_to_dev(idev), "%s: in wrong state: %d\n",
-			 __func__, state);
+		dev_warn(scirdev_to_dev(idev), "%s: in wrong state: %s\n",
+			 __func__, dev_state_name(state));
 		/* Return the frame back to the controller */
 		sci_controller_release_frame(ihost, frame_index);
 		return SCI_FAILURE_INVALID_STATE;
@@ -502,8 +512,8 @@ enum sci_status sci_remote_device_start_io(struct isci_host *ihost,
 	case SCI_DEV_RESETTING:
 	case SCI_DEV_FINAL:
 	default:
-		dev_warn(scirdev_to_dev(idev), "%s: in wrong state: %d\n",
-			 __func__, state);
+		dev_warn(scirdev_to_dev(idev), "%s: in wrong state: %s\n",
+			 __func__, dev_state_name(state));
 		return SCI_FAILURE_INVALID_STATE;
 	case SCI_DEV_READY:
 		/* attempt to start an io request for this device object. The remote
@@ -637,8 +647,8 @@ enum sci_status sci_remote_device_complete_io(struct isci_host *ihost,
 	case SCI_DEV_FAILED:
 	case SCI_DEV_FINAL:
 	default:
-		dev_warn(scirdev_to_dev(idev), "%s: in wrong state: %d\n",
-			 __func__, state);
+		dev_warn(scirdev_to_dev(idev), "%s: in wrong state: %s\n",
+			 __func__, dev_state_name(state));
 		return SCI_FAILURE_INVALID_STATE;
 	case SCI_DEV_READY:
 	case SCI_STP_DEV_AWAIT_RESET:
@@ -721,8 +731,8 @@ enum sci_status sci_remote_device_start_task(struct isci_host *ihost,
 	case SCI_DEV_RESETTING:
 	case SCI_DEV_FINAL:
 	default:
-		dev_warn(scirdev_to_dev(idev), "%s: in wrong state: %d\n",
-			 __func__, state);
+		dev_warn(scirdev_to_dev(idev), "%s: in wrong state: %s\n",
+			 __func__, dev_state_name(state));
 		return SCI_FAILURE_INVALID_STATE;
 	case SCI_STP_DEV_IDLE:
 	case SCI_STP_DEV_CMD:
@@ -853,8 +863,8 @@ static enum sci_status sci_remote_device_destruct(struct isci_remote_device *ide
 	struct isci_host *ihost;
 
 	if (state != SCI_DEV_STOPPED) {
-		dev_warn(scirdev_to_dev(idev), "%s: in wrong state: %d\n",
-			 __func__, state);
+		dev_warn(scirdev_to_dev(idev), "%s: in wrong state: %s\n",
+			 __func__, dev_state_name(state));
 		return SCI_FAILURE_INVALID_STATE;
 	}
 
@@ -1204,8 +1214,8 @@ static enum sci_status sci_remote_device_start(struct isci_remote_device *idev,
 	enum sci_status status;
 
 	if (state != SCI_DEV_STOPPED) {
-		dev_warn(scirdev_to_dev(idev), "%s: in wrong state: %d\n",
-			 __func__, state);
+		dev_warn(scirdev_to_dev(idev), "%s: in wrong state: %s\n",
+			 __func__, dev_state_name(state));
 		return SCI_FAILURE_INVALID_STATE;
 	}
 
diff --git a/drivers/scsi/isci/remote_device.h b/drivers/scsi/isci/remote_device.h
index 98c2801..58637ee 100644
--- a/drivers/scsi/isci/remote_device.h
+++ b/drivers/scsi/isci/remote_device.h
@@ -179,122 +179,101 @@ enum sci_status sci_remote_device_reset_complete(
 /**
  * enum sci_remote_device_states - This enumeration depicts all the states
  *    for the common remote device state machine.
+ * @SCI_DEV_INITIAL: Simply the initial state for the base remote device
+ * state machine.
  *
+ * @SCI_DEV_STOPPED: This state indicates that the remote device has
+ * successfully been stopped.  In this state no new IO operations are
+ * permitted.  This state is entered from the INITIAL state.  This state
+ * is entered from the STOPPING state.
  *
+ * @SCI_DEV_STARTING: This state indicates the the remote device is in
+ * the process of becoming ready (i.e. starting).  In this state no new
+ * IO operations are permitted.  This state is entered from the STOPPED
+ * state.
+ *
+ * @SCI_DEV_READY: This state indicates the remote device is now ready.
+ * Thus, the user is able to perform IO operations on the remote device.
+ * This state is entered from the STARTING state.
+ *
+ * @SCI_STP_DEV_IDLE: This is the idle substate for the stp remote
+ * device.  When there are no active IO for the device it is is in this
+ * state.
+ *
+ * @SCI_STP_DEV_CMD: This is the command state for for the STP remote
+ * device.  This state is entered when the device is processing a
+ * non-NCQ command.  The device object will fail any new start IO
+ * requests until this command is complete.
+ *
+ * @SCI_STP_DEV_NCQ: This is the NCQ state for the STP remote device.
+ * This state is entered when the device is processing an NCQ reuqest.
+ * It will remain in this state so long as there is one or more NCQ
+ * requests being processed.
+ *
+ * @SCI_STP_DEV_NCQ_ERROR: This is the NCQ error state for the STP
+ * remote device.  This state is entered when an SDB error FIS is
+ * received by the device object while in the NCQ state.  The device
+ * object will only accept a READ LOG command while in this state.
+ *
+ * @SCI_STP_DEV_ATAPI_ERROR: This is the ATAPI error state for the STP
+ * ATAPI remote device.  This state is entered when ATAPI device sends
+ * error status FIS without data while the device object is in CMD
+ * state.  A suspension event is expected in this state.  The device
+ * object will resume right away.
+ *
+ * @SCI_STP_DEV_AWAIT_RESET: This is the READY substate indicates the
+ * device is waiting for the RESET task coming to be recovered from
+ * certain hardware specific error.
+ *
+ * @SCI_SMP_DEV_IDLE: This is the ready operational substate for the
+ * remote device.  This is the normal operational state for a remote
+ * device.
+ *
+ * @SCI_SMP_DEV_CMD: This is the suspended state for the remote device.
+ * This is the state that the device is placed in when a RNC suspend is
+ * received by the SCU hardware.
+ *
+ * @SCI_DEV_STOPPING: This state indicates that the remote device is in
+ * the process of stopping.  In this state no new IO operations are
+ * permitted, but existing IO operations are allowed to complete.  This
+ * state is entered from the READY state.  This state is entered from
+ * the FAILED state.
+ *
+ * @SCI_DEV_FAILED: This state indicates that the remote device has
+ * failed.  In this state no new IO operations are permitted.  This
+ * state is entered from the INITIALIZING state.  This state is entered
+ * from the READY state.
+ *
+ * @SCI_DEV_RESETTING: This state indicates the device is being reset.
+ * In this state no new IO operations are permitted.  This state is
+ * entered from the READY state.
+ *
+ * @SCI_DEV_FINAL: Simply the final state for the base remote device
+ * state machine.
  */
-enum sci_remote_device_states {
-	/**
-	 * Simply the initial state for the base remote device state machine.
-	 */
-	SCI_DEV_INITIAL,
-
-	/**
-	 * This state indicates that the remote device has successfully been
-	 * stopped.  In this state no new IO operations are permitted.
-	 * This state is entered from the INITIAL state.
-	 * This state is entered from the STOPPING state.
-	 */
-	SCI_DEV_STOPPED,
-
-	/**
-	 * This state indicates the the remote device is in the process of
-	 * becoming ready (i.e. starting).  In this state no new IO operations
-	 * are permitted.
-	 * This state is entered from the STOPPED state.
-	 */
-	SCI_DEV_STARTING,
-
-	/**
-	 * This state indicates the remote device is now ready.  Thus, the user
-	 * is able to perform IO operations on the remote device.
-	 * This state is entered from the STARTING state.
-	 */
-	SCI_DEV_READY,
-
-	/**
-	 * This is the idle substate for the stp remote device.  When there are no
-	 * active IO for the device it is is in this state.
-	 */
-	SCI_STP_DEV_IDLE,
-
-	/**
-	 * This is the command state for for the STP remote device.  This state is
-	 * entered when the device is processing a non-NCQ command.  The device object
-	 * will fail any new start IO requests until this command is complete.
-	 */
-	SCI_STP_DEV_CMD,
-
-	/**
-	 * This is the NCQ state for the STP remote device.  This state is entered
-	 * when the device is processing an NCQ reuqest.  It will remain in this state
-	 * so long as there is one or more NCQ requests being processed.
-	 */
-	SCI_STP_DEV_NCQ,
-
-	/**
-	 * This is the NCQ error state for the STP remote device.  This state is
-	 * entered when an SDB error FIS is received by the device object while in the
-	 * NCQ state.  The device object will only accept a READ LOG command while in
-	 * this state.
-	 */
-	SCI_STP_DEV_NCQ_ERROR,
-
-	/**
-	 * This is the ATAPI error state for the STP ATAPI remote device.
-	 * This state is entered when ATAPI device sends error status FIS
-	 * without data while the device object is in CMD state.
-	 * A suspension event is expected in this state.
-	 * The device object will resume right away.
-	 */
-	SCI_STP_DEV_ATAPI_ERROR,
-
-	/**
-	 * This is the READY substate indicates the device is waiting for the RESET task
-	 * coming to be recovered from certain hardware specific error.
-	 */
-	SCI_STP_DEV_AWAIT_RESET,
-
-	/**
-	 * This is the ready operational substate for the remote device.  This is the
-	 * normal operational state for a remote device.
-	 */
-	SCI_SMP_DEV_IDLE,
-
-	/**
-	 * This is the suspended state for the remote device.  This is the state that
-	 * the device is placed in when a RNC suspend is received by the SCU hardware.
-	 */
-	SCI_SMP_DEV_CMD,
-
-	/**
-	 * This state indicates that the remote device is in the process of
-	 * stopping.  In this state no new IO operations are permitted, but
-	 * existing IO operations are allowed to complete.
-	 * This state is entered from the READY state.
-	 * This state is entered from the FAILED state.
-	 */
-	SCI_DEV_STOPPING,
-
-	/**
-	 * This state indicates that the remote device has failed.
-	 * In this state no new IO operations are permitted.
-	 * This state is entered from the INITIALIZING state.
-	 * This state is entered from the READY state.
-	 */
-	SCI_DEV_FAILED,
-
-	/**
-	 * This state indicates the device is being reset.
-	 * In this state no new IO operations are permitted.
-	 * This state is entered from the READY state.
-	 */
-	SCI_DEV_RESETTING,
-
-	/**
-	 * Simply the final state for the base remote device state machine.
-	 */
-	SCI_DEV_FINAL,
-};
+#define REMOTE_DEV_STATES {\
+	C(DEV_INITIAL),\
+	C(DEV_STOPPED),\
+	C(DEV_STARTING),\
+	C(DEV_READY),\
+	C(STP_DEV_IDLE),\
+	C(STP_DEV_CMD),\
+	C(STP_DEV_NCQ),\
+	C(STP_DEV_NCQ_ERROR),\
+	C(STP_DEV_ATAPI_ERROR),\
+	C(STP_DEV_AWAIT_RESET),\
+	C(SMP_DEV_IDLE),\
+	C(SMP_DEV_CMD),\
+	C(DEV_STOPPING),\
+	C(DEV_FAILED),\
+	C(DEV_RESETTING),\
+	C(DEV_FINAL),\
+	}
+#undef C
+#define C(a) SCI_##a
+enum sci_remote_device_states REMOTE_DEV_STATES;
+#undef C
+const char *dev_state_name(enum sci_remote_device_states state);
 
 static inline struct isci_remote_device *rnc_to_dev(struct sci_remote_node_context *rnc)
 {
diff --git a/drivers/scsi/isci/remote_node_context.c b/drivers/scsi/isci/remote_node_context.c
index 748e833..3a94634 100644
--- a/drivers/scsi/isci/remote_node_context.c
+++ b/drivers/scsi/isci/remote_node_context.c
@@ -60,18 +60,15 @@
 #include "scu_event_codes.h"
 #include "scu_task_context.h"
 
+#undef C
+#define C(a) (#a)
+const char *rnc_state_name(enum scis_sds_remote_node_context_states state)
+{
+	static const char * const strings[] = RNC_STATES;
 
-/**
- *
- * @sci_rnc: The RNC for which the is posted request is being made.
- *
- * This method will return true if the RNC is not in the initial state.  In all
- * other states the RNC is considered active and this will return true. The
- * destroy request of the state machine drives the RNC back to the initial
- * state.  If the state machine changes then this routine will also have to be
- * changed. bool true if the state machine is not in the initial state false if
- * the state machine is in the initial state
- */
+	return strings[state];
+}
+#undef C
 
 /**
  *
diff --git a/drivers/scsi/isci/remote_node_context.h b/drivers/scsi/isci/remote_node_context.h
index 41580ad..a241e0f 100644
--- a/drivers/scsi/isci/remote_node_context.h
+++ b/drivers/scsi/isci/remote_node_context.h
@@ -85,61 +85,50 @@ struct sci_remote_node_context;
 typedef void (*scics_sds_remote_node_context_callback)(void *);
 
 /**
- * This is the enumeration of the remote node context states.
+ * enum sci_remote_node_context_states
+ * @SCI_RNC_INITIAL initial state for a remote node context.  On a resume
+ * request the remote node context will transition to the posting state.
+ *
+ * @SCI_RNC_POSTING: transition state that posts the RNi to the hardware. Once
+ * the RNC is posted the remote node context will be made ready.
+ *
+ * @SCI_RNC_INVALIDATING: transition state that will post an RNC invalidate to
+ * the hardware.  Once the invalidate is complete the remote node context will
+ * transition to the posting state.
+ *
+ * @SCI_RNC_RESUMING: transition state that will post an RNC resume to the
+ * hardare.  Once the event notification of resume complete is received the
+ * remote node context will transition to the ready state.
+ *
+ * @SCI_RNC_READY: state that the remote node context must be in to accept io
+ * request operations.
+ *
+ * @SCI_RNC_TX_SUSPENDED: state that the remote node context transitions to when
+ * it gets a TX suspend notification from the hardware.
+ *
+ * @SCI_RNC_TX_RX_SUSPENDED: state that the remote node context transitions to
+ * when it gets a TX RX suspend notification from the hardware.
+ *
+ * @SCI_RNC_AWAIT_SUSPENSION: wait state for the remote node context that waits
+ * for a suspend notification from the hardware.  This state is entered when
+ * either there is a request to supend the remote node context or when there is
+ * a TC completion where the remote node will be suspended by the hardware.
  */
-enum scis_sds_remote_node_context_states {
-	/**
-	 * This state is the initial state for a remote node context.  On a resume
-	 * request the remote node context will transition to the posting state.
-	 */
-	SCI_RNC_INITIAL,
-
-	/**
-	 * This is a transition state that posts the RNi to the hardware. Once the RNC
-	 * is posted the remote node context will be made ready.
-	 */
-	SCI_RNC_POSTING,
-
-	/**
-	 * This is a transition state that will post an RNC invalidate to the
-	 * hardware.  Once the invalidate is complete the remote node context will
-	 * transition to the posting state.
-	 */
-	SCI_RNC_INVALIDATING,
-
-	/**
-	 * This is a transition state that will post an RNC resume to the hardare.
-	 * Once the event notification of resume complete is received the remote node
-	 * context will transition to the ready state.
-	 */
-	SCI_RNC_RESUMING,
-
-	/**
-	 * This is the state that the remote node context must be in to accept io
-	 * request operations.
-	 */
-	SCI_RNC_READY,
-
-	/**
-	 * This is the state that the remote node context transitions to when it gets
-	 * a TX suspend notification from the hardware.
-	 */
-	SCI_RNC_TX_SUSPENDED,
-
-	/**
-	 * This is the state that the remote node context transitions to when it gets
-	 * a TX RX suspend notification from the hardware.
-	 */
-	SCI_RNC_TX_RX_SUSPENDED,
-
-	/**
-	 * This state is a wait state for the remote node context that waits for a
-	 * suspend notification from the hardware.  This state is entered when either
-	 * there is a request to supend the remote node context or when there is a TC
-	 * completion where the remote node will be suspended by the hardware.
-	 */
-	SCI_RNC_AWAIT_SUSPENSION
-};
+#define RNC_STATES {\
+	C(RNC_INITIAL),\
+	C(RNC_POSTING),\
+	C(RNC_INVALIDATING),\
+	C(RNC_RESUMING),\
+	C(RNC_READY),\
+	C(RNC_TX_SUSPENDED),\
+	C(RNC_TX_RX_SUSPENDED),\
+	C(RNC_AWAIT_SUSPENSION),\
+	}
+#undef C
+#define C(a) SCI_##a
+enum scis_sds_remote_node_context_states RNC_STATES;
+#undef C
+const char *rnc_state_name(enum scis_sds_remote_node_context_states state);
 
 /**
  *
diff --git a/drivers/scsi/isci/request.c b/drivers/scsi/isci/request.c
index 1a39ce5..ab18ee0fe 100644
--- a/drivers/scsi/isci/request.c
+++ b/drivers/scsi/isci/request.c
@@ -61,6 +61,16 @@
 #include "scu_event_codes.h"
 #include "sas.h"
 
+#undef C
+#define C(a) (#a)
+const char *req_state_name(enum sci_base_request_states state)
+{
+	static const char * const strings[] = REQUEST_STATES;
+
+	return strings[state];
+}
+#undef C
+
 static struct scu_sgl_element_pair *to_sgl_element_pair(struct isci_request *ireq,
 							int idx)
 {
@@ -910,7 +920,8 @@ enum sci_status sci_request_complete(struct isci_request *ireq)
 
 	state = ireq->sm.current_state_id;
 	if (WARN_ONCE(state != SCI_REQ_COMPLETED,
-		      "isci: request completion from wrong state (%d)\n", state))
+		      "isci: request completion from wrong state (%s)\n",
+		      req_state_name(state)))
 		return SCI_FAILURE_INVALID_STATE;
 
 	if (ireq->saved_rx_frame_index != SCU_INVALID_FRAME_INDEX)
@@ -931,8 +942,8 @@ enum sci_status sci_io_request_event_handler(struct isci_request *ireq,
 	state = ireq->sm.current_state_id;
 
 	if (state != SCI_REQ_STP_PIO_DATA_IN) {
-		dev_warn(&ihost->pdev->dev, "%s: (%x) in wrong state %d\n",
-			 __func__, event_code, state);
+		dev_warn(&ihost->pdev->dev, "%s: (%x) in wrong state %s\n",
+			 __func__, event_code, req_state_name(state));
 
 		return SCI_FAILURE_INVALID_STATE;
 	}
@@ -2306,12 +2317,8 @@ sci_io_request_tc_completion(struct isci_request *ireq,
 		return atapi_data_tc_completion_handler(ireq, completion_code);
 
 	default:
-		dev_warn(&ihost->pdev->dev,
-			 "%s: SCIC IO Request given task completion "
-			 "notification %x while in wrong state %d\n",
-			 __func__,
-			 completion_code,
-			 state);
+		dev_warn(&ihost->pdev->dev, "%s: %x in wrong state %s\n",
+			 __func__, completion_code, req_state_name(state));
 		return SCI_FAILURE_INVALID_STATE;
 	}
 }
diff --git a/drivers/scsi/isci/request.h b/drivers/scsi/isci/request.h
index bcf2f37..057f237 100644
--- a/drivers/scsi/isci/request.h
+++ b/drivers/scsi/isci/request.h
@@ -182,134 +182,103 @@ static inline struct isci_request *to_ireq(struct isci_stp_request *stp_req)
 }
 
 /**
- * enum sci_base_request_states - This enumeration depicts all the states for
- *    the common request state machine.
+ * enum sci_base_request_states - request state machine states
  *
+ * @SCI_REQ_INIT: Simply the initial state for the base request state machine.
  *
+ * @SCI_REQ_CONSTRUCTED: This state indicates that the request has been
+ * constructed.  This state is entered from the INITIAL state.
+ *
+ * @SCI_REQ_STARTED: This state indicates that the request has been started.
+ * This state is entered from the CONSTRUCTED state.
+ *
+ * @SCI_REQ_STP_UDMA_WAIT_TC_COMP:
+ * @SCI_REQ_STP_UDMA_WAIT_D2H:
+ * @SCI_REQ_STP_NON_DATA_WAIT_H2D:
+ * @SCI_REQ_STP_NON_DATA_WAIT_D2H:
+ *
+ * @SCI_REQ_STP_PIO_WAIT_H2D: While in this state the IO request object is
+ * waiting for the TC completion notification for the H2D Register FIS
+ *
+ * @SCI_REQ_STP_PIO_WAIT_FRAME: While in this state the IO request object is
+ * waiting for either a PIO Setup FIS or a D2H register FIS.  The type of frame
+ * received is based on the result of the prior frame and line conditions.
+ *
+ * @SCI_REQ_STP_PIO_DATA_IN: While in this state the IO request object is
+ * waiting for a DATA frame from the device.
+ *
+ * @SCI_REQ_STP_PIO_DATA_OUT: While in this state the IO request object is
+ * waiting to transmit the next data frame to the device.
+ *
+ * @SCI_REQ_ATAPI_WAIT_H2D: While in this state the IO request object is
+ * waiting for the TC completion notification for the H2D Register FIS
+ *
+ * @SCI_REQ_ATAPI_WAIT_PIO_SETUP: While in this state the IO request object is
+ * waiting for either a PIO Setup.
+ *
+ * @SCI_REQ_ATAPI_WAIT_D2H: The non-data IO transit to this state in this state
+ * after receiving TC completion. While in this state IO request object is
+ * waiting for D2H status frame as UF.
+ *
+ * @SCI_REQ_ATAPI_WAIT_TC_COMP: When transmitting raw frames hardware reports
+ * task context completion after every frame submission, so in the
+ * non-accelerated case we need to expect the completion for the "cdb" frame.
+ *
+ * @SCI_REQ_TASK_WAIT_TC_COMP: The AWAIT_TC_COMPLETION sub-state indicates that
+ * the started raw task management request is waiting for the transmission of
+ * the initial frame (i.e. command, task, etc.).
+ *
+ * @SCI_REQ_TASK_WAIT_TC_RESP: This sub-state indicates that the started task
+ * management request is waiting for the reception of an unsolicited frame
+ * (i.e.  response IU).
+ *
+ * @SCI_REQ_SMP_WAIT_RESP: This sub-state indicates that the started task
+ * management request is waiting for the reception of an unsolicited frame
+ * (i.e.  response IU).
+ *
+ * @SCI_REQ_SMP_WAIT_TC_COMP: The AWAIT_TC_COMPLETION sub-state indicates that
+ * the started SMP request is waiting for the transmission of the initial frame
+ * (i.e.  command, task, etc.).
+ *
+ * @SCI_REQ_COMPLETED: This state indicates that the request has completed.
+ * This state is entered from the STARTED state. This state is entered from the
+ * ABORTING state.
+ *
+ * @SCI_REQ_ABORTING: This state indicates that the request is in the process
+ * of being terminated/aborted.  This state is entered from the CONSTRUCTED
+ * state.  This state is entered from the STARTED state.
+ *
+ * @SCI_REQ_FINAL: Simply the final state for the base request state machine.
  */
-enum sci_base_request_states {
-	/*
-	 * Simply the initial state for the base request state machine.
-	 */
-	SCI_REQ_INIT,
-
-	/*
-	 * This state indicates that the request has been constructed.
-	 * This state is entered from the INITIAL state.
-	 */
-	SCI_REQ_CONSTRUCTED,
-
-	/*
-	 * This state indicates that the request has been started. This state
-	 * is entered from the CONSTRUCTED state.
-	 */
-	SCI_REQ_STARTED,
-
-	SCI_REQ_STP_UDMA_WAIT_TC_COMP,
-	SCI_REQ_STP_UDMA_WAIT_D2H,
-
-	SCI_REQ_STP_NON_DATA_WAIT_H2D,
-	SCI_REQ_STP_NON_DATA_WAIT_D2H,
-
-	/*
-	 * While in this state the IO request object is waiting for the TC
-	 * completion notification for the H2D Register FIS
-	 */
-	SCI_REQ_STP_PIO_WAIT_H2D,
-
-	/*
-	 * While in this state the IO request object is waiting for either a
-	 * PIO Setup FIS or a D2H register FIS.  The type of frame received is
-	 * based on the result of the prior frame and line conditions.
-	 */
-	SCI_REQ_STP_PIO_WAIT_FRAME,
-
-	/*
-	 * While in this state the IO request object is waiting for a DATA
-	 * frame from the device.
-	 */
-	SCI_REQ_STP_PIO_DATA_IN,
-
-	/*
-	 * While in this state the IO request object is waiting to transmit
-	 * the next data frame to the device.
-	 */
-	SCI_REQ_STP_PIO_DATA_OUT,
-
-	/*
-	 * While in this state the IO request object is waiting for the TC
-	 * completion notification for the H2D Register FIS
-	 */
-	SCI_REQ_ATAPI_WAIT_H2D,
-
-	/*
-	 * While in this state the IO request object is waiting for either a
-	 * PIO Setup.
-	 */
-	SCI_REQ_ATAPI_WAIT_PIO_SETUP,
-
-	/*
-	 * The non-data IO transit to this state in this state after receiving
-	 * TC completion. While in this state IO request object is waiting for
-	 * D2H status frame as UF.
-	 */
-	SCI_REQ_ATAPI_WAIT_D2H,
-
-	/*
-	 * When transmitting raw frames hardware reports task context completion
-	 * after every frame submission, so in the non-accelerated case we need
-	 * to expect the completion for the "cdb" frame.
-	 */
-	SCI_REQ_ATAPI_WAIT_TC_COMP,
-
-	/*
-	 * The AWAIT_TC_COMPLETION sub-state indicates that the started raw
-	 * task management request is waiting for the transmission of the
-	 * initial frame (i.e. command, task, etc.).
-	 */
-	SCI_REQ_TASK_WAIT_TC_COMP,
-
-	/*
-	 * This sub-state indicates that the started task management request
-	 * is waiting for the reception of an unsolicited frame
-	 * (i.e. response IU).
-	 */
-	SCI_REQ_TASK_WAIT_TC_RESP,
-
-	/*
-	 * This sub-state indicates that the started task management request
-	 * is waiting for the reception of an unsolicited frame
-	 * (i.e. response IU).
-	 */
-	SCI_REQ_SMP_WAIT_RESP,
-
-	/*
-	 * The AWAIT_TC_COMPLETION sub-state indicates that the started SMP
-	 * request is waiting for the transmission of the initial frame
-	 * (i.e. command, task, etc.).
-	 */
-	SCI_REQ_SMP_WAIT_TC_COMP,
-
-	/*
-	 * This state indicates that the request has completed.
-	 * This state is entered from the STARTED state. This state is entered
-	 * from the ABORTING state.
-	 */
-	SCI_REQ_COMPLETED,
-
-	/*
-	 * This state indicates that the request is in the process of being
-	 * terminated/aborted.
-	 * This state is entered from the CONSTRUCTED state.
-	 * This state is entered from the STARTED state.
-	 */
-	SCI_REQ_ABORTING,
-
-	/*
-	 * Simply the final state for the base request state machine.
-	 */
-	SCI_REQ_FINAL,
-};
+#define REQUEST_STATES {\
+	C(REQ_INIT),\
+	C(REQ_CONSTRUCTED),\
+	C(REQ_STARTED),\
+	C(REQ_STP_UDMA_WAIT_TC_COMP),\
+	C(REQ_STP_UDMA_WAIT_D2H),\
+	C(REQ_STP_NON_DATA_WAIT_H2D),\
+	C(REQ_STP_NON_DATA_WAIT_D2H),\
+	C(REQ_STP_PIO_WAIT_H2D),\
+	C(REQ_STP_PIO_WAIT_FRAME),\
+	C(REQ_STP_PIO_DATA_IN),\
+	C(REQ_STP_PIO_DATA_OUT),\
+	C(REQ_ATAPI_WAIT_H2D),\
+	C(REQ_ATAPI_WAIT_PIO_SETUP),\
+	C(REQ_ATAPI_WAIT_D2H),\
+	C(REQ_ATAPI_WAIT_TC_COMP),\
+	C(REQ_TASK_WAIT_TC_COMP),\
+	C(REQ_TASK_WAIT_TC_RESP),\
+	C(REQ_SMP_WAIT_RESP),\
+	C(REQ_SMP_WAIT_TC_COMP),\
+	C(REQ_COMPLETED),\
+	C(REQ_ABORTING),\
+	C(REQ_FINAL),\
+	}
+#undef C
+#define C(a) SCI_##a
+enum sci_base_request_states REQUEST_STATES;
+#undef C
+const char *req_state_name(enum sci_base_request_states state);
 
 enum sci_status sci_request_start(struct isci_request *ireq);
 enum sci_status sci_io_request_terminate(struct isci_request *ireq);


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v1 4/5] isci: improve phy event warnings
  2012-02-10  9:18 [PATCH v1 0/5] isci updates for -next Dan Williams
                   ` (2 preceding siblings ...)
  2012-02-10  9:18 ` [PATCH v1 3/5] isci: debug, provide state-enum-to-string conversions Dan Williams
@ 2012-02-10  9:18 ` Dan Williams
  2012-02-10  9:18 ` [PATCH v1 5/5] isci: improvements in driver unloading routine Dan Williams
  4 siblings, 0 replies; 8+ messages in thread
From: Dan Williams @ 2012-02-10  9:18 UTC (permalink / raw)
  To: linux-scsi

isci occasionally spews messages like:

isci 0000:03:00.0: sci_phy_event_handler: PHY starting substate machine received unexpected event_code b3940000

...which is not very helpful, since we don't know which controller,
which phy, the exact state, or a decode of the event.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/scsi/isci/phy.c |  123 +++++++++++++++++++++++++++--------------------
 1 files changed, 71 insertions(+), 52 deletions(-)

diff --git a/drivers/scsi/isci/phy.c b/drivers/scsi/isci/phy.c
index 87064f1..fab3586 100644
--- a/drivers/scsi/isci/phy.c
+++ b/drivers/scsi/isci/phy.c
@@ -77,12 +77,17 @@ enum sas_linkrate sci_phy_linkrate(struct isci_phy *iphy)
 	return iphy->max_negotiated_speed;
 }
 
-static struct device *sciphy_to_dev(struct isci_phy *iphy)
+static struct isci_host *phy_to_host(struct isci_phy *iphy)
 {
 	struct isci_phy *table = iphy - iphy->phy_index;
 	struct isci_host *ihost = container_of(table, typeof(*ihost), phys[0]);
 
-	return &ihost->pdev->dev;
+	return ihost;
+}
+
+static struct device *sciphy_to_dev(struct isci_phy *iphy)
+{
+	return &phy_to_host(iphy)->pdev->dev;
 }
 
 static enum sci_status
@@ -609,6 +614,60 @@ static void sci_phy_complete_link_training(struct isci_phy *iphy,
 	sci_change_state(&iphy->sm, next_state);
 }
 
+static const char *phy_event_name(u32 event_code)
+{
+	switch (scu_get_event_code(event_code)) {
+	case SCU_EVENT_PORT_SELECTOR_DETECTED:
+		return "port selector";
+	case SCU_EVENT_SENT_PORT_SELECTION:
+		return "port selection";
+	case SCU_EVENT_HARD_RESET_TRANSMITTED:
+		return "tx hard reset";
+	case SCU_EVENT_HARD_RESET_RECEIVED:
+		return "rx hard reset";
+	case SCU_EVENT_RECEIVED_IDENTIFY_TIMEOUT:
+		return "identify timeout";
+	case SCU_EVENT_LINK_FAILURE:
+		return "link fail";
+	case SCU_EVENT_SATA_SPINUP_HOLD:
+		return "sata spinup hold";
+	case SCU_EVENT_SAS_15_SSC:
+	case SCU_EVENT_SAS_15:
+		return "sas 1.5";
+	case SCU_EVENT_SAS_30_SSC:
+	case SCU_EVENT_SAS_30:
+		return "sas 3.0";
+	case SCU_EVENT_SAS_60_SSC:
+	case SCU_EVENT_SAS_60:
+		return "sas 6.0";
+	case SCU_EVENT_SATA_15_SSC:
+	case SCU_EVENT_SATA_15:
+		return "sata 1.5";
+	case SCU_EVENT_SATA_30_SSC:
+	case SCU_EVENT_SATA_30:
+		return "sata 3.0";
+	case SCU_EVENT_SATA_60_SSC:
+	case SCU_EVENT_SATA_60:
+		return "sata 6.0";
+	case SCU_EVENT_SAS_PHY_DETECTED:
+		return "sas detect";
+	case SCU_EVENT_SATA_PHY_DETECTED:
+		return "sata detect";
+	default:
+		return "unknown";
+	}
+}
+
+#define phy_event_dbg(iphy, state, code) \
+	dev_dbg(sciphy_to_dev(iphy), "phy-%d:%d: %s event: %s (%x)\n", \
+		phy_to_host(iphy)->id, iphy->phy_index, \
+		phy_state_name(state), phy_event_name(code), code)
+
+#define phy_event_warn(iphy, state, code) \
+	dev_warn(sciphy_to_dev(iphy), "phy-%d:%d: %s event: %s (%x)\n", \
+		phy_to_host(iphy)->id, iphy->phy_index, \
+		phy_state_name(state), phy_event_name(code), code)
+
 enum sci_status sci_phy_event_handler(struct isci_phy *iphy, u32 event_code)
 {
 	enum sci_phy_states state = iphy->sm.current_state_id;
@@ -625,11 +684,7 @@ enum sci_status sci_phy_event_handler(struct isci_phy *iphy, u32 event_code)
 			iphy->is_in_link_training = true;
 			break;
 		default:
-			dev_dbg(sciphy_to_dev(iphy),
-				"%s: PHY starting substate machine received "
-				"unexpected event_code %x\n",
-				__func__,
-				event_code);
+			phy_event_dbg(iphy, state, event_code);
 			return SCI_FAILURE;
 		}
 		return SCI_SUCCESS;
@@ -666,11 +721,7 @@ enum sci_status sci_phy_event_handler(struct isci_phy *iphy, u32 event_code)
 			sci_change_state(&iphy->sm, SCI_PHY_STARTING);
 			break;
 		default:
-			dev_warn(sciphy_to_dev(iphy),
-				 "%s: PHY starting substate machine received "
-				 "unexpected event_code %x\n",
-				 __func__, event_code);
-
+			phy_event_warn(iphy, state, event_code);
 			return SCI_FAILURE;
 			break;
 		}
@@ -695,10 +746,7 @@ enum sci_status sci_phy_event_handler(struct isci_phy *iphy, u32 event_code)
 			sci_change_state(&iphy->sm, SCI_PHY_STARTING);
 			break;
 		default:
-			dev_warn(sciphy_to_dev(iphy),
-				 "%s: PHY starting substate machine received "
-				 "unexpected event_code %x\n",
-				 __func__, event_code);
+			phy_event_warn(iphy, state, event_code);
 			return SCI_FAILURE;
 		}
 		return SCI_SUCCESS;
@@ -709,11 +757,7 @@ enum sci_status sci_phy_event_handler(struct isci_phy *iphy, u32 event_code)
 			sci_change_state(&iphy->sm, SCI_PHY_STARTING);
 			break;
 		default:
-			dev_warn(sciphy_to_dev(iphy),
-				"%s: PHY starting substate machine received unexpected "
-				"event_code %x\n",
-				__func__,
-				event_code);
+			phy_event_warn(iphy, state, event_code);
 			return SCI_FAILURE;
 		}
 		return SCI_SUCCESS;
@@ -737,11 +781,7 @@ enum sci_status sci_phy_event_handler(struct isci_phy *iphy, u32 event_code)
 			break;
 
 		default:
-			dev_warn(sciphy_to_dev(iphy),
-				 "%s: PHY starting substate machine received "
-				 "unexpected event_code %x\n",
-				 __func__, event_code);
-
+			phy_event_warn(iphy, state, event_code);
 			return SCI_FAILURE;
 		}
 		return SCI_SUCCESS;
@@ -769,12 +809,7 @@ enum sci_status sci_phy_event_handler(struct isci_phy *iphy, u32 event_code)
 			sci_phy_start_sas_link_training(iphy);
 			break;
 		default:
-			dev_warn(sciphy_to_dev(iphy),
-				 "%s: PHY starting substate machine received "
-				 "unexpected event_code %x\n",
-				 __func__,
-				 event_code);
-
+			phy_event_warn(iphy, state, event_code);
 			return SCI_FAILURE;
 		}
 		return SCI_SUCCESS;
@@ -811,11 +846,7 @@ enum sci_status sci_phy_event_handler(struct isci_phy *iphy, u32 event_code)
 			sci_phy_start_sas_link_training(iphy);
 			break;
 		default:
-			dev_warn(sciphy_to_dev(iphy),
-				 "%s: PHY starting substate machine received "
-				 "unexpected event_code %x\n",
-				 __func__, event_code);
-
+			phy_event_warn(iphy, state, event_code);
 			return SCI_FAILURE;
 		}
 
@@ -833,12 +864,7 @@ enum sci_status sci_phy_event_handler(struct isci_phy *iphy, u32 event_code)
 			break;
 
 		default:
-			dev_warn(sciphy_to_dev(iphy),
-				 "%s: PHY starting substate machine received "
-				 "unexpected event_code %x\n",
-				 __func__,
-				 event_code);
-
+			phy_event_warn(iphy, state, event_code);
 			return SCI_FAILURE;
 		}
 		return SCI_SUCCESS;
@@ -856,10 +882,7 @@ enum sci_status sci_phy_event_handler(struct isci_phy *iphy, u32 event_code)
 				iphy->bcn_received_while_port_unassigned = true;
 			break;
 		default:
-			dev_warn(sciphy_to_dev(iphy),
-				 "%sP SCIC PHY 0x%p ready state machine received "
-				 "unexpected event_code %x\n",
-				 __func__, iphy, event_code);
+			phy_event_warn(iphy, state, event_code);
 			return SCI_FAILURE_INVALID_STATE;
 		}
 		return SCI_SUCCESS;
@@ -870,11 +893,7 @@ enum sci_status sci_phy_event_handler(struct isci_phy *iphy, u32 event_code)
 			sci_change_state(&iphy->sm, SCI_PHY_STARTING);
 			break;
 		default:
-			dev_warn(sciphy_to_dev(iphy),
-				 "%s: SCIC PHY 0x%p resetting state machine received "
-				 "unexpected event_code %x\n",
-				 __func__, iphy, event_code);
-
+			phy_event_warn(iphy, state, event_code);
 			return SCI_FAILURE_INVALID_STATE;
 			break;
 		}


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v1 5/5] isci: improvements in driver unloading routine
  2012-02-10  9:18 [PATCH v1 0/5] isci updates for -next Dan Williams
                   ` (3 preceding siblings ...)
  2012-02-10  9:18 ` [PATCH v1 4/5] isci: improve phy event warnings Dan Williams
@ 2012-02-10  9:18 ` Dan Williams
  4 siblings, 0 replies; 8+ messages in thread
From: Dan Williams @ 2012-02-10  9:18 UTC (permalink / raw)
  To: linux-scsi; +Cc: Andrzej Jakowski

From: Andrzej Jakowski <andrzej.jakowski@intel.com>

This patch fixes scenario where driver removal should be possible
only when driver is in READY state. Also it removes redundant
invocation of routine disabling SCU interrupts - this method is
called somewhere else in driver deinitialization path.

Signed-off-by: Andrzej Jakowski <andrzej.jakowski@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/scsi/isci/init.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/isci/init.c b/drivers/scsi/isci/init.c
index c3fe39b..7c7de43 100644
--- a/drivers/scsi/isci/init.c
+++ b/drivers/scsi/isci/init.c
@@ -60,6 +60,7 @@
 #include <linux/efi.h>
 #include <asm/string.h>
 #include <scsi/scsi_host.h>
+#include "host.h"
 #include "isci.h"
 #include "task.h"
 #include "probe_roms.h"
@@ -553,9 +554,9 @@ static void __devexit isci_pci_remove(struct pci_dev *pdev)
 	int i;
 
 	for_each_isci_host(i, ihost, pdev) {
+		wait_for_start(ihost);
 		isci_unregister(ihost);
 		isci_host_deinit(ihost);
-		sci_controller_disable_interrupts(ihost);
 	}
 }
 


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v1 1/5] isci: T10 DIF support
  2012-02-10  9:18 ` [PATCH v1 1/5] isci: T10 DIF support Dan Williams
@ 2012-02-13 17:14   ` Martin K. Petersen
  0 siblings, 0 replies; 8+ messages in thread
From: Martin K. Petersen @ 2012-02-13 17:14 UTC (permalink / raw)
  To: Dan Williams; +Cc: linux-scsi, Dave Jiang, Martin K. Petersen

>>>>> "Dan" == Dan Williams <dan.j.williams@intel.com> writes:

Dan> This allows the controller to do WRITE_INSERT and READ_STRIP for
Dan> SAS disks that support protection information. SAS disks must be
Dan> formatted with protection information to use this feature via
Dan> sg_format.

I have not been able to locate a suitable machine for testing. But the
way this patch hooks into SCSI looks correct.

Acked-by: Martin K. Petersen <martin.petersen@oracle.com>

-- 
Martin K. Petersen	Oracle Linux Engineering

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v1 3/5] isci: debug, provide state-enum-to-string conversions
  2012-02-10  9:18 ` [PATCH v1 3/5] isci: debug, provide state-enum-to-string conversions Dan Williams
@ 2012-02-19 15:17   ` James Bottomley
  0 siblings, 0 replies; 8+ messages in thread
From: James Bottomley @ 2012-02-19 15:17 UTC (permalink / raw)
  To: Dan Williams; +Cc: linux-scsi

On Fri, 2012-02-10 at 01:18 -0800, Dan Williams wrote:
> Debugging the driver requires tracing the state transtions and tracing
> state names is less work than decoding numbers.
> 
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

I've got 1 now and 2 was sent as a previous patch, so I have it.  This
one doesn't apply:

patching file drivers/scsi/isci/phy.c
Hunk #2 succeeded at 456 (offset -8 lines).
Hunk #3 succeeded at 482 (offset -8 lines).
Hunk #4 succeeded at 496 (offset -8 lines).
Hunk #5 succeeded at 546 (offset -8 lines).
Hunk #6 succeeded at 872 (offset -8 lines).
Hunk #7 succeeded at 966 (offset -8 lines).
patching file drivers/scsi/isci/phy.h
Hunk #1 succeeded at 344 (offset 1 line).
patching file drivers/scsi/isci/port.c
Hunk #1 FAILED at 60.
Hunk #2 succeeded at 1097 (offset 43 lines).
Hunk #3 succeeded at 1172 (offset 43 lines).
Hunk #4 succeeded at 1187 (offset 43 lines).
Hunk #5 succeeded at 1282 (offset 43 lines).
Hunk #6 succeeded at 1332 (offset 43 lines).
Hunk #7 succeeded at 1375 (offset 43 lines).
Hunk #8 succeeded at 1405 (offset 43 lines).
Hunk #9 succeeded at 1425 (offset 43 lines).
Hunk #10 succeeded at 1440 (offset 43 lines).
1 out of 10 hunks FAILED -- saving rejects to file
drivers/scsi/isci/port.c.rej
patching file drivers/scsi/isci/port.h
Hunk #1 succeeded at 147 (offset 3 lines).
patching file drivers/scsi/isci/remote_device.c
patching file drivers/scsi/isci/remote_device.h
Hunk #1 succeeded at 180 (offset 1 line).
patching file drivers/scsi/isci/remote_node_context.c
patching file drivers/scsi/isci/remote_node_context.h
patching file drivers/scsi/isci/request.c
Hunk #2 succeeded at 957 (offset 37 lines).
Hunk #3 succeeded at 979 (offset 37 lines).
Hunk #4 succeeded at 2466 (offset 149 lines).
patching file drivers/scsi/isci/request.h
Hunk #1 FAILED at 182.
1 out of 1 hunk FAILED -- saving rejects to file
drivers/scsi/isci/request.h.rej

James



^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2012-02-19 15:17 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-02-10  9:18 [PATCH v1 0/5] isci updates for -next Dan Williams
2012-02-10  9:18 ` [PATCH v1 1/5] isci: T10 DIF support Dan Williams
2012-02-13 17:14   ` Martin K. Petersen
2012-02-10  9:18 ` [PATCH v1 2/5] isci: enable clock gating Dan Williams
2012-02-10  9:18 ` [PATCH v1 3/5] isci: debug, provide state-enum-to-string conversions Dan Williams
2012-02-19 15:17   ` James Bottomley
2012-02-10  9:18 ` [PATCH v1 4/5] isci: improve phy event warnings Dan Williams
2012-02-10  9:18 ` [PATCH v1 5/5] isci: improvements in driver unloading routine Dan Williams

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).