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

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

[Kumar Gala]

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.

Also, fixed a compile warning in the file.

Signed-off-by: Kumar Gala <galak@kernel.crashing.org>

---
commit 625f68c82bae16c53f684c5512b0176c243c6068
tree 5657155434c9a44fa9ee3e0329756e354daf4845
parent 820ac48b82821c6d38747ea49f98aeca05ca2e2b
author Kumar Gala <galak@kernel.crashing.org> Fri, 24 Feb 2006 11:03:12 -0600
committer Kumar Gala <galak@kernel.crashing.org> Fri, 24 Feb 2006 11:03:12 -0600

 arch/powerpc/kernel/prom.c |   54 +++++++++++++++++++++++++++++++-------------
 1 files changed, 38 insertions(+), 16 deletions(-)

diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index 294832a..6dbd217 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -816,8 +816,6 @@ void __init unflatten_device_tree(void)
 {
 	unsigned long start, mem, size;
 	struct device_node **allnextp = &allnodes;
-	char *p = NULL;
-	int l = 0;
 
 	DBG(" -> unflatten_device_tree()\n");
 
@@ -857,19 +855,6 @@ void __init unflatten_device_tree(void)
 	if (of_chosen == NULL)
 		of_chosen = of_find_node_by_path("/chosen@0");
 
-	/* Retreive command line */
-	if (of_chosen != NULL) {
-		p = (char *)get_property(of_chosen, "bootargs", &l);
-		if (p != NULL && l > 0)
-			strlcpy(cmd_line, p, min(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);
-
 	DBG(" <- unflatten_device_tree()\n");
 }
 
@@ -940,6 +925,8 @@ static int __init early_init_dt_scan_cho
 {
 	u32 *prop;
 	unsigned long *lprop;
+	unsigned long l;
+	char *p;
 
 	DBG("search \"chosen\", depth: %d, uname: %s\n", depth, uname);
 
@@ -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;
+	}
+
 	/* break now */
 	return 1;
 }
@@ -1124,7 +1146,7 @@ static void __init early_reserve_mem(voi
 			size_32 = *(reserve_map_32++);
 			if (size_32 == 0)
 				break;
-			DBG("reserving: %lx -> %lx\n", base_32, size_32);
+			DBG("reserving: %x -> %x\n", base_32, size_32);
 			lmb_reserve(base_32, size_32);
 		}
 		return;

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

[Segher Boessenkool]

I can confirm this works on systems with "real" OF, too.  Furthermore,
the patch looks sane to me.

Acked-by: Segher Boessenkool <segher@kernel.crashing.org>

> 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.
>
> Also, fixed a compile warning in the file.
>
> Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
>
> ---
> commit 625f68c82bae16c53f684c5512b0176c243c6068
> tree 5657155434c9a44fa9ee3e0329756e354daf4845
> parent 820ac48b82821c6d38747ea49f98aeca05ca2e2b
> author Kumar Gala <galak@kernel.crashing.org> Fri, 24 Feb 2006 
> 11:03:12 -0600
> committer Kumar Gala <galak@kernel.crashing.org> Fri, 24 Feb 2006 
> 11:03:12 -0600
>
>  arch/powerpc/kernel/prom.c |   54 
> +++++++++++++++++++++++++++++++-------------
>  1 files changed, 38 insertions(+), 16 deletions(-)
>
> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index 294832a..6dbd217 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -816,8 +816,6 @@ void __init unflatten_device_tree(void)
>  {
>  	unsigned long start, mem, size;
>  	struct device_node **allnextp = &allnodes;
> -	char *p = NULL;
> -	int l = 0;
>
>  	DBG(" -> unflatten_device_tree()\n");
>
> @@ -857,19 +855,6 @@ void __init unflatten_device_tree(void)
>  	if (of_chosen == NULL)
>  		of_chosen = of_find_node_by_path("/chosen@0");
>
> -	/* Retreive command line */
> -	if (of_chosen != NULL) {
> -		p = (char *)get_property(of_chosen, "bootargs", &l);
> -		if (p != NULL && l > 0)
> -			strlcpy(cmd_line, p, min(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);
> -
>  	DBG(" <- unflatten_device_tree()\n");
>  }
>
> @@ -940,6 +925,8 @@ static int __init early_init_dt_scan_cho
>  {
>  	u32 *prop;
>  	unsigned long *lprop;
> +	unsigned long l;
> +	char *p;
>
>  	DBG("search \"chosen\", depth: %d, uname: %s\n", depth, uname);
>
> @@ -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;
> +	}
> +
>  	/* break now */
>  	return 1;
>  }
> @@ -1124,7 +1146,7 @@ static void __init early_reserve_mem(voi
>  			size_32 = *(reserve_map_32++);
>  			if (size_32 == 0)
>  				break;
> -			DBG("reserving: %lx -> %lx\n", base_32, size_32);
> +			DBG("reserving: %x -> %x\n", base_32, size_32);
>  			lmb_reserve(base_32, size_32);
>  		}
>  		return;
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
>

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

[Michael Ellerman]

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

Hi Kumar,

On Sat, 25 Feb 2006 03:54, Kumar Gala wrote:
> 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?

And I think a switch would be easier to read for the K/M/G handling.

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

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

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

[Kumar Gala]

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

> Hi Kumar,
>
> On Sat, 25 Feb 2006 03:54, Kumar Gala wrote:
>> 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.

> And I think a switch would be easier to read for the K/M/G handling.

I should probably use memparse() now that I've found it :)

- k

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

[Kumar Gala]

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.

Also, fixed a compile warning in the file.

Signed-off-by: Kumar Gala <galak@kernel.crashing.org>

---
commit a49869ffbf01f3998523357c85f7b55a6d064cda
tree efd39700c07ac02cb6216ebf8d6d0d2adf7be36a
parent 820ac48b82821c6d38747ea49f98aeca05ca2e2b
author Kumar Gala <galak@kernel.crashing.org> Fri, 24 Feb 2006 17:08:54 -0600
committer Kumar Gala <galak@kernel.crashing.org> Fri, 24 Feb 2006 17:08:54 -0600

 arch/powerpc/kernel/prom.c |   42 ++++++++++++++++++++++++++----------------
 1 files changed, 26 insertions(+), 16 deletions(-)

diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index 294832a..670654b 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -816,8 +816,6 @@ void __init unflatten_device_tree(void)
 {
 	unsigned long start, mem, size;
 	struct device_node **allnextp = &allnodes;
-	char *p = NULL;
-	int l = 0;
 
 	DBG(" -> unflatten_device_tree()\n");
 
@@ -857,19 +855,6 @@ void __init unflatten_device_tree(void)
 	if (of_chosen == NULL)
 		of_chosen = of_find_node_by_path("/chosen@0");
 
-	/* Retreive command line */
-	if (of_chosen != NULL) {
-		p = (char *)get_property(of_chosen, "bootargs", &l);
-		if (p != NULL && l > 0)
-			strlcpy(cmd_line, p, min(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);
-
 	DBG(" <- unflatten_device_tree()\n");
 }
 
@@ -940,6 +925,8 @@ static int __init early_init_dt_scan_cho
 {
 	u32 *prop;
 	unsigned long *lprop;
+	unsigned long l;
+	char *p;
 
 	DBG("search \"chosen\", depth: %d, uname: %s\n", depth, uname);
 
@@ -1004,6 +991,29 @@ 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;
+
+		for (q = cmd_line; (p = strstr(q, "mem=")) != 0; ) {
+			q = p + 4;
+			if (p > cmd_line && p[-1] != ' ')
+				continue;
+			memory_limit = memparse(q, &q);
+		}
+	}
+
 	/* break now */
 	return 1;
 }
@@ -1124,7 +1134,7 @@ static void __init early_reserve_mem(voi
 			size_32 = *(reserve_map_32++);
 			if (size_32 == 0)
 				break;
-			DBG("reserving: %lx -> %lx\n", base_32, size_32);
+			DBG("reserving: %x -> %x\n", base_32, size_32);
 			lmb_reserve(base_32, size_32);
 		}
 		return;

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

[Kumar Gala]

On Sat, 25 Feb 2006, Michael Ellerman wrote:

> On Sat, 25 Feb 2006 09:43, Kumar Gala wrote:
> > On Feb 24, 2006, at 4:27 PM, Michael Ellerman wrote:
> > > Hi Kumar,
> > >
> > > On Sat, 25 Feb 2006 03:54, Kumar Gala wrote:
> > >> 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.

- kumar

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

[Michael Ellerman]

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

On Sat, 25 Feb 2006 09:43, Kumar Gala wrote:
> On Feb 24, 2006, at 4:27 PM, Michael Ellerman wrote:
> > Hi Kumar,
> >
> > On Sat, 25 Feb 2006 03:54, Kumar Gala wrote:
> >> 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?

> > And I think a switch would be easier to read for the K/M/G handling.
>
> I should probably use memparse() now that I've found it :)

Even better.

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

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

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

[Michael Ellerman]

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

On Sat, 25 Feb 2006 10:18, Kumar Gala wrote:
> On Sat, 25 Feb 2006, Michael Ellerman wrote:
> > On Sat, 25 Feb 2006 09:43, Kumar Gala wrote:
> > > On Feb 24, 2006, at 4:27 PM, Michael Ellerman wrote:
> > > > Hi Kumar,
> > > >
> > > > On Sat, 25 Feb 2006 03:54, Kumar Gala wrote:
> > > >> 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

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

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

[Dave Hansen]

On Fri, 2006-02-24 at 10:54 -0600, Kumar Gala wrote:
> +       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;
> +       } 

You may want to check out lib/cmdline.c's memparse() function.  I think
it does this for you.

-- Dave

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

[Kumar Gala]

On Feb 26, 2006, at 2:38 PM, Dave Hansen wrote:

> On Fri, 2006-02-24 at 10:54 -0600, Kumar Gala wrote:
>> +       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;
>> +       }
>
> You may want to check out lib/cmdline.c's memparse() function.  I  
> think
> it does this for you.

Yeah, found it after I sent the patch.  Since Linus applied this  
version, I'll provide Paul a version that changes this code to use  
memparse for post 2.6.16.

- kumar