From: Andres Salomon on
On Tue, 29 Jun 2010 00:50:08 -0700
Grant Likely <grant.likely(a)secretlab.ca> wrote:

> On Mon, Jun 28, 2010 at 7:00 PM, Andres Salomon <dilinger(a)queued.net> wrote:
> >
> > Stick code into drivers/of/pdt.c (Prom Device Tree) that other
> > architectures with OpenFirmware resident in memory can make use of.
> >
> > Signed-off-by: Andres Salomon <dilinger(a)queued.net>
>
> Hi Andres,
>
> The patch itself looks fine, but there are currently two methods for
> extracting the device tree from open firmware; one in arch/powerpc
> using the flattened format, and one in arch/sparc. I don't want to
> end up maintaining both methods in drivers/of, and there has also some
> discussions on moving the powerpc version into common code. I've been
> thinking about using the powerpc approach to support ARM platforms
> using both the flat tree and real OFW.
>
> Ben, what say you? Before I do anything I'd like to have your opinion.
>

So you're saying that you want ARM (and sparc, and OLPC) to generate a
flat tree by calling into OFW? Sparc and OLPC have very similar
mechanisms for getting device tree info from OFW, so it makes sense to
share code between them.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Andres Salomon on
On Tue, 29 Jun 2010 15:42:54 -0600
Grant Likely <grant.likely(a)secretlab.ca> wrote:

> On Tue, Jun 29, 2010 at 9:03 AM, Andres Salomon <dilinger(a)queued.net>
> wrote:
[...]
>
> > Sparc and OLPC have very similar
> > mechanisms for getting device tree info from OFW, so it makes sense
> > to share code between them.
>
> Other than the flattened tree step; is the powerpc method dissimilar
> from the Sparc and OLPC method for talking to OFW? (This is not a
> rhetorical question, I'm want to know if I'm missing some details).
> The main difference I know about is that OFW can still kept alive at
> runtime for sparc, which powerpc does not do. However, keeping OFW
> callable is a separate issue from how to extract the device tree.
>

After having a look at powerpc's flatten_device_tree, I don't see any
obvious reason why OLPC couldn't use this (though it still strikes me
as weird to go from prom->fdt->dt when the option of prom->dt is
available and less complex). Do you already have the patches that put
this into drivers/of/?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Andres Salomon on
On Wed, 30 Jun 2010 15:52:51 -0600
Grant Likely <grant.likely(a)secretlab.ca> wrote:

> On Tue, Jun 29, 2010 at 5:36 PM, Andres Salomon <dilinger(a)queued.net>
> wrote:
> > On Tue, 29 Jun 2010 15:42:54 -0600
> > Grant Likely <grant.likely(a)secretlab.ca> wrote:
> >
> >> On Tue, Jun 29, 2010 at 9:03 AM, Andres Salomon
> >> <dilinger(a)queued.net> wrote:
> > [...]
> >>
> >> > �Sparc and OLPC have very similar
> >> > mechanisms for getting device tree info from OFW, so it makes
> >> > sense to share code between them.
> >>
> >> Other than the flattened tree step; is the powerpc method
> >> dissimilar from the Sparc and OLPC method for talking to OFW?
> >> �(This is not a rhetorical question, I'm want to know if I'm
> >> missing some details). The main difference I know about is that
> >> OFW can still kept alive at runtime for sparc, which powerpc does
> >> not do. �However, keeping OFW callable is a separate issue from
> >> how to extract the device tree.
> >>
> >
> > After having a look at powerpc's flatten_device_tree, I don't see
> > any obvious reason why OLPC couldn't use this
>
> Okay.
>
> >(though it still strikes me
> > as weird to go from prom->fdt->dt when the option of prom->dt is
> > available and less complex).
>
> Which is why I didn't simply NACK it out of hand. It is a valid
> point.
>
> I'd probably be happier if both fdt and pdt used a shared set of
> utility functions for allocating and tying together the device_node
> data structures. Then at least there would be only one instance of
> code to deal with the fiddly data structure interconnections.

The problem with unifying it is that both methods have different
assumptions; the fdt determines the size of the unflattened dt in
advance, and allocates a contiguous block of memory up front (the
memory block being smaller than the pdt memory usage because various
strings are already present in the fdt, so the unflattened tree simply
points at those strings). The pdt, otoh, allocates more memory as the
tree is discovered (by calling into the prom).

Possible options include:

- For the pdt, calling into the prom twice for each property/node,
getting the size of everything during the first tree traversal,
allocating memory, and then creating the tree during the second
traversal. This is slow and I don't see much point to it.

- For the pdt, calling into the prom once for each property/node to
create a fdt, and then unflattening it. This is better than the
previous option, but I don't think the prom->fdt code will be very
nice.

- Changing the fdt code to allocate memory more dynamically; rather
than a first run through the fdt to determine the unflattened tree
size, simply allocate memory chunks as we run through the tree.
Memory might still be backed the same way (calling
early_init_dt_alloc_memory_arch to grab a chunk of memory, and then
allocating chunks of it via unflatten_dt_alloc), but the difference
would be that it wouldn't necessarily be contiguous, and in how it's
done. For example, early_init_dt_alloc_memory_arch might be called
initially to allocate a 16k chunk, and once unflatten_dt_alloc runs
out of memory in that chunk, another 16k chunk might be allocated.
This would also translate to sparc's prom_early_alloc being used to
allocate the chunk, and then node/property structs being
allocated from those chunks. (The 16k choice
is completely arbitrary; people more familiar w/ the early memory
setup of ppc/sparc might have a better suggestion)

That last option is what I think makes the most sense; however, are
there technical reasons (other than how unflatten_device_tree is
written) that the unflattened dt must be in a contiguous memory
region? The chunk allocation code would be backed by things like
__alloc_bootmem and lmb_alloc, and the dt_alloc code would be new code
that simply tracks the amount remaining in a list of chunks, and
allocates a new chunk if necessary.


>
> The nice thing about the powerpc version is that the tree can be
> extracted really early in the boot process, before any of the core
> kernel infrastructure or memory management is initialized.
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Andres Salomon on
On Tue, 6 Jul 2010 03:21:21 -0600
Grant Likely <grant.likely(a)secretlab.ca> wrote:

> On Mon, Jun 28, 2010 at 8:00 PM, Andres Salomon <dilinger(a)queued.net>
> wrote:
> >
> > Stick code into drivers/of/pdt.c (Prom Device Tree) that other
> > architectures with OpenFirmware resident in memory can make use of.
> >
> > Signed-off-by: Andres Salomon <dilinger(a)queued.net>
>
> Some more comments below...
>

Thanks for the review!



> > �arch/sparc/Kconfig � � � � � � �| � �1 +
> > �arch/sparc/include/asm/prom.h � | � 15 +++-
> > �arch/sparc/kernel/prom.h � � � �| � 14 ---
> > �arch/sparc/kernel/prom_common.c | �173
> > +------------------------------ drivers/of/Kconfig � � � � � � �|
> > �4 + drivers/of/Makefile � � � � � � | � �1 +
> > �drivers/of/pdt.c � � � � � � � �| �225
> > +++++++++++++++++++++++++++++++++++++++ include/linux/of_pdt.h
> > � � �| � 37 +++++++ 8 files changed, 282 insertions(+), 188
> > deletions(-) create mode 100644 drivers/of/pdt.c
> > �create mode 100644 include/linux/of_pdt.h
> >
> > diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
> > index 6f1470b..b4cb63b 100644
> > --- a/arch/sparc/Kconfig
> > +++ b/arch/sparc/Kconfig
> > @@ -150,6 +150,7 @@ config ARCH_NO_VIRT_TO_BUS
> >
> > �config OF
> > � � � �def_bool y
> > + � � � select OF_PROMTREE
> >
> > �config ARCH_SUPPORTS_DEBUG_PAGEALLOC
> > � � � �def_bool y if SPARC64
> > diff --git a/arch/sparc/include/asm/prom.h
> > b/arch/sparc/include/asm/prom.h index f845828..0834c2a 100644
> > --- a/arch/sparc/include/asm/prom.h
> > +++ b/arch/sparc/include/asm/prom.h
> > @@ -18,6 +18,7 @@
> > �* 2 of the License, or (at your option) any later version.
> > �*/
> > �#include <linux/types.h>
> > +#include <linux/of_pdt.h>
> > �#include <linux/proc_fs.h>
> > �#include <linux/mutex.h>
> > �#include <asm/atomic.h>
> > @@ -65,8 +66,18 @@ extern struct device_node *of_console_device;
> > �extern char *of_console_path;
> > �extern char *of_console_options;
> >
> > -extern void (*prom_build_more)(struct device_node *dp, struct
> > device_node ***nextp); -extern char *build_full_name(struct
> > device_node *dp); +/* stuff used by of/pdt */
> > +extern void * prom_early_alloc(unsigned long size);
> > +extern void irq_trans_init(struct device_node *dp);
> > +extern char *build_path_component(struct device_node *dp);
> > +
> > +extern char *prom_firstprop(int node, char *buffer);
> > +extern char *prom_nextprop(int node, const char *oprop, char
> > *buffer); +extern int prom_getproplen(int node, const char *prop);
> > +extern int prom_getproperty(int node, const char *prop,
> > + � � � � � � � � � � � � � char *buffer, int bufsize);
> > +extern int prom_getchild(int node);
> > +extern int prom_getsibling(int node);
>
> These become the API required by of/pdt. They should be defined in a
> arch-independent header file. Something like include/linux/of_pdt.h
>
> Right now only OLPC will be using this, so static function definitions
> are just fine. However, if there is ever more than one method for
> talking to OFW, then these hooks will need to be converted into an ops
> structure so the right one can be passed in at runtime.
>

Note that sparc and OLPC actually use slightly different function
signatures; OLPC uses phandles for nodes, while sparc uses ints. Not a
huge difference, but enough that I didn't want to mess w/ a generic
version of it early in the process. I agree that op structs would be
nicer, and will probably move towards that.

[...]

> > diff --git a/include/linux/of_pdt.h b/include/linux/of_pdt.h
> > new file mode 100644
> > index 0000000..f62616e
> > --- /dev/null
> > +++ b/include/linux/of_pdt.h
> > @@ -0,0 +1,37 @@
> > +#include <linux/of.h> �/* linux/of.h gets to determine #include
> > ordering */
>
> Do you really need this #include in this way? Can it be moved inside
> the #ifndef OF_PDT block below?
>

Not sure, will try out different variants and see what breaks.

[...]

> Another general comment, I'm still not thrilled with this code having
> its own independent method for building the tree, but I doubt the
> existing add/remove nodes and properties code is usable early enough
> to be suitable for sparc. How early do you extract the device tree on
> OLPC? How are you going to use the data?

Not that early; very early code fetches information necessary to call
into the PROM, and ensures that the kernel doesn't clobber OFW's
memory. After that, we can build the dt at any point during init.
The data is to be exported via proc for userspace to use in
determining hardware (and firmware) info.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Andres Salomon on
On Tue, 6 Jul 2010 16:06:54 -0600
Grant Likely <grant.likely(a)secretlab.ca> wrote:
[...]
> Okay, so it is only the userspace interface that you're interested in,
> correct? You have no needs/plans (as of yet) to register devices out
> of the device tree?
>

Correct.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/