Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 14 Sep 2013 10:27:35 -0700
From:      John-Mark Gurney <jmg@funkthat.com>
To:        Ian Lepore <ian@freebsd.org>
Cc:        freebsd-sparc64 <freebsd-sparc64@freebsd.org>, freebsd-arm@freebsd.org, freebsd-ppc@freebsd.org
Subject:   Re: Patch for Cross-Reference Phandles
Message-ID:  <20130914172735.GX68682@funkthat.com>
In-Reply-To: <1379173405.1197.7.camel@revolution.hippie.lan>
References:  <522CC7E0.9060508@freebsd.org> <5231D8A6.9080501@freebsd.org> <20130914133155.GA32845@alchemy.franken.de> <52346D71.4080407@freebsd.org> <1379173405.1197.7.camel@revolution.hippie.lan>

next in thread | previous in thread | raw e-mail | index | archive | help
Ian Lepore wrote this message on Sat, Sep 14, 2013 at 09:43 -0600:
> On Sat, 2013-09-14 at 09:06 -0500, Nathan Whitehorn wrote:
> > On 09/14/13 08:31, Marius Strobl wrote:
> > > On Thu, Sep 12, 2013 at 10:07:18AM -0500, Nathan Whitehorn wrote:
> > >> On 09/08/13 13:54, Nathan Whitehorn wrote:
> > >>> Open Firmware has three namespaces for handles:
> > >>> 1. Instance handles, for open devices
> > >>> 2. Package handles for the client interface
> > >>> 3. Package handles for device tree cross references
> > >>>
> > >>> On Powermac hardware, we assume that (2) and (3) are identical and
> > >>> call both phandles. On embedded FDT systems, you can't open devices
> > >>> and so we abuse ihandle_t for (3). IBM pSeries hardware, however, has
> > >>> all three things. With that in mind, I'd like to start separating
> > >>> them. The patch at
> > >>> http://people.freebsd.org/~nwhitehorn/xref_phandle.diff adds a new
> > >>> function (OF_child_xref_phandle) that takes a phandle of type (3) and
> > >>> turns into one of type (2) by searching for entries named "phandle",
> > >>> "ibm,phandle", or "linux,phandle" in the tree. This should work for
> > >>> FDT as well, but is not connected in the patch to anything actually
> > >>> FDT related.
> > >>>
> > >>> Comments would be appreciated. I'd like to get to get this as in as
> > >>> soon as possible (given the HEAD freeze) otherwise.
> > >>> -Nathan
> > >> Since I haven't heard anything, it shouldn't affect any existing
> > >> platforms or code, and it would be nice to have this interface available
> > >> in the 10.x series, I plan to ask re@ for commit approval tomorrow.
> > >> Please let me know if there are any objections.
> > > Technically that patch is fine. It would be nice if you could fix the
> > > style bugs before committing, though. In OF_child_xref_phandle(), the
> > > variables should go above the comment and you don't need to initialize
> > > the static global one in ofw_pcibus.c to zero explicitly. If you do
> > > nevertheless, spaces should go before and after the '='.
> > >
> > 
> > Thanks! I think the explicit zero initialization adds some clarity for 
> > people reading the code, so I'd like to keep it. The rest of the changes 
> > have been made.
> > -Nathan
> 
> The explicit zero init also makes it 4 bytes harder to fit the kernel
> into a small flash on embedded systems.  We should be eliminating
> zero-inits that move items from bss to data, not adding new ones.

I just ran a simple test:
int foo = 0;

and compiled it, both with:
FreeBSD clang version 3.3 (tags/RELEASE_33/final 183502) 20130610
Target: x86_64-unknown-freebsd10.0
Thread model: posix

and:
FreeBSD clang version 3.3 (trunk 178860) 20130405
Target: armv6--freebsd
Thread model: posix

and:
gcc (GCC) 4.2.1 20070831 patched [FreeBSD]
Copyright (C) 2007 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

all are smart enough to know that zero init is the same as locating it
in bss, and do so... and not put it in data...  Oh, these were all
compiled w/o any options, just <compile> -c foo.c...

-- 
  John-Mark Gurney				Voice: +1 415 225 5579

     "All that I will do, has been done, All that I have, has not."



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20130914172735.GX68682>