[U-Boot] ad-hoc config error

[Tom Rini]

On Wed, Sep 21, 2016 at 03:22:59PM +0000, york sun wrote:
> On 09/20/2016 03:30 PM, Tom Rini wrote:
> > On Tue, Sep 20, 2016 at 09:40:00PM +0000, york sun wrote:
> >> On 09/20/2016 02:36 PM, Tom Rini wrote:
> >>> On Tue, Sep 20, 2016 at 09:22:09PM +0000, york sun wrote:
> >>>
> >>>> Tom and Simon,
> >>>>
> >>>> After commit 371244cb19f9804711dd66e4281ff7979915fd2e, all merges with
> >>>> new macros defined will have the compiling error. How shall we fix it?
> >>>> Some macros can be added to Kconfig. But some are for local use, better
> >>>> than magic numbers. Adding them to the white-list doesn't sound right.
> >>>> What's your suggestion?
> >>>
> >>> Things that don't belong in Kconfig don't belong in the CONFIG namespace
> >>> either, probably.  For example, the cache line stuff is in Kconfig and
> >>> select'ed but for another example, various magic numbers used in the TI
> >>> secure boot stuff (which can vary from SoC to SoC) are just not in the
> >>> CONFIG namespace now.
> >>>
> >>
> >> For those can be put in Kconfig, I can convert.
> >> Can you point me examples to use macros for magic numbers?
> >> What about the white listed macros? Are we supposed to leave them there,
> >> or slowly convert them to other name space?
> >
> > Things on the whitelist should be converted, either to Kconfig or moved
> > out of the namespace.  Can you give me an example of something you
> > aren't sure how to convert?
> >
> For example,
> 
> CONFIG_SYS_DDR_MODE_1_1000
> CONFIG_SYS_DDR_MODE_1_1200
> CONFIG_SYS_DDR_MODE_1_1333
> CONFIG_SYS_DDR_MODE_1_667
> CONFIG_SYS_DDR_MODE_1_800
> CONFIG_SYS_DDR_MODE_1_900
> 
> Those are DDR parameters defined per board if used. What will be proper 
> names to convert to?

Poking at this a bit more, looking at say
board/freescale/corenet_ds/p4080ds_ddr.c (which I found grepping for
CONFIG_SYS_DDR_MODE_1_1200) reminds me of
arch/arm/cpu/armv7/am33xx/ddr.c and board/ti/am335x/board.c and no, I'm
not convinced that:
        .timing_cfg_0 = CONFIG_SYS_DDR_TIMING_0_800,
is more clear than:
        .timing_cfg_0 = 0xcc330104,

Especially since there's not a call back to a TRM/whatever that
describes the order of the fields for each register.  To me a link like
that is more descriptive.  I further assume that, after a bit more
grepping, these values, like the counterparts for am33xx are DDR chip
specific so I might go so far as to point at
arch/arm/include/asm/arch-omap3/mem.h where we have a series of
#define MEMORYVENDOR_MEMCTRL_FIELD 0xMAGIC
as if you re-use the part found on the ref board on your custom board
you can use (or at least start with) those values.

And yes, naming of memory controller related magic values is a mess and
is inconsistent even over the TI parts.  My first reaction is that the
am33xx stuff, which came later, worked out "better" than the omap3 way
did.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160921/b11e3f71/attachment.sig>