Re: port-evbarm/47250: Add passing of bootargs from U-Boot for Marvell SheevaPlug

[SAITOH Masanobu <msaitoh%execsw.org@localhost>]

(2012/11/27 4:50), toby.karyadi%gmail.com@localhost wrote:
>> Number:         47250
>> Category:       port-evbarm
>> Synopsis:       Add passing of bootargs from U-Boot for Marvell SheevaPlug
>> Confidential:   no
>> Severity:       non-critical
>> Priority:       medium
>> Responsible:    port-evbarm-maintainer
>> State:          open
>> Class:          change-request
>> Submitter-Id:   net
>> Arrival-Date:   Mon Nov 26 19:50:00 +0000 2012
>> Originator:     Toby Karyadi
>> Release:        current
>> Organization:
>> Environment:
> NetBSD kowari.simplecubes.com 6.99.15 NetBSD 6.99.15 (SHEEVAAXE) #6: Mon Nov 
> 26 12:36:10 EST 2012  
> archie%zutu.simplecubes.com@localhost:/mnt/v01/build/src/current/2012.11.17.05.20.00/obj/sys/arch/evbarm/compile/SHEEVAAXE
>  evbarm
> 
>> Description:
> The intent of the patchset below is to allow specifying the root device and 
> boothowto via U-Boot's special bootargs environment variable. 
> 
> Example:
> Marvell>> env print bootcmd
> bootcmd=mmc init; fatload mmc 0:1 0x1000000 netbsd.ub; setenv bootargs 
> root=ld0a; bootm 0x1000000;
> 
> With the bootcmd U-Boot env var setup like above, U-Boot's 'boot' cmd would 
> just boot up my netbsd filesystem on the sdcard. It would also allow booting 
> up in single user mode, break in the debugger, etc, e.g.:
> 
> # Start in single user mode
> setenv bootargs root=ld0a -s
> # ... or
> setenv bootargs root=ld0a single
> # Invoke kernel debugger
> setenv bootargs root=ld0a kdb
> 
> I believe most of the boot arg options are defined in 
> arm32_machdep.c:parse_mi_bootargs(), I might be wrong, but at any rate, they 
> are similar to the ones described in boot(8).
> 
> The code in evbarm/evbarm/autoconf.c is basically swipped/modeled after other 
> archs, like acorn32, hpcarm, but specifically shark/shark. I don't know how 
> this affects other evbarm based devices, but I suspect it's benign, but I 
> have no way of testing it. In netbsd-6, the changes in 
> evbarm/evbarm/autoconf.c would have been sufficient, however, recent boot 
> init code refactoring work between beagle and marvell also seemed to have 
> accidentally removed the code to setup the boot_args value based on the 
> bootargs value read in from U-Boot in marvell_machdep.c. 
> 
> I've tested the code and it works as intended, but please review. The patch 
> applies cleanly against current as of 2012.11.17.05.20.00.
>> How-To-Repeat:
> N/A.
>> Fix:
> --- src/sys/arch/evbarm/marvell/marvell_machdep.c.orig  2012-10-22 
> 11:43:32.000000000 -0400
> +++ src/sys/arch/evbarm/marvell/marvell_machdep.c       2012-11-26 
> 13:30:04.000000000 -0500
> @@ -375,6 +375,10 @@
>         /* we've a specific device_register routine */
>         evbarm_device_register = marvell_device_register;
>  
> +       /* parse bootargs from U-Boot */
> +       boot_args = bootargs;
> +       parse_mi_bootargs(boot_args);
> +
>         return initarm_common(KERNEL_VM_BASE, KERNEL_VM_SIZE, NULL, 0);
>  }
>  
> --- src/sys/arch/evbarm/evbarm/autoconf.c.orig  2012-10-27 13:17:46.000000000 
> -0400
> +++ src/sys/arch/evbarm/evbarm/autoconf.c       2012-11-26 13:09:31.000000000 
> -0500
> @@ -45,15 +45,73 @@
>  
>  #include <machine/autoconf.h>
>  #include <machine/intr.h>
> +#include <machine/bootconfig.h>
>  
>  void   (*evbarm_device_register)(device_t, void *);
>  
> +#ifndef MEMORY_DISK_IS_ROOT
> +static void get_device(char *name);
> +static void set_root_device(void);
> +#endif
> +
> +#ifndef MEMORY_DISK_IS_ROOT
> +/* Decode a device name to a major and minor number */
> +
> +static void
> +get_device(char *name)
> +{
> +       int unit, part;
> +       char devname[16], *cp;
> +       device_t dv;
> +       
> +       if (strncmp(name, "/dev/", 5) == 0)
> +               name += 5;
> +       
> +       if (devsw_name2blk(name, devname, sizeof(devname)) == -1)
> +               return;
> +       
> +       name += strlen(devname);
> +       unit = part = 0;
> +       
> +       cp = name;
> +       while (*cp >= '0' && *cp <= '9')
> +               unit = (unit * 10) + (*cp++ - '0');
> +       if (cp == name)
> +               return;
> +       
> +       if (*cp >= 'a' && *cp <= ('a' + MAXPARTITIONS))
> +               part = *cp - 'a';
> +       else if (*cp != '\0' && *cp != ' ')
> +               return;
> +       if ((dv = device_find_by_driver_unit(devname, unit)) != NULL) {
> +               booted_device = dv;
> +               booted_partition = part;
> +       }
> +}
> +
> +
> +/* Set the rootdev variable from the root specifier in the boot args */
> +
> +static void
> +set_root_device(void)
> +{
> +       char *ptr;
> +
> +       if (boot_args &&
> +               get_bootconf_option(boot_args, "root", BOOTOPT_TYPE_STRING, 
> &ptr))
> +               get_device(ptr);
> +}
> +#endif
> +
>  /*
>   * Set up the root device from the boot args
>   */
>  void
>  cpu_rootconf(void)
>  {
> +#ifndef MEMORY_DISK_IS_ROOT
> +        set_root_device();
> +#endif
>         aprint_normal("boot device: %s\n",
>             booted_device != NULL ? device_xname(booted_device) : 
> "<unknown>");
>         rootconf();

 I verified that this patch works on my OPEN-RD.

 This request has two changes:

        1) Add missing bootargs stuff for marvell which was accidentally removed
           from marvell_machdep.c by rev. 1.17.

        2) Add get_device(), set_root_device() and use on evbarm like some 
other archs.

Offcourse 1) is ok and I think 2) is appropriate.

 If no objection, I'll commit it.

-- 
-----------------------------------------------
                SAITOH Masanobu (msaitoh%execsw.org@localhost
                                 msaitoh%netbsd.org@localhost)