[PATCH] pm8001: optimization surrounding call to sas_ssp_task_response

[PATCH] pm8001: optimization surrounding call to sas_ssp_task_response

[Mark Salyzyn]

Optimization. Since sas_ssp_task_response sets both the ts->resp and ts->stat values, there is no need to set them before the call. Remaining changes are all cosmetic debug print messages.

Signed-off-by: mark_salyzyn@xyratex.com
Cc: jack_wang@usish.com
Cc: JBottomley@parallels.com
Cc: crystal_yu@usish.com
Cc: john_gong@usish.com
Cc: lindar_liu <lindar_liu@usish.com>

 drivers/scsi/pm8001/pm8001_hwi.c |   16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/scsi/pm8001/pm8001_hwi.c b/drivers/scsi/pm8001/pm8001_hwi.c
index b7b92f7..6b63a54 100644
--- a/drivers/scsi/pm8001/pm8001_hwi.c
+++ b/drivers/scsi/pm8001/pm8001_hwi.c
@@ -1474,13 +1474,11 @@ mpi_ssp_completion(struct pm8001_hba_info *pm8001_ha , void *piomb)
 	switch (status) {
 	case IO_SUCCESS:
 		PM8001_IO_DBG(pm8001_ha, pm8001_printk("IO_SUCCESS"
-			",param = %d\n", param));
+			", param = %d\n", param));
 		if (param == 0) {
 			ts->resp = SAS_TASK_COMPLETE;
 			ts->stat = SAM_STAT_GOOD;
 		} else {
-			ts->resp = SAS_TASK_COMPLETE;
-			ts->stat = SAS_PROTO_RESPONSE;
 			ts->residual = param;
 			iu = &psspPayload->ssp_resp_iu;
 			sas_ssp_task_response(pm8001_ha->dev, t, iu);
@@ -1497,7 +1495,7 @@ mpi_ssp_completion(struct pm8001_hba_info *pm8001_ha , void *piomb)
 	case IO_UNDERFLOW:
 		/* SSP Completion with error */
 		PM8001_IO_DBG(pm8001_ha, pm8001_printk("IO_UNDERFLOW"
-			",param = %d\n", param));
+			", param = %d\n", param));
 		ts->resp = SAS_TASK_COMPLETE;
 		ts->stat = SAS_DATA_UNDERRUN;
 		ts->residual = param;
