[PATCH 1/3] param: Adapt MN10300 to the new parameter handling regime

[PATCH 1/3] param: Adapt MN10300 to the new parameter handling regime

[David Howells]

Make MN10300 use the new core parameter code.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 arch/mn10300/kernel/setup.c |   50 +++++++++++++++----------------------------
 1 files changed, 18 insertions(+), 32 deletions(-)


diff --git a/arch/mn10300/kernel/setup.c b/arch/mn10300/kernel/setup.c
index e8c5d0b..310d542 100644
--- a/arch/mn10300/kernel/setup.c
+++ b/arch/mn10300/kernel/setup.c
@@ -60,6 +60,7 @@ static struct resource data_resource = {
 static unsigned long __initdata phys_memory_base;
 static unsigned long __initdata phys_memory_end;
 static unsigned long __initdata memory_end;
+static unsigned long __initdata memory_param;
 unsigned long memory_size;
 
 struct thread_info *__current_ti = &init_thread_union.thread_info;
@@ -79,41 +80,17 @@ void __init arch_get_boot_command_line(void)
 }
 
 /*
- * FIXME: use core_param
+ * handle a specific memory limit handed over the kernel command line
  */
-static void __init parse_mem_cmdline(void)
+static int __init mem_parameter(char *data)
 {
-	char *from, *to, c;
+	if (!data || !*data)
+		return 0;
 
-	/* see if there's an explicit memory size option */
-	from = redboot_command_line;
-	to = redboot_command_line;
-	c = ' ';
-
-	for (;;) {
-		if (c == ' ' && !memcmp(from, "mem=", 4)) {
-			if (to != redboot_command_line)
-				to--;
-			memory_size = memparse(from + 4, &from);
-		}
-
-		c = *(from++);
-		if (!c)
-			break;
-
-		*(to++) = c;
-	}
-
-	*to = '\0';
-
-	if (memory_size == 0)
-		panic("Memory size not known\n");
-
-	memory_end = (unsigned long) CONFIG_KERNEL_RAM_BASE_ADDRESS +
-		memory_size;
-	if (memory_end > phys_memory_end)
-		memory_end = phys_memory_end;
+	memory_param = memparse(data, NULL);
+	return 0;
 }
+early_param("mem", mem_parameter);
 
 /*
  * architecture specific setup
@@ -125,7 +102,6 @@ void __init setup_arch(void)
 
 	cpu_init();
 	unit_setup();
-	parse_mem_cmdline();
 
 	init_mm.start_code = (unsigned long)&_text;
 	init_mm.end_code = (unsigned long) &_etext;
@@ -223,6 +199,16 @@ void __init cpu_init(void)
 
 	phys_memory_end = phys_memory_base + memory_size;
 
+	if (memory_param)
+		memory_size = memory_param;
+	if (memory_size == 0)
+		panic("Memory size not known\n");
+
+	memory_end = (unsigned long) CONFIG_KERNEL_RAM_BASE_ADDRESS +
+		memory_size;
+	if (memory_end > phys_memory_end)
+		memory_end = phys_memory_end;
+
 #ifdef CONFIG_FPU
 	fpu_init_state();
 #endif

[PATCH 2/3] [PATCH] param: Stop gcc from inlining empty weak functions

[David Howells]

Stop gcc from inlining empty weak functions by making them non-empty.  The gcc
I have for FRV (4.1.2) will inline empty weak functions that are in the same
file, even if those functions are marked 'noinline'.

Without this patch, the default arch_get_boot_command_line() is rolled into
start_kernel(), and the arch specific one is not called, thus making the
kernel unbootable.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 init/main.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)


diff --git a/init/main.c b/init/main.c
index 92ad2fe..665993b 100644
--- a/init/main.c
+++ b/init/main.c
@@ -522,15 +522,18 @@ static void __init boot_cpu_init(void)
 
 void __init __weak smp_setup_processor_id(void)
 {
+	barrier();
 }
 
 void __init __weak thread_info_cache_init(void)
 {
+	barrier();
 }
 
 /* If the arch already sets boot_command_line, we need do nothing. */
 void __init __weak arch_get_boot_command_line(void)
 {
+	barrier();
 }
 
 /* Ideally, this would take a 'const char *cmdline' param. */

Re: [PATCH 2/3] [PATCH] param: Stop gcc from inlining empty weak functions

[Rusty Russell]

