linux-edac.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Saheed O. Bolarinwa" <refactormyself@gmail.com>
To: helgaas@kernel.org, Borislav Petkov <bp@alien8.de>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Tony Luck <tony.luck@intel.com>
Cc: "Saheed O. Bolarinwa" <refactormyself@gmail.com>,
	bjorn@helgaas.com, skhan@linuxfoundation.org,
	linux-kernel-mentees@lists.linuxfoundation.org,
	linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
	James Morse <james.morse@arm.com>,
	Robert Richter <rrichter@marvell.com>,
	linux-edac@vger.kernel.org
Subject: [RFC PATCH 06/17] edac: Drop uses of pci_read_config_*() return value
Date: Sat,  1 Aug 2020 13:24:35 +0200	[thread overview]
Message-ID: <20200801112446.149549-7-refactormyself@gmail.com> (raw)
In-Reply-To: <20200801112446.149549-1-refactormyself@gmail.com>

The return value of pci_read_config_*() may not indicate a device error.
However, the value read by these functions is more likely to indicate
this kind of error. This presents two overlapping ways of reporting
errors and complicates error checking.

It is possible to move to one single way of checking for error if the
dependency on the return value of these functions is removed, then it
can later be made to return void.

Remove all uses of the return value of pci_read_config_*().
Check the actual value read for ~0. In this case, ~0 is an invalid value
thus it indicates some kind of error.

drivers/edac/amd8111_edac.c edac_pci_read_dword() :
None of the callers of edac_pci_read_dword() uses the return value. The
obtained value can be checked for validity to confirm success.

Change the return type of edac_pci_read_dword() to void.

Suggested-by: Bjorn Helgaas <bjorn@helgaas.com>
Signed-off-by: Saheed O. Bolarinwa <refactormyself@gmail.com>
---
 drivers/edac/amd64_edac.c      |  8 +++-----
 drivers/edac/amd8111_edac.c    | 16 +++++-----------
 drivers/edac/amd8131_edac.c    |  6 ++----
 drivers/edac/i82443bxgx_edac.c |  3 ++-
 drivers/edac/sb_edac.c         | 12 ++++++++----
 drivers/edac/skx_common.c      | 18 ++++++++++++------
 6 files changed, 32 insertions(+), 31 deletions(-)

diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index 6262f6370c5d..f798eb17cb23 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -57,14 +57,12 @@ static const struct scrubrate {
 int __amd64_read_pci_cfg_dword(struct pci_dev *pdev, int offset,
 			       u32 *val, const char *func)
 {
-	int err = 0;
-
-	err = pci_read_config_dword(pdev, offset, val);
-	if (err)
+	pci_read_config_dword(pdev, offset, val);
+	if (*val == (u32)~0)
 		amd64_warn("%s: error reading F%dx%03x.\n",
 			   func, PCI_FUNC(pdev->devfn), offset);
 
-	return err;
+	return -ENODEV;
 }
 
 int __amd64_write_pci_cfg_dword(struct pci_dev *pdev, int offset,
diff --git a/drivers/edac/amd8111_edac.c b/drivers/edac/amd8111_edac.c
index 7508aa416ddb..ebf6deaf1d3d 100644
--- a/drivers/edac/amd8111_edac.c
+++ b/drivers/edac/amd8111_edac.c
@@ -34,24 +34,18 @@ enum amd8111_edac_pcis {
 };
 
 /* Wrapper functions for accessing PCI configuration space */
-static int edac_pci_read_dword(struct pci_dev *dev, int reg, u32 *val32)
+static void edac_pci_read_dword(struct pci_dev *dev, int reg, u32 *val32)
 {
-	int ret;
-
-	ret = pci_read_config_dword(dev, reg, val32);
-	if (ret != 0)
+	pci_read_config_dword(dev, reg, val32);
+	if (val32 == (u32)~0)
 		printk(KERN_ERR AMD8111_EDAC_MOD_STR
 			" PCI Access Read Error at 0x%x\n", reg);
-
-	return ret;
 }
 
 static void edac_pci_read_byte(struct pci_dev *dev, int reg, u8 *val8)
 {
-	int ret;
-
-	ret = pci_read_config_byte(dev, reg, val8);
-	if (ret != 0)
+	pci_read_config_byte(dev, reg, val8);
+	if (val8 == (u8)~0)
 		printk(KERN_ERR AMD8111_EDAC_MOD_STR
 			" PCI Access Read Error at 0x%x\n", reg);
 }
diff --git a/drivers/edac/amd8131_edac.c b/drivers/edac/amd8131_edac.c
index 169353710982..6df98c05391d 100644
--- a/drivers/edac/amd8131_edac.c
+++ b/drivers/edac/amd8131_edac.c
@@ -26,10 +26,8 @@
 /* Wrapper functions for accessing PCI configuration space */
 static void edac_pci_read_dword(struct pci_dev *dev, int reg, u32 *val32)
 {
-	int ret;
-
-	ret = pci_read_config_dword(dev, reg, val32);
-	if (ret != 0)
+	pci_read_config_dword(dev, reg, val32);
+	if (val32 == (u32)~0)
 		printk(KERN_ERR AMD8131_EDAC_MOD_STR
 			" PCI Access Read Error at 0x%x\n", reg);
 }
diff --git a/drivers/edac/i82443bxgx_edac.c b/drivers/edac/i82443bxgx_edac.c
index a2ca929e2168..d6797ed7ac65 100644
--- a/drivers/edac/i82443bxgx_edac.c
+++ b/drivers/edac/i82443bxgx_edac.c
@@ -243,7 +243,8 @@ static int i82443bxgx_edacmc_probe1(struct pci_dev *pdev, int dev_idx)
 	/* Something is really hosed if PCI config space reads from
 	 * the MC aren't working.
 	 */
-	if (pci_read_config_dword(pdev, I82443BXGX_NBXCFG, &nbxcfg))
+	pci_read_config_dword(pdev, I82443BXGX_NBXCFG, &nbxcfg);
+	if (nbxcfg == (u32)~0)
 		return -EIO;
 
 	layers[0].type = EDAC_MC_LAYER_CHIP_SELECT;
diff --git a/drivers/edac/sb_edac.c b/drivers/edac/sb_edac.c
index d414698ca324..e56a06d68a4e 100644
--- a/drivers/edac/sb_edac.c
+++ b/drivers/edac/sb_edac.c
@@ -1697,13 +1697,15 @@ static int get_dimm_config(struct mem_ctl_info *mci)
 
 		if (knl_get_dimm_capacity(pvt, knl_mc_sizes) != 0)
 			return -1;
-		if (pci_read_config_dword(pvt->pci_ta, KNL_MCMTR, &pvt->info.mcmtr)) {
+		pci_read_config_dword(pvt->pci_ta, KNL_MCMTR, &pvt->info.mcmtr);
+		if (pvt->info.mcmtr == (u32)~0) {
 			edac_dbg(0, "Failed to read KNL_MCMTR register\n");
 			return -ENODEV;
 		}
 	} else {
 		if (pvt->info.type == HASWELL || pvt->info.type == BROADWELL) {
-			if (pci_read_config_dword(pvt->pci_ha, HASWELL_HASYSDEFEATURE2, &reg)) {
+			pci_read_config_dword(pvt->pci_ha, HASWELL_HASYSDEFEATURE2, &reg);
+			if (reg == (u32)~0) {
 				edac_dbg(0, "Failed to read HASWELL_HASYSDEFEATURE2 register\n");
 				return -ENODEV;
 			}
@@ -1714,7 +1716,8 @@ static int get_dimm_config(struct mem_ctl_info *mci)
 				goto next;
 			}
 		}
-		if (pci_read_config_dword(pvt->pci_ras, RASENABLES, &reg)) {
+		pci_read_config_dword(pvt->pci_ras, RASENABLES, &reg);
+		if (reg == (u32)~0) {
 			edac_dbg(0, "Failed to read RASENABLES register\n");
 			return -ENODEV;
 		}
@@ -1727,7 +1730,8 @@ static int get_dimm_config(struct mem_ctl_info *mci)
 		}
 
 next:
-		if (pci_read_config_dword(pvt->pci_ta, MCMTR, &pvt->info.mcmtr)) {
+		pci_read_config_dword(pvt->pci_ta, MCMTR, &pvt->info.mcmtr);
+		if (pvt->info.mcmtr == (u32)~0) {
 			edac_dbg(0, "Failed to read MCMTR register\n");
 			return -ENODEV;
 		}
diff --git a/drivers/edac/skx_common.c b/drivers/edac/skx_common.c
index 6d8d6dc626bf..7956c75c289f 100644
--- a/drivers/edac/skx_common.c
+++ b/drivers/edac/skx_common.c
@@ -161,7 +161,8 @@ int skx_get_src_id(struct skx_dev *d, int off, u8 *id)
 {
 	u32 reg;
 
-	if (pci_read_config_dword(d->util_all, off, &reg)) {
+	pci_read_config_dword(d->util_all, off, &reg);
+	if (reg == (u32)~0) {
 		skx_printk(KERN_ERR, "Failed to read src id\n");
 		return -ENODEV;
 	}
@@ -174,7 +175,8 @@ int skx_get_node_id(struct skx_dev *d, u8 *id)
 {
 	u32 reg;
 
-	if (pci_read_config_dword(d->util_all, 0xf4, &reg)) {
+	pci_read_config_dword(d->util_all, 0xf4, &reg);
+	if (reg == (u32)~0) {
 		skx_printk(KERN_ERR, "Failed to read node id\n");
 		return -ENODEV;
 	}
@@ -220,7 +222,8 @@ int skx_get_all_bus_mappings(struct res_config *cfg, struct list_head **list)
 			return -ENOMEM;
 		}
 
-		if (pci_read_config_dword(pdev, cfg->busno_cfg_offset, &reg)) {
+		pci_read_config_dword(pdev, cfg->busno_cfg_offset, &reg);
+		if (reg == (u32)~0) {
 			kfree(d);
 			pci_dev_put(pdev);
 			skx_printk(KERN_ERR, "Failed to read bus idx\n");
@@ -259,19 +262,22 @@ int skx_get_hi_lo(unsigned int did, int off[], u64 *tolm, u64 *tohm)
 		return -ENODEV;
 	}
 
-	if (pci_read_config_dword(pdev, off[0], &reg)) {
+	pci_read_config_dword(pdev, off[0], &reg);
+	if (reg == (u32)~0) {
 		skx_printk(KERN_ERR, "Failed to read tolm\n");
 		goto fail;
 	}
 	skx_tolm = reg;
 
-	if (pci_read_config_dword(pdev, off[1], &reg)) {
+	pci_read_config_dword(pdev, off[1], &reg);
+	if (reg == (u32)~0) {
 		skx_printk(KERN_ERR, "Failed to read lower tohm\n");
 		goto fail;
 	}
 	skx_tohm = reg;
 
-	if (pci_read_config_dword(pdev, off[2], &reg)) {
+	pci_read_config_dword(pdev, off[2], &reg);
+	if (reg == (u32)~0) {
 		skx_printk(KERN_ERR, "Failed to read upper tohm\n");
 		goto fail;
 	}
-- 
2.18.4


  reply	other threads:[~2020-08-01 12:25 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-01 11:24 [RFC PATCH 00/17] Drop uses of pci_read_config_*() return value Saheed O. Bolarinwa
2020-08-01 11:24 ` Saheed O. Bolarinwa [this message]
2020-08-01 12:56 ` Borislav Petkov
2020-08-02 14:53   ` Tom Rix
2020-08-02 17:28   ` Saheed Bolarinwa
2020-08-02 18:46     ` Borislav Petkov
2020-08-02 19:14       ` Bjorn Helgaas
2020-08-02 20:18         ` Borislav Petkov
2020-08-03  6:56         ` Christoph Hellwig

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200801112446.149549-7-refactormyself@gmail.com \
    --to=refactormyself@gmail.com \
    --cc=bjorn@helgaas.com \
    --cc=bp@alien8.de \
    --cc=helgaas@kernel.org \
    --cc=james.morse@arm.com \
    --cc=linux-edac@vger.kernel.org \
    --cc=linux-kernel-mentees@lists.linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=rrichter@marvell.com \
    --cc=skhan@linuxfoundation.org \
    --cc=tony.luck@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).