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

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

[Yuli Barcohen]

Hi,

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.

-- 
========================================================================
 Yuli Barcohen       | Phone +972-9-765-1788 |  Software Project Leader
 yuli at arabellasw.com | Fax   +972-9-765-7494 | Arabella Software, Israel
========================================================================
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 8272.patch.bz2
Type: application/octet-stream
Size: 6202 bytes
Desc: PQ27e support
Url : http://lists.denx.de/pipermail/u-boot/attachments/20040229/f2f77a17/attachment.obj 

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

[Wolfgang Denk]

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

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

[Yuli Barcohen]

>>>>> 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
========================================================================

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

[Wolfgang Denk]

In message <16469.31569.535909.246506@gargle.gargle.HOWL> you wrote:
>
>     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?

I don't know. I just want to understand the consequences.

>     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;

First, ther is at least one redundand "gd->CPM_DATAONLY_SIZE;" here.

>     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'

Sorry for chosing a poor description.

> 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).

Why do you need a separate  #define  then?  Isn't  it  sufficient  to
#define a correct value for CPM_DATAONLY_SIZE then?

>     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

So why not just #define a correct value?

> After the explanations, what changes would you suggest?

Use the existing variables and just assign correct values?

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
When the bosses talk about improving  productivity,  they  are  never
talking about themselves.

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

[Yuli Barcohen]

>>>>> Wolfgang Denk writes:

    Wolfgang> In message <16469.31569.535909.246506@gargle.gargle.HOWL>
    Wolfgang> you wrote:

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

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

    Wolfgang> I don't know. I just want to understand the consequences.

    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> First, ther is at least one redundand
    Wolfgang> "gd->CPM_DATAONLY_SIZE;" here.

