Patchwork [1/5] powerpc: booke: Don't hard-code size of struct tlbcam

login
register
mail settings
Submitter Trent Piepho
Date Dec. 9, 2008, 3:34 a.m.
Message ID <1228793699-23110-1-git-send-email-tpiepho@freescale.com>
Download mbox | patch
Permalink /patch/12881/
State Accepted
Delegated to: Kumar Gala
Headers show

Comments

Trent Piepho - Dec. 9, 2008, 3:34 a.m.
Some assembly code in head_fsl_booke.S hard-coded the size of struct tlbcam
to 20 when it indexed the TLBCAM table.  Anyone changing the size of struct
tlbcam would not know to expect that.

The kernel already has a system to get the size of C structures into
assembly language files, asm-offsets, so let's use it.

The definition of the struct gets moved to a header, so that asm-offsets.c
can include it.

Signed-off-by: Trent Piepho <tpiepho@freescale.com>
---
 arch/powerpc/kernel/asm-offsets.c    |    8 ++++++++
 arch/powerpc/kernel/head_fsl_booke.S |    2 +-
 arch/powerpc/mm/fsl_booke_mmu.c      |    8 +-------
 arch/powerpc/mm/mmu_decl.h           |    9 +++++++++
 4 files changed, 19 insertions(+), 8 deletions(-)
Josh Boyer - Dec. 9, 2008, 2:26 p.m.
On Mon,  8 Dec 2008 19:34:55 -0800
Trent Piepho <tpiepho@freescale.com> wrote:

> Some assembly code in head_fsl_booke.S hard-coded the size of struct tlbcam
> to 20 when it indexed the TLBCAM table.  Anyone changing the size of struct
> tlbcam would not know to expect that.
> 
> The kernel already has a system to get the size of C structures into
> assembly language files, asm-offsets, so let's use it.
> 
> The definition of the struct gets moved to a header, so that asm-offsets.c
> can include it.

I don't mean to be overly picky, but your patch subjects and changelog
descriptions are a bit wrong.  This series pertains to FSL BookE chips,
not BookE in general.  There are other variants of BookE, such as 4xx.

If you could keep that in mind for future revisions, I'd appreciate
it.  Something like:

[PATCH] powerpc/fsl-booke: 

or something similar would be a bit more correct.  Unless you really
are changing something global to all BookE processors (which is sort of
rare at the moment).

josh
Kumar Gala - Jan. 7, 2009, 4:03 p.m.
On Dec 8, 2008, at 9:34 PM, Trent Piepho wrote:

> Some assembly code in head_fsl_booke.S hard-coded the size of struct  
> tlbcam
> to 20 when it indexed the TLBCAM table.  Anyone changing the size of  
> struct
> tlbcam would not know to expect that.
>
> The kernel already has a system to get the size of C structures into
> assembly language files, asm-offsets, so let's use it.
>
> The definition of the struct gets moved to a header, so that asm- 
> offsets.c
> can include it.
>
> Signed-off-by: Trent Piepho <tpiepho@freescale.com>
> ---
> arch/powerpc/kernel/asm-offsets.c    |    8 ++++++++
> arch/powerpc/kernel/head_fsl_booke.S |    2 +-
> arch/powerpc/mm/fsl_booke_mmu.c      |    8 +-------
> arch/powerpc/mm/mmu_decl.h           |    9 +++++++++
> 4 files changed, 19 insertions(+), 8 deletions(-)

applied to next

- k
Kumar Gala - Jan. 7, 2009, 4:17 p.m.
On Dec 9, 2008, at 8:26 AM, Josh Boyer wrote:

