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

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