On Thursday 04 December 2008 03:02:12 David Howells wrote:
> Stop gcc from inlining empty weak functions by making them non-empty.  The gcc
> I have for FRV (4.1.2) will inline empty weak functions that are in the same
> file, even if those functions are marked 'noinline'.
> 
> Without this patch, the default arch_get_boot_command_line() is rolled into
> start_kernel(), and the arch specific one is not called, thus making the
> kernel unbootable.
> 
> Signed-off-by: David Howells <dhowells@redhat.com>

Ouch.  I'd prefer to fix this by moving the weak noop version to
kernel/params.c.  You may still need to fix the rest.

Here's what I ended up with (I'll eventually fold it):

commit 4858874241391aa62b5818781f702883e4ddf4ed
Author: Rusty Russell <rusty@rustcorp.com.au>
Date:   Fri Dec 5 18:56:34 2008 +1030

    Move arch_get_boot_command_line out of init/main.c for broken gccs.

diff --git a/include/linux/init.h b/include/linux/init.h
index 71cd0f5..8cd1a93 100644
--- a/include/linux/init.h
+++ b/include/linux/init.h
@@ -150,7 +150,7 @@ extern unsigned int reset_devices;
 
 /* used by init/main.c */
 void setup_arch(void);
-void arch_get_cmdline(char *cmdline);
+void arch_get_boot_command_line(void);
 void prepare_namespace(void);
 
 extern void (*late_time_init)(void);
diff --git a/init/main.c b/init/main.c
index 9595dba..f6032ba 100644
--- a/init/main.c
+++ b/init/main.c
@@ -528,11 +528,6 @@ void __init __weak thread_info_cache_init(void)
 {
 }
 
-/* If the arch already sets boot_command_line, we need do nothing. */
-void __init __weak arch_get_boot_command_line(void)
-{
-}
-
 /* Ideally, this would take a 'const char *cmdline' param. */
 asmlinkage void __init start_kernel(void)
 {
diff --git a/kernel/params.c b/kernel/params.c
index 69dd623..7fac630 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -30,6 +30,15 @@
 #define DEBUGP(fmt, a...)
 #endif
 
+/*
+ * If the arch already sets boot_command_line, we need do nothing. 
+ * This is not in init/main.c because David Howells reports that on FRV
+ * gcc 4.1.2 will inline empty functions, ignoring noinline and weak.
+ */
+void __init __weak arch_get_boot_command_line(void)
+{
+}
+
 static inline char dash2underscore(char c)
 {
 	if (c == '-')

Re: [PATCH 2/3] [PATCH] param: Stop gcc from inlining empty weak functions

[David Howells]

Rusty Russell <rusty@rustcorp.com.au> wrote:

> commit 4858874241391aa62b5818781f702883e4ddf4ed
> Author: Rusty Russell <rusty@rustcorp.com.au>
> Date:   Fri Dec 5 18:56:34 2008 +1030
> 
>     Move arch_get_boot_command_line out of init/main.c for broken gccs.

Acked-by: David Howells <dhowells@redhat.com>

[PATCH 3/3] [PATCH] param: Adapt FRV to the new parameter handling regime

[David Howells]

Make FRV use the new core parameter code.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 arch/frv/kernel/setup.c |   43 ++++++++++++++-----------------------------
 1 files changed, 14 insertions(+), 29 deletions(-)


diff --git a/arch/frv/kernel/setup.c b/arch/frv/kernel/setup.c
index 9548961..2f1506d 100644
--- a/arch/frv/kernel/setup.c
+++ b/arch/frv/kernel/setup.c
@@ -718,36 +718,25 @@ void __cpuinit calibrate_delay(void)
 
 } /* end calibrate_delay() */
 
-/*****************************************************************************/
 /*
- * look through the command line for some things we need to know immediately
- * FIXME: Use core_param or early_param.
+ * handle a specific memory limit handed over the kernel command line
  */
-static void __init parse_cmdline_early(char *cmdline)
+static int __init mem_parameter(char *data)
 {
-	if (!cmdline)
-		return;
-
-	while (*cmdline) {
-		if (*cmdline == ' ')
-			cmdline++;
-
-		/* "mem=XXX[kKmM]" sets SDRAM size to <mem>, overriding the value we worked
-		 * out from the SDRAM controller mask register
-		 */
-		if (!memcmp(cmdline, "mem=", 4)) {
-			unsigned long long mem_size;
-
-			mem_size = memparse(cmdline + 4, &cmdline);
-			memory_end = memory_start + mem_size;
-		}
-
-		while (*cmdline && *cmdline != ' ')
-			cmdline++;
-	}
+	unsigned long long mem_size;
 
-} /* end parse_cmdline_early() */
+	if (!data || !*data)
+		return 0;
+	mem_size = memparse(data, NULL);
+	memory_end = memory_start + mem_size;
+	return 0;
+}
+early_param("mem", mem_parameter);
 
