Create a single cmd_call() function to handle command execution
PATCH 1/2 Create a single cmd_call() function to handle command execution
We should aim for a single point of entry to the commands, whichever parser is used.
--- common/command.c | 10 ++++++++++ common/hush.c | 9 +++------ common/main.c | 3 +-- include/command.h | 12 ++++++++++++ 4 files changed, 26 insertions(+), 8 deletions(-)
common/command.c +10 -0
@@ -487,3 +487,13 @@ void fixup_cmdtable(cmd_tbl_t *cmdtp, int size) } }#endif + +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; +}
common/hush.c +3 -6
@@ -1695,13 +1695,10 @@ static int run_pipe_real(struct pipe *pi) rcode = x->function (child);#else /* OK - call function to do the command */ - - rcode = (cmdtp->cmd) -(cmdtp, flag,child->argc-i,&child->argv[i]); -if ( !cmdtp->repeatable ) + rcode =cmd_call (cmdtp, flag, child->argc-i, + &child->argv[i]); +if (!cmdtp->repeatable) flag_repeat =0 ; - -#endif child->argv-=i;/* XXX restore hack so free() can work right */ #ifndef __U_BOOT__
common/main.c +1 -2
@@ -1376,9 +1376,8 @@ int run_command (const char *cmd, int flag)#endif /* OK - call function to do the command */ -if ((cmdtp->cmd) (cmdtp, flag, argc, argv) !=0 ) { +if (cmd_call (cmdtp, flag, argc, argv) !=0 ) rc =-1 ; - } repeatable &= cmdtp->repeatable;
include/command.h +12 -0
@@ -147,4 +147,16 @@ extern int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);#if defined(CONFIG_NEEDS_MANUAL_RELOC )void fixup_cmdtable (cmd_tbl_t *cmdtp,int size);#endif + +/** +* Call a command function. This should be the only route in U-Boot to call +* a command, so that we can track whether we are waiting for input or +* executing a command. +* +* @param cmdtp Pointer to the command to execute +* @param flag Some flags normally 0 (see CMD_FLAG_.. above) +* @param argc Number of arguments (arg 0 must be the command text) +* @param argv Arguments +*/ +int cmd_call (cmd_tbl_t *cmdtp,int flag,int argc,char *const argv[]);#endif /* __COMMAND_H */
Signature · 1 line
1.7.3.1
PATCH 2/2 RFC: Add XON/XOFF support
This is just for testing - please try it out and report back with results. For me it works on Minicom but not ser2net.
This needs to be controlled by an environment variable, CONFIG option or both. We may need a way of specifying flow control on a per-device basis. We may need to do something special for X-modem transfers, etc. This might turn out to be a can of worms.
--- common/command.c | 1 + common/console.c | 12 ++++++++++++ common/main.c | 3 +++ include/common.h | 2 ++ 4 files changed, 18 insertions(+), 0 deletions(-)
common/command.c +1 -0
@@ -492,6 +492,7 @@ int cmd_call(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) {int result; +console_suspend_input (); result = (cmdtp->cmd)(cmdtp, flag, argc, argv);if (result)debug ("Command failed, result=%d" , result);
common/console.c +12 -0
@@ -199,6 +199,18 @@ static inline void console_doenv(int file, struct stdio_dev *dev) }#endif /* defined(CONFIG_CONSOLE_MUX) */ +void console_suspend_input (void ) +{ +/* Send XOFF to tell the other end to stop sending */ +console_putc (stdout,'S' -'@' ); +} + +void console_resume_input (void ) +{ +/* Send XON to tell the other end to start sending */ +console_putc (stdout,'Q' -'@' ); +} +/** U-Boot INITIAL CONSOLE-NOT COMPATIBLE FUNCTIONS *************************/ int serial_printf (const char *fmt, ...)
common/main.c +3 -0
@@ -948,6 +948,7 @@ int readline_into_buffer (const char *const prompt, char * buffer)if (prompt)puts (prompt); +console_resume_input (); rc =cread_line (prompt, p, &len);return rc <0 ? rc : len;
@@ -967,6 +968,8 @@ int readline_into_buffer (const char *const prompt, char * buffer) } col = plen; +console_resume_input (); +for (;;) {#ifdef CONFIG_BOOT_RETRY_TIME while (!tstc ()) {/* while no incoming data */
include/common.h +2 -0
@@ -738,6 +738,8 @@ int ctrlc (void);int had_ctrlc (void );/* have we had a Control-C since last clear? */ void clear_ctrlc (void );/* clear the Control-C condition */ int disable_ctrlc (int );/* 1 to disable, 0 to enable Control-C detect */ +void console_suspend_input (void ); +void console_resume_input (void );/* * STDIO based functions (can always be used)
Signature · 1 line
1.7.3.1
PATCH 2/2 RFC: Add XON/XOFF support
Hi Simon,
Quoted 21 lines · Simon Glass
This is just for testing - please try it out and report back with results. For me it works on Minicom but not ser2net.
This needs to be controlled by an environment variable, CONFIG option or both. We may need a way of specifying flow control on a per-device basis. We may need to do something special for X-modem transfers, etc. This might turn out to be a can of worms.
Signed-off-by Simon Glass <[email protected]>--- common/command.c | 1 + common/console.c | 12 ++++++++++++ common/main.c | 3 +++ include/common.h | 2 ++ 4 files changed, 18 insertions(+), 0 deletions(-)common/command.c +0
-0
common/command.c int cmd_call(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) · line 495
Quoted 4 lines
common/command.c +1
-0@@ -492,6 +492,7 @@ int cmd_call(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) {int result; +console_suspend_input ();
Not the best name, but I cannot think of better right now
Quoted 8 lines
common/command.c +0
-0result = (cmdtp->cmd)(cmdtp, flag, argc, argv);if (result)debug ("Command failed, result=%d" , result);common/console.c +0
-0
common/console.c static inline void console_doenv(int file, struct stdio_dev *dev) · line 205
Quoted 7 lines
common/console.c +4
-0@@ -199,6 +199,18 @@ static inline void console_doenv(int file, struct stdio_dev *dev) }#endif /* defined(CONFIG_CONSOLE_MUX) */ +void console_suspend_input (void ) +{ +/* Send XOFF to tell the other end to stop sending */ +console_putc (stdout,'S' -'@' );
Uugh - #define XON and XOFF please
Quoted 16 lines
common/console.c +8
-0+} + +void console_resume_input (void ) +{ +/* Send XON to tell the other end to start sending */ +console_putc (stdout,'Q' -'@' ); +} +/** U-Boot INITIAL CONSOLE-NOT COMPATIBLE FUNCTIONS *************************/ int serial_printf (const char *fmt, ...)common/main.c +0
-0
common/main.c int readline_into_buffer (const char *const prompt, char * buffer) · line 951
Quoted 4 lines
common/main.c +1
-0@@ -948,6 +948,7 @@ int readline_into_buffer (const char *const prompt, char * buffer)if (prompt)puts (prompt); +console_resume_input ();
Not needed - move to getc()
common/main.c int readline_into_buffer (const char *const prompt, char * buffer) · line 971
Quoted 7 lines
common/main.c +1
-0rc =cread_line (prompt, p, &len);return rc <0 ? rc : len;@@ -967,6 +968,8 @@ int readline_into_buffer (const char *const prompt, char * buffer) } col = plen; +console_resume_input ();
Not needed (in getc())
Quoted 9 lines
common/main.c +1
-0+for (;;) {#ifdef CONFIG_BOOT_RETRY_TIME while (!tstc ()) {/* while no incoming data */ include/common.h +0
-0
include/common.h int ctrlc (void); · line 745
Quoted 8 lines
include/common.h +2
-0@@ -738,6 +738,8 @@ int ctrlc (void);int had_ctrlc (void );/* have we had a Control-C since last clear? */ void clear_ctrlc (void );/* clear the Control-C condition */ int disable_ctrlc (int );/* 1 to disable, 0 to enable Control-C detect */ +void console_suspend_input (void ); +void console_resume_input (void );/* * STDIO based functions (can always be used)
What about the secondart 'rx_flush()' I mentioned?
Regards,
Graeme
PATCH 2/2 RFC: Add XON/XOFF support
Hi Graeme,
Quoted 42 lines · Graeme Russ
Hi Simon,
Quoted 21 lines · Simon Glass
This is just for testing - please try it out and report back with results. For me it works on Minicom but not ser2net.
This needs to be controlled by an environment variable, CONFIG option or both. We may need a way of specifying flow control on a per-device basis. We may need to do something special for X-modem transfers, etc. This might turn out to be a can of worms.
Signed-off-by Simon Glass <[email protected]>---
?common/command.c | ? ?1 + ?common/console.c | ? 12 ++++++++++++ ?common/main.c ? ?| ? ?3 +++ ?include/common.h | ? ?2 ++?4 files changed, 18 insertions(+), 0 deletions(-)
common/command.c +0
-0common/command.c int cmd_call(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) · line 495
Quoted 4 lines
common/command.c +1
-0@@ -492,6 +492,7 @@ int cmd_call(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) ?{ ? ? ? int result; + ? ? console_suspend_input();Not the best name, but I cannot think of better right now
Quoted 11 lines
common/command.c +0
-0? ? ? result = (cmdtp->cmd)(cmdtp, flag, argc, argv); ? ? ? if (result) ? ? ? ? ? ? ? debug("Command failed, result=%d", result);common/console.c +0
-0@@ -199,6 +199,18 @@ static inline void console_doenv(int file, struct stdio_dev *dev) ?}?#endif /*
defined(CONFIG_CONSOLE_MUX)*/
Quoted 6 lines
common/console.c static inline void console_doenv(int file, struct stdio_dev *dev)
Quoted 4 lines
common/console.c +4
-0+void console_suspend_input (void ) +{ + ? ? /* Send XOFF to tell the other end to stop sending */ + ? ? console_putc(stdout, 'S' - '@');Uugh - #define XON and XOFF please
OK
Quoted 18 lines
Quoted 18 lines
common/console.c +8
-0+} + +void console_resume_input (void ) +{ + ? ? /* Send XON to tell the other end to start sending */ + ? ? console_putc(stdout, 'Q' - '@'); +} +?/** U-Boot INITIAL CONSOLE-NOT COMPATIBLE FUNCTIONS *************************/
?int
serial_printf(const char *fmt, ...)common/main.c +0
-0@@ -948,6 +948,7 @@ int readline_into_buffer (const char *const prompt, char * buffer)
Quoted 5 lines
common/main.c int readline_into_buffer (const char *const prompt, char * buffer) · line 951
Quoted 3 lines
common/main.c +1
-0? ? ? ? ? ? ? if (prompt) ? ? ? ? ? ? ? ? ? ? ? puts (prompt); + ? ? ? ? ? ? console_resume_input();Not needed - move to
getc()
Well we need to thing about the other issues mentioned in my commit message also.
Quoted 26 lines
common/main.c int readline_into_buffer (const char *const prompt, char * buffer) · line 971
Quoted 8 lines
common/main.c +1
-0? ? ? ? ? ? ? rc = cread_line(prompt, p, &len); ? ? ? ? ? ? ? return rc < 0 ? rc : len;@@ -967,6 +968,8 @@ int readline_into_buffer (const char *const prompt, char * buffer) ? ? ? } ? ? ? col = plen; + ? ? console_resume_input();Not needed (in
getc())Quoted 15 lines
common/main.c +1
-0+ ? ? ? for (;;) {?#ifdef
CONFIG_BOOT_RETRY_TIME
? ? ? ? ? ? ? while (!tstc()) { ? ? ? /* while no incoming data */include/common.h +0
-0@@ -738,6 +738,8 @@ int ? ? ? ctrlc (void);?int ?had_ctrlc (void); ? ? ? /* have we had a Control-C since last clear? */ ?void
clear_ctrlc(void); ? ? /* clear the Control-C condition */ ?int ?disable_ctrlc (int); ? ?/* 1 to disable, 0 to enable Control-C detect */include/common.h +2
-0+void console_suspend_input (void ); +void console_resume_input (void );
Quoted 4 lines
Quoted 2 lines
?/*
? * STDIO based functions (can always be used)What about the secondart '
rx_flush()' I mentioned?
Will await discussions before doing a new patch :-)
Regards,
Simon
Quoted 5 lines
Regards,
Graeme
PATCH 2/2 RFC: Add XON/XOFF support
In reply to Simon GlassQuoted 7 lines · On Monday 24 October 2011 23:52:24 Simon Glass wrote:
This is just for testing - please try it out and report back with results. For me it works on Minicom but not ser2net.
This needs to be controlled by an environment variable, CONFIG option or both. We may need a way of specifying flow control on a per-device basis. We may need to do something special for X-modem transfers, etc. This might turn out to be a can of worms.
we have CONFIG_HWFLOW already, so CONFIG_SWFLOW would probably be the natural choice. and then have the funcs be named swflow_xxx (although the hwflow code does it via one and uses its arg to figure out what to do).
-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/e2f12354/attachment.pgp
PATCH 2/2 RFC: Add XON/XOFF support
In reply to Simon GlassHi Simon,
Le 25/10/2011 05:52, Simon Glass a ?crit :
Quoted 2 lines
This is just for testing - please try it out and report back with results. For me it works on Minicom but not ser2net.
Exactly how does it not work, i.e. what happens that you did not expect, or what does not happen that you did expect?
Quoted 2 lines
This needs to be controlled by an environment variable, CONFIG option or both. We may need a way of specifying flow control on a per-device basis.
Hmm... We want XON/XOFF on the console only. What other device would require this?
Quoted 2 lines
We may need to do something special for X-modem transfers, etc. This might turn out to be a can of worms.
I think any U-Boot command that uses the console as a means of data transfer should 'resume console' when starting, and can safely do so because if the command was invoked, then most certainly the other end of the console is not going to fire away; it is going to wait for U-Boot to get ready for transfer. Suspending the console after transfer should be done if resume was done.
BTW:
Quoted 25 lines
Signed-off-by Simon Glass<[email protected]>--- common/command.c | 1 + common/console.c | 12 ++++++++++++ common/main.c | 3 +++ include/common.h | 2 ++ 4 files changed, 18 insertions(+), 0 deletions(-)common/command.c +1
-0@@ -492,6 +492,7 @@ int cmd_call(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) {int result; +console_suspend_input (); result = (cmdtp->cmd)(cmdtp, flag, argc, argv);if (result)debug ("Command failed, result=%d" , result);common/console.c +0
-0
common/console.c static inline void console_doenv(int file, struct stdio_dev *dev) · line 206
Quoted 8 lines
common/console.c +5
-0@@ -199,6 +199,18 @@ static inline void console_doenv(int file, struct stdio_dev *dev) }#endif /* defined(CONFIG_CONSOLE_MUX) */ +void console_suspend_input (void ) +{ +/* Send XOFF to tell the other end to stop sending */ +console_putc (stdout,'S' -'@' ); +}
Please define XOFF and XON.
Quoted 14 lines
common/console.c +6
-0+void console_resume_input (void ) +{ +/* Send XON to tell the other end to start sending */ +console_putc (stdout,'Q' -'@' ); +} +/** U-Boot INITIAL CONSOLE-NOT COMPATIBLE FUNCTIONS *************************/ int serial_printf (const char *fmt, ...)common/main.c +0
-0
common/main.c int readline_into_buffer (const char *const prompt, char * buffer) · line 951
Quoted 4 lines
common/main.c +1
-0@@ -948,6 +948,7 @@ int readline_into_buffer (const char *const prompt, char * buffer)if (prompt)puts (prompt); +console_resume_input ();
This makes XON sent on each readline. XOFF should be sent only if XOFF was sent before, IMO (and then, XOFF should only be sent if it was not sent already.
Plus, why send it on readlines? Why not just surround the 'result = (cmdtp->cmd)(cmdtp, flag, argc, argv);' above with a 'suspend' before and a 'resume' after?
Quoted 16 lines
common/main.c +2
-0rc =cread_line (prompt, p,&len);return rc<0 ? rc : len;@@ -967,6 +968,8 @@ int readline_into_buffer (const char *const prompt, char * buffer) } col = plen; +console_resume_input (); +for (;;) {#ifdef CONFIG_BOOT_RETRY_TIME while (!tstc ()) {/* while no incoming data */ include/common.h +0
-0
include/common.h int ctrlc (void); · line 745
Quoted 8 lines
include/common.h +2
-0@@ -738,6 +738,8 @@ int ctrlc (void);int had_ctrlc (void );/* have we had a Control-C since last clear? */ void clear_ctrlc (void );/* clear the Control-C condition */ int disable_ctrlc (int );/* 1 to disable, 0 to enable Control-C detect */ +void console_suspend_input (void ); +void console_resume_input (void );/* * STDIO based functions (can always be used)
Amicalement,
Signature · 1 line
Albert.
PATCH 1/2 Create a single cmd_call() function to handle command execution
In reply to Simon GlassDear Simon Glass,
Quoted 19 lines · In message <[email protected]> you wrote:
We should aim for a single point of entry to the commands, whichever parser is used.
Signed-off-by Simon Glass <[email protected]>--- common/command.c | 10 ++++++++++ common/hush.c | 9 +++------ common/main.c | 3 +-- include/command.h | 12 ++++++++++++ 4 files changed, 26 insertions(+), 8 deletions(-)common/command.c +0
-0@@ -487,3 +487,13 @@ void fixup_cmdtable(cmd_tbl_t *cmdtp, int size) } }#endif
common/command.c void fixup_cmdtable(cmd_tbl_t *cmdtp, int size) · line 499
Quoted 10 lines
common/command.c +10
-0+ +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; +}
What exactly is the purpose of this additional function? Except for the debug() it provides only overhead and no benefit.
I don't think I want to have that.
Best regards,
Wolfgang Denk
Signature · 6 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Mr. Cole's Axiom:
The sum of the intelligence on the planet is a constant;
the population is growing.PATCH 1/2 Create a single cmd_call() function to handle command execution
Hi Wolfgang,
Quoted 27 lines · Wolfgang Denk
Dear Simon Glass,
Quoted 24 lines · In message <[email protected]> you wrote:
We should aim for a single point of entry to the commands, whichever parser is used.
Signed-off-by Simon Glass <[email protected]>--- common/command.c | 10 ++++++++++ common/hush.c | 9 +++------ common/main.c | 3 +-- include/command.h | 12 ++++++++++++ 4 files changed, 26 insertions(+), 8 deletions(-)common/command.c +5
-0@@ -487,3 +487,13 @@ void fixup_cmdtable(cmd_tbl_t *cmdtp, int size) } }#endif + +int cmd_call (cmd_tbl_t *cmdtp,int flag,int argc,char *const argv[]) +{ +int result; +
Quoted 8 lines
common/command.c void fixup_cmdtable(cmd_tbl_t *cmdtp, int size) · line 499
Quoted 5 lines
common/command.c +5
-0+ result = (cmdtp->cmd)(cmdtp, flag, argc, argv); +if (result) +debug ("Command failed, result=%d" , result); +return result; +}What exactly is the purpose of this additional function? Except for the
debug()it provides only overhead and no benefit.
It provides a single location to issue an XOFF immediately prior to running a (potentially long running) command
Quoted 1 line
I don't think I want to have that.
Well it does make things cleaner if we do end up implementing software flow control
Regards,
Graeme
PATCH 1/2 Create a single cmd_call() function to handle command execution
Hi Wolfgang,
Quoted 39 lines · Graeme Russ
Hi Wolfgang,
Quoted 27 lines · Wolfgang Denk
Dear Simon Glass,
Quoted 24 lines · In message <[email protected]> you wrote:
We should aim for a single point of entry to the commands, whichever parser is used.
Signed-off-by Simon Glass <[email protected]>---
?common/command.c ?| ? 10 ++++++++++ ?common/hush.c ? ? | ? ?9 +++------ ?common/main.c ? ? | ? ?3 +-- ?include/command.h | ? 12 ++++++++++++?4 files changed, 26 insertions(+), 8 deletions(-)
common/command.c +0
-0@@ -487,3 +487,13 @@ void fixup_cmdtable(cmd_tbl_t *cmdtp, int size) ? ? ?} ?}?#endif
common/command.c +5
-0+ +int cmd_call (cmd_tbl_t *cmdtp,int flag,int argc,char *const argv[]) +{ + ? ?int result; +Quoted 8 lines
Quoted 5 lines
+ ? ?result = (cmdtp->cmd)(cmdtp, flag, argc, argv);
+ ? ?if (result)
+ ? ? ? ? ? ?debug("Command failed, result=%d", result);
+ ? ?return result;
+}What exactly is the purpose of this additional function? ?Except for the
debug()it provides only overhead and no benefit.
Quoted 7 lines
It provides a single location to issue an XOFF immediately prior to running a (potentially long running) command
Quoted 1 line
I don't think I want to have that.
Well it does make things cleaner if we do end up implementing software flow control
What Graeme said :-)
It could probably be done in an inline fashion though to avoid overhead.
Regards,
Simon
Quoted 5 lines
Regards,
Graeme
PATCH 1/2 Create a single cmd_call() function to handle command execution
In reply to Graeme RussDear Graeme Russ,
Quoted 11 lines · In message <[email protected]> you wrote:
Quoted 3 lines
What exactly is the purpose of this additional function? Except for the
debug()it provides only overhead and no benefit.It provides a single location to issue an XOFF immediately prior to running a (potentially long running) command
Quoted 1 line
I don't think I want to have that.
Well it does make things cleaner if we do end up implementing software flow control
If, and only if. _And_ if we agree that we should hook into command execution, which I don't consider a good idea.
Serial flow control has nothing to do with that.
Best regards,
Wolfgang Denk
Signature · 5 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de You can fool some of the people all of the time, and You can fool all of the people some of the time, but You can't fool mom.
PATCH 1/2 Create a single cmd_call() function to handle command execution
In reply to Simon GlassReview
Quoted 9 lines · Simon Glass
(quoted diff) +9
-0+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
Quoted 1 line
cmdtp->maxargs) returncmd_usage(cmdtp);".
and perhaps even the find_cmd(argv[0]) lookup ...
-mike
PATCH 1/2 Create a single cmd_call() function to handle command execution
Hi Mike,
Quoted 14 lines · Mike Frysinger
Review
Quoted 9 lines · Simon Glass
(quoted diff) +9
-0+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
Quoted 1 line
cmdtp->maxargs) returncmd_usage(cmdtp);".
Yes, that might turn this into a patch worth accepting on its merits.
Quoted 3 lines
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
Continuing down the slippery slope, for find_cmd() there is sometimes a secondary command, and there are also argument to decode. We could devise a printf-style command description to specify the arguments for many/most commands. Even less code. More complicated.
Regards,
Simon
PATCH 1/2 Create a single cmd_call() function to handle command execution
Quoted 16 lines · On Tuesday 25 October 2011 19:05:02 Simon Glass wrote:
Quoted 14 lines · Mike Frysinger
common/command.c void fixup_cmdtable(cmd_tbl_t *cmdtp, int size) · line 499
Quoted 9 lines · Simon Glass
common/command.c +9
-0+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
Quoted 1 line
cmdtp->maxargs) returncmd_usage(cmdtp);".
Quoted 8 lines
Yes, that might turn this into a patch worth accepting on its merits.
Quoted 2 lines
and perhaps even the
find_cmd(argv[0])lookup ...
-mikeHow about if the commands return error codes, one of which means 'print usage'? Then we might remove 265 calls to
cmd_usageand 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