[PATCHv2] commands: add of_display_timings
Teresa Gamez
t.gamez at phytec.de
Mon Sep 29 05:50:11 PDT 2014
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 at 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
>
More information about the barebox
mailing list