[U-Boot-Users] Re: Patch: Support for PQ27e (8247/48/71/72) chips and MPC8272ADS board

Yuli Barcohen
2004-03-15T09:45:53Z

>>>>> Wolfgang Denk writes:

Wolfgang> Dear Yuli, in message
Wolfgang> <16450.14241.889562.784232@gargle.gargle.HOWL> you wrote:

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

Wolfgang> Thanks.

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

Sorry, I promise it was last time I forgot it.

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

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

Wolfgang> Index: cpu/mpc8260/commproc.c
Wolfgang> ========================================
Wolfgang> RCS file:
Wolfgang> /home/CVS/u-boot/u-boot/cpu/mpc8260/commproc.c,v
Wolfgang> retrieving revision 1.1.1.3 diff -p -u -r1.1.1.3
Wolfgang> commproc.c
Wolfgang> --- cpu/mpc8260/commproc.c 6 Aug 2003 17:03:01 -0000
Wolfgang> 1.1.1.3
Wolfgang> +++ cpu/mpc8260/commproc.c 29 Feb 2004 18:45:04 -0000
Wolfgang> @@ -20,13 +20,6 @@
Wolfgang> #include <common.h> include <asm/cpm_8260.h>

Wolfgang> -/*
Wolfgang> - * because we have stack and init data in dual port ram
Wolfgang> - * we must reduce the size
Wolfgang> - */
Wolfgang> -#undef CPM_DATAONLY_SIZE
Wolfgang> -#define CPM_DATAONLY_SIZE ((uint)(8 * 1024) - CPM_DATAONLY_BASE)

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

I checked it on four different boards with different PQ2 chips. In fact,
for older (pre-PQ27e) chips this patch does not change memory map so
they aren't affected in any way. Am I missing anything?

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

Wolfgang> /* Reset CPM

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

There should be some misunderstanding here. PQ27E_DATAONLY_SIZE is NOT
board-specific (I won't add board-specific things into common
files). PQ27e is the common name for the 8247/8248/8271/8272 (the `e'
stands for `encryption'). These chips have got much less internal memory
(DPRAM) and at different addresses so they must have separate
#defines. CPM_DATAONLY_SIZE is just incorrect for PQ27e. PQ27e have got
only 4K of "data only" RAM and not 8K (and at different base, BTW).

Wolfgang> The same happens again in other places, like:

Wolfgang> cpu/mpc8260/ether_fcc.c:

Wolfgang> PQ27E_FCC_SPECIAL_BASE vs. CPM_FCC_SPECIAL_BASE

It's for the same reason. PQ27e have not got RAM at 0xB000, other PQ2s
have not got RAM at 0x9000. There is simply no common base for the two
families so I had to #define new constants. Any suggestions for better
solution are welcome.

Wolfgang> Please clean up and resubmit.

After the explanations, what changes would you suggest?

--
========================================================================
Yuli Barcohen | Phone +972-9-765-1788 | Software Project Leader
yuli at arabellasw.com | Fax +972-9-765-7494 | Arabella Software, Israel
========================================================================