Re: [PATCH 6/9] powerpc/eeh: Clarify arguments to eeh_reset_device()

[Russell Currey <ruscur@russell.cc>]

On Tue, 2018-03-06 at 10:59 +1100, Sam Bobroff wrote:
> It is currently difficult to understand the behaviour of
> eeh_reset_device() due to the way it's parameters are used. In
> particular, when 'bus' is NULL, it's value is still necessary so the
> same value is looked up again locally under a different name
> ('frozen_bus') but behaviour is changed.
> 
> To clarify this, add a new parameter 'eeh_aware_driver', and have the
> caller set it when it would have passed NULL for 'bus' and always
> pass
> a value for 'bus'. Then change any test that was on 'bus' to one on
> '!eeh_aware_driver' and replace uses of 'frozen_bus' with 'bus'.
> 
> Also update the function's comment.
> 
> This should not change behaviour.
> 
> Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
> ---
>  arch/powerpc/kernel/eeh_driver.c | 22 ++++++++++++----------
>  1 file changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/eeh_driver.c
> b/arch/powerpc/kernel/eeh_driver.c
> index cb584d72b0a5..6c3577133223 100644
> --- a/arch/powerpc/kernel/eeh_driver.c
> +++ b/arch/powerpc/kernel/eeh_driver.c
> @@ -619,17 +619,19 @@ int eeh_pe_reset_and_recover(struct eeh_pe *pe)
>  
>  /**
>   * eeh_reset_device - Perform actual reset of a pci slot
> + * @eeh_aware_driver: Does the device's driver provide EEH support?
>   * @pe: EEH PE
>   * @bus: PCI bus corresponding to the isolcated slot
> + * @rmv_data: Optional, list to record removed devices
>   *
>   * This routine must be called to do reset on the indicated PE.
>   * During the reset, udev might be invoked because those affected
>   * PCI devices will be removed and then added.
>   */

Just a nitpick here, I would prefer that the PE remain the first
argument of the function since that's one thing that's pretty standard
across the EEH implementation.  I'd also say that "driver_aware" is
clearer than "aware_driver".

Reviewed-by: Russell Currey <ruscur@russell.cc>