* [PATCHv2] commands: add of_display_timings
@ 2014-09-26 11:22 Teresa Gámez
2014-09-29 6:05 ` Sascha Hauer
0 siblings, 1 reply; 4+ messages in thread
From: Teresa Gámez @ 2014-09-26 11:22 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>
---
Changes v2:
- removed cast
- added free(ftd)
- freed root
commands/Kconfig | 15 ++++
commands/Makefile | 1 +
commands/of_display_timings.c | 182 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 198 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..9a6f8c8
--- /dev/null
+++ b/commands/of_display_timings.c
@@ -0,0 +1,182 @@
+/*
+ * 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, 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);
+ free(fdt);
+ 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);
+
+ if (internal_root) {
+ of_set_root_node(NULL);
+ of_set_root_node(internal_root);
+ of_delete_node(root);
+ }
+out:
+ 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: [PATCHv2] commands: add of_display_timings
2014-09-26 11:22 [PATCHv2] commands: add of_display_timings Teresa Gámez
@ 2014-09-29 6:05 ` Sascha Hauer
2014-09-29 12:50 ` Teresa Gamez
0 siblings, 1 reply; 4+ messages in thread
From: Sascha Hauer @ 2014-09-29 6:05 UTC (permalink / raw)
To: Teresa Gámez; +Cc: barebox
On Fri, Sep 26, 2014 at 01:22:31PM +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.
>
> Signed-off-by: Teresa Gámez <t.gamez@phytec.de>
> ---
> Changes v2:
> - removed cast
> - added free(ftd)
> - freed root
>
> commands/Kconfig | 15 ++++
> commands/Makefile | 1 +
> commands/of_display_timings.c | 182 +++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 198 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..9a6f8c8
> --- /dev/null
> +++ b/commands/of_display_timings.c
> @@ -0,0 +1,182 @@
> +/*
> + * 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, 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;
You're right, there's no place to free this string.
BTW we have xstrdup for duplicating strings.
> + /*
> + * 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);
Since the only device tree function you are using is
for_each_node_by_name(), could you add a:
#define for_each_node_by_name_from(dn, root, name) \
for (dn = of_find_node_by_name(root, name); dn; \
dn = of_find_node_by_name(dn, name))
to include/of.h and remove the temporary overwriting of the
internal device tree?
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: [PATCHv2] commands: add of_display_timings
2014-09-29 6:05 ` Sascha Hauer
@ 2014-09-29 12:50 ` Teresa Gamez
2014-09-29 13:37 ` Sascha Hauer
0 siblings, 1 reply; 4+ messages in thread
From: Teresa Gamez @ 2014-09-29 12:50 UTC (permalink / raw)
To: Sascha Hauer; +Cc: barebox
Hello Sascha,
Am Montag, den 29.09.2014, 08:05 +0200 schrieb Sascha Hauer:
> On Fri, Sep 26, 2014 at 01:22:31PM +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.
> >
> > Signed-off-by: Teresa Gámez <t.gamez@phytec.de>
> > ---
> > Changes v2:
> > - removed cast
> > - added free(ftd)
> > - freed root
> >
...
> > +
> > +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;
>
> You're right, there's no place to free this string.
>
> BTW we have xstrdup for duplicating strings.
>
>
> > + /*
> > + * 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);
>
> Since the only device tree function you are using is
> for_each_node_by_name(), could you add a:
>
> #define for_each_node_by_name_from(dn, root, name) \
> for (dn = of_find_node_by_name(root, name); dn; \
> dn = of_find_node_by_name(dn, name))
>
> to include/of.h and remove the temporary overwriting of the
> internal device tree?
I'm also using of_parse_phandle() which calls of_find_node_by_phandle().
And this is using the root_node again.
I can create *_from() functions for both of them as overwriting the root
node is really not pretty..
Teresa
>
> Sascha
>
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCHv2] commands: add of_display_timings
2014-09-29 12:50 ` Teresa Gamez
@ 2014-09-29 13:37 ` Sascha Hauer
0 siblings, 0 replies; 4+ messages in thread
From: Sascha Hauer @ 2014-09-29 13:37 UTC (permalink / raw)
To: Teresa Gamez; +Cc: barebox
On Mon, Sep 29, 2014 at 02:50:11PM +0200, Teresa Gamez wrote:
> Hello Sascha,
>
> Am Montag, den 29.09.2014, 08:05 +0200 schrieb Sascha Hauer:
> > On Fri, Sep 26, 2014 at 01:22:31PM +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.
> > >
> > > Signed-off-by: Teresa Gámez <t.gamez@phytec.de>
> > > ---
> > > Changes v2:
> > > - removed cast
> > > - added free(ftd)
> > > - freed root
> > >
>
> ...
> > > +
> > > +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;
> >
> > You're right, there's no place to free this string.
> >
> > BTW we have xstrdup for duplicating strings.
> >
> >
> > > + /*
> > > + * 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);
> >
> > Since the only device tree function you are using is
> > for_each_node_by_name(), could you add a:
> >
> > #define for_each_node_by_name_from(dn, root, name) \
> > for (dn = of_find_node_by_name(root, name); dn; \
> > dn = of_find_node_by_name(dn, name))
> >
> > to include/of.h and remove the temporary overwriting of the
> > internal device tree?
>
> I'm also using of_parse_phandle() which calls of_find_node_by_phandle().
> And this is using the root_node again.
>
> I can create *_from() functions for both of them as overwriting the root
> node is really not pretty..
Yes, I think it's better to create functions that can be passed a root
node. It's probably not the last time we need to modify an external
device tree. Overwriting the root node maybe has funny side effects in
the future.
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
end of thread, other threads:[~2014-09-29 13:37 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-26 11:22 [PATCHv2] commands: add of_display_timings Teresa Gámez
2014-09-29 6:05 ` Sascha Hauer
2014-09-29 12:50 ` Teresa Gamez
2014-09-29 13:37 ` Sascha Hauer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox