[U-Boot] [PATCH 3/4][v3] powerpc/85xx:Make debug exception vector accessible

[U-Boot] [PATCH 3/4][v3] powerpc/85xx:Make debug exception vector accessible

[Prabhakar Kushwaha]

Debugging of e500 and e500v1 processer requires debug exception vecter (IVPR +
IVOR15) to have valid and fetchable OP code.

While executing in translated space (AS=1), whenever a debug exception is
generated, the MSR[DS/IS] gets cleared i.e. AS=0 and the processor tries to
fetch an instruction from the debug exception vector (IVPR + IVOR15); since now
we are in AS=0, the application needs to ensure the proper TLB configuration to
have (IVOR + IVOR15) accessible from AS=0 also.

Create a temporary TLB in AS0 to make sure debug exception verctor is
accessible on debug exception.

Signed-off-by: Radu Lazarescu <radu.lazarescu@freescale.com>
Signed-off-by: Marius Grigoras <marius.grigoras@freescale.com>
Signed-off-by: Prabhakar Kushwaha <prabhakar@freescale.com>
---
  Based upon git://git.denx.de/u-boot.git branch master

 Changes for v2: 
	- Put Temporary TLB creation under #define

 Changes for v3: 
	- Removed unnecessary CONFIG_E500
	- Updated CONFG_SYS_RAMBOOT mas2 properties
	- Added secure boot Debug TLB.
	- Avoid temp TLB creation for NAND SPL
	- removed unnecessary mas7 updation

  Tested on
  - SoC having E500 Family processor (P1010RDB, BSC9131RDB)
  - SoC having E500MC Family processor (P4080DS, P3041DS)

 arch/powerpc/cpu/mpc85xx/cpu_init_early.c |   32 +++++++++++++-
 arch/powerpc/cpu/mpc85xx/start.S          |   66 +++++++++++++++++++++++++++++
 arch/powerpc/include/asm/config_mpc85xx.h |    3 +-
 3 files changed, 99 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/cpu/mpc85xx/cpu_init_early.c b/arch/powerpc/cpu/mpc85xx/cpu_init_early.c
index 091af7c..dacfdd1 100644
--- a/arch/powerpc/cpu/mpc85xx/cpu_init_early.c
+++ b/arch/powerpc/cpu/mpc85xx/cpu_init_early.c
@@ -1,5 +1,5 @@
 /*
- * Copyright 2009-2011 Freescale Semiconductor, Inc
+ * Copyright 2009-2012 Freescale Semiconductor, Inc
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License as
@@ -53,6 +53,36 @@ void setup_ifc(void)
 
 	asm volatile("isync;msync;tlbwe;isync");
 
+#if defined(CONFIG_SYS_PPC_E500_DEBUG_TLB)
+/*
+ * TLB entry for debuggging in AS1
+ * Create temporary TLB entry in AS0 to handle debug exception
+ * As on debug exception MSR is cleared i.e. Address space is changed
+ * to 0. A TLB entry (in AS0) is required to handle debug exception generated
+ * in AS1.
+ *
+ * TLB entry is created for IVPR + IVOR15 to map on valid OP code address
+ * bacause flash's physical address is going to change as
+ * CONFIG_SYS_FLASH_BASE_PHYS.
+ */
+	_mas0 = MAS0_TLBSEL(1) |
+			MAS0_ESEL(CONFIG_SYS_PPC_E500_DEBUG_TLB);
+	_mas1 = MAS1_VALID | MAS1_TID(0) | MAS1_IPROT |
+			MAS1_TSIZE(BOOKE_PAGESZ_4M);
+	_mas2 = FSL_BOOKE_MAS2(CONFIG_SYS_TEXT_BASE, MAS2_I|MAS2_G);
+	_mas3 = FSL_BOOKE_MAS3(flash_phys, 0, MAS3_SW|MAS3_SR|MAS3_SX);
+	_mas7 = FSL_BOOKE_MAS7(flash_phys);
+
+	mtspr(MAS0, _mas0);
+	mtspr(MAS1, _mas1);
+	mtspr(MAS2, _mas2);
+	mtspr(MAS3, _mas3);
+	mtspr(MAS7, _mas7);
+
+	asm volatile("isync;msync;tlbwe;isync");
+#endif
+
+	/* Change flash's physical address */
 	out_be32(&(ifc_regs->cspr_cs[0].cspr), CONFIG_SYS_CSPR0);
 	out_be32(&(ifc_regs->csor_cs[0].csor), CONFIG_SYS_CSOR0);
 	out_be32(&(ifc_regs->amask_cs[0].amask), CONFIG_SYS_AMASK0);