No, no, it's just E-mail formatting of the excerpt after several
replies. There is no redundant code in the patch.

    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.

    Yuli> There should be some misunderstanding
    Yuli> here. PQ27E_DATAONLY_SIZE is NOT board-specific (I won't add
    Yuli> board-specific things into common files). PQ27e is the common
    Yuli> name for the 8247/8248/8271/8272 (the `e'

    Wolfgang> Sorry for chosing a poor description.

    Yuli> stands for `encryption'). These chips have got much less
    Yuli> internal memory (DPRAM) and at different addresses so they
    Yuli> must have separate
    Yuli> #defines. CPM_DATAONLY_SIZE is just incorrect for PQ27e. PQ27e
    Yuli> have got only 4K of "data only" RAM and not 8K (and at
    Yuli> different base, BTW).

    Wolfgang> Why do you need a separate #define then?  Isn't it
    Wolfgang> sufficient to
    Wolfgang> #define a correct value for CPM_DATAONLY_SIZE then?

This would require that, for every PQ27e-based board, it would be
defined in the board configuration file that it's PQ27e and not just
8260. Then CPM_DATAONLY_SIZE can be defined conditionally on this
definition in cpm_8260.h. I personally don't like such compile-time
definitions because chip version can be easily detected in run-time.

    Wolfgang> PQ27E_FCC_SPECIAL_BASE vs. CPM_FCC_SPECIAL_BASE

    Yuli> It's for the same reason. PQ27e have not got RAM at 0xB000,
    Yuli> other PQ2s

    Wolfgang> So why not just #define a correct value?

    Yuli> After the explanations, what changes would you suggest?

    Wolfgang> Use the existing variables and just assign correct values?

Well, the question is if it's better that the CPU-specific code will
silently handle the differences or we'll make it the user's
responsibility to define the difference manually. I think automatic
handling is better but if you insist on #ifdefs, I can implement it too.

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

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

[Wolfgang Denk]

In message <16469.44734.573362.958637@gargle.gargle.HOWL> you wrote:
> 
> No, no, it's just E-mail formatting of the excerpt after several
> replies. There is no redundant code in the patch.

You are right. Sorry for the confusion.

> This would require that, for every PQ27e-based board, it would be
> defined in the board configuration file that it's PQ27e and not just
> 8260. Then CPM_DATAONLY_SIZE can be defined conditionally on this
> definition in cpm_8260.h. I personally don't like such compile-time
> definitions because chip version can be easily detected in run-time.

The advantage of compile-time definitions  is  that  they  result  in
smaller code.

>     Wolfgang> PQ27E_FCC_SPECIAL_BASE vs. CPM_FCC_SPECIAL_BASE
...
> Well, the question is if it's better that the CPU-specific code will
> silently handle the differences or we'll make it the user's
> responsibility to define the difference manually. I think automatic
> handling is better but if you insist on #ifdefs, I can implement it too.

Sorry, but I can't follow.

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

[Kumar Gala]

Looking at the patch, I was wondering if there was any reason to not  
enable both RS232 ports by default.

- kumar

On Feb 29, 2004, at 1:04 PM, Yuli Barcohen wrote:

> Hi,
>
> 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.
>
> --  
> ======================================================================= 
> =
>  Yuli Barcohen       | Phone +972-9-765-1788 |  Software Project Leader
>  yuli at arabellasw.com | Fax   +972-9-765-7494 | Arabella Software,  
> Israel
> ======================================================================= 
> =
>
> <8272.patch.bz2>

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

[Yuli Barcohen]

>>>>> Kumar Gala writes:

    Kumar> Looking at the patch, I was wondering if there was any reason
    Kumar> to not enable both RS232 ports by default.

Only the console port was enabled before the patch. Is there any reason
for the patch to change this behaviour? Also, I see no reason to enable
any peripheral which isn't going to be used.

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

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

[Kumar Gala]

Yuli,

I agree with only enabling ports that will be used.  I guess I would  
expect the port to be used in Linux.  I'm working on an updated 2.6 SCC  
uart driver and realized that I needed to enable the 2nd port in BCSR.   
I had expected that u-boot would have already handled it.

- kumar

On Mar 16, 2004, at 1:23 AM, Yuli Barcohen wrote:

>>>>>> Kumar Gala writes:
>
>     Kumar> Looking at the patch, I was wondering if there was any  
> reason
>     Kumar> to not enable both RS232 ports by default.
>
> Only the console port was enabled before the patch. Is there any reason
> for the patch to change this behaviour? Also, I see no reason to enable
> any peripheral which isn't going to be used.
>
> --  
> ======================================================================= 
> =
>  Yuli Barcohen       | Phone +972-9-765-1788 |  Software Project Leader
>  yuli at arabellasw.com | Fax   +972-9-765-7494 | Arabella Software,  
> Israel
> ======================================================================= 
> =

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

[Yuli Barcohen]

>>>>> Kumar Gala writes:

    Kumar> Yuli, I agree with only enabling ports that will be used.  I
    Kumar> guess I would expect the port to be used in Linux.

If it's to be used in Linux but not in the U-Boot, let Linux initialise
it.

    Kumar> I'm working on an updated 2.6 SCC uart driver and realized
    Kumar> that I needed to enable the 2nd port in BCSR.  I had expected
    Kumar> that u-boot would have already handled it.

I see. In Arabella Linux, there is clear separation between boot and
kernel and between chip-specific and board-specific code. ADS-specific
file in Linux kernel is the place where the BCSR handling lives, so
we've got generic SCC UART driver and perform all the initialisations
(including board-specific BCSR) only when some code requests them.

    Kumar> Looking at the patch, I was wondering if there was any reason
    Kumar> to not enable both RS232 ports by default.

    Yuli> Only the console port was enabled before the patch. Is there
    Yuli> any reason for the patch to change this behaviour? Also, I see
    Yuli> no reason to enable any peripheral which isn't going to be
    Yuli> used.

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

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

[Yuli Barcohen]

>>>>> Wolfgang Denk writes:

    Yuli> This would require that, for every PQ27e-based board, it would
    Yuli> be defined in the board configuration file that it's PQ27e and
    Yuli> not just
    Yuli> 8260. Then CPM_DATAONLY_SIZE can be defined conditionally on
    Yuli> this definition in cpm_8260.h. I personally don't like such
    Yuli> compile-time definitions because chip version can be easily
    Yuli> detected in run-time.

    Wolfgang> The advantage of compile-time definitions is that they
    Wolfgang> result in smaller code.

Of course, but here we're speaking about couple of `if's.

    Wolfgang> PQ27E_FCC_SPECIAL_BASE vs. CPM_FCC_SPECIAL_BASE ...

    Yuli> Well, the question is if it's better that the CPU-specific
    Yuli> code will silently handle the differences or we'll make it the
    Yuli> user's responsibility to define the difference manually. I
    Yuli> think automatic handling is better but if you insist on
    Yuli> #ifdefs, I can implement it too.

    Wolfgang> Sorry, but I can't follow.

    Wolfgang> From what I have seen your patches used _two_ #defines
    Wolfgang> instead of one.

Well, my explanations can be confusing... I'll try to explain it
differently. You're right, there are two #defines in one common file and
it's all what needed for PQ27e support. If you prefer the compile-time
solution, this means that: new CPU type is to be defined (CONFIG_MPC8272
or something like this) in addition to CONFIG_MPC8260, this type must be
used for every PQ27e based board instead of CONFIG_MPC8260, and in a
common file it must be defined that CONFIG_MPC8272 is a flavour of
CONFIG_MPC8260 otherwise the common MPC8260 code won't work for
PQ27e. IMHO this means much more work and more possibilities to make a
mistake, especially when porting U-Boot to a new board, but you're the
maintainer so make your choice.

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

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

[Dan Malek]

Sorry to jump into this late, but since I do tons of PQ work
I want to add my comments.

> .... I personally don't like such
>     Yuli> compile-time definitions because chip version can be easily
>     Yuli> detected in run-time.

No, it can't in all cases.  You can determine the core type and can
sometimes find something about the CPM rom version, but tracking down
and maintaining the permutations isn't trivial.  You have great inside
information on this stuff, but it doesn't help the rest of us add or
maintain this software.

>     Wolfgang> The advantage of compile-time definitions is that they
>     Wolfgang> result in smaller code.

I agree.  We've been doing it this way in Linux for years, the compiler
makes it pretty clear when you don't have something #defined.  What's
run-time code going to do when it can't determine the core or CPM type?
Assume some defaults and hope a message gets printed?

Thanks.


	-- Dan

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

[Kumar Gala]

On Mar 17, 2004, at 4:54 AM, Yuli Barcohen wrote:

>>>>>> Kumar Gala writes:
>
>     Kumar> Yuli, I agree with only enabling ports that will be used.  I
>     Kumar> guess I would expect the port to be used in Linux.
>
> If it's to be used in Linux but not in the U-Boot, let Linux initialise
> it.
>
>     Kumar> I'm working on an updated 2.6 SCC uart driver and realized
>     Kumar> that I needed to enable the 2nd port in BCSR.  I had 
> expected
>     Kumar> that u-boot would have already handled it.
>
> I see. In Arabella Linux, there is clear separation between boot and
> kernel and between chip-specific and board-specific code. ADS-specific
> file in Linux kernel is the place where the BCSR handling lives, so
> we've got generic SCC UART driver and perform all the initialisations
> (including board-specific BCSR) only when some code requests them.
>

Your correct, I will have the platform code for the board deal with 
initializing the BCSR in Linux.

Thanks

- kumar

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

[Yuli Barcohen]

>>>>> Dan Malek writes:

    Dan> Sorry to jump into this late, but since I do tons of PQ work I
    Dan> want to add my comments.

    Yuli> .... I personally don't like such
    Yuli> compile-time definitions because chip version can be easily
    Yuli> detected in run-time.

    Dan> No, it can't in all cases.  You can determine the core type and
    Dan> can sometimes find something about the CPM rom version, but
    Dan> tracking down and maintaining the permutations isn't trivial.

Well, you can't know if it's 8271 or 8272, for example, but you don't
need this info. You can detect the "family" (HiP4, HiP7, etc.) and this
is enough for the tasks which we are discussing.

    Dan> You have great inside information on this stuff,

No, I don't (and I won't disclose such information in GPL code if I
had).

    Dan> but it doesn't help the rest of us add or maintain this
    Dan> software.

    Wolfgang> The advantage of compile-time definitions is that they
    Wolfgang> result in smaller code.

    Dan> I agree.  We've been doing it this way in Linux for years, the
    Dan> compiler makes it pretty clear when you don't have something
    Dan> #defined.

