From: Prabhakar Kushwaha <prabhakar@freescale.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 2/4] powerpc/85xx:Fix MSR[DE] bit in MSR to support debugger
Date: Tue, 13 Mar 2012 10:54:51 +0530 [thread overview]
Message-ID: <4F5EDA23.6000109@freescale.com> (raw)
In-Reply-To: <20120307120336.A9C93202C7F@gemini.denx.de>
Hi Wolfgang,
Sorry for delayed response.
On Wednesday 07 March 2012 05:33 PM, Wolfgang Denk wrote:
> Dear Prabhakar Kushwaha,
>
> In message<4F56DD59.1080304@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?
OK.
after having internal discussion about different processor support. we
can make this code permanently enabled.
>>>> @@ -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.
Sure,
I believe we should not have any #define for this piece of code to make
it consistent across the file.
>>> 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.
I will take care of this
--Prabhakar
next prev parent reply other threads:[~2012-03-13 5:24 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-02-15 8:54 [U-Boot] [PATCH 2/4] powerpc/85xx:Fix MSR[DE] bit in MSR to support debugger Prabhakar Kushwaha
2012-03-06 14:40 ` Wolfgang Denk
2012-03-07 4:00 ` Prabhakar Kushwaha
2012-03-07 12:03 ` Wolfgang Denk
2012-03-13 5:24 ` Prabhakar Kushwaha [this message]
2012-03-07 4:13 ` Prabhakar Kushwaha
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4F5EDA23.6000109@freescale.com \
--to=prabhakar@freescale.com \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox