Re: [PATCH 05/18] EDAC/amd64: Merge struct amd64_family_type into struct amd64_pvt

From: Borislav Petkov <bp@alien8.de>
To: Yazen Ghannam <yazen.ghannam@amd.com>
Cc: linux-edac@vger.kernel.org, linux-kernel@vger.kernel.org,
	Smita.KoralahalliChannabasappa@amd.com, muralidhara.mk@amd.com,
	naveenkrishna.chatradhi@amd.com
Subject: Re: [PATCH 05/18] EDAC/amd64: Merge struct amd64_family_type into struct amd64_pvt
Date: Fri, 13 May 2022 17:21:31 +0200	[thread overview]
Message-ID: <Yn53e7RZGBJ7yL76@zn.tnic> (raw)
In-Reply-To: <20220509145534.44912-6-yazen.ghannam@amd.com>

On Mon, May 09, 2022 at 02:55:21PM +0000, Yazen Ghannam wrote:
> From: Muralidhara M K <muralidhara.mk@amd.com>
> 
> Future AMD systems will support heterogeneous "AMD Node" types, e.g.
> CPU+GPU. Therefore, a global "family type" shared across all "AMD Nodes"
> is no longer appropriate.
> 
> Move struct low_ops routines and members of struct amd64_family_type
> to struct amd64_pvt.
> 
> Currently, there are many code branches that split between "modern" and
> "legacy" systems. Another code branch will be needed in order to cover
> "GPU" cases. However, rather than introduce another branching case in
> multiple functions, the current branching code should be switched to a
> set of function pointers. This change makes the code more readable and
> simplifies adding support for new families/models.
> 
> In order to reuse code, define two sets of function pointers. Use one
> for modern systems (Family 17h and later). This will not change between
> current CPU families. Use another set of function pointers for legacy
> systems (before Family 17h). Use the Family 16h versions as default
> for the legacy ops since these are the latest, and adjust the function
> pointers as needed for older families.
> 
> Rename the Family 17h functions to use a "umc" prefix. This is to
> indicate that the functions apply to all modern CPU familes (17h, 18h,

Unknown word [familes] in commit message.
Suggestions: ['families',

> and 19h) which all have Unified Memory Controllers (UMCs).
> 
> Signed-off-by: Muralidhara M K <muralidhara.mk@amd.com>
> Signed-off-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@amd.com>
> [Rebased/reworked patch and reworded commit message]
> Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
> ---
>  drivers/edac/amd64_edac.c | 353 +++++++++++++-------------------------
>  drivers/edac/amd64_edac.h |  66 +++----
>  2 files changed, 140 insertions(+), 279 deletions(-)

That diffstat looks good.

And it simplifies and streamlines the code real nicely.

Thx.

-- 
Regards/Gruss,
    Boris.

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