@@ -1659,7 +1657,7 @@ mpi_ssp_completion(struct pm8001_hba_info *pm8001_ha , void *piomb)
 		break;
 	}
 	PM8001_IO_DBG(pm8001_ha,
-		pm8001_printk("scsi_status = %x \n ",
+		pm8001_printk("scsi_status = %x\n",
 		psspPayload->ssp_resp_iu.status));
 	spin_lock_irqsave(&t->task_state_lock, flags);
 	t->task_state_flags &= ~SAS_TASK_STATE_PENDING;
@@ -1842,7 +1840,7 @@ static void mpi_ssp_event(struct pm8001_hba_info *pm8001_ha , void *piomb)
 		break;
 	case IO_XFER_CMD_FRAME_ISSUED:
 		PM8001_IO_DBG(pm8001_ha,
-			pm8001_printk("  IO_XFER_CMD_FRAME_ISSUED\n"));
+			pm8001_printk("IO_XFER_CMD_FRAME_ISSUED\n"));
 		return;
 	default:
 		PM8001_IO_DBG(pm8001_ha,
@@ -3137,7 +3135,7 @@ static int mpi_dereg_resp(struct pm8001_hba_info *pm8001_ha, void *piomb)
 	device_id = le32_to_cpu(registerRespPayload->device_id);
 	if (status != 0)
 		PM8001_MSG_DBG(pm8001_ha,
-			pm8001_printk(" deregister device failed ,status = %x"
+			pm8001_printk("deregister device failed, status = %x"
 			", device_id = %x\n", status, device_id));
 	return 0;
 }
@@ -3218,7 +3216,7 @@ mpi_general_event(struct pm8001_hba_info *pm8001_ha , void *piomb)
 		(struct general_event_resp *)(piomb + 4);
 	status = le32_to_cpu(pPayload->status);
 	PM8001_MSG_DBG(pm8001_ha,
-		pm8001_printk(" status = 0x%x\n", status));
+		pm8001_printk("OPC_OUB_GENERAL_EVENT: status = 0x%x\n", status);
 	for (i = 0; i < GENERAL_EVENT_PAYLOAD; i++)
 		PM8001_MSG_DBG(pm8001_ha,
 			pm8001_printk("inb_IOMB_payload[0x%x] 0x%x,\n", i,
@@ -3532,7 +3530,7 @@ static void process_one_iomb(struct pm8001_hba_info *pm8001_ha, void *piomb)
 		break;
 	case OPC_OUB_DEREG_DEV:
 		PM8001_MSG_DBG(pm8001_ha,
-			pm8001_printk("unresgister the deviece\n"));
+			pm8001_printk("OPC_OUB_DEREG_DEV\n"));
 		mpi_dereg_resp(pm8001_ha, piomb);
 		break;
 	case OPC_OUB_GET_DEV_HANDLE:

[PATCH] pm8001: Add FUNC_GET_EVENTS

[Mark Salyzyn]

The pm8001 driver is missing the FUNC_GET_EVENTS handler in the phy control function. Since the pm8001_bar4_shift function was not designed to be called at runtime, we had to take some pre-emptive additional attention to protect the hardware accesses throughout the driver.

Signed-off-by: mark_salyzyn@xyratex.com
Cc: jack_wang@usish.com
Cc: JBottomley@parallels.com
Cc: crystal_yu@usish.com
Cc: john_gong@usish.com
Cc: lindar_liu <lindar_liu@usish.com>

 drivers/scsi/pm8001/pm8001_hwi.c |   84 ++++++++++++++++++++++++++++++++++++++++-------------------
 drivers/scsi/pm8001/pm8001_sas.c |   34 +++++++++++++++++++++++
 drivers/scsi/pm8001/pm8001_sas.h |    1 
 3 files changed, 92 insertions(+), 27 deletions(-)

diff --git a/drivers/scsi/pm8001/pm8001_hwi.c b/drivers/scsi/pm8001/pm8001_hwi.c
index b7b92f7..4bddb2a 100644
--- a/drivers/scsi/pm8001/pm8001_hwi.c
+++ b/drivers/scsi/pm8001/pm8001_hwi.c
@@ -338,26 +338,25 @@ update_outbnd_queue_table(struct pm8001_hba_info *pm8001_ha, int number)
 }
 
 /**
- * bar4_shift - function is called to shift BAR base address
- * @pm8001_ha : our hba card information
+ * pm8001_bar4_shift - function is called to shift BAR base address
+ * @pm8001_ha : our hba card infomation
  * @shiftValue : shifting value in memory bar.
  */
-static int bar4_shift(struct pm8001_hba_info *pm8001_ha, u32 shiftValue)
+int pm8001_bar4_shift(struct pm8001_hba_info *pm8001_ha, u32 shiftValue)
 {
 	u32 regVal;
-	u32 max_wait_count;
+	unsigned long start;
 
 	/* program the inbound AXI translation Lower Address */
 	pm8001_cw32(pm8001_ha, 1, SPC_IBW_AXI_TRANSLATION_LOW, shiftValue);
 
 	/* confirm the setting is written */
-	max_wait_count = 1 * 1000 * 1000;  /* 1 sec */
+	start = jiffies + HZ; /* 1 sec */
 	do {
-		udelay(1);
 		regVal = pm8001_cr32(pm8001_ha, 1, SPC_IBW_AXI_TRANSLATION_LOW);
-	} while ((regVal != shiftValue) && (--max_wait_count));
+	} while ((regVal != shiftValue) && time_before(jiffies, start));
 
-	if (!max_wait_count) {
+	if (regVal != shiftValue) {
 		PM8001_INIT_DBG(pm8001_ha,
 			pm8001_printk("TIMEOUT:SPC_IBW_AXI_TRANSLATION_LOW"
 			" = 0x%x\n", regVal));
@@ -388,16 +387,23 @@ mpi_set_phys_g3_with_ssc(struct pm8001_hba_info *pm8001_ha, u32 SSCbit)
     * Using shifted destination address 0x3_0000:0x1074 + 0x4000*N (N=0:3)
     * Using shifted destination address 0x4_0000:0x1074 + 0x4000*(N-4) (N=4:7)
     */
-	if (-1 == bar4_shift(pm8001_ha, SAS2_SETTINGS_LOCAL_PHY_0_3_SHIFT_ADDR))
+	spin_lock_irqsave(&pm8001_ha->lock, flags);
+	if (-1 == pm8001_bar4_shift(pm8001_ha,
+				SAS2_SETTINGS_LOCAL_PHY_0_3_SHIFT_ADDR)) {
+		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
 		return;
+	}
 
 	for (i = 0; i < 4; i++) {
 		offset = SAS2_SETTINGS_LOCAL_PHY_0_3_OFFSET + 0x4000 * i;
 		pm8001_cw32(pm8001_ha, 2, offset, 0x80001501);
 	}
 	/* shift membase 3 for SAS2_SETTINGS_LOCAL_PHY 4 - 7 */
-	if (-1 == bar4_shift(pm8001_ha, SAS2_SETTINGS_LOCAL_PHY_4_7_SHIFT_ADDR))
+	if (-1 == pm8001_bar4_shift(pm8001_ha,
+				SAS2_SETTINGS_LOCAL_PHY_4_7_SHIFT_ADDR)) {
+		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
 		return;
+	}
 	for (i = 4; i < 8; i++) {
 		offset = SAS2_SETTINGS_LOCAL_PHY_4_7_OFFSET + 0x4000 * (i-4);
 		pm8001_cw32(pm8001_ha, 2, offset, 0x80001501);
@@ -421,7 +427,8 @@ mpi_set_phys_g3_with_ssc(struct pm8001_hba_info *pm8001_ha, u32 SSCbit)
 	pm8001_cw32(pm8001_ha, 2, 0xd8, 0x8000C016);
 
 	/*set the shifted destination address to 0x0 to avoid error operation */
-	bar4_shift(pm8001_ha, 0x0);
+	pm8001_bar4_shift(pm8001_ha, 0x0);
+	spin_unlock_irqrestore(&pm8001_ha->lock, flags);
 	return;
 }
 
@@ -437,6 +444,7 @@ mpi_set_open_retry_interval_reg(struct pm8001_hba_info *pm8001_ha,
 	u32 offset;
 	u32 value;
 	u32 i;
+	unsigned long flags;
 
 #define OPEN_RETRY_INTERVAL_PHY_0_3_SHIFT_ADDR 0x00030000
 #define OPEN_RETRY_INTERVAL_PHY_4_7_SHIFT_ADDR 0x00040000
@@ -445,24 +453,30 @@ mpi_set_open_retry_interval_reg(struct pm8001_hba_info *pm8001_ha,
 #define OPEN_RETRY_INTERVAL_REG_MASK 0x0000FFFF
 
 	value = interval & OPEN_RETRY_INTERVAL_REG_MASK;
+	spin_lock_irqsave(&pm8001_ha->lock, flags);
 	/* shift bar and set the OPEN_REJECT(RETRY) interval time of PHY 0 -3.*/
-	if (-1 == bar4_shift(pm8001_ha,
-			     OPEN_RETRY_INTERVAL_PHY_0_3_SHIFT_ADDR))
+	if (-1 == pm8001_bar4_shift(pm8001_ha,
+			     OPEN_RETRY_INTERVAL_PHY_0_3_SHIFT_ADDR)) {
+		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
 		return;
+	}
 	for (i = 0; i < 4; i++) {
 		offset = OPEN_RETRY_INTERVAL_PHY_0_3_OFFSET + 0x4000 * i;
 		pm8001_cw32(pm8001_ha, 2, offset, value);
 	}
 
-	if (-1 == bar4_shift(pm8001_ha,
-			     OPEN_RETRY_INTERVAL_PHY_4_7_SHIFT_ADDR))
+	if (-1 == pm8001_bar4_shift(pm8001_ha,
+			     OPEN_RETRY_INTERVAL_PHY_4_7_SHIFT_ADDR)) {
+		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
 		return;
+	}
 	for (i = 4; i < 8; i++) {
 		offset = OPEN_RETRY_INTERVAL_PHY_4_7_OFFSET + 0x4000 * (i-4);
 		pm8001_cw32(pm8001_ha, 2, offset, value);
 	}
 	/*set the shifted destination address to 0x0 to avoid error operation */
-	bar4_shift(pm8001_ha, 0x0);
+	pm8001_bar4_shift(pm8001_ha, 0x0);
+	spin_unlock_irqrestore(&pm8001_ha->lock, flags);
 	return;
 }
 
@@ -688,8 +702,11 @@ static u32 soft_reset_ready_check(struct pm8001_hba_info *pm8001_ha)
 		PM8001_INIT_DBG(pm8001_ha,
 			pm8001_printk("Firmware is ready for reset .\n"));
 	} else {
-	/* Trigger NMI twice via RB6 */
-		if (-1 == bar4_shift(pm8001_ha, RB6_ACCESS_REG)) {
+		unsigned long flags;
+		/* Trigger NMI twice via RB6 */
+		spin_lock_irqsave(&pm8001_ha->lock, flags);
+		if (-1 == pm8001_bar4_shift(pm8001_ha, RB6_ACCESS_REG)) {
+			spin_unlock_irqrestore(&pm8001_ha->lock, flags);
 			PM8001_FAIL_DBG(pm8001_ha,
 				pm8001_printk("Shift Bar4 to 0x%x failed\n",
 					RB6_ACCESS_REG));
@@ -715,8 +732,10 @@ static u32 soft_reset_ready_check(struct pm8001_hba_info *pm8001_ha)
 			PM8001_FAIL_DBG(pm8001_ha,
 				pm8001_printk("SCRATCH_PAD3 value = 0x%x\n",
 				pm8001_cr32(pm8001_ha, 0, MSGU_SCRATCH_PAD_3)));
+			spin_unlock_irqrestore(&pm8001_ha->lock, flags);
 			return -1;
 		}
+		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
 	}
 	return 0;
 }
@@ -733,6 +752,7 @@ pm8001_chip_soft_rst(struct pm8001_hba_info *pm8001_ha, u32 signature)
 	u32	regVal, toggleVal;
 	u32	max_wait_count;
 	u32	regVal1, regVal2, regVal3;
+	unsigned long flags;
 
 	/* step1: Check FW is ready for soft reset */
 	if (soft_reset_ready_check(pm8001_ha) != 0) {
@@ -743,7 +763,9 @@ pm8001_chip_soft_rst(struct pm8001_hba_info *pm8001_ha, u32 signature)
 	/* step 2: clear NMI status register on AAP1 and IOP, write the same
 	value to clear */
 	/* map 0x60000 to BAR4(0x20), BAR2(win) */
-	if (-1 == bar4_shift(pm8001_ha, MBIC_AAP1_ADDR_BASE)) {
+	spin_lock_irqsave(&pm8001_ha->lock, flags);
+	if (-1 == pm8001_bar4_shift(pm8001_ha, MBIC_AAP1_ADDR_BASE)) {
+		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
 		PM8001_FAIL_DBG(pm8001_ha,
 			pm8001_printk("Shift Bar4 to 0x%x failed\n",
 			MBIC_AAP1_ADDR_BASE));
@@ -754,7 +776,8 @@ pm8001_chip_soft_rst(struct pm8001_hba_info *pm8001_ha, u32 signature)
 		pm8001_printk("MBIC - NMI Enable VPE0 (IOP)= 0x%x\n", regVal));
 	pm8001_cw32(pm8001_ha, 2, MBIC_NMI_ENABLE_VPE0_IOP, 0x0);
 	/* map 0x70000 to BAR4(0x20), BAR2(win) */
-	if (-1 == bar4_shift(pm8001_ha, MBIC_IOP_ADDR_BASE)) {
+	if (-1 == pm8001_bar4_shift(pm8001_ha, MBIC_IOP_ADDR_BASE)) {
+		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
 		PM8001_FAIL_DBG(pm8001_ha,
 			pm8001_printk("Shift Bar4 to 0x%x failed\n",
 			MBIC_IOP_ADDR_BASE));
@@ -796,7 +819,8 @@ pm8001_chip_soft_rst(struct pm8001_hba_info *pm8001_ha, u32 signature)
 
 	/* read required registers for confirmming */
 	/* map 0x0700000 to BAR4(0x20), BAR2(win) */
-	if (-1 == bar4_shift(pm8001_ha, GSM_ADDR_BASE)) {
+	if (-1 == pm8001_bar4_shift(pm8001_ha, GSM_ADDR_BASE)) {
+		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
 		PM8001_FAIL_DBG(pm8001_ha,
 			pm8001_printk("Shift Bar4 to 0x%x failed\n",
 			GSM_ADDR_BASE));
@@ -862,7 +886,8 @@ pm8001_chip_soft_rst(struct pm8001_hba_info *pm8001_ha, u32 signature)
 	/* step 5: delay 10 usec */
 	udelay(10);
 	/* step 5-b: set GPIO-0 output control to tristate anyway */
-	if (-1 == bar4_shift(pm8001_ha, GPIO_ADDR_BASE)) {
+	if (-1 == pm8001_bar4_shift(pm8001_ha, GPIO_ADDR_BASE)) {
+		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
 		PM8001_INIT_DBG(pm8001_ha,
 				pm8001_printk("Shift Bar4 to 0x%x failed\n",
 				GPIO_ADDR_BASE));
@@ -878,7 +903,8 @@ pm8001_chip_soft_rst(struct pm8001_hba_info *pm8001_ha, u32 signature)
 
 	/* Step 6: Reset the IOP and AAP1 */
 	/* map 0x00000 to BAR4(0x20), BAR2(win) */
-	if (-1 == bar4_shift(pm8001_ha, SPC_TOP_LEVEL_ADDR_BASE)) {
+	if (-1 == pm8001_bar4_shift(pm8001_ha, SPC_TOP_LEVEL_ADDR_BASE)) {
+		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
 		PM8001_FAIL_DBG(pm8001_ha,
 			pm8001_printk("SPC Shift Bar4 to 0x%x failed\n",
 			SPC_TOP_LEVEL_ADDR_BASE));
@@ -915,7 +941,8 @@ pm8001_chip_soft_rst(struct pm8001_hba_info *pm8001_ha, u32 signature)
 
 	/* step 11: reads and sets the GSM Configuration and Reset Register */
 	/* map 0x0700000 to BAR4(0x20), BAR2(win) */
-	if (-1 == bar4_shift(pm8001_ha, GSM_ADDR_BASE)) {
+	if (-1 == pm8001_bar4_shift(pm8001_ha, GSM_ADDR_BASE)) {
+		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
 		PM8001_FAIL_DBG(pm8001_ha,
 			pm8001_printk("SPC Shift Bar4 to 0x%x failed\n",
 			GSM_ADDR_BASE));
@@ -968,7 +995,8 @@ pm8001_chip_soft_rst(struct pm8001_hba_info *pm8001_ha, u32 signature)
 
 	/* step 13: bring the IOP and AAP1 out of reset */
 	/* map 0x00000 to BAR4(0x20), BAR2(win) */
-	if (-1 == bar4_shift(pm8001_ha, SPC_TOP_LEVEL_ADDR_BASE)) {
+	if (-1 == pm8001_bar4_shift(pm8001_ha, SPC_TOP_LEVEL_ADDR_BASE)) {
+		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
 		PM8001_FAIL_DBG(pm8001_ha,
 			pm8001_printk("Shift Bar4 to 0x%x failed\n",
 			SPC_TOP_LEVEL_ADDR_BASE));
@@ -1010,6 +1038,7 @@ pm8001_chip_soft_rst(struct pm8001_hba_info *pm8001_ha, u32 signature)
 				pm8001_printk("SCRATCH_PAD3 value = 0x%x\n",
 				pm8001_cr32(pm8001_ha, 0,
 				MSGU_SCRATCH_PAD_3)));
+			spin_unlock_irqrestore(&pm8001_ha->lock, flags);
 			return -1;
 		}
 
@@ -1039,9 +1068,12 @@ pm8001_chip_soft_rst(struct pm8001_hba_info *pm8001_ha, u32 signature)
 				pm8001_printk("SCRATCH_PAD3 value = 0x%x\n",
 				pm8001_cr32(pm8001_ha, 0,
 				MSGU_SCRATCH_PAD_3)));
+			spin_unlock_irqrestore(&pm8001_ha->lock, flags);
 			return -1;
 		}
 	}
+	pm8001_bar4_shift(pm8001_ha, 0);
+	spin_unlock_irqrestore(&pm8001_ha->lock, flags);
 
 	PM8001_INIT_DBG(pm8001_ha,
 		pm8001_printk("SPC soft reset Complete\n"));
@@ -1157,8 +1189,8 @@ pm8001_chip_msix_interrupt_disable(struct pm8001_hba_info *pm8001_ha,
 	msi_index = int_vec_idx * MSIX_TABLE_ELEMENT_SIZE;
 	msi_index += MSIX_TABLE_BASE;
 	pm8001_cw32(pm8001_ha, 0,  msi_index, MSIX_INTERRUPT_DISABLE);
-
 }
+
 /**
  * pm8001_chip_interrupt_enable - enable PM8001 chip interrupt
  * @pm8001_ha: our hba card information
diff --git a/drivers/scsi/pm8001/pm8001_sas.c b/drivers/scsi/pm8001/pm8001_sas.c
index fb3dc99..957bd6a 100644
--- a/drivers/scsi/pm8001/pm8001_sas.c
+++ b/drivers/scsi/pm8001/pm8001_sas.c
@@ -166,6 +166,7 @@ int pm8001_phy_control(struct asd_sas_phy *sas_phy, enum phy_func func,
 	struct pm8001_hba_info *pm8001_ha = NULL;
 	struct sas_phy_linkrates *rates;
 	DECLARE_COMPLETION_ONSTACK(completion);
+	unsigned long flags;
 	pm8001_ha = sas_phy->ha->lldd_ha;
 	pm8001_ha->phy[phy_id].enable_completion = &completion;
 	switch (func) {
@@ -188,7 +189,9 @@ int pm8001_phy_control(struct asd_sas_phy *sas_phy, enum phy_func func,
 		break;
 	case PHY_FUNC_HARD_RESET:
 		if (pm8001_ha->phy[phy_id].phy_state == 0) {
+			spin_lock_irqsave(&pm8001_ha->lock, flags);
 			PM8001_CHIP_DISP->phy_start_req(pm8001_ha, phy_id);
+			spin_unlock_irqrestore(&pm8001_ha->lock, flags);
 			wait_for_completion(&completion);
 		}
 		PM8001_CHIP_DISP->phy_ctl_req(pm8001_ha, phy_id,
@@ -196,21 +199,50 @@ int pm8001_phy_control(struct asd_sas_phy *sas_phy, enum phy_func func,
 		break;
 	case PHY_FUNC_LINK_RESET:
 		if (pm8001_ha->phy[phy_id].phy_state == 0) {
+			spin_lock_irqsave(&pm8001_ha->lock, flags);
 			PM8001_CHIP_DISP->phy_start_req(pm8001_ha, phy_id);
+			spin_unlock_irqrestore(&pm8001_ha->lock, flags);
 			wait_for_completion(&completion);
 		}
+		spin_lock_irqsave(&pm8001_ha->lock, flags);
 		PM8001_CHIP_DISP->phy_ctl_req(pm8001_ha, phy_id,
 					      PHY_LINK_RESET);
+		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
 		break;
 	case PHY_FUNC_RELEASE_SPINUP_HOLD:
+		spin_lock_irqsave(&pm8001_ha->lock, flags);
 		PM8001_CHIP_DISP->phy_ctl_req(pm8001_ha, phy_id,
 					      PHY_LINK_RESET);
+		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
 		break;
 	case PHY_FUNC_DISABLE:
+		spin_lock_irqsave(&pm8001_ha->lock, flags);
 		PM8001_CHIP_DISP->phy_stop_req(pm8001_ha, phy_id);
+		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
 		break;
+	case PHY_FUNC_GET_EVENTS:
+		spin_lock_irqsave(&pm8001_ha->lock, flags);
+		if (-1 == pm8001_bar4_shift(pm8001_ha,
+					(phy_id < 4) ? 0x30000 : 0x40000)) {
+			spin_unlock_irqrestore(&pm8001_ha->lock, flags);
+			return -EINVAL;
+		}
+		{
+			struct sas_phy *phy = sas_phy->phy;
+			uint32_t *qp = (uint32_t *)(((char *)
+				pm8001_ha->io_mem[2].memvirtaddr)
+				+ 0x1034 + (0x4000 * (phy_id & 3)));
+
+			phy->invalid_dword_count = qp[0];
+			phy->running_disparity_error_count = qp[1];
+			phy->loss_of_dword_sync_count = qp[3];
+			phy->phy_reset_problem_count = qp[4];
+		}
+		pm8001_bar4_shift(pm8001_ha, 0);
+		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
+		return 0;
 	default:
-		rc = -ENOSYS;
+		rc = -EOPNOTSUPP;
 	}
 	msleep(300);
 	return rc;
diff --git a/drivers/scsi/pm8001/pm8001_sas.h b/drivers/scsi/pm8001/pm8001_sas.h
index 93959fe..83a48f3 100644
--- a/drivers/scsi/pm8001/pm8001_sas.h
+++ b/drivers/scsi/pm8001/pm8001_sas.h
@@ -488,6 +488,7 @@ int pm8001_mem_alloc(struct pci_dev *pdev, void **virt_addr,
 	dma_addr_t *pphys_addr, u32 *pphys_addr_hi, u32 *pphys_addr_lo,
 	u32 mem_size, u32 align);
 
+int pm8001_bar4_shift(struct pm8001_hba_info *pm8001_ha, u32 shiftValue);
 
 /* ctl shared API */
 extern struct device_attribute *pm8001_host_attrs[];

Re: [PATCH] pm8001: optimization surrounding call to sas_ssp_task_response

[jack_wang]

Thanks Mark for fix. 
Acked-by: jack_wang@usish.com
Sorry for reply twice, forget to change text format.


--------------
jack_wang
>Optimization. Since sas_ssp_task_response sets both the ts->resp and ts->stat values, there is no need to set them before the call. Remaining changes are all cosmetic debug print messages.
>
>Signed-off-by: mark_salyzyn@xyratex.com
>Cc: jack_wang@usish.com
>Cc: JBottomley@parallels.com
>Cc: crystal_yu@usish.com
>Cc: john_gong@usish.com
>Cc: lindar_liu <lindar_liu@usish.com>
>
> drivers/scsi/pm8001/pm8001_hwi.c |   16 +++++++---------
> 1 file changed, 7 insertions(+), 9 deletions(-)
>
>diff --git a/drivers/scsi/pm8001/pm8001_hwi.c b/drivers/scsi/pm8001/pm8001_hwi.c
>index b7b92f7..6b63a54 100644
>--- a/drivers/scsi/pm8001/pm8001_hwi.c
>+++ b/drivers/scsi/pm8001/pm8001_hwi.c
>@@ -1474,13 +1474,11 @@ mpi_ssp_completion(struct pm8001_hba_info *pm8001_ha , void *piomb)
> 	switch (status) {
> 	case IO_SUCCESS:
> 		PM8001_IO_DBG(pm8001_ha, pm8001_printk("IO_SUCCESS"
>-			",param = %d\n", param));
>+			", param = %d\n", param));
> 		if (param == 0) {
> 			ts->resp = SAS_TASK_COMPLETE;
> 			ts->stat = SAM_STAT_GOOD;
> 		} else {
>-			ts->resp = SAS_TASK_COMPLETE;
>-			ts->stat = SAS_PROTO_RESPONSE;
> 			ts->residual = param;
> 			iu = &psspPayload->ssp_resp_iu;
> 			sas_ssp_task_response(pm8001_ha->dev, t, iu);
>@@ -1497,7 +1495,7 @@ mpi_ssp_completion(struct pm8001_hba_info *pm8001_ha , void *piomb)
> 	case IO_UNDERFLOW:
> 		/* SSP Completion with error */
> 		PM8001_IO_DBG(pm8001_ha, pm8001_printk("IO_UNDERFLOW"
>-			",param = %d\n", param));
>+			", param = %d\n", param));
> 		ts->resp = SAS_TASK_COMPLETE;
> 		ts->stat = SAS_DATA_UNDERRUN;
> 		ts->residual = param;
>@@ -1659,7 +1657,7 @@ mpi_ssp_completion(struct pm8001_hba_info *pm8001_ha , void *piomb)
> 		break;
> 	}
> 	PM8001_IO_DBG(pm8001_ha,
>-		pm8001_printk("scsi_status = %x \n ",
>+		pm8001_printk("scsi_status = %x\n",
> 		psspPayload->ssp_resp_iu.status));
> 	spin_lock_irqsave(&t->task_state_lock, flags);
> 	t->task_state_flags &= ~SAS_TASK_STATE_PENDING;
>@@ -1842,7 +1840,7 @@ static void mpi_ssp_event(struct pm8001_hba_info *pm8001_ha , void *piomb)
> 		break;
> 	case IO_XFER_CMD_FRAME_ISSUED:
> 		PM8001_IO_DBG(pm8001_ha,
>-			pm8001_printk("  IO_XFER_CMD_FRAME_ISSUED\n"));
>+			pm8001_printk("IO_XFER_CMD_FRAME_ISSUED\n"));
> 		return;
> 	default:
> 		PM8001_IO_DBG(pm8001_ha,
>@@ -3137,7 +3135,7 @@ static int mpi_dereg_resp(struct pm8001_hba_info *pm8001_ha, void *piomb)
> 	device_id = le32_to_cpu(registerRespPayload->device_id);
> 	if (status != 0)
> 		PM8001_MSG_DBG(pm8001_ha,
>-			pm8001_printk(" deregister device failed ,status = %x"
>+			pm8001_printk("deregister device failed, status = %x"
> 			", device_id = %x\n", status, device_id));
> 	return 0;
> }
>@@ -3218,7 +3216,7 @@ mpi_general_event(struct pm8001_hba_info *pm8001_ha , void *piomb)
> 		(struct general_event_resp *)(piomb + 4);
> 	status = le32_to_cpu(pPayload->status);
> 	PM8001_MSG_DBG(pm8001_ha,
>-		pm8001_printk(" status = 0x%x\n", status));
>+		pm8001_printk("OPC_OUB_GENERAL_EVENT: status = 0x%x\n", status);
> 	for (i = 0; i < GENERAL_EVENT_PAYLOAD; i++)
> 		PM8001_MSG_DBG(pm8001_ha,
> 			pm8001_printk("inb_IOMB_payload[0x%x] 0x%x,\n", i,
>@@ -3532,7 +3530,7 @@ static void process_one_iomb(struct pm8001_hba_info *pm8001_ha, void *piomb)
> 		break;
> 	case OPC_OUB_DEREG_DEV:
> 		PM8001_MSG_DBG(pm8001_ha,
>-			pm8001_printk("unresgister the deviece\n"));
>+			pm8001_printk("OPC_OUB_DEREG_DEV\n"));
> 		mpi_dereg_resp(pm8001_ha, piomb);
> 		break;
> 	case OPC_OUB_GET_DEV_HANDLE:
>
>--
>To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>__________ Information from ESET NOD32 Antivirus, version of virus signature database 5659 (20101129) __________
>
>The message was checked by ESET NOD32 Antivirus.
>
>http://www.eset.com
>
>
>

Re: [PATCH] pm8001: Add FUNC_GET_EVENTS

[jack_wang]

Thanks Mark for catching this. Looks good to me, but I need to verify on Monday on Hardware then acked.



--------------
jack_wang
>The pm8001 driver is missing the FUNC_GET_EVENTS handler in the phy control function. Since the pm8001_bar4_shift function was not designed to be called at runtime, we had to take some pre-emptive additional attention to protect the hardware accesses throughout the driver.
>
>Signed-off-by: mark_salyzyn@xyratex.com
>Cc: jack_wang@usish.com
>Cc: JBottomley@parallels.com
>Cc: crystal_yu@usish.com
>Cc: john_gong@usish.com
>Cc: lindar_liu <lindar_liu@usish.com>
>
> drivers/scsi/pm8001/pm8001_hwi.c |   84 ++++++++++++++++++++++++++++++++++++++++-------------------
> drivers/scsi/pm8001/pm8001_sas.c |   34 +++++++++++++++++++++++
> drivers/scsi/pm8001/pm8001_sas.h |    1 
> 3 files changed, 92 insertions(+), 27 deletions(-)
>
>diff --git a/drivers/scsi/pm8001/pm8001_hwi.c b/drivers/scsi/pm8001/pm8001_hwi.c
>index b7b92f7..4bddb2a 100644
>--- a/drivers/scsi/pm8001/pm8001_hwi.c
>+++ b/drivers/scsi/pm8001/pm8001_hwi.c
>@@ -338,26 +338,25 @@ update_outbnd_queue_table(struct pm8001_hba_info *pm8001_ha, int number)
> }
> 
> /**
>- * bar4_shift - function is called to shift BAR base address
>- * @pm8001_ha : our hba card information
>+ * pm8001_bar4_shift - function is called to shift BAR base address
>+ * @pm8001_ha : our hba card infomation
>  * @shiftValue : shifting value in memory bar.
>  */
>-static int bar4_shift(struct pm8001_hba_info *pm8001_ha, u32 shiftValue)
>+int pm8001_bar4_shift(struct pm8001_hba_info *pm8001_ha, u32 shiftValue)
> {
> 	u32 regVal;
>-	u32 max_wait_count;
>+	unsigned long start;
> 
> 	/* program the inbound AXI translation Lower Address */
> 	pm8001_cw32(pm8001_ha, 1, SPC_IBW_AXI_TRANSLATION_LOW, shiftValue);
> 
> 	/* confirm the setting is written */
>-	max_wait_count = 1 * 1000 * 1000;  /* 1 sec */
>+	start = jiffies + HZ; /* 1 sec */
> 	do {
>-		udelay(1);
> 		regVal = pm8001_cr32(pm8001_ha, 1, SPC_IBW_AXI_TRANSLATION_LOW);
>-	} while ((regVal != shiftValue) && (--max_wait_count));
>+	} while ((regVal != shiftValue) && time_before(jiffies, start));
> 
>-	if (!max_wait_count) {
>+	if (regVal != shiftValue) {
> 		PM8001_INIT_DBG(pm8001_ha,
> 			pm8001_printk("TIMEOUT:SPC_IBW_AXI_TRANSLATION_LOW"
> 			" = 0x%x\n", regVal));
>@@ -388,16 +387,23 @@ mpi_set_phys_g3_with_ssc(struct pm8001_hba_info *pm8001_ha, u32 SSCbit)
>     * Using shifted destination address 0x3_0000:0x1074 + 0x4000*N (N=0:3)
>     * Using shifted destination address 0x4_0000:0x1074 + 0x4000*(N-4) (N=4:7)
>     */
>-	if (-1 == bar4_shift(pm8001_ha, SAS2_SETTINGS_LOCAL_PHY_0_3_SHIFT_ADDR))
>+	spin_lock_irqsave(&pm8001_ha->lock, flags);
>+	if (-1 == pm8001_bar4_shift(pm8001_ha,
>+				SAS2_SETTINGS_LOCAL_PHY_0_3_SHIFT_ADDR)) {
>+		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
> 		return;
>+	}
> 
> 	for (i = 0; i < 4; i++) {
> 		offset = SAS2_SETTINGS_LOCAL_PHY_0_3_OFFSET + 0x4000 * i;
> 		pm8001_cw32(pm8001_ha, 2, offset, 0x80001501);
> 	}
> 	/* shift membase 3 for SAS2_SETTINGS_LOCAL_PHY 4 - 7 */
>-	if (-1 == bar4_shift(pm8001_ha, SAS2_SETTINGS_LOCAL_PHY_4_7_SHIFT_ADDR))
>+	if (-1 == pm8001_bar4_shift(pm8001_ha,
>+				SAS2_SETTINGS_LOCAL_PHY_4_7_SHIFT_ADDR)) {
>+		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
> 		return;
>+	}
> 	for (i = 4; i < 8; i++) {
> 		offset = SAS2_SETTINGS_LOCAL_PHY_4_7_OFFSET + 0x4000 * (i-4);
> 		pm8001_cw32(pm8001_ha, 2, offset, 0x80001501);
>@@ -421,7 +427,8 @@ mpi_set_phys_g3_with_ssc(struct pm8001_hba_info *pm8001_ha, u32 SSCbit)
> 	pm8001_cw32(pm8001_ha, 2, 0xd8, 0x8000C016);
> 
> 	/*set the shifted destination address to 0x0 to avoid error operation */
>-	bar4_shift(pm8001_ha, 0x0);
>+	pm8001_bar4_shift(pm8001_ha, 0x0);
>+	spin_unlock_irqrestore(&pm8001_ha->lock, flags);
> 	return;
> }
> 
>@@ -437,6 +444,7 @@ mpi_set_open_retry_interval_reg(struct pm8001_hba_info *pm8001_ha,
> 	u32 offset;
> 	u32 value;
> 	u32 i;
>+	unsigned long flags;
> 
> #define OPEN_RETRY_INTERVAL_PHY_0_3_SHIFT_ADDR 0x00030000
> #define OPEN_RETRY_INTERVAL_PHY_4_7_SHIFT_ADDR 0x00040000
>@@ -445,24 +453,30 @@ mpi_set_open_retry_interval_reg(struct pm8001_hba_info *pm8001_ha,
> #define OPEN_RETRY_INTERVAL_REG_MASK 0x0000FFFF
> 
> 	value = interval & OPEN_RETRY_INTERVAL_REG_MASK;
>+	spin_lock_irqsave(&pm8001_ha->lock, flags);
> 	/* shift bar and set the OPEN_REJECT(RETRY) interval time of PHY 0 -3.*/
>-	if (-1 == bar4_shift(pm8001_ha,
>-			     OPEN_RETRY_INTERVAL_PHY_0_3_SHIFT_ADDR))
>+	if (-1 == pm8001_bar4_shift(pm8001_ha,
>+			     OPEN_RETRY_INTERVAL_PHY_0_3_SHIFT_ADDR)) {
>+		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
> 		return;
>+	}
> 	for (i = 0; i < 4; i++) {
> 		offset = OPEN_RETRY_INTERVAL_PHY_0_3_OFFSET + 0x4000 * i;
> 		pm8001_cw32(pm8001_ha, 2, offset, value);
> 	}
> 
>-	if (-1 == bar4_shift(pm8001_ha,
>-			     OPEN_RETRY_INTERVAL_PHY_4_7_SHIFT_ADDR))
>+	if (-1 == pm8001_bar4_shift(pm8001_ha,
>+			     OPEN_RETRY_INTERVAL_PHY_4_7_SHIFT_ADDR)) {
>+		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
> 		return;
>+	}
> 	for (i = 4; i < 8; i++) {
> 		offset = OPEN_RETRY_INTERVAL_PHY_4_7_OFFSET + 0x4000 * (i-4);
> 		pm8001_cw32(pm8001_ha, 2, offset, value);
> 	}
> 	/*set the shifted destination address to 0x0 to avoid error operation */
>-	bar4_shift(pm8001_ha, 0x0);
>+	pm8001_bar4_shift(pm8001_ha, 0x0);
>+	spin_unlock_irqrestore(&pm8001_ha->lock, flags);
> 	return;
> }
> 
>@@ -688,8 +702,11 @@ static u32 soft_reset_ready_check(struct pm8001_hba_info *pm8001_ha)
> 		PM8001_INIT_DBG(pm8001_ha,
> 			pm8001_printk("Firmware is ready for reset .\n"));
> 	} else {
>-	/* Trigger NMI twice via RB6 */
>-		if (-1 == bar4_shift(pm8001_ha, RB6_ACCESS_REG)) {
>+		unsigned long flags;
>+		/* Trigger NMI twice via RB6 */
>+		spin_lock_irqsave(&pm8001_ha->lock, flags);
>+		if (-1 == pm8001_bar4_shift(pm8001_ha, RB6_ACCESS_REG)) {
>+			spin_unlock_irqrestore(&pm8001_ha->lock, flags);
> 			PM8001_FAIL_DBG(pm8001_ha,
> 				pm8001_printk("Shift Bar4 to 0x%x failed\n",
> 					RB6_ACCESS_REG));
>@@ -715,8 +732,10 @@ static u32 soft_reset_ready_check(struct pm8001_hba_info *pm8001_ha)
> 			PM8001_FAIL_DBG(pm8001_ha,
> 				pm8001_printk("SCRATCH_PAD3 value = 0x%x\n",
> 				pm8001_cr32(pm8001_ha, 0, MSGU_SCRATCH_PAD_3)));
>+			spin_unlock_irqrestore(&pm8001_ha->lock, flags);
> 			return -1;
> 		}
>+		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
> 	}
> 	return 0;
> }
>@@ -733,6 +752,7 @@ pm8001_chip_soft_rst(struct pm8001_hba_info *pm8001_ha, u32 signature)
> 	u32	regVal, toggleVal;
> 	u32	max_wait_count;
> 	u32	regVal1, regVal2, regVal3;
>+	unsigned long flags;
> 
> 	/* step1: Check FW is ready for soft reset */
> 	if (soft_reset_ready_check(pm8001_ha) != 0) {
>@@ -743,7 +763,9 @@ pm8001_chip_soft_rst(struct pm8001_hba_info *pm8001_ha, u32 signature)
> 	/* step 2: clear NMI status register on AAP1 and IOP, write the same
> 	value to clear */
> 	/* map 0x60000 to BAR4(0x20), BAR2(win) */
>-	if (-1 == bar4_shift(pm8001_ha, MBIC_AAP1_ADDR_BASE)) {
>+	spin_lock_irqsave(&pm8001_ha->lock, flags);
>+	if (-1 == pm8001_bar4_shift(pm8001_ha, MBIC_AAP1_ADDR_BASE)) {
>+		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
> 		PM8001_FAIL_DBG(pm8001_ha,
> 			pm8001_printk("Shift Bar4 to 0x%x failed\n",
> 			MBIC_AAP1_ADDR_BASE));
>@@ -754,7 +776,8 @@ pm8001_chip_soft_rst(struct pm8001_hba_info *pm8001_ha, u32 signature)
> 		pm8001_printk("MBIC - NMI Enable VPE0 (IOP)= 0x%x\n", regVal));
> 	pm8001_cw32(pm8001_ha, 2, MBIC_NMI_ENABLE_VPE0_IOP, 0x0);
> 	/* map 0x70000 to BAR4(0x20), BAR2(win) */
>-	if (-1 == bar4_shift(pm8001_ha, MBIC_IOP_ADDR_BASE)) {
>+	if (-1 == pm8001_bar4_shift(pm8001_ha, MBIC_IOP_ADDR_BASE)) {
>+		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
> 		PM8001_FAIL_DBG(pm8001_ha,
> 			pm8001_printk("Shift Bar4 to 0x%x failed\n",
> 			MBIC_IOP_ADDR_BASE));
>@@ -796,7 +819,8 @@ pm8001_chip_soft_rst(struct pm8001_hba_info *pm8001_ha, u32 signature)
> 
> 	/* read required registers for confirmming */
> 	/* map 0x0700000 to BAR4(0x20), BAR2(win) */
>-	if (-1 == bar4_shift(pm8001_ha, GSM_ADDR_BASE)) {
>+	if (-1 == pm8001_bar4_shift(pm8001_ha, GSM_ADDR_BASE)) {
>+		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
> 		PM8001_FAIL_DBG(pm8001_ha,
> 			pm8001_printk("Shift Bar4 to 0x%x failed\n",
> 			GSM_ADDR_BASE));
>@@ -862,7 +886,8 @@ pm8001_chip_soft_rst(struct pm8001_hba_info *pm8001_ha, u32 signature)
> 	/* step 5: delay 10 usec */
> 	udelay(10);
> 	/* step 5-b: set GPIO-0 output control to tristate anyway */
>-	if (-1 == bar4_shift(pm8001_ha, GPIO_ADDR_BASE)) {
>+	if (-1 == pm8001_bar4_shift(pm8001_ha, GPIO_ADDR_BASE)) {
>+		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
> 		PM8001_INIT_DBG(pm8001_ha,
> 				pm8001_printk("Shift Bar4 to 0x%x failed\n",
> 				GPIO_ADDR_BASE));
>@@ -878,7 +903,8 @@ pm8001_chip_soft_rst(struct pm8001_hba_info *pm8001_ha, u32 signature)
> 
> 	/* Step 6: Reset the IOP and AAP1 */
> 	/* map 0x00000 to BAR4(0x20), BAR2(win) */
>-	if (-1 == bar4_shift(pm8001_ha, SPC_TOP_LEVEL_ADDR_BASE)) {
>+	if (-1 == pm8001_bar4_shift(pm8001_ha, SPC_TOP_LEVEL_ADDR_BASE)) {
>+		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
> 		PM8001_FAIL_DBG(pm8001_ha,
> 			pm8001_printk("SPC Shift Bar4 to 0x%x failed\n",
> 			SPC_TOP_LEVEL_ADDR_BASE));
>@@ -915,7 +941,8 @@ pm8001_chip_soft_rst(struct pm8001_hba_info *pm8001_ha, u32 signature)
> 
> 	/* step 11: reads and sets the GSM Configuration and Reset Register */
> 	/* map 0x0700000 to BAR4(0x20), BAR2(win) */
>-	if (-1 == bar4_shift(pm8001_ha, GSM_ADDR_BASE)) {
>+	if (-1 == pm8001_bar4_shift(pm8001_ha, GSM_ADDR_BASE)) {
>+		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
> 		PM8001_FAIL_DBG(pm8001_ha,
> 			pm8001_printk("SPC Shift Bar4 to 0x%x failed\n",
> 			GSM_ADDR_BASE));
>@@ -968,7 +995,8 @@ pm8001_chip_soft_rst(struct pm8001_hba_info *pm8001_ha, u32 signature)
> 
> 	/* step 13: bring the IOP and AAP1 out of reset */
> 	/* map 0x00000 to BAR4(0x20), BAR2(win) */
>-	if (-1 == bar4_shift(pm8001_ha, SPC_TOP_LEVEL_ADDR_BASE)) {
>+	if (-1 == pm8001_bar4_shift(pm8001_ha, SPC_TOP_LEVEL_ADDR_BASE)) {
>+		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
> 		PM8001_FAIL_DBG(pm8001_ha,
> 			pm8001_printk("Shift Bar4 to 0x%x failed\n",
> 			SPC_TOP_LEVEL_ADDR_BASE));
>@@ -1010,6 +1038,7 @@ pm8001_chip_soft_rst(struct pm8001_hba_info *pm8001_ha, u32 signature)
> 				pm8001_printk("SCRATCH_PAD3 value = 0x%x\n",
> 				pm8001_cr32(pm8001_ha, 0,
> 				MSGU_SCRATCH_PAD_3)));
>+			spin_unlock_irqrestore(&pm8001_ha->lock, flags);
> 			return -1;
> 		}
> 
>@@ -1039,9 +1068,12 @@ pm8001_chip_soft_rst(struct pm8001_hba_info *pm8001_ha, u32 signature)
> 				pm8001_printk("SCRATCH_PAD3 value = 0x%x\n",
> 				pm8001_cr32(pm8001_ha, 0,
> 				MSGU_SCRATCH_PAD_3)));
>+			spin_unlock_irqrestore(&pm8001_ha->lock, flags);
> 			return -1;
> 		}
> 	}
>+	pm8001_bar4_shift(pm8001_ha, 0);
>+	spin_unlock_irqrestore(&pm8001_ha->lock, flags);
> 
> 	PM8001_INIT_DBG(pm8001_ha,
> 		pm8001_printk("SPC soft reset Complete\n"));
>@@ -1157,8 +1189,8 @@ pm8001_chip_msix_interrupt_disable(struct pm8001_hba_info *pm8001_ha,
> 	msi_index = int_vec_idx * MSIX_TABLE_ELEMENT_SIZE;
> 	msi_index += MSIX_TABLE_BASE;
> 	pm8001_cw32(pm8001_ha, 0,  msi_index, MSIX_INTERRUPT_DISABLE);
>-
> }
>+
> /**
>  * pm8001_chip_interrupt_enable - enable PM8001 chip interrupt
>  * @pm8001_ha: our hba card information
>diff --git a/drivers/scsi/pm8001/pm8001_sas.c b/drivers/scsi/pm8001/pm8001_sas.c
>index fb3dc99..957bd6a 100644
>--- a/drivers/scsi/pm8001/pm8001_sas.c
>+++ b/drivers/scsi/pm8001/pm8001_sas.c
>@@ -166,6 +166,7 @@ int pm8001_phy_control(struct asd_sas_phy *sas_phy, enum phy_func func,
> 	struct pm8001_hba_info *pm8001_ha = NULL;
> 	struct sas_phy_linkrates *rates;
> 	DECLARE_COMPLETION_ONSTACK(completion);
>+	unsigned long flags;
> 	pm8001_ha = sas_phy->ha->lldd_ha;
> 	pm8001_ha->phy[phy_id].enable_completion = &completion;
> 	switch (func) {
>@@ -188,7 +189,9 @@ int pm8001_phy_control(struct asd_sas_phy *sas_phy, enum phy_func func,
> 		break;
> 	case PHY_FUNC_HARD_RESET:
> 		if (pm8001_ha->phy[phy_id].phy_state == 0) {
>+			spin_lock_irqsave(&pm8001_ha->lock, flags);
> 			PM8001_CHIP_DISP->phy_start_req(pm8001_ha, phy_id);
>+			spin_unlock_irqrestore(&pm8001_ha->lock, flags);
> 			wait_for_completion(&completion);
> 		}
> 		PM8001_CHIP_DISP->phy_ctl_req(pm8001_ha, phy_id,
>@@ -196,21 +199,50 @@ int pm8001_phy_control(struct asd_sas_phy *sas_phy, enum phy_func func,
> 		break;
> 	case PHY_FUNC_LINK_RESET:
> 		if (pm8001_ha->phy[phy_id].phy_state == 0) {
>+			spin_lock_irqsave(&pm8001_ha->lock, flags);
> 			PM8001_CHIP_DISP->phy_start_req(pm8001_ha, phy_id);
>+			spin_unlock_irqrestore(&pm8001_ha->lock, flags);
> 			wait_for_completion(&completion);
> 		}
>+		spin_lock_irqsave(&pm8001_ha->lock, flags);
> 		PM8001_CHIP_DISP->phy_ctl_req(pm8001_ha, phy_id,
> 					      PHY_LINK_RESET);
>+		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
> 		break;
> 	case PHY_FUNC_RELEASE_SPINUP_HOLD:
>+		spin_lock_irqsave(&pm8001_ha->lock, flags);
> 		PM8001_CHIP_DISP->phy_ctl_req(pm8001_ha, phy_id,
> 					      PHY_LINK_RESET);
>+		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
> 		break;
> 	case PHY_FUNC_DISABLE:
>+		spin_lock_irqsave(&pm8001_ha->lock, flags);
> 		PM8001_CHIP_DISP->phy_stop_req(pm8001_ha, phy_id);
>+		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
> 		break;
>+	case PHY_FUNC_GET_EVENTS:
>+		spin_lock_irqsave(&pm8001_ha->lock, flags);
>+		if (-1 == pm8001_bar4_shift(pm8001_ha,
>+					(phy_id < 4) ? 0x30000 : 0x40000)) {
>+			spin_unlock_irqrestore(&pm8001_ha->lock, flags);
>+			return -EINVAL;
>+		}
>+		{
>+			struct sas_phy *phy = sas_phy->phy;
>+			uint32_t *qp = (uint32_t *)(((char *)
>+				pm8001_ha->io_mem[2].memvirtaddr)
>+				+ 0x1034 + (0x4000 * (phy_id & 3)));
>+
>+			phy->invalid_dword_count = qp[0];
>+			phy->running_disparity_error_count = qp[1];
>+			phy->loss_of_dword_sync_count = qp[3];
>+			phy->phy_reset_problem_count = qp[4];
>+		}
>+		pm8001_bar4_shift(pm8001_ha, 0);
>+		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
>+		return 0;
> 	default:
>-		rc = -ENOSYS;
>+		rc = -EOPNOTSUPP;
> 	}
> 	msleep(300);
> 	return rc;
>diff --git a/drivers/scsi/pm8001/pm8001_sas.h b/drivers/scsi/pm8001/pm8001_sas.h
>index 93959fe..83a48f3 100644
>--- a/drivers/scsi/pm8001/pm8001_sas.h
>+++ b/drivers/scsi/pm8001/pm8001_sas.h
>@@ -488,6 +488,7 @@ int pm8001_mem_alloc(struct pci_dev *pdev, void **virt_addr,
> 	dma_addr_t *pphys_addr, u32 *pphys_addr_hi, u32 *pphys_addr_lo,
> 	u32 mem_size, u32 align);
> 
>+int pm8001_bar4_shift(struct pm8001_hba_info *pm8001_ha, u32 shiftValue);
> 
> /* ctl shared API */
> extern struct device_attribute *pm8001_host_attrs[];
>
>______________________________________________________________________
>This email may contain privileged or confidential information, which should only be used for the purpose for which it was sent by Xyratex. No further rights or licenses are granted to use such information. If you are not the intended recipient of this message, please notify the sender by return and delete it. You may not use, copy, disclose or rely on the information contained in it.
> 
>Internet email is susceptible to data corruption, interception and unauthorised amendment for which Xyratex does not accept liability. While we have taken reasonable precautions to ensure that this email is free of viruses, Xyratex does not accept liability for the presence of any computer viruses in this email, nor for any losses caused as a result of viruses.
> 
>Xyratex Technology Limited (03134912), Registered in England & Wales, Registered Office, Langstone Road, Havant, Hampshire, PO9 1SA.
> 
>The Xyratex group of companies also includes, Xyratex Ltd, registered in Bermuda, Xyratex International Inc, registered in California, Xyratex (Malaysia) Sdn Bhd registered in Malaysia, Xyratex Technology (Wuxi) Co Ltd registered in The People's Republic of China and Xyratex Japan Limited registered in Japan.
>______________________________________________________________________
> 
>
>
>__________ Information from ESET NOD32 Antivirus, version of virus signature database 5659 (20101129) __________
>
>The message was checked by ESET NOD32 Antivirus.
>
>http://www.eset.com
>
>
>

[PATCH] pm8001: Add FUNC_GET_EVENTS (take 2)

[Mark Salyzyn]

Jack noticed I dropped a patch fragment associated with a flags automatic variable in  mpi_set_phys_g3_with_ssc (ooops) and that the pre-emptive locking that piggy-backed this patch was not in-fact necessary because of underlying atomic accesses to the hardware. Here is the updated patch fixing these two issues.


The pm8001 driver is missing the FUNC_GET_EVENTS handler in the phy control function. Since the pm8001_bar4_shift function was not designed to be called at runtime, added locking surrounding the adjustment for all accesses.

Signed-off-by: mark_salyzyn@xyratex.com
Cc: jack_wang@usish.com
Cc: JBottomley@parallels.com
Cc: crystal_yu@usish.com
Cc: john_gong@usish.com
Cc: lindar_liu <lindar_liu@usish.com>

 drivers/scsi/pm8001/pm8001_hwi.c |   85 ++++++++++++++++++++++++++++++++++++++++-------------------
 drivers/scsi/pm8001/pm8001_sas.c |   24 +++++++++++++++-
 drivers/scsi/pm8001/pm8001_sas.h |    1 
 3 files changed, 83 insertions(+), 27 deletions(-)

diff --git a/drivers/scsi/pm8001/pm8001_hwi.c b/drivers/scsi/pm8001/pm8001_hwi.c
index b7b92f7..f3c44b9 100644
--- a/drivers/scsi/pm8001/pm8001_hwi.c
+++ b/drivers/scsi/pm8001/pm8001_hwi.c
@@ -338,26 +338,25 @@ update_outbnd_queue_table(struct pm8001_hba_info *pm8001_ha, int number)
 }
 
 /**
- * bar4_shift - function is called to shift BAR base address
- * @pm8001_ha : our hba card information
+ * pm8001_bar4_shift - function is called to shift BAR base address
+ * @pm8001_ha : our hba card infomation
  * @shiftValue : shifting value in memory bar.
  */
-static int bar4_shift(struct pm8001_hba_info *pm8001_ha, u32 shiftValue)
+int pm8001_bar4_shift(struct pm8001_hba_info *pm8001_ha, u32 shiftValue)
 {
 	u32 regVal;
-	u32 max_wait_count;
+	unsigned long start;
 
 	/* program the inbound AXI translation Lower Address */
 	pm8001_cw32(pm8001_ha, 1, SPC_IBW_AXI_TRANSLATION_LOW, shiftValue);
 
 	/* confirm the setting is written */
-	max_wait_count = 1 * 1000 * 1000;  /* 1 sec */
+	start = jiffies + HZ; /* 1 sec */
 	do {
-		udelay(1);
 		regVal = pm8001_cr32(pm8001_ha, 1, SPC_IBW_AXI_TRANSLATION_LOW);
-	} while ((regVal != shiftValue) && (--max_wait_count));
+	} while ((regVal != shiftValue) && time_before(jiffies, start));
 
-	if (!max_wait_count) {
+	if (regVal != shiftValue) {
 		PM8001_INIT_DBG(pm8001_ha,
 			pm8001_printk("TIMEOUT:SPC_IBW_AXI_TRANSLATION_LOW"
 			" = 0x%x\n", regVal));
@@ -375,6 +374,7 @@ static void __devinit
 mpi_set_phys_g3_with_ssc(struct pm8001_hba_info *pm8001_ha, u32 SSCbit)
 {
 	u32 value, offset, i;
+	unsigned long flags;
 
 #define SAS2_SETTINGS_LOCAL_PHY_0_3_SHIFT_ADDR 0x00030000
 #define SAS2_SETTINGS_LOCAL_PHY_4_7_SHIFT_ADDR 0x00040000
@@ -388,16 +388,23 @@ mpi_set_phys_g3_with_ssc(struct pm8001_hba_info *pm8001_ha, u32 SSCbit)
     * Using shifted destination address 0x3_0000:0x1074 + 0x4000*N (N=0:3)
     * Using shifted destination address 0x4_0000:0x1074 + 0x4000*(N-4) (N=4:7)
     */
-	if (-1 == bar4_shift(pm8001_ha, SAS2_SETTINGS_LOCAL_PHY_0_3_SHIFT_ADDR))
+	spin_lock_irqsave(&pm8001_ha->lock, flags);
+	if (-1 == pm8001_bar4_shift(pm8001_ha,
+				SAS2_SETTINGS_LOCAL_PHY_0_3_SHIFT_ADDR)) {
+		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
 		return;
+	}
 
 	for (i = 0; i < 4; i++) {
 		offset = SAS2_SETTINGS_LOCAL_PHY_0_3_OFFSET + 0x4000 * i;
 		pm8001_cw32(pm8001_ha, 2, offset, 0x80001501);
 	}
 	/* shift membase 3 for SAS2_SETTINGS_LOCAL_PHY 4 - 7 */
-	if (-1 == bar4_shift(pm8001_ha, SAS2_SETTINGS_LOCAL_PHY_4_7_SHIFT_ADDR))
+	if (-1 == pm8001_bar4_shift(pm8001_ha,
+				SAS2_SETTINGS_LOCAL_PHY_4_7_SHIFT_ADDR)) {
+		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
 		return;
+	}
 	for (i = 4; i < 8; i++) {
 		offset = SAS2_SETTINGS_LOCAL_PHY_4_7_OFFSET + 0x4000 * (i-4);
 		pm8001_cw32(pm8001_ha, 2, offset, 0x80001501);
@@ -421,7 +428,8 @@ mpi_set_phys_g3_with_ssc(struct pm8001_hba_info *pm8001_ha, u32 SSCbit)
 	pm8001_cw32(pm8001_ha, 2, 0xd8, 0x8000C016);
 
 	/*set the shifted destination address to 0x0 to avoid error operation */
-	bar4_shift(pm8001_ha, 0x0);
+	pm8001_bar4_shift(pm8001_ha, 0x0);
+	spin_unlock_irqrestore(&pm8001_ha->lock, flags);
 	return;
 }
 
@@ -437,6 +445,7 @@ mpi_set_open_retry_interval_reg(struct pm8001_hba_info *pm8001_ha,
 	u32 offset;
 	u32 value;
 	u32 i;
+	unsigned long flags;
 
 #define OPEN_RETRY_INTERVAL_PHY_0_3_SHIFT_ADDR 0x00030000
 #define OPEN_RETRY_INTERVAL_PHY_4_7_SHIFT_ADDR 0x00040000
@@ -445,24 +454,30 @@ mpi_set_open_retry_interval_reg(struct pm8001_hba_info *pm8001_ha,
 #define OPEN_RETRY_INTERVAL_REG_MASK 0x0000FFFF
 
 	value = interval & OPEN_RETRY_INTERVAL_REG_MASK;
+	spin_lock_irqsave(&pm8001_ha->lock, flags);
 	/* shift bar and set the OPEN_REJECT(RETRY) interval time of PHY 0 -3.*/
-	if (-1 == bar4_shift(pm8001_ha,
-			     OPEN_RETRY_INTERVAL_PHY_0_3_SHIFT_ADDR))
+	if (-1 == pm8001_bar4_shift(pm8001_ha,
+			     OPEN_RETRY_INTERVAL_PHY_0_3_SHIFT_ADDR)) {
+		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
 		return;
+	}
 	for (i = 0; i < 4; i++) {
 		offset = OPEN_RETRY_INTERVAL_PHY_0_3_OFFSET + 0x4000 * i;
 		pm8001_cw32(pm8001_ha, 2, offset, value);
 	}
 
-	if (-1 == bar4_shift(pm8001_ha,
-			     OPEN_RETRY_INTERVAL_PHY_4_7_SHIFT_ADDR))
+	if (-1 == pm8001_bar4_shift(pm8001_ha,
+			     OPEN_RETRY_INTERVAL_PHY_4_7_SHIFT_ADDR)) {
+		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
 		return;
+	}
 	for (i = 4; i < 8; i++) {
 		offset = OPEN_RETRY_INTERVAL_PHY_4_7_OFFSET + 0x4000 * (i-4);
 		pm8001_cw32(pm8001_ha, 2, offset, value);
 	}
 	/*set the shifted destination address to 0x0 to avoid error operation */
-	bar4_shift(pm8001_ha, 0x0);
+	pm8001_bar4_shift(pm8001_ha, 0x0);
+	spin_unlock_irqrestore(&pm8001_ha->lock, flags);
 	return;
 }
 
@@ -688,8 +703,11 @@ static u32 soft_reset_ready_check(struct pm8001_hba_info *pm8001_ha)
 		PM8001_INIT_DBG(pm8001_ha,
 			pm8001_printk("Firmware is ready for reset .\n"));
 	} else {
-	/* Trigger NMI twice via RB6 */
-		if (-1 == bar4_shift(pm8001_ha, RB6_ACCESS_REG)) {
+		unsigned long flags;
+		/* Trigger NMI twice via RB6 */
+		spin_lock_irqsave(&pm8001_ha->lock, flags);
+		if (-1 == pm8001_bar4_shift(pm8001_ha, RB6_ACCESS_REG)) {
+			spin_unlock_irqrestore(&pm8001_ha->lock, flags);
 			PM8001_FAIL_DBG(pm8001_ha,
 				pm8001_printk("Shift Bar4 to 0x%x failed\n",
 					RB6_ACCESS_REG));
@@ -715,8 +733,10 @@ static u32 soft_reset_ready_check(struct pm8001_hba_info *pm8001_ha)
 			PM8001_FAIL_DBG(pm8001_ha,
 				pm8001_printk("SCRATCH_PAD3 value = 0x%x\n",
 				pm8001_cr32(pm8001_ha, 0, MSGU_SCRATCH_PAD_3)));
+			spin_unlock_irqrestore(&pm8001_ha->lock, flags);
 			return -1;
 		}
+		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
 	}
 	return 0;
 }
@@ -733,6 +753,7 @@ pm8001_chip_soft_rst(struct pm8001_hba_info *pm8001_ha, u32 signature)
 	u32	regVal, toggleVal;
 	u32	max_wait_count;
 	u32	regVal1, regVal2, regVal3;
+	unsigned long flags;
 
 	/* step1: Check FW is ready for soft reset */
 	if (soft_reset_ready_check(pm8001_ha) != 0) {
@@ -743,7 +764,9 @@ pm8001_chip_soft_rst(struct pm8001_hba_info *pm8001_ha, u32 signature)
 	/* step 2: clear NMI status register on AAP1 and IOP, write the same
 	value to clear */
 	/* map 0x60000 to BAR4(0x20), BAR2(win) */
-	if (-1 == bar4_shift(pm8001_ha, MBIC_AAP1_ADDR_BASE)) {
+	spin_lock_irqsave(&pm8001_ha->lock, flags);
+	if (-1 == pm8001_bar4_shift(pm8001_ha, MBIC_AAP1_ADDR_BASE)) {
+		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
 		PM8001_FAIL_DBG(pm8001_ha,
 			pm8001_printk("Shift Bar4 to 0x%x failed\n",
 			MBIC_AAP1_ADDR_BASE));
@@ -754,7 +777,8 @@ pm8001_chip_soft_rst(struct pm8001_hba_info *pm8001_ha, u32 signature)
 		pm8001_printk("MBIC - NMI Enable VPE0 (IOP)= 0x%x\n", regVal));
 	pm8001_cw32(pm8001_ha, 2, MBIC_NMI_ENABLE_VPE0_IOP, 0x0);
 	/* map 0x70000 to BAR4(0x20), BAR2(win) */
-	if (-1 == bar4_shift(pm8001_ha, MBIC_IOP_ADDR_BASE)) {
+	if (-1 == pm8001_bar4_shift(pm8001_ha, MBIC_IOP_ADDR_BASE)) {
+		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
 		PM8001_FAIL_DBG(pm8001_ha,
 			pm8001_printk("Shift Bar4 to 0x%x failed\n",
 			MBIC_IOP_ADDR_BASE));
@@ -796,7 +820,8 @@ pm8001_chip_soft_rst(struct pm8001_hba_info *pm8001_ha, u32 signature)
 
 	/* read required registers for confirmming */
 	/* map 0x0700000 to BAR4(0x20), BAR2(win) */
-	if (-1 == bar4_shift(pm8001_ha, GSM_ADDR_BASE)) {
+	if (-1 == pm8001_bar4_shift(pm8001_ha, GSM_ADDR_BASE)) {
+		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
 		PM8001_FAIL_DBG(pm8001_ha,
 			pm8001_printk("Shift Bar4 to 0x%x failed\n",
 			GSM_ADDR_BASE));
@@ -862,7 +887,8 @@ pm8001_chip_soft_rst(struct pm8001_hba_info *pm8001_ha, u32 signature)
 	/* step 5: delay 10 usec */
 	udelay(10);
 	/* step 5-b: set GPIO-0 output control to tristate anyway */
-	if (-1 == bar4_shift(pm8001_ha, GPIO_ADDR_BASE)) {
+	if (-1 == pm8001_bar4_shift(pm8001_ha, GPIO_ADDR_BASE)) {
+		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
 		PM8001_INIT_DBG(pm8001_ha,
 				pm8001_printk("Shift Bar4 to 0x%x failed\n",
 				GPIO_ADDR_BASE));
@@ -878,7 +904,8 @@ pm8001_chip_soft_rst(struct pm8001_hba_info *pm8001_ha, u32 signature)
 
 	/* Step 6: Reset the IOP and AAP1 */
 	/* map 0x00000 to BAR4(0x20), BAR2(win) */
-	if (-1 == bar4_shift(pm8001_ha, SPC_TOP_LEVEL_ADDR_BASE)) {
+	if (-1 == pm8001_bar4_shift(pm8001_ha, SPC_TOP_LEVEL_ADDR_BASE)) {
+		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
 		PM8001_FAIL_DBG(pm8001_ha,
 			pm8001_printk("SPC Shift Bar4 to 0x%x failed\n",
 			SPC_TOP_LEVEL_ADDR_BASE));
@@ -915,7 +942,8 @@ pm8001_chip_soft_rst(struct pm8001_hba_info *pm8001_ha, u32 signature)
 
 	/* step 11: reads and sets the GSM Configuration and Reset Register */
 	/* map 0x0700000 to BAR4(0x20), BAR2(win) */
-	if (-1 == bar4_shift(pm8001_ha, GSM_ADDR_BASE)) {
+	if (-1 == pm8001_bar4_shift(pm8001_ha, GSM_ADDR_BASE)) {
+		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
 		PM8001_FAIL_DBG(pm8001_ha,
 			pm8001_printk("SPC Shift Bar4 to 0x%x failed\n",
 			GSM_ADDR_BASE));
@@ -968,7 +996,8 @@ pm8001_chip_soft_rst(struct pm8001_hba_info *pm8001_ha, u32 signature)
 
 	/* step 13: bring the IOP and AAP1 out of reset */
 	/* map 0x00000 to BAR4(0x20), BAR2(win) */
-	if (-1 == bar4_shift(pm8001_ha, SPC_TOP_LEVEL_ADDR_BASE)) {
+	if (-1 == pm8001_bar4_shift(pm8001_ha, SPC_TOP_LEVEL_ADDR_BASE)) {
+		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
 		PM8001_FAIL_DBG(pm8001_ha,
 			pm8001_printk("Shift Bar4 to 0x%x failed\n",
 			SPC_TOP_LEVEL_ADDR_BASE));
@@ -1010,6 +1039,7 @@ pm8001_chip_soft_rst(struct pm8001_hba_info *pm8001_ha, u32 signature)
 				pm8001_printk("SCRATCH_PAD3 value = 0x%x\n",
 				pm8001_cr32(pm8001_ha, 0,
 				MSGU_SCRATCH_PAD_3)));
+			spin_unlock_irqrestore(&pm8001_ha->lock, flags);
 			return -1;
 		}
 
@@ -1039,9 +1069,12 @@ pm8001_chip_soft_rst(struct pm8001_hba_info *pm8001_ha, u32 signature)
 				pm8001_printk("SCRATCH_PAD3 value = 0x%x\n",
 				pm8001_cr32(pm8001_ha, 0,
 				MSGU_SCRATCH_PAD_3)));
+			spin_unlock_irqrestore(&pm8001_ha->lock, flags);
 			return -1;
 		}
 	}
+	pm8001_bar4_shift(pm8001_ha, 0);
+	spin_unlock_irqrestore(&pm8001_ha->lock, flags);
 
 	PM8001_INIT_DBG(pm8001_ha,
 		pm8001_printk("SPC soft reset Complete\n"));
@@ -1157,8 +1190,8 @@ pm8001_chip_msix_interrupt_disable(struct pm8001_hba_info *pm8001_ha,
 	msi_index = int_vec_idx * MSIX_TABLE_ELEMENT_SIZE;
 	msi_index += MSIX_TABLE_BASE;
 	pm8001_cw32(pm8001_ha, 0,  msi_index, MSIX_INTERRUPT_DISABLE);
-
 }
+
 /**
  * pm8001_chip_interrupt_enable - enable PM8001 chip interrupt
  * @pm8001_ha: our hba card information
diff --git a/drivers/scsi/pm8001/pm8001_sas.c b/drivers/scsi/pm8001/pm8001_sas.c
index fb3dc99..b673524 100644
--- a/drivers/scsi/pm8001/pm8001_sas.c
+++ b/drivers/scsi/pm8001/pm8001_sas.c
@@ -166,6 +166,7 @@ int pm8001_phy_control(struct asd_sas_phy *sas_phy, enum phy_func func,
 	struct pm8001_hba_info *pm8001_ha = NULL;
 	struct sas_phy_linkrates *rates;
 	DECLARE_COMPLETION_ONSTACK(completion);
+	unsigned long flags;
 	pm8001_ha = sas_phy->ha->lldd_ha;
 	pm8001_ha->phy[phy_id].enable_completion = &completion;
 	switch (func) {
@@ -209,8 +210,29 @@ int pm8001_phy_control(struct asd_sas_phy *sas_phy, enum phy_func func,
 	case PHY_FUNC_DISABLE:
 		PM8001_CHIP_DISP->phy_stop_req(pm8001_ha, phy_id);
 		break;
+	case PHY_FUNC_GET_EVENTS:
+		spin_lock_irqsave(&pm8001_ha->lock, flags);
+		if (-1 == pm8001_bar4_shift(pm8001_ha,
+					(phy_id < 4) ? 0x30000 : 0x40000)) {
+			spin_unlock_irqrestore(&pm8001_ha->lock, flags);
+			return -EINVAL;
+		}
+		{
+			struct sas_phy *phy = sas_phy->phy;
+			uint32_t *qp = (uint32_t *)(((char *)
+				pm8001_ha->io_mem[2].memvirtaddr)
+				+ 0x1034 + (0x4000 * (phy_id & 3)));
+
+			phy->invalid_dword_count = qp[0];
+			phy->running_disparity_error_count = qp[1];
+			phy->loss_of_dword_sync_count = qp[3];
+			phy->phy_reset_problem_count = qp[4];
+		}
+		pm8001_bar4_shift(pm8001_ha, 0);
+		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
+		return 0;
 	default:
-		rc = -ENOSYS;
+		rc = -EOPNOTSUPP;
 	}
 	msleep(300);
 	return rc;
diff --git a/drivers/scsi/pm8001/pm8001_sas.h b/drivers/scsi/pm8001/pm8001_sas.h
index 93959fe..83a48f3 100644
--- a/drivers/scsi/pm8001/pm8001_sas.h
+++ b/drivers/scsi/pm8001/pm8001_sas.h
@@ -488,6 +488,7 @@ int pm8001_mem_alloc(struct pci_dev *pdev, void **virt_addr,
 	dma_addr_t *pphys_addr, u32 *pphys_addr_hi, u32 *pphys_addr_lo,
 	u32 mem_size, u32 align);
 
+int pm8001_bar4_shift(struct pm8001_hba_info *pm8001_ha, u32 shiftValue);
 
 /* ctl shared API */
 extern struct device_attribute *pm8001_host_attrs[];

Re: [PATCH] pm8001: Add FUNC_GET_EVENTS (take 2)

[jack_wang]

Ack-by: Jack Wang <jack_wang@usish.com>
Thanks.


--------------
jack_wang
>Jack noticed I dropped a patch fragment associated with a flags automatic variable in  mpi_set_phys_g3_with_ssc (ooops) and that the pre-emptive locking that piggy-backed this patch was not in-fact necessary because of underlying atomic accesses to the hardware. Here is the updated patch fixing these two issues.
>
>
>The pm8001 driver is missing the FUNC_GET_EVENTS handler in the phy control function. Since the pm8001_bar4_shift function was not designed to be called at runtime, added locking surrounding the adjustment for all accesses.
>
>Signed-off-by: mark_salyzyn@xyratex.com
>Cc: jack_wang@usish.com
>Cc: JBottomley@parallels.com
>Cc: crystal_yu@usish.com
>Cc: john_gong@usish.com
>Cc: lindar_liu <lindar_liu@usish.com>
>
> drivers/scsi/pm8001/pm8001_hwi.c |   85 ++++++++++++++++++++++++++++++++++++++++-------------------
> drivers/scsi/pm8001/pm8001_sas.c |   24 +++++++++++++++-
> drivers/scsi/pm8001/pm8001_sas.h |    1 
> 3 files changed, 83 insertions(+), 27 deletions(-)
>
>diff --git a/drivers/scsi/pm8001/pm8001_hwi.c b/drivers/scsi/pm8001/pm8001_hwi.c
>index b7b92f7..f3c44b9 100644
>--- a/drivers/scsi/pm8001/pm8001_hwi.c
>+++ b/drivers/scsi/pm8001/pm8001_hwi.c
>@@ -338,26 +338,25 @@ update_outbnd_queue_table(struct pm8001_hba_info *pm8001_ha, int number)
> }
> 
> /**
>- * bar4_shift - function is called to shift BAR base address
>- * @pm8001_ha : our hba card information
>+ * pm8001_bar4_shift - function is called to shift BAR base address
>+ * @pm8001_ha : our hba card infomation
>  * @shiftValue : shifting value in memory bar.
>  */
>-static int bar4_shift(struct pm8001_hba_info *pm8001_ha, u32 shiftValue)
>+int pm8001_bar4_shift(struct pm8001_hba_info *pm8001_ha, u32 shiftValue)
> {
> 	u32 regVal;
>-	u32 max_wait_count;
>+	unsigned long start;
> 
> 	/* program the inbound AXI translation Lower Address */
> 	pm8001_cw32(pm8001_ha, 1, SPC_IBW_AXI_TRANSLATION_LOW, shiftValue);
> 
> 	/* confirm the setting is written */
>-	max_wait_count = 1 * 1000 * 1000;  /* 1 sec */
>+	start = jiffies + HZ; /* 1 sec */
> 	do {
>-		udelay(1);
> 		regVal = pm8001_cr32(pm8001_ha, 1, SPC_IBW_AXI_TRANSLATION_LOW);
>-	} while ((regVal != shiftValue) && (--max_wait_count));
>+	} while ((regVal != shiftValue) && time_before(jiffies, start));
> 
>-	if (!max_wait_count) {
>+	if (regVal != shiftValue) {
> 		PM8001_INIT_DBG(pm8001_ha,
> 			pm8001_printk("TIMEOUT:SPC_IBW_AXI_TRANSLATION_LOW"
> 			" = 0x%x\n", regVal));
>@@ -375,6 +374,7 @@ static void __devinit
> mpi_set_phys_g3_with_ssc(struct pm8001_hba_info *pm8001_ha, u32 SSCbit)
> {
> 	u32 value, offset, i;
>+	unsigned long flags;
> 
> #define SAS2_SETTINGS_LOCAL_PHY_0_3_SHIFT_ADDR 0x00030000
> #define SAS2_SETTINGS_LOCAL_PHY_4_7_SHIFT_ADDR 0x00040000
>@@ -388,16 +388,23 @@ mpi_set_phys_g3_with_ssc(struct pm8001_hba_info *pm8001_ha, u32 SSCbit)
>     * Using shifted destination address 0x3_0000:0x1074 + 0x4000*N (N=0:3)
>     * Using shifted destination address 0x4_0000:0x1074 + 0x4000*(N-4) (N=4:7)
>     */
>-	if (-1 == bar4_shift(pm8001_ha, SAS2_SETTINGS_LOCAL_PHY_0_3_SHIFT_ADDR))
>+	spin_lock_irqsave(&pm8001_ha->lock, flags);
>+	if (-1 == pm8001_bar4_shift(pm8001_ha,
>+				SAS2_SETTINGS_LOCAL_PHY_0_3_SHIFT_ADDR)) {
>+		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
> 		return;
>+	}
> 
> 	for (i = 0; i < 4; i++) {
> 		offset = SAS2_SETTINGS_LOCAL_PHY_0_3_OFFSET + 0x4000 * i;
> 		pm8001_cw32(pm8001_ha, 2, offset, 0x80001501);
> 	}
> 	/* shift membase 3 for SAS2_SETTINGS_LOCAL_PHY 4 - 7 */
>-	if (-1 == bar4_shift(pm8001_ha, SAS2_SETTINGS_LOCAL_PHY_4_7_SHIFT_ADDR))
>+	if (-1 == pm8001_bar4_shift(pm8001_ha,
>+				SAS2_SETTINGS_LOCAL_PHY_4_7_SHIFT_ADDR)) {
>+		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
> 		return;
>+	}
> 	for (i = 4; i < 8; i++) {
> 		offset = SAS2_SETTINGS_LOCAL_PHY_4_7_OFFSET + 0x4000 * (i-4);
> 		pm8001_cw32(pm8001_ha, 2, offset, 0x80001501);
>@@ -421,7 +428,8 @@ mpi_set_phys_g3_with_ssc(struct pm8001_hba_info *pm8001_ha, u32 SSCbit)
> 	pm8001_cw32(pm8001_ha, 2, 0xd8, 0x8000C016);
> 
> 	/*set the shifted destination address to 0x0 to avoid error operation */
>-	bar4_shift(pm8001_ha, 0x0);
>+	pm8001_bar4_shift(pm8001_ha, 0x0);
>+	spin_unlock_irqrestore(&pm8001_ha->lock, flags);
> 	return;
> }
> 
>@@ -437,6 +445,7 @@ mpi_set_open_retry_interval_reg(struct pm8001_hba_info *pm8001_ha,
> 	u32 offset;
> 	u32 value;
> 	u32 i;
>+	unsigned long flags;
> 
> #define OPEN_RETRY_INTERVAL_PHY_0_3_SHIFT_ADDR 0x00030000
> #define OPEN_RETRY_INTERVAL_PHY_4_7_SHIFT_ADDR 0x00040000
>@@ -445,24 +454,30 @@ mpi_set_open_retry_interval_reg(struct pm8001_hba_info *pm8001_ha,
> #define OPEN_RETRY_INTERVAL_REG_MASK 0x0000FFFF
> 
> 	value = interval & OPEN_RETRY_INTERVAL_REG_MASK;
>+	spin_lock_irqsave(&pm8001_ha->lock, flags);
> 	/* shift bar and set the OPEN_REJECT(RETRY) interval time of PHY 0 -3.*/
>-	if (-1 == bar4_shift(pm8001_ha,
>-			     OPEN_RETRY_INTERVAL_PHY_0_3_SHIFT_ADDR))
>+	if (-1 == pm8001_bar4_shift(pm8001_ha,
>+			     OPEN_RETRY_INTERVAL_PHY_0_3_SHIFT_ADDR)) {
>+		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
> 		return;
>+	}
> 	for (i = 0; i < 4; i++) {
> 		offset = OPEN_RETRY_INTERVAL_PHY_0_3_OFFSET + 0x4000 * i;
> 		pm8001_cw32(pm8001_ha, 2, offset, value);
> 	}
> 
>-	if (-1 == bar4_shift(pm8001_ha,
>-			     OPEN_RETRY_INTERVAL_PHY_4_7_SHIFT_ADDR))
>+	if (-1 == pm8001_bar4_shift(pm8001_ha,
>+			     OPEN_RETRY_INTERVAL_PHY_4_7_SHIFT_ADDR)) {
>+		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
> 		return;
>+	}
> 	for (i = 4; i < 8; i++) {
> 		offset = OPEN_RETRY_INTERVAL_PHY_4_7_OFFSET + 0x4000 * (i-4);
> 		pm8001_cw32(pm8001_ha, 2, offset, value);
> 	}
> 	/*set the shifted destination address to 0x0 to avoid error operation */
>-	bar4_shift(pm8001_ha, 0x0);
>+	pm8001_bar4_shift(pm8001_ha, 0x0);
>+	spin_unlock_irqrestore(&pm8001_ha->lock, flags);
> 	return;
> }
> 
>@@ -688,8 +703,11 @@ static u32 soft_reset_ready_check(struct pm8001_hba_info *pm8001_ha)
> 		PM8001_INIT_DBG(pm8001_ha,
> 			pm8001_printk("Firmware is ready for reset .\n"));
> 	} else {
>-	/* Trigger NMI twice via RB6 */
>-		if (-1 == bar4_shift(pm8001_ha, RB6_ACCESS_REG)) {
>+		unsigned long flags;
>+		/* Trigger NMI twice via RB6 */
>+		spin_lock_irqsave(&pm8001_ha->lock, flags);
>+		if (-1 == pm8001_bar4_shift(pm8001_ha, RB6_ACCESS_REG)) {
>+			spin_unlock_irqrestore(&pm8001_ha->lock, flags);
> 			PM8001_FAIL_DBG(pm8001_ha,
> 				pm8001_printk("Shift Bar4 to 0x%x failed\n",
> 					RB6_ACCESS_REG));
>@@ -715,8 +733,10 @@ static u32 soft_reset_ready_check(struct pm8001_hba_info *pm8001_ha)
> 			PM8001_FAIL_DBG(pm8001_ha,
> 				pm8001_printk("SCRATCH_PAD3 value = 0x%x\n",
> 				pm8001_cr32(pm8001_ha, 0, MSGU_SCRATCH_PAD_3)));
>+			spin_unlock_irqrestore(&pm8001_ha->lock, flags);
> 			return -1;
> 		}
>+		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
> 	}
> 	return 0;
> }
>@@ -733,6 +753,7 @@ pm8001_chip_soft_rst(struct pm8001_hba_info *pm8001_ha, u32 signature)
> 	u32	regVal, toggleVal;
> 	u32	max_wait_count;
> 	u32	regVal1, regVal2, regVal3;
>+	unsigned long flags;
> 
> 	/* step1: Check FW is ready for soft reset */
> 	if (soft_reset_ready_check(pm8001_ha) != 0) {
>@@ -743,7 +764,9 @@ pm8001_chip_soft_rst(struct pm8001_hba_info *pm8001_ha, u32 signature)
> 	/* step 2: clear NMI status register on AAP1 and IOP, write the same
> 	value to clear */
> 	/* map 0x60000 to BAR4(0x20), BAR2(win) */
>-	if (-1 == bar4_shift(pm8001_ha, MBIC_AAP1_ADDR_BASE)) {
>+	spin_lock_irqsave(&pm8001_ha->lock, flags);
>+	if (-1 == pm8001_bar4_shift(pm8001_ha, MBIC_AAP1_ADDR_BASE)) {
>+		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
> 		PM8001_FAIL_DBG(pm8001_ha,
> 			pm8001_printk("Shift Bar4 to 0x%x failed\n",
> 			MBIC_AAP1_ADDR_BASE));
>@@ -754,7 +777,8 @@ pm8001_chip_soft_rst(struct pm8001_hba_info *pm8001_ha, u32 signature)
> 		pm8001_printk("MBIC - NMI Enable VPE0 (IOP)= 0x%x\n", regVal));
> 	pm8001_cw32(pm8001_ha, 2, MBIC_NMI_ENABLE_VPE0_IOP, 0x0);
> 	/* map 0x70000 to BAR4(0x20), BAR2(win) */
>-	if (-1 == bar4_shift(pm8001_ha, MBIC_IOP_ADDR_BASE)) {
>+	if (-1 == pm8001_bar4_shift(pm8001_ha, MBIC_IOP_ADDR_BASE)) {
>+		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
> 		PM8001_FAIL_DBG(pm8001_ha,
> 			pm8001_printk("Shift Bar4 to 0x%x failed\n",
> 			MBIC_IOP_ADDR_BASE));
>@@ -796,7 +820,8 @@ pm8001_chip_soft_rst(struct pm8001_hba_info *pm8001_ha, u32 signature)
> 
> 	/* read required registers for confirmming */
> 	/* map 0x0700000 to BAR4(0x20), BAR2(win) */
>-	if (-1 == bar4_shift(pm8001_ha, GSM_ADDR_BASE)) {
>+	if (-1 == pm8001_bar4_shift(pm8001_ha, GSM_ADDR_BASE)) {
>+		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
> 		PM8001_FAIL_DBG(pm8001_ha,
> 			pm8001_printk("Shift Bar4 to 0x%x failed\n",
> 			GSM_ADDR_BASE));
>@@ -862,7 +887,8 @@ pm8001_chip_soft_rst(struct pm8001_hba_info *pm8001_ha, u32 signature)
> 	/* step 5: delay 10 usec */
> 	udelay(10);
> 	/* step 5-b: set GPIO-0 output control to tristate anyway */
>-	if (-1 == bar4_shift(pm8001_ha, GPIO_ADDR_BASE)) {
>+	if (-1 == pm8001_bar4_shift(pm8001_ha, GPIO_ADDR_BASE)) {
>+		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
> 		PM8001_INIT_DBG(pm8001_ha,
> 				pm8001_printk("Shift Bar4 to 0x%x failed\n",
> 				GPIO_ADDR_BASE));
>@@ -878,7 +904,8 @@ pm8001_chip_soft_rst(struct pm8001_hba_info *pm8001_ha, u32 signature)
> 
> 	/* Step 6: Reset the IOP and AAP1 */
> 	/* map 0x00000 to BAR4(0x20), BAR2(win) */
>-	if (-1 == bar4_shift(pm8001_ha, SPC_TOP_LEVEL_ADDR_BASE)) {
>+	if (-1 == pm8001_bar4_shift(pm8001_ha, SPC_TOP_LEVEL_ADDR_BASE)) {
>+		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
> 		PM8001_FAIL_DBG(pm8001_ha,
> 			pm8001_printk("SPC Shift Bar4 to 0x%x failed\n",
> 			SPC_TOP_LEVEL_ADDR_BASE));
>@@ -915,7 +942,8 @@ pm8001_chip_soft_rst(struct pm8001_hba_info *pm8001_ha, u32 signature)
> 
> 	/* step 11: reads and sets the GSM Configuration and Reset Register */
> 	/* map 0x0700000 to BAR4(0x20), BAR2(win) */
>-	if (-1 == bar4_shift(pm8001_ha, GSM_ADDR_BASE)) {
>+	if (-1 == pm8001_bar4_shift(pm8001_ha, GSM_ADDR_BASE)) {
>+		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
> 		PM8001_FAIL_DBG(pm8001_ha,
> 			pm8001_printk("SPC Shift Bar4 to 0x%x failed\n",
> 			GSM_ADDR_BASE));
>@@ -968,7 +996,8 @@ pm8001_chip_soft_rst(struct pm8001_hba_info *pm8001_ha, u32 signature)
> 
> 	/* step 13: bring the IOP and AAP1 out of reset */
> 	/* map 0x00000 to BAR4(0x20), BAR2(win) */
>-	if (-1 == bar4_shift(pm8001_ha, SPC_TOP_LEVEL_ADDR_BASE)) {
>+	if (-1 == pm8001_bar4_shift(pm8001_ha, SPC_TOP_LEVEL_ADDR_BASE)) {
>+		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
> 		PM8001_FAIL_DBG(pm8001_ha,
> 			pm8001_printk("Shift Bar4 to 0x%x failed\n",
> 			SPC_TOP_LEVEL_ADDR_BASE));
>@@ -1010,6 +1039,7 @@ pm8001_chip_soft_rst(struct pm8001_hba_info *pm8001_ha, u32 signature)
> 				pm8001_printk("SCRATCH_PAD3 value = 0x%x\n",
> 				pm8001_cr32(pm8001_ha, 0,
> 				MSGU_SCRATCH_PAD_3)));
>+			spin_unlock_irqrestore(&pm8001_ha->lock, flags);
> 			return -1;
> 		}
> 
>@@ -1039,9 +1069,12 @@ pm8001_chip_soft_rst(struct pm8001_hba_info *pm8001_ha, u32 signature)
> 				pm8001_printk("SCRATCH_PAD3 value = 0x%x\n",
> 				pm8001_cr32(pm8001_ha, 0,
> 				MSGU_SCRATCH_PAD_3)));
>+			spin_unlock_irqrestore(&pm8001_ha->lock, flags);
> 			return -1;
> 		}
> 	}
>+	pm8001_bar4_shift(pm8001_ha, 0);
>+	spin_unlock_irqrestore(&pm8001_ha->lock, flags);
> 
> 	PM8001_INIT_DBG(pm8001_ha,
> 		pm8001_printk("SPC soft reset Complete\n"));
>@@ -1157,8 +1190,8 @@ pm8001_chip_msix_interrupt_disable(struct pm8001_hba_info *pm8001_ha,
> 	msi_index = int_vec_idx * MSIX_TABLE_ELEMENT_SIZE;
> 	msi_index += MSIX_TABLE_BASE;
> 	pm8001_cw32(pm8001_ha, 0,  msi_index, MSIX_INTERRUPT_DISABLE);
>-
> }
>+
> /**
>  * pm8001_chip_interrupt_enable - enable PM8001 chip interrupt
>  * @pm8001_ha: our hba card information
>diff --git a/drivers/scsi/pm8001/pm8001_sas.c b/drivers/scsi/pm8001/pm8001_sas.c
>index fb3dc99..b673524 100644
>--- a/drivers/scsi/pm8001/pm8001_sas.c
>+++ b/drivers/scsi/pm8001/pm8001_sas.c
>@@ -166,6 +166,7 @@ int pm8001_phy_control(struct asd_sas_phy *sas_phy, enum phy_func func,
> 	struct pm8001_hba_info *pm8001_ha = NULL;
> 	struct sas_phy_linkrates *rates;
> 	DECLARE_COMPLETION_ONSTACK(completion);
>+	unsigned long flags;
> 	pm8001_ha = sas_phy->ha->lldd_ha;
> 	pm8001_ha->phy[phy_id].enable_completion = &completion;
> 	switch (func) {
>@@ -209,8 +210,29 @@ int pm8001_phy_control(struct asd_sas_phy *sas_phy, enum phy_func func,
> 	case PHY_FUNC_DISABLE:
> 		PM8001_CHIP_DISP->phy_stop_req(pm8001_ha, phy_id);
> 		break;
>+	case PHY_FUNC_GET_EVENTS:
>+		spin_lock_irqsave(&pm8001_ha->lock, flags);
>+		if (-1 == pm8001_bar4_shift(pm8001_ha,
>+					(phy_id < 4) ? 0x30000 : 0x40000)) {
>+			spin_unlock_irqrestore(&pm8001_ha->lock, flags);
>+			return -EINVAL;
>+		}
>+		{
>+			struct sas_phy *phy = sas_phy->phy;
>+			uint32_t *qp = (uint32_t *)(((char *)
>+				pm8001_ha->io_mem[2].memvirtaddr)
>+				+ 0x1034 + (0x4000 * (phy_id & 3)));
>+
>+			phy->invalid_dword_count = qp[0];
>+			phy->running_disparity_error_count = qp[1];
>+			phy->loss_of_dword_sync_count = qp[3];
>+			phy->phy_reset_problem_count = qp[4];
>+		}
>+		pm8001_bar4_shift(pm8001_ha, 0);
>+		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
>+		return 0;
> 	default:
>-		rc = -ENOSYS;
>+		rc = -EOPNOTSUPP;
> 	}
> 	msleep(300);
> 	return rc;
>diff --git a/drivers/scsi/pm8001/pm8001_sas.h b/drivers/scsi/pm8001/pm8001_sas.h
>index 93959fe..83a48f3 100644
>--- a/drivers/scsi/pm8001/pm8001_sas.h
>+++ b/drivers/scsi/pm8001/pm8001_sas.h
>@@ -488,6 +488,7 @@ int pm8001_mem_alloc(struct pci_dev *pdev, void **virt_addr,
> 	dma_addr_t *pphys_addr, u32 *pphys_addr_hi, u32 *pphys_addr_lo,
> 	u32 mem_size, u32 align);
> 
>+int pm8001_bar4_shift(struct pm8001_hba_info *pm8001_ha, u32 shiftValue);
> 
> /* ctl shared API */
> extern struct device_attribute *pm8001_host_attrs[];
>
>______________________________________________________________________
>This email may contain privileged or confidential information, which should only be used for the purpose for which it was sent by Xyratex. No further rights or licenses are granted to use such information. If you are not the intended recipient of this message, please notify the sender by return and delete it. You may not use, copy, disclose or rely on the information contained in it.
> 
>Internet email is susceptible to data corruption, interception and unauthorised amendment for which Xyratex does not accept liability. While we have taken reasonable precautions to ensure that this email is free of viruses, Xyratex does not accept liability for the presence of any computer viruses in this email, nor for any losses caused as a result of viruses.
> 
>Xyratex Technology Limited (03134912), Registered in England & Wales, Registered Office, Langstone Road, Havant, Hampshire, PO9 1SA.
> 
>The Xyratex group of companies also includes, Xyratex Ltd, registered in Bermuda, Xyratex International Inc, registered in California, Xyratex (Malaysia) Sdn Bhd registered in Malaysia, Xyratex Technology (Wuxi) Co Ltd registered in The People's Republic of China and Xyratex Japan Limited registered in Japan.
>______________________________________________________________________
> 
>
>
>__________ Information from ESET NOD32 Antivirus, version of virus signature database 5659 (20101129) __________
>
>The message was checked by ESET NOD32 Antivirus.
>
>http://www.eset.com
>
>
>

Re: [PATCH] pm8001: Add FUNC_GET_EVENTS (take 2)

[Jack Wang]

Dear Mark,

This patch seems broken, I see this when apply:

patch -p1 < pm8001_get_event.patch 
patching file drivers/scsi/pm8001/pm8001_hwi.c
patch: **** malformed patch at line 56: @@ -375,6 +374,7 @@ static void
__devinit  mpi_set_phys_g3_with_ssc(struct pm8001_hba_info *pm8001_ha, u32
SSCbit)  {

> Jack noticed I dropped a patch fragment associated with a flags automatic
> variable in  mpi_set_phys_g3_with_ssc (ooops) and that the pre-emptive
> locking that piggy-backed this patch was not in-fact necessary because of
> underlying atomic accesses to the hardware. Here is the updated patch
fixing
> these two issues.
> 
> 
> The pm8001 driver is missing the FUNC_GET_EVENTS handler in the phy
control
> function. Since the pm8001_bar4_shift function was not designed to be
called
> at runtime, added locking surrounding the adjustment for all accesses.
> 
> Signed-off-by: mark_salyzyn@xyratex.com
> Cc: jack_wang@usish.com
> Cc: JBottomley@parallels.com
> Cc: crystal_yu@usish.com
> Cc: john_gong@usish.com
> Cc: lindar_liu <lindar_liu@usish.com>
> 
>  drivers/scsi/pm8001/pm8001_hwi.c |   85
> ++++++++++++++++++++++++++++++++++++++++-------------------
>  drivers/scsi/pm8001/pm8001_sas.c |   24 +++++++++++++++-
>  drivers/scsi/pm8001/pm8001_sas.h |    1
>  3 files changed, 83 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/scsi/pm8001/pm8001_hwi.c
> b/drivers/scsi/pm8001/pm8001_hwi.c
> index b7b92f7..f3c44b9 100644
> --- a/drivers/scsi/pm8001/pm8001_hwi.c
> +++ b/drivers/scsi/pm8001/pm8001_hwi.c
> @@ -338,26 +338,25 @@ update_outbnd_queue_table(struct pm8001_hba_info
> *pm8001_ha, int number)
>  }
> 
>  /**
> - * bar4_shift - function is called to shift BAR base address
> - * @pm8001_ha : our hba card information
> + * pm8001_bar4_shift - function is called to shift BAR base address
> + * @pm8001_ha : our hba card infomation
>   * @shiftValue : shifting value in memory bar.
>   */
> -static int bar4_shift(struct pm8001_hba_info *pm8001_ha, u32 shiftValue)
> +int pm8001_bar4_shift(struct pm8001_hba_info *pm8001_ha, u32 shiftValue)
>  {
>  	u32 regVal;
> -	u32 max_wait_count;
> +	unsigned long start;
> 
>  	/* program the inbound AXI translation Lower Address */
>  	pm8001_cw32(pm8001_ha, 1, SPC_IBW_AXI_TRANSLATION_LOW, shiftValue);
> 
>  	/* confirm the setting is written */
> -	max_wait_count = 1 * 1000 * 1000;  /* 1 sec */
> +	start = jiffies + HZ; /* 1 sec */
>  	do {
> -		udelay(1);
>  		regVal = pm8001_cr32(pm8001_ha, 1,
SPC_IBW_AXI_TRANSLATION_LOW);
> -	} while ((regVal != shiftValue) && (--max_wait_count));
> +	} while ((regVal != shiftValue) && time_before(jiffies, start));
> 
> -	if (!max_wait_count) {
> +	if (regVal != shiftValue) {
>  		PM8001_INIT_DBG(pm8001_ha,
>  			pm8001_printk("TIMEOUT:SPC_IBW_AXI_TRANSLATION_LOW"
>  			" = 0x%x\n", regVal));
> @@ -375,6 +374,7 @@ static void __devinit
>  mpi_set_phys_g3_with_ssc(struct pm8001_hba_info *pm8001_ha, u32 SSCbit)
>  {
>  	u32 value, offset, i;
> +	unsigned long flags;
> 
>  #define SAS2_SETTINGS_LOCAL_PHY_0_3_SHIFT_ADDR 0x00030000
>  #define SAS2_SETTINGS_LOCAL_PHY_4_7_SHIFT_ADDR 0x00040000
> @@ -388,16 +388,23 @@ mpi_set_phys_g3_with_ssc(struct pm8001_hba_info
> *pm8001_ha, u32 SSCbit)
>      * Using shifted destination address 0x3_0000:0x1074 + 0x4000*N
(N=0:3)
>      * Using shifted destination address 0x4_0000:0x1074 + 0x4000*(N-4)
> (N=4:7)
>      */
> -	if (-1 == bar4_shift(pm8001_ha,
> SAS2_SETTINGS_LOCAL_PHY_0_3_SHIFT_ADDR))
> +	spin_lock_irqsave(&pm8001_ha->lock, flags);
> +	if (-1 == pm8001_bar4_shift(pm8001_ha,
> +				SAS2_SETTINGS_LOCAL_PHY_0_3_SHIFT_ADDR)) {
> +		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
>  		return;
> +	}
> 
>  	for (i = 0; i < 4; i++) {
>  		offset = SAS2_SETTINGS_LOCAL_PHY_0_3_OFFSET + 0x4000 * i;
>  		pm8001_cw32(pm8001_ha, 2, offset, 0x80001501);
>  	}
>  	/* shift membase 3 for SAS2_SETTINGS_LOCAL_PHY 4 - 7 */
> -	if (-1 == bar4_shift(pm8001_ha,
> SAS2_SETTINGS_LOCAL_PHY_4_7_SHIFT_ADDR))
> +	if (-1 == pm8001_bar4_shift(pm8001_ha,
> +				SAS2_SETTINGS_LOCAL_PHY_4_7_SHIFT_ADDR)) {
> +		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
>  		return;
> +	}
>  	for (i = 4; i < 8; i++) {
>  		offset = SAS2_SETTINGS_LOCAL_PHY_4_7_OFFSET + 0x4000 *
(i-4);
>  		pm8001_cw32(pm8001_ha, 2, offset, 0x80001501);
> @@ -421,7 +428,8 @@ mpi_set_phys_g3_with_ssc(struct pm8001_hba_info
> *pm8001_ha, u32 SSCbit)
>  	pm8001_cw32(pm8001_ha, 2, 0xd8, 0x8000C016);
> 
>  	/*set the shifted destination address to 0x0 to avoid error
operation */
> -	bar4_shift(pm8001_ha, 0x0);
> +	pm8001_bar4_shift(pm8001_ha, 0x0);
> +	spin_unlock_irqrestore(&pm8001_ha->lock, flags);
>  	return;
>  }
> 
> @@ -437,6 +445,7 @@ mpi_set_open_retry_interval_reg(struct pm8001_hba_info
> *pm8001_ha,
>  	u32 offset;
>  	u32 value;
>  	u32 i;
> +	unsigned long flags;
> 
>  #define OPEN_RETRY_INTERVAL_PHY_0_3_SHIFT_ADDR 0x00030000
>  #define OPEN_RETRY_INTERVAL_PHY_4_7_SHIFT_ADDR 0x00040000
> @@ -445,24 +454,30 @@ mpi_set_open_retry_interval_reg(struct
pm8001_hba_info
> *pm8001_ha,
>  #define OPEN_RETRY_INTERVAL_REG_MASK 0x0000FFFF
> 
>  	value = interval & OPEN_RETRY_INTERVAL_REG_MASK;
> +	spin_lock_irqsave(&pm8001_ha->lock, flags);
>  	/* shift bar and set the OPEN_REJECT(RETRY) interval time of PHY 0
-3.*/
> -	if (-1 == bar4_shift(pm8001_ha,
> -			     OPEN_RETRY_INTERVAL_PHY_0_3_SHIFT_ADDR))
> +	if (-1 == pm8001_bar4_shift(pm8001_ha,
> +			     OPEN_RETRY_INTERVAL_PHY_0_3_SHIFT_ADDR)) {
> +		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
>  		return;
> +	}
>  	for (i = 0; i < 4; i++) {
>  		offset = OPEN_RETRY_INTERVAL_PHY_0_3_OFFSET + 0x4000 * i;
>  		pm8001_cw32(pm8001_ha, 2, offset, value);
>  	}
> 
> -	if (-1 == bar4_shift(pm8001_ha,
> -			     OPEN_RETRY_INTERVAL_PHY_4_7_SHIFT_ADDR))
> +	if (-1 == pm8001_bar4_shift(pm8001_ha,
> +			     OPEN_RETRY_INTERVAL_PHY_4_7_SHIFT_ADDR)) {
> +		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
>  		return;
> +	}
>  	for (i = 4; i < 8; i++) {
>  		offset = OPEN_RETRY_INTERVAL_PHY_4_7_OFFSET + 0x4000 *
(i-4);
>  		pm8001_cw32(pm8001_ha, 2, offset, value);
>  	}
>  	/*set the shifted destination address to 0x0 to avoid error
operation */
> -	bar4_shift(pm8001_ha, 0x0);
> +	pm8001_bar4_shift(pm8001_ha, 0x0);
> +	spin_unlock_irqrestore(&pm8001_ha->lock, flags);
>  	return;
>  }
> 
> @@ -688,8 +703,11 @@ static u32 soft_reset_ready_check(struct
pm8001_hba_info
> *pm8001_ha)
>  		PM8001_INIT_DBG(pm8001_ha,
>  			pm8001_printk("Firmware is ready for reset .\n"));
>  	} else {
> -	/* Trigger NMI twice via RB6 */
> -		if (-1 == bar4_shift(pm8001_ha, RB6_ACCESS_REG)) {
> +		unsigned long flags;
> +		/* Trigger NMI twice via RB6 */
> +		spin_lock_irqsave(&pm8001_ha->lock, flags);
> +		if (-1 == pm8001_bar4_shift(pm8001_ha, RB6_ACCESS_REG)) {
> +			spin_unlock_irqrestore(&pm8001_ha->lock, flags);
>  			PM8001_FAIL_DBG(pm8001_ha,
>  				pm8001_printk("Shift Bar4 to 0x%x failed\n",
>  					RB6_ACCESS_REG));
> @@ -715,8 +733,10 @@ static u32 soft_reset_ready_check(struct
pm8001_hba_info
> *pm8001_ha)
>  			PM8001_FAIL_DBG(pm8001_ha,
>  				pm8001_printk("SCRATCH_PAD3 value = 0x%x\n",
>  				pm8001_cr32(pm8001_ha, 0,
MSGU_SCRATCH_PAD_3)));
> +			spin_unlock_irqrestore(&pm8001_ha->lock, flags);
>  			return -1;
>  		}
> +		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
>  	}
>  	return 0;
>  }
> @@ -733,6 +753,7 @@ pm8001_chip_soft_rst(struct pm8001_hba_info
*pm8001_ha,
> u32 signature)
>  	u32	regVal, toggleVal;
>  	u32	max_wait_count;
>  	u32	regVal1, regVal2, regVal3;
> +	unsigned long flags;
> 
>  	/* step1: Check FW is ready for soft reset */
>  	if (soft_reset_ready_check(pm8001_ha) != 0) {
> @@ -743,7 +764,9 @@ pm8001_chip_soft_rst(struct pm8001_hba_info
*pm8001_ha,
> u32 signature)
>  	/* step 2: clear NMI status register on AAP1 and IOP, write the same
>  	value to clear */
>  	/* map 0x60000 to BAR4(0x20), BAR2(win) */
> -	if (-1 == bar4_shift(pm8001_ha, MBIC_AAP1_ADDR_BASE)) {
> +	spin_lock_irqsave(&pm8001_ha->lock, flags);
> +	if (-1 == pm8001_bar4_shift(pm8001_ha, MBIC_AAP1_ADDR_BASE)) {
> +		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
>  		PM8001_FAIL_DBG(pm8001_ha,
>  			pm8001_printk("Shift Bar4 to 0x%x failed\n",
>  			MBIC_AAP1_ADDR_BASE));
> @@ -754,7 +777,8 @@ pm8001_chip_soft_rst(struct pm8001_hba_info
*pm8001_ha,
> u32 signature)
>  		pm8001_printk("MBIC - NMI Enable VPE0 (IOP)= 0x%x\n",
regVal));
>  	pm8001_cw32(pm8001_ha, 2, MBIC_NMI_ENABLE_VPE0_IOP, 0x0);
>  	/* map 0x70000 to BAR4(0x20), BAR2(win) */
> -	if (-1 == bar4_shift(pm8001_ha, MBIC_IOP_ADDR_BASE)) {
> +	if (-1 == pm8001_bar4_shift(pm8001_ha, MBIC_IOP_ADDR_BASE)) {
> +		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
>  		PM8001_FAIL_DBG(pm8001_ha,
>  			pm8001_printk("Shift Bar4 to 0x%x failed\n",
>  			MBIC_IOP_ADDR_BASE));
> @@ -796,7 +820,8 @@ pm8001_chip_soft_rst(struct pm8001_hba_info
*pm8001_ha,
> u32 signature)
> 
>  	/* read required registers for confirmming */
>  	/* map 0x0700000 to BAR4(0x20), BAR2(win) */
> -	if (-1 == bar4_shift(pm8001_ha, GSM_ADDR_BASE)) {
> +	if (-1 == pm8001_bar4_shift(pm8001_ha, GSM_ADDR_BASE)) {
> +		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
>  		PM8001_FAIL_DBG(pm8001_ha,
>  			pm8001_printk("Shift Bar4 to 0x%x failed\n",
>  			GSM_ADDR_BASE));
> @@ -862,7 +887,8 @@ pm8001_chip_soft_rst(struct pm8001_hba_info
*pm8001_ha,
> u32 signature)
>  	/* step 5: delay 10 usec */
>  	udelay(10);
>  	/* step 5-b: set GPIO-0 output control to tristate anyway */
> -	if (-1 == bar4_shift(pm8001_ha, GPIO_ADDR_BASE)) {
> +	if (-1 == pm8001_bar4_shift(pm8001_ha, GPIO_ADDR_BASE)) {
> +		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
>  		PM8001_INIT_DBG(pm8001_ha,
>  				pm8001_printk("Shift Bar4 to 0x%x failed\n",
>  				GPIO_ADDR_BASE));
> @@ -878,7 +904,8 @@ pm8001_chip_soft_rst(struct pm8001_hba_info
*pm8001_ha,
> u32 signature)
> 
>  	/* Step 6: Reset the IOP and AAP1 */
>  	/* map 0x00000 to BAR4(0x20), BAR2(win) */
> -	if (-1 == bar4_shift(pm8001_ha, SPC_TOP_LEVEL_ADDR_BASE)) {
> +	if (-1 == pm8001_bar4_shift(pm8001_ha, SPC_TOP_LEVEL_ADDR_BASE)) {
> +		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
>  		PM8001_FAIL_DBG(pm8001_ha,
>  			pm8001_printk("SPC Shift Bar4 to 0x%x failed\n",
>  			SPC_TOP_LEVEL_ADDR_BASE));
> @@ -915,7 +942,8 @@ pm8001_chip_soft_rst(struct pm8001_hba_info
*pm8001_ha,
> u32 signature)
> 
>  	/* step 11: reads and sets the GSM Configuration and Reset Register
*/
>  	/* map 0x0700000 to BAR4(0x20), BAR2(win) */
> -	if (-1 == bar4_shift(pm8001_ha, GSM_ADDR_BASE)) {
> +	if (-1 == pm8001_bar4_shift(pm8001_ha, GSM_ADDR_BASE)) {
> +		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
>  		PM8001_FAIL_DBG(pm8001_ha,
>  			pm8001_printk("SPC Shift Bar4 to 0x%x failed\n",
>  			GSM_ADDR_BASE));
> @@ -968,7 +996,8 @@ pm8001_chip_soft_rst(struct pm8001_hba_info
*pm8001_ha,
> u32 signature)
> 
>  	/* step 13: bring the IOP and AAP1 out of reset */
>  	/* map 0x00000 to BAR4(0x20), BAR2(win) */
> -	if (-1 == bar4_shift(pm8001_ha, SPC_TOP_LEVEL_ADDR_BASE)) {
> +	if (-1 == pm8001_bar4_shift(pm8001_ha, SPC_TOP_LEVEL_ADDR_BASE)) {
> +		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
>  		PM8001_FAIL_DBG(pm8001_ha,
>  			pm8001_printk("Shift Bar4 to 0x%x failed\n",
>  			SPC_TOP_LEVEL_ADDR_BASE));
> @@ -1010,6 +1039,7 @@ pm8001_chip_soft_rst(struct pm8001_hba_info
*pm8001_ha,
> u32 signature)
>  				pm8001_printk("SCRATCH_PAD3 value = 0x%x\n",
>  				pm8001_cr32(pm8001_ha, 0,
>  				MSGU_SCRATCH_PAD_3)));
> +			spin_unlock_irqrestore(&pm8001_ha->lock, flags);
>  			return -1;
>  		}
> 
> @@ -1039,9 +1069,12 @@ pm8001_chip_soft_rst(struct pm8001_hba_info
*pm8001_ha,
> u32 signature)
>  				pm8001_printk("SCRATCH_PAD3 value = 0x%x\n",
>  				pm8001_cr32(pm8001_ha, 0,
>  				MSGU_SCRATCH_PAD_3)));
> +			spin_unlock_irqrestore(&pm8001_ha->lock, flags);
>  			return -1;
>  		}
>  	}
> +	pm8001_bar4_shift(pm8001_ha, 0);
> +	spin_unlock_irqrestore(&pm8001_ha->lock, flags);
> 
>  	PM8001_INIT_DBG(pm8001_ha,
>  		pm8001_printk("SPC soft reset Complete\n"));
> @@ -1157,8 +1190,8 @@ pm8001_chip_msix_interrupt_disable(struct
> pm8001_hba_info *pm8001_ha,
>  	msi_index = int_vec_idx * MSIX_TABLE_ELEMENT_SIZE;
>  	msi_index += MSIX_TABLE_BASE;
>  	pm8001_cw32(pm8001_ha, 0,  msi_index, MSIX_INTERRUPT_DISABLE);
> -
>  }
> +
>  /**
>   * pm8001_chip_interrupt_enable - enable PM8001 chip interrupt
>   * @pm8001_ha: our hba card information
> diff --git a/drivers/scsi/pm8001/pm8001_sas.c
> b/drivers/scsi/pm8001/pm8001_sas.c
> index fb3dc99..b673524 100644
> --- a/drivers/scsi/pm8001/pm8001_sas.c
> +++ b/drivers/scsi/pm8001/pm8001_sas.c
> @@ -166,6 +166,7 @@ int pm8001_phy_control(struct asd_sas_phy *sas_phy,
enum
> phy_func func,
>  	struct pm8001_hba_info *pm8001_ha = NULL;
>  	struct sas_phy_linkrates *rates;
>  	DECLARE_COMPLETION_ONSTACK(completion);
> +	unsigned long flags;
>  	pm8001_ha = sas_phy->ha->lldd_ha;
>  	pm8001_ha->phy[phy_id].enable_completion = &completion;
>  	switch (func) {
> @@ -209,8 +210,29 @@ int pm8001_phy_control(struct asd_sas_phy *sas_phy,
enum
> phy_func func,
>  	case PHY_FUNC_DISABLE:
>  		PM8001_CHIP_DISP->phy_stop_req(pm8001_ha, phy_id);
>  		break;
> +	case PHY_FUNC_GET_EVENTS:
> +		spin_lock_irqsave(&pm8001_ha->lock, flags);
> +		if (-1 == pm8001_bar4_shift(pm8001_ha,
> +					(phy_id < 4) ? 0x30000 : 0x40000)) {
> +			spin_unlock_irqrestore(&pm8001_ha->lock, flags);
> +			return -EINVAL;
> +		}
> +		{
> +			struct sas_phy *phy = sas_phy->phy;
> +			uint32_t *qp = (uint32_t *)(((char *)
> +				pm8001_ha->io_mem[2].memvirtaddr)
> +				+ 0x1034 + (0x4000 * (phy_id & 3)));
> +
> +			phy->invalid_dword_count = qp[0];
> +			phy->running_disparity_error_count = qp[1];
> +			phy->loss_of_dword_sync_count = qp[3];
> +			phy->phy_reset_problem_count = qp[4];
> +		}
> +		pm8001_bar4_shift(pm8001_ha, 0);
> +		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
> +		return 0;
>  	default:
> -		rc = -ENOSYS;
> +		rc = -EOPNOTSUPP;
>  	}
>  	msleep(300);
>  	return rc;
> diff --git a/drivers/scsi/pm8001/pm8001_sas.h
> b/drivers/scsi/pm8001/pm8001_sas.h
> index 93959fe..83a48f3 100644
> --- a/drivers/scsi/pm8001/pm8001_sas.h
> +++ b/drivers/scsi/pm8001/pm8001_sas.h
> @@ -488,6 +488,7 @@ int pm8001_mem_alloc(struct pci_dev *pdev, void
> **virt_addr,
>  	dma_addr_t *pphys_addr, u32 *pphys_addr_hi, u32 *pphys_addr_lo,
>  	u32 mem_size, u32 align);
> 
> +int pm8001_bar4_shift(struct pm8001_hba_info *pm8001_ha, u32 shiftValue);
> 
>  /* ctl shared API */
>  extern struct device_attribute *pm8001_host_attrs[];
> 
> ______________________________________________________________________
> This email may contain privileged or confidential information, which
should
> only be used for the purpose for which it was sent by Xyratex. No further
rights
> or licenses are granted to use such information. If you are not the
intended
> recipient of this message, please notify the sender by return and delete
it.
> You may not use, copy, disclose or rely on the information contained in
it.
> 
> Internet email is susceptible to data corruption, interception and
> unauthorised amendment for which Xyratex does not accept liability. While
we
> have taken reasonable precautions to ensure that this email is free of
viruses,
> Xyratex does not accept liability for the presence of any computer viruses
in
> this email, nor for any losses caused as a result of viruses.
> 
> Xyratex Technology Limited (03134912), Registered in England & Wales,
> Registered Office, Langstone Road, Havant, Hampshire, PO9 1SA.
> 
> The Xyratex group of companies also includes, Xyratex Ltd, registered in
> Bermuda, Xyratex International Inc, registered in California, Xyratex
> (Malaysia) Sdn Bhd registered in Malaysia, Xyratex Technology (Wuxi) Co
Ltd
> registered in The People's Republic of China and Xyratex Japan Limited
> registered in Japan.
> ______________________________________________________________________
> 

Re: Re: [PATCH] pm8001: Add FUNC_GET_EVENTS (take 2)

[jack_wang]

Thanks, maybe my copy & paste break the patch.


--------------
jack_wang
>I pulled from the email that came back from the reflector, the patch applied cleanly with no malformed warnings. Enclosed is that email.
>
>
>
>
>__________ Information from ESET NOD32 Antivirus, version of virus signature database 5659 (20101129) __________
>
>The message was checked by ESET NOD32 Antivirus.
>
>http://www.eset.com
>
>