[PATCH 3/8] of/fdt: Add unflatten_partial_device_tree

Grant Likely grant.likely at secretlab.ca
Wed Aug 11 03:53:23 EST 2010


On Fri, Jul 23, 2010 at 4:10 PM, Stephen Neuendorffer
<stephen.neuendorffer at xilinx.com> wrote:
>
>
>> -----Original Message-----
>> From: glikely at secretlab.ca [mailto:glikely at secretlab.ca] On Behalf Of Grant Likely
>> Sent: Friday, July 16, 2010 12:25 PM
>> To: Stephen Neuendorffer
>> Cc: devicetree-discuss at lists.ozlabs.org
>> Subject: Re: [PATCH 3/8] of/fdt: Add unflatten_partial_device_tree
>>
>> On Fri, Jul 16, 2010 at 12:13 PM, Stephen Neuendorffer
>> <stephen.neuendorffer at xilinx.com> wrote:
>> > @@ -391,6 +401,8 @@ unsigned long __init unflatten_dt_node(unsigned long mem,
>> >        return mem;
>> >  }
>> >
>> > +#if defined(CONFIG_MICROBLAZE) || defined(CONFIG_POWERPC) || defined(CONFIG_SPARC)
>> > +
>>
>> Rather hacky.. There's got to be a better way.  Might be better to add
>> a new Kconfig symbol.
>>
>> Also, the test is wrong.  ARM is going to want this.  OLPC x86 might
>> want it.  SPARC doesn't use it.
>
> I think the right thing is to factor the __init/early related things into a separate file.
> Sound reasonable?  Then whoever wants it can select it.

Heh, I forgot about this converation we had earlier.  I wouldn't
bother with a separate file (but I don't know, give it a try and see
what it looks like), but a separate 'def_bool n' config symbol sounds
best.

>
>> >  #ifdef CONFIG_BLK_DEV_INITRD
>> >  /**
>> >  * early_init_dt_check_for_initrd - Decode initrd location from flat tree
>> > @@ -403,12 +415,14 @@ void __init early_init_dt_check_for_initrd(unsigned long node)
>> >
>> >        pr_debug("Looking for initrd properties... ");
>> >
>> > -       prop = of_get_flat_dt_prop(node, "linux,initrd-start", &len);
>> > +       prop = of_get_flat_dt_prop(node, "linux,initrd-start",
>> > +                                  &len, initial_boot_params);
>> >        if (!prop)
>> >                return;
>> >        start = of_read_ulong(prop, len/4);
>> >
>> > -       prop = of_get_flat_dt_prop(node, "linux,initrd-end", &len);
>> > +       prop = of_get_flat_dt_prop(node, "linux,initrd-end",
>> > +                                  &len, initial_boot_params);
>> >        if (!prop)
>> >                return;
>> >        end = of_read_ulong(prop, len/4);
>> > @@ -436,12 +450,14 @@ int __init early_init_dt_scan_root(unsigned long node, const char *uname,
>> >        dt_root_size_cells = OF_ROOT_NODE_SIZE_CELLS_DEFAULT;
>> >        dt_root_addr_cells = OF_ROOT_NODE_ADDR_CELLS_DEFAULT;
>> >
>> > -       prop = of_get_flat_dt_prop(node, "#size-cells", NULL);
>> > +       prop = of_get_flat_dt_prop(node, "#size-cells",
>> > +                                  NULL, initial_boot_params);
>> >        if (prop)
>> >                dt_root_size_cells = be32_to_cpup(prop);
>> >        pr_debug("dt_root_size_cells = %x\n", dt_root_size_cells);
>> >
>> > -       prop = of_get_flat_dt_prop(node, "#address-cells", NULL);
>> > +       prop = of_get_flat_dt_prop(node, "#address-cells",
>> > +                                  NULL, initial_boot_params);
>> >        if (prop)
>> >                dt_root_addr_cells = be32_to_cpup(prop);
>> >        pr_debug("dt_root_addr_cells = %x\n", dt_root_addr_cells);
>>
>> This is the update to the static globals I was mentioning that must not be done.
>>
>> > @@ -450,7 +466,7 @@ int __init early_init_dt_scan_root(unsigned long node, const char *uname,
>> >        return 1;
>> >  }
>> >
>> > -u64 __init dt_mem_next_cell(int s, __be32 **cellp)
>> > +u64  dt_mem_next_cell(int s, __be32 **cellp)
>> >  {
>> >        __be32 *p = *cellp;
>> >
>> > @@ -464,7 +480,8 @@ u64 __init dt_mem_next_cell(int s, __be32 **cellp)
>> >  int __init early_init_dt_scan_memory(unsigned long node, const char *uname,
>> >                                     int depth, void *data)
>> >  {
>> > -       char *type = of_get_flat_dt_prop(node, "device_type", NULL);
>> > +       char *type = of_get_flat_dt_prop(node, "device_type",
>> > +                                        NULL, initial_boot_params);
>> >        __be32 *reg, *endp;
>> >        unsigned long l;
>> >
>> > @@ -479,9 +496,10 @@ int __init early_init_dt_scan_memory(unsigned long node, const char *uname,
>> >        } else if (strcmp(type, "memory") != 0)
>> >                return 0;
>> >
>> > -       reg = of_get_flat_dt_prop(node, "linux,usable-memory", &l);
>> > +       reg = of_get_flat_dt_prop(node, "linux,usable-memory",
>> > +                                 &l, initial_boot_params);
>> >        if (reg == NULL)
>> > -               reg = of_get_flat_dt_prop(node, "reg", &l);
>> > +               reg = of_get_flat_dt_prop(node, "reg", &l, initial_boot_params);
>> >        if (reg == NULL)
>> >                return 0;
>> >
>> > @@ -521,12 +539,12 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
>> >
>> >        early_init_dt_check_for_initrd(node);
>> >
>> > +#ifdef CONFIG_CMDLINE
>> >        /* Retreive command line */
>> > -       p = of_get_flat_dt_prop(node, "bootargs", &l);
>> > +       p = of_get_flat_dt_prop(node, "bootargs", &l, initial_boot_params);
>> >        if (p != NULL && l > 0)
>> >                strlcpy(cmd_line, p, min((int)l, COMMAND_LINE_SIZE));
>> >
>> > -#ifdef CONFIG_CMDLINE
>>
>> Why is this being changed?
>
> Because it requires a cmd_line global, which all architectures (especially x86) don't seem to have)
> Probably if the early stuff get's factored out, then this problem goes away.

