[PATCH 1/2] of: Added of_set_property_to_child_phandle

[PATCH 1/2] of: Added of_set_property_to_child_phandle

[Teresa Gámez]

Set a property to a phandle of a child node. This may be used
for selections like display-timings.

Signed-off-by: Teresa Gámez <t.gamez@phytec.de>
---
 drivers/of/base.c |   21 +++++++++++++++++++++
 include/of.h      |    1 +
 2 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index c440a69..eb5e1a1 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -336,6 +336,27 @@ phandle of_node_create_phandle(struct device_node *node)
 }
 EXPORT_SYMBOL(of_node_create_phandle);
 
+int of_set_property_to_child_phandle(struct device_node *node, char *prop_name)
+{
+	int ret;
+	phandle p;
+
+	/* Check if property exist */
+	if (!of_get_property(of_get_parent(node), prop_name, NULL))
+		return -EINVAL;
+
+	/* Create or get existing phandle of child node */
+	p = of_node_create_phandle(node);
+	p = cpu_to_be32(p);
+
+	node = of_get_parent(node);
+
+	ret = of_set_property(node, prop_name, &p, sizeof(p), 0);
+
+	return ret;
+}
+EXPORT_SYMBOL(of_set_property_to_child_phandle);
+
 /*
  * Find a property with a given name for a given node
  * and return the value.
diff --git a/include/of.h b/include/of.h
index e6993fd..45ef56d 100644
--- a/include/of.h
+++ b/include/of.h
@@ -692,6 +692,7 @@ int of_device_disable_path(const char *path);
 
 phandle of_get_tree_max_phandle(struct device_node *root);
 phandle of_node_create_phandle(struct device_node *node);
+int of_set_property_to_child_phandle(struct device_node *node, char *prop_name);
 struct device_node *of_find_node_by_alias(struct device_node *root,
 		const char *alias);
 struct device_node *of_find_node_by_path_or_alias(struct device_node *root,
-- 
1.7.0.4


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 2/2] commands: add of_display_timings

[Teresa Gámez]

A lot of boards use display-timings nodes to define the timings of one or more
displays. This command makes it possible to list and select displays which are
defined in a device tree.

Signed-off-by: Teresa Gámez <t.gamez@phytec.de>
---
 commands/Kconfig              |   15 ++++
 commands/Makefile             |    1 +
 commands/of_display_timings.c |  180 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 196 insertions(+), 0 deletions(-)
 create mode 100644 commands/of_display_timings.c

diff --git a/commands/Kconfig b/commands/Kconfig
index 3a49baf..0563e63 100644
--- a/commands/Kconfig
+++ b/commands/Kconfig
@@ -1973,6 +1973,21 @@ config CMD_OF_PROPERTY
 	  [00 11 22 .. nn]  - byte stream
 	  If the value does not start with '<' or '[' it is interpreted as string
 
+config CMD_OF_DISPLAY_TIMINGS
+	tristate
+	select OFTREE
+	prompt "of_display_timings"
+	help
+	  List and select display timings
+
+	  Usage: of_display_timings [-lS] [-s path] [-f dtb]
+
+	  Options:
+		  -l		list path of all available display-timings
+		  -S		list path of all selected display-timings
+		  -s path	select display-timings and register oftree fixup
+		  -f dtb	work on dtb. Has no effect on -s option
+
 config CMD_OFTREE
 	tristate
 	select OFTREE
diff --git a/commands/Makefile b/commands/Makefile
index 52b6137..f5ecbe6 100644
--- a/commands/Makefile
+++ b/commands/Makefile
@@ -76,6 +76,7 @@ obj-$(CONFIG_CMD_OFTREE)	+= oftree.o
 obj-$(CONFIG_CMD_OF_PROPERTY)	+= of_property.o
 obj-$(CONFIG_CMD_OF_NODE)	+= of_node.o
 obj-$(CONFIG_CMD_OF_DUMP)	+= of_dump.o
+obj-$(CONFIG_CMD_OF_DISPLAY_TIMINGS)	+= of_display_timings.o
 obj-$(CONFIG_CMD_MAGICVAR)	+= magicvar.o
 obj-$(CONFIG_CMD_IOMEM)		+= iomemport.o
 obj-$(CONFIG_CMD_LINUX_EXEC)	+= linux_exec.o
diff --git a/commands/of_display_timings.c b/commands/of_display_timings.c
new file mode 100644
index 0000000..67763f8
--- /dev/null
+++ b/commands/of_display_timings.c
@@ -0,0 +1,180 @@
+/*
+ * of_display_timings.c - list and select display_timings
+ *
+ * Copyright (c) 2014 Teresa Gámez <t.gamez@phytec.de> PHYTEC Messtechnik GmbH
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <common.h>
+#include <filetype.h>
+#include <libfile.h>
+#include <of.h>
+#include <command.h>
+#include <malloc.h>
+#include <complete.h>
+#include <asm/byteorder.h>
+#include <getopt.h>
+#include <linux/err.h>
+#include <string.h>
+
+static int of_display_timing(struct device_node *root, void *timingpath)
+{
+	int ret = 0;
+	struct device_node *display;
+
+	display = of_find_node_by_path_from(root, (char *) timingpath);
+	if (!display) {
+		pr_err("Path to display-timings is not vaild.\n");
+		ret = -EINVAL;
+		goto out;
+	}
+
+	ret = of_set_property_to_child_phandle(display, "native-mode");
+	if (ret)
+		pr_err("Could not set display\n");
+out:
+	return ret;
+}
+
+static int do_of_display_timings(int argc, char *argv[])
+{
+	int opt;
+	int list = 0;
+	int selected = 0;
+	int ret = 0;
+	struct device_node *root = NULL;
+	struct device_node *internal_root = NULL;
+	struct device_node *display = NULL;
+	struct device_node *timings = NULL;
+	char *timingpath = NULL;
+	char *dtbfile = NULL;
+
+	while ((opt = getopt(argc, argv, "sS:lf:")) > 0) {
+		switch (opt) {
+		case 'l':
+			list = 1;
+			break;
+		case 'f':
+			dtbfile = optarg;
+			break;
+		case 's':
+			selected = 1;
+			break;
+		case 'S':
+			timingpath = xzalloc(strlen(optarg) + 1);
+			strcpy(timingpath, optarg);
+			break;
+		default:
+			return COMMAND_ERROR_USAGE;
+		}
+	}
+
+	/* Check if external dtb given */
+	if (dtbfile) {
+		void *fdt;
+		size_t size;
+
+		fdt = read_file(dtbfile, &size);
+		if (!fdt) {
+			pr_err("unable to read %s: %s\n", dtbfile,
+				strerror(errno));
+			return -errno;
+		}
+
+		if (file_detect_type(fdt, size) != filetype_oftree) {
+			pr_err("%s is not a oftree file.\n", dtbfile);
+			return -EINVAL;
+		}
+
+		root = of_unflatten_dtb(fdt);
+
+		free(fdt);
+
+		if (IS_ERR(root)) {
+			ret = PTR_ERR(root);
+			goto out;
+		}
+
+		/*
+		 * We set the external node to our internal node as
+		 * long as the command runs. This makes life easier to
+		 * use of tree functions. We backup the internal root node
+		 * and set it back at the end.
+		 */
+		internal_root = of_get_root_node();
+		of_set_root_node(NULL);
+		of_set_root_node(root);
+	} else {
+		root = of_get_root_node();
+	}
+
+	if (list) {
+		int found = 0;
+		const char *node = "display-timings";
+
+		for_each_node_by_name(display, node) {
+			for_each_child_of_node(display, timings) {
+				printf("%s\n", timings->full_name);
+				found = 1;
+			}
+		}
+
+		if (!found)
+			printf("No display-timings found.\n");
+	}
+
+	if (selected) {
+		int found = 0;
+		const char *node = "display-timings";
+
+		for_each_node_by_name(display, node) {
+			timings = of_parse_phandle(display, "native-mode", 0);
+			if (!timings)
+				continue;
+
+			printf("%s\n", timings->full_name);
+			found = 1;
+		}
+
+		if (!found)
+			printf("No selected display-timings found.\n");
+	}
+
+	if (timingpath)
+		of_register_fixup(of_display_timing, timingpath);
+out:
+	if (internal_root) {
+		of_set_root_node(NULL);
+		of_set_root_node(internal_root);
+	}
+
+	return ret;
+}
+
+BAREBOX_CMD_HELP_START(of_display_timings)
+BAREBOX_CMD_HELP_TEXT("Options:")
+BAREBOX_CMD_HELP_OPT("-l",  "list path of all available display-timings\n")
+BAREBOX_CMD_HELP_OPT("-s",  "list path of all selected display-timings\n")
+BAREBOX_CMD_HELP_OPT("-S path",  "select display-timings and register oftree fixup\n")
+BAREBOX_CMD_HELP_OPT("-f dtb",  "work on dtb. Has no effect on -s option\n")
+BAREBOX_CMD_HELP_END
+
+BAREBOX_CMD_START(of_display_timings)
+	.cmd	= do_of_display_timings,
+	BAREBOX_CMD_DESC("print and select display-timings")
+	BAREBOX_CMD_OPTS("[-ls] [-S path] [-f dtb]")
+	BAREBOX_CMD_GROUP(CMD_GRP_MISC)
+	BAREBOX_CMD_COMPLETE(devicetree_file_complete)
+	BAREBOX_CMD_HELP(cmd_of_display_timings_help)
+BAREBOX_CMD_END
-- 
1.7.0.4


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/2] commands: add of_display_timings

