Re: [PATCH 0/8] PCI: Align return value of pcie capability accessors with other accessors

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

 



On Tue, Jun 09, 2020 at 05:39:42PM +0200, refactormyself@xxxxxxxxx wrote:
> From: Bolarinwa Olayemi Saheed <refactormyself@xxxxxxxxx>
> 
> PATCH 1 to 7:
> 
> PCIBIOS_ error codes have positive values and they are passed down the
> call heirarchy from accessors. For functions which are meant to return
> only a negative value on failure, passing on this value is a bug.
> 
> To mitigate this, call pcibios_err_to_errno() before passing on return
> value from pcie capability accessors call heirarchy. This function
> converts any positive PCIBIOS_ error codes to negative non-PCI generic
> error values.
> 
> PATCH 8:
> 
> The pcie capability accessors can return 0, -EINVAL, or any PCIBIOS_ error
> code. The pci accessor on the other hand can only return 0 or any PCIBIOS_
> error code.This inconsistency among these accessor makes it harder for
> callers to check for errors.
> 
> Return PCIBIOS_BAD_REGISTER_NUMBER instead of -EINVAL in all pcie
> capability accessors.
> 
> 
> Bolarinwa Olayemi Saheed (8):
>   PCI: Convert PCIBIOS_ error codes to non-PCI generic error codes
>   PCI: Convert PCIBIOS_ error codes to non-PCI generic error codes
>   PCI: Convert PCIBIOS_ error codes to non-PCI generic error codes
>   PCI: Convert PCIBIOS_ error codes to non-PCI generic error codes
>   PCI: Convert PCIBIOS_ error codes to non-PCI generic error codes
>   PCI: Convert PCIBIOS_ error codes to non-PCI generic error codes
>   PCI: Convert PCIBIOS_ error codes to non-PCI generic error codes
>   PCI: Align return value of pcie capability accessors with other
>     accessors
> 
>  drivers/dma/ioat/init.c               |  4 ++--
>  drivers/infiniband/hw/hfi1/pcie.c     | 18 +++++++++++++-----
>  drivers/pci/access.c                  |  8 ++++----
>  drivers/pci/pci.c                     | 10 ++++++++--
>  drivers/pci/pcie/aer.c                | 12 ++++++++++--
>  drivers/scsi/smartpqi/smartpqi_init.c |  6 +++++-
>  6 files changed, 42 insertions(+), 16 deletions(-)

This is beautiful!  Tiny patches that are easy to review and verify,
and I think it's definitely going the right direction.

Nits that apply to all:

  - These are really fixing driver bugs (even though the PCI core is
    making it harder than necessary for the drivers), so make the
    driver subject lines match the individual driver history, e.g.,

      dmaengine: ioatdma: Convert PCIBIOS_* errors to generic -E* errors
      IB/hfi1: Convert PCIBIOS_* errors to generic -E* errors

    Find the driver or subsystem prefix by running "git log --oneline"
    on each file you're touching.

  - In commit logs, use "PCIe", not "pcie".  Also "non-PCI generic"
    might be a little redundant; I think "generic" is enough.

  - Order "Suggested-by" before "Signed-off-by".

When you repost this, be sure to include a "v2" in the subject.

Bjorn



[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