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

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

[refactormyself]

From: Bolarinwa Olayemi Saheed <refactormyself@gmail.com>

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(-)

-- 
2.18.2

[PATCH 1/8] PCI: Convert PCIBIOS_ error codes to non-PCI generic error codes

[refactormyself]

From: Bolarinwa Olayemi Saheed <refactormyself@gmail.com>

ioat3_dma_probe() returns PCIBIOS_ error codes from pcie capability
accessors.

PCIBIOS_ error codes have positive values. Passing on these values is
inconsistent with functions which return only a negative value on failure.

Before passing on return value of pcie capability accessors, call
pcibios_err_to_errno() to convert any positive PCIBIOS_ error codes to
negative non-PCI generic error values.

Signed-off-by: Bolarinwa Olayemi Saheed <refactormyself@gmail.com>
Suggested-by: Bjorn Helgaas <bjorn@helgaas.com>
---
 drivers/dma/ioat/init.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/ioat/init.c b/drivers/dma/ioat/init.c
index 60e9afbb896c..fc8889c2a88f 100644
--- a/drivers/dma/ioat/init.c
+++ b/drivers/dma/ioat/init.c
@@ -1195,13 +1195,13 @@ static int ioat3_dma_probe(struct ioatdma_device *ioat_dma, int dca)
 	/* disable relaxed ordering */
 	err = pcie_capability_read_word(pdev, IOAT_DEVCTRL_OFFSET, &val16);
 	if (err)
-		return err;
+		return pcibios_err_to_errno(err);
 
 	/* clear relaxed ordering enable */
 	val16 &= ~IOAT_DEVCTRL_ROE;
 	err = pcie_capability_write_word(pdev, IOAT_DEVCTRL_OFFSET, val16);
 	if (err)
