[PATCH v3] EDAC: i7300: disable error reporting if init fails and refactor helper

[PATCH v3] EDAC: i7300: disable error reporting if init fails and refactor helper

[Tushar Tibude]

If error reporting is enabled during initialization but initialization
fails immediately after, or during normal driver exit, error reporting
is left enabled in the mask register even after exit.

Replace i7300_enable_error_reporting() with i7300_set_error_reporting()
to combine enabling/disabling. Disable reporting at initialization
failure and driver exit, before call to i7300_put_devices() for cleanup.
Add enabled reporting flag to i7300_pvt.

This ensures clean hardware handling by disabling any unused error
reporting bits before exiting.

Signed-off-by: Tushar Tibude <tushar.tibude1000@gmail.com>
---
v2:
 - Replace disable function with i7300_set_error_reporting
 - Disable reporting at normal driver exit
 - Move enabled_error_reporting flag initialization
v3:
 - Move enabled_error_reporting flag to i7300_pvt struct
 - Conditionally disable error reporting at normal driver exit
 drivers/edac/i7300_edac.c | 33 ++++++++++++++++++++++++---------
 1 file changed, 24 insertions(+), 9 deletions(-)

diff --git a/drivers/edac/i7300_edac.c b/drivers/edac/i7300_edac.c
index 69068f8d0..64bc2d805 100644
--- a/drivers/edac/i7300_edac.c
+++ b/drivers/edac/i7300_edac.c
@@ -111,6 +111,9 @@ struct i7300_pvt {
 
 	/* Temporary buffer for use when preparing error messages */
 	char *tmp_prt_buffer;
+
+	/* Hardware error reporting status */
+	bool enabled_error_reporting;
 };
 
 /* FIXME: Why do we need to have this static? */
@@ -550,11 +553,12 @@ static void i7300_clear_error(struct mem_ctl_info *mci)
 }
 
 /**
- * i7300_enable_error_reporting() - Enable the memory reporting logic at the
+ * i7300_set_error_reporting() - Enable or disable the memory reporting logic at the
  *				    hardware
  * @mci: struct mem_ctl_info pointer
+ * @enable: enables if 'true', disables if 'false'
  */
-static void i7300_enable_error_reporting(struct mem_ctl_info *mci)
+static void i7300_set_error_reporting(struct mem_ctl_info *mci, bool enable)
 {
 	struct i7300_pvt *pvt = mci->pvt_info;
 	u32 fbd_error_mask;
@@ -563,8 +567,11 @@ static void i7300_enable_error_reporting(struct mem_ctl_info *mci)
 	pci_read_config_dword(pvt->pci_dev_16_1_fsb_addr_map,
 			      EMASK_FBD, &fbd_error_mask);
 
-	/* Enable with a '0' */
-	fbd_error_mask &= ~(EMASK_FBD_ERR_MASK);
+	/* Enable with 0, disable with 1 */
+	if (enable)
+		fbd_error_mask &= ~(EMASK_FBD_ERR_MASK);
+	else
+		fbd_error_mask |= EMASK_FBD_ERR_MASK;
 
 	pci_write_config_dword(pvt->pci_dev_16_1_fsb_addr_map,
 			       EMASK_FBD, fbd_error_mask);
@@ -1087,17 +1094,19 @@ static int i7300_init_one(struct pci_dev *pdev, const struct pci_device_id *id)
 	if (i7300_get_mc_regs(mci)) {
 		edac_dbg(0, "MC: Setting mci->edac_cap to EDAC_FLAG_NONE because i7300_init_csrows() returned nonzero value\n");
 		mci->edac_cap = EDAC_FLAG_NONE;	/* no csrows found */
+		pvt->enabled_error_reporting = false;
 	} else {
 		edac_dbg(1, "MC: Enable error reporting now\n");
-		i7300_enable_error_reporting(mci);
+		i7300_set_error_reporting(mci, true);
+		pvt->enabled_error_reporting = true;
 	}
 
 	/* add this new MC control structure to EDAC's list of MCs */
 	if (edac_mc_add_mc(mci)) {
 		edac_dbg(0, "MC: failed edac_mc_add_mc()\n");
-		/* FIXME: perhaps some code should go here that disables error
-		 * reporting if we just enabled it
-		 */
+		/* Disable error reporting if we just enabled it */
+		if (pvt->enabled_error_reporting)
+			i7300_set_error_reporting(mci, false);
 		goto fail1;
 	}
 
@@ -1134,6 +1143,7 @@ static int i7300_init_one(struct pci_dev *pdev, const struct pci_device_id *id)
 static void i7300_remove_one(struct pci_dev *pdev)
 {
 	struct mem_ctl_info *mci;
+	struct i7300_pvt *pvt;
 	char *tmp;
 
 	edac_dbg(0, "\n");
@@ -1145,7 +1155,12 @@ static void i7300_remove_one(struct pci_dev *pdev)
 	if (!mci)
 		return;
 
-	tmp = ((struct i7300_pvt *)mci->pvt_info)->tmp_prt_buffer;
+	pvt = (struct i7300_pvt *)mci->pvt_info;
+	tmp = pvt->tmp_prt_buffer;
+
+	/* Disable error reporting before unregistering device */
+	if (pvt->enabled_error_reporting)
+		i7300_set_error_reporting(mci, false);
 
 	/* retrieve references to resources, and free those resources */
 	i7300_put_devices(mci);
-- 
2.43.0

RE: [PATCH v3] EDAC: i7300: disable error reporting if init fails and refactor helper

[Zhuo, Qiuxu]

> From: Tushar Tibude <tushar.tibude1000@gmail.com>
> Sent: Wednesday, April 29, 2026 5:48 PM
> To: mchehab@kernel.org; bp@alien8.de; Luck, Tony <tony.luck@intel.com>;
> linux-edac@vger.kernel.org; linux-kernel@vger.kernel.org
> Cc: Zhuo, Qiuxu <qiuxu.zhuo@intel.com>; Tushar Tibude
> <tushar.tibude1000@gmail.com>
> Subject: [PATCH v3] EDAC: i7300: disable error reporting if init fails and
> refactor helper
> 
> If error reporting is enabled during initialization but initialization fails
> immediately after, or during normal driver exit, error reporting is left enabled
> in the mask register even after exit.
> 
> Replace i7300_enable_error_reporting() with i7300_set_error_reporting() to
> combine enabling/disabling. Disable reporting at initialization failure and
> driver exit, before call to i7300_put_devices() for cleanup.
> Add enabled reporting flag to i7300_pvt.
> 
> This ensures clean hardware handling by disabling any unused error reporting
> bits before exiting.
> 
> Signed-off-by: Tushar Tibude <tushar.tibude1000@gmail.com>
> ---
> v2:
>  - Replace disable function with i7300_set_error_reporting
>  - Disable reporting at normal driver exit
>  - Move enabled_error_reporting flag initialization
> v3:
>  - Move enabled_error_reporting flag to i7300_pvt struct
>  - Conditionally disable error reporting at normal driver exit
> drivers/edac/i7300_edac.c | 33 ++++++++++++++++++++++++---------
>  1 file changed, 24 insertions(+), 9 deletions(-)

LGTM. Thanks.

  Reviewed-by: Qiuxu Zhuo <qiuxu.zhuo@intel.com>