[PATCH] mtd: phram: dot not crash when built-in and passing boot param

[PATCH] mtd: phram: dot not crash when built-in and passing boot param

[h-fache]

From: Hervé Fache <h-fache@ti.com>

This patch is based on Ville Herva's similar patch to block2mtd.

Trying to pass a parameter through the kernel command line when built-in would
crash the kernel, as phram_setup() was called so early that kmalloc() was not
functional yet.

This patch only saves the parameter string at the early boot stage, and parses
it later when init_phram() is called. The same happens in both module and
built-in cases.

With this patch, I can boot with a statically-compiled phram, and mount a
ext2 root fs from physical RAM, without the need for a initrd.

This has been tested in built-in and module cases, with and without a
parameter string.

Signed-off-by: Hervé Fache <h-fache@ti.com>
---
 drivers/mtd/devices/phram.c |   28 ++++++++++++++++++++++++++--
 1 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/devices/phram.c b/drivers/mtd/devices/phram.c
index 23423bd..727cb5d 100644
--- a/drivers/mtd/devices/phram.c
+++ b/drivers/mtd/devices/phram.c
@@ -1,6 +1,7 @@
 /**
  * Copyright (c) ????		Jochen Schäuble <psionic@psionic.de>
  * Copyright (c) 2003-2004	Joern Engel <joern@wh.fh-wedel.de>
+ * Copyright (c) 2012		Hervé Fache <h-fache@ti.com>
  *
  * Usage:
  *
@@ -233,7 +234,16 @@ static inline void kill_final_newline(char *str)
 	return 1;		\
 } while (0)
 
-static int phram_setup(const char *val, struct kernel_param *kp)
+/* This shall contain the module parameter if any. It is of the form:
+   * phram=<device>,<address>,<size> for module case
+   * phram.phram=<device>,<address>,<size> for built-in case
+   We leave 64 bytes for the device name, 12 for the address and 12 for the
+   size.
+   Example: phram.phram=rootfs,0xa0000000,512Mi
+*/
+static __initdata char phram_paramline[64+12+12];
+
+static int phram_setup(const char *val)
 {
 	char buf[64+12+12], *str = buf;
 	char *token[3];
@@ -282,12 +292,26 @@ static int phram_setup(const char *val, struct kernel_param *kp)
 	return ret;
 }
 
-module_param_call(phram, phram_setup, NULL, NULL, 000);
+static int phram_param_call(const char *val, struct kernel_param *kp)
+{
+	/* This function is always called before 'init_phram()', whether built-in or
+	   module. */
+	if (strlen(val) >= sizeof(phram_paramline))
+		return -ENOSPC;
+	strcpy(phram_paramline, val);
+
+	return 0;
+}
+
+module_param_call(phram, phram_param_call, NULL, NULL, 000);
 MODULE_PARM_DESC(phram, "Memory region to map. \"phram=<name>,<start>,<length>\"");
 
 
 static int __init init_phram(void)
 {
+	if (phram_paramline[0])
+		return phram_setup(phram_paramline);
+
 	return 0;
 }
 
-- 
1.7.9

Re: [PATCH] mtd: phram: dot not crash when built-in and passing boot param

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 387 bytes --]

On Tue, 2012-03-13 at 10:52 +0100, h-fache@ti.com wrote:
>   * Copyright (c) ????		Jochen Schäuble <psionic@psionic.de>
>   * Copyright (c) 2003-2004	Joern Engel <joern@wh.fh-wedel.de>
> + * Copyright (c) 2012		Hervé Fache <h-fache@ti.com>

I think a little change does not justify adding own copyright to the
whole driver, or does it?


-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] mtd: phram: dot not crash when built-in and passing boot param

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 964 bytes --]

On Tue, 2012-03-13 at 10:52 +0100, h-fache@ti.com wrote:
> +/* This shall contain the module parameter if any. It is of the form:
> +   * phram=<device>,<address>,<size> for module case
> +   * phram.phram=<device>,<address>,<size> for built-in case
> +   We leave 64 bytes for the device name, 12 for the address and 12 for the
> +   size.
> +   Example: phram.phram=rootfs,0xa0000000,512Mi
> +*/

Sorry for this, but please, while you are on it, still, turn all your
comments into the standard Linux-style comments. This is not that
important, but just cleaner.

> +static __initdata char phram_paramline[64+12+12];

Despite that you told that there are not section mismatches, which I
doubted and thus decided to check, I see that this patch _does_ cause a
section mismatch:

+WARNING: modpost: Found 1 section mismatch(es).
+To see full details build your kernel with:

Could you please fix this?

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH] mtd: phram: dot not crash when built-in and passing boot param

[h-fache]

From: Hervé Fache <h-fache@ti.com>

This patch is based on Ville Herva's similar patch to block2mtd.

Trying to pass a parameter through the kernel command line when built-in would
crash the kernel, as phram_setup() was called so early that kmalloc() was not
functional yet.

This patch only saves the parameter string at the early boot stage, and parses
it later when init_phram() is called. The same happens in both module and
built-in cases.

With this patch, I can boot with a statically-compiled phram, and mount a
ext2 root fs from physical RAM, without the need for a initrd.

This has been tested in built-in and module cases, with and without a
parameter string.

Signed-off-by: Hervé Fache <h-fache@ti.com>
---
 drivers/mtd/devices/phram.c |   28 ++++++++++++++++++++++++++--
 1 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/devices/phram.c b/drivers/mtd/devices/phram.c
index 23423bd..dd55ae2 100644
--- a/drivers/mtd/devices/phram.c
+++ b/drivers/mtd/devices/phram.c
@@ -233,7 +233,16 @@ static inline void kill_final_newline(char *str)
 	return 1;		\
 } while (0)
 
-static int phram_setup(const char *val, struct kernel_param *kp)
+/* This shall contain the module parameter if any. It is of the form:
+ * - phram=<device>,<address>,<size> for module case
+ * - phram.phram=<device>,<address>,<size> for built-in case
+ * We leave 64 bytes for the device name, 12 for the address and 12 for the
+ * size.
+ * Example: phram.phram=rootfs,0xa0000000,512Mi
+ */
+static __initdata char phram_paramline[64+12+12];
+
+static int phram_setup(const char *val)
 {
 	char buf[64+12+12], *str = buf;
 	char *token[3];
@@ -282,12 +291,27 @@ static int phram_setup(const char *val, struct kernel_param *kp)
 	return ret;
 }
 
-module_param_call(phram, phram_setup, NULL, NULL, 000);
+static int __init phram_param_call(const char *val, struct kernel_param *kp)
+{
+	/* This function is always called before 'init_phram()', whether
+	 * built-in or module.
+	 */
+	if (strlen(val) >= sizeof(phram_paramline))
+		return -ENOSPC;
+	strcpy(phram_paramline, val);
+
+	return 0;
+}
+
+module_param_call(phram, phram_param_call, NULL, NULL, 000);
 MODULE_PARM_DESC(phram, "Memory region to map. \"phram=<name>,<start>,<length>\"");
 
 
 static int __init init_phram(void)
 {
+	if (phram_paramline[0])
+		return phram_setup(phram_paramline);
+
 	return 0;
 }
 
-- 
1.7.9

Re: [PATCH] mtd: phram: dot not crash when built-in and passing boot param

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 242 bytes --]

On Tue, 2012-03-13 at 16:07 +0100, h-fache@ti.com wrote:
> From: Hervé Fache <h-fache@ti.com>
> 
> This patch is based on Ville Herva's similar patch to block2mtd.

Pushed to l2-mtd.git, thanks!

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]