From mboxrd@z Thu Jan  1 00:00:00 1970
From: Prabhakar Kushwaha <prabhakar@freescale.com>
Date: Wed, 7 Mar 2012 09:43:20 +0530
Subject: [U-Boot] [PATCH 2/4] powerpc/85xx:Fix MSR[DE] bit in MSR to
 support debugger
In-Reply-To: <20120306144050.6CDFF202D7D@gemini.denx.de>
References: <1329296042-28506-1-git-send-email-prabhakar@freescale.com>
	<20120306144050.6CDFF202D7D@gemini.denx.de>
Message-ID: <4F56E060.4040500@freescale.com>
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

Hi Wolfgang,

On Tuesday 06 March 2012 08:10 PM, Wolfgang Denk wrote:
> Dear Prabhakar Kushwaha,
>
> In message<1329296042-28506-1-git-send-email-prabhakar@freescale.com>  you wrote:
>> Debugger's ability to debug an application is constrained by the
>> architecture's debug IP / run-control solution that may impose certain
>> requirements for the application itself.
>> Similarly, when referring to the e500 and e500v2 architecture, there are two
>> basic rules any application has to respect in order to allow full debugging
>> support:
>>          1. Keep MSR[DE] bit set
>>          2. Have a valid opcode that can be fetched from the debug exception
>> 	vector [IVPR|IVOR15].
>> Here MSR = Machine State register
>>
>> This patch makes sure of point "1" and make MSR[DE] bit is set uniformaly
>> across the different execution in address space.
> Does it make sense to repeast the whole README here?

I agree. it should not be.
But as i sent a series of patch, this change require some explanation.

I will change the description.

>> diff --git a/arch/powerpc/cpu/mpc85xx/start.S b/arch/powerpc/cpu/mpc85xx/start.S
>> index 7bfa2d5..09111e6 100644
>> --- 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.


>>   #if defined(CONFIG_SECURE_BOOT)&&  defined(CONFIG_E500MC)
>>   	/* ISBC uses L2 as stack.
>> @@ -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.


> Also, CONFIG_E500_V1_V2 is undocumented.

Definition of this #define is part of documentation. I will add comment 
before using it.

--Prabhakar