On Wed, Jan 15, 2025 at 04:42:27PM +0800, Ruidong Tian wrote:
> Subject: Re: [PATCH v3 4/5] RAS/ATL: Unified ATL interface for ARM64 and AMD
The condensed patch description in the subject line should start with a
uppercase letter and should be written in imperative tone.
> Translate device normalize address in AMD, also named logical address,
> to system physical address is a common interface in RAS. Provides common
> interface both for AMD and ARM.
This needs a lot more explanation.
> Signed-off-by: Ruidong Tian <tianruidong@xxxxxxxxxxxxxxxxx>
> ---
> drivers/edac/amd64_edac.c | 2 +-
> drivers/ras/aest/aest-core.c | 12 ++++++------
> drivers/ras/amd/atl/core.c | 4 ++--
> drivers/ras/amd/atl/internal.h | 2 +-
> drivers/ras/amd/atl/umc.c | 3 ++-
> drivers/ras/ras.c | 24 +++++++++++-------------
> include/linux/ras.h | 9 ++++-----
> 7 files changed, 27 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
> index ddfbdb66b794..1e9c96e4daa8 100644
> --- a/drivers/edac/amd64_edac.c
> +++ b/drivers/edac/amd64_edac.c
> @@ -2832,7 +2832,7 @@ static void decode_umc_error(int node_id, struct mce *m)
> a_err.ipid = m->ipid;
> a_err.cpu = m->extcpu;
>
> - sys_addr = amd_convert_umc_mca_addr_to_sys_addr(&a_err);
> + sys_addr = convert_ras_la_to_spa(&a_err);
No, this is not how all this is done. You don't rename functions and make them
generic - you *extract* generic functionality into generic functions and have
other functions which use them, call them.
And you do that when there are users, not before.
Ok, that should be enough feedback for now.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
