[U-Boot] [PATCH 2/4] powerpc/85xx:Fix MSR[DE] bit in MSR to support debugger

[Wolfgang Denk]

Dear Prabhakar Kushwaha,

In message <4F56DD59.1080304 at freescale.com> you wrote:
> 
> >> --- a/arch/powerpc/cpu/mpc85xx/start.S
> >> +++ b/arch/powerpc/cpu/mpc85xx/start.S
> >> @@ -82,6 +82,11 @@
> >>   	.globl _start_e500
> >>
> >>   _start_e500:
> >> +#if defined(CONFIG_E500_V1_V2)
> >> +/* Enable debug exception */
> >> +	li	r1,MSR_DE
> >> +	mtmsr 	r1
> >> +#endif
> > Why is this #ifdef needed here?
> 
> This #ifdef is to make sure MSR_DE bit is set before u-boot's actual 
> code start executing.
> it is for overcoming one of the requirement of debugging i.e. Keep 
> MSR_DE bit set.

Sorry, this makes no sense to me.  The #ifdef does not change the
order of execution, it just enables or disables the code.  Are there
any situations where omitting this code is required, or where it makes
sense?

> >> @@ -729,8 +734,8 @@ create_init_ram_area:
> >>   	msync
> >>   	tlbwe
> >>
> >> -	lis	r6,MSR_IS|MSR_DS at h
> >> -	ori	r6,r6,MSR_IS|MSR_DS at l
> >> +	lis	r6,MSR_IS|MSR_DS|MSR_DE at h
> >> +	ori	r6,r6,MSR_IS|MSR_DS|MSR_DE at l
> > And why don;t we need such an #ifdef here?
> 
> I found similar piece of code in start.S without any #define at label 
> _start_cont. That's why i made it like this.
> if required i will put it under #define.

Please handle in a consistent way. See above.

> > Also, CONFIG_E500_V1_V2 is undocumented.
> 
> Definition of this #define is part of documentation. I will add comment 
> before using it.

CONFIG_ options must be explained in the README.

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
"I like your game but we have to change the rules."