Re: [PATCH] Open Firmware device tree virtual filesystem

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 ([email protected]), 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.


Segher

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux