Re: [PATCH] EDAC, {skx|i10nm}_edac: Fix randconfig build error

From: Luck, Tony
Date: Thu Mar 14 2019 - 18:00:13 EST


On Thu, Mar 14, 2019 at 12:04:13PM +0100, Borislav Petkov wrote:
> On Thu, Mar 14, 2019 at 08:09:06AM +0100, Arnd Bergmann wrote:
> > > So where should we go. Proposed solutions are piling up:
> > >
> > > 1) Make skx_common a module
> > > [downside: have to EXPORT everything in it]
> > > 2) Move module-ish bits out of skx_common
> > > [downside: perhaps fragile]
> > > 3) #include skx_common.c into {skx,i10nm}_edac.c
> > > [downside: no patch yet, bigger code size]
> >
> > Sorry to add on to the already long list, but one more idea after
> > looking at the two files:
> >
> > 4) Have a single driver module, with the skx_common.c containing
> > the top-level module_init/module_exit functions that call into the
> > hardware specific versions.
> >
> > This is the opposite of the usual recommendation (the driver
> > is already structured nicely otherwise), but it would be cheap
> > way out here.
>
> This is what I think right now: if this is going to become such a pain,
> then we better keep those two drivers completely separate. Who cares if
> there's some code duplication?! Better that than some obscure randconfig
> build breakages and fixes introducing more ugliness. IOW, we should keep
> it simple.
>
> Unless, Tony, you want to be able to make changes to the common code in
> one place and don't want to patch both. Then I'd librarize the common
> functionality, i.e., do something along the lines of 2).

There's a lot of common code. Patching the same thing in two
different files seems like make-work (and a recipe for things
to be forgotten/missed).

I made a patch based on option #3. Rough steps were:

$ cat skx_common.c >> skx_common.h
$ git rm skx_common.c
$ git mv skx_base.c skx_edac.c
$ git mv i10nm_base.c i10nm_edac.c
$ edit Makefile for changes of names
$ edit skx_common.h to add "static" everywhere
$ edit to fix the build issues

Not entirely happy with some of the bits in that last step.

Patch looks like this: