Re: [PATCH v5 08/11] PCI: Unify pci_reset_function_locked() and __pci_reset_function_locked()

[Alex Williamson <alex.williamson@redhat.com>]

On Thu, 11 Oct 2018 04:50:00 +0000
Sinan Kaya <okaya@kernel.org> wrote:

> The difference between pci_reset_function_locked() and
> __pci_reset_function_locked() is the saving and restoring of the registers.
> Unify these API by adding saverestore argument that caller passes.
> 
> Signed-off-by: Sinan Kaya <okaya@kernel.org>
> ---
>  drivers/infiniband/hw/hfi1/pcie.c               |  2 +-
>  drivers/net/ethernet/cavium/liquidio/lio_main.c |  2 +-
>  drivers/pci/pci.c                               | 10 +++++++---
>  drivers/pci/pci.h                               |  1 +
>  drivers/xen/xen-pciback/pci_stub.c              |  6 +++---
>  include/linux/pci.h                             |  4 ++--
>  6 files changed, 15 insertions(+), 10 deletions(-)

TBH, I'm not a fan of this patch or the remainder in this series.
Having a function prefixed with underscored or specifically indicate
locked tells callers that these are special cases and should be
understood before using.  Adding bool parameters to the common
functions ensures that every caller now needs to understand those
special cases and potentially get them wrong.  Also, a string of random
bool options to a function means that any time we want to use it we
need to go reexamine the function definition.  It's not intuitive to
use or review.  Thanks,

Alex