Re: [2.6 patch] make drivers/mtd/cmdlinepart.c:mtdpart_setup() static
From: Juha Yrjölä
Date: Sun Jul 09 2006 - 12:11:01 EST
David Woodhouse wrote:
--- linux-2.6.17-mm2-full/drivers/mtd/cmdlinepart.c.old 2006-06-26 23:18:39.000000000 +0200
+++ linux-2.6.17-mm2-full/drivers/mtd/cmdlinepart.c 2006-06-26 23:18:51.000000000 +0200
@@ -346,7 +346,7 @@
*
* This function needs to be visible for bootloaders.
*/
-int mtdpart_setup(char *s)
+static int mtdpart_setup(char *s)
Patch lacks sufficient explanation. Explain the relevance of the comment
immediately above the function declaration, in the context of your
patch. Explain your decision to change the behaviour, but not change the
comment itself.
My explanation regarding the relevance of the comment is that it seems
to be nonsense.
Do I miss something, or why and how should a bootloader access
in-kernel functions?
I'm not entirely sure, but allegedly it does -- Juha, can you elaborate?
Our bootloader doesn't access in-kernel functions, for obvious reasons.
The comment in cmdlinepart.c is unfortunately inaccurate. The
bootloader does need a mechanism for passing the partition table to the
kernel, though. Our partition table is generated on-the-fly by the
bootloader to guarantee that each partition gets a certain number of
non-bad NAND blocks.
We used to do this by passing the partition table in a string compatible
with cmdlinepart syntax. The kernel NAND driver then just passed the
string received from the bootloader to mtdpart_setup().
Nowadays there is a better way of doing this, so as far as we are
concerned, mtdpart_setup() can be made static again. We'll migrate our
MTD drivers to use the platform_data mechanism instead.
Cheers,
Juha
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/