On Fri, Mar 04, 2011 at 05:12:34PM +0000, Daniel Drake wrote:
> In response to new device tree code in the kernel, OLPC will start
> using it for probing of certain devices. However, some firmware fixes
> are needed to put the devicetree into a usable state.
>
> Retain compatibility with old firmware by fixing up the device tree
> at boot-time if it does not contain the new nodes/properties that
> we need for probing.
>
> Signed-off-by: Daniel Drake <[email protected]>
Hi Daniel,
The concept looks good. here are a few comments on the
implementation.
g.
> ---
> arch/x86/platform/olpc/olpc_dt.c | 87
> ++++++++++++++++++++++++++++++++++++++
> 1 files changed, 87 insertions(+), 0 deletions(-)
>
> diff --git a/arch/x86/platform/olpc/olpc_dt.c
> b/arch/x86/platform/olpc/olpc_dt.c
> index 09cbede..58bba78 100644
> --- a/arch/x86/platform/olpc/olpc_dt.c
> +++ b/arch/x86/platform/olpc/olpc_dt.c
> @@ -22,6 +22,7 @@
> #include <linux/of_platform.h>
> #include <linux/of_pdt.h>
> #include <asm/olpc_ofw.h>
> +#include <asm/olpc.h>
>
> static phandle __init olpc_dt_getsibling(phandle node)
> {
> @@ -164,6 +165,91 @@ static struct of_pdt_ops prom_olpc_ops __initdata = {
> .pkg2path = olpc_dt_pkg2path,
> };
>
> +static char __init *prom_alloc_string(const char *str, int *len)
> +{
> + int _len = strlen(str);
> + char *output = prom_early_alloc(_len + 1);
> +
> + strcpy(output, str);
> + if (len)
> + *len = _len;
> + return output;
> +}
> +
> +static void __init prom_alloc_property(struct device_node *node,
> + const char *name, const char *value)
> +{
> + struct property *p = prom_early_alloc(sizeof(struct property));
How early is this running? If the allocator is set up by now, then
this should use kzalloc()...
> +
> + p->name = prom_alloc_string(name, NULL);
> + p->value = prom_alloc_string(value, &p->length);
... which would also mean that kstrdup()/kmemdup() can be used here
directly.
> + prom_add_property(node, p);
> +}
This would be useful in common code. prom_create_property() might
be a better name. It should also accept a length argument for the
size of the data.
> +
> +/* Add dcon device as child of display */
> +static void __init add_dcon_node(struct device_node *display)
> +{
> + struct device_node *node = prom_early_alloc(sizeof(struct device_node));
> + unsigned long flags;
> +
> + kref_init(&node->kref);
> + node->name = prom_alloc_string("dcon", NULL);
> + node->full_name = (char *) node->name;
> + node->parent = display;
> +
> + write_lock_irqsave(&devtree_lock, flags);
> + node->sibling = node->parent->child;
> + node->allnext = allnodes;
> + node->parent->child = node;
> + allnodes = node;
> + write_unlock_irqrestore(&devtree_lock, flags);
> +
This should be using the existing of_attach_node() function to link
the node into the tree (in fact I know you've seen it because the
above 6 lines are identical). :-) Also, there already exists a function
"new_node" in arch/powerpc/platforms/iseries/vio.c that would be
useful to move into drivers/of/base.c
> + prom_alloc_property(node, "compatible", "olpc,xo1-dcon");
> +}
> +
> +static void __init olpc_dt_fixup(void)
> +{
> + struct device_node *node;
> +
> + /*
> + * Use battery's compatible property to determine if we're running a
> + * new-enough firmware. If we have this property, no fixups are needed.
> + */
> + node = of_find_node_by_name(NULL, "battery");
> + if (node && of_get_property(node, "compatible", NULL)) {
> + of_node_put(node);
> + return;
> + }
> +
> + /*
> + * Add "compatible" property to battery, it was missing from earlier
> + * firmware releases.
> + */
> + prom_alloc_property(node, "compatible", "olpc,xo1-battery");
> + of_node_put(node);
> +
> + /*
> + * Mark XO-1 RTC as compatible with olpc,xo1-rtc - this was not done in
> + * earlier firmware releases.
> + */
> + node = of_find_node_by_name(NULL, "rtc");
> + if (olpc_platform_info.boardrev < olpc_board_pre(0xd0) && node) {
> + struct property *p = of_get_property(node, "compatible", NULL);
> + prom_remove_property(node, p);
What if p is NULL? prom_remove_property() looks like it won't break
if it is, but that looks to be only by luck. I think this can be more
defensive.
> + prom_alloc_property(node, "compatible", "olpc,xo1-rtc");
> + }
> + of_node_put(node);
> +
> + /*
> + * Add "dcon" device node, it was missing from earlier firmware
> + * releases.
> + */
> + node = of_find_node_by_name(NULL, "display");
> + if (node)
> + add_dcon_node(node);
> + of_node_put(node);
> +}
> +
> void __init olpc_dt_build_devicetree(void)
> {
> phandle root;
> @@ -177,6 +263,7 @@ void __init olpc_dt_build_devicetree(void)
> return;
> }
> of_pdt_build_devicetree(root, &prom_olpc_ops);
> + olpc_dt_fixup();
>
> pr_info("PROM DT: Built device tree with %u bytes of memory.\n",
> prom_early_allocated);
> --
> 1.7.4
>
_______________________________________________
devicetree-discuss mailing list
[email protected]
https://lists.ozlabs.org/listinfo/devicetree-discuss
