Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Wed, 27 Oct 2021 21:08:35 +0000
From:   Yazen Ghannam <yazen.ghannam@amd.com>
To:     Naveen Krishna Chatradhi <nchatrad@amd.com>
Cc:     linux-edac@vger.kernel.org, x86@kernel.org,
        linux-kernel@vger.kernel.org, bp@alien8.de, mingo@redhat.com,
        mchehab@kernel.org, Muralidhara M K <muralimk@amd.com>
Subject: Re: [PATCH v5 3/5] EDAC/amd64: Extend family ops functions
Message-ID: <YXm/0wBVvplOzFva@yaz-ubuntu>
References: <20211025145018.29985-1-nchatrad@amd.com>
 <20211025145018.29985-4-nchatrad@amd.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20211025145018.29985-4-nchatrad@amd.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?rTir4ar9ilakWh7RqxI4GHSkV1LFskCq59SGgE4IqCXxIjM557t8iOcViaJ5?=
 =?us-ascii?Q?tQLizkqGI++dSeM50k+LxQAKIdRFd72zwqCNY/Dp4/AGSQsYHJXPUrZU3xU0?=
 =?us-ascii?Q?X95CEV9iYWXJXbgFu3yX40TnZF3wwgDCZrlWA0i6MM+sQq5H5TEZIhaaze0A?=
 =?us-ascii?Q?yeMohIoT7KupPBnjTgCh+g0dnlAWvHIki+1ezYIaRADkcBxrlTC5moT5i3GR?=
 =?us-ascii?Q?L1dZvDhqaWnIEjCimFMxXkGZtndxGxaFbvNe0fAvSL9zSYWjPcF0TqN3/gfa?=
 =?us-ascii?Q?L4Uy6dCZ86Da74mHBzy45qC7dcwcRQ7hkjBohZ1BQOXHSNOk0bt56D6x/l5Z?=
 =?us-ascii?Q?/pF72WvNExNS17jNF58kTgwqzGLT0bcbVrAdG8t+pU4V7s4jgYVwOV3kyoKw?=
 =?us-ascii?Q?BwyCqcszeTaELk3WcRvhXZRlqQSxUZMmbgNgAENLf6VsJT5v0YG5Ln/6rtvP?=
 =?us-ascii?Q?UZfVwmb0NywGdenJhsuOMOE2+2HTdj8/AbfEsfxGdkcAf81ixhrWsYH9QUs2?=
 =?us-ascii?Q?nP0E63O9dHtHAzev8whXxOyFQDwHVebEWWnDqLzdqO6wBAA+uW3umZ/hIR33?=
 =?us-ascii?Q?7Pfzt3ZOxK7e1fnqnaFBa8RtEE59Dw0w8+RcrPUWgVeIoDwWFQxQIl0O1jYx?=
 =?us-ascii?Q?Wgz5SFhyEsxuKF59rail6OI4x4T/Sr34l9OG7NlJT9JAjke+y8vn3HyNTFi2?=
 =?us-ascii?Q?j7NOFfFwukw2o01FqFhDLRC3bhryXAz4bnhQ7ZoM3df9NUgs32L+uf7xx9hJ?=
 =?us-ascii?Q?bjcuV2i0LWV2P8JRqfXAV13ZQNu8peQIAZhRXmCRrpyVBU+4xm8FZBF/pfT0?=
 =?us-ascii?Q?dZe2Yn2XxCvv7ySoAcn+GDxASr+/q3fVrtzFWJg+Yk3CWdPhSWmJg3h9c2Dx?=
 =?us-ascii?Q?hdTmLRj3T9zXwwidNm3UCUQZEH7QQb5CCRmzzaxuAE77XSr+X6XPZ80OJPGP?=
 =?us-ascii?Q?L0ZAobWOOhV3F4XwqY4iE5b2rL4Sxi5V1oeiPIWjRgtFB4R8PN68kTrg4elc?=
 =?us-ascii?Q?IcOzYOyoa5owpvV1ODqYo/MGIJaxhbosV8m6t1BJ4f/48Zw9EU+wp0X8Rdr7?=
 =?us-ascii?Q?edNEZ5AHVnmTpzjE7m2K0CTyWkWEwF1C+n7P4NLJjFFms8NlsPbE9dvz2OK7?=
 =?us-ascii?Q?6TmEvIoK0HpeO4l3Sea3e1plu8NEFpGe+xOtdri98e7BzeJlqssywPGe3Wao?=
 =?us-ascii?Q?ImvkbcNLZSy71/rSTJiPUPoTLSJ5VZtJbw/Jt166Fz+iOkhe7kwrIYD3szoe?=
 =?us-ascii?Q?RgVDTWI13LVNNHjVwPhNRDfDFRY/m142WhdyiqDkiZrEC6GWjOsHDlg3nM/E?=
 =?us-ascii?Q?BL1LUKVdUBxY0G0UsI2NVEL0xLfUV4ntNeagsrLD9MhxfWngR2/m22Qe1SH8?=
 =?us-ascii?Q?F4FIn4yEXx9J2amj2HsZpH+3eAUuFRJ9SPOa/Y2+u8nXPqDMiJTucuiP6gEW?=
 =?us-ascii?Q?GIdjIqgqb91971hTF07zDkDGJqH5VO11+BYRrTlDViqv5lvAEfp1mzEpjvTq?=
 =?us-ascii?Q?6y2vr5mciRmyNui1m6GclLr+5ZXcekYx+GPDo6vePa4GtH2egZjEgaGosXay?=
 =?us-ascii?Q?8fwTrza76Mb0qNq1PhCSHaoneRPHLAMHXC68AajL1XaarS+H6E2uN+lI6eYF?=
 =?us-ascii?Q?MNEq5h4aLcVj8gA7gh2yDHw=3D?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: eabaf3ac-0f39-4dca-8150-08d9998df715
X-MS-Exchange-CrossTenant-AuthSource: BN8PR12MB3108.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 27 Oct 2021 21:08:46.6324
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: YcjxwwOOKJwWRshoyN+C12CSagX9Neio+F/AzjF2DhFUTRIuYdNg3SkERuXg0348cXJUe9i/cqleeDfCFblYHA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN8PR12MB2948
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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