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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 10/11/2018 12:02 PM, Alex Williamson 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@xxxxxxxxxx>
---
  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,

Thanks for the feedback. I wanted to get it out to gather feedback.

I think it is quite straightforward for people familiar with the existing
API to know what they are dealing with. I didn't know the difference
between

__pci_reset_function_locked() and pci_reset_function_locked()

for a long time. This was an attempt to minimize reset function flavors
and ask explicitly what user wants rather than having them use the wrong
function and suffer the consequence with random failures.



[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux