[PATCH 1/3] fdt.c: Add non-boottime device tree functions

[Grant Likely]

On Wed, Nov 17, 2010 at 11:46:07AM -0800, Stephen Neuendorffer wrote:
> 
> 
> > -----Original Message-----
> > From: Grant Likely [mailto:glikely at secretlab.ca] On Behalf Of Grant
> Likely
> > Sent: Wednesday, November 17, 2010 11:42 AM
> > To: Stephen Neuendorffer
> > Cc: dirk.brandewie at gmail.com; devicetree-discuss at lists.ozlabs.org
> > Subject: Re: [PATCH 1/3] fdt.c: Add non-boottime device tree functions
> > 
> > On Wed, Nov 17, 2010 at 11:15:43AM -0800, Stephen Neuendorffer wrote:
> > > In preparation for providing run-time handling of device trees,
> factor
> > > out some of the basic functions so that they take an arbitrary blob,
> > > rather than relying on the single boot-time tree.
> > >
> > > Signed-off-by: Stephen Neuendorffer
> <stephen.neuendorffer at xilinx.com>
> > > ---
> > >  drivers/of/fdt.c       |  129
> ++++++++++++++++++++++++++++++-----------------
> > >  include/linux/of_fdt.h |    9 +++
> > >  2 files changed, 91 insertions(+), 47 deletions(-)
> > >
> > > diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> > > index c1360e0..6ae207a 100644
> > > --- a/drivers/of/fdt.c
> > > +++ b/drivers/of/fdt.c
> > > @@ -22,6 +22,86 @@
> > >
> > >  #include <asm/page.h>
> > >
> > > +char *fdt_get_string(u32 offset,
> > > +			  struct boot_param_header *blob)
> > 
> > nitpicking: Encouraged naming convention is of_flat_dt_* for this
> > file.  Also, I think it makes more sense for blob to be the first
> > argument in all of these functions.
> 
> OK... Half of the other functions don't follow that convention, tho...
> :)

I know.  It's a mess.  But I need to start with something to start
regaining sanity.  :)

>  
> > > +{
> > > +	return ((char *)blob) +
> > > +		be32_to_cpu(blob->off_dt_strings) + offset;
> > > +}
> > > +
> > > +/**
> > > + * of_get_flat_dt_prop_blob - Given a node in the given flat blob,
> return
> > > + * the property ptr
> > > + *
> > > + * This function can be used within scan_flattened_dt callback to
> get
> > > + * access to properties
> > > + */
> > > +void *fdt_get_property(unsigned long node, const char *name,
> > > +				 unsigned long *size,
> > > +				 struct boot_param_header *blob)
> > 
> > Colocate this function with the defintiion of
> > of_flat_dt_get_property()
> 
> I was trying to separate them lexically from the __init functions, to
> avoid confusion
> and with the perhaps later intention of putting the early stuff in a
> separate file.

Fair enough, but if so then split the reorg into a separate patch so
that this patch shows only the functional changes.  Moving stuff
around makes the diff much larger than it needs to be.

> 
> > > +{
> > > +	unsigned long p = node;
> > 
> > I think 'p' can be made type void* to eliminate all the casting in
> > this function (although I understand that this patch just moves code;
> > so that change doesn't need to be in this patch.
> 
> I don't intend to make such changes.  (I agree that it would be nice
> if the tree no longer references the blob, however).

I'm okay with that.  Mostly I was thinking out loud that it needs to
be done.  It is *not* a blocker on this patch.

g.