Re: [PATCH 1/2] OF: update stdout-path parsing
From: Grant Likely <hidden>
Date: 2014-02-11 14:54:01
Also in:
linux-arm-kernel, linux-devicetree
On Sat, 8 Feb 2014 12:53:52 +0100, Jean-Christophe PLAGNIOL-VILLARD [off-list ref] wrote:
Today we only check if the linux,stdout-path property is present and retreive the node. But the DT specification allow to pass the serial options too as this linux,stdout-path = &UART0; or with options inux,stdout-path = "/plb@0/serial@84000000:115200";
typo. :-)
So update the parsing to support and also the backward compatible stdout-path property.
Looks reasonable to me. Comment below, but otherwise: Acked-by: Grant Likely <redacted>
quoted hunk ↗ jump to hunk
Cc: linux-serial@vger.kernel.org Cc: Sascha Hauer <s.hauer@pengutronix.de> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <redacted> --- drivers/of/base.c | 57 +++++++++++++++++++++++++++++++++++++++++++++--------- include/linux/of.h | 4 ++-- 2 files changed, 50 insertions(+), 11 deletions(-)diff --git a/drivers/of/base.c b/drivers/of/base.c index ff85450..ab9ff2f 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c@@ -34,6 +34,7 @@ EXPORT_SYMBOL(of_allnodes); struct device_node *of_chosen; struct device_node *of_aliases; static struct device_node *of_stdout; +static char *of_stdout_option; DEFINE_MUTEX(of_aliases_mutex);@@ -1783,6 +1784,45 @@ static void of_alias_add(struct alias_prop *ap, struct device_node *np, } /** + * of_stdout_path_scan - check if a device node matches the + * linux,stdout-path property and parse it + */ +static void of_stdout_path_scan(void) +{ + const char *name; + const char *tmp; + const char *tmp_option; + + name = of_get_property(of_chosen, "linux,stdout-path", NULL); + if (!name) + name = of_get_property(of_chosen, "stdout-path", NULL);
Should protect against unterminated strings. Use of_property_read_string()
+
+ if (!name)
+ return;
+
+ tmp_option = strchr(name, ':');
+
+ if (!tmp_option) {
+ of_stdout = of_find_node_by_path(name);
+ return;
+ }
+
+ tmp = kstrndup(name, strlen(name) - strlen(tmp_option), GFP_KERNEL);
+ if (!tmp)
+ return;Having to do a strdup is kind of ugly and expensive when the only problem is the terminating colon. I would rather see of_find_node_by_path modified to do the right thing by ignoring the colon which is illegal in path names anyway.
+ + of_stdout = of_find_node_by_path(tmp); + if (!of_stdout) + goto out; + + tmp_option++; + of_stdout_option = kstrdup(tmp_option, GFP_KERNEL);
The strdup is unnecessary. tmp_option is already null terminated, is immutable, and contains the data you need.
+ +out: + kfree(tmp);
Please reverse the to eliminate the non-error goto: of_stdout = of_find_node_by_path(tmp); if (of_stdout) of_stdout_option = tmp_option++; kfree(tmp);
quoted hunk ↗ jump to hunk
+} + +/** * of_alias_scan - Scan all properties of 'aliases' node * * The function scans all the properties of 'aliases' node and populate@@ -1800,13 +1840,8 @@ void of_alias_scan(void * (*dt_alloc)(u64 size, u64 align)) if (of_chosen == NULL) of_chosen = of_find_node_by_path("/chosen@0"); - if (of_chosen) { - const char *name; - - name = of_get_property(of_chosen, "linux,stdout-path", NULL); - if (name) - of_stdout = of_find_node_by_path(name); - } + if (of_chosen) + of_stdout_path_scan();
Move the of_chosen check directly into the function.
quoted hunk ↗ jump to hunk
of_aliases = of_find_node_by_path("/aliases"); if (!of_aliases)@@ -1923,13 +1958,17 @@ EXPORT_SYMBOL_GPL(of_prop_next_string); * linux,stdout-path property * * Check if this device node matches the linux,stdout-path property - * in the chosen node. return true if yes, false otherwise. + * in the chosen node. return true if yes, false otherwise with the option if + * requested. */ -int of_device_is_stdout_path(struct device_node *dn) +int of_device_is_stdout_path(struct device_node *dn, char** option) { if (!of_stdout) return false; + if (option) + *option = of_stdout_option; + return of_stdout == dn; } EXPORT_SYMBOL_GPL(of_device_is_stdout_path);diff --git a/include/linux/of.h b/include/linux/of.h index 70c64ba..a8afe44 100644 --- a/include/linux/of.h +++ b/include/linux/of.h@@ -352,7 +352,7 @@ const __be32 *of_prop_next_u32(struct property *prop, const __be32 *cur, */ const char *of_prop_next_string(struct property *prop, const char *cur); -int of_device_is_stdout_path(struct device_node *dn); +int of_device_is_stdout_path(struct device_node *dn, char** option); #else /* CONFIG_OF */@@ -542,7 +542,7 @@ static inline int of_machine_is_compatible(const char *compat) return 0; } -static inline int of_device_is_stdout_path(struct device_node *dn) +static inline int of_device_is_stdout_path(struct device_node *dn, char** option) { return 0; }-- 1.9.rc1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel