[PATCH 1/3] fdt.c: Add non-boottime device tree functions
Grant Likely
grant.likely at secretlab.ca
Thu Nov 18 07:00:12 EST 2010
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.
More information about the devicetree-discuss
mailing list