I'm considering changing this so that cmd_line is set with an
arch-specific callback.

>
>> >  #ifndef CONFIG_CMDLINE_FORCE
>> >        if (p == NULL || l == 0 || (l == 1 && (*p) == 0))
>> >  #endif
>> > @@ -535,12 +553,21 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
>> >
>> >        early_init_dt_scan_chosen_arch(node);
>> >
>> > +#ifdef CONFIG_CMDLINE
>> >        pr_debug("Command line is: %s\n", cmd_line);
>> > +#endif /* CONFIG_CMDLINE */
>> >
>> >        /* break now */
>> >        return 1;
>> >  }
>> >
>> > +
>> > +static unsigned long early_device_tree_alloc(u64 size, u64 align)
>> > +{
>> > +       unsigned long mem = early_init_dt_alloc_memory_arch(size, align);
>> > +       return (unsigned long) __va(mem);
>> > +}
>> > +
>> >  /**
>> >  * unflatten_device_tree - create tree of device_nodes from flat blob
>> >  *
>> > @@ -551,58 +578,98 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
>> >  */
>> >  void __init unflatten_device_tree(void)
>> >  {
>> > +       __unflatten_device_tree(initial_boot_params, &allnodes,
>> > +                               early_device_tree_alloc);
>> > +
>> > +       /* Get pointer to OF "/chosen" node for use everywhere */
>> > +       of_chosen = of_find_node_by_path("/chosen");
>> > +       if (of_chosen == NULL)
>> > +               of_chosen = of_find_node_by_path("/chosen at 0");
>> > +}
>> > +
>> > +#endif
>> > +
>> > +static unsigned long kernel_tree_alloc(u64 size, u64 align)
>> > +{
>> > +       return (unsigned long) kzalloc(size, GFP_KERNEL);
>> > +}
>> > +
>> > +/**
>> > + * unflatten_partial_device_tree - create tree of device_nodes from flat blob
>> > + *
>> > + * unflattens the device-tree passed by the firmware, creating the
>> > + * tree of struct device_node. It also fills the "name" and "type"
>> > + * pointers of the nodes so the normal device-tree walking functions
>> > + * can be used.
>> > + */
>> > +void unflatten_partial_device_tree(unsigned long *blob,
>> > +                                  struct device_node **mynodes)
>> > +{
>> > +       __unflatten_device_tree(blob, mynodes, &kernel_tree_alloc);
>> > +}
>> > +EXPORT_SYMBOL(unflatten_partial_device_tree);
>>
>> unflatten_partial_device_tree and unflatten_device_tree should be
>> defined *below* __unflatten_device_tree().
>
> Well, I wanted to avoid that, because it made the ordering requirements in the file more nasty
> and it makes the #ifdefs more complex.

Shouldn't be.  The #ifdefs are trivial (once you add the new CONFIG
symbol), and there are only two references to
__unflatten_device_tree().

Cheers,
g.


More information about the devicetree-discuss mailing list