Date:   Wed, 15 Dec 2021 19:01:22 +0100
From:   Borislav Petkov <bp@alien8.de>
To:     Yazen Ghannam <yazen.ghannam@amd.com>
Cc:     linux-edac@vger.kernel.org, linux-kernel@vger.kernel.org,
        mchehab@kernel.org, tony.luck@intel.com, james.morse@arm.com,
        rric@kernel.org, Smita.KoralahalliChannabasappa@amd.com,
        william.roche@oracle.com
Subject: Re: [PATCH v2 1/2] EDAC/amd64: Check register values from all UMCs
Message-ID: <YbotciKVDsH1Fl1H@zn.tnic>
References: <20211215155309.2711917-1-yazen.ghannam@amd.com>
 <20211215155309.2711917-2-yazen.ghannam@amd.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <20211215155309.2711917-2-yazen.ghannam@amd.com>
Precedence: bulk

On Wed, Dec 15, 2021 at 03:53:08PM +0000, Yazen Ghannam wrote:
> -		if ((pvt->umc[0].dimm_cfg | pvt->umc[1].dimm_cfg) & BIT(5))
> +		u32 umc_cfg = 0, dimm_cfg = 0, i = 0;
> +
> +		for_each_umc(i) {
> +			umc_cfg  |= pvt->umc[i].umc_cfg;
> +			dimm_cfg |= pvt->umc[i].dimm_cfg;
> +		}
> +
> +		if (dimm_cfg & BIT(5))
>  			pvt->dram_type = MEM_LRDDR4;
> -		else if ((pvt->umc[0].dimm_cfg | pvt->umc[1].dimm_cfg) & BIT(4))
> +		else if (dimm_cfg & BIT(4))

You're working here under the assumption that bit 4 and 5 will have the
same value on all those UMCs.

You're probably going to say that that is how the BIOS is programming
them so they should be all the same and any other configuration is
invalid but lemme still ask about it explicitly.

And if so, this would probably need a comment above it which I can add
when applying...

Hmm?

-- 
Regards/Gruss,
    Boris.

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