public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Mike Frysinger <vapier@gentoo.org>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 1/2] Create a single cmd_call() function to handle command execution
Date: Sun, 30 Oct 2011 17:28:53 -0400	[thread overview]
Message-ID: <201110301728.54590.vapier@gentoo.org> (raw)
In-Reply-To: <CAPnjgZ2Gw8Oa6_fbFwGamgq-5iuCv=jLp-af7QG_nrAzGHZtOA@mail.gmail.com>

On Tuesday 25 October 2011 19:05:02 Simon Glass wrote:
> On Tue, Oct 25, 2011 at 6:57 AM, Mike Frysinger wrote:
> > On Mon, Oct 24, 2011 at 23:52, Simon Glass wrote:
> >> +int cmd_call(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> >> +{
> >> +       int result;
> >> +
> >> +       result = (cmdtp->cmd)(cmdtp, flag, argc, argv);
> >> +       if (result)
> >> +               debug("Command failed, result=%d", result);
> >> +       return result;
> >> +}
> > 
> > i don't think this goes for enough.  it should integrate the "if (argc
> > 
> >> cmdtp->maxargs) return cmd_usage(cmdtp);".
> 
> Yes, that might turn this into a patch worth accepting on its merits.
> 
> > and perhaps even the find_cmd(argv[0]) lookup ...
> > -mike
> 
> How about if the commands return error codes, one of which means
> 'print usage'? Then we might remove 265 calls to cmd_usage and even
> reduce code size :-O

i'm not sure hardcoding a specific value across all commands is possible.  for 
most commands, we do just return -1/0/1, which probably should be normalized 
into 0/1 ...

but the overhead with these calls is probably not that bad since we return via 
it.  so the asm isn't a matter of setting up the args, making the call, moving 
the return into the right place, and then returning ... we set up the args and 
jump to cmd_usage.  for some arches (depending on the calling convention), we 
might not even have to touch the args since the one arg is already in the 
right place.

you could prototype the overhead:
-	return cmd_usage(cmdtp);
+	return -500;
but i wouldn't be surprised if there was no difference, or even if this was 
actually more overhead ...
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20111030/9afbb393/attachment.pgp 

      reply	other threads:[~2011-10-30 21:28 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-10-25  3:52 [U-Boot] [PATCH 1/2] Create a single cmd_call() function to handle command execution Simon Glass
2011-10-25  3:52 ` [U-Boot] [PATCH 2/2] RFC: Add XON/XOFF support Simon Glass
2011-10-25  8:09   ` Graeme Russ
2011-10-25 23:56     ` Simon Glass
2011-10-30 21:30   ` Mike Frysinger
2011-10-30 23:53   ` Albert ARIBAUD
2011-10-25  7:46 ` [U-Boot] [PATCH 1/2] Create a single cmd_call() function to handle command execution Wolfgang Denk
2011-10-25  8:03   ` Graeme Russ
2011-10-25 13:33     ` Simon Glass
2011-10-25 18:20     ` Wolfgang Denk
2011-10-25 13:57 ` Mike Frysinger
2011-10-25 23:05   ` Simon Glass
2011-10-30 21:28     ` Mike Frysinger [this message]

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=201110301728.54590.vapier@gentoo.org \
    --to=vapier@gentoo.org \
    --cc=u-boot@lists.denx.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