From mboxrd@z Thu Jan  1 00:00:00 1970
From: Wolfgang Denk <wd@denx.de>
Date: Sun, 14 Mar 2004 00:54:40 +0100
Subject: [U-Boot-Users] Patch: Support for PQ27e (8247/48/71/72) chips and MPC8272ADS board 
In-Reply-To: Your message of "Sun, 29 Feb 2004 21:04:01 +0200."
             <16450.14241.889562.784232@gargle.gargle.HOWL> 
Message-ID: <20040313235445.6B232C0655@atlas.denx.de>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Dear Yuli,

in message <16450.14241.889562.784232@gargle.gargle.HOWL> you wrote:
> 
> This patch adds support for the newest additions to Motorola PQII family
> (MPC8247/8248/8271/8272) known also as PQ27e. They've got different
> DPRAM layout (as was discussed on the list) so common files were
> changed. I tested the code on several boards I've got (using both old
> and new chips). Motorola released new derivative of MPC8260ADS
> evaluation board supporting these chips: MPC8272ADS. This patch adds
> support for the new board.

Thanks.

Please don't forget the entry for the CHANGELOG file.

Please add the appropriate additions to the CREDITS  and  MAINTAINERS
files, if needed.

I don't like the following parts of your patch:


	Index: cpu/mpc8260/commproc.c
	===================================================================
	RCS file: /home/CVS/u-boot/u-boot/cpu/mpc8260/commproc.c,v
	retrieving revision 1.1.1.3
	diff -p -u -r1.1.1.3 commproc.c
	--- cpu/mpc8260/commproc.c	6 Aug 2003 17:03:01 -0000	1.1.1.3
	+++ cpu/mpc8260/commproc.c	29 Feb 2004 18:45:04 -0000
	@@ -20,13 +20,6 @@
	 #include <common.h>
	 #include <asm/cpm_8260.h>
	 
	-/*
	- * because we have stack and init data in dual port ram
	- * we must reduce the size
	- */
	-#undef	CPM_DATAONLY_SIZE
	-#define CPM_DATAONLY_SIZE	((uint)(8 * 1024) - CPM_DATAONLY_BASE)

Are you 100% sure this does not break any existing boards?

	 void
	 m8260_cpm_reset(void)
	 {
	@@ -38,7 +31,10 @@ m8260_cpm_reset(void)
		/* Reclaim the DP memory for our use.
		*/
		gd->dp_alloc_base = CPM_DATAONLY_BASE;
	-	gd->dp_alloc_top = gd->dp_alloc_base + CPM_DATAONLY_SIZE;
	+	if (is_pq27e())
	+	   gd->dp_alloc_top = gd->dp_alloc_base + PQ27E_DATAONLY_SIZE;
	+	else
	+	   gd->dp_alloc_top = gd->dp_alloc_base + CPM_DATAONLY_SIZE;
	 
		/*
		 * Reset CPM

Please do not add a board-specific #define  PQ27E_DATAONLY_SIZE  when
we already have a CPM_DATAONLY_SIZE which could be used.


The same happens again in other places, like:

cpu/mpc8260/ether_fcc.c:

	PQ27E_FCC_SPECIAL_BASE vs. CPM_FCC_SPECIAL_BASE



Please clean up and resubmit.

Best regards,

Wolfgang Denk

-- 
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-4596-87  Fax: (+49)-8142-4596-88  Email: wd at denx.de
As long as we're going to reinvent the wheel again, we might as  well
try making it round this time.                        - Mike Dennison