linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [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

* [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

* 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

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).