Re: [PATCH] Open Firmware device tree virtual filesystem

From: Segher Boessenkool
Date: Sun Dec 31 2006 - 23:23:53 EST

Some comments, mostly coding style:

- 0xb0 - 0x13f Free. Add more parameters here if you really need them.
+ 0xb0 16 bytes Open Firmware information (magic, version, callback, idt)

Is there an OF ISA binding for x86 somewhere? And don't
point me to the source code, I'd like to see an actual
reference doc ;-)

+// printk(KERN_WARNING "CALLOFW: %s\n", name);

Please remove disabled code. And don't use // comments
at all please.

+ if (call_firmware == NULL)
+ return (-1);

No parentheses around return value.

+ argarray[0] = (int)name;

This would warn on 64-bit systems, better write it as
(u32)(u64)name (and make sure that "name" is somewhere
in the low 4GB of memory). This doesn't handle 64-bit
client interface either btw.

+ while (numargs--) {
+ argarray[argnum++] = va_arg(ap, int);
+ }

No braces around single statements.

+#undef MAXARGS

Why this #undef? That's nasty style, and this is at the
end of file anyway.

+#if 0

Better use a normal comment.

+Here are call templates for all the standard OFW client services.

You missed "instance-to-interposed-path" (standard, although
not required).

+ * By Mitch Bradley (wmb@xxxxxxxxxxxxx), with assistance from David Kahn.
+ * Most of the basic virtual file system structure was taken from a
+ * "promfs" example written by Arnd Bergmann. + *

My mailer messed up this line, that means you have trailing
spaces here :-)

+ + if (root == 0) {

Same here.

+++ b/include/asm-i386/callofw.h
@@ -0,0 +1,22 @@
+#ifndef _I386_PROM_H
+#define _I386_PROM_H

Better make the #define correspond to the file name.


To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at