From owner-freebsd-arm@FreeBSD.ORG Mon Mar 17 15:25:44 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 3B086672; Mon, 17 Mar 2014 15:25:44 +0000 (UTC) Received: from mail-qg0-x22f.google.com (mail-qg0-x22f.google.com [IPv6:2607:f8b0:400d:c04::22f]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id C609E160; Mon, 17 Mar 2014 15:25:43 +0000 (UTC) Received: by mail-qg0-f47.google.com with SMTP id 63so547827qgz.6 for ; Mon, 17 Mar 2014 08:25:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:references:mime-version:in-reply-to:content-type :content-transfer-encoding:message-id:cc:from:subject:date:to; bh=CDx8H3VpNT6cP9DdgPN60/O2KSUq3Ih2eR5hYirtKJs=; b=jypqfRIfocYGUAwuLcaXt/V0UYn4D5Vi4qk4dXtw08DVSv+YJd8ehjRz2sJeYhuCo8 L4pUU3UjU7fNi3sa1Aa6TPmGHhCNlMqpOfdhiiGJNNrYq5sxCdbMbyACdE94bigyVceF YhPsN8tzxz/ZPY/7eE1LmM3ERoBTC0E7mMVhNdnZobSZ6cTIcKXneyyHiQ5/B1PbEMrl /2YYoaPlT1/z83jj+onFddeBbHNAjyWx8dr2JqdBU9z7spVYxHelfadwKg4+WZRNEW/A cE+yvufl4pPkQDkMou0XYq41HmgblsTs1qihtNGKydjzXJTYrqm3EdSZYfl9UEk/psKU uWrQ== X-Received: by 10.140.102.215 with SMTP id w81mr2529996qge.109.1395069942330; Mon, 17 Mar 2014 08:25:42 -0700 (PDT) Received: from [172.16.0.103] (c-174-54-20-106.hsd1.pa.comcast.net. [174.54.20.106]) by mx.google.com with ESMTPSA id 30sm21497537qgt.4.2014.03.17.08.25.40 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Mon, 17 Mar 2014 08:25:40 -0700 (PDT) Sender: Patrick Kelsey References: <1395064661.1149.565.camel@revolution.hippie.lan> <1395067218.1149.570.camel@revolution.hippie.lan> Mime-Version: 1.0 (1.0) In-Reply-To: <1395067218.1149.570.camel@revolution.hippie.lan> Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: quoted-printable Message-Id: <80EF2F0E-B4E3-4E06-933A-685F78F2EBD7@ieee.org> X-Mailer: iPhone Mail (10B329) From: Patrick Kelsey Subject: Re: Booting FreeBSD from eMMC on BeagleBone Black Date: Mon, 17 Mar 2014 11:25:37 -0400 To: Ian Lepore 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 15:25:44 -0000 On Mar 17, 2014, at 10:40 AM, Ian Lepore wrote: > On Mon, 2014-03-17 at 10:21 -0400, Patrick Kelsey wrote: >> On Mon, Mar 17, 2014 at 9:57 AM, Ian Lepore wrote: >>=20 >>> On Sun, 2014-03-16 at 22:16 -0400, Patrick Kelsey wrote: >>>> On Sun, Mar 16, 2014 at 9:33 PM, Rui Paulo wrote: >>>>=20 >>>>> On 16 Mar 2014, at 14:59, Patrick Kelsey wrote: >>>>>=20 >>>>>> - Improved disk probing support that will now by default find and >>> use the >>>>>> first suitable partition among the available storage devices. >>>>>=20 >>>>> I think this introduced a bug where, if you have a non-responsive boot= >>>>> device, ubldr will stop and won't try network booting: >>>>>=20 >>>>> ## Starting application at 0x01000054 ... >>>>> Consoles: U-Boot console >>>>> Compatible API signature found @1d800a8 >>>>> Number of U-Boot devices: 2 >>>>>=20 >>>>> 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=3D0 slice=3D0 partition=3D-1...disk0: read failed, error= =3D1 >>>>>=20 >>>>> Checking unit=3D1 slice=3D0 partition=3D-1... >>>>> Checking unit=3D2 slice=3D0 partition=3D-1... >>>>> Checking unit=3D3 slice=3D0 partition=3D-1... >>>>> Checking unit=3D4 slice=3D0 partition=3D-1... >>>>> Checking unit=3D5 slice=3D0 partition=3D-1... >>>>>=20 >>>>> can't load 'kernel' >>>>>=20 >>>>> Type '?' for a list of commands, 'help' for more detailed help. >>>>> loader> >>>>>=20 >>>>> 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 I= an >>>> did when integrating my patch. >>>>=20 >>>> I believe sys/boot/uboot/common/main.c, starting around line 442, shoul= d >>>> look like this: >>>>=20 >>>> if (strcmp(devsw[i]->dv_name, "disk") =3D=3D 0 && >>>> (load_type =3D=3D -1 || (load_type & DEV_TYP_STOR))) {= >>>> if (probe_disks(i, load_type, load_unit, >>> load_slice, >>>> load_partition) =3D=3D 0) >>>> break; >>>> } >>>>=20 >>>> if (strcmp(devsw[i]->dv_name, "net") =3D=3D 0 && >>>> (load_type =3D=3D -1 || (load_type & DEV_TYP_NET))) >>>> break; >>>>=20 >>>>=20 >>>> Can you give that a try? >>>>=20 >>>> -Patrick >>>=20 >>> 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. >>>=20 >>> 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 t= he >> "net" conditional that are the issue - I had the order of the tests in th= e >> "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 s= o >> that a load_type of -1 cannot short circuit it in a loop iteration that i= s >> 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. >>=20 >> -Patrick >=20 > I don't think so. With the parens nested correctly now, there's no > difference between >=20 > if ((condition A) && (condition B)) versus=20 > if ((condition B) && (condition A)) >=20 > 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). >=20 > Plus I've done something radical and actually tested it. :) >=20 > 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=3D0 slice=3D partition=3D...disk0: real size !=3D= size >=20 > Checking unit=3D1 slice=3D partition=3D... > Checking unit=3D2 slice=3D partition=3D... > Checking unit=3D3 slice=3D partition=3D... > Checking unit=3D4 slice=3D partition=3D... > Checking unit=3D5 slice=3D partition=3D... > Found U-Boot device: net > | > /boot/kernel/kernel data=3D0x4e3a08+0x305f8 syms=3D[0x4+0x7ee30+0x4+0x4e3d= 9] > Hit [Enter] to boot immediately, or any other key for command prompt. >=20 Speaking of short-circuit... I'm with you (and moving bedtime up a couple of= hours tonight). The saving grace is that I was wrong about my being wrong?= I guess I should have just thrown you under the bus alone :)=