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

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

The following reply was made to PR port-evbarm/47250; it has been noted by 
GNATS.

From: SAITOH Masanobu <msaitoh%execsw.org@localhost>
To: gnats-bugs%NetBSD.org@localhost
Cc: toby.karyadi%gmail.com@localhost, 
port-evbarm-maintainer%netbsd.org@localhost, 
 gnats-admin%netbsd.org@localhost, netbsd-bugs%netbsd.org@localhost, 
msaitoh%execsw.org@localhost
Subject: Re: port-evbarm/47250: Add passing of bootargs from U-Boot for Marvell
 SheevaPlug
Date: Sat, 01 Dec 2012 04:22:40 +0900

 (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)