Re: [bug report] EDAC, mellanox: Add ECC support for BlueField DDR4

[Borislav Petkov]

On Thu, Oct 24, 2024 at 11:20:45AM +0300, Dan Carpenter wrote:
> Hello Shravan Kumar Ramani,
> 
> Commit 82413e562ea6 ("EDAC, mellanox: Add ECC support for BlueField
> DDR4") from Jun 25, 2019 (linux-next), leads to the following Smatch
> static checker warning:
> 
> drivers/edac/bluefield_edac.c:205 bluefield_gather_report_ecc() error: uninitialized symbol 'dram_syndrom'.
> drivers/edac/bluefield_edac.c:219 bluefield_gather_report_ecc() error: uninitialized symbol 'dram_additional_info'.
> drivers/edac/bluefield_edac.c:231 bluefield_gather_report_ecc() error: uninitialized symbol 'edea0'.
> drivers/edac/bluefield_edac.c:231 bluefield_gather_report_ecc() error: uninitialized symbol 'edea1'.
> drivers/edac/bluefield_edac.c:256 bluefield_edac_check() error: uninitialized symbol 'ecc_count'.
> 
> drivers/edac/bluefield_edac.c
>     173 static void bluefield_gather_report_ecc(struct mem_ctl_info *mci,
>     174                                         int error_cnt,
>     175                                         int is_single_ecc)
>     176 {
>     177         struct bluefield_edac_priv *priv = mci->pvt_info;
>     178         u32 dram_additional_info, err_prank, edea0, edea1;
>     179         u32 ecc_latch_select, dram_syndrom, serr, derr, syndrom;
>     180         enum hw_event_mc_err_type ecc_type;
>     181         u64 ecc_dimm_addr;
>     182         int ecc_dimm, err;
>     183 
>     184         ecc_type = is_single_ecc ? HW_EVENT_ERR_CORRECTED :
>     185                                    HW_EVENT_ERR_UNCORRECTED;
>     186 
>     187         /*
>     188          * Tell the External Memory Interface to populate the relevant
>     189          * registers with information about the last ECC error occurrence.
>     190          */
>     191         ecc_latch_select = MLXBF_ECC_LATCH_SEL__START;
>     192         err = bluefield_edac_writel(priv, MLXBF_ECC_LATCH_SEL, ecc_latch_select);
>     193         if (err)
>     194                 dev_err(priv->dev, "ECC latch select write failed.\n");
>     195 
>     196         /*
>     197          * Verify that the ECC reported info in the registers is of the
>     198          * same type as the one asked to report. If not, just report the
>     199          * error without the detailed information.
>     200          */
>     201         err = bluefield_edac_readl(priv, MLXBF_SYNDROM, &dram_syndrom);
>     202         if (err)
>     203                 dev_err(priv->dev, "DRAM syndrom read failed.\n");
> 
> If bluefield_edac_readl() fails then dram_syndrom is uninitialized.
> 
>     204 
> --> 205         serr = FIELD_GET(MLXBF_SYNDROM__SERR, dram_syndrom);
>     206         derr = FIELD_GET(MLXBF_SYNDROM__DERR, dram_syndrom);
>     207         syndrom = FIELD_GET(MLXBF_SYNDROM__SYN, dram_syndrom);
> 

This looks forgotten.

I'm thinking of:

diff --git a/MAINTAINERS b/MAINTAINERS
index 8e0736dc2ee0..061149ade8c0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8222,8 +8222,7 @@ F:	Documentation/devicetree/bindings/edac/aspeed-sdram-edac.txt
 F:	drivers/edac/aspeed_edac.c
 
 EDAC-BLUEFIELD
-M:	Shravan Kumar Ramani <shravankr@xxxxxxxxxx>
-S:	Supported
+S:	Orphan
 F:	drivers/edac/bluefield_edac.c
 
 EDAC-CALXEDA

but lemme Cc people who have touched this recently first.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette