[U-Boot] [PATCH 1/1] MX5:MX53: support for freescale MX53LOCO board

[Wolfgang Denk]

Dear Jason Liu,

In message <BANLkTinApAmm6hrcSofdYA6r7KUPMr8L+w at mail.gmail.com> you wrote:
> 
> Yes, I have already thought about it before I submit patches. The fact is that
> there is there are two DDR chips connected to CS0 and CS1 of MX53 on loco
> board, but each DDR chip is 512MB, but CS0 and CS1 can support up to 1GB,
> which means the memory space is not continuous, when set:

The mapping is fix, and you cannot remap it as a single, contiguous
region?

> gd->ram_size = get_ram_size((volatile void *)CONFIG_SYS_SDRAM_BASE,
>                           PHYS_SDRAM_1_SIZE + PHYS_SDRAM_2_SIZE);
> it will return the same as:
>
> gd->ram_size = get_ram_size((volatile void *)CONFIG_SYS_SDRAM_BASE,
>                             PHYS_SDRAM_1_SIZE);

You have always to test each bank on it's own, i. e.

	size1 = get_ram_size(..., PHYS_SDRAM_1_SIZE);
	size2 = get_ram_size(..., PHYS_SDRAM_2_SIZE);

	gd->ram_size = get_ram_size = size1 + size2;

> I think the gd->ram_size is just used for uboot, but

Right, but it is supposed to show the full RAM size, and not only half
of it (otherwise it should be gd->half_ram_size or similar ;-)

> If you still think that I really need fix it, I will do it.

THis needs fixing.

> > This has been discussed before: this code should be factored out into
> > common code.
>
> Yes, I think I can send out another clean up patch for it to fix all
> the boards including
> mx31/35/51/53 later.

No. Wrong order.  Please don't add to the mess and clean up later.
Instead, clean up now, and base your patch on th already cleaned up
code.

> > Please remove the '1' from all #defines that just enable features,
> > i.  e. where no specific numeric value is needed.
>
> Yes, I can send out another clean up patch to fix all the i.mx board config>  file

Again: No.  Please clean up NOW.

This patch will not be accepted as is.

> BTW, this patch set has been send out 2 months ago and Stefano has
> send out pull request
> to Albert and Albert has pulled in already, do you agree I send out
> another patch to address
> your comments? Thanks for your consideration.

No, this patch has serious bugs (memory size calculation) and other
issues (needs cleanup) and is thus rejected.

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
"Anyone attempting to generate random numbers by deterministic  means
is, of course, living in a state of sin."          - John Von Neumann