Re: [PATCH v5 3/5] EDAC/amd64: Extend family ops functions

From: Yazen Ghannam
Date: Wed Oct 27 2021 - 17:08:51 EST


On Mon, Oct 25, 2021 at 08:20:16PM +0530, Naveen Krishna Chatradhi wrote:
...
> @@ -3106,8 +3141,9 @@ static void read_mc_regs(struct amd64_pvt *pvt)
> edac_dbg(0, " TOP_MEM2 disabled\n");
> }
>
> - if (pvt->umc) {
> - __read_mc_regs_df(pvt);
> + if (pvt->ops->get_mc_regs) {
> + pvt->ops->get_mc_regs(pvt);

This entire read_mc_regs() function can be split up like how you've done the
others. So get_mc_regs() is set for all family types. The common code can be
split out into another function.

> +
> amd64_read_pci_cfg(pvt->F0, DF_DHAR, &pvt->dhar);
>
> goto skip;
> @@ -3154,7 +3190,10 @@ static void read_mc_regs(struct amd64_pvt *pvt)
> }
>
> skip:
> - read_dct_base_mask(pvt);
> + pvt->ops->prep_chip_select(pvt);
> +
> + if (pvt->ops->get_base_mask)

This check is redundant since it's done below in per_family_init().

...
> @@ -3703,6 +3739,20 @@ static struct amd64_family_type *per_family_init(struct amd64_pvt *pvt)
> return NULL;
> }
>
> + /* ops required for all the families */
> + if (!pvt->ops->early_channel_count || !pvt->ops->dbam_to_cs ||
> + !pvt->ops->prep_chip_select || !pvt->ops->get_base_mask ||
> + !pvt->ops->display_misc_regs || !pvt->ops->populate_csrows) {
> + edac_dbg(1, "Common helper routines not defined.\n");
> + return NULL;
> + }
> +
> + /* ops required for families 17h and later */
> + if (pvt->fam >= 0x17 && (!pvt->ops->get_umc_err_info || !pvt->ops->get_mc_regs)) {
> + edac_dbg(1, "Platform specific helper routines not defined.\n");
> + return NULL;
> + }
> +
> return fam_type;
> }
>

Thanks,
Yazen