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

[Yazen Ghannam]

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