+/*
+ * retrieve the boot command line from RedBoot and stash it somewhere more
+ * appropriate
+ */
 void __init arch_get_boot_command_line(void)
 {
 	memcpy(boot_command_line, redboot_command_line, COMMAND_LINE_SIZE);
@@ -796,10 +785,6 @@ void __init setup_arch(void)
 #endif
 #endif
 
-	/* deal with the command line - RedBoot may have passed one to the kernel */
-	memcpy(command_line, boot_command_line, sizeof(command_line));
-	parse_cmdline_early(command_line);
-
 	/* set up the memory description
 	 * - by now the stack is part of the init task */
 	printk("Memory %08lx-%08lx\n", memory_start, memory_end);

Re: [PATCH 1/3] param: Adapt MN10300 to the new parameter handling regime

[Rusty Russell]

On Thursday 04 December 2008 03:02:07 David Howells wrote:
> Make MN10300 use the new core parameter code.
> 
> Signed-off-by: David Howells <dhowells@redhat.com>

Hi David,

   Maybe we should add a new param type to the general code so this can be
done with core_param.  See below. Not sure if it's a win tho (many archs want
mem=num@pos, x86 wants mem=nopentium, etc).

I've applied this and 3/3 meanwhile.

Thanks!
Rusty.

param: add "mem" type for module_param/core_param

core_param is now called early enough to be useful for arch's mem=
parameter.  Some archs want fancier parsing, but this allows:

	static unsigned long mem_override;
	core_param(mem, mem_override, mem, 0444);

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
--- a/include/linux/moduleparam.h
+++ b/include/linux/moduleparam.h
@@ -94,7 +94,7 @@ struct kparam_array
 	__module_param_call(MODULE_PARAM_PREFIX, name, set, get, arg, perm)
 
 /* Helper functions: type is byte, short, ushort, int, uint, long,
-   ulong, charp, bool or invbool, or XXX if you define param_get_XXX,
+   ulong, charp, bool or invbool, mem or XXX if you define param_get_XXX,
    param_set_XXX and param_check_XXX. */
 #define module_param_named(name, value, type, perm)			   \
 	param_check_##type(name, &(value));				   \
