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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.