Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Tue, 26 Feb 2019 14:17:47 +0100
From:   Borislav Petkov <bp@alien8.de>
To:     "Ghannam, Yazen" <Yazen.Ghannam@amd.com>
Cc:     "linux-edac@vger.kernel.org" <linux-edac@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 3/5] EDAC/amd64: Recognize x16 Symbol Size
Message-ID: <20190226131747.GD14836@zn.tnic>
References: <20190219202536.15462-1-Yazen.Ghannam@amd.com>
 <20190219202536.15462-3-Yazen.Ghannam@amd.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <20190219202536.15462-3-Yazen.Ghannam@amd.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Tue, Feb 19, 2019 at 08:25:54PM +0000, Ghannam, Yazen wrote:
> From: Yazen Ghannam <yazen.ghannam@amd.com>
> 
> Future AMD systems may support x16 symbol sizes.
> 
> Recognize if a system is using x16 symbol size. Also, simplify the print
> statement.
> 
> Note that a x16 syndrome vector table is not necessary like with x4 or
> x8. This is because systems that support x16 symbol sizes will be SMCA
> systems. In which case, the syndrome can be directly extracted from the
> MCA_SYND[Syndrome] field.
> 
> Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
> ---
>  drivers/edac/amd64_edac.c | 15 +++++++++------
>  drivers/edac/amd64_edac.h |  2 +-
>  2 files changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
> index 507d824fe45a..bacd2cb22f29 100644
> --- a/drivers/edac/amd64_edac.c
> +++ b/drivers/edac/amd64_edac.c
> @@ -897,8 +897,7 @@ static void dump_misc_regs(struct amd64_pvt *pvt)
>  
>  	edac_dbg(1, "  DramHoleValid: %s\n", dhar_valid(pvt) ? "yes" : "no");
>  
> -	amd64_info("using %s syndromes.\n",
> -			((pvt->ecc_sym_sz == 8) ? "x8" : "x4"));
> +	amd64_info("using x%u syndromes.\n", pvt->ecc_sym_sz);
>  }
>  
>  /*
> @@ -2609,10 +2608,14 @@ static void determine_ecc_sym_sz(struct amd64_pvt *pvt)
>  
>  		for_each_umc(i) {
>  			/* Check enabled channels only: */
> -			if ((pvt->umc[i].sdp_ctrl & UMC_SDP_INIT) &&
> -			    (pvt->umc[i].ecc_ctrl & BIT(7))) {
> -				pvt->ecc_sym_sz = 8;
> -				break;
> +			if (pvt->umc[i].sdp_ctrl & UMC_SDP_INIT) {
> +				if (pvt->umc[i].ecc_ctrl & BIT(9)) {
> +					pvt->ecc_sym_sz = 16;
> +					break;
> +				} else if (pvt->umc[i].ecc_ctrl & BIT(7)) {
> +					pvt->ecc_sym_sz = 8;
> +					break;
> +				}
>  			}
>  		}
>  

Let's simplify this function a bit. Diff ontop:

---
diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index bacd2cb22f29..df21b00dca08 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -2611,18 +2611,14 @@ static void determine_ecc_sym_sz(struct amd64_pvt *pvt)
 			if (pvt->umc[i].sdp_ctrl & UMC_SDP_INIT) {
 				if (pvt->umc[i].ecc_ctrl & BIT(9)) {
 					pvt->ecc_sym_sz = 16;
-					break;
+					return;
 				} else if (pvt->umc[i].ecc_ctrl & BIT(7)) {
 					pvt->ecc_sym_sz = 8;
-					break;
+					return;
 				}
 			}
 		}
-
-		return;
-	}
-
-	if (pvt->fam >= 0x10) {
+	} else if (pvt->fam >= 0x10) {
 		u32 tmp;
 
 		amd64_read_pci_cfg(pvt->F3, EXT_NB_MCA_CFG, &tmp);

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.