diff --git a/arch/powerpc/cpu/mpc85xx/start.S b/arch/powerpc/cpu/mpc85xx/start.S
index 597151b..1cfd08a 100644
--- a/arch/powerpc/cpu/mpc85xx/start.S
+++ b/arch/powerpc/cpu/mpc85xx/start.S
@@ -182,6 +182,72 @@ l2_disabled:
 	andi.	r1,r3,L1CSR0_DCE at l
 	beq	2b
 
+#if defined(CONFIG_SYS_PPC_E500_DEBUG_TLB)
+/*
+ * TLB entry for debuggging in AS1
+ * Create temporary TLB entry in AS0 to handle debug exception
+ * As on debug exception MSR is cleared i.e. Address space is changed
+ * to 0. A TLB entry (in AS0) is required to handle debug exception generated
+ * in AS1.
+ */
+
+	lis     r6,FSL_BOOKE_MAS0(1,
+			CONFIG_SYS_PPC_E500_DEBUG_TLB, 0)@h
+	ori     r6,r6,FSL_BOOKE_MAS0(1,
+			CONFIG_SYS_PPC_E500_DEBUG_TLB, 0)@l
+
+#if !defined(CONFIG_SYS_RAMBOOT) && !defined(CONFIG_SECURE_BOOT)
+/*
+ * TLB entry is created for IVPR + IVOR15 to map on valid OP code address
+ * bacause flash's virtual address maps to 0xff800000 - 0xffffffff.
+ * and this window is outside of 4K boot window.
+ */
+	lis     r7,FSL_BOOKE_MAS1(1, 1, 0, 0, BOOKE_PAGESZ_4M)@h
+	ori     r7,r7,FSL_BOOKE_MAS1(1, 1, 0, 0, BOOKE_PAGESZ_4M)@l
+
+	lis     r8,FSL_BOOKE_MAS2(CONFIG_SYS_MONITOR_BASE & 0xffc00000,
+							(MAS2_I|MAS2_G))@h
+	ori     r8,r8,FSL_BOOKE_MAS2(CONFIG_SYS_MONITOR_BASE & 0xffc00000,
+							(MAS2_I|MAS2_G))@l
+
+	/* The 85xx has the default boot window 0xff800000 - 0xffffffff */
+	lis     r9,FSL_BOOKE_MAS3(0xffc00000, 0, (MAS3_SX|MAS3_SW|MAS3_SR))@h
+	ori     r9,r9,FSL_BOOKE_MAS3(0xffc00000, 0, (MAS3_SX|MAS3_SW|MAS3_SR))@l
+#elif !defined(CONFIG_SYS_RAMBOOT) && defined(CONFIG_SECURE_BOOT)
+	lis     r7,FSL_BOOKE_MAS1(1, 1, 0, 0, BOOKE_PAGESZ_1M)@h
+	ori     r7,r7,FSL_BOOKE_MAS1(1, 1, 0, 0, BOOKE_PAGESZ_1M)@l
+
+	lis     r8,FSL_BOOKE_MAS2(CONFIG_SYS_MONITOR_BASE,(MAS2_I|MAS2_G))@h
+	ori     r8,r8,FSL_BOOKE_MAS2(CONFIG_SYS_MONITOR_BASE,(MAS2_I|MAS2_G))@l
+
+	lis     r9,FSL_BOOKE_MAS3(CONFIG_SYS_PBI_FLASH_WINDOW, 0,
+						(MAS3_SX|MAS3_SW|MAS3_SR))@h
+	ori     r9,r9,FSL_BOOKE_MAS3(CONFIG_SYS_PBI_FLASH_WINDOW, 0,
+						(MAS3_SX|MAS3_SW|MAS3_SR))@l
+#else
+/*
+ * TLB entry is created for IVPR + IVOR15 to map on valid OP code address
+ * because "nexti" will resize TLB to 4K
+ */
+	lis     r7,FSL_BOOKE_MAS1(1, 1, 0, 0, BOOKE_PAGESZ_256K)@h
+	ori     r7,r7,FSL_BOOKE_MAS1(1, 1, 0, 0, BOOKE_PAGESZ_256K)@l
+
+	lis     r8,FSL_BOOKE_MAS2(CONFIG_SYS_MONITOR_BASE, (MAS2_I))@h
+	ori     r8,r8,FSL_BOOKE_MAS2(CONFIG_SYS_MONITOR_BASE,
+							(MAS2_I))@l
+	lis     r9,FSL_BOOKE_MAS3(CONFIG_SYS_MONITOR_BASE, 0,
+						(MAS3_SX|MAS3_SW|MAS3_SR))@h
+	ori     r9,r9,FSL_BOOKE_MAS3(CONFIG_SYS_MONITOR_BASE, 0,
+						(MAS3_SX|MAS3_SW|MAS3_SR))@l
+#endif
+	mtspr   MAS0,r6
+	mtspr   MAS1,r7
+	mtspr   MAS2,r8
+	mtspr   MAS3,r9
+	tlbwe
+	isync
+#endif
+
 /*
  * Ne need to setup interrupt vector for NAND SPL
  * because NAND SPL never compiles it.
diff --git a/arch/powerpc/include/asm/config_mpc85xx.h b/arch/powerpc/include/asm/config_mpc85xx.h
index 8654625..268c56e 100644
--- a/arch/powerpc/include/asm/config_mpc85xx.h
+++ b/arch/powerpc/include/asm/config_mpc85xx.h
@@ -1,5 +1,5 @@
 /*
- * Copyright 2011 Freescale Semiconductor, Inc.
+ * Copyright 2011-2012 Freescale Semiconductor, Inc.
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License as
@@ -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
 #define CONFIG_TSECV2
 #define CONFIG_SYS_FSL_SEC_COMPAT	4
 #define CONFIG_FSL_SATA_V2
-- 
1.7.5.4

[U-Boot] [PATCH 3/4][v3] powerpc/85xx:Make debug exception vector accessible

[Andy Fleming]

On Mon, Mar 26, 2012 at 4:00 AM, Prabhakar Kushwaha
<prabhakar@freescale.com> wrote:
> Debugging of e500 and e500v1 processer requires debug exception vecter (IVPR +
> IVOR15) to have valid and fetchable OP code.
>
> While executing in translated space (AS=1), whenever a debug exception is
> generated, the MSR[DS/IS] gets cleared i.e. AS=0 and the processor tries to
> fetch an instruction from the debug exception vector (IVPR + IVOR15); since now
> we are in AS=0, the application needs to ensure the proper TLB configuration to
> have (IVOR + IVOR15) accessible from AS=0 also.
>
> Create a temporary TLB in AS0 to make sure debug exception verctor is
> accessible on debug exception.
>
> Signed-off-by: Radu Lazarescu <radu.lazarescu@freescale.com>
> Signed-off-by: Marius Grigoras <marius.grigoras@freescale.com>
> Signed-off-by: Prabhakar Kushwaha <prabhakar@freescale.com>
> ---
> ?Based upon git://git.denx.de/u-boot.git branch master
>
> ?Changes for v2:
> ? ? ? ?- Put Temporary TLB creation under #define
>
> ?Changes for v3:
> ? ? ? ?- Removed unnecessary CONFIG_E500
> ? ? ? ?- Updated CONFG_SYS_RAMBOOT mas2 properties
> ? ? ? ?- Added secure boot Debug TLB.
> ? ? ? ?- Avoid temp TLB creation for NAND SPL
> ? ? ? ?- removed unnecessary mas7 updation
>
> ?Tested on
> ?- SoC having E500 Family processor (P1010RDB, BSC9131RDB)
> ?- SoC having E500MC Family processor (P4080DS, P3041DS)
>
> ?arch/powerpc/cpu/mpc85xx/cpu_init_early.c | ? 32 +++++++++++++-
> ?arch/powerpc/cpu/mpc85xx/start.S ? ? ? ? ?| ? 66 +++++++++++++++++++++++++++++
> ?arch/powerpc/include/asm/config_mpc85xx.h | ? ?3 +-
> ?3 files changed, 99 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/cpu/mpc85xx/cpu_init_early.c b/arch/powerpc/cpu/mpc85xx/cpu_init_early.c
> index 091af7c..dacfdd1 100644
> --- a/arch/powerpc/cpu/mpc85xx/cpu_init_early.c
> +++ b/arch/powerpc/cpu/mpc85xx/cpu_init_early.c
> @@ -1,5 +1,5 @@
> ?/*
> - * Copyright 2009-2011 Freescale Semiconductor, Inc
> + * Copyright 2009-2012 Freescale Semiconductor, Inc
> ?*
> ?* This program is free software; you can redistribute it and/or
> ?* modify it under the terms of the GNU General Public License as
> @@ -53,6 +53,36 @@ void setup_ifc(void)
>
> ? ? ? ?asm volatile("isync;msync;tlbwe;isync");
>
> +#if defined(CONFIG_SYS_PPC_E500_DEBUG_TLB)
> +/*
> + * TLB entry for debuggging in AS1
> + * Create temporary TLB entry in AS0 to handle debug exception
> + * As on debug exception MSR is cleared i.e. Address space is changed
> + * to 0. A TLB entry (in AS0) is required to handle debug exception generated
> + * in AS1.
> + *
> + * TLB entry is created for IVPR + IVOR15 to map on valid OP code address
> + * bacause flash's physical address is going to change as
> + * CONFIG_SYS_FLASH_BASE_PHYS.
> + */
> + ? ? ? _mas0 = MAS0_TLBSEL(1) |
> + ? ? ? ? ? ? ? ? ? ? ? MAS0_ESEL(CONFIG_SYS_PPC_E500_DEBUG_TLB);
> + ? ? ? _mas1 = MAS1_VALID | MAS1_TID(0) | MAS1_IPROT |
> + ? ? ? ? ? ? ? ? ? ? ? MAS1_TSIZE(BOOKE_PAGESZ_4M);
> + ? ? ? _mas2 = FSL_BOOKE_MAS2(CONFIG_SYS_TEXT_BASE, MAS2_I|MAS2_G);
> + ? ? ? _mas3 = FSL_BOOKE_MAS3(flash_phys, 0, MAS3_SW|MAS3_SR|MAS3_SX);
> + ? ? ? _mas7 = FSL_BOOKE_MAS7(flash_phys);
> +
> + ? ? ? mtspr(MAS0, _mas0);
> + ? ? ? mtspr(MAS1, _mas1);
> + ? ? ? mtspr(MAS2, _mas2);
> + ? ? ? mtspr(MAS3, _mas3);
> + ? ? ? mtspr(MAS7, _mas7);
> +
> + ? ? ? asm volatile("isync;msync;tlbwe;isync");
> +#endif
> +
> + ? ? ? /* Change flash's physical address */
> ? ? ? ?out_be32(&(ifc_regs->cspr_cs[0].cspr), CONFIG_SYS_CSPR0);
> ? ? ? ?out_be32(&(ifc_regs->csor_cs[0].csor), CONFIG_SYS_CSOR0);
> ? ? ? ?out_be32(&(ifc_regs->amask_cs[0].amask), CONFIG_SYS_AMASK0);
> diff --git a/arch/powerpc/cpu/mpc85xx/start.S b/arch/powerpc/cpu/mpc85xx/start.S
> index 597151b..1cfd08a 100644
> --- a/arch/powerpc/cpu/mpc85xx/start.S
> +++ b/arch/powerpc/cpu/mpc85xx/start.S
> @@ -182,6 +182,72 @@ l2_disabled:
> ? ? ? ?andi. ? r1,r3,L1CSR0_DCE at l
> ? ? ? ?beq ? ? 2b
>
> +#if defined(CONFIG_SYS_PPC_E500_DEBUG_TLB)
> +/*
> + * TLB entry for debuggging in AS1
> + * Create temporary TLB entry in AS0 to handle debug exception
> + * As on debug exception MSR is cleared i.e. Address space is changed
> + * to 0. A TLB entry (in AS0) is required to handle debug exception generated
> + * in AS1.
> + */
> +
> + ? ? ? lis ? ? r6,FSL_BOOKE_MAS0(1,
> + ? ? ? ? ? ? ? ? ? ? ? CONFIG_SYS_PPC_E500_DEBUG_TLB, 0)@h
> + ? ? ? ori ? ? r6,r6,FSL_BOOKE_MAS0(1,
> + ? ? ? ? ? ? ? ? ? ? ? CONFIG_SYS_PPC_E500_DEBUG_TLB, 0)@l
> +
> +#if !defined(CONFIG_SYS_RAMBOOT) && !defined(CONFIG_SECURE_BOOT)
> +/*
> + * TLB entry is created for IVPR + IVOR15 to map on valid OP code address
> + * bacause flash's virtual address maps to 0xff800000 - 0xffffffff.
> + * and this window is outside of 4K boot window.
> + */
> + ? ? ? lis ? ? r7,FSL_BOOKE_MAS1(1, 1, 0, 0, BOOKE_PAGESZ_4M)@h
> + ? ? ? ori ? ? r7,r7,FSL_BOOKE_MAS1(1, 1, 0, 0, BOOKE_PAGESZ_4M)@l
> +
> + ? ? ? lis ? ? r8,FSL_BOOKE_MAS2(CONFIG_SYS_MONITOR_BASE & 0xffc00000,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? (MAS2_I|MAS2_G))@h
> + ? ? ? ori ? ? r8,r8,FSL_BOOKE_MAS2(CONFIG_SYS_MONITOR_BASE & 0xffc00000,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? (MAS2_I|MAS2_G))@l
> +
> + ? ? ? /* The 85xx has the default boot window 0xff800000 - 0xffffffff */
> + ? ? ? lis ? ? r9,FSL_BOOKE_MAS3(0xffc00000, 0, (MAS3_SX|MAS3_SW|MAS3_SR))@h
> + ? ? ? ori ? ? r9,r9,FSL_BOOKE_MAS3(0xffc00000, 0, (MAS3_SX|MAS3_SW|MAS3_SR))@l
> +#elif !defined(CONFIG_SYS_RAMBOOT) && defined(CONFIG_SECURE_BOOT)
> + ? ? ? lis ? ? r7,FSL_BOOKE_MAS1(1, 1, 0, 0, BOOKE_PAGESZ_1M)@h
> + ? ? ? ori ? ? r7,r7,FSL_BOOKE_MAS1(1, 1, 0, 0, BOOKE_PAGESZ_1M)@l
> +
> + ? ? ? lis ? ? r8,FSL_BOOKE_MAS2(CONFIG_SYS_MONITOR_BASE,(MAS2_I|MAS2_G))@h
> + ? ? ? ori ? ? r8,r8,FSL_BOOKE_MAS2(CONFIG_SYS_MONITOR_BASE,(MAS2_I|MAS2_G))@l
> +
> + ? ? ? lis ? ? r9,FSL_BOOKE_MAS3(CONFIG_SYS_PBI_FLASH_WINDOW, 0,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? (MAS3_SX|MAS3_SW|MAS3_SR))@h
> + ? ? ? ori ? ? r9,r9,FSL_BOOKE_MAS3(CONFIG_SYS_PBI_FLASH_WINDOW, 0,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? (MAS3_SX|MAS3_SW|MAS3_SR))@l
> +#else
> +/*
> + * TLB entry is created for IVPR + IVOR15 to map on valid OP code address
> + * because "nexti" will resize TLB to 4K
> + */
> + ? ? ? lis ? ? r7,FSL_BOOKE_MAS1(1, 1, 0, 0, BOOKE_PAGESZ_256K)@h
> + ? ? ? ori ? ? r7,r7,FSL_BOOKE_MAS1(1, 1, 0, 0, BOOKE_PAGESZ_256K)@l
> +
> + ? ? ? lis ? ? r8,FSL_BOOKE_MAS2(CONFIG_SYS_MONITOR_BASE, (MAS2_I))@h
> + ? ? ? ori ? ? r8,r8,FSL_BOOKE_MAS2(CONFIG_SYS_MONITOR_BASE,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? (MAS2_I))@l
> + ? ? ? lis ? ? r9,FSL_BOOKE_MAS3(CONFIG_SYS_MONITOR_BASE, 0,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? (MAS3_SX|MAS3_SW|MAS3_SR))@h
> + ? ? ? ori ? ? r9,r9,FSL_BOOKE_MAS3(CONFIG_SYS_MONITOR_BASE, 0,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? (MAS3_SX|MAS3_SW|MAS3_SR))@l
> +#endif
> + ? ? ? mtspr ? MAS0,r6
> + ? ? ? mtspr ? MAS1,r7
> + ? ? ? mtspr ? MAS2,r8
> + ? ? ? mtspr ? MAS3,r9
> + ? ? ? tlbwe
> + ? ? ? isync
> +#endif
> +
> ?/*
> ?* Ne need to setup interrupt vector for NAND SPL
> ?* because NAND SPL never compiles it.
> diff --git a/arch/powerpc/include/asm/config_mpc85xx.h b/arch/powerpc/include/asm/config_mpc85xx.h
> index 8654625..268c56e 100644
> --- a/arch/powerpc/include/asm/config_mpc85xx.h
> +++ b/arch/powerpc/include/asm/config_mpc85xx.h
> @@ -1,5 +1,5 @@
> ?/*
> - * Copyright 2011 Freescale Semiconductor, Inc.
> + * Copyright 2011-2012 Freescale Semiconductor, Inc.
> ?*
> ?* This program is free software; you can redistribute it and/or
> ?* modify it under the terms of the GNU General Public License as
> @@ -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

If that won't work (I haven't looked carefully as to whether 3 is OK
on all platforms), then you need to go through and choose values that
work...

Andy

[U-Boot] [PATCH 3/4][v3] powerpc/85xx:Make debug exception vector accessible

[Scott Wood]

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.

-Scot

[U-Boot] [PATCH 3/4][v3] powerpc/85xx:Make debug exception vector accessible

[Andy Fleming]

On Tue, Apr 24, 2012 at 4:10 PM, Scott Wood <scottwood@freescale.com> 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:

#if defined(CONFIG_E500MC)
#define CONFIG_SYS_NUM_TLBCAMS          64
#elif defined(CONFIG_E500)
#define CONFIG_SYS_NUM_TLBCAMS          16
-->
#endif


Andy

[U-Boot] [PATCH 3/4][v3] powerpc/85xx:Make debug exception vector accessible

[Prabhakar Kushwaha]

Hi,


On Wednesday 25 April 2012 02:50 AM, Andy Fleming wrote:
> On Tue, Apr 24, 2012 at 4:10 PM, Scott Wood<scottwood@freescale.com>  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.

> #if defined(CONFIG_E500MC)
> #define CONFIG_SYS_NUM_TLBCAMS          64
> #elif defined(CONFIG_E500)
> #define CONFIG_SYS_NUM_TLBCAMS          16
> -->
> #endif
>
is anything missing here ??


Thanks,
Prabhakar

[U-Boot] [PATCH 3/4][v3] powerpc/85xx:Make debug exception vector accessible

[Andy Fleming]

On Tue, Apr 24, 2012 at 10:48 PM, Prabhakar Kushwaha
<prabhakar@freescale.com> wrote:
> Hi,
>
>
>
> On Wednesday 25 April 2012 02:50 AM, Andy Fleming wrote:
>>
>> On Tue, Apr 24, 2012 at 4:10 PM, Scott Wood<scottwood@freescale.com>
>> ?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.

Andy

[U-Boot] [PATCH 3/4][v3] powerpc/85xx:Make debug exception vector accessible

[Scott Wood]

On 04/24/2012 11:05 PM, Andy Fleming wrote:
> On Tue, Apr 24, 2012 at 10:48 PM, Prabhakar Kushwaha
> <prabhakar@freescale.com>  wrote:
>> Hi,
>>
>>
>>
>> On Wednesday 25 April 2012 02:50 AM, Andy Fleming wrote:
>>>
>>> On Tue, Apr 24, 2012 at 4:10 PM, Scott Wood<scottwood@freescale.com>
>>>   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

[U-Boot] [PATCH 3/4][v3] powerpc/85xx:Make debug exception vector accessible

[Prabhakar Kushwaha]

Hi Scott,

On Wednesday 25 April 2012 08:20 PM, Scott Wood wrote:
> On 04/24/2012 11:05 PM, Andy Fleming wrote:
>> On Tue, Apr 24, 2012 at 10:48 PM, Prabhakar Kushwaha
>> <prabhakar@freescale.com>  wrote:
>>> Hi,
>>>
>>>
>>>
>>> On Wednesday 25 April 2012 02:50 AM, Andy Fleming wrote:
>>>>
>>>> On Tue, Apr 24, 2012 at 4:10 PM, Scott Wood<scottwood@freescale.com>
>>>>   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.

I believe , let this patch-set be applied in upstream. then during 
enabling debugging support of BSC9131 , i will enable for other SoC like 
P1020, P2020 etc.


Regards,
Prabhakar

[U-Boot] [PATCH 3/4][v3] powerpc/85xx:Make debug exception vector accessible

[Andy Fleming]

Argh, forgot to copy the list.

>>
>>
>> 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.
>
>
> I believe , let this patch-set be applied in upstream. then during enabling
> debugging support of BSC9131 , i will enable for other SoC like P1020, P2020
> etc.
>

Our point is that this isn't an SoC-specific feature. It's a feature
of the core. Change your code to enable it for e500 and e500v2, and
then test it on a few systems to make sure it works. We don't want to
have to enable it for each SoC, one by one.

Andy