[U-Boot-Users] [Patch] CFI flash for ARM development boards

[Wolfgang Denk]

In message <89A528FE6DB0FA44877BB2F05B84671801F8518E at ZIPPY.Emea.Arm.com> you wrote:
> 
> Attached a patch to allow all supported ARM development boards
> (Integrator/CP, VersatilePB, VersatileAB) to use CFI flash.

Ummm... what makes you think that this is  a  complete  list  of  all
"supported ARM development boards"?

> CHANGELOG:
> * Patch by Peter Pearse, 14 Mar 2005
>   Change all supported ARM development boards to use CFI flash.

The comment is misleading (only a few boards are  changed,  not  "all
supported ARM development boards".

Then the patch seems to contain support for a new board which is  not
even mentioned in the CHANGELOG.

It seems you disable use of several "flash.c" files, but I don't  see
that  you  delete these files in your patch. Please cleanup obsoleted
files.


Finally, it seems that your patch requires a function platform_init()
to be provided by the baord dependent code, but you add this only for
a few board, thus breaking a lot of boards which now  fail  to  build
because of "undefined reference to `platform_init'" errors.

It is obvious that you did not run the  MAKEALL  script  as  required
BEFORE submitting a patch.

Also, the code looks suspicious to me:

@@ -110,8 +103,8 @@ unsigned long flash_init (void)
 		nbanks = CFG_MAX_FLASH_BANKS;
 
 	/* Enable flash write */
-	cpcr[1] |= 3;
-
+	cpcr[1] |= cpcr[1] |= CPMASK_FLWREN | CPMASK_FLVPPEN;
+	


I think this is not what you intended to submit?


Please fix the problems and resubmit.


Rejected.



Best regards,

Wolfgang Denk

-- 
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
The idea of male and female are universal constants.
	-- Kirk, "Metamorphosis", stardate 3219.8