Re: [v6 1/4] mpt3sas: Separate out mpt3sas_wait_for_ioc_to_operational

Bjorn Helgaas
2018-10-30T13:22:36Z

On Tue, Oct 23, 2018 at 03:51:18PM +0530, Suganath Prabu 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()"
>
> Signed-off-by: Suganath Prabu <suganath-prabu.subramani@broadcom.com>

I'm not really a reviewer of mpt3sas, but since I commented earlier,
I took a quick look at this. Minor comments:

- Remove leading spaces from changelog and reformat to use the whole
width, e.g.,

No functional changes. This section of code "wait for IOC to be
operational" is used in many places across the driver. Factor
this code out into a new mpt3sas_wait_for_ioc_to_operational().

- "mpt3sas_wait_for_ioc_to_operational" isn't really grammatical in
English; maybe "mpt3sas_wait_for_ioc" would be better?

- It would be slightly easier to review this if it were split into three
patches: (1) add and use the IOC_OPERATIONAL_WAIT_COUNT #define, (2)
factor out mpt3sas_wait_for_ioc() using the exact same code structure
(the while loop), and (3) convert mpt3sas_wait_for_ioc() to use the do
loop instead of the while. The idea is that each individual patch is
simpler and more obvious because it only contains a single logical
change.

Reviewed-by: Bjorn Helgaas <bhelgaas@google.com>