[PATCH] EDAC: i7300: disable error reporting if init fails

Tushar Tibude posted 1 patch 1 month, 2 weeks ago
drivers/edac/i7300_edac.c | 30 +++++++++++++++++++++++++++---
1 file changed, 27 insertions(+), 3 deletions(-)
[PATCH] EDAC: i7300: disable error reporting if init fails
Posted by Tushar Tibude 1 month, 2 weeks ago
If error reporting is enabled during initialization but initialization
fails immediately after, error reporting is left enabled in the mask
register even after exit.

Create the i7300_disable_error_reporting() function to disable error
reporting bits in the mask register EMASK_FBD. Call it at initialization
failure, before call to i7300_put_devices() for cleanup.

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

Signed-off-by: Tushar Tibude <tushar.tibude1000@gmail.com>
---
 drivers/edac/i7300_edac.c | 30 +++++++++++++++++++++++++++---
 1 file changed, 27 insertions(+), 3 deletions(-)

diff --git a/drivers/edac/i7300_edac.c b/drivers/edac/i7300_edac.c
index 69068f8d0..989cf53e9 100644
--- a/drivers/edac/i7300_edac.c
+++ b/drivers/edac/i7300_edac.c
@@ -570,6 +570,27 @@ static void i7300_enable_error_reporting(struct mem_ctl_info *mci)
 			       EMASK_FBD, fbd_error_mask);
 }
 
+/**
+ * i7300_disable_error_reporting() - Disable the memory reporting logic at the
+ *				     hardware
+ * @mci: struct mem_ctl_info pointer
+ */
+static void i7300_disable_error_reporting(struct mem_ctl_info *mci)
+{
+	struct i7300_pvt *pvt = mci->pvt_info;
+	u32 fbd_error_mask;
+
+	/* Read the FBD Error Mask Register */
+	pci_read_config_dword(pvt->pci_dev_16_1_fsb_addr_map,
+			      EMASK_FBD, &fbd_error_mask);
+
+	/* Disable by writing '1' */
+	fbd_error_mask |= EMASK_FBD_ERR_MASK;
+
+	pci_write_config_dword(pvt->pci_dev_16_1_fsb_addr_map,
+			       EMASK_FBD, fbd_error_mask);
+}
+
 /************************************************
  * i7300 Functions related to memory enumberation
  ************************************************/
@@ -1025,6 +1046,7 @@ static int i7300_init_one(struct pci_dev *pdev, const struct pci_device_id *id)
 	struct edac_mc_layer layers[3];
 	struct i7300_pvt *pvt;
 	int rc;
+	bool enabled_error_reporting;
 
 	/* wake up device */
 	rc = pci_enable_device(pdev);
@@ -1084,20 +1106,22 @@ static int i7300_init_one(struct pci_dev *pdev, const struct pci_device_id *id)
 
 	/* initialize the MC control structure 'csrows' table
 	 * with the mapping and control information */
+	enabled_error_reporting = false;
 	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 */
 	} else {
 		edac_dbg(1, "MC: Enable error reporting now\n");
 		i7300_enable_error_reporting(mci);
+		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 (enabled_error_reporting)
+			i7300_disable_error_reporting(mci);
 		goto fail1;
 	}
 
-- 
2.43.0
RE: [PATCH] EDAC: i7300: disable error reporting if init fails
Posted by Zhuo, Qiuxu 1 month, 2 weeks ago
> From: Tushar Tibude <tushar.tibude1000@gmail.com>
> Sent: Monday, April 27, 2026 8:26 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: Tushar Tibude <tushar.tibude1000@gmail.com>
> Subject: [PATCH] EDAC: i7300: disable error reporting if init fails
> 
> If error reporting is enabled during initialization but initialization fails
> immediately after, error reporting is left enabled in the mask register even
> after exit.
> 
> Create the i7300_disable_error_reporting() function to disable error reporting
> bits in the mask register EMASK_FBD. Call it at initialization failure, before call
> to i7300_put_devices() for cleanup.
> 
> This ensures clean hardware handling by disabling any unused error reporting
> bits before exiting.
> 
> Signed-off-by: Tushar Tibude <tushar.tibude1000@gmail.com>
> ---
>  drivers/edac/i7300_edac.c | 30 +++++++++++++++++++++++++++---
>  1 file changed, 27 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/edac/i7300_edac.c b/drivers/edac/i7300_edac.c index
> 69068f8d0..989cf53e9 100644
> --- a/drivers/edac/i7300_edac.c
> +++ b/drivers/edac/i7300_edac.c
> @@ -570,6 +570,27 @@ static void i7300_enable_error_reporting(struct
> mem_ctl_info *mci)
>  			       EMASK_FBD, fbd_error_mask);
>  }
> 
> +/**
> + * i7300_disable_error_reporting() - Disable the memory reporting logic at
> the
> + *				     hardware
> + * @mci: struct mem_ctl_info pointer
> + */
> +static void i7300_disable_error_reporting(struct mem_ctl_info *mci) {
> +	struct i7300_pvt *pvt = mci->pvt_info;
> +	u32 fbd_error_mask;
> +
> +	/* Read the FBD Error Mask Register */
> +	pci_read_config_dword(pvt->pci_dev_16_1_fsb_addr_map,
> +			      EMASK_FBD, &fbd_error_mask);
> +
> +	/* Disable by writing '1' */
> +	fbd_error_mask |= EMASK_FBD_ERR_MASK;
> +
> +	pci_write_config_dword(pvt->pci_dev_16_1_fsb_addr_map,
> +			       EMASK_FBD, fbd_error_mask);
> +}
> +

i7300_enable_error_reporting() and i7300_disable_error_reporting() have very
similar implementations. Would it make sense to combine them into a single helper, 
e.g. i7300_enable_error_reporting(mci, bool enable), to reduce code duplication?

'enable' is true to enable error reporting and false to disable error reporting.

>  /************************************************
>   * i7300 Functions related to memory enumberation
>   ************************************************/
> @@ -1025,6 +1046,7 @@ static int i7300_init_one(struct pci_dev *pdev, const
> struct pci_device_id *id)
>  	struct edac_mc_layer layers[3];
>  	struct i7300_pvt *pvt;
>  	int rc;
> +	bool enabled_error_reporting;
> 
>  	/* wake up device */
>  	rc = pci_enable_device(pdev);
> @@ -1084,20 +1106,22 @@ static int i7300_init_one(struct pci_dev *pdev,
> const struct pci_device_id *id)
> 
>  	/* initialize the MC control structure 'csrows' table
>  	 * with the mapping and control information */
> +	enabled_error_reporting = false;

Move this flag setting to

>  	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 */

here.

>  	} else {
>  		edac_dbg(1, "MC: Enable error reporting now\n");
>  		i7300_enable_error_reporting(mci);
> +		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 (enabled_error_reporting)
> +			i7300_disable_error_reporting(mci);
>  		goto fail1;
>  	}
> 
> --
> 2.43.0
> 

Need to disable error reporting in i7300_remove_one()?
Enabling it during initialization and disabling it during teardown ensures proper pairing.

Thanks!
-Qiuxu