NetBSD Problem Report #47250

From www@NetBSD.org  Mon Nov 26 19:47:45 2012
Return-Path: <www@NetBSD.org>
Received: from mail.netbsd.org (mail.netbsd.org [149.20.53.66])
	by www.NetBSD.org (Postfix) with ESMTP id 116B663E915
	for <gnats-bugs@gnats.NetBSD.org>; Mon, 26 Nov 2012 19:47:45 +0000 (UTC)
Message-Id: <20121126194743.DAC0B63E915@www.NetBSD.org>
Date: Mon, 26 Nov 2012 19:47:43 +0000 (UTC)
From: toby.karyadi@gmail.com
Reply-To: toby.karyadi@gmail.com
To: gnats-bugs@NetBSD.org
Subject: Add passing of bootargs from U-Boot for Marvell SheevaPlug 
X-Send-Pr-Version: www-1.0

>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:          closed
>Class:          change-request
>Submitter-Id:   net
>Arrival-Date:   Mon Nov 26 19:50:00 +0000 2012
>Closed-Date:    Sun Dec 02 18:22:59 +0000 2012
>Last-Modified:  Sun Dec 02 18:25:13 +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:/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();

>Release-Note:

>Audit-Trail:
From: SAITOH Masanobu <msaitoh@execsw.org>
To: gnats-bugs@NetBSD.org
Cc: toby.karyadi@gmail.com, port-evbarm-maintainer@netbsd.org, 
 gnats-admin@netbsd.org, netbsd-bugs@netbsd.org, msaitoh@execsw.org
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 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:/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
                                  msaitoh@netbsd.org)

From: "SAITOH Masanobu" <msaitoh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/47250 CVS commit: src/sys/arch/evbarm/marvell
Date: Sun, 2 Dec 2012 18:20:21 +0000

 Module Name:	src
 Committed By:	msaitoh
 Date:		Sun Dec  2 18:20:21 UTC 2012

 Modified Files:
 	src/sys/arch/evbarm/marvell: marvell_machdep.c

 Log Message:
 Get bootargs and parse them. This was accidentally removed in rev. 1.17.
 Fixes PR#47250.


 To generate a diff of this commit:
 cvs rdiff -u -r1.19 -r1.20 src/sys/arch/evbarm/marvell/marvell_machdep.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: open->closed
State-Changed-By: msaitoh@NetBSD.org
State-Changed-When: Sun, 02 Dec 2012 18:22:59 +0000
State-Changed-Why:
Fixed. Thanks.


From: "SAITOH Masanobu" <msaitoh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/47250 CVS commit: src/sys/arch/evbarm/evbarm
Date: Sun, 2 Dec 2012 18:22:45 +0000

 Module Name:	src
 Committed By:	msaitoh
 Date:		Sun Dec  2 18:22:45 UTC 2012

 Modified Files:
 	src/sys/arch/evbarm/evbarm: autoconf.c

 Log Message:
 Add get_device(), set_root_device() to get root device via bootargs.
 This is same as some other archs. PR#47250.


 To generate a diff of this commit:
 cvs rdiff -u -r1.14 -r1.15 src/sys/arch/evbarm/evbarm/autoconf.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

>Unformatted:

Home	PR Database Search