Messages in this thread | | | Date | Thu, 19 May 2022 14:42:55 +0000 | From | Yazen Ghannam <> | Subject | Re: [PATCH 06/18] EDAC/amd64: Add prep_chip_selects() into pvt->ops |
| |
On Wed, May 18, 2022 at 10:10:05AM +0200, Borislav Petkov wrote: > On Mon, May 09, 2022 at 02:55:22PM +0000, Yazen Ghannam wrote: > > From: Muralidhara M K <muralidhara.mk@amd.com> > > > > GPU Nodes will need to set the number of available Chip Selects, i.e. > > Base and Mask values, differently than existing systems. A function > > pointer should be used rather than introduce another branching condition. > > Yeah, it looks to me like all that detection logic should be split > eventually. Looking at read_mc_regs(), it has > > if (pvt->umc) { > __read_mc_regs_df(pvt); > > goto skip; > } > > at the top, then a whole bunch of legacy stuff and then at the skip > label some common stuff... > > Another thing you could consider is to have common functionality carved > out into helpers with "common" in the name and then call those from both > UMC and DCT paths. Perhaps that'll help keep the init paths sane. That > is, short of splitting this driver. > > We did the splitting for Intel and there we have a common, librarized > code which gets linked into a couple of drivers. You don't have to do > this too - just putting it out there as an alternative. > > The per-family function pointers design could be good too, if done > right. The advantage being, you have a single driver for all, yadda > yadda... >
Yep, I've actually considered splitting this driver. But I think at this point it'd be best to keep what we have, and then write a new driver if and when a major change happens in future platforms.
> > Prepare for this by adding prep_chip_selects() to pvt->ops and set it > > as needed based on currently supported systems. > > > > Use a "umc" prefix for modern systems, since these use Unified Memory > > Controllers (UMCs). > > > > Use a "dct" prefix for newly-defined legacy functions, since these > > systems use DRAM Controllers (DCTs). > > > > Signed-off-by: Muralidhara M K <muralidhara.mk@amd.com> > > Signed-off-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@amd.com> > > What does Naveen's SOB mean here? Co-developed-by perhaps? >
Yes, that's right. Sorry I missed it. I'll include it in the next revision.
Thanks, Yazen
| |