Subject: Re: PPC4xx ECC Configs, Defines and Source

From: Benjamin Herrenschmidt <[email-masked]>
Date: Wed, 10 Dec 2008 19:53:36 +1100

> >I thought I might leverage the definitions Stefan Roese came up with for
> >u-boot for the base memory controller:
> >
> >    CONFIG_PPC4xx_IBM_SDRAM:   Applicable to 405GP, 405CR, 405EP, AP1000,
> >                               and ML2
> >    CONFIG_PPC4xx_IBM_DDR:     Applicable to 440GP, 440GX, 440EP, and 440GR
> >    CONFIG_PPC4xx_DENALI_DDR2: Applicable to 440EPX and 440GRX
> >    CONFIG_PPC4xx_IBM_DDR2:    Applicable to 405EX[r], 440SP, 440SPe, 460EX
> >                               and 460GT.
> 
> Config options for what?  Enabling certain function?  Not sure that's needed
> if we can get away with it by just binding to the appropriate controller.

You still want to have ways to enable compilation of only selected ones,
so that if I build a kernel for only a single board, I don't carry the
code for everybody else...

The way I see things is that there would be one master config option
that is user visible "Support for ECC monitoring on 4xx SoCs".

The various SoCs themselves are already select'ed by the various
platforms you enabled, so the SoC Kconfig entries could silently then
select which type of memory controllers are enabled

The combination of those types and the master option defines which
sub-drivers are actually built.

> >Controlling whether ECC monitoring and reporting support should be compiled
> >in or a module:
> >
> >    CONFIG_PPC_ECC or CONFIG_ECC
> 
> That's a bit too generic, unless you are trying to make a menu list under
> that which would be used to allow people to enable things like:
> CONFIG_4XX_ECC, CONFIG_FSL_ECC, etc.

Agreed.

> Why is there a need to have so many files?  I would think you could
> have a single file with all the ECC monitoring implementations in it
> called ppc4xx_ecc.c (or such).  Surely they would share some amount
> of code?

Well, it depends how much they share, but I'd rather have separate files
with helpers in ppc4xx_soc.c if it's small, that way, it's easier to
only build selected files based on what SoC support is enabled.

Cheers,
Ben.