Re: [PATCH v6 1/5] x86/amd_nb: Add support for northbridges on Aldebaran

[Borislav Petkov <bp@alien8.de>]

On Thu, Nov 04, 2021 at 06:48:29PM +0530, Chatradhi, Naveen Krishna wrote:
> I know, this is confusion. we will try to give a meaning for definition
> here.

Well, not that - you will need to keep adding PCI device IDs for the
future GPUs supporting this stuff.

> How about, defining a new struct
> 
> +struct system_topology {
> +       const struct pci_device_id *misc_ids;
> +       const struct pci_device_id *link_ids;
> +       const struct pci_device_id *root_ids;
> +       u16 roots_per_misc;
> +       u16 misc_count;
> +       u16 root_count;
> +};

Well, how does having a single struct help make things easier?

IOW, if you use accessors to get the information you need, it doesn't
really matter what the underlying organization of the data is. And if
it helps to keep 'em separate because stuff is simple altogether, then,
they should be separate.

So, before you ask "How about", think of answering the question "Why
should it be done this way? What are the advantages?"

> This way, creating appropriate number MCs under EDAC and existing exported
> APIs can remain the same.

Why does that matter?

Also, have you verified in what order the init_amd_nbs() fs initcall and
amd64_edac_init() get executed?

I'm going to venture a pretty sure guess that the initcall runs first
and that amd_cache_northbridges() call in amd64_edac_init() is probably
not even needed anymore...

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette