mail archive of the barebox mailing list
 help / color / mirror / Atom feed
From: Teresa Gamez <t.gamez@phytec.de>
To: Sascha Hauer <s.hauer@pengutronix.de>
Cc: barebox@lists.infradead.org
Subject: Re: [PATCHv2] commands: add of_display_timings
Date: Mon, 29 Sep 2014 14:50:11 +0200	[thread overview]
Message-ID: <1411995011.12138.8.camel@lws-gamez.phytec.de> (raw)
In-Reply-To: <20140929060525.GQ4992@pengutronix.de>

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

  reply	other threads:[~2014-09-29 12:50 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-26 11:22 Teresa Gámez
2014-09-29  6:05 ` Sascha Hauer
2014-09-29 12:50   ` Teresa Gamez [this message]
2014-09-29 13:37     ` Sascha Hauer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1411995011.12138.8.camel@lws-gamez.phytec.de \
    --to=t.gamez@phytec.de \
    --cc=barebox@lists.infradead.org \
    --cc=s.hauer@pengutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox