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:   Mon, 8 Nov 2021 14:34:20 +0100
From:   Borislav Petkov <bp@alien8.de>
To:     "Chatradhi, Naveen Krishna" <nchatrad@amd.com>
Cc:     linux-edac@vger.kernel.org, x86@kernel.org,
        linux-kernel@vger.kernel.org, mingo@redhat.com, mchehab@kernel.org,
        yazen.ghannam@amd.com, Muralidhara M K <muralimk@amd.com>
Subject: Re: [PATCH v6 1/5] x86/amd_nb: Add support for northbridges on
 Aldebaran
Message-ID: <YYknXBpOUQtV1aZ8@zn.tnic>
References: <20211028130106.15701-1-nchatrad@amd.com>
 <20211028130106.15701-2-nchatrad@amd.com>
 <YYF9ei59G/OUyZqR@zn.tnic>
 <b7f3639a-e46c-25e8-270b-04860074fd3c@amd.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <b7f3639a-e46c-25e8-270b-04860074fd3c@amd.com>
Precedence: bulk

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