[U-Boot] [PATCH] Rename CONFIG_SYS_MDDRC memory constants for all mpc512x boards

[Wolfgang Denk]

Dear "Martha J Marx",

In message <200909101515289.SM07320 at cw4mb41> you wrote:
> 
> I know this was a while ago ... but when I first posted the Elpida patch you
> asked me to FIX the my memory constant changes.  At that time you said the
> following
> 
> >
> > While performing such a global rename, please let's get rid of the
> > CONFIG_SYS_ names. These are NOT options that can be changed by the
> > user in the board config file, so they should not look like such.
> >
> 
> This is when I tried to change some constants because they had "MICRON" in
> there.  I decided to take out MICRON since they are being shared by both
> ELPIDA and MICRON.

I remember. I was wrong. By then, the MPC5121ADS was the only MPC512x
board, and the changes were to board/ads5121/ads5121.c (and
include/configs/ads5121.h). We (well, especially I) assumed that this
was strictly board-specific code.

But since then, a number of other MPC512x boards have been added, and
in this process it turned out that this is actually common code,
which can be used unchanged by all boards. So it was moved from
"private" to "public". Also it became clear that the values to insert
here are no magic constants, but board-specific configuration options,
and for these the name should be CONFIG_SYS_*.

> How bout I just make a whole new set of constants for ELPIDA ??? and Leave
> the "MICRON" ones the way they stand ?

I think all such renaming doesn't bring us forward.

Looking at the code it seems we have just a list of constants that
need to get written all to the same mddrc.ddr_command register. To
make this configurable, we should probably not try to add more such
stuff, but separate code and data to simplify the design.

I think we should turn the existing code:

	/* Initialize DDR */
	for (i = 0; i < 10; i++)
		out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_NOP);

	out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_PCHG_ALL);
	out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_NOP);
	out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_RFSH);
	out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_NOP);
	out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_RFSH);
	out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_NOP);
	out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_INIT_DEV_OP);
	out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_NOP);
	out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_EM2);
	out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_NOP);
	out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_PCHG_ALL);
	out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_EM2);
	out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_EM3);
	out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_EN_DLL);
	out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_INIT_DEV_OP);
	out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_PCHG_ALL);
	out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_RFSH);
	out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_INIT_DEV_OP);
	out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_OCD_DEFAULT);
	out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_PCHG_ALL);
	out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_NOP);

into something like this:

u32 micron_table[] = {
	CONFIG_SYS_MICRON_NOP,
	CONFIG_SYS_MICRON_NOP,
	CONFIG_SYS_MICRON_NOP,
	CONFIG_SYS_MICRON_NOP,
	CONFIG_SYS_MICRON_NOP,
	CONFIG_SYS_MICRON_NOP,
	CONFIG_SYS_MICRON_NOP,
	CONFIG_SYS_MICRON_NOP,
	CONFIG_SYS_MICRON_NOP,
	CONFIG_SYS_MICRON_NOP,

	CONFIG_SYS_MICRON_PCHG_ALL,
	CONFIG_SYS_MICRON_NOP,
	CONFIG_SYS_MICRON_RFSH,
	CONFIG_SYS_MICRON_NOP,
	CONFIG_SYS_MICRON_RFSH,
	CONFIG_SYS_MICRON_NOP,
	CONFIG_SYS_MICRON_INIT_DEV_OP,
	CONFIG_SYS_MICRON_NOP,
	CONFIG_SYS_MICRON_EM2,
	CONFIG_SYS_MICRON_NOP,
	CONFIG_SYS_MICRON_PCHG_ALL,
	CONFIG_SYS_MICRON_EM2,
	CONFIG_SYS_MICRON_EM3,
	CONFIG_SYS_MICRON_EN_DLL,
	CONFIG_SYS_MICRON_INIT_DEV_OP,
	CONFIG_SYS_MICRON_PCHG_ALL,
	CONFIG_SYS_MICRON_RFSH,
	CONFIG_SYS_MICRON_INIT_DEV_OP,
	CONFIG_SYS_MICRON_OCD_DEFAULT,
	CONFIG_SYS_MICRON_PCHG_ALL,
	CONFIG_SYS_MICRON_NOP,
};
...

long int fixed_sdram(u32 ram_table[], int table_size)
{
	int i;
	...
	for (i=0; i<table_size; ++i)
		out_be32(&im->mddrc.ddr_sys_config, ram_table[i])
	...
}

The board specific code would then be changed from

	fixed_sdram()

into for example

	fixed_sdram(micron_table,sizeof(micron_table)/sizeof(u32))

In board/freescale/mpc5121ads/mpc5121ads.c, you could provide an
additional set of data (elpida_table[]), and then pass either of these
to fixed_sdram().

What do you think?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
The optimum committee has no members.
                                                   - Norman Augustine