* [RFC PATCH v2 0/5] Add generic macros for declaring various CPU structs
@ 2011-06-14 10:58 Dave Martin
2011-06-14 10:58 ` [RFC PATCH v2 1/5] ARM: mm: Add generic proc/arch struct definition macros Dave Martin
` (5 more replies)
0 siblings, 6 replies; 15+ messages in thread
From: Dave Martin @ 2011-06-14 10:58 UTC (permalink / raw)
To: linux-arm-kernel
Based on previous discussions, this series provides an alternative
macro for defining CPU-specific structures compactly.
I don't get to have so much fun with macros in this version, but
it's more straightforward and actually allows the declarations to
be collapsed down further by taking advantage of common naming
conventions.
In this RFC, only the arch names, processor_functions and
cache_functions are turned into macros, to show how this could
work. This could be straightforwardly extended to cover tlb_fns,
but proc_info is more complex and may require a bit more thought.
A couple of files are ported to use the macros as an example.
These patches are for illustration only and **untested** (although
I have build-tested omap2plus_defconfig).
Unresolved issues:
* The architecture/CPU name strings were previously split out into
separate places, with the CPU name in .text and the other
strings in .rodata. This looked anomalous, so now a common
mergeable string section .rodata.str is used instead. This can
be changed if needed.
* For consistency, I've renamed the arch/CPU name string labels.
If that is seen as unnecessary churn, it can be undone.
* It's unclear whether it's better to switch sections inside the
macros or outside. Currently I switch inside, but the previous
section is always restored using .popsection at the end of the
macro to avoid confusion. I'm assuming that there will never
be a need to put any of this data in a custom section: if that's
a bad assumption, we could move the section directives back
outside the macros.
Dave Martin (5):
ARM: mm: Add generic proc/arch struct definition macros
ARM: proc-v7: Use new generic struct definition macros
ARM: cache-v7: Use new generic struct definition macros
ARM: proc-v6: Use new generic struct definition macros
ARM: cache-v6: Use new generic struct definition macros
arch/arm/mm/cache-v6.S | 16 +--------
arch/arm/mm/cache-v7.S | 16 +--------
arch/arm/mm/proc-macros.S | 79 +++++++++++++++++++++++++++++++++++++++++++++
arch/arm/mm/proc-v6.S | 43 +++---------------------
arch/arm/mm/proc-v7.S | 46 ++++---------------------
5 files changed, 94 insertions(+), 106 deletions(-)
--
1.7.4.1
^ permalink raw reply [flat|nested] 15+ messages in thread
* [RFC PATCH v2 1/5] ARM: mm: Add generic proc/arch struct definition macros
2011-06-14 10:58 [RFC PATCH v2 0/5] Add generic macros for declaring various CPU structs Dave Martin
@ 2011-06-14 10:58 ` Dave Martin
2011-06-15 23:57 ` Nicolas Pitre
2011-06-14 10:58 ` [RFC PATCH v2 2/5] ARM: proc-v7: Use new generic " Dave Martin
` (4 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Dave Martin @ 2011-06-14 10:58 UTC (permalink / raw)
To: linux-arm-kernel
This patch adds some generic macros to reduce boilerplate when
declaring certain common structures in arch/arm/mm/*.S
Thanks to Russell King for outlining what the
define_processor_functions macro could look like.
Signed-off-by: Dave Martin <dave.martin@linaro.org>
---
arch/arm/mm/proc-macros.S | 79 +++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 79 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mm/proc-macros.S b/arch/arm/mm/proc-macros.S
index 34261f9..3c5ffbb 100644
--- a/arch/arm/mm/proc-macros.S
+++ b/arch/arm/mm/proc-macros.S
@@ -4,6 +4,7 @@
* VMA_VM_FLAGS
* VM_EXEC
*/
+#include <linux/init.h>
#include <asm/asm-offsets.h>
#include <asm/thread_info.h>
@@ -254,3 +255,81 @@
mcr p15, 0, r0, c7, c10, 1 @ clean L1 D line
mcr p15, 0, ip, c7, c10, 4 @ data write barrier
.endm
+
+.macro define_processor_functions name:req, dabort:req, pabort:req, nommu=0, suspend=0
+ .pushsection .text
+
+ __INITDATA
+
+ .type \name\()_processor_functions, #object
+
+ENTRY(\name\()_processor_functions)
+ .word \dabort\()_abort
+ .word \pabort\()_pabort
+ .word cpu_\name\()_proc_init
+ .word cpu_\name\()_proc_fin
+ .word cpu_\name\()_reset
+ .word cpu_\name\()_do_idle
+ .word cpu_\name\()_dcache_clean_area
+ .word cpu_\name\()_switch_mm
+
+ .if \nommu
+ .word 0
+ .else
+ .word cpu_\name\()_set_pte_ext
+ .endif
+
+ .if \suspend
+ .word cpu_\name\()_suspend_size
+ .word cpu_\name\()_do_suspend
+ .word cpu_\name\()_do_resume
+ .else
+ .word 0
+ .word 0
+ .word 0
+ .endif
+
+ .size \name\()_processor_functions, . - \name\()_processor_functions
+
+ .popsection
+.endm
+
+.macro define_proc_names name:req, arch:req, elf:req, cpu:req
+ .pushsection .rodata.str, "aMS", 1
+
+ .type \name\()_arch_name, #object
+\name\()_arch_name: .asciz "\arch"
+ .size \name\()_arch_name, . - \name\()_arch_name
+
+ .type \name\()_elf_name, #object
+\name\()_elf_name: .asciz "\elf"
+ .size \name\()_elf_name, . - \name\()_elf_name
+
+ .type \name\()_cpu_name, #object
+\name\()_cpu_name: .asciz "\cpu"
+ .size \name\()_cpu_name, . - \name\()_cpu_name
+
+ .popsection
+.endm
+
+.macro define_cache_functions name:req
+ .pushsection .text
+
+ __INITDATA
+
+ .type \name\()_cache_fns, #object
+ENTRY(\name\()_cache_fns)
+ .long \name\()_flush_icache_all
+ .long \name\()_flush_kern_cache_all
+ .long \name\()_flush_user_cache_all
+ .long \name\()_flush_user_cache_range
+ .long \name\()_coherent_kern_range
+ .long \name\()_coherent_user_range
+ .long \name\()_flush_kern_dcache_area
+ .long \name\()_dma_map_area
+ .long \name\()_dma_unmap_area
+ .long \name\()_dma_flush_range
+ .size \name\()_cache_fns, . - \name\()_cache_fns
+
+ .popsection
+.endm
--
1.7.4.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [RFC PATCH v2 2/5] ARM: proc-v7: Use new generic struct definition macros
2011-06-14 10:58 [RFC PATCH v2 0/5] Add generic macros for declaring various CPU structs Dave Martin
2011-06-14 10:58 ` [RFC PATCH v2 1/5] ARM: mm: Add generic proc/arch struct definition macros Dave Martin
@ 2011-06-14 10:58 ` Dave Martin
2011-06-16 10:15 ` Will Deacon
2011-06-14 10:58 ` [RFC PATCH v2 3/5] ARM: cache-v7: " Dave Martin
` (3 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Dave Martin @ 2011-06-14 10:58 UTC (permalink / raw)
To: linux-arm-kernel
Signed-off-by: Dave Martin <dave.martin@linaro.org>
---
arch/arm/mm/proc-v7.S | 46 ++++++++--------------------------------------
1 files changed, 8 insertions(+), 38 deletions(-)
diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S
index b3b566e..c5cf6ea 100644
--- a/arch/arm/mm/proc-v7.S
+++ b/arch/arm/mm/proc-v7.S
@@ -171,8 +171,6 @@ ENTRY(cpu_v7_set_pte_ext)
mov pc, lr
ENDPROC(cpu_v7_set_pte_ext)
-cpu_v7_name:
- .ascii "ARMv7 Processor"
.align
/*
@@ -403,36 +401,8 @@ v7_crval:
__v7_setup_stack:
.space 4 * 11 @ 11 registers
- __INITDATA
-
- .type v7_processor_functions, #object
-ENTRY(v7_processor_functions)
- .word v7_early_abort
- .word v7_pabort
- .word cpu_v7_proc_init
- .word cpu_v7_proc_fin
- .word cpu_v7_reset
- .word cpu_v7_do_idle
- .word cpu_v7_dcache_clean_area
- .word cpu_v7_switch_mm
- .word cpu_v7_set_pte_ext
- .word 0
- .word 0
- .word 0
- .size v7_processor_functions, . - v7_processor_functions
-
- .section ".rodata"
-
- .type cpu_arch_name, #object
-cpu_arch_name:
- .asciz "armv7"
- .size cpu_arch_name, . - cpu_arch_name
-
- .type cpu_elf_name, #object
-cpu_elf_name:
- .asciz "v7"
- .size cpu_elf_name, . - cpu_elf_name
- .align
+ define_processor_functions v7, dabort=v7_early, pabort=v7
+ define_proc_names v7, "armv7", "v7", "ARMv7 Processor"
.section ".proc.info.init", #alloc, #execinstr
@@ -455,10 +425,10 @@ __v7_ca9mp_proc_info:
PMD_SECT_AP_WRITE | \
PMD_SECT_AP_READ
W(b) __v7_ca9mp_setup
- .long cpu_arch_name
- .long cpu_elf_name
+ .long v7_arch_name
+ .long v7_elf_name
.long HWCAP_SWP|HWCAP_HALF|HWCAP_THUMB|HWCAP_FAST_MULT|HWCAP_EDSP|HWCAP_TLS
- .long cpu_v7_name
+ .long v7_cpu_name
.long v7_processor_functions
.long v7wbi_tlb_fns
.long v6_user_fns
@@ -487,10 +457,10 @@ __v7_proc_info:
PMD_SECT_AP_WRITE | \
PMD_SECT_AP_READ
W(b) __v7_setup
- .long cpu_arch_name
- .long cpu_elf_name
+ .long v7_arch_name
+ .long v7_elf_name
.long HWCAP_SWP|HWCAP_HALF|HWCAP_THUMB|HWCAP_FAST_MULT|HWCAP_EDSP|HWCAP_TLS
- .long cpu_v7_name
+ .long v7_cpu_name
.long v7_processor_functions
.long v7wbi_tlb_fns
.long v6_user_fns
--
1.7.4.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [RFC PATCH v2 3/5] ARM: cache-v7: Use new generic struct definition macros
2011-06-14 10:58 [RFC PATCH v2 0/5] Add generic macros for declaring various CPU structs Dave Martin
2011-06-14 10:58 ` [RFC PATCH v2 1/5] ARM: mm: Add generic proc/arch struct definition macros Dave Martin
2011-06-14 10:58 ` [RFC PATCH v2 2/5] ARM: proc-v7: Use new generic " Dave Martin
@ 2011-06-14 10:58 ` Dave Martin
2011-06-14 10:58 ` [RFC PATCH v2 4/5] ARM: proc-v6: " Dave Martin
` (2 subsequent siblings)
5 siblings, 0 replies; 15+ messages in thread
From: Dave Martin @ 2011-06-14 10:58 UTC (permalink / raw)
To: linux-arm-kernel
Signed-off-by: Dave Martin <dave.martin@linaro.org>
---
arch/arm/mm/cache-v7.S | 16 +---------------
1 files changed, 1 insertions(+), 15 deletions(-)
diff --git a/arch/arm/mm/cache-v7.S b/arch/arm/mm/cache-v7.S
index d32f02b..be4e94b 100644
--- a/arch/arm/mm/cache-v7.S
+++ b/arch/arm/mm/cache-v7.S
@@ -323,18 +323,4 @@ ENTRY(v7_dma_unmap_area)
mov pc, lr
ENDPROC(v7_dma_unmap_area)
- __INITDATA
-
- .type v7_cache_fns, #object
-ENTRY(v7_cache_fns)
- .long v7_flush_icache_all
- .long v7_flush_kern_cache_all
- .long v7_flush_user_cache_all
- .long v7_flush_user_cache_range
- .long v7_coherent_kern_range
- .long v7_coherent_user_range
- .long v7_flush_kern_dcache_area
- .long v7_dma_map_area
- .long v7_dma_unmap_area
- .long v7_dma_flush_range
- .size v7_cache_fns, . - v7_cache_fns
+ define_cache_functions v7
--
1.7.4.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [RFC PATCH v2 4/5] ARM: proc-v6: Use new generic struct definition macros
2011-06-14 10:58 [RFC PATCH v2 0/5] Add generic macros for declaring various CPU structs Dave Martin
` (2 preceding siblings ...)
2011-06-14 10:58 ` [RFC PATCH v2 3/5] ARM: cache-v7: " Dave Martin
@ 2011-06-14 10:58 ` Dave Martin
2011-06-14 10:58 ` [RFC PATCH v2 5/5] ARM: cache-v6: " Dave Martin
2011-06-16 10:12 ` [RFC PATCH v2 0/5] Add generic macros for declaring various CPU structs Will Deacon
5 siblings, 0 replies; 15+ messages in thread
From: Dave Martin @ 2011-06-14 10:58 UTC (permalink / raw)
To: linux-arm-kernel
Signed-off-by: Dave Martin <dave.martin@linaro.org>
---
arch/arm/mm/proc-v6.S | 43 +++++--------------------------------------
1 files changed, 5 insertions(+), 38 deletions(-)
diff --git a/arch/arm/mm/proc-v6.S b/arch/arm/mm/proc-v6.S
index 1d2b845..d7ae4b7 100644
--- a/arch/arm/mm/proc-v6.S
+++ b/arch/arm/mm/proc-v6.S
@@ -170,11 +170,6 @@ cpu_resume_l1_flags:
#endif
- .type cpu_v6_name, #object
-cpu_v6_name:
- .asciz "ARMv6-compatible processor"
- .size cpu_v6_name, . - cpu_v6_name
-
.align
__CPUINIT
@@ -237,36 +232,8 @@ __v6_setup:
v6_crval:
crval clear=0x01e0fb7f, mmuset=0x00c0387d, ucset=0x00c0187c
- __INITDATA
-
- .type v6_processor_functions, #object
-ENTRY(v6_processor_functions)
- .word v6_early_abort
- .word v6_pabort
- .word cpu_v6_proc_init
- .word cpu_v6_proc_fin
- .word cpu_v6_reset
- .word cpu_v6_do_idle
- .word cpu_v6_dcache_clean_area
- .word cpu_v6_switch_mm
- .word cpu_v6_set_pte_ext
- .word cpu_v6_suspend_size
- .word cpu_v6_do_suspend
- .word cpu_v6_do_resume
- .size v6_processor_functions, . - v6_processor_functions
-
- .section ".rodata"
-
- .type cpu_arch_name, #object
-cpu_arch_name:
- .asciz "armv6"
- .size cpu_arch_name, . - cpu_arch_name
-
- .type cpu_elf_name, #object
-cpu_elf_name:
- .asciz "v6"
- .size cpu_elf_name, . - cpu_elf_name
- .align
+ define_processor_functions v6, dabort=v6_early, pabort=v6, suspend=1
+ define_proc_names v6, "armv6", "v6", "ARMv6-compatible processor"
.section ".proc.info.init", #alloc, #execinstr
@@ -292,11 +259,11 @@ __v6_proc_info:
PMD_SECT_AP_WRITE | \
PMD_SECT_AP_READ
b __v6_setup
- .long cpu_arch_name
- .long cpu_elf_name
+ .long v6_arch_name
+ .long v6_elf_name
/* See also feat_v6_fixup() for HWCAP_TLS */
.long HWCAP_SWP|HWCAP_HALF|HWCAP_THUMB|HWCAP_FAST_MULT|HWCAP_EDSP|HWCAP_JAVA|HWCAP_TLS
- .long cpu_v6_name
+ .long v6_cpu_name
.long v6_processor_functions
.long v6wbi_tlb_fns
.long v6_user_fns
--
1.7.4.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [RFC PATCH v2 5/5] ARM: cache-v6: Use new generic struct definition macros
2011-06-14 10:58 [RFC PATCH v2 0/5] Add generic macros for declaring various CPU structs Dave Martin
` (3 preceding siblings ...)
2011-06-14 10:58 ` [RFC PATCH v2 4/5] ARM: proc-v6: " Dave Martin
@ 2011-06-14 10:58 ` Dave Martin
2011-06-16 10:12 ` [RFC PATCH v2 0/5] Add generic macros for declaring various CPU structs Will Deacon
5 siblings, 0 replies; 15+ messages in thread
From: Dave Martin @ 2011-06-14 10:58 UTC (permalink / raw)
To: linux-arm-kernel
Signed-off-by: Dave Martin <dave.martin@linaro.org>
---
arch/arm/mm/cache-v6.S | 16 +---------------
1 files changed, 1 insertions(+), 15 deletions(-)
diff --git a/arch/arm/mm/cache-v6.S b/arch/arm/mm/cache-v6.S
index 73b4a8b..93d3bf8 100644
--- a/arch/arm/mm/cache-v6.S
+++ b/arch/arm/mm/cache-v6.S
@@ -328,18 +328,4 @@ ENTRY(v6_dma_unmap_area)
mov pc, lr
ENDPROC(v6_dma_unmap_area)
- __INITDATA
-
- .type v6_cache_fns, #object
-ENTRY(v6_cache_fns)
- .long v6_flush_icache_all
- .long v6_flush_kern_cache_all
- .long v6_flush_user_cache_all
- .long v6_flush_user_cache_range
- .long v6_coherent_kern_range
- .long v6_coherent_user_range
- .long v6_flush_kern_dcache_area
- .long v6_dma_map_area
- .long v6_dma_unmap_area
- .long v6_dma_flush_range
- .size v6_cache_fns, . - v6_cache_fns
+ define_cache_functions v6
--
1.7.4.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [RFC PATCH v2 1/5] ARM: mm: Add generic proc/arch struct definition macros
2011-06-14 10:58 ` [RFC PATCH v2 1/5] ARM: mm: Add generic proc/arch struct definition macros Dave Martin
@ 2011-06-15 23:57 ` Nicolas Pitre
2011-06-16 10:03 ` Dave Martin
0 siblings, 1 reply; 15+ messages in thread
From: Nicolas Pitre @ 2011-06-15 23:57 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, 14 Jun 2011, Dave Martin wrote:
> This patch adds some generic macros to reduce boilerplate when
> declaring certain common structures in arch/arm/mm/*.S
>
> Thanks to Russell King for outlining what the
> define_processor_functions macro could look like.
>
> Signed-off-by: Dave Martin <dave.martin@linaro.org>
> ---
> arch/arm/mm/proc-macros.S | 79 +++++++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 79 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mm/proc-macros.S b/arch/arm/mm/proc-macros.S
> index 34261f9..3c5ffbb 100644
> --- a/arch/arm/mm/proc-macros.S
> +++ b/arch/arm/mm/proc-macros.S
> @@ -254,3 +255,81 @@
> mcr p15, 0, r0, c7, c10, 1 @ clean L1 D line
> mcr p15, 0, ip, c7, c10, 4 @ data write barrier
> .endm
> +
> +.macro define_processor_functions name:req, dabort:req, pabort:req, nommu=0, suspend=0
> + .pushsection .text
> +
> + __INITDATA
Was this __INITDATA right after a .pushsection intentional? One might
be confused about the effective section after this.
The __INITDATA tag is actually doing a
.section ".init.data","aw",%progbits
Maybe this could be used as argument to .pushsection instead of .text
(which in this case should probably have been .rodata otherwise anyway).
Nicolas
^ permalink raw reply [flat|nested] 15+ messages in thread
* [RFC PATCH v2 1/5] ARM: mm: Add generic proc/arch struct definition macros
2011-06-15 23:57 ` Nicolas Pitre
@ 2011-06-16 10:03 ` Dave Martin
2011-06-20 3:13 ` Nicolas Pitre
0 siblings, 1 reply; 15+ messages in thread
From: Dave Martin @ 2011-06-16 10:03 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Jun 15, 2011 at 07:57:13PM -0400, Nicolas Pitre wrote:
> On Tue, 14 Jun 2011, Dave Martin wrote:
>
> > This patch adds some generic macros to reduce boilerplate when
> > declaring certain common structures in arch/arm/mm/*.S
> >
> > Thanks to Russell King for outlining what the
> > define_processor_functions macro could look like.
> >
> > Signed-off-by: Dave Martin <dave.martin@linaro.org>
> > ---
> > arch/arm/mm/proc-macros.S | 79 +++++++++++++++++++++++++++++++++++++++++++++
> > 1 files changed, 79 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/arm/mm/proc-macros.S b/arch/arm/mm/proc-macros.S
> > index 34261f9..3c5ffbb 100644
> > --- a/arch/arm/mm/proc-macros.S
> > +++ b/arch/arm/mm/proc-macros.S
> > @@ -254,3 +255,81 @@
> > mcr p15, 0, r0, c7, c10, 1 @ clean L1 D line
> > mcr p15, 0, ip, c7, c10, 4 @ data write barrier
> > .endm
> > +
> > +.macro define_processor_functions name:req, dabort:req, pabort:req, nommu=0, suspend=0
> > + .pushsection .text
> > +
> > + __INITDATA
>
> Was this __INITDATA right after a .pushsection intentional? One might
> be confused about the effective section after this.
The .pushsection is needed in order to save the previous state of the
section stack so that it can be restored later with .popsection.
.pushsection syntactically requires an argument, though in this case it's
not useful, so I just specify .text since this should be harmless.
This is kind of unclear, so at the very least it could benefit from a
comment.
>
> The __INITDATA tag is actually doing a
>
> .section ".init.data","aw",%progbits
>
> Maybe this could be used as argument to .pushsection instead of .text
> (which in this case should probably have been .rodata otherwise anyway).
The problem is that __INITDATA conflates the information about the section
with the action of actually switching to the section. Nowhere are the
section name and flags available by themselves. Since the point
of a macro is to avoid copy-paste, I prefer not to manually copy-paste
this stuff into a .pushsection directive in a random assembler file...
Perhaps we should instead use
__INITDATA
/* ... */
.previous
.endm
I tend to avoid ".previous" because I consider it somewhat broken:
in the presence of macros, the section restored by .previous is
indeterminate because a called macro may switch sections and hence
change what .previous will refer to. There is no way to restore what
.previous points at after switching sections.
In this case, we don't call any nested macro, so .previous _should_
refer to the right thing. We mess up .previous for the caller, but
then we always mess that up when switching sections anyway. Nothing
in arch/arm/mm/*.S uses .previous, so we shouldn't get any problems
resulting from this, in this particular case.
Alternatively, we could propose a change <linux/init.h> to split the
__INITDATA macro up, and define the related macros in a sane way:
#define __INITDATA_NAME .init.data
#define __INITDATA_FLAGS "aw",%progbtis
#define __initdata __section(__INITDATA_NAME)
#define __INITDATA .section __stringify(__INITDATA_NAME),__INITDATA_FLAGS
Any thoughts on that?
Cheers
---Dave
^ permalink raw reply [flat|nested] 15+ messages in thread
* [RFC PATCH v2 0/5] Add generic macros for declaring various CPU structs
2011-06-14 10:58 [RFC PATCH v2 0/5] Add generic macros for declaring various CPU structs Dave Martin
` (4 preceding siblings ...)
2011-06-14 10:58 ` [RFC PATCH v2 5/5] ARM: cache-v6: " Dave Martin
@ 2011-06-16 10:12 ` Will Deacon
2011-06-16 10:15 ` Russell King - ARM Linux
2011-06-16 10:43 ` Dave Martin
5 siblings, 2 replies; 15+ messages in thread
From: Will Deacon @ 2011-06-16 10:12 UTC (permalink / raw)
To: linux-arm-kernel
Hi Dave,
On Tue, Jun 14, 2011 at 11:58:20AM +0100, Dave Martin wrote:
> Based on previous discussions, this series provides an alternative
> macro for defining CPU-specific structures compactly.
>
> I don't get to have so much fun with macros in this version, but
> it's more straightforward and actually allows the declarations to
> be collapsed down further by taking advantage of common naming
> conventions.
>
> In this RFC, only the arch names, processor_functions and
> cache_functions are turned into macros, to show how this could
> work. This could be straightforwardly extended to cover tlb_fns,
> but proc_info is more complex and may require a bit more thought.
In general, this looks good to me and I'm happy to rebase my A5/A15 core
patches on top of this series.
> * For consistency, I've renamed the arch/CPU name string labels.
> If that is seen as unnecessary churn, it can be undone.
I don't see the win here, so let's leave the names like they are to avoid
unnecessary conflicts with other patches dealing with proc_info structs.
Will
^ permalink raw reply [flat|nested] 15+ messages in thread
* [RFC PATCH v2 2/5] ARM: proc-v7: Use new generic struct definition macros
2011-06-14 10:58 ` [RFC PATCH v2 2/5] ARM: proc-v7: Use new generic " Dave Martin
@ 2011-06-16 10:15 ` Will Deacon
0 siblings, 0 replies; 15+ messages in thread
From: Will Deacon @ 2011-06-16 10:15 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Jun 14, 2011 at 11:58:22AM +0100, Dave Martin wrote:
> diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S
> index b3b566e..c5cf6ea 100644
> --- a/arch/arm/mm/proc-v7.S
> +++ b/arch/arm/mm/proc-v7.S
> @@ -171,8 +171,6 @@ ENTRY(cpu_v7_set_pte_ext)
> mov pc, lr
> ENDPROC(cpu_v7_set_pte_ext)
>
> -cpu_v7_name:
> - .ascii "ARMv7 Processor"
> .align
>
> /*
> @@ -403,36 +401,8 @@ v7_crval:
> __v7_setup_stack:
> .space 4 * 11 @ 11 registers
>
> - __INITDATA
> -
> - .type v7_processor_functions, #object
> -ENTRY(v7_processor_functions)
> - .word v7_early_abort
> - .word v7_pabort
> - .word cpu_v7_proc_init
> - .word cpu_v7_proc_fin
> - .word cpu_v7_reset
> - .word cpu_v7_do_idle
> - .word cpu_v7_dcache_clean_area
> - .word cpu_v7_switch_mm
> - .word cpu_v7_set_pte_ext
> - .word 0
> - .word 0
> - .word 0
> - .size v7_processor_functions, . - v7_processor_functions
> -
> - .section ".rodata"
> -
> - .type cpu_arch_name, #object
> -cpu_arch_name:
> - .asciz "armv7"
> - .size cpu_arch_name, . - cpu_arch_name
> -
> - .type cpu_elf_name, #object
> -cpu_elf_name:
> - .asciz "v7"
> - .size cpu_elf_name, . - cpu_elf_name
> - .align
> + define_processor_functions v7, dabort=v7_early, pabort=v7
> + define_proc_names v7, "armv7", "v7", "ARMv7 Processor"
It's not required, but it might be nice to use named arguments here anyway
since v7, "armv7", "v7" and "ARMv7 Processor" are all quite similar and it
would be easier to read if it's explicitly stated what's what.
Will
^ permalink raw reply [flat|nested] 15+ messages in thread
* [RFC PATCH v2 0/5] Add generic macros for declaring various CPU structs
2011-06-16 10:12 ` [RFC PATCH v2 0/5] Add generic macros for declaring various CPU structs Will Deacon
@ 2011-06-16 10:15 ` Russell King - ARM Linux
2011-06-16 10:34 ` Dave Martin
2011-06-16 10:43 ` Dave Martin
1 sibling, 1 reply; 15+ messages in thread
From: Russell King - ARM Linux @ 2011-06-16 10:15 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Jun 16, 2011 at 11:12:41AM +0100, Will Deacon wrote:
> > * For consistency, I've renamed the arch/CPU name string labels.
> > If that is seen as unnecessary churn, it can be undone.
>
> I don't see the win here, so let's leave the names like they are to avoid
> unnecessary conflicts with other patches dealing with proc_info structs.
One of the issues is that there are proc-* files which use the same
strings for several entries, for example, proc-xscale.S.
In general, the ELF name and arch name should be the same across all,
just the CPU name should differ.
^ permalink raw reply [flat|nested] 15+ messages in thread
* [RFC PATCH v2 0/5] Add generic macros for declaring various CPU structs
2011-06-16 10:15 ` Russell King - ARM Linux
@ 2011-06-16 10:34 ` Dave Martin
0 siblings, 0 replies; 15+ messages in thread
From: Dave Martin @ 2011-06-16 10:34 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Jun 16, 2011 at 11:15:36AM +0100, Russell King - ARM Linux wrote:
> On Thu, Jun 16, 2011 at 11:12:41AM +0100, Will Deacon wrote:
> > > * For consistency, I've renamed the arch/CPU name string labels.
> > > If that is seen as unnecessary churn, it can be undone.
> >
> > I don't see the win here, so let's leave the names like they are to avoid
> > unnecessary conflicts with other patches dealing with proc_info structs.
>
> One of the issues is that there are proc-* files which use the same
> strings for several entries, for example, proc-xscale.S.
>
> In general, the ELF name and arch name should be the same across all,
> just the CPU name should differ.
Hmmm, that explains why those strings were declared a bit differently.
A few of possible solutions to this:
1) Keep my macros as they are, but use a couple of #defines to make sure
that arch_name and cpu_name are the same everywhere in a given
proc-*.S file. Each cpu will get its own symbol for each of these
strings, but the strings will be merged by the linker and so won't
be duplicated in the kernel image.
2) Split define_proc_names into two macros, say, define_arch_names and
define_cpu_name. define_arch_names would be used once in the
file.
3) Define a generic macro to define a string. That would help automate
string declarations, but really it would no longer be arch-
specific and could live outside arch/arm.
4) Get rid of the define_proc_names macro altogether, and declare those
things in the existing way.
Any view on these approaches?
Cheers
---Dave
^ permalink raw reply [flat|nested] 15+ messages in thread
* [RFC PATCH v2 0/5] Add generic macros for declaring various CPU structs
2011-06-16 10:12 ` [RFC PATCH v2 0/5] Add generic macros for declaring various CPU structs Will Deacon
2011-06-16 10:15 ` Russell King - ARM Linux
@ 2011-06-16 10:43 ` Dave Martin
1 sibling, 0 replies; 15+ messages in thread
From: Dave Martin @ 2011-06-16 10:43 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Jun 16, 2011 at 11:12:41AM +0100, Will Deacon wrote:
> Hi Dave,
>
> On Tue, Jun 14, 2011 at 11:58:20AM +0100, Dave Martin wrote:
> > Based on previous discussions, this series provides an alternative
> > macro for defining CPU-specific structures compactly.
> >
> > I don't get to have so much fun with macros in this version, but
> > it's more straightforward and actually allows the declarations to
> > be collapsed down further by taking advantage of common naming
> > conventions.
> >
> > In this RFC, only the arch names, processor_functions and
> > cache_functions are turned into macros, to show how this could
> > work. This could be straightforwardly extended to cover tlb_fns,
> > but proc_info is more complex and may require a bit more thought.
>
> In general, this looks good to me and I'm happy to rebase my A5/A15 core
> patches on top of this series.
>
> > * For consistency, I've renamed the arch/CPU name string labels.
> > If that is seen as unnecessary churn, it can be undone.
>
> I don't see the win here, so let's leave the names like they are to avoid
> unnecessary conflicts with other patches dealing with proc_info structs.
Agreed:
Having thought about this, and taking into account Russell's comments
about problems with this approach in proc-*.S files declaring multiple
CPUs, we should revert to not renaming any of those labels.
I'll follow up with updated patches after Russell has given his view,
but assume for now that the proc_info struct contents themselves
won't need to change.
Cheers
---Dave
^ permalink raw reply [flat|nested] 15+ messages in thread
* [RFC PATCH v2 1/5] ARM: mm: Add generic proc/arch struct definition macros
2011-06-16 10:03 ` Dave Martin
@ 2011-06-20 3:13 ` Nicolas Pitre
2011-06-20 10:56 ` Dave Martin
0 siblings, 1 reply; 15+ messages in thread
From: Nicolas Pitre @ 2011-06-20 3:13 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, 16 Jun 2011, Dave Martin wrote:
> Alternatively, we could propose a change <linux/init.h> to split the
> __INITDATA macro up, and define the related macros in a sane way:
>
> #define __INITDATA_NAME .init.data
> #define __INITDATA_FLAGS "aw",%progbtis
> #define __initdata __section(__INITDATA_NAME)
> #define __INITDATA .section __stringify(__INITDATA_NAME),__INITDATA_FLAGS
>
> Any thoughts on that?
I do like it.
Nicolas
^ permalink raw reply [flat|nested] 15+ messages in thread
* [RFC PATCH v2 1/5] ARM: mm: Add generic proc/arch struct definition macros
2011-06-20 3:13 ` Nicolas Pitre
@ 2011-06-20 10:56 ` Dave Martin
0 siblings, 0 replies; 15+ messages in thread
From: Dave Martin @ 2011-06-20 10:56 UTC (permalink / raw)
To: linux-arm-kernel
On Sun, Jun 19, 2011 at 11:13:46PM -0400, Nicolas Pitre wrote:
> On Thu, 16 Jun 2011, Dave Martin wrote:
>
> > Alternatively, we could propose a change <linux/init.h> to split the
> > __INITDATA macro up, and define the related macros in a sane way:
> >
> > #define __INITDATA_NAME .init.data
> > #define __INITDATA_FLAGS "aw",%progbtis
> > #define __initdata __section(__INITDATA_NAME)
> > #define __INITDATA .section __stringify(__INITDATA_NAME),__INITDATA_FLAGS
> >
> > Any thoughts on that?
>
> I do like it.
I think that all this data (the cache, tlb and processor funcs tables, and
the name strings) should theoretically go in __INITRODATA, since it is not
written and is only referenced via .proc.info.init so is not findable after
init data has been discarded anyway. This data is all copied by setup.c
during startup, AFAICT.
However, the current section assignment situation is somewhat
inconsistent.
* processor_functions, cache_fns: __INITDATA (writable)
* tlb_fns: various (tlb-v3.S has __INITDATA; tlb-v7.S has __INIT)
* name strings: .rodata (not initdata)
I don't really understand how this stuff interacts with CPU hotplug however;
maybe this stuff should all be in __CPUINITRODATA instead.
Since I will likely patch all the proc-*.S and cache-*.S files, I will
try to avoid any potential breakage in the initial series; so I'll keep
the section directives out of the macros so that we can prove in an
automated way that there is zero change the generated binaries.
This still brings much of the benefits.
Moving the section directives in the macros could possibly occur as a
separate follow-up.
Cheers
---Dave
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2011-06-20 10:56 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-06-14 10:58 [RFC PATCH v2 0/5] Add generic macros for declaring various CPU structs Dave Martin
2011-06-14 10:58 ` [RFC PATCH v2 1/5] ARM: mm: Add generic proc/arch struct definition macros Dave Martin
2011-06-15 23:57 ` Nicolas Pitre
2011-06-16 10:03 ` Dave Martin
2011-06-20 3:13 ` Nicolas Pitre
2011-06-20 10:56 ` Dave Martin
2011-06-14 10:58 ` [RFC PATCH v2 2/5] ARM: proc-v7: Use new generic " Dave Martin
2011-06-16 10:15 ` Will Deacon
2011-06-14 10:58 ` [RFC PATCH v2 3/5] ARM: cache-v7: " Dave Martin
2011-06-14 10:58 ` [RFC PATCH v2 4/5] ARM: proc-v6: " Dave Martin
2011-06-14 10:58 ` [RFC PATCH v2 5/5] ARM: cache-v6: " Dave Martin
2011-06-16 10:12 ` [RFC PATCH v2 0/5] Add generic macros for declaring various CPU structs Will Deacon
2011-06-16 10:15 ` Russell King - ARM Linux
2011-06-16 10:34 ` Dave Martin
2011-06-16 10:43 ` Dave Martin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).