lkml.org 
[lkml]   [2017]   [Aug]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 3/3] EDAC, ghes: Make it a proper module
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.
--

\
 
 \ /
  Last update: 2017-08-02 05:20    [W:0.059 / U:13.644 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site