* [PATCH 1/4] powerpc: Make BUG_ON & WARN_ON play nice with compile-time optimisations
@ 2006-03-23 12:32 Michael Ellerman
2006-03-23 12:33 ` [PATCH 2/4] powerpc: Change firmware_has_feature() to a macro Michael Ellerman
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Michael Ellerman @ 2006-03-23 12:32 UTC (permalink / raw)
To: Paul Mackerras, linuxppc-dev
Change BUG_ON and WARN_ON to give the compiler a chance to perform
compile-time optimsations. Depending on the complexity of the condition,
the compiler may not do this very well, so if it's important check the
object code.
Current GCC's (4.x) produce good code as long as the condition does not
include a function call, including a static inline.
Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
include/asm-powerpc/bug.h | 30 ++++++++++++++++++++++++++++--
1 files changed, 28 insertions(+), 2 deletions(-)
Index: to-merge/include/asm-powerpc/bug.h
===================================================================
--- to-merge.orig/include/asm-powerpc/bug.h
+++ to-merge/include/asm-powerpc/bug.h
@@ -30,6 +30,12 @@ struct bug_entry *find_bug(unsigned long
#ifdef CONFIG_BUG
+/*
+ * BUG_ON() and WARN_ON() do their best to cooperate with compile-time
+ * optimisations. However depending on the complexity of the condition
+ * some compiler versions may not produce optimal results.
+ */
+
#define BUG() do { \
__asm__ __volatile__( \
"1: twi 31,0,0\n" \
@@ -40,17 +46,36 @@ struct bug_entry *find_bug(unsigned long
} while (0)
#define BUG_ON(x) do { \
- __asm__ __volatile__( \
+ if (__builtin_constant_p(x)) { \
+ if (x) \
+ BUG(); \
+ } else { \
+ __asm__ __volatile__( \
"1: "PPC_TLNEI" %0,0\n" \
".section __bug_table,\"a\"\n" \
"\t"PPC_LONG" 1b,%1,%2,%3\n" \
".previous" \
: : "r" ((long)(x)), "i" (__LINE__), \
"i" (__FILE__), "i" (__FUNCTION__)); \
+ } \
} while (0)
-#define WARN_ON(x) do { \
+#define WARN() do { \
__asm__ __volatile__( \
+ "1: twi 31,0,0\n" \
+ ".section __bug_table,\"a\"\n" \
+ "\t"PPC_LONG" 1b,%0,%1,%2\n" \
+ ".previous" \
+ : : "i" (__LINE__ + BUG_WARNING_TRAP), \
+ "i" (__FILE__), "i" (__FUNCTION__)); \
+} while (0)
+
+#define WARN_ON(x) do { \
+ if (__builtin_constant_p(x)) { \
+ if (x) \
+ WARN(); \
+ } else { \
+ __asm__ __volatile__( \
"1: "PPC_TLNEI" %0,0\n" \
".section __bug_table,\"a\"\n" \
"\t"PPC_LONG" 1b,%1,%2,%3\n" \
@@ -58,6 +83,7 @@ struct bug_entry *find_bug(unsigned long
: : "r" ((long)(x)), \
"i" (__LINE__ + BUG_WARNING_TRAP), \
"i" (__FILE__), "i" (__FUNCTION__)); \
+ } \
} while (0)
#define HAVE_ARCH_BUG
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 2/4] powerpc: Change firmware_has_feature() to a macro 2006-03-23 12:32 [PATCH 1/4] powerpc: Make BUG_ON & WARN_ON play nice with compile-time optimisations Michael Ellerman @ 2006-03-23 12:33 ` Michael Ellerman 2006-03-23 12:33 ` [PATCH 3/4] powerpc: Rename and export ppc64_firmware_features Michael Ellerman 2006-03-23 12:33 ` [PATCH 4/4] powerpc: Cope with duplicate node & property names in /proc/device-tree Michael Ellerman 2 siblings, 0 replies; 6+ messages in thread From: Michael Ellerman @ 2006-03-23 12:33 UTC (permalink / raw) To: Paul Mackerras, linuxppc-dev So that we can use firmware_has_feature() in a BUG_ON() and have the compiler elide the code entirely if the feature can never be set, change firmware_has_feature to a macro. Unfortunate, but necessary at least until GCC bug #26724 is fixed. Signed-off-by: Michael Ellerman <michael@ellerman.id.au> --- include/asm-powerpc/firmware.h | 8 +++----- 1 files changed, 3 insertions(+), 5 deletions(-) Index: to-merge/include/asm-powerpc/firmware.h =================================================================== --- to-merge.orig/include/asm-powerpc/firmware.h +++ to-merge/include/asm-powerpc/firmware.h @@ -84,11 +84,9 @@ enum { */ extern unsigned long ppc64_firmware_features; -static inline unsigned long firmware_has_feature(unsigned long feature) -{ - return (FW_FEATURE_ALWAYS & feature) || - (FW_FEATURE_POSSIBLE & ppc64_firmware_features & feature); -} +#define firmware_has_feature(feature) \ + ((FW_FEATURE_ALWAYS & (feature)) || \ + (FW_FEATURE_POSSIBLE & ppc64_firmware_features & (feature))) extern void system_reset_fwnmi(void); extern void machine_check_fwnmi(void); ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 3/4] powerpc: Rename and export ppc64_firmware_features 2006-03-23 12:32 [PATCH 1/4] powerpc: Make BUG_ON & WARN_ON play nice with compile-time optimisations Michael Ellerman 2006-03-23 12:33 ` [PATCH 2/4] powerpc: Change firmware_has_feature() to a macro Michael Ellerman @ 2006-03-23 12:33 ` Michael Ellerman 2006-03-23 13:17 ` Stephen Rothwell 2006-03-23 12:33 ` [PATCH 4/4] powerpc: Cope with duplicate node & property names in /proc/device-tree Michael Ellerman 2 siblings, 1 reply; 6+ messages in thread From: Michael Ellerman @ 2006-03-23 12:33 UTC (permalink / raw) To: Paul Mackerras, linuxppc-dev We need to export ppc64_firmware_features for modules. Before we do that I think we should probably rename it to powerpc_firmware_features. Signed-off-by: Michael Ellerman <michael@ellerman.id.au> --- arch/powerpc/kernel/firmware.c | 2 +- arch/powerpc/platforms/iseries/setup.c | 4 ++-- arch/powerpc/platforms/pseries/firmware.c | 2 +- arch/powerpc/platforms/pseries/setup.c | 2 +- include/asm-powerpc/firmware.h | 5 +++-- 5 files changed, 8 insertions(+), 7 deletions(-) Index: to-merge/arch/powerpc/kernel/firmware.c =================================================================== --- to-merge.orig/arch/powerpc/kernel/firmware.c +++ to-merge/arch/powerpc/kernel/firmware.c @@ -17,4 +17,4 @@ #include <asm/firmware.h> -unsigned long ppc64_firmware_features; +unsigned long powerpc_firmware_features; Index: to-merge/arch/powerpc/platforms/iseries/setup.c =================================================================== --- to-merge.orig/arch/powerpc/platforms/iseries/setup.c +++ to-merge/arch/powerpc/platforms/iseries/setup.c @@ -680,8 +680,8 @@ static int __init iseries_probe(int plat if (PLATFORM_ISERIES_LPAR != platform) return 0; - ppc64_firmware_features |= FW_FEATURE_ISERIES; - ppc64_firmware_features |= FW_FEATURE_LPAR; + powerpc_firmware_features |= FW_FEATURE_ISERIES; + powerpc_firmware_features |= FW_FEATURE_LPAR; return 1; } Index: to-merge/arch/powerpc/platforms/pseries/firmware.c =================================================================== --- to-merge.orig/arch/powerpc/platforms/pseries/firmware.c +++ to-merge/arch/powerpc/platforms/pseries/firmware.c @@ -91,7 +91,7 @@ void __init fw_feature_init(void) continue; /* we have a match */ - ppc64_firmware_features |= + powerpc_firmware_features |= firmware_features_table[i].val; break; } Index: to-merge/arch/powerpc/platforms/pseries/setup.c =================================================================== --- to-merge.orig/arch/powerpc/platforms/pseries/setup.c +++ to-merge/arch/powerpc/platforms/pseries/setup.c @@ -386,7 +386,7 @@ static int __init pSeries_probe(int plat */ if (platform == PLATFORM_PSERIES_LPAR) - ppc64_firmware_features |= FW_FEATURE_LPAR; + powerpc_firmware_features |= FW_FEATURE_LPAR; return 1; } Index: to-merge/include/asm-powerpc/firmware.h =================================================================== --- to-merge.orig/include/asm-powerpc/firmware.h +++ to-merge/include/asm-powerpc/firmware.h @@ -82,11 +82,12 @@ enum { /* This is used to identify firmware features which are available * to the kernel. */ -extern unsigned long ppc64_firmware_features; +extern unsigned long powerpc_firmware_features; +EXPORT_SYMBOL_GPL(powerpc_firmware_features); #define firmware_has_feature(feature) \ ((FW_FEATURE_ALWAYS & (feature)) || \ - (FW_FEATURE_POSSIBLE & ppc64_firmware_features & (feature))) + (FW_FEATURE_POSSIBLE & powerpc_firmware_features & (feature))) extern void system_reset_fwnmi(void); extern void machine_check_fwnmi(void); ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 3/4] powerpc: Rename and export ppc64_firmware_features 2006-03-23 12:33 ` [PATCH 3/4] powerpc: Rename and export ppc64_firmware_features Michael Ellerman @ 2006-03-23 13:17 ` Stephen Rothwell 2006-03-23 23:40 ` [PATCH] " Michael Ellerman 0 siblings, 1 reply; 6+ messages in thread From: Stephen Rothwell @ 2006-03-23 13:17 UTC (permalink / raw) To: Michael Ellerman; +Cc: linuxppc-dev, paulus [-- Attachment #1: Type: text/plain, Size: 298 bytes --] On Thu, 23 Mar 2006 23:33:11 +1100 Michael Ellerman <michael@ellerman.id.au> wrote: > > +EXPORT_SYMBOL_GPL(powerpc_firmware_features); This should be in firmware.c not a header file. -- Cheers, Stephen Rothwell sfr@canb.auug.org.au http://www.canb.auug.org.au/~sfr/ [-- Attachment #2: Type: application/pgp-signature, Size: 191 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] powerpc: Rename and export ppc64_firmware_features 2006-03-23 13:17 ` Stephen Rothwell @ 2006-03-23 23:40 ` Michael Ellerman 0 siblings, 0 replies; 6+ messages in thread From: Michael Ellerman @ 2006-03-23 23:40 UTC (permalink / raw) To: Paul Mackerras; +Cc: linuxppc-dev, Stephen Rothwell We need to export ppc64_firmware_features for modules. Before we do that I think we should probably rename it to powerpc_firmware_features. Signed-off-by: Michael Ellerman <michael@ellerman.id.au> --- arch/powerpc/kernel/firmware.c | 3 ++- arch/powerpc/platforms/iseries/setup.c | 4 ++-- arch/powerpc/platforms/pseries/firmware.c | 2 +- arch/powerpc/platforms/pseries/setup.c | 2 +- include/asm-powerpc/firmware.h | 4 ++-- 5 files changed, 8 insertions(+), 7 deletions(-) Index: to-merge/arch/powerpc/kernel/firmware.c =================================================================== --- to-merge.orig/arch/powerpc/kernel/firmware.c +++ to-merge/arch/powerpc/kernel/firmware.c @@ -17,4 +17,5 @@ #include <asm/firmware.h> -unsigned long ppc64_firmware_features; +unsigned long powerpc_firmware_features; +EXPORT_SYMBOL_GPL(powerpc_firmware_features); Index: to-merge/arch/powerpc/platforms/iseries/setup.c =================================================================== --- to-merge.orig/arch/powerpc/platforms/iseries/setup.c +++ to-merge/arch/powerpc/platforms/iseries/setup.c @@ -680,8 +680,8 @@ static int __init iseries_probe(int plat if (PLATFORM_ISERIES_LPAR != platform) return 0; - ppc64_firmware_features |= FW_FEATURE_ISERIES; - ppc64_firmware_features |= FW_FEATURE_LPAR; + powerpc_firmware_features |= FW_FEATURE_ISERIES; + powerpc_firmware_features |= FW_FEATURE_LPAR; return 1; } Index: to-merge/arch/powerpc/platforms/pseries/firmware.c =================================================================== --- to-merge.orig/arch/powerpc/platforms/pseries/firmware.c +++ to-merge/arch/powerpc/platforms/pseries/firmware.c @@ -91,7 +91,7 @@ void __init fw_feature_init(void) continue; /* we have a match */ - ppc64_firmware_features |= + powerpc_firmware_features |= firmware_features_table[i].val; break; } Index: to-merge/arch/powerpc/platforms/pseries/setup.c =================================================================== --- to-merge.orig/arch/powerpc/platforms/pseries/setup.c +++ to-merge/arch/powerpc/platforms/pseries/setup.c @@ -386,7 +386,7 @@ static int __init pSeries_probe(int plat */ if (platform == PLATFORM_PSERIES_LPAR) - ppc64_firmware_features |= FW_FEATURE_LPAR; + powerpc_firmware_features |= FW_FEATURE_LPAR; return 1; } Index: to-merge/include/asm-powerpc/firmware.h =================================================================== --- to-merge.orig/include/asm-powerpc/firmware.h +++ to-merge/include/asm-powerpc/firmware.h @@ -82,11 +82,11 @@ enum { /* This is used to identify firmware features which are available * to the kernel. */ -extern unsigned long ppc64_firmware_features; +extern unsigned long powerpc_firmware_features; #define firmware_has_feature(feature) \ ((FW_FEATURE_ALWAYS & (feature)) || \ - (FW_FEATURE_POSSIBLE & ppc64_firmware_features & (feature))) + (FW_FEATURE_POSSIBLE & powerpc_firmware_features & (feature))) extern void system_reset_fwnmi(void); extern void machine_check_fwnmi(void); ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 4/4] powerpc: Cope with duplicate node & property names in /proc/device-tree 2006-03-23 12:32 [PATCH 1/4] powerpc: Make BUG_ON & WARN_ON play nice with compile-time optimisations Michael Ellerman 2006-03-23 12:33 ` [PATCH 2/4] powerpc: Change firmware_has_feature() to a macro Michael Ellerman 2006-03-23 12:33 ` [PATCH 3/4] powerpc: Rename and export ppc64_firmware_features Michael Ellerman @ 2006-03-23 12:33 ` Michael Ellerman 2 siblings, 0 replies; 6+ messages in thread From: Michael Ellerman @ 2006-03-23 12:33 UTC (permalink / raw) To: Paul Mackerras, linuxppc-dev Various dodgy firmware might give us nodes and/or properties in the device tree with conflicting names. That's generally ok, except for when we export the device tree via /proc, so check when we're creating the proc device tree and munge names accordingly. Tested on a faked device tree with kexec, would be good if someone with actual bogus firmware could try it, but just for completeness. Signed-off-by: Michael Ellerman <michael@ellerman.id.au> --- fs/proc/proc_devtree.c | 93 +++++++++++++++++++++++++++++++++++++------------ 1 files changed, 72 insertions(+), 21 deletions(-) Index: to-merge/fs/proc/proc_devtree.c =================================================================== --- to-merge.orig/fs/proc/proc_devtree.c +++ to-merge/fs/proc/proc_devtree.c @@ -52,7 +52,8 @@ static int property_read_proc(char *page * Add a property to a node */ static struct proc_dir_entry * -__proc_device_tree_add_prop(struct proc_dir_entry *de, struct property *pp) +__proc_device_tree_add_prop(struct proc_dir_entry *de, struct property *pp, + char *name) { struct proc_dir_entry *ent; @@ -60,14 +61,14 @@ __proc_device_tree_add_prop(struct proc_ * Unfortunately proc_register puts each new entry * at the beginning of the list. So we rearrange them. */ - ent = create_proc_read_entry(pp->name, - strncmp(pp->name, "security-", 9) + ent = create_proc_read_entry(name, + strncmp(name, "security-", 9) ? S_IRUGO : S_IRUSR, de, property_read_proc, pp); if (ent == NULL) return NULL; - if (!strncmp(pp->name, "security-", 9)) + if (!strncmp(name, "security-", 9)) ent->size = 0; /* don't leak number of password chars */ else ent->size = pp->length; @@ -78,7 +79,7 @@ __proc_device_tree_add_prop(struct proc_ void proc_device_tree_add_prop(struct proc_dir_entry *pde, struct property *prop) { - __proc_device_tree_add_prop(pde, prop); + __proc_device_tree_add_prop(pde, prop, prop->name); } void proc_device_tree_remove_prop(struct proc_dir_entry *pde, @@ -106,6 +107,61 @@ void proc_device_tree_update_prop(struct } /* + * Various dodgy firmware might give us nodes and/or properties with + * conflicting names. That's generally ok, except for exporting via /proc, + * so munge names here to ensure they're unique. + */ + +static int duplicate_name(struct proc_dir_entry *de, char *name) +{ + struct proc_dir_entry *ent; + + for (ent = de->subdir; ent != NULL; ent = ent->next) + if (strcmp(ent->name, name) == 0) + return 1; + + return 0; +} + +static char *fixup_name(struct device_node *np, struct proc_dir_entry *de, + char *name) +{ + char *fixed_name; + int fixup_len = strlen(name) + 2 + 1; /* name + #x + \0 */ + int i = 1, size; + +realloc: + fixed_name = kmalloc(fixup_len, GFP_KERNEL); + if (fixed_name == NULL) { + printk(KERN_ERR "device-tree: Out of memory trying to fixup " + "name \"%s\"\n", name); + return name; + } + +retry: + size = snprintf(fixed_name, fixup_len, "%s#%d", name, i); + size++; /* account for NULL */ + + if (size > fixup_len) { + /* We ran out of space, free and reallocate. */ + kfree(fixed_name); + fixup_len = size; + goto realloc; + } + + if (duplicate_name(de, fixed_name)) { + /* Multiple duplicates. Retry with a different offset. */ + i++; + goto retry; + } + + printk(KERN_WARNING "device-tree: Duplicate name in %s, " + "renamed to \"%s\"\n", np->full_name, fixed_name); + + return fixed_name; +} + +/* * Process a node, adding entries for its children and its properties. */ void proc_device_tree_add_node(struct device_node *np, @@ -118,35 +174,30 @@ void proc_device_tree_add_node(struct de set_node_proc_entry(np, de); for (child = NULL; (child = of_get_next_child(np, child));) { + /* Use everything after the last slash, or the full name */ p = strrchr(child->full_name, '/'); if (!p) p = child->full_name; else ++p; + + if (duplicate_name(de, p)) + p = fixup_name(np, de, p); + ent = proc_mkdir(p, de); if (ent == 0) break; proc_device_tree_add_node(child, ent); } of_node_put(child); + for (pp = np->properties; pp != 0; pp = pp->next) { - /* - * Yet another Apple device-tree bogosity: on some machines, - * they have properties & nodes with the same name. Those - * properties are quite unimportant for us though, thus we - * simply "skip" them here, but we do have to check. - */ - for (ent = de->subdir; ent != NULL; ent = ent->next) - if (!strcmp(ent->name, pp->name)) - break; - if (ent != NULL) { - printk(KERN_WARNING "device-tree: property \"%s\" name" - " conflicts with node in %s\n", pp->name, - np->full_name); - continue; - } + p = pp->name; + + if (duplicate_name(de, p)) + p = fixup_name(np, de, p); - ent = __proc_device_tree_add_prop(de, pp); + ent = __proc_device_tree_add_prop(de, pp, p); if (ent == 0) break; } ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2006-03-23 23:40 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2006-03-23 12:32 [PATCH 1/4] powerpc: Make BUG_ON & WARN_ON play nice with compile-time optimisations Michael Ellerman 2006-03-23 12:33 ` [PATCH 2/4] powerpc: Change firmware_has_feature() to a macro Michael Ellerman 2006-03-23 12:33 ` [PATCH 3/4] powerpc: Rename and export ppc64_firmware_features Michael Ellerman 2006-03-23 13:17 ` Stephen Rothwell 2006-03-23 23:40 ` [PATCH] " Michael Ellerman 2006-03-23 12:33 ` [PATCH 4/4] powerpc: Cope with duplicate node & property names in /proc/device-tree Michael Ellerman
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).