Hi Bjorn, Thanks for reviewing. On Thu, Sep 27, 2018 at 2:33 AM Bjorn Helgaas <helgaas@xxxxxxxxxx> wrote: > > On Wed, Sep 26, 2018 at 09:52:35AM +0530, Suganath Prabu S wrote: > > Introduce mpt3sas_wait_for_ioc_to_operational. > > > > This section of code "wait for IOC to be operational" > > is used in many places across the driver, > > and hence moved this section of code in to the function > > "mpt3sas_wait_for_ioc_to_operational". > > > > Also added HBA hot unplug checks, and this returns with > > error code EFAULT, if it detects HBA is hot unplugged > > or IOC is not in operational state. > > This should be two patches: > > 1) Factor out the "wait for IOC" code. This should not cause any > functional changes (I didn't verify in your code, but this is the > objective). > > 2) Add the hot unplug checks. > > This makes the patches much easier to review. > Will move hot unplug check to separate patch. > > V2 change set: > > used pci_device_is_present instead of > > mpt3sas_base_pci_device_is_unplugged > > > > v4 Change set: > > Dont split strings in print statement > > I don't know the convention in drivers/scsi, but in driver/pci, I > remove this sort of v2/v3 commentary from the changelog because it's > really not of interest after the patch is merged. It's nice to have > it in the email, but I think if you put it after a line containing > only "--" it will be in the email but not the changelog. >Thanks, Will add change sets after -- > > Signed-off-by: Suganath Prabu S <suganath-prabu.subramani@xxxxxxxxxxxx> > > --- > > drivers/scsi/mpt3sas/mpt3sas_base.c | 92 +++++++++++++++++++------------- > > drivers/scsi/mpt3sas/mpt3sas_base.h | 4 ++ > > drivers/scsi/mpt3sas/mpt3sas_config.c | 28 +++------- > > drivers/scsi/mpt3sas/mpt3sas_ctl.c | 26 ++------- > > drivers/scsi/mpt3sas/mpt3sas_transport.c | 75 +++++--------------------- > > 5 files changed, 81 insertions(+), 144 deletions(-) > > > > diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c > > index c880e72..9f1d8fb 100644 > > --- a/drivers/scsi/mpt3sas/mpt3sas_base.c > > +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c > > @@ -5176,6 +5176,53 @@ _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 > > "wait_count" is really a timeout and maybe should be named "timeout". >Yes, wait_count is timeout, will rename wait_count to timeout > > + * Return: Returns EFAULT, if HBA is hot unplugged or IOC is > > + * not in operational state, within the wait_count. > > + * And returns 0, If not hot unplugged Or ioc is in > > + * operational state. > > I think you mean something like: > > Waits up to wait_count seconds for the IOC to become operational. > Returns 0 if IOC is present and operational; otherwise returns > -EFAULT. > Yes. > > + */ > > + > > +int > > +mpt3sas_wait_for_ioc_to_operational(struct MPT3SAS_ADAPTER *ioc, > > + int wait_count)