And these #defines were one of the reasons that we developed completely
new Linux support for PQs. Now we've got one kernel image running on
MPC8260ADS, MPC8266ADS, PQ2FADS, and MPC8272ADS. You would have four
different images. Probably that images would be smaller by 2K, maybe 5K,
but if the price is maintaining four images instead of one, I vote for
one. The same is true for other board families (FADS, etc.) I agree that
this can be less important for boot loaders which in many cases contain
too much board-specific code to build one-for-all image.

    Dan> What's run-time code going to do when it can't determine the
    Dan> core or CPM type?  Assume some defaults and hope a message gets
    Dan> printed?

Maybe such a thing can happen but in many years of development for
Motorola controllers I personally never had a problem with run-time
detection of chip features.

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

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

[Kumar Gala]

Yuli,

Have you tried this patch with CONFIG_CONS_INDEX = 2?  Just wondering 
because it seems like the IO ports for SCC4 uart are not setup.

- kumar

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

[Yuli Barcohen]

>>>>> Kumar Gala writes:

    Kumar> Yuli, Have you tried this patch with CONFIG_CONS_INDEX = 2?
    Kumar> Just wondering because it seems like the IO ports for SCC4
    Kumar> uart are not setup.

CONFIG_CONS_INDEX = 2 means SCC2, not SCC4. You have to define
CONFIG_CONS_INDEX = 4 to use the lower RS-232 on MPC8272ADS. Ports
initialisation is static (I have to make it conditional like I did for
FCCs) so the ports table reflects current state (i.e. console on
SCC1). I haven't tried the SCC4 in U-Boot, but it works in Linux.

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

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

