* [PATCH 1/2] of: Added of_set_property_to_child_phandle @ 2014-09-24 9:33 Teresa Gámez 2014-09-24 9:33 ` [PATCH 2/2] commands: add of_display_timings Teresa Gámez 0 siblings, 1 reply; 4+ messages in thread From: Teresa Gámez @ 2014-09-24 9:33 UTC (permalink / raw) To: barebox 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 2/2] commands: add of_display_timings 2014-09-24 9:33 [PATCH 1/2] of: Added of_set_property_to_child_phandle Teresa Gámez @ 2014-09-24 9:33 ` Teresa Gámez 2014-09-25 6:07 ` Sascha Hauer 0 siblings, 1 reply; 4+ messages in thread From: Teresa Gámez @ 2014-09-24 9:33 UTC (permalink / raw) To: barebox 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 2/2] commands: add of_display_timings 2014-09-24 9:33 ` [PATCH 2/2] commands: add of_display_timings Teresa Gámez @ 2014-09-25 6:07 ` Sascha Hauer 2014-09-25 11:22 ` Teresa Gamez 0 siblings, 1 reply; 4+ messages in thread From: Sascha Hauer @ 2014-09-25 6:07 UTC (permalink / raw) To: Teresa Gámez; +Cc: barebox 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 2/2] commands: add of_display_timings 2014-09-25 6:07 ` Sascha Hauer @ 2014-09-25 11:22 ` Teresa Gamez 0 siblings, 0 replies; 4+ messages in thread From: Teresa Gamez @ 2014-09-25 11:22 UTC (permalink / raw) To: Sascha Hauer; +Cc: barebox 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-09-25 11:22 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-09-24 9:33 [PATCH 1/2] of: Added of_set_property_to_child_phandle Teresa Gámez 2014-09-24 9:33 ` [PATCH 2/2] commands: add of_display_timings Teresa Gámez 2014-09-25 6:07 ` Sascha Hauer 2014-09-25 11:22 ` Teresa Gamez
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox