From mboxrd@z Thu Jan 1 00:00:00 1970 From: Scott Wood Date: Wed, 25 Apr 2012 09:50:30 -0500 Subject: [U-Boot] [PATCH 3/4][v3] powerpc/85xx:Make debug exception vector accessible In-Reply-To: References: <1332752441-28334-1-git-send-email-prabhakar@freescale.com> <4F9716D2.1030109@freescale.com> <4F977414.6010003@freescale.com> Message-ID: <4F980F36.1030308@freescale.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 04/24/2012 11:05 PM, Andy Fleming wrote: > On Tue, Apr 24, 2012 at 10:48 PM, Prabhakar Kushwaha > wrote: >> Hi, >> >> >> >> On Wednesday 25 April 2012 02:50 AM, Andy Fleming wrote: >>> >>> On Tue, Apr 24, 2012 at 4:10 PM, Scott Wood >>> wrote: >>>> >>>> On 04/24/2012 03:45 PM, Andy Fleming wrote: >>>>> >>>>> On Mon, Mar 26, 2012 at 4:00 AM, Prabhakar Kushwaha >>>>>> >>>>>> @@ -107,6 +107,7 @@ >>>>>> #define CONFIG_MAX_CPUS 1 >>>>>> #define CONFIG_FSL_SDHC_V2_3 >>>>>> #define CONFIG_SYS_FSL_NUM_LAWS 12 >>>>>> +#define CONFIG_SYS_PPC_E500_DEBUG_TLB 3 >>>>> >>>>> >>>>> You've only enabled this for one processor. Maybe the P1010. Nowhere >>>>> in the patch description does it mention that this is only being >>>>> enabled for one chip. As it is, I think you meant to enable it for >>>>> more chips, and I'm wondering why it's not enabled for the other chips >>>>> mentioned in the comments above as being test platforms for this >>>>> patch... >>>>> >>>>> Probably, for this feature, we should make a default definition, which >>>>> can be overridden by other parts if necessary. >>>>> >>>>> In other words, add something like this near the end: >>>>> >>>>> #ifndef CONFIG_SYS_PPC_E500_DEBUG_TLB >>>>> #define CONFIG_SYS_PPC_E500_DEBUG_TLB 3 >>>>> #endif >>>> >>>> We don't want to enable this on e500mc (testing in that context just >>>> means that we didn't break the code path where this isn't enabled). I'm >>>> not sure whether it will break anything if we do enable it on e500mc, >>>> but we don't need it. >>> >>> Ok, but we still either need this to be enabled for all e500/e500v2. >>> OR split off this config option into a separate patch where it's >>> mentioned that this is only being enabled on the P1010. >>> >>> Actually, it should probably be added to this area that deals in >>> core-specific config options: >> >> >> I agree. I will split the debugger patch and P1010 debug enable. >> A separate patch will be send for P1010 SoC. >> For other e500v2 processor based SoC,as i am not sure about their >> verification. It will be send as and when verified. > > That should be fine. Enabling it on individual SoCs based on verification conflicts with putting this in the core-specific config area. Just test with a reasonable sample of SoCs, grep for TLB assignments to look for any conflicts, and then enable it in the generic e500v2 area that Andy pointed out. -Scott