V5 Change set:
V5 post has only defect fixes.
We are reworking and incorporating the suggestions from Bjorn.
And after covering tests, we ll be post Hot-Plug Surprise
removal patches.
V4 Change set:
Reframe split strings in print statement, to avoid
V3 Change Set:
Simplified function "mpt3sas_base_pci_device_is_available" and
made inline
V2 changes:
Replaced mpt3sas_base_pci_device_is_unplugged with
pci_device_is_present.
V1 changes:
In Patch 0001 - unlock mutex, if active reset is in progress.
mpt3sas: Separate out mpt3sas_wait_for_ioc_to_operational
mpt3sas: Fix Sync cache command failure during driver unload
mpt3sas:Fix driver modifying persistent data.
mpt3sas: Bump driver version to 27.100.00.00.
drivers/scsi/mpt3sas/mpt3sas_base.c | 79 +++++++++++++++++++-------------
drivers/scsi/mpt3sas/mpt3sas_base.h | 8 +++-
drivers/scsi/mpt3sas/mpt3sas_config.c | 29 +++---------
drivers/scsi/mpt3sas/mpt3sas_ctl.c | 22 ++-------
drivers/scsi/mpt3sas/mpt3sas_scsih.c | 38 ++++++++++++++-
drivers/scsi/mpt3sas/mpt3sas_transport.c | 73 +++++++----------------------
6 files changed, 115 insertions(+), 134 deletions(-)
--
1.8.3.1
* If EEDPTagMode field in manufacturing page11 is set,
unset it. This is needed to fix a hardware bug
in SAS3/SAS2 cards, So, skipping EEDPTagMode changes
in Manufacturing page11 for SAS35 controllers.
* Fix driver modifying NVRAM/persistent data in
Manufacturing page11 along with current copy. Driver should
change only current copy of Manufacturing page11.
Signed-off-by: Suganath Prabu <[email protected]>
---
drivers/scsi/mpt3sas/mpt3sas_base.c | 2 +-
drivers/scsi/mpt3sas/mpt3sas_config.c | 4 ----
2 files changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
index a6c217c..770f52b 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -4062,7 +4062,7 @@ _base_static_config_pages(struct MPT3SAS_ADAPTER *ioc)
* flag unset in NVDATA.
*/
mpt3sas_config_get_manufacturing_pg11(ioc, &mpi_reply, &ioc->manu_pg11);
- if (ioc->manu_pg11.EEDPTagMode == 0) {
+ if ((!ioc->is_gen35_ioc) && (ioc->manu_pg11.EEDPTagMode == 0)) {
pr_err("%s: overriding NVDATA EEDPTagMode setting\n",
ioc->name);
ioc->manu_pg11.EEDPTagMode &= ~0x3;
diff --git a/drivers/scsi/mpt3sas/mpt3sas_config.c b/drivers/scsi/mpt3sas/mpt3sas_config.c
index 14a195c..aa41cb9 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_config.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_config.c
@@ -660,10 +660,6 @@ mpt3sas_config_set_manufacturing_pg11(struct MPT3SAS_ADAPTER *ioc,
r = _config_request(ioc, &mpi_request, mpi_reply,
MPT3_CONFIG_PAGE_DEFAULT_TIMEOUT, config_page,
sizeof(*config_page));
- mpi_request.Action = MPI2_CONFIG_ACTION_PAGE_WRITE_NVRAM;
- r = _config_request(ioc, &mpi_request, mpi_reply,
- MPT3_CONFIG_PAGE_DEFAULT_TIMEOUT, config_page,
- sizeof(*config_page));
out:
return r;
}
--
1.8.3.1
Modify driver version to 27.100.00.00
(which is equivalent to PH8 OOB driver)
Signed-off-by: Suganath Prabu <[email protected]>
---
drivers/scsi/mpt3sas/mpt3sas_base.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.h b/drivers/scsi/mpt3sas/mpt3sas_base.h
index c860ed2..7fdaf29 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.h
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.h
@@ -74,8 +74,8 @@
#define MPT3SAS_DRIVER_NAME "mpt3sas"
#define MPT3SAS_AUTHOR "Avago Technologies <[email protected]>"
#define MPT3SAS_DESCRIPTION "LSI MPT Fusion SAS 3.0 Device Driver"
-#define MPT3SAS_DRIVER_VERSION "26.100.00.00"
-#define MPT3SAS_MAJOR_VERSION 26
+#define MPT3SAS_DRIVER_VERSION "27.100.00.00"
+#define MPT3SAS_MAJOR_VERSION 27
#define MPT3SAS_MINOR_VERSION 100
#define MPT3SAS_BUILD_VERSION 0
#define MPT3SAS_RELEASE_VERSION 00
--
1.8.3.1
No functional changes. This section of code
"wait for IOC to be operational" is used in many places
across the driver, and hence moved this code in to
a function "mpt3sas_wait_for_ioc_to_operational()"
Signed-off-by: Suganath Prabu <[email protected]>
---
drivers/scsi/mpt3sas/mpt3sas_base.c | 77 +++++++++++++++++++-------------
drivers/scsi/mpt3sas/mpt3sas_base.h | 4 ++
drivers/scsi/mpt3sas/mpt3sas_config.c | 25 +++--------
drivers/scsi/mpt3sas/mpt3sas_ctl.c | 22 ++-------
drivers/scsi/mpt3sas/mpt3sas_transport.c | 66 +++++----------------------
5 files changed, 71 insertions(+), 123 deletions(-)
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 166b607..a6c217c 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -5079,6 +5079,43 @@ _base_send_ioc_reset(struct MPT3SAS_ADAPTER *ioc, u8 reset_type, int timeout)
}
/**
+ * mpt3sas_wait_for_ioc_to_operational - IOC's operational
+ * state and HBA hot unplug status are checked here.
+ * @ioc: per adapter object
+ * @wait_count: timeout in seconds
+ *
+ * Return: Waits up to timeout seconds for the IOC to
+ * become operational. Returns 0 if IOC is present
+ * and operational; otherwise returns -EFAULT.
+ */
+
+int
+mpt3sas_wait_for_ioc_to_operational(struct MPT3SAS_ADAPTER *ioc,
+ int timeout)
+{
+ int wait_state_count = 0;
+ u32 ioc_state;
+
+ ioc_state = mpt3sas_base_get_iocstate(ioc, 1);
+ while (ioc_state != MPI2_IOC_STATE_OPERATIONAL) {
+
+ if (wait_state_count++ == timeout) {
+ ioc_err(ioc, "%s: failed due to ioc not operational\n",
+ __func__);
+ return -EFAULT;
+ }
+ ssleep(1);
+ ioc_state = mpt3sas_base_get_iocstate(ioc, 1);
+ ioc_info(ioc, "%s: waiting for operational state(count=%d)\n",
+ __func__, wait_state_count);
+ }
+ if (wait_state_count)
+ ioc_info(ioc, "ioc is operational\n");
+
+ return 0;
+}
+
+/**
* _base_handshake_req_reply_wait - send request thru doorbell interface
* @ioc: per adapter object
* @request_bytes: request length
@@ -5212,11 +5249,9 @@ mpt3sas_base_sas_iounit_control(struct MPT3SAS_ADAPTER *ioc,
Mpi2SasIoUnitControlRequest_t *mpi_request)
{
u16 smid;
- u32 ioc_state;
u8 issue_reset = 0;
int rc;
void *request;
- u16 wait_state_count;
dinitprintk(ioc, ioc_info(ioc, "%s\n", __func__));
@@ -5228,20 +5263,10 @@ mpt3sas_base_sas_iounit_control(struct MPT3SAS_ADAPTER *ioc,
goto out;
}
- wait_state_count = 0;
- ioc_state = mpt3sas_base_get_iocstate(ioc, 1);
- while (ioc_state != MPI2_IOC_STATE_OPERATIONAL) {
- if (wait_state_count++ == 10) {
- ioc_err(ioc, "%s: failed due to ioc not operational\n",
- __func__);
- rc = -EFAULT;
- goto out;
- }
- ssleep(1);
- ioc_state = mpt3sas_base_get_iocstate(ioc, 1);
- ioc_info(ioc, "%s: waiting for operational state(count=%d)\n",
- __func__, wait_state_count);
- }
+ rc = mpt3sas_wait_for_ioc_to_operational(ioc,
+ IOC_OPERATIONAL_WAIT_COUNT);
+ if (rc)
+ goto out;
smid = mpt3sas_base_get_smid(ioc, ioc->base_cb_idx);
if (!smid) {
@@ -5307,11 +5332,9 @@ mpt3sas_base_scsi_enclosure_processor(struct MPT3SAS_ADAPTER *ioc,
Mpi2SepReply_t *mpi_reply, Mpi2SepRequest_t *mpi_request)
{
u16 smid;
- u32 ioc_state;
u8 issue_reset = 0;
int rc;
void *request;
- u16 wait_state_count;
dinitprintk(ioc, ioc_info(ioc, "%s\n", __func__));
@@ -5323,20 +5346,10 @@ mpt3sas_base_scsi_enclosure_processor(struct MPT3SAS_ADAPTER *ioc,
goto out;
}
- wait_state_count = 0;
- ioc_state = mpt3sas_base_get_iocstate(ioc, 1);
- while (ioc_state != MPI2_IOC_STATE_OPERATIONAL) {
- if (wait_state_count++ == 10) {
- ioc_err(ioc, "%s: failed due to ioc not operational\n",
- __func__);
- rc = -EFAULT;
- goto out;
- }
- ssleep(1);
- ioc_state = mpt3sas_base_get_iocstate(ioc, 1);
- ioc_info(ioc, "%s: waiting for operational state(count=%d)\n",
- __func__, wait_state_count);
- }
+ rc = mpt3sas_wait_for_ioc_to_operational(ioc,
+ IOC_OPERATIONAL_WAIT_COUNT);
+ if (rc)
+ goto out;
smid = mpt3sas_base_get_smid(ioc, ioc->base_cb_idx);
if (!smid) {
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.h b/drivers/scsi/mpt3sas/mpt3sas_base.h
index 8f1d6b0..c860ed2 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.h
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.h
@@ -139,6 +139,8 @@
#define DEFAULT_NUM_FWCHAIN_ELEMTS 8
#define FW_IMG_HDR_READ_TIMEOUT 15
+
+#define IOC_OPERATIONAL_WAIT_COUNT 10
/*
* NVMe defines
*/
@@ -1487,6 +1489,8 @@ mpt3sas_wait_for_commands_to_complete(struct MPT3SAS_ADAPTER *ioc);
u8 mpt3sas_base_check_cmd_timeout(struct MPT3SAS_ADAPTER *ioc,
u8 status, void *mpi_request, int sz);
+int mpt3sas_wait_for_ioc_to_operational(struct MPT3SAS_ADAPTER *ioc,
+ int wait_count);
/* scsih shared API */
struct scsi_cmnd *mpt3sas_scsih_scsi_lookup_get(struct MPT3SAS_ADAPTER *ioc,
diff --git a/drivers/scsi/mpt3sas/mpt3sas_config.c b/drivers/scsi/mpt3sas/mpt3sas_config.c
index 0220944..14a195c 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_config.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_config.c
@@ -300,11 +300,10 @@ _config_request(struct MPT3SAS_ADAPTER *ioc, Mpi2ConfigRequest_t
void *config_page, u16 config_page_sz)
{
u16 smid;
- u32 ioc_state;
Mpi2ConfigRequest_t *config_request;
int r;
u8 retry_count, issue_host_reset = 0;
- u16 wait_state_count;
+
struct config_request mem;
u32 ioc_status = UINT_MAX;
@@ -361,23 +360,11 @@ _config_request(struct MPT3SAS_ADAPTER *ioc, Mpi2ConfigRequest_t
ioc_info(ioc, "%s: attempting retry (%d)\n",
__func__, retry_count);
}
- wait_state_count = 0;
- ioc_state = mpt3sas_base_get_iocstate(ioc, 1);
- while (ioc_state != MPI2_IOC_STATE_OPERATIONAL) {
- if (wait_state_count++ == MPT3_CONFIG_PAGE_DEFAULT_TIMEOUT) {
- ioc_err(ioc, "%s: failed due to ioc not operational\n",
- __func__);
- ioc->config_cmds.status = MPT3_CMD_NOT_USED;
- r = -EFAULT;
- goto free_mem;
- }
- ssleep(1);
- ioc_state = mpt3sas_base_get_iocstate(ioc, 1);
- ioc_info(ioc, "%s: waiting for operational state(count=%d)\n",
- __func__, wait_state_count);
- }
- if (wait_state_count)
- ioc_info(ioc, "%s: ioc is operational\n", __func__);
+
+ r = mpt3sas_wait_for_ioc_to_operational(ioc,
+ MPT3_CONFIG_PAGE_DEFAULT_TIMEOUT);
+ if (r)
+ goto free_mem;
smid = mpt3sas_base_get_smid(ioc, ioc->config_cb_idx);
if (!smid) {
diff --git a/drivers/scsi/mpt3sas/mpt3sas_ctl.c b/drivers/scsi/mpt3sas/mpt3sas_ctl.c
index 0f6305c..34c11a5 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_ctl.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_ctl.c
@@ -641,7 +641,6 @@ _ctl_do_mpt_command(struct MPT3SAS_ADAPTER *ioc, struct mpt3_ioctl_command karg,
MPI2DefaultReply_t *mpi_reply;
Mpi26NVMeEncapsulatedRequest_t *nvme_encap_request = NULL;
struct _pcie_device *pcie_device = NULL;
- u32 ioc_state;
u16 smid;
u8 timeout;
u8 issue_reset;
@@ -654,7 +653,6 @@ _ctl_do_mpt_command(struct MPT3SAS_ADAPTER *ioc, struct mpt3_ioctl_command karg,
dma_addr_t data_in_dma = 0;
size_t data_in_sz = 0;
long ret;
- u16 wait_state_count;
u16 device_handle = MPT3SAS_INVALID_DEVICE_HANDLE;
u8 tr_method = MPI26_SCSITASKMGMT_MSGFLAGS_PROTOCOL_LVL_RST_PCIE;
@@ -666,22 +664,10 @@ _ctl_do_mpt_command(struct MPT3SAS_ADAPTER *ioc, struct mpt3_ioctl_command karg,
goto out;
}
- wait_state_count = 0;
- ioc_state = mpt3sas_base_get_iocstate(ioc, 1);
- while (ioc_state != MPI2_IOC_STATE_OPERATIONAL) {
- if (wait_state_count++ == 10) {
- ioc_err(ioc, "%s: failed due to ioc not operational\n",
- __func__);
- ret = -EFAULT;
- goto out;
- }
- ssleep(1);
- ioc_state = mpt3sas_base_get_iocstate(ioc, 1);
- ioc_info(ioc, "%s: waiting for operational state(count=%d)\n",
- __func__, wait_state_count);
- }
- if (wait_state_count)
- ioc_info(ioc, "%s: ioc is operational\n", __func__);
+ ret = mpt3sas_wait_for_ioc_to_operational(ioc,
+ IOC_OPERATIONAL_WAIT_COUNT);
+ if (ret)
+ goto out;
mpi_request = kzalloc(ioc->request_sz, GFP_KERNEL);
if (!mpi_request) {
diff --git a/drivers/scsi/mpt3sas/mpt3sas_transport.c b/drivers/scsi/mpt3sas/mpt3sas_transport.c
index 031b420..7acb408 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_transport.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_transport.c
@@ -296,7 +296,6 @@ _transport_expander_report_manufacture(struct MPT3SAS_ADAPTER *ioc,
struct rep_manu_request *manufacture_request;
int rc;
u16 smid;
- u32 ioc_state;
void *psge;
u8 issue_reset = 0;
void *data_out = NULL;
@@ -304,7 +303,6 @@ _transport_expander_report_manufacture(struct MPT3SAS_ADAPTER *ioc,
dma_addr_t data_in_dma;
size_t data_in_sz;
size_t data_out_sz;
- u16 wait_state_count;
if (ioc->shost_recovery || ioc->pci_error_recovery) {
ioc_info(ioc, "%s: host reset in progress!\n", __func__);
@@ -320,22 +318,10 @@ _transport_expander_report_manufacture(struct MPT3SAS_ADAPTER *ioc,
}
ioc->transport_cmds.status = MPT3_CMD_PENDING;
- wait_state_count = 0;
- ioc_state = mpt3sas_base_get_iocstate(ioc, 1);
- while (ioc_state != MPI2_IOC_STATE_OPERATIONAL) {
- if (wait_state_count++ == 10) {
- ioc_err(ioc, "%s: failed due to ioc not operational\n",
- __func__);
- rc = -EFAULT;
- goto out;
- }
- ssleep(1);
- ioc_state = mpt3sas_base_get_iocstate(ioc, 1);
- ioc_info(ioc, "%s: waiting for operational state(count=%d)\n",
- __func__, wait_state_count);
- }
- if (wait_state_count)
- ioc_info(ioc, "%s: ioc is operational\n", __func__);
+ rc = mpt3sas_wait_for_ioc_to_operational(ioc,
+ IOC_OPERATIONAL_WAIT_COUNT);
+ if (rc)
+ goto out;
smid = mpt3sas_base_get_smid(ioc, ioc->transport_cb_idx);
if (!smid) {
@@ -1077,13 +1063,11 @@ _transport_get_expander_phy_error_log(struct MPT3SAS_ADAPTER *ioc,
struct phy_error_log_reply *phy_error_log_reply;
int rc;
u16 smid;
- u32 ioc_state;
void *psge;
u8 issue_reset = 0;
void *data_out = NULL;
dma_addr_t data_out_dma;
u32 sz;
- u16 wait_state_count;
if (ioc->shost_recovery || ioc->pci_error_recovery) {
ioc_info(ioc, "%s: host reset in progress!\n", __func__);
@@ -1099,22 +1083,10 @@ _transport_get_expander_phy_error_log(struct MPT3SAS_ADAPTER *ioc,
}
ioc->transport_cmds.status = MPT3_CMD_PENDING;
- wait_state_count = 0;
- ioc_state = mpt3sas_base_get_iocstate(ioc, 1);
- while (ioc_state != MPI2_IOC_STATE_OPERATIONAL) {
- if (wait_state_count++ == 10) {
- ioc_err(ioc, "%s: failed due to ioc not operational\n",
- __func__);
- rc = -EFAULT;
- goto out;
- }
- ssleep(1);
- ioc_state = mpt3sas_base_get_iocstate(ioc, 1);
- ioc_info(ioc, "%s: waiting for operational state(count=%d)\n",
- __func__, wait_state_count);
- }
- if (wait_state_count)
- ioc_info(ioc, "%s: ioc is operational\n", __func__);
+ rc = mpt3sas_wait_for_ioc_to_operational(ioc,
+ IOC_OPERATIONAL_WAIT_COUNT);
+ if (rc)
+ goto out;
smid = mpt3sas_base_get_smid(ioc, ioc->transport_cb_idx);
if (!smid) {
@@ -1381,13 +1353,11 @@ _transport_expander_phy_control(struct MPT3SAS_ADAPTER *ioc,
struct phy_control_reply *phy_control_reply;
int rc;
u16 smid;
- u32 ioc_state;
void *psge;
u8 issue_reset = 0;
void *data_out = NULL;
dma_addr_t data_out_dma;
u32 sz;
- u16 wait_state_count;
if (ioc->shost_recovery || ioc->pci_error_recovery) {
ioc_info(ioc, "%s: host reset in progress!\n", __func__);
@@ -1403,22 +1373,10 @@ _transport_expander_phy_control(struct MPT3SAS_ADAPTER *ioc,
}
ioc->transport_cmds.status = MPT3_CMD_PENDING;
- wait_state_count = 0;
- ioc_state = mpt3sas_base_get_iocstate(ioc, 1);
- while (ioc_state != MPI2_IOC_STATE_OPERATIONAL) {
- if (wait_state_count++ == 10) {
- ioc_err(ioc, "%s: failed due to ioc not operational\n",
- __func__);
- rc = -EFAULT;
- goto out;
- }
- ssleep(1);
- ioc_state = mpt3sas_base_get_iocstate(ioc, 1);
- ioc_info(ioc, "%s: waiting for operational state(count=%d)\n",
- __func__, wait_state_count);
- }
- if (wait_state_count)
- ioc_info(ioc, "%s: ioc is operational\n", __func__);
+ rc = mpt3sas_wait_for_ioc_to_operational(ioc,
+ IOC_OPERATIONAL_WAIT_COUNT);
+ if (rc)
+ goto out;
smid = mpt3sas_base_get_smid(ioc, ioc->transport_cb_idx);
if (!smid) {
--
1.8.3.1
This is to fix Sync cache and start stop command
failures with DID_NO_CONNECT during driver unload.
1) Release drives first from SML, then remove internally
in driver.
2) And allow sync cache and Start stop commands to firmware,
even when remove_host flag is set.
Signed-off-by: Suganath Prabu <[email protected]>
---
drivers/scsi/mpt3sas/mpt3sas_scsih.c | 38 ++++++++++++++++++++++++++++++--
drivers/scsi/mpt3sas/mpt3sas_transport.c | 7 ++++--
2 files changed, 41 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
index 4d73b5e..df56cbe 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
@@ -3748,6 +3748,40 @@ _scsih_tm_tr_complete(struct MPT3SAS_ADAPTER *ioc, u16 smid, u8 msix_index,
return _scsih_check_for_pending_tm(ioc, smid);
}
+/** _scsih_allow_scmd_to_device - check whether scmd needs to
+ * issue to IOC or not.
+ * @ioc: per adapter object
+ * @scmd: pointer to scsi command object
+ *
+ * Returns true if scmd can be issued to IOC otherwise returns false.
+ */
+inline bool _scsih_allow_scmd_to_device(struct MPT3SAS_ADAPTER *ioc,
+ struct scsi_cmnd *scmd)
+{
+
+ if (ioc->pci_error_recovery)
+ return false;
+
+ if (ioc->hba_mpi_version_belonged == MPI2_VERSION) {
+ if (ioc->remove_host)
+ return false;
+
+ return true;
+ }
+
+ if (ioc->remove_host) {
+
+ switch (scmd->cmnd[0]) {
+ case SYNCHRONIZE_CACHE:
+ case START_STOP:
+ return true;
+ default:
+ return false;
+ }
+ }
+
+ return true;
+}
/**
* _scsih_sas_control_complete - completion routine
@@ -4571,7 +4605,7 @@ scsih_qcmd(struct Scsi_Host *shost, struct scsi_cmnd *scmd)
return 0;
}
- if (ioc->pci_error_recovery || ioc->remove_host) {
+ if (!(_scsih_allow_scmd_to_device(ioc, scmd))) {
scmd->result = DID_NO_CONNECT << 16;
scmd->scsi_done(scmd);
return 0;
@@ -9641,6 +9675,7 @@ static void scsih_remove(struct pci_dev *pdev)
/* release all the volumes */
_scsih_ir_shutdown(ioc);
+ sas_remove_host(shost);
list_for_each_entry_safe(raid_device, next, &ioc->raid_device_list,
list) {
if (raid_device->starget) {
@@ -9682,7 +9717,6 @@ static void scsih_remove(struct pci_dev *pdev)
ioc->sas_hba.num_phys = 0;
}
- sas_remove_host(shost);
mpt3sas_base_detach(ioc);
spin_lock(&gioc_lock);
list_del(&ioc->list);
diff --git a/drivers/scsi/mpt3sas/mpt3sas_transport.c b/drivers/scsi/mpt3sas/mpt3sas_transport.c
index 7acb408..8ce45e2 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_transport.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_transport.c
@@ -808,10 +808,13 @@ mpt3sas_transport_port_remove(struct MPT3SAS_ADAPTER *ioc, u64 sas_address,
mpt3sas_port->remote_identify.sas_address,
mpt3sas_phy->phy_id);
mpt3sas_phy->phy_belongs_to_port = 0;
- sas_port_delete_phy(mpt3sas_port->port, mpt3sas_phy->phy);
+ if (!ioc->remove_host)
+ sas_port_delete_phy(mpt3sas_port->port,
+ mpt3sas_phy->phy);
list_del(&mpt3sas_phy->port_siblings);
}
- sas_port_delete(mpt3sas_port->port);
+ if (!ioc->remove_host)
+ sas_port_delete(mpt3sas_port->port);
kfree(mpt3sas_port);
}
--
1.8.3.1
On Wed, Oct 17, 2018 at 8:59 AM Suganath Prabu
<[email protected]> wrote:
>
> No functional changes. This section of code
> "wait for IOC to be operational" is used in many places
> across the driver, and hence moved this code in to
> a function "mpt3sas_wait_for_ioc_to_operational()"
> + ioc_state = mpt3sas_base_get_iocstate(ioc, 1);
> + while (ioc_state != MPI2_IOC_STATE_OPERATIONAL) {
> +
Do we need this blank line?
> + if (wait_state_count++ == timeout) {
> + ioc_err(ioc, "%s: failed due to ioc not operational\n",
> + __func__);
> + return -EFAULT;
> + }
> + ssleep(1);
> + ioc_state = mpt3sas_base_get_iocstate(ioc, 1);
> + ioc_info(ioc, "%s: waiting for operational state(count=%d)\n",
> + __func__, wait_state_count);
> + }
I understand this is part of existing code, but can you consider to
modify it to something like
do {
ioc_state = mpt3sas_base_get_iocstate(ioc, 1);
if (ioc_state == MPI2_IOC_STATE_OPERATIONAL)
break;
ioc_info(ioc, "%s: waiting for operational state(count=%d)\n",
__func__, ++wait_state_count);
while (timeout--);
if (!timeout) {
ioc_err(ioc, "%s: failed due to ioc not operational\n", __func__);
return -EFAULT;
}
Less lines, more understandable in my view.
> + rc = mpt3sas_wait_for_ioc_to_operational(ioc,
> + IOC_OPERATIONAL_WAIT_COUNT);
This can be one line (yes, I aware that is 82 characters, but it
improves readability from my p.o.v.).
> + rc = mpt3sas_wait_for_ioc_to_operational(ioc,
> + IOC_OPERATIONAL_WAIT_COUNT);
Ditto.
> u16 smid;
> - u32 ioc_state;
> Mpi2ConfigRequest_t *config_request;
> int r;
> u8 retry_count, issue_host_reset = 0;
> - u16 wait_state_count;
> +
Usually we don't split definition block.
> struct config_request mem;
> u32 ioc_status = UINT_MAX;
> + ret = mpt3sas_wait_for_ioc_to_operational(ioc,
> + IOC_OPERATIONAL_WAIT_COUNT);
One line?
> + rc = mpt3sas_wait_for_ioc_to_operational(ioc,
> + IOC_OPERATIONAL_WAIT_COUNT);
Ditto.
> + rc = mpt3sas_wait_for_ioc_to_operational(ioc,
> + IOC_OPERATIONAL_WAIT_COUNT);
Ditto.
> + rc = mpt3sas_wait_for_ioc_to_operational(ioc,
> + IOC_OPERATIONAL_WAIT_COUNT);
Ditto.
--
With Best Regards,
Andy Shevchenko
On Wed, Oct 17, 2018 at 11:17 AM Andy Shevchenko
<[email protected]> wrote:
>
> On Wed, Oct 17, 2018 at 8:59 AM Suganath Prabu
> <[email protected]> wrote:
> >
> > No functional changes. This section of code
> > "wait for IOC to be operational" is used in many places
> > across the driver, and hence moved this code in to
> > a function "mpt3sas_wait_for_ioc_to_operational()"
>
> > + ioc_state = mpt3sas_base_get_iocstate(ioc, 1);
> > + while (ioc_state != MPI2_IOC_STATE_OPERATIONAL) {
> > +
>
> Do we need this blank line?
>
> > + if (wait_state_count++ == timeout) {
> > + ioc_err(ioc, "%s: failed due to ioc not operational\n",
> > + __func__);
> > + return -EFAULT;
> > + }
> > + ssleep(1);
> > + ioc_state = mpt3sas_base_get_iocstate(ioc, 1);
> > + ioc_info(ioc, "%s: waiting for operational state(count=%d)\n",
> > + __func__, wait_state_count);
> > + }
>
> I understand this is part of existing code, but can you consider to
> modify it to something like
>
> do {
> ioc_state = mpt3sas_base_get_iocstate(ioc, 1);
> if (ioc_state == MPI2_IOC_STATE_OPERATIONAL)
> break;
Forgot ssleep(1); here.
> ioc_info(ioc, "%s: waiting for operational state(count=%d)\n",
> __func__, ++wait_state_count);
> while (timeout--);
> if (!timeout) {
> ioc_err(ioc, "%s: failed due to ioc not operational\n", __func__);
> return -EFAULT;
> }
> Less lines, more understandable in my view.
--
With Best Regards,
Andy Shevchenko
On Wed, Oct 17, 2018 at 8:59 AM Suganath Prabu
<[email protected]> wrote:
>
> * If EEDPTagMode field in manufacturing page11 is set,
> unset it. This is needed to fix a hardware bug
> in SAS3/SAS2 cards, So, skipping EEDPTagMode changes
> in Manufacturing page11 for SAS35 controllers.
>
> * Fix driver modifying NVRAM/persistent data in
> Manufacturing page11 along with current copy. Driver should
> change only current copy of Manufacturing page11.
> - if (ioc->manu_pg11.EEDPTagMode == 0) {
> + if ((!ioc->is_gen35_ioc) && (ioc->manu_pg11.EEDPTagMode == 0)) {
No need to have parens.
--
With Best Regards,
Andy Shevchenko
On Wed, Oct 17, 2018 at 8:59 AM Suganath Prabu
<[email protected]> wrote:
>
> This is to fix Sync cache and start stop command
> failures with DID_NO_CONNECT during driver unload.
>
> 1) Release drives first from SML, then remove internally
> in driver.
> 2) And allow sync cache and Start stop commands to firmware,
> even when remove_host flag is set.
> + if (ioc->hba_mpi_version_belonged == MPI2_VERSION) {
> + if (ioc->remove_host)
> + return false;
> +
> + return true;
> + }
> +
> + if (ioc->remove_host) {
> +
> + switch (scmd->cmnd[0]) {
> + case SYNCHRONIZE_CACHE:
> + case START_STOP:
> + return true;
> + default:
> + return false;
> + }
> + }
> +
> + return true;
Wouldn't be the same as
if (!ioc->remove_host || ioc->hba_mpi_version_belonged == MPI2_VERSION)
return !ioc->remove_host;
switch (scmd->cmnd[0]) {
case SYNCHRONIZE_CACHE:
case START_STOP:
return true;
default:
return false;
}
?
--
With Best Regards,
Andy Shevchenko
On Wed, Oct 17, 2018 at 1:54 PM Andy Shevchenko
<[email protected]> wrote:
>
> On Wed, Oct 17, 2018 at 8:59 AM Suganath Prabu
> <[email protected]> wrote:
> >
> > * If EEDPTagMode field in manufacturing page11 is set,
> > unset it. This is needed to fix a hardware bug
> > in SAS3/SAS2 cards, So, skipping EEDPTagMode changes
> > in Manufacturing page11 for SAS35 controllers.
> >
> > * Fix driver modifying NVRAM/persistent data in
> > Manufacturing page11 along with current copy. Driver should
> > change only current copy of Manufacturing page11.
>
> > - if (ioc->manu_pg11.EEDPTagMode == 0) {
> > + if ((!ioc->is_gen35_ioc) && (ioc->manu_pg11.EEDPTagMode == 0)) {
>
> No need to have parens.
>
Yes, We ll remove parentheses in v6 patch set.
Thanks,
>
> --
> With Best Regards,
> Andy Shevchenko
On Thu, Oct 18, 2018 at 10:10 AM Suganath Prabu Subramani
<[email protected]> wrote:
> On Wed, Oct 17, 2018 at 1:49 PM Andy Shevchenko <[email protected]> wrote:
>> On Wed, Oct 17, 2018 at 11:17 AM Andy Shevchenko
>> <[email protected]> wrote:
>> > I understand this is part of existing code, but can you consider to
>> > modify it to something like
>> >
>> > do {
>> > ioc_state = mpt3sas_base_get_iocstate(ioc, 1);
>> > if (ioc_state == MPI2_IOC_STATE_OPERATIONAL)
>> > break;
>>
>> Forgot ssleep(1); here.
>>
>> > ioc_info(ioc, "%s: waiting for operational state(count=%d)\n",
>> > __func__, ++wait_state_count);
>> > while (timeout--);
Just noticed this should be --timeout.
>> > if (!timeout) {
>> > ioc_err(ioc, "%s: failed due to ioc not operational\n", __func__);
>> > return -EFAULT;
>> > }
>> > Less lines, more understandable in my view.
>
> Yes, We 'll take this change and resend.
--
With Best Regards,
Andy Shevchenko
On Thu, Oct 18, 2018 at 10:11 AM Suganath Prabu Subramani
<[email protected]> wrote:
>
>
>
> On Wed, Oct 17, 2018 at 2:02 PM Andy Shevchenko <[email protected]> wrote:
>>
>> On Wed, Oct 17, 2018 at 8:59 AM Suganath Prabu
>> <[email protected]> wrote:
>> >
>> > This is to fix Sync cache and start stop command
>> > failures with DID_NO_CONNECT during driver unload.
>> >
>> > 1) Release drives first from SML, then remove internally
>> > in driver.
>> > 2) And allow sync cache and Start stop commands to firmware,
>> > even when remove_host flag is set.
>>
>> > + if (ioc->hba_mpi_version_belonged == MPI2_VERSION) {
>> > + if (ioc->remove_host)
>> > + return false;
>> > +
>> > + return true;
>> > + }
>> > +
>> > + if (ioc->remove_host) {
>> > +
>> > + switch (scmd->cmnd[0]) {
>> > + case SYNCHRONIZE_CACHE:
>> > + case START_STOP:
>> > + return true;
>> > + default:
>> > + return false;
>> > + }
>> > + }
>> > +
>> > + return true;
>>
>> Wouldn't be the same as
>> Yes it is same, but i feel original code is more readable.
OK (I didn't compare assembly, but I assume this is slow path).
>> if (!ioc->remove_host || ioc->hba_mpi_version_belonged == MPI2_VERSION)
>> return !ioc->remove_host;
>>
>> switch (scmd->cmnd[0]) {
>> case SYNCHRONIZE_CACHE:
>> case START_STOP:
>> return true;
>> default:
>> return false;
>> }
>>
>> ?
>>
>> --
>> With Best Regards,
>> Andy Shevchenko
--
With Best Regards,
Andy Shevchenko