[Wolfgang Denk]

In message <16472.27359.566539.641092@gargle.gargle.HOWL> Yuli Barcohen wrote:
> 
>     Dan> I agree.  We've been doing it this way in Linux for years, the
>     Dan> compiler makes it pretty clear when you don't have something
>     Dan> #defined.
> 
> And these #defines were one of the reasons that we developed completely
> new Linux support for PQs. Now we've got one kernel image running on
> MPC8260ADS, MPC8266ADS, PQ2FADS, and MPC8272ADS. You would have four
> different images. Probably that images would be smaller by 2K, maybe 5K,
> but if the price is maintaining four images instead of one, I vote for
> one. The same is true for other board families (FADS, etc.) I agree that
> this can be less important for boot loaders which in many cases contain
> too much board-specific code to build one-for-all image.


Let's stop this discussion here. We don't need another religious war.

As Dan said, Linux has been using the #define style for a long  time,
and  for good reasons, and U-Boot always followed this style - and if
it was only because of the smaller code size.

There are many more  situations  where  run-time  code  may  be  very
convenient from the software ddeveloper's point of view, but where it
is not practical for the intended purpose. Think of memory footprint,
boot time, time of error detection etc.

> Maybe such a thing can happen but in many years of development for
> Motorola controllers I personally never had a problem with run-time
> detection of chip features.

Than you have been just lucky, or not the end user of the solution.



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
The universe does not have laws - it has habits, and  habits  can  be
broken.

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

[Yuli Barcohen]

>>>>> Wolfgang Denk writes:

    Wolfgang> Let's stop this discussion here. We don't need another
    Wolfgang> religious war.

I hope you believe that religious war is something I want to avoid, not
to start. As I wrote in the beginning of this discussion, if the
maintainer (you) insists on #ifdefs, I'll send a patch using #ifdefs.

    Wolfgang> The universe does not have laws - it has habits, and
    Wolfgang> habits can be broken.

Even more so with respect to U-Boot and Linux:)

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

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

[Wolfgang Denk]

In message <761C62B8-7757-11D8-A4C6-000393DBC2E8@motorola.com> you wrote:
> 
> I agree with only enabling ports that will be used.  I guess I would  
> expect the port to be used in Linux.  I'm working on an updated 2.6 SCC  
> uart driver and realized that I needed to enable the 2nd port in BCSR.   
> I had expected that u-boot would have already handled it.

The philosophy is to enable  and/or  initialize  only  those  devices
which are actually used by U-Boot itself. Even the network interfaces
get  initialized  only at the moment when you issue the first command
to access the network.

On the other hand a Linux device driver should assume  it  finds  the
device  in  an  unknown  random  (*)  state,  it  should  perform all
initialization it needs itself, and it should deinitialize the device
(if the driver should get unloaded) into an idle state.

(*) "random" is not completely random, of course - it  still  assumes
that  the device is well-behaving, i. e. that interrupts are disabled
etc.

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
"When anyone says `theoretically,' they really mean `not really.'"
- David Parnas