Messages in this thread | | | From | Andy Shevchenko <> | Date | Wed, 17 Oct 2018 11:19:32 +0300 | Subject | Re: [v5 1/4] mpt3sas: Separate out mpt3sas_wait_for_ioc_to_operational |
| |
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
| |