> On Mon,  8 Dec 2008 19:34:55 -0800
> Trent Piepho <tpiepho@freescale.com> wrote:
>
>> Some assembly code in head_fsl_booke.S hard-coded the size of  
>> struct tlbcam
>> to 20 when it indexed the TLBCAM table.  Anyone changing the size  
>> of struct
>> tlbcam would not know to expect that.
>>
>> The kernel already has a system to get the size of C structures into
>> assembly language files, asm-offsets, so let's use it.
>>
>> The definition of the struct gets moved to a header, so that asm- 
>> offsets.c
>> can include it.
>
> I don't mean to be overly picky, but your patch subjects and changelog
> descriptions are a bit wrong.  This series pertains to FSL BookE  
> chips,
> not BookE in general.  There are other variants of BookE, such as 4xx.
>
> If you could keep that in mind for future revisions, I'd appreciate
> it.  Something like:
>
> [PATCH] powerpc/fsl-booke:
>
> or something similar would be a bit more correct.  Unless you really
> are changing something global to all BookE processors (which is sort  
> of
> rare at the moment).

I fixed this in the actually commit ;)

- k

Patch

diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
index 050abfd..4d62a8d 100644
--- a/arch/powerpc/kernel/asm-offsets.c
+++ b/arch/powerpc/kernel/asm-offsets.c
@@ -56,6 +56,10 @@ 
 #include "head_booke.h"
 #endif
 
+#if defined(CONFIG_FSL_BOOKE)
+#include "../mm/mmu_decl.h"
+#endif
+
 int main(void)
 {
 	DEFINE(THREAD, offsetof(struct task_struct, thread));
@@ -271,6 +275,10 @@  int main(void)
 	DEFINE(SAVED_KSP_LIMIT, STACK_INT_FRAME_SIZE+offsetof(struct exception_regs, saved_ksp_limit));
 #endif
 
+#ifdef CONFIG_FSL_BOOKE
+	DEFINE(TLBCAM_SIZE, sizeof(struct tlbcam));
+#endif
+
 	DEFINE(CLONE_VM, CLONE_VM);
 	DEFINE(CLONE_UNTRACED, CLONE_UNTRACED);
 
diff --git a/arch/powerpc/kernel/head_fsl_booke.S b/arch/powerpc/kernel/head_fsl_booke.S
index 9a4639c..c591acb 100644
--- a/arch/powerpc/kernel/head_fsl_booke.S
+++ b/arch/powerpc/kernel/head_fsl_booke.S
@@ -909,7 +909,7 @@  KernelSPE:
 _GLOBAL(loadcam_entry)
 	lis	r4,TLBCAM@ha
 	addi	r4,r4,TLBCAM@l
-	mulli	r5,r3,20
+	mulli	r5,r3,TLBCAM_SIZE
 	add	r3,r5,r4
 	lwz	r4,0(r3)
 	mtspr	SPRN_MAS0,r4
diff --git a/arch/powerpc/mm/fsl_booke_mmu.c b/arch/powerpc/mm/fsl_booke_mmu.c
index 23cee39..c9ee59a 100644
--- a/arch/powerpc/mm/fsl_booke_mmu.c
+++ b/arch/powerpc/mm/fsl_booke_mmu.c
@@ -61,13 +61,7 @@  static unsigned long __cam0, __cam1, __cam2;
 
 #define NUM_TLBCAMS	(16)
 
-struct tlbcam {
-   	u32	MAS0;
-	u32	MAS1;
-	u32	MAS2;
-	u32	MAS3;
-	u32	MAS7;
-} TLBCAM[NUM_TLBCAMS];
+struct tlbcam TLBCAM[NUM_TLBCAMS];
 
 struct tlbcamrange {
    	unsigned long start;
diff --git a/arch/powerpc/mm/mmu_decl.h b/arch/powerpc/mm/mmu_decl.h
index fab3cfa..f0d0aae 100644
--- a/arch/powerpc/mm/mmu_decl.h
+++ b/arch/powerpc/mm/mmu_decl.h
@@ -27,6 +27,15 @@  extern void hash_preload(struct mm_struct *mm, unsigned long ea,
 
 
 #ifdef CONFIG_PPC32
+
+struct tlbcam {
+	u32	MAS0;
+	u32	MAS1;
+	u32	MAS2;
+	u32	MAS3;
+	u32	MAS7;
+};
+
 extern void mapin_ram(void);
 extern int map_page(unsigned long va, phys_addr_t pa, int flags);
 extern void setbat(int index, unsigned long virt, phys_addr_t phys,