Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752088AbdHBDTk (ORCPT ); Tue, 1 Aug 2017 23:19:40 -0400 Received: from mail.skyhub.de ([5.9.137.197]:54856 "EHLO mail.skyhub.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751791AbdHBDTh (ORCPT ); Tue, 1 Aug 2017 23:19:37 -0400 Date: Wed, 2 Aug 2017 05:18:50 +0200 From: Borislav Petkov To: "Kani, Toshimitsu" Cc: "mchehab@infradead.org" , "linux-kernel@vger.kernel.org" , "tony.luck@intel.com" , "linux-edac@vger.kernel.org" , "rjw@rjwysocki.net" Subject: Re: [PATCH 3/3] EDAC, ghes: Make it a proper module Message-ID: <20170802031850.GA4331@nazgul.tnic> References: <20170726084827.11447-1-bp@alien8.de> <20170726084827.11447-4-bp@alien8.de> <1501267290.2042.89.camel@hpe.com> <20170729064715.GB30603@nazgul.tnic> <1501531803.2042.95.camel@hpe.com> <20170801094612.GA18647@nazgul.tnic> <1501632599.2042.104.camel@hpe.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <1501632599.2042.104.camel@hpe.com> User-Agent: Mutt/1.6.0 (2016-04-01) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2071 Lines: 50 On Wed, Aug 02, 2017 at 12:19:29AM +0000, Kani, Toshimitsu wrote: > 1. Device-probing-logic should belong to a driver, and should remain > private to a driver. When we add the white-list, it should be added to > ghes_edac. Nonsense. There are a lot of examples where driver probing depends on outside modalities like built-in quirks and such. > 2. ghes_edac is an extension to the ghes driver as they both are > specific to ghes. ghes_edac is merely ghes driver's edac error- > reporting wrapper than an independent edac driver. It looks OK to let > ghes_edac get registered as part of ghes_probe() and leave it as an > unconventional edac driver. Except that GHES wants to report into the EDAC infrastructure so it better has a wrapper for it. One of the directions I explored when looking at this is to stick ghes_edac functionality into ghes.c or so and make it completely independent from EDAC. Would've been much cleaner. > 3. EDAC does not have its managed probe-chain. All edac drivers are > called from module_init list. They independently probe the hardware > and get unloaded when not needed. The core edac is simply a set of > library to them. I think it's good to keep them independent, and not > to introduce a new central mechanism for a special case like ghes_edac. They're independent because before GHES we needed to load one driver per system. Until the bolted-on thing came. And it is bolted on because the already overwhelmed firmware decided to do error reporting too. So the only real reason why I'm fine with keeping the current situation is the whitelist. Because then, we can at least control what loads and what not. But then we need: 1. A clean mechanism for the platform drivers to query whether another agent is loaded (ghes_edac) and not do any probing then. 2. ghes_edac needs to drop that multiple probing thing as its dmi_walk(ghes_edac_count_dimms, &num_dimm) already probes *all* DIMMs on the system so no need to do that multiple times. -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. --