lkml.org 
[lkml]   [2022]   [Feb]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH v4 01/24] EDAC/amd64: Define Data Fabric operations
On Thu, Jan 27, 2022 at 08:40:52PM +0000, Yazen Ghannam wrote:
> Define a stub to hold operations for different Data Fabric versions.
> This will be filled in following patches.
>
> Set the operations at init-time as appropriate for each model/family
> group.
>
> Also, start a glossary of acronyms used in the translation code.
>
> Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
> ---
> Link:
> https://lore.kernel.org/r/20211028175728.121452-6-yazen.ghannam@amd.com
>
> v3->v4:
> * Started glossary.
> * Included pr_debug() for failing case.
>
> v2->v3:
> * Was patch 6 in v2.
> * "df_ops" is set at init time.
>
> v1->v2:
> * New in v2.
>
> drivers/edac/amd64_edac.c | 21 +++++++++++++++++++++
> 1 file changed, 21 insertions(+)
>
> diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
> index fba609ada0e6..639dfbea3348 100644
> --- a/drivers/edac/amd64_edac.c
> +++ b/drivers/edac/amd64_edac.c
> @@ -988,6 +988,12 @@ static int sys_addr_to_csrow(struct mem_ctl_info *mci, u64 sys_addr)
> return csrow;
> }
>
> +/*
> + * Glossary of acronyms used in address translation for Zen-based systems
> + *
> + * DF = Data Fabric

Yeah, "DF: Data Fabric" is probably easier to parse, without that weird
spacing and equals sign.

> + */
> +
> /* Protect the PCI config register pairs used for DF indirect access. */
> static DEFINE_MUTEX(df_indirect_mutex);
>
> @@ -1058,6 +1064,14 @@ struct addr_ctx {
> u8 inst_id;
> };
>
> +struct data_fabric_ops {
> +};

I know that this is not the only example but we have struct definitions
interspersed with functions in the .c file while former should be all in
the header. It is a lot cleaner to have definitions and inline functions
in the header and the actual functionality in the C file but I leave it
up to you to decide what you prefer.

> +
> +struct data_fabric_ops df2_ops = {
> +};
> +
> +struct data_fabric_ops *df_ops;
> +
> static int umc_normaddr_to_sysaddr(u64 norm_addr, u16 nid, u8 umc, u64 *sys_addr)
> {
> u64 dram_base_addr, dram_limit_addr, dram_hole_base;
> @@ -1072,6 +1086,11 @@ static int umc_normaddr_to_sysaddr(u64 norm_addr, u16 nid, u8 umc, u64 *sys_addr
>
> struct addr_ctx ctx;
>
> + if (!df_ops) {
> + pr_debug("Data Fabric Operations not set");

That probably wants to be a WARN_ON_ONCE() so that it is loud and
prominent when it happens...

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette
\
 
 \ /
  Last update: 2022-02-11 20:12    [W:0.302 / U:2.072 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site