-		return err;
+		return pcibios_err_to_errno(err);
 
 	if (ioat_dma->cap & IOAT_CAP_DPS)
 		writeb(ioat_pending_level + 1,
-- 
2.18.2

[PATCH 2/8] PCI: Convert PCIBIOS_ error codes to non-PCI generic error codes

[refactormyself]

From: Bolarinwa Olayemi Saheed <refactormyself@gmail.com>

pcie_speeds() returns PCIBIOS_ error codes from pcie capability accessors.

PCIBIOS_ error codes have positive values. Passing on these values is
inconsistent with functions which return only a negative value on failure.

Before passing on return value of pcie capability accessors, call
pcibios_err_to_errno() to convert any positive PCIBIOS_ error codes to
negative non-PCI generic error values.

Signed-off-by: Bolarinwa Olayemi Saheed <refactormyself@gmail.com>
Suggested-by: Bjorn Helgaas <bjorn@helgaas.com>
---
 drivers/infiniband/hw/hfi1/pcie.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/hw/hfi1/pcie.c b/drivers/infiniband/hw/hfi1/pcie.c
index 1a6268d61977..eb53781d0c6a 100644
--- a/drivers/infiniband/hw/hfi1/pcie.c
+++ b/drivers/infiniband/hw/hfi1/pcie.c
@@ -306,7 +306,7 @@ int pcie_speeds(struct hfi1_devdata *dd)
 	ret = pcie_capability_read_dword(dd->pcidev, PCI_EXP_LNKCAP, &linkcap);
 	if (ret) {
 		dd_dev_err(dd, "Unable to read from PCI config\n");
-		return ret;
+		return pcibios_err_to_errno(ret);
 	}
 
 	if ((linkcap & PCI_EXP_LNKCAP_SLS) != PCI_EXP_LNKCAP_SLS_8_0GB) {
-- 
2.18.2

[PATCH 3/8] PCI: Convert PCIBIOS_ error codes to non-PCI generic error codes

[refactormyself]

From: Bolarinwa Olayemi Saheed <refactormyself@gmail.com>

restore_pci_variables() and save_pci_variables() return PCIBIOS_ error
codes from pcie capability accessors.

PCIBIOS_ error codes have positive values. Passing on these values is
inconsistent with functions which return only a negative value on failure.

Before passing on return value of pcie capability accessors, call
pcibios_err_to_errno() to convert any positive PCIBIOS_ error codes to
negative non-PCI generic error values.

Signed-off-by: Bolarinwa Olayemi Saheed <refactormyself@gmail.com>
Suggested-by: Bjorn Helgaas <bjorn@helgaas.com>
---
 drivers/infiniband/hw/hfi1/pcie.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/infiniband/hw/hfi1/pcie.c b/drivers/infiniband/hw/hfi1/pcie.c
index eb53781d0c6a..eb2790e9db36 100644
--- a/drivers/infiniband/hw/hfi1/pcie.c
+++ b/drivers/infiniband/hw/hfi1/pcie.c
@@ -334,7 +334,11 @@ int pcie_speeds(struct hfi1_devdata *dd)
 	return 0;
 }
 
-/* restore command and BARs after a reset has wiped them out */
+/**
+ * restore command and BARs after a reset has wiped them out
+ *
+ * Returns 0 on success, otherwise a negative error value
+ */
 int restore_pci_variables(struct hfi1_devdata *dd)
 {
 	int ret = 0;
@@ -386,10 +390,14 @@ int restore_pci_variables(struct hfi1_devdata *dd)
 
 error:
 	dd_dev_err(dd, "Unable to write to PCI config\n");
-	return ret;
+	return pcibios_err_to_errno(ret);
 }
 
-/* Save BARs and command to rewrite after device reset */
+/**
+ * Save BARs and command to rewrite after device reset
+ *
+ * Returns 0 on success, otherwise a negative error value
+ */
 int save_pci_variables(struct hfi1_devdata *dd)
 {
 	int ret = 0;
@@ -441,7 +449,7 @@ int save_pci_variables(struct hfi1_devdata *dd)
 
 error:
 	dd_dev_err(dd, "Unable to read from PCI config\n");
-	return ret;
+	return pcibios_err_to_errno(ret);
 }
 
 /*
-- 
2.18.2

Re: [PATCH 3/8] PCI: Convert PCIBIOS_ error codes to non-PCI generic error codes

[Bjorn Helgaas]

On Tue, Jun 09, 2020 at 05:39:45PM +0200, refactormyself@gmail.com wrote:
> From: Bolarinwa Olayemi Saheed <refactormyself@gmail.com>
> 
> restore_pci_variables() and save_pci_variables() return PCIBIOS_ error
> codes from pcie capability accessors.
> 
> PCIBIOS_ error codes have positive values. Passing on these values is
> inconsistent with functions which return only a negative value on failure.
> 
> Before passing on return value of pcie capability accessors, call
> pcibios_err_to_errno() to convert any positive PCIBIOS_ error codes to
> negative non-PCI generic error values.
> 
> Signed-off-by: Bolarinwa Olayemi Saheed <refactormyself@gmail.com>
> Suggested-by: Bjorn Helgaas <bjorn@helgaas.com>
> ---
>  drivers/infiniband/hw/hfi1/pcie.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/hfi1/pcie.c b/drivers/infiniband/hw/hfi1/pcie.c
> index eb53781d0c6a..eb2790e9db36 100644
> --- a/drivers/infiniband/hw/hfi1/pcie.c
> +++ b/drivers/infiniband/hw/hfi1/pcie.c
> @@ -334,7 +334,11 @@ int pcie_speeds(struct hfi1_devdata *dd)
>  	return 0;
>  }
>  
> -/* restore command and BARs after a reset has wiped them out */
> +/**
> + * restore command and BARs after a reset has wiped them out
> + *
> + * Returns 0 on success, otherwise a negative error value
> + */
>  int restore_pci_variables(struct hfi1_devdata *dd)
>  {
>  	int ret = 0;

These initializations are obviously superfluous.  Generally
I wouldn't encourage mixing fixes into a single patch, but
removing the initialization and adding pcibios_err_to_errno()
are both so trivial that I think it would be OK here.

Also, I didn't notice before, but these driver patches need to be cc'd
to the driver maintainers.  In this case, get_maintainers.pl says:

  Mike Marciniszyn <mike.marciniszyn@intel.com> (supporter:HFI1 DRIVER)
  Dennis Dalessandro <dennis.dalessandro@intel.com> (supporter:HFI1 DRIVER)
  Doug Ledford <dledford@redhat.com> (supporter:INFINIBAND SUBSYSTEM)
  Jason Gunthorpe <jgg@ziepe.ca> (supporter:INFINIBAND SUBSYSTEM)
  linux-rdma@vger.kernel.org (open list:HFI1 DRIVER)
  linux-kernel@vger.kernel.org (open list)

I would also cc *all* the driver maintainers for all the patches on
the cover letter and add a note about how we might merge these.
They're all independent, so they *could* go individually via the
separate driver trees, but I think it might make sense to merge them
all together via the PCI tree just so the collection of similar fixes
is merged all at once.

> @@ -386,10 +390,14 @@ int restore_pci_variables(struct hfi1_devdata *dd)
>  
>  error:
>  	dd_dev_err(dd, "Unable to write to PCI config\n");
> -	return ret;
> +	return pcibios_err_to_errno(ret);
>  }
>  
> -/* Save BARs and command to rewrite after device reset */
> +/**
> + * Save BARs and command to rewrite after device reset
> + *
> + * Returns 0 on success, otherwise a negative error value
> + */
>  int save_pci_variables(struct hfi1_devdata *dd)
>  {
>  	int ret = 0;
> @@ -441,7 +449,7 @@ int save_pci_variables(struct hfi1_devdata *dd)
>  
>  error:
>  	dd_dev_err(dd, "Unable to read from PCI config\n");
> -	return ret;
> +	return pcibios_err_to_errno(ret);
>  }
>  
>  /*
> -- 
> 2.18.2
> 

[PATCH 4/8] PCI: Convert PCIBIOS_ error codes to non-PCI generic error codes

[refactormyself]

From: Bolarinwa Olayemi Saheed <refactormyself@gmail.com>

Both pcie_set_readrq() and pcie_set_readrq() return PCIBIOS_ error codes
which were passed down the call heirarchy from pcie capability accessors.

PCIBIOS_ error codes have positive values. Passing on these values is
inconsistent with functions which return only a negative value on failure.

Before passing on return value of pcie capability accessors, call
pcibios_err_to_errno() to convert any positive PCIBIOS_ error codes to
negative non-PCI generic error values.

Signed-off-by: Bolarinwa Olayemi Saheed <refactormyself@gmail.com>
Suggested-by: Bjorn Helgaas <bjorn@helgaas.com>
---
 drivers/pci/pci.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 595fcf59843f..fa49e5f9e4d1 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -5698,6 +5698,7 @@ EXPORT_SYMBOL(pcie_get_readrq);
 int pcie_set_readrq(struct pci_dev *dev, int rq)
 {
 	u16 v;
+	int ret;
 
 	if (rq < 128 || rq > 4096 || !is_power_of_2(rq))
 		return -EINVAL;
@@ -5716,8 +5717,10 @@ int pcie_set_readrq(struct pci_dev *dev, int rq)
 
 	v = (ffs(rq) - 8) << 12;
 
-	return pcie_capability_clear_and_set_word(dev, PCI_EXP_DEVCTL,
+	ret = pcie_capability_clear_and_set_word(dev, PCI_EXP_DEVCTL,
 						  PCI_EXP_DEVCTL_READRQ, v);
+
+	return pcibios_err_to_errno(ret);
 }
 EXPORT_SYMBOL(pcie_set_readrq);
 
@@ -5748,6 +5751,7 @@ EXPORT_SYMBOL(pcie_get_mps);
 int pcie_set_mps(struct pci_dev *dev, int mps)
 {
 	u16 v;
+	int ret;
 
 	if (mps < 128 || mps > 4096 || !is_power_of_2(mps))
 		return -EINVAL;
@@ -5757,8 +5761,10 @@ int pcie_set_mps(struct pci_dev *dev, int mps)
 		return -EINVAL;
 	v <<= 5;
 
-	return pcie_capability_clear_and_set_word(dev, PCI_EXP_DEVCTL,
+	ret = pcie_capability_clear_and_set_word(dev, PCI_EXP_DEVCTL,
 						  PCI_EXP_DEVCTL_PAYLOAD, v);
+
+	return pcibios_err_to_errno(ret);
 }
 EXPORT_SYMBOL(pcie_set_mps);
 
-- 
2.18.2

[PATCH 5/8] PCI: Convert PCIBIOS_ error codes to non-PCI generic error codes

[refactormyself]

From: Bolarinwa Olayemi Saheed <refactormyself@gmail.com>

pqi_set_pcie_completion_timeout() return PCIBIOS_ error codes which were
passed on down the call heirarchy from pcie capability accessors.

PCIBIOS_ error codes have positive values. Passing on these values is
inconsistent with functions which return only a negative value on failure.

Before passing on return value of pcie capability accessors, call
pcibios_err_to_errno() to convert any positive PCIBIOS_ error codes to
negative non-PCI generic error values.

Signed-off-by: Bolarinwa Olayemi Saheed <refactormyself@gmail.com>
Suggested-by: Bjorn Helgaas <bjorn@helgaas.com>
---
 drivers/scsi/smartpqi/smartpqi_init.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/smartpqi/smartpqi_init.c b/drivers/scsi/smartpqi/smartpqi_init.c
index cd157f11eb22..bd38c8cea56e 100644
--- a/drivers/scsi/smartpqi/smartpqi_init.c
+++ b/drivers/scsi/smartpqi/smartpqi_init.c
@@ -7423,8 +7423,12 @@ static int pqi_ctrl_init_resume(struct pqi_ctrl_info *ctrl_info)
 static inline int pqi_set_pcie_completion_timeout(struct pci_dev *pci_dev,
 	u16 timeout)
 {
-	return pcie_capability_clear_and_set_word(pci_dev, PCI_EXP_DEVCTL2,
+	int rc;
+
+	rc = pcie_capability_clear_and_set_word(pci_dev, PCI_EXP_DEVCTL2,
 		PCI_EXP_DEVCTL2_COMP_TIMEOUT, timeout);
+
+	return pcibios_err_to_errno(rc);
 }
 
 static int pqi_pci_init(struct pqi_ctrl_info *ctrl_info)
-- 
2.18.2

[PATCH 6/8] PCI: Convert PCIBIOS_ error codes to non-PCI generic error codes

[refactormyself]

From: Bolarinwa Olayemi Saheed <refactormyself@gmail.com>

pci_enable_pcie_error_reporting() return PCIBIOS_ error codes which were
passed on down the call heirarchy pcie capability accessors.

PCIBIOS_ error codes have positive values. Passing on these values is
inconsistent with functions which return only a negative value on failure.

Before passing on return value of pcie capability accessors, call
pcibios_err_to_errno() to convert any positive PCIBIOS_ error codes to
negative non-PCI generic error values.

Signed-off-by: Bolarinwa Olayemi Saheed <refactormyself@gmail.com>
Suggested-by: Bjorn Helgaas <bjorn@helgaas.com>
---
 drivers/pci/pcie/aer.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index f4274d301235..95d480a52078 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -349,13 +349,17 @@ bool aer_acpi_firmware_first(void)
 
 int pci_enable_pcie_error_reporting(struct pci_dev *dev)
 {
+	int rc;
+
 	if (pcie_aer_get_firmware_first(dev))
 		return -EIO;
 
 	if (!dev->aer_cap)
 		return -EIO;
 
-	return pcie_capability_set_word(dev, PCI_EXP_DEVCTL, PCI_EXP_AER_FLAGS);
+	rc = pcie_capability_set_word(dev, PCI_EXP_DEVCTL, PCI_EXP_AER_FLAGS);
+
+	return rc;
 }
 EXPORT_SYMBOL_GPL(pci_enable_pcie_error_reporting);
 
-- 
2.18.2

Re: [PATCH 6/8] PCI: Convert PCIBIOS_ error codes to non-PCI generic error codes

[Bjorn Helgaas]

On Tue, Jun 09, 2020 at 05:39:48PM +0200, refactormyself@gmail.com wrote:
> From: Bolarinwa Olayemi Saheed <refactormyself@gmail.com>
> 
> pci_enable_pcie_error_reporting() return PCIBIOS_ error codes which were
> passed on down the call heirarchy pcie capability accessors.
> 
> PCIBIOS_ error codes have positive values. Passing on these values is
> inconsistent with functions which return only a negative value on failure.
> 
> Before passing on return value of pcie capability accessors, call
> pcibios_err_to_errno() to convert any positive PCIBIOS_ error codes to
> negative non-PCI generic error values.
> 
> Signed-off-by: Bolarinwa Olayemi Saheed <refactormyself@gmail.com>
> Suggested-by: Bjorn Helgaas <bjorn@helgaas.com>
> ---
>  drivers/pci/pcie/aer.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
> index f4274d301235..95d480a52078 100644
> --- a/drivers/pci/pcie/aer.c
> +++ b/drivers/pci/pcie/aer.c
> @@ -349,13 +349,17 @@ bool aer_acpi_firmware_first(void)
>  
>  int pci_enable_pcie_error_reporting(struct pci_dev *dev)
>  {
> +	int rc;
> +
>  	if (pcie_aer_get_firmware_first(dev))
>  		return -EIO;
>  
>  	if (!dev->aer_cap)
>  		return -EIO;
>  
> -	return pcie_capability_set_word(dev, PCI_EXP_DEVCTL, PCI_EXP_AER_FLAGS);
> +	rc = pcie_capability_set_word(dev, PCI_EXP_DEVCTL, PCI_EXP_AER_FLAGS);
> +
> +	return rc;

I think this is missing the pcibios_err_to_errno()?

>  }
>  EXPORT_SYMBOL_GPL(pci_enable_pcie_error_reporting);
>  
> -- 
> 2.18.2
> 

[PATCH 7/8] PCI: Convert PCIBIOS_ error codes to non-PCI generic error codes

[refactormyself]

From: Bolarinwa Olayemi Saheed <refactormyself@gmail.com>

pci_disable_pcie_error_reporting() returns PCIBIOS_ error code which were
passed down the call heirarchy from pcie capability accessors.

PCIBIOS_ error codes have positive values. Passing on these values is
inconsistent with functions which return only a negative value on failure.

Before passing on return value of pcie capability accessors, call
pcibios_err_to_errno() to convert any positive PCIBIOS_ error codes to
negative non-PCI generic error values.

Signed-off-by: Bolarinwa Olayemi Saheed <refactormyself@gmail.com>
Suggested-by: Bjorn Helgaas <bjorn@helgaas.com>
---
 drivers/pci/pcie/aer.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index 95d480a52078..53e2ecb64c72 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -365,11 +365,15 @@ EXPORT_SYMBOL_GPL(pci_enable_pcie_error_reporting);
 
 int pci_disable_pcie_error_reporting(struct pci_dev *dev)
 {
+	int rc;
+
 	if (pcie_aer_get_firmware_first(dev))
 		return -EIO;
 
-	return pcie_capability_clear_word(dev, PCI_EXP_DEVCTL,
+	rc = pcie_capability_clear_word(dev, PCI_EXP_DEVCTL,
 					  PCI_EXP_AER_FLAGS);
+
+	return rc;
 }
 EXPORT_SYMBOL_GPL(pci_disable_pcie_error_reporting);
 
-- 
2.18.2

Re: [PATCH 7/8] PCI: Convert PCIBIOS_ error codes to non-PCI generic error codes

[Bjorn Helgaas]

On Tue, Jun 09, 2020 at 05:39:49PM +0200, refactormyself@gmail.com wrote:
> From: Bolarinwa Olayemi Saheed <refactormyself@gmail.com>
> 
> pci_disable_pcie_error_reporting() returns PCIBIOS_ error code which were
> passed down the call heirarchy from pcie capability accessors.
> 
> PCIBIOS_ error codes have positive values. Passing on these values is
> inconsistent with functions which return only a negative value on failure.
> 
> Before passing on return value of pcie capability accessors, call
> pcibios_err_to_errno() to convert any positive PCIBIOS_ error codes to
> negative non-PCI generic error values.
> 
> Signed-off-by: Bolarinwa Olayemi Saheed <refactormyself@gmail.com>
> Suggested-by: Bjorn Helgaas <bjorn@helgaas.com>
> ---
>  drivers/pci/pcie/aer.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
> index 95d480a52078..53e2ecb64c72 100644
> --- a/drivers/pci/pcie/aer.c
> +++ b/drivers/pci/pcie/aer.c
> @@ -365,11 +365,15 @@ EXPORT_SYMBOL_GPL(pci_enable_pcie_error_reporting);
>  
>  int pci_disable_pcie_error_reporting(struct pci_dev *dev)
>  {
> +	int rc;
> +
>  	if (pcie_aer_get_firmware_first(dev))
>  		return -EIO;
>  
> -	return pcie_capability_clear_word(dev, PCI_EXP_DEVCTL,
> +	rc = pcie_capability_clear_word(dev, PCI_EXP_DEVCTL,
>  					  PCI_EXP_AER_FLAGS);
> +
> +	return rc;

Also missing pcibios_err_to_errno()?

>  }
>  EXPORT_SYMBOL_GPL(pci_disable_pcie_error_reporting);
>  
> -- 
> 2.18.2
> 

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

[refactormyself]

From: Bolarinwa Olayemi Saheed <refactormyself@gmail.com>

The pcie capability accessors can return 0, -EINVAL, or any PCIBIOS_ error
code. PCIBIOS_ error codes have positive values. 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.

Signed-off-by: Bolarinwa Olayemi Saheed <refactormyself@gmail.com>
Suggested-by: Bjorn Helgaas <bjorn@helgaas.com>
---
 drivers/pci/access.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/access.c b/drivers/pci/access.c
index 79c4a2ef269a..cbb3804903a0 100644
--- a/drivers/pci/access.c
+++ b/drivers/pci/access.c
@@ -409,7 +409,7 @@ int pcie_capability_read_word(struct pci_dev *dev, int pos, u16 *val)
 
 	*val = 0;
 	if (pos & 1)
-		return -EINVAL;
+		return PCIBIOS_BAD_REGISTER_NUMBER;
 
 	if (pcie_capability_reg_implemented(dev, pos)) {
 		ret = pci_read_config_word(dev, pci_pcie_cap(dev) + pos, val);
@@ -444,7 +444,7 @@ int pcie_capability_read_dword(struct pci_dev *dev, int pos, u32 *val)
 
 	*val = 0;
 	if (pos & 3)
-		return -EINVAL;
+		return PCIBIOS_BAD_REGISTER_NUMBER;
 
 	if (pcie_capability_reg_implemented(dev, pos)) {
 		ret = pci_read_config_dword(dev, pci_pcie_cap(dev) + pos, val);
@@ -469,7 +469,7 @@ EXPORT_SYMBOL(pcie_capability_read_dword);
 int pcie_capability_write_word(struct pci_dev *dev, int pos, u16 val)
 {
 	if (pos & 1)
-		return -EINVAL;
+		return PCIBIOS_BAD_REGISTER_NUMBER;
 
 	if (!pcie_capability_reg_implemented(dev, pos))
 		return 0;
@@ -481,7 +481,7 @@ EXPORT_SYMBOL(pcie_capability_write_word);
 int pcie_capability_write_dword(struct pci_dev *dev, int pos, u32 val)
 {
 	if (pos & 3)
-		return -EINVAL;
+		return PCIBIOS_BAD_REGISTER_NUMBER;
 
 	if (!pcie_capability_reg_implemented(dev, pos))
 		return 0;
-- 
2.18.2

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

[Bjorn Helgaas]

On Tue, Jun 09, 2020 at 05:39:50PM +0200, refactormyself@gmail.com wrote:
> From: Bolarinwa Olayemi Saheed <refactormyself@gmail.com>
> 
> The pcie capability accessors can return 0, -EINVAL, or any PCIBIOS_ error
> code. PCIBIOS_ error codes have positive values. 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.

s/pcie/PCIe/ (also below)
s/pci/PCI/
s/code.This/code. This/

> Return PCIBIOS_BAD_REGISTER_NUMBER instead of -EINVAL in all pcie
> capability accessors.
> 
> Signed-off-by: Bolarinwa Olayemi Saheed <refactormyself@gmail.com>
> Suggested-by: Bjorn Helgaas <bjorn@helgaas.com>
> ---
>  drivers/pci/access.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pci/access.c b/drivers/pci/access.c
> index 79c4a2ef269a..cbb3804903a0 100644
> --- a/drivers/pci/access.c
> +++ b/drivers/pci/access.c
> @@ -409,7 +409,7 @@ int pcie_capability_read_word(struct pci_dev *dev, int pos, u16 *val)
>  
>  	*val = 0;
>  	if (pos & 1)
> -		return -EINVAL;
> +		return PCIBIOS_BAD_REGISTER_NUMBER;
>  
>  	if (pcie_capability_reg_implemented(dev, pos)) {
>  		ret = pci_read_config_word(dev, pci_pcie_cap(dev) + pos, val);
> @@ -444,7 +444,7 @@ int pcie_capability_read_dword(struct pci_dev *dev, int pos, u32 *val)
>  
>  	*val = 0;
>  	if (pos & 3)
> -		return -EINVAL;
> +		return PCIBIOS_BAD_REGISTER_NUMBER;
>  
>  	if (pcie_capability_reg_implemented(dev, pos)) {
>  		ret = pci_read_config_dword(dev, pci_pcie_cap(dev) + pos, val);
> @@ -469,7 +469,7 @@ EXPORT_SYMBOL(pcie_capability_read_dword);
>  int pcie_capability_write_word(struct pci_dev *dev, int pos, u16 val)
>  {
>  	if (pos & 1)
> -		return -EINVAL;
> +		return PCIBIOS_BAD_REGISTER_NUMBER;
>  
>  	if (!pcie_capability_reg_implemented(dev, pos))
>  		return 0;
> @@ -481,7 +481,7 @@ EXPORT_SYMBOL(pcie_capability_write_word);
>  int pcie_capability_write_dword(struct pci_dev *dev, int pos, u32 val)
>  {
>  	if (pos & 3)
> -		return -EINVAL;
> +		return PCIBIOS_BAD_REGISTER_NUMBER;
>  
>  	if (!pcie_capability_reg_implemented(dev, pos))
>  		return 0;
> -- 
> 2.18.2
> 

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

[Bjorn Helgaas]

On Tue, Jun 09, 2020 at 05:39:42PM +0200, refactormyself@gmail.com wrote:
> From: Bolarinwa Olayemi Saheed <refactormyself@gmail.com>
> 
> 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