@@ -184,6 +184,10 @@ extern int param_get_invbool(char *buffe
 extern int param_get_invbool(char *buffer, struct kernel_param *kp);
 #define param_check_invbool(name, p) __param_check(name, p, int)
 
+extern int param_set_mem(const char *val, struct kernel_param *kp);
+extern int param_get_mem(char *buffer, struct kernel_param *kp);
+#define param_check_mem(name, p) __param_check(name, p, unsigned long)
+
 /* Comma-separated array: *nump is set to number they actually specified. */
 #define module_param_array_named(name, array, type, nump, perm)		\
 	static const struct kparam_array __param_arr_##name		\
diff --git a/kernel/params.c b/kernel/params.c
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -264,6 +264,44 @@ int param_get_invbool(char *buffer, stru
 int param_get_invbool(char *buffer, struct kernel_param *kp)
 {
 	return sprintf(buffer, "%c", (*(int *)kp->arg) ? 'N' : 'Y');
+}
+
+int param_set_mem(const char *val, struct kernel_param *kp)
+{
+	unsigned long long mem;
+	char *endp;
+
+	if (!val || !*val)
+		return -EINVAL;
+
+	mem = memparse(val, &endp);
+	if (*endp)
+		return -EINVAL;
+
+	if ((unsigned long)mem != mem)
+		return -E2BIG;
+	*(unsigned long *)kp->arg = mem;
+	return 0;
+}
+
+int param_get_mem(char *buffer, struct kernel_param *kp)
+{
+	unsigned long mem = *(unsigned long *)kp->arg;
+	const char *suffix = "";
+
+	if (mem > 0) {
+		if (mem % (1024*1024*1024) == 0) {
+			suffix = "G";
+			mem /= 1024*1024*1024;
+		} else if (mem % (1024*1024) == 0) {
+			suffix = "M";
+			mem /= 1024*1024;
+		} else if (mem % 1024 == 0) {
+			suffix = "K";
+			mem /= 1024;
+		}
+	}		
+	return sprintf(buffer, "%lu%s", mem, suffix);
 }
 
 /* We break the rule and mangle the string. */

Re: [PATCH 1/3] param: Adapt MN10300 to the new parameter handling regime

[David Howells]

Rusty Russell <rusty@rustcorp.com.au> wrote:

>    Maybe we should add a new param type to the general code so this can be
> done with core_param.  See below. Not sure if it's a win tho (many archs want
> mem=num@pos, x86 wants mem=nopentium, etc).

I tried applying your patch, and then I added:

	#include <linux/moduleparam.h>

and put this into my code in place of what I had:

	static unsigned long mem_override;
	core_param(mem, mem_override, mem, 0444);

I get the following error, however:

  CC      arch/mn10300/kernel/setup.o
arch/mn10300/kernel/setup.c:86: error: 'param_mem_keeps_reference' undeclared here (not in a function)

I think you're missing:

	#define param_mem_keeps_reference 0

from the stuff you added.

With that, the core_param stuff does work for mem=...  But I object to
mem_override not being __initdata.  I also don't think the parameter should
appear in sysfs - that's just a waste of resources.  It should, perhaps,
appear in /proc/cmdline, but for some reason it does not.  I can live without
that, though, since its effect appears in /proc/meminfo.

Also, something else to consider: If CONFIG_MODULES=n and CONFIG_SYSFS=n,
should the contents of kernel/params.c be discarded along with the __init
sections?

David

Re: [PATCH 1/3] param: Adapt MN10300 to the new parameter handling regime

[Rusty Russell]

On Friday 05 December 2008 22:28:16 David Howells wrote:
> I think you're missing:
> 
> 	#define param_mem_keeps_reference 0
> 
> from the stuff you added.

Ah thanks, I didn't actually *cough* test it.

> With that, the core_param stuff does work for mem=...  But I object to
> mem_override not being __initdata.  I also don't think the parameter should
> appear in sysfs - that's just a waste of resources.

If you set the perm to 0, then it won't appear in sys, and hence can be
__initdata.

> It should, perhaps,
> appear in /proc/cmdline, but for some reason it does not.

Hmm, that's more concering.  I'll dig into this in the morning.

> I can live without
> that, though, since its effect appears in /proc/meminfo.
> 
> Also, something else to consider: If CONFIG_MODULES=n and CONFIG_SYSFS=n,
> should the contents of kernel/params.c be discarded along with the __init
> sections?

Yes, I think so.  YA __init variant, but it can be local to kernel/params.c
I think.

Cheers,
Rusty.

Re: [PATCH 1/3] param: Adapt MN10300 to the new parameter handling regime

[David Howells]

Rusty Russell <rusty@rustcorp.com.au> wrote:

> > With that, the core_param stuff does work for mem=...  But I object to
> > mem_override not being __initdata.  I also don't think the parameter should
> > appear in sysfs - that's just a waste of resources.
> 
> If you set the perm to 0, then it won't appear in sys, and hence can be
> __initdata.

That seemed to work.

David

Re: [PATCH 1/3] param: Adapt MN10300 to the new parameter handling regime

[Rusty Russell]

On Friday 05 December 2008 23:25:22 Rusty Russell wrote:
> On Friday 05 December 2008 22:28:16 David Howells wrote:
> > With that, the core_param stuff does work for mem=...
...
> > It should, perhaps,
> > appear in /proc/cmdline, but for some reason it does not.
> 
> Hmm, that's more concering.  I'll dig into this in the morning.

OK, I can't reproduce it.  I was thinking some weird corner case
with not restoring the string in parse_args, but putting in a dummy
"mem" core_param() on x86 works as first, middle and last arg on cmdline,
and command line shows up correctly in /proc/cmdline.

Any chance I can ask you to verify that?  Is the commandline printk'd
on boot also wrong?

Thanks,
Rusty.

Re: [PATCH 1/3] param: Adapt MN10300 to the new parameter handling regime

[David Howells]

Rusty Russell <rusty@rustcorp.com.au> wrote:

> > > It should, perhaps,
> > > appear in /proc/cmdline, but for some reason it does not.
> > 
> > Hmm, that's more concering.  I'll dig into this in the morning.
> 
> OK, I can't reproduce it.  I was thinking some weird corner case
> with not restoring the string in parse_args, but putting in a dummy
> "mem" core_param() on x86 works as first, middle and last arg on cmdline,
> and command line shows up correctly in /proc/cmdline.
> 
> Any chance I can ask you to verify that?  Is the commandline printk'd
> on boot also wrong?

I think I must've tested it wrong, probably by forgetting to paste in a mem=
option on the kernel command line when I was trying to check /proc/cmdline.
Leastways, it works now.

David