[PATCH 1/2] OF: update stdout-path parsing

[PATCH 1/2] OF: update stdout-path parsing

[Jean-Christophe PLAGNIOL-VILLARD]

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 at 0/serial at 84000000:115200";

So update the parsing to support and also the backward compatible stdout-path
property.

Cc: linux-serial at vger.kernel.org
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
 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);
+
+	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;
+
+	of_stdout = of_find_node_by_path(tmp);
+	if (!of_stdout)
+		goto out;
+
+	tmp_option++;
+	of_stdout_option = kstrdup(tmp_option, GFP_KERNEL);
+
+out:
+	kfree(tmp);
+}
+
+/**
  * 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 at 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();
 
 	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

[PATCH 2/2] serial: atmel: evaluate linux,stdout-path property

[Jean-Christophe PLAGNIOL-VILLARD]

if the driver is built as module skip it as add_preferred_console is not
exported

Cc: linux-serial at vger.kernel.org
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
 drivers/tty/serial/atmel_serial.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index a49f10d..2ade64e 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -2044,6 +2044,19 @@ static struct uart_ops atmel_pops = {
 #endif
 };
 
+static void atmel_of_preferred_console(struct atmel_uart_port *atmel_port,
+				       struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct uart_port *port = &atmel_port->uart;
+	char* option;
+
+	if (!np || !of_device_is_stdout_path(np, &option))
+		return;
+
+	add_preferred_console(ATMEL_DEVICENAME, port->line, option);
+}
+
 /*
  * Configure the port from the platform device resource info.
  */
@@ -2059,6 +2072,9 @@ static int atmel_init_port(struct atmel_uart_port *atmel_port,
 
 	atmel_init_rs485(atmel_port, pdev);
 
+	if (IS_BUILTIN(CONFIG_SERIAL_ATMEL))
+		atmel_of_preferred_console(atmel_port, pdev);
+
 	port->iotype		= UPIO_MEM;
 	port->flags		= UPF_BOOT_AUTOCONF;
 	port->ops		= &atmel_pops;
-- 
1.9.rc1

[PATCH 1/2] OF: update stdout-path parsing

[Grant Likely]

On Sat,  8 Feb 2014 12:53:52 +0100, Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> 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 at 0/serial at 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 <grant.likely@linaro.org>

> 
> Cc: linux-serial at vger.kernel.org
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> ---
>  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);

> +}
> +
> +/**
>   * 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 at 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.

>  
>  	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 at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel