Re: [RFC 2/3] OLPC: add missing elements to device tree
From: Grant Likely <hidden>
Date: 2011-03-05 04:50:42
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 <redacted>
Hi Daniel, The concept looks good. here are a few comments on the implementation. g.
quoted hunk
--- 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.
quoted hunk
+ 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