[U-Boot] [PATCH 1/2] Create a single cmd_call() function to handle command execution

Mike Frysinger vapier at gentoo.org
Sun Oct 30 22:28:53 CET 2011


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 


More information about the U-Boot mailing list