From owner-freebsd-arm@FreeBSD.ORG Mon Mar 17 14:40:23 2014 Return-Path: Delivered-To: freebsd-arm@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 0662C7AC; Mon, 17 Mar 2014 14:40:23 +0000 (UTC) Received: from mho-01-ewr.mailhop.org (mho-03-ewr.mailhop.org [204.13.248.66]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id BB899BC7; Mon, 17 Mar 2014 14:40:22 +0000 (UTC) Received: from c-24-8-230-52.hsd1.co.comcast.net ([24.8.230.52] helo=damnhippie.dyndns.org) by mho-01-ewr.mailhop.org with esmtpsa (TLSv1:AES256-SHA:256) (Exim 4.72) (envelope-from ) id 1WPYiD-000OjJ-Jr; Mon, 17 Mar 2014 14:40:21 +0000 Received: from [172.22.42.240] (revolution.hippie.lan [172.22.42.240]) by damnhippie.dyndns.org (8.14.3/8.14.3) with ESMTP id s2HEeItd067389; Mon, 17 Mar 2014 08:40:18 -0600 (MDT) (envelope-from ian@FreeBSD.org) X-Mail-Handler: Dyn Standard SMTP by Dyn X-Originating-IP: 24.8.230.52 X-Report-Abuse-To: abuse@dyndns.com (see http://www.dyndns.com/services/sendlabs/outbound_abuse.html for abuse reporting information) X-MHO-User: U2FsdGVkX1+e0pPy4851OM+DZ7V+Rwbo Subject: Re: Booting FreeBSD from eMMC on BeagleBone Black From: Ian Lepore To: Patrick Kelsey In-Reply-To: References: <1395064661.1149.565.camel@revolution.hippie.lan> Content-Type: text/plain; charset="us-ascii" Date: Mon, 17 Mar 2014 08:40:18 -0600 Message-ID: <1395067218.1149.570.camel@revolution.hippie.lan> Mime-Version: 1.0 X-Mailer: Evolution 2.32.1 FreeBSD GNOME Team Port Content-Transfer-Encoding: 7bit Cc: freebsd-arm X-BeenThere: freebsd-arm@freebsd.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: "Porting FreeBSD to ARM processors." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 17 Mar 2014 14:40:23 -0000 On Mon, 2014-03-17 at 10:21 -0400, Patrick Kelsey wrote: > On Mon, Mar 17, 2014 at 9:57 AM, Ian Lepore wrote: > > > On Sun, 2014-03-16 at 22:16 -0400, Patrick Kelsey wrote: > > > On Sun, Mar 16, 2014 at 9:33 PM, Rui Paulo wrote: > > > > > > > On 16 Mar 2014, at 14:59, Patrick Kelsey wrote: > > > > > > > > > - Improved disk probing support that will now by default find and > > use the > > > > > first suitable partition among the available storage devices. > > > > > > > > I think this introduced a bug where, if you have a non-responsive boot > > > > device, ubldr will stop and won't try network booting: > > > > > > > > ## Starting application at 0x01000054 ... > > > > Consoles: U-Boot console > > > > Compatible API signature found @1d800a8 > > > > Number of U-Boot devices: 2 > > > > > > > > FreeBSD/armv6 U-Boot loader, Revision 1.2 > > > > (rpaulo@zedfs.local, Fri Mar 14 22:35:47 PDT 2014) > > > > DRAM: 256MB > > > > Unknown device type '' <------------ this is new > > > > Found U-Boot device: disk > > > > Probing all storage devices... > > > > Checking unit=0 slice=0 partition=-1...disk0: read failed, error=1 > > > > > > > > Checking unit=1 slice=0 partition=-1... > > > > Checking unit=2 slice=0 partition=-1... > > > > Checking unit=3 slice=0 partition=-1... > > > > Checking unit=4 slice=0 partition=-1... > > > > Checking unit=5 slice=0 partition=-1... > > > > > > > > can't load 'kernel' > > > > > > > > Type '?' for a list of commands, 'help' for more detailed help. > > > > loader> > > > > > > > > It stops here and doesn't try net0 booting. > > > > > > > > > > > I think the problem is that some of the conditionals in > > > sys/boot/uboot/common/main.c:main() are broken. I believe I sowed the > > seed > > > for this in the original patch I sent to Ian, which appears to have had > > an > > > out-of-order set of tests in the disk conditional, which in hindsight > > > turned out to work due to a friendly coincidence (namely disk appearing > > > before net in the devsw). That bad-pattern conditional seems to have > > > gotten munged a bit further and propagated in some of the refactoring Ian > > > did when integrating my patch. > > > > > > I believe sys/boot/uboot/common/main.c, starting around line 442, should > > > look like this: > > > > > > if (strcmp(devsw[i]->dv_name, "disk") == 0 && > > > (load_type == -1 || (load_type & DEV_TYP_STOR))) { > > > if (probe_disks(i, load_type, load_unit, > > load_slice, > > > load_partition) == 0) > > > break; > > > } > > > > > > if (strcmp(devsw[i]->dv_name, "net") == 0 && > > > (load_type == -1 || (load_type & DEV_TYP_NET))) > > > break; > > > > > > > > > Can you give that a try? > > > > > > -Patrick > > > > This was my bad. I think I was so enmeshed in whether the strange > > original strncmp() calls were really just a silly form of strcmp() (they > > were) that I didn't even notice I screwed up the overall logic. > > > > Rather than putting the strcmp() first as shown above, I just adjusted > > the parens in the network if() to be what I should have done originally. > > > > > I don't believe that approach will fix it. It's not just the parens in the > "net" conditional that are the issue - I had the order of the tests in the > "disk" conditional backwards to begin with. To get the desired behavior > (proper fallback to net), the test of devsw[]->dv_name needs to be first so > that a load_type of -1 cannot short circuit it in a loop iteration that is > for another type. Otherwise, when load_type is -1, coming out of an > unsuccessful probe_disks() will result in an exit from the loop via the > "net" conditional, and you will wind up out of the loop, but without > currdev set up properly for the net device (it will still be set up for > "disk", and with a possibly non-zero unit number). Putting the test of > devsw[]->dv_name first ensures that the loop won't be exited on behalf of a > given type without currdev being set up for that type. > > -Patrick I don't think so. With the parens nested correctly now, there's no difference between if ((condition A) && (condition B)) versus if ((condition B) && (condition A)) I've just always had a prejudice for testing the simple local-var conditions first with && conditions (even when performance doesn't matter, as in this case). Plus I've done something radical and actually tested it. :) MMC: no card present Number of U-Boot devices: 2 U-Boot env: loaderdev not set, will probe all devices. Found U-Boot device: disk Probing all disk devices... Checking unit=0 slice= partition=...disk0: real size != size Checking unit=1 slice= partition=... Checking unit=2 slice= partition=... Checking unit=3 slice= partition=... Checking unit=4 slice= partition=... Checking unit=5 slice= partition=... Found U-Boot device: net | /boot/kernel/kernel data=0x4e3a08+0x305f8 syms=[0x4+0x7ee30+0x4+0x4e3d9] Hit [Enter] to boot immediately, or any other key for command prompt. -- Ian