[Sascha Hauer]

Hi Teresa,

On Wed, Sep 24, 2014 at 11:33:11AM +0200, Teresa Gámez wrote:
> A lot of boards use display-timings nodes to define the timings of one or more
> displays. This command makes it possible to list and select displays which are
> defined in a device tree.

Nice idea.

> +static int of_display_timing(struct device_node *root, void *timingpath)
> +{
> +	int ret = 0;
> +	struct device_node *display;
> +
> +	display = of_find_node_by_path_from(root, (char *) timingpath);

Unnecessary cast.

> +static int do_of_display_timings(int argc, char *argv[])
> +{
> +	int opt;
> +	int list = 0;
> +	int selected = 0;
> +	int ret = 0;
> +	struct device_node *root = NULL;
> +	struct device_node *internal_root = NULL;
> +	struct device_node *display = NULL;
> +	struct device_node *timings = NULL;
> +	char *timingpath = NULL;
> +	char *dtbfile = NULL;
> +
> +	while ((opt = getopt(argc, argv, "sS:lf:")) > 0) {
> +		switch (opt) {
> +		case 'l':
> +			list = 1;
> +			break;
> +		case 'f':
> +			dtbfile = optarg;
> +			break;

Do we really need this option? It can only be used for showing display
timings in an external dtb, not for manipulating it. If I plan to use
the external tree I can still do a oftree -f / oftree -l dtb to exchange
the internal tree.

> +		case 's':
> +			selected = 1;
> +			break;
> +		case 'S':
> +			timingpath = xzalloc(strlen(optarg) + 1);
> +			strcpy(timingpath, optarg);

You never free timingpath. But why are you making a copy of the string?
This shouldn't be necessary.

> +			break;
> +		default:
> +			return COMMAND_ERROR_USAGE;
> +		}
> +	}
> +
> +	/* Check if external dtb given */
> +	if (dtbfile) {
> +		void *fdt;
> +		size_t size;
> +
> +		fdt = read_file(dtbfile, &size);
> +		if (!fdt) {
> +			pr_err("unable to read %s: %s\n", dtbfile,
> +				strerror(errno));
> +			return -errno;
> +		}
> +
> +		if (file_detect_type(fdt, size) != filetype_oftree) {
> +			pr_err("%s is not a oftree file.\n", dtbfile);
> +			return -EINVAL;

Here you lose memory.

> +		}
> +
> +		root = of_unflatten_dtb(fdt);

This must be freed after use with of_delete_node().

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/2] commands: add of_display_timings

[Teresa Gamez]

Hello Sascha,

Am Donnerstag, den 25.09.2014, 08:07 +0200 schrieb Sascha Hauer:
> Hi Teresa,
> 
> On Wed, Sep 24, 2014 at 11:33:11AM +0200, Teresa Gámez wrote:
> > A lot of boards use display-timings nodes to define the timings of one or more
> > displays. This command makes it possible to list and select displays which are
> > defined in a device tree.
> 
> Nice idea.
> 
> > +static int of_display_timing(struct device_node *root, void *timingpath)
> > +{
> > +	int ret = 0;
> > +	struct device_node *display;
> > +
> > +	display = of_find_node_by_path_from(root, (char *) timingpath);
> 
> Unnecessary cast.

Removed it.
> 
> > +static int do_of_display_timings(int argc, char *argv[])
> > +{
> > +	int opt;
> > +	int list = 0;
> > +	int selected = 0;
> > +	int ret = 0;
> > +	struct device_node *root = NULL;
> > +	struct device_node *internal_root = NULL;
> > +	struct device_node *display = NULL;
> > +	struct device_node *timings = NULL;
> > +	char *timingpath = NULL;
> > +	char *dtbfile = NULL;
> > +
> > +	while ((opt = getopt(argc, argv, "sS:lf:")) > 0) {
> > +		switch (opt) {
> > +		case 'l':
> > +			list = 1;
> > +			break;
> > +		case 'f':
> > +			dtbfile = optarg;
> > +			break;
> 
> Do we really need this option? It can only be used for showing display
> timings in an external dtb, not for manipulating it. If I plan to use
> the external tree I can still do a oftree -f / oftree -l dtb to exchange
> the internal tree.

Well, it just gives the same feature like of_dump has. It is not that 
necessary, but it makes it more handy. 

> 
> > +		case 's':
> > +			selected = 1;
> > +			break;
> > +		case 'S':
> > +			timingpath = xzalloc(strlen(optarg) + 1);
> > +			strcpy(timingpath, optarg);
> 
> You never free timingpath. But why are you making a copy of the string?
> This shouldn't be necessary.

I need to save the string for later usage in the fixup function. This is
called after the command has finished. I think the optarg pointer to
argv is not longer vaild then.

The fixup can be called multiple times (canceled boot, of_dump -F,..).
I don't see a save place to free timingpath.

Teresa

> 
> > +			break;
> > +		default:
> > +			return COMMAND_ERROR_USAGE;
> > +		}
> > +	}
> > +
> > +	/* Check if external dtb given */
> > +	if (dtbfile) {
> > +		void *fdt;
> > +		size_t size;
> > +
> > +		fdt = read_file(dtbfile, &size);
> > +		if (!fdt) {
> > +			pr_err("unable to read %s: %s\n", dtbfile,
> > +				strerror(errno));
> > +			return -errno;
> > +		}
> > +
> > +		if (file_detect_type(fdt, size) != filetype_oftree) {
> > +			pr_err("%s is not a oftree file.\n", dtbfile);
> > +			return -EINVAL;
> 
> Here you lose memory.
> 
> > +		}
> > +
> > +		root = of_unflatten_dtb(fdt);
> 
> This must be freed after use with of_delete_node().
> 
> Sascha
> 
> 



_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox