[U-Boot,v3,7/7] MIPS: bootm: Add support for Vcore III linux kernel

Message ID 20181205171054.926-8-gregory.clement@bootlin.com
State Superseded
Delegated to: Daniel Schwierzeck
Headers show
Series
  • ] Add support for the SoCs found in Microsemi switches
Related show

Commit Message

Gregory CLEMENT Dec. 5, 2018, 5:10 p.m.
The kernels built for the Vcore III linux kernel have different
expectation in the way the data were passed.

Unlike with yamon, the command line is expected to be a single string
passed in argv[1]. An other expectation is that the arguments are located
in the cached address space.

However, like yamon, they expect that rd_start and rd_size was passed by
the bootloader in the command line of the kernel, and besides that it
also wait for the root=/dev/ram0.

Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>
---
 arch/mips/lib/bootm.c | 78 ++++++++++++++++++++++++++++++++-----------
 1 file changed, 58 insertions(+), 20 deletions(-)

Comments

Daniel Schwierzeck Dec. 10, 2018, 5:30 p.m. | #1
Am 05.12.18 um 18:10 schrieb Gregory CLEMENT:
> The kernels built for the Vcore III linux kernel have different
> expectation in the way the data were passed.
> 
> Unlike with yamon, the command line is expected to be a single string
> passed in argv[1]. An other expectation is that the arguments are located
> in the cached address space.
> 
> However, like yamon, they expect that rd_start and rd_size was passed by
> the bootloader in the command line of the kernel, and besides that it
> also wait for the root=/dev/ram0.



> 
> Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>
> ---
>  arch/mips/lib/bootm.c | 78 ++++++++++++++++++++++++++++++++-----------
>  1 file changed, 58 insertions(+), 20 deletions(-)
> 
> diff --git a/arch/mips/lib/bootm.c b/arch/mips/lib/bootm.c
> index deca5189e3..a3831c28a4 100644
> --- a/arch/mips/lib/bootm.c
> +++ b/arch/mips/lib/bootm.c
> @@ -15,6 +15,11 @@ DECLARE_GLOBAL_DATA_PTR;
>  #define	LINUX_MAX_ENVS		256
>  #define	LINUX_MAX_ARGS		256
>  
> +enum legacy_boot_type {
> +	LEGACY_BOOT_YAML,
> +	LEGACY_BOOT_VCORE,
> +};
> +
>  static int linux_argc;
>  static char **linux_argv;
>  static char *linux_argp;
> @@ -44,22 +49,47 @@ void arch_lmb_reserve(struct lmb *lmb)
>  	lmb_reserve(lmb, sp, gd->ram_top - sp);
>  }
>  
> -static void linux_cmdline_init(void)
> +static void linux_cmdline_init(enum legacy_boot_type boot_type)
>  {
> +	switch (boot_type) {
> +		/*
> +		 * Vcore III linux kernels expect arguments in the cached
> +		 * address space. They also expect the command line being a
> +		 * single string in the first argument
> +		 */
> +	case LEGACY_BOOT_VCORE:
> +		linux_argv = (char **)(gd->bd->bi_boot_params);
> +		linux_argp = (char *)(linux_argv + LINUX_MAX_ARGS);
> +		linux_argv[1] = linux_argp;
> +		break;
> +	case LEGACY_BOOT_YAML:
> +		/* fall-through */
> +	default:
> +		linux_argv = (char **)UNCACHED_SDRAM(gd->bd->bi_boot_params);
> +		linux_argp = (char *)(linux_argv + LINUX_MAX_ARGS);
> +	}
>  	linux_argc = 1;
> -	linux_argv = (char **)UNCACHED_SDRAM(gd->bd->bi_boot_params);
>  	linux_argv[0] = 0;
> -	linux_argp = (char *)(linux_argv + LINUX_MAX_ARGS);
>  }
>  
> -static void linux_cmdline_set(const char *value, size_t len)
> +static void linux_cmdline_set(const char *value, size_t len,
> +			      enum legacy_boot_type boot_type)
>  {
> -	linux_argv[linux_argc] = linux_argp;
>  	memcpy(linux_argp, value, len);
> -	linux_argp[len] = 0;
> -
> +	switch (boot_type) {
> +	case LEGACY_BOOT_VCORE:
> +		linux_argv[linux_argc] = linux_argp;
> +		linux_argp[len] = 0;
> +		linux_argc++;
> +		break;
> +	case LEGACY_BOOT_YAML:
> +		/* fall-through */
> +	default:
> +		linux_argp[len] = ' ';
> +		linux_argp[len + 1] = 0;
> +		linux_argc = 2;
> +	}
>  	linux_argp += len + 1;
> -	linux_argc++;
>  }
>  
>  static void linux_cmdline_dump(void)
> @@ -73,12 +103,11 @@ static void linux_cmdline_dump(void)
>  		debug("   arg %03d: %s\n", i, linux_argv[i]);
>  }
>  
> -static void linux_cmdline_legacy(bootm_headers_t *images)
> +static void linux_cmdline_legacy(bootm_headers_t *images,
> +				 enum legacy_boot_type boot_type)
>  {
>  	const char *bootargs, *next, *quote;
> -
> -	linux_cmdline_init();
> -
> +	linux_cmdline_init(boot_type);
>  	bootargs = env_get("bootargs");
>  	if (!bootargs)
>  		return;
> @@ -104,7 +133,7 @@ static void linux_cmdline_legacy(bootm_headers_t *images)
>  		if (!next)
>  			next = bootargs + strlen(bootargs);
>  
> -		linux_cmdline_set(bootargs, next - bootargs);
> +		linux_cmdline_set(bootargs, next - bootargs, boot_type);
>  
>  		if (*next)
>  			next++;
> @@ -113,7 +142,8 @@ static void linux_cmdline_legacy(bootm_headers_t *images)
>  	}
>  }
>  
> -static void linux_cmdline_append(bootm_headers_t *images)
> +static void linux_cmdline_append(bootm_headers_t *images,
> +				 enum legacy_boot_type boot_type)
>  {
>  	char buf[24];
>  	ulong mem, rd_start, rd_size;
> @@ -121,7 +151,7 @@ static void linux_cmdline_append(bootm_headers_t *images)
>  	/* append mem */
>  	mem = gd->ram_size >> 20;
>  	sprintf(buf, "mem=%luM", mem);
> -	linux_cmdline_set(buf, strlen(buf));
> +	linux_cmdline_set(buf, strlen(buf), boot_type);
>  
>  	/* append rd_start and rd_size */
>  	rd_start = images->initrd_start;
> @@ -129,9 +159,13 @@ static void linux_cmdline_append(bootm_headers_t *images)
>  
>  	if (rd_size) {
>  		sprintf(buf, "rd_start=0x%08lX", rd_start);
> -		linux_cmdline_set(buf, strlen(buf));
> +		linux_cmdline_set(buf, strlen(buf), boot_type);
>  		sprintf(buf, "rd_size=0x%lX", rd_size);
> -		linux_cmdline_set(buf, strlen(buf));
> +		linux_cmdline_set(buf, strlen(buf), boot_type);
> +		if (boot_type ==  LEGACY_BOOT_VCORE) {
> +			sprintf(buf, "root=/dev/ram0");
> +			linux_cmdline_set(buf, strlen(buf), boot_type);
> +		}

if possible, add this via the bootargs environment variable from your
boot script

>  	}
>  }
>  
> @@ -276,11 +310,15 @@ static void boot_prep_linux(bootm_headers_t *images)
>  		boot_reloc_fdt(images);
>  		boot_setup_fdt(images);
>  	} else {
> -		if (CONFIG_IS_ENABLED(MIPS_BOOT_CMDLINE_LEGACY)) {
> -			linux_cmdline_legacy(images);
> +		if (CONFIG_IS_ENABLED(SOC_VCOREIII)) {
> +			linux_cmdline_legacy(images, LEGACY_BOOT_VCORE);
> +			linux_cmdline_append(images, LEGACY_BOOT_VCORE);
> +			linux_cmdline_dump();

as mentioned in the last review, please add this in a generic way. I
want to avoid SoC specific code in this file. You should add a Kconfig
symbol like CONFIG_MIPS_BOOT_CMDLINE_SINGLESTRING and imply this in your
SoC or board config. I think you could also use the U-Boot bootargs
buffer directly or make a copy for the kernel. There is no need to hack
the linux_cmdline_* functions.

> +		} else if (CONFIG_IS_ENABLED(MIPS_BOOT_CMDLINE_LEGACY)) {
> +			linux_cmdline_legacy(images, 0);
>  
>  			if (!CONFIG_IS_ENABLED(MIPS_BOOT_ENV_LEGACY))
> -				linux_cmdline_append(images);
> +				linux_cmdline_append(images, LEGACY_BOOT_YAML);
>  
>  			linux_cmdline_dump();
>  		}
>
Gregory CLEMENT Dec. 14, 2018, 10:43 a.m. | #2
Hi Daniel,
 
 On lun., déc. 10 2018, Daniel Schwierzeck <daniel.schwierzeck@gmail.com> wrote:

> Am 05.12.18 um 18:10 schrieb Gregory CLEMENT:
>> The kernels built for the Vcore III linux kernel have different
>> expectation in the way the data were passed.
>> 
>> Unlike with yamon, the command line is expected to be a single string
>> passed in argv[1]. An other expectation is that the arguments are located
>> in the cached address space.
>> 
>> However, like yamon, they expect that rd_start and rd_size was passed by
>> the bootloader in the command line of the kernel, and besides that it
>> also wait for the root=/dev/ram0.
>
>
>
>> 
>> Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>
>> ---
>>  arch/mips/lib/bootm.c | 78 ++++++++++++++++++++++++++++++++-----------
>>  1 file changed, 58 insertions(+), 20 deletions(-)
>> 
>> diff --git a/arch/mips/lib/bootm.c b/arch/mips/lib/bootm.c
>> index deca5189e3..a3831c28a4 100644
>> --- a/arch/mips/lib/bootm.c
>> +++ b/arch/mips/lib/bootm.c
>> @@ -15,6 +15,11 @@ DECLARE_GLOBAL_DATA_PTR;
>>  #define	LINUX_MAX_ENVS		256
>>  #define	LINUX_MAX_ARGS		256
>>  
>> +enum legacy_boot_type {
>> +	LEGACY_BOOT_YAML,
>> +	LEGACY_BOOT_VCORE,
>> +};
>> +
>>  static int linux_argc;
>>  static char **linux_argv;
>>  static char *linux_argp;
>> @@ -44,22 +49,47 @@ void arch_lmb_reserve(struct lmb *lmb)
>>  	lmb_reserve(lmb, sp, gd->ram_top - sp);
>>  }
>>  
>> -static void linux_cmdline_init(void)
>> +static void linux_cmdline_init(enum legacy_boot_type boot_type)
>>  {
>> +	switch (boot_type) {
>> +		/*
>> +		 * Vcore III linux kernels expect arguments in the cached
>> +		 * address space. They also expect the command line being a
>> +		 * single string in the first argument
>> +		 */
>> +	case LEGACY_BOOT_VCORE:
>> +		linux_argv = (char **)(gd->bd->bi_boot_params);
>> +		linux_argp = (char *)(linux_argv + LINUX_MAX_ARGS);
>> +		linux_argv[1] = linux_argp;
>> +		break;
>> +	case LEGACY_BOOT_YAML:
>> +		/* fall-through */
>> +	default:
>> +		linux_argv = (char **)UNCACHED_SDRAM(gd->bd->bi_boot_params);
>> +		linux_argp = (char *)(linux_argv + LINUX_MAX_ARGS);
>> +	}
>>  	linux_argc = 1;
>> -	linux_argv = (char **)UNCACHED_SDRAM(gd->bd->bi_boot_params);
>>  	linux_argv[0] = 0;
>> -	linux_argp = (char *)(linux_argv + LINUX_MAX_ARGS);
>>  }
>>  
>> -static void linux_cmdline_set(const char *value, size_t len)
>> +static void linux_cmdline_set(const char *value, size_t len,
>> +			      enum legacy_boot_type boot_type)
>>  {
>> -	linux_argv[linux_argc] = linux_argp;
>>  	memcpy(linux_argp, value, len);
>> -	linux_argp[len] = 0;
>> -
>> +	switch (boot_type) {
>> +	case LEGACY_BOOT_VCORE:
>> +		linux_argv[linux_argc] = linux_argp;
>> +		linux_argp[len] = 0;
>> +		linux_argc++;
>> +		break;
>> +	case LEGACY_BOOT_YAML:
>> +		/* fall-through */
>> +	default:
>> +		linux_argp[len] = ' ';
>> +		linux_argp[len + 1] = 0;
>> +		linux_argc = 2;
>> +	}
>>  	linux_argp += len + 1;
>> -	linux_argc++;
>>  }
>>  
>>  static void linux_cmdline_dump(void)
>> @@ -73,12 +103,11 @@ static void linux_cmdline_dump(void)
>>  		debug("   arg %03d: %s\n", i, linux_argv[i]);
>>  }
>>  
>> -static void linux_cmdline_legacy(bootm_headers_t *images)
>> +static void linux_cmdline_legacy(bootm_headers_t *images,
>> +				 enum legacy_boot_type boot_type)
>>  {
>>  	const char *bootargs, *next, *quote;
>> -
>> -	linux_cmdline_init();
>> -
>> +	linux_cmdline_init(boot_type);
>>  	bootargs = env_get("bootargs");
>>  	if (!bootargs)
>>  		return;
>> @@ -104,7 +133,7 @@ static void linux_cmdline_legacy(bootm_headers_t *images)
>>  		if (!next)
>>  			next = bootargs + strlen(bootargs);
>>  
>> -		linux_cmdline_set(bootargs, next - bootargs);
>> +		linux_cmdline_set(bootargs, next - bootargs, boot_type);
>>  
>>  		if (*next)
>>  			next++;
>> @@ -113,7 +142,8 @@ static void linux_cmdline_legacy(bootm_headers_t *images)
>>  	}
>>  }
>>  
>> -static void linux_cmdline_append(bootm_headers_t *images)
>> +static void linux_cmdline_append(bootm_headers_t *images,
>> +				 enum legacy_boot_type boot_type)
>>  {
>>  	char buf[24];
>>  	ulong mem, rd_start, rd_size;
>> @@ -121,7 +151,7 @@ static void linux_cmdline_append(bootm_headers_t *images)
>>  	/* append mem */
>>  	mem = gd->ram_size >> 20;
>>  	sprintf(buf, "mem=%luM", mem);
>> -	linux_cmdline_set(buf, strlen(buf));
>> +	linux_cmdline_set(buf, strlen(buf), boot_type);
>>  
>>  	/* append rd_start and rd_size */
>>  	rd_start = images->initrd_start;
>> @@ -129,9 +159,13 @@ static void linux_cmdline_append(bootm_headers_t *images)
>>  
>>  	if (rd_size) {
>>  		sprintf(buf, "rd_start=0x%08lX", rd_start);
>> -		linux_cmdline_set(buf, strlen(buf));
>> +		linux_cmdline_set(buf, strlen(buf), boot_type);
>>  		sprintf(buf, "rd_size=0x%lX", rd_size);
>> -		linux_cmdline_set(buf, strlen(buf));
>> +		linux_cmdline_set(buf, strlen(buf), boot_type);
>> +		if (boot_type ==  LEGACY_BOOT_VCORE) {
>> +			sprintf(buf, "root=/dev/ram0");
>> +			linux_cmdline_set(buf, strlen(buf), boot_type);
>> +		}
>
> if possible, add this via the bootargs environment variable from your
> boot script

It is not straightforward to put in the environment variable because it
depend of the fact there is ramdisk or not, but as requested I will
remove this part.


>
>>  	}
>>  }
>>  
>> @@ -276,11 +310,15 @@ static void boot_prep_linux(bootm_headers_t *images)
>>  		boot_reloc_fdt(images);
>>  		boot_setup_fdt(images);
>>  	} else {
>> -		if (CONFIG_IS_ENABLED(MIPS_BOOT_CMDLINE_LEGACY)) {
>> -			linux_cmdline_legacy(images);
>> +		if (CONFIG_IS_ENABLED(SOC_VCOREIII)) {
>> +			linux_cmdline_legacy(images, LEGACY_BOOT_VCORE);
>> +			linux_cmdline_append(images, LEGACY_BOOT_VCORE);
>> +			linux_cmdline_dump();
>
> as mentioned in the last review, please add this in a generic way. I
> want to avoid SoC specific code in this file. You should add a Kconfig
> symbol like CONFIG_MIPS_BOOT_CMDLINE_SINGLESTRING and imply this in your
> SoC or board config. I think you could also use the U-Boot bootargs
> buffer directly or make a copy for the kernel. There is no need to hack
> the linux_cmdline_* functions.

OK I will use a more generic KConfig symbol. 

Gregory

>
>> +		} else if (CONFIG_IS_ENABLED(MIPS_BOOT_CMDLINE_LEGACY)) {
>> +			linux_cmdline_legacy(images, 0);
>>  
>>  			if (!CONFIG_IS_ENABLED(MIPS_BOOT_ENV_LEGACY))
>> -				linux_cmdline_append(images);
>> +				linux_cmdline_append(images, LEGACY_BOOT_YAML);
>>  
>>  			linux_cmdline_dump();
>>  		}
>> 
>
> -- 
> - Daniel

Patch

diff --git a/arch/mips/lib/bootm.c b/arch/mips/lib/bootm.c
index deca5189e3..a3831c28a4 100644
--- a/arch/mips/lib/bootm.c
+++ b/arch/mips/lib/bootm.c
@@ -15,6 +15,11 @@  DECLARE_GLOBAL_DATA_PTR;
 #define	LINUX_MAX_ENVS		256
 #define	LINUX_MAX_ARGS		256
 
+enum legacy_boot_type {
+	LEGACY_BOOT_YAML,
+	LEGACY_BOOT_VCORE,
+};
+
 static int linux_argc;
 static char **linux_argv;
 static char *linux_argp;
@@ -44,22 +49,47 @@  void arch_lmb_reserve(struct lmb *lmb)
 	lmb_reserve(lmb, sp, gd->ram_top - sp);
 }
 
-static void linux_cmdline_init(void)
+static void linux_cmdline_init(enum legacy_boot_type boot_type)
 {
+	switch (boot_type) {
+		/*
+		 * Vcore III linux kernels expect arguments in the cached
+		 * address space. They also expect the command line being a
+		 * single string in the first argument
+		 */
+	case LEGACY_BOOT_VCORE:
+		linux_argv = (char **)(gd->bd->bi_boot_params);
+		linux_argp = (char *)(linux_argv + LINUX_MAX_ARGS);
+		linux_argv[1] = linux_argp;
+		break;
+	case LEGACY_BOOT_YAML:
+		/* fall-through */
+	default:
+		linux_argv = (char **)UNCACHED_SDRAM(gd->bd->bi_boot_params);
+		linux_argp = (char *)(linux_argv + LINUX_MAX_ARGS);
+	}
 	linux_argc = 1;
-	linux_argv = (char **)UNCACHED_SDRAM(gd->bd->bi_boot_params);
 	linux_argv[0] = 0;
-	linux_argp = (char *)(linux_argv + LINUX_MAX_ARGS);
 }
 
-static void linux_cmdline_set(const char *value, size_t len)
+static void linux_cmdline_set(const char *value, size_t len,
+			      enum legacy_boot_type boot_type)
 {
-	linux_argv[linux_argc] = linux_argp;
 	memcpy(linux_argp, value, len);
-	linux_argp[len] = 0;
-
+	switch (boot_type) {
+	case LEGACY_BOOT_VCORE:
+		linux_argv[linux_argc] = linux_argp;
+		linux_argp[len] = 0;
+		linux_argc++;
+		break;
+	case LEGACY_BOOT_YAML:
+		/* fall-through */
+	default:
+		linux_argp[len] = ' ';
+		linux_argp[len + 1] = 0;
+		linux_argc = 2;
+	}
 	linux_argp += len + 1;
-	linux_argc++;
 }
 
 static void linux_cmdline_dump(void)
@@ -73,12 +103,11 @@  static void linux_cmdline_dump(void)
 		debug("   arg %03d: %s\n", i, linux_argv[i]);
 }
 
-static void linux_cmdline_legacy(bootm_headers_t *images)
+static void linux_cmdline_legacy(bootm_headers_t *images,
+				 enum legacy_boot_type boot_type)
 {
 	const char *bootargs, *next, *quote;
-
-	linux_cmdline_init();
-
+	linux_cmdline_init(boot_type);
 	bootargs = env_get("bootargs");
 	if (!bootargs)
 		return;
@@ -104,7 +133,7 @@  static void linux_cmdline_legacy(bootm_headers_t *images)
 		if (!next)
 			next = bootargs + strlen(bootargs);
 
-		linux_cmdline_set(bootargs, next - bootargs);
+		linux_cmdline_set(bootargs, next - bootargs, boot_type);
 
 		if (*next)
 			next++;
@@ -113,7 +142,8 @@  static void linux_cmdline_legacy(bootm_headers_t *images)
 	}
 }
 
-static void linux_cmdline_append(bootm_headers_t *images)
+static void linux_cmdline_append(bootm_headers_t *images,
+				 enum legacy_boot_type boot_type)
 {
 	char buf[24];
 	ulong mem, rd_start, rd_size;
@@ -121,7 +151,7 @@  static void linux_cmdline_append(bootm_headers_t *images)
 	/* append mem */
 	mem = gd->ram_size >> 20;
 	sprintf(buf, "mem=%luM", mem);
-	linux_cmdline_set(buf, strlen(buf));
+	linux_cmdline_set(buf, strlen(buf), boot_type);
 
 	/* append rd_start and rd_size */
 	rd_start = images->initrd_start;
@@ -129,9 +159,13 @@  static void linux_cmdline_append(bootm_headers_t *images)
 
 	if (rd_size) {
 		sprintf(buf, "rd_start=0x%08lX", rd_start);
-		linux_cmdline_set(buf, strlen(buf));
+		linux_cmdline_set(buf, strlen(buf), boot_type);
 		sprintf(buf, "rd_size=0x%lX", rd_size);
-		linux_cmdline_set(buf, strlen(buf));
+		linux_cmdline_set(buf, strlen(buf), boot_type);
+		if (boot_type ==  LEGACY_BOOT_VCORE) {
+			sprintf(buf, "root=/dev/ram0");
+			linux_cmdline_set(buf, strlen(buf), boot_type);
+		}
 	}
 }
 
@@ -276,11 +310,15 @@  static void boot_prep_linux(bootm_headers_t *images)
 		boot_reloc_fdt(images);
 		boot_setup_fdt(images);
 	} else {
-		if (CONFIG_IS_ENABLED(MIPS_BOOT_CMDLINE_LEGACY)) {
-			linux_cmdline_legacy(images);
+		if (CONFIG_IS_ENABLED(SOC_VCOREIII)) {
+			linux_cmdline_legacy(images, LEGACY_BOOT_VCORE);
+			linux_cmdline_append(images, LEGACY_BOOT_VCORE);
+			linux_cmdline_dump();
+		} else if (CONFIG_IS_ENABLED(MIPS_BOOT_CMDLINE_LEGACY)) {
+			linux_cmdline_legacy(images, 0);
 
 			if (!CONFIG_IS_ENABLED(MIPS_BOOT_ENV_LEGACY))
-				linux_cmdline_append(images);
+				linux_cmdline_append(images, LEGACY_BOOT_YAML);
 
 			linux_cmdline_dump();
 		}