* [v5 0/4] mpt3sas: Hot-Plug Surprise removal support on IOC. @ 2018-10-17 5:59 Suganath Prabu 2018-10-17 5:59 ` [v5 1/4] mpt3sas: Separate out mpt3sas_wait_for_ioc_to_operational Suganath Prabu ` (3 more replies) 0 siblings, 4 replies; 12+ messages in thread From: Suganath Prabu @ 2018-10-17 5:59 UTC (permalink / raw) To: helgaas, lukas, linux-scsi, linux-pci, linux-kernel Cc: benh, ruscur, sbobroff, oohall, andy.shevchenko, Sathya.Prakash, sreekanth.reddy, Suganath Prabu 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [v5 1/4] mpt3sas: Separate out mpt3sas_wait_for_ioc_to_operational 2018-10-17 5:59 [v5 0/4] mpt3sas: Hot-Plug Surprise removal support on IOC Suganath Prabu @ 2018-10-17 5:59 ` Suganath Prabu 2018-10-17 8:17 ` Andy Shevchenko 2018-10-17 5:59 ` [v5 2/4] mpt3sas: Fix Sync cache command failure during driver unload Suganath Prabu ` (2 subsequent siblings) 3 siblings, 1 reply; 12+ messages in thread From: Suganath Prabu @ 2018-10-17 5:59 UTC (permalink / raw) To: helgaas, lukas, linux-scsi, linux-pci, linux-kernel Cc: benh, ruscur, sbobroff, oohall, andy.shevchenko, Sathya.Prakash, sreekanth.reddy, Suganath Prabu 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 <suganath-prabu.subramani@broadcom.com> --- 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 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [v5 1/4] mpt3sas: Separate out mpt3sas_wait_for_ioc_to_operational 2018-10-17 5:59 ` [v5 1/4] mpt3sas: Separate out mpt3sas_wait_for_ioc_to_operational Suganath Prabu @ 2018-10-17 8:17 ` Andy Shevchenko 2018-10-17 8:19 ` Andy Shevchenko 0 siblings, 1 reply; 12+ messages in thread From: Andy Shevchenko @ 2018-10-17 8:17 UTC (permalink / raw) To: Suganath Prabu Subramani Cc: Bjorn Helgaas, Lukas Wunner, linux-scsi, linux-pci, Linux Kernel Mailing List, Benjamin Herrenschmidt, ruscur, sbobroff, oohall, Sathya Prakash, Sreekanth Reddy On Wed, Oct 17, 2018 at 8:59 AM Suganath Prabu <suganath-prabu.subramani@broadcom.com> 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [v5 1/4] mpt3sas: Separate out mpt3sas_wait_for_ioc_to_operational 2018-10-17 8:17 ` Andy Shevchenko @ 2018-10-17 8:19 ` Andy Shevchenko [not found] ` <CA+RiK66_gpLvzFyztdQ3BdX11wn2uRfxoF6hhRg1TXoERe7z3Q@mail.gmail.com> 0 siblings, 1 reply; 12+ messages in thread From: Andy Shevchenko @ 2018-10-17 8:19 UTC (permalink / raw) To: Suganath Prabu Subramani Cc: Bjorn Helgaas, Lukas Wunner, linux-scsi, linux-pci, Linux Kernel Mailing List, Benjamin Herrenschmidt, ruscur, sbobroff, oohall, Sathya Prakash, Sreekanth Reddy On Wed, Oct 17, 2018 at 11:17 AM Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > > On Wed, Oct 17, 2018 at 8:59 AM Suganath Prabu > <suganath-prabu.subramani@broadcom.com> 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
[parent not found: <CA+RiK66_gpLvzFyztdQ3BdX11wn2uRfxoF6hhRg1TXoERe7z3Q@mail.gmail.com>]
* Re: [v5 1/4] mpt3sas: Separate out mpt3sas_wait_for_ioc_to_operational [not found] ` <CA+RiK66_gpLvzFyztdQ3BdX11wn2uRfxoF6hhRg1TXoERe7z3Q@mail.gmail.com> @ 2018-10-18 7:22 ` Andy Shevchenko 0 siblings, 0 replies; 12+ messages in thread From: Andy Shevchenko @ 2018-10-18 7:22 UTC (permalink / raw) To: Suganath Prabu Subramani Cc: Bjorn Helgaas, Lukas Wunner, linux-scsi, linux-pci, Linux Kernel Mailing List, Benjamin Herrenschmidt, ruscur, sbobroff, Oliver, Sathya Prakash, Sreekanth Reddy On Thu, Oct 18, 2018 at 10:10 AM Suganath Prabu Subramani <suganath-prabu.subramani@broadcom.com> wrote: > On Wed, Oct 17, 2018 at 1:49 PM Andy Shevchenko <andy.shevchenko@gmail.com> wrote: >> On Wed, Oct 17, 2018 at 11:17 AM Andy Shevchenko >> <andy.shevchenko@gmail.com> 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [v5 2/4] mpt3sas: Fix Sync cache command failure during driver unload 2018-10-17 5:59 [v5 0/4] mpt3sas: Hot-Plug Surprise removal support on IOC Suganath Prabu 2018-10-17 5:59 ` [v5 1/4] mpt3sas: Separate out mpt3sas_wait_for_ioc_to_operational Suganath Prabu @ 2018-10-17 5:59 ` Suganath Prabu 2018-10-17 8:32 ` Andy Shevchenko 2018-10-17 5:59 ` [v5 3/4] mpt3sas:Fix driver modifying persistent data Suganath Prabu 2018-10-17 5:59 ` [v5 4/4] mpt3sas: Bump driver version to 27.100.00.00 Suganath Prabu 3 siblings, 1 reply; 12+ messages in thread From: Suganath Prabu @ 2018-10-17 5:59 UTC (permalink / raw) To: helgaas, lukas, linux-scsi, linux-pci, linux-kernel Cc: benh, ruscur, sbobroff, oohall, andy.shevchenko, Sathya.Prakash, sreekanth.reddy, Suganath Prabu 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 <suganath-prabu.subramani@broadcom.com> --- 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 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [v5 2/4] mpt3sas: Fix Sync cache command failure during driver unload 2018-10-17 5:59 ` [v5 2/4] mpt3sas: Fix Sync cache command failure during driver unload Suganath Prabu @ 2018-10-17 8:32 ` Andy Shevchenko [not found] ` <CA+RiK67067aV9Ky5m8uCGLGcvjbBRxqFVeyB0CsYcvSw7rgqJg@mail.gmail.com> 0 siblings, 1 reply; 12+ messages in thread From: Andy Shevchenko @ 2018-10-17 8:32 UTC (permalink / raw) To: Suganath Prabu Subramani Cc: Bjorn Helgaas, Lukas Wunner, linux-scsi, linux-pci, Linux Kernel Mailing List, Benjamin Herrenschmidt, ruscur, sbobroff, oohall, Sathya Prakash, Sreekanth Reddy On Wed, Oct 17, 2018 at 8:59 AM Suganath Prabu <suganath-prabu.subramani@broadcom.com> 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
[parent not found: <CA+RiK67067aV9Ky5m8uCGLGcvjbBRxqFVeyB0CsYcvSw7rgqJg@mail.gmail.com>]
* Re: [v5 2/4] mpt3sas: Fix Sync cache command failure during driver unload [not found] ` <CA+RiK67067aV9Ky5m8uCGLGcvjbBRxqFVeyB0CsYcvSw7rgqJg@mail.gmail.com> @ 2018-10-18 7:23 ` Andy Shevchenko 0 siblings, 0 replies; 12+ messages in thread From: Andy Shevchenko @ 2018-10-18 7:23 UTC (permalink / raw) To: Suganath Prabu Subramani Cc: Bjorn Helgaas, Lukas Wunner, linux-scsi, linux-pci, Linux Kernel Mailing List, Benjamin Herrenschmidt, ruscur, sbobroff, Oliver, Sathya Prakash, Sreekanth Reddy On Thu, Oct 18, 2018 at 10:11 AM Suganath Prabu Subramani <suganath-prabu.subramani@broadcom.com> wrote: > > > > On Wed, Oct 17, 2018 at 2:02 PM Andy Shevchenko <andy.shevchenko@gmail.com> wrote: >> >> On Wed, Oct 17, 2018 at 8:59 AM Suganath Prabu >> <suganath-prabu.subramani@broadcom.com> 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [v5 3/4] mpt3sas:Fix driver modifying persistent data. 2018-10-17 5:59 [v5 0/4] mpt3sas: Hot-Plug Surprise removal support on IOC Suganath Prabu 2018-10-17 5:59 ` [v5 1/4] mpt3sas: Separate out mpt3sas_wait_for_ioc_to_operational Suganath Prabu 2018-10-17 5:59 ` [v5 2/4] mpt3sas: Fix Sync cache command failure during driver unload Suganath Prabu @ 2018-10-17 5:59 ` Suganath Prabu 2018-10-17 8:24 ` Andy Shevchenko 2018-10-17 5:59 ` [v5 4/4] mpt3sas: Bump driver version to 27.100.00.00 Suganath Prabu 3 siblings, 1 reply; 12+ messages in thread From: Suganath Prabu @ 2018-10-17 5:59 UTC (permalink / raw) To: helgaas, lukas, linux-scsi, linux-pci, linux-kernel Cc: benh, ruscur, sbobroff, oohall, andy.shevchenko, Sathya.Prakash, sreekanth.reddy, Suganath Prabu * 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 <suganath-prabu.subramani@broadcom.com> --- 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 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [v5 3/4] mpt3sas:Fix driver modifying persistent data. 2018-10-17 5:59 ` [v5 3/4] mpt3sas:Fix driver modifying persistent data Suganath Prabu @ 2018-10-17 8:24 ` Andy Shevchenko 2018-10-18 7:16 ` Suganath Prabu Subramani 0 siblings, 1 reply; 12+ messages in thread From: Andy Shevchenko @ 2018-10-17 8:24 UTC (permalink / raw) To: Suganath Prabu Subramani Cc: Bjorn Helgaas, Lukas Wunner, linux-scsi, linux-pci, Linux Kernel Mailing List, Benjamin Herrenschmidt, ruscur, sbobroff, oohall, Sathya Prakash, Sreekanth Reddy On Wed, Oct 17, 2018 at 8:59 AM Suganath Prabu <suganath-prabu.subramani@broadcom.com> 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [v5 3/4] mpt3sas:Fix driver modifying persistent data. 2018-10-17 8:24 ` Andy Shevchenko @ 2018-10-18 7:16 ` Suganath Prabu Subramani 0 siblings, 0 replies; 12+ messages in thread From: Suganath Prabu Subramani @ 2018-10-18 7:16 UTC (permalink / raw) To: Andy Shevchenko Cc: helgaas, lukas, linux-scsi, linux-pci, linux-kernel, benh, ruscur, sbobroff, Oliver, Sathya Prakash, Sreekanth Reddy On Wed, Oct 17, 2018 at 1:54 PM Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > > On Wed, Oct 17, 2018 at 8:59 AM Suganath Prabu > <suganath-prabu.subramani@broadcom.com> 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [v5 4/4] mpt3sas: Bump driver version to 27.100.00.00. 2018-10-17 5:59 [v5 0/4] mpt3sas: Hot-Plug Surprise removal support on IOC Suganath Prabu ` (2 preceding siblings ...) 2018-10-17 5:59 ` [v5 3/4] mpt3sas:Fix driver modifying persistent data Suganath Prabu @ 2018-10-17 5:59 ` Suganath Prabu 3 siblings, 0 replies; 12+ messages in thread From: Suganath Prabu @ 2018-10-17 5:59 UTC (permalink / raw) To: helgaas, lukas, linux-scsi, linux-pci, linux-kernel Cc: benh, ruscur, sbobroff, oohall, andy.shevchenko, Sathya.Prakash, sreekanth.reddy, Suganath Prabu Modify driver version to 27.100.00.00 (which is equivalent to PH8 OOB driver) Signed-off-by: Suganath Prabu <suganath-prabu.subramani@broadcom.com> --- 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 <MPT-FusionLinux.pdl@avagotech.com>" #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 ^ permalink raw reply related [flat|nested] 12+ messages in thread
end of thread, other threads:[~2018-10-18 7:23 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-10-17 5:59 [v5 0/4] mpt3sas: Hot-Plug Surprise removal support on IOC Suganath Prabu 2018-10-17 5:59 ` [v5 1/4] mpt3sas: Separate out mpt3sas_wait_for_ioc_to_operational Suganath Prabu 2018-10-17 8:17 ` Andy Shevchenko 2018-10-17 8:19 ` Andy Shevchenko [not found] ` <CA+RiK66_gpLvzFyztdQ3BdX11wn2uRfxoF6hhRg1TXoERe7z3Q@mail.gmail.com> 2018-10-18 7:22 ` Andy Shevchenko 2018-10-17 5:59 ` [v5 2/4] mpt3sas: Fix Sync cache command failure during driver unload Suganath Prabu 2018-10-17 8:32 ` Andy Shevchenko [not found] ` <CA+RiK67067aV9Ky5m8uCGLGcvjbBRxqFVeyB0CsYcvSw7rgqJg@mail.gmail.com> 2018-10-18 7:23 ` Andy Shevchenko 2018-10-17 5:59 ` [v5 3/4] mpt3sas:Fix driver modifying persistent data Suganath Prabu 2018-10-17 8:24 ` Andy Shevchenko 2018-10-18 7:16 ` Suganath Prabu Subramani 2018-10-17 5:59 ` [v5 4/4] mpt3sas: Bump driver version to 27.100.00.00 Suganath Prabu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).