Re: [PATCH] powerpc: Fix mem= cmdline handling on arch/powerpc for !MULTIPLATFORM

On Sat, 25 Feb 2006 10:18, Kumar Gala wrote:

On Sat, 25 Feb 2006, Michael Ellerman wrote:

quoted

On Sat, 25 Feb 2006 09:43, Kumar Gala wrote:

quoted

On Feb 24, 2006, at 4:27 PM, Michael Ellerman wrote:

quoted

Hi Kumar,

On Sat, 25 Feb 2006 03:54, Kumar Gala wrote:

quoted

mem= command line option was being ignored in arch/powerpc if we
were not
a CONFIG_MULTIPLATFORM (which is handled via prom_init stub). The
initial
command line extraction and parsing needed to be moved earlier in
the boot
process and have code to actual parse mem= and do something about
it.

@@ -1004,6 +991,41 @@ static int __init early_init_dt_scan_cho
                crashk_res.end = crashk_res.start + *lprop - 1;
 #endif

+	/* Retreive command line */
+ 	p = of_get_flat_dt_prop(node, "bootargs", &l);
+	if (p != NULL && l > 0)
+		strlcpy(cmd_line, p, min((int)l, COMMAND_LINE_SIZE));
+
+#ifdef CONFIG_CMDLINE
+	if (l == 0 || (l == 1 && (*p) == 0))
+		strlcpy(cmd_line, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
+#endif /* CONFIG_CMDLINE */
+
+	DBG("Command line is: %s\n", cmd_line);
+
+	if (strstr(cmd_line, "mem=")) {
+		char *p, *q;
+		unsigned long maxmem = 0;
+
+		for (q = cmd_line; (p = strstr(q, "mem=")) != 0; ) {
+			q = p + 4;
+			if (p > cmd_line && p[-1] != ' ')
+				continue;
+			maxmem = simple_strtoul(q, &q, 0);
+			if (*q == 'k' || *q == 'K') {
+				maxmem <<= 10;
+				++q;
+			} else if (*q == 'm' || *q == 'M') {
+				maxmem <<= 20;
+				++q;
+			} else if (*q == 'g' || *q == 'G') {
+				maxmem <<= 30;
+				++q;
+			}
+		}
+		memory_limit = maxmem;
+	}
+

Why not make the mem= parsing an early_param() handler and then call
parse_early_param() here?

This would put constraints on the early_param()'s that I dont think
we should impose.

All they should really be doing is parsing the string and setting some
variables, so that seems reasonable to me. Is there anything in
particular?

If you ever had to do some memory allocation as part of the parsing that
might be an issue, since we haven't setup the LMB at that point.

Sure, but I think it's reasonable to say "don't allocate memory in an 
early_param handler", it is an _early_ param after all. But I guess we'll 
have to agree to disagree until someone else chimes in with an opinion :)

cheers

-- 
Michael Ellerman
IBM OzLabs

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person