Re: [U-Boot] [PATCH v2 0/9] Unified command execution in one place

2012-01-17 Thread Simon Glass
Hi Stefan,

On Wed, Jan 11, 2012 at 11:43 PM, Stefan Roese  wrote:
> On Saturday 10 December 2011 19:43:52 Simon Glass wrote:
>> At present two parsers have similar code to execute commands. Also
>> cmd_usage() is called all over the place. This series adds a single
>> function which processes commands called cmd_process().
>>
>> This new function understands return codes, and in particular
>> CMD_RET_USAGE to indicate a usage error. So rather than calling
>> cmd_usage() themselves, the command handlers can just return this
>> error.
>>
>> There appears to be a run_command2() which is used to run commands
>> with the selected parser. This series changes this in two separate
>> steps to just run_command(), and renames the old run_command() to
>> builtin_run_command(). No one should call this outside main.c since
>> if the hush parser is being used it is wrong to call it. The
>> built-in parser code could move into a separate file perhaps in a
>> future patch.
>>
>> The overall series reduces code size on ARM by about 1KB on
>> my ~160KB U-Boot text region when the hush parser is used, and around
>> 60 bytes when it isn't.
>>
>> As an aside the only user of parse_line() is fsl_ddr_interactive()
>> which seems to have its own command line interface which operates
>> before DRAM is set up. Do I have this right? Is there no way this
>> could be done later from a normal U-Boot command?
>
> Whole series:
>
> Applied to u-boot-staging/s...@denx.de. Thanks.

Thanks for your efforts with this, sorry about the return code
problem. I have re-done the series keeping the original return code
behaviour - let's wait and see what people think of that.

Regards,
Simon

>
> Best regards,
> Stefan
>
> --
> DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-0 Fax: (+49)-8142-66989-80 Email: off...@denx.de
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2 0/9] Unified command execution in one place

2012-01-11 Thread Stefan Roese
On Saturday 10 December 2011 19:43:52 Simon Glass wrote:
> At present two parsers have similar code to execute commands. Also
> cmd_usage() is called all over the place. This series adds a single
> function which processes commands called cmd_process().
> 
> This new function understands return codes, and in particular
> CMD_RET_USAGE to indicate a usage error. So rather than calling
> cmd_usage() themselves, the command handlers can just return this
> error.
> 
> There appears to be a run_command2() which is used to run commands
> with the selected parser. This series changes this in two separate
> steps to just run_command(), and renames the old run_command() to
> builtin_run_command(). No one should call this outside main.c since
> if the hush parser is being used it is wrong to call it. The
> built-in parser code could move into a separate file perhaps in a
> future patch.
> 
> The overall series reduces code size on ARM by about 1KB on
> my ~160KB U-Boot text region when the hush parser is used, and around
> 60 bytes when it isn't.
> 
> As an aside the only user of parse_line() is fsl_ddr_interactive()
> which seems to have its own command line interface which operates
> before DRAM is set up. Do I have this right? Is there no way this
> could be done later from a normal U-Boot command?

Whole series:

Applied to u-boot-staging/s...@denx.de. Thanks.
 
Best regards,
Stefan

--
DENX Software Engineering GmbH,  MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-0 Fax: (+49)-8142-66989-80 Email: off...@denx.de
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2 0/9] Unified command execution in one place

2012-01-11 Thread Stefan Roese
Hi Simon,

On Thursday 12 January 2012 05:37:00 Simon Glass wrote:
> Hi,
> 
> On Sat, Dec 10, 2011 at 10:43 AM, Simon Glass  wrote:
> > At present two parsers have similar code to execute commands. Also
> > cmd_usage() is called all over the place. This series adds a single
> > function which processes commands called cmd_process().
> > 
> > This new function understands return codes, and in particular
> > CMD_RET_USAGE to indicate a usage error. So rather than calling
> > cmd_usage() themselves, the command handlers can just return this
> > error.
> > 
> > There appears to be a run_command2() which is used to run commands
> > with the selected parser. This series changes this in two separate
> > steps to just run_command(), and renames the old run_command() to
> > builtin_run_command(). No one should call this outside main.c since
> > if the hush parser is being used it is wrong to call it. The
> > built-in parser code could move into a separate file perhaps in a
> > future patch.
> > 
> > The overall series reduces code size on ARM by about 1KB on
> > my ~160KB U-Boot text region when the hush parser is used, and around
> > 60 bytes when it isn't.
> > 
> > As an aside the only user of parse_line() is fsl_ddr_interactive()
> > which seems to have its own command line interface which operates
> > before DRAM is set up. Do I have this right? Is there no way this
> > could be done later from a normal U-Boot command?
> > 
> > (I have run this through MAKEALL and it seems clean)
> > 
> > Changes in v2:
> > - Fix minor errors one of which created a warning
> > - Squash i2c patch into the common/ patch
> > 
> > Simon Glass (9):
> >  Remove CMD_PXE's static on run_command()
> >  Rename run_command() to builtin_run_command()
> >  Rename run_command2() to run_command()
> >  Stop using builtin_run_command()
> >  Don't include standard parser if hush is used
> >  Create a single cmd_call() function to handle command execution
> >  Remove interleave of non-U-Boot code in hush
> >  Add cmd_process() to process commands in one place
> >  Convert cmd_usage() calls in common to use a return value
> 
> Does any of the maintainers feel inclined to pick up these patches? It
> still seems to rebase cleanly against master, so it might be ok as is,
> but happy to resend if needed.

Looking into it right now. Stay tuned...
 
Best regards,
Stefan

--
DENX Software Engineering GmbH,  MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-0 Fax: (+49)-8142-66989-80 Email: off...@denx.de
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2 0/9] Unified command execution in one place

2012-01-11 Thread Simon Glass
Hi,

On Sat, Dec 10, 2011 at 10:43 AM, Simon Glass  wrote:
> At present two parsers have similar code to execute commands. Also
> cmd_usage() is called all over the place. This series adds a single
> function which processes commands called cmd_process().
>
> This new function understands return codes, and in particular
> CMD_RET_USAGE to indicate a usage error. So rather than calling
> cmd_usage() themselves, the command handlers can just return this
> error.
>
> There appears to be a run_command2() which is used to run commands
> with the selected parser. This series changes this in two separate
> steps to just run_command(), and renames the old run_command() to
> builtin_run_command(). No one should call this outside main.c since
> if the hush parser is being used it is wrong to call it. The
> built-in parser code could move into a separate file perhaps in a
> future patch.
>
> The overall series reduces code size on ARM by about 1KB on
> my ~160KB U-Boot text region when the hush parser is used, and around
> 60 bytes when it isn't.
>
> As an aside the only user of parse_line() is fsl_ddr_interactive()
> which seems to have its own command line interface which operates
> before DRAM is set up. Do I have this right? Is there no way this
> could be done later from a normal U-Boot command?
>
> (I have run this through MAKEALL and it seems clean)
>
> Changes in v2:
> - Fix minor errors one of which created a warning
> - Squash i2c patch into the common/ patch
>
> Simon Glass (9):
>  Remove CMD_PXE's static on run_command()
>  Rename run_command() to builtin_run_command()
>  Rename run_command2() to run_command()
>  Stop using builtin_run_command()
>  Don't include standard parser if hush is used
>  Create a single cmd_call() function to handle command execution
>  Remove interleave of non-U-Boot code in hush
>  Add cmd_process() to process commands in one place
>  Convert cmd_usage() calls in common to use a return value

Does any of the maintainers feel inclined to pick up these patches? It
still seems to rebase cleanly against master, so it might be ok as is,
but happy to resend if needed.

Regards,
Simon

>
>  arch/arm/cpu/arm926ejs/kirkwood/cpu.c |    7 +--
>  board/esd/common/cmd_loadpci.c        |    2 +-
>  board/esd/du440/du440.c               |    2 +-
>  common/cmd_bedbug.c                   |    6 +-
>  common/cmd_bmp.c                      |    6 +-
>  common/cmd_boot.c                     |    2 +-
>  common/cmd_bootm.c                    |   10 +---
>  common/cmd_cache.c                    |    4 +-
>  common/cmd_dataflash_mmc_mux.c        |    2 +-
>  common/cmd_date.c                     |    3 +-
>  common/cmd_dcr.c                      |    8 +-
>  common/cmd_df.c                       |    2 +-
>  common/cmd_eeprom.c                   |    2 +-
>  common/cmd_ext2.c                     |    4 +-
>  common/cmd_fdc.c                      |    2 +-
>  common/cmd_fdos.c                     |    2 +-
>  common/cmd_fdt.c                      |   14 ++--
>  common/cmd_fitupd.c                   |    2 +-
>  common/cmd_flash.c                    |   14 ++--
>  common/cmd_fpga.c                     |    4 +-
>  common/cmd_gpio.c                     |    2 +-
>  common/cmd_i2c.c                      |   32 +-
>  common/cmd_ide.c                      |   10 ++--
>  common/cmd_irq.c                      |    2 +-
>  common/cmd_itest.c                    |    2 +-
>  common/cmd_led.c                      |    6 +-
>  common/cmd_load.c                     |    2 +-
>  common/cmd_log.c                      |    4 +-
>  common/cmd_md5sum.c                   |    2 +-
>  common/cmd_mdio.c                     |    2 +-
>  common/cmd_mem.c                      |   22 
>  common/cmd_mfsl.c                     |   10 ++--
>  common/cmd_mgdisk.c                   |    2 +-
>  common/cmd_mii.c                      |    4 +-
>  common/cmd_misc.c                     |    2 +-
>  common/cmd_mmc.c                      |   14 ++--
>  common/cmd_mmc_spi.c                  |    3 +-
>  common/cmd_mp.c                       |    8 +-
>  common/cmd_mtdparts.c                 |    2 +-
>  common/cmd_nand.c                     |    6 +-
>  common/cmd_net.c                      |    6 +-
>  common/cmd_nvedit.c                   |   22 
>  common/cmd_onenand.c                  |   12 ++--
>  common/cmd_otp.c                      |    2 +-
>  common/cmd_pci.c                      |    2 +-
>  common/cmd_portio.c                   |    4 +-
>  common/cmd_pxe.c                      |   10 ++--
>  common/cmd_reiser.c                   |    4 +-
>  common/cmd_sata.c                     |    8 +-
>  common/cmd_scsi.c                     |   15 +++--
>  common/cmd_setexpr.c                  |    2 +-
>  common/cmd_sf.c                       |    2 +-
>  common/cmd_sha1sum.c                  |    2 +-
>  common/cmd_source.c                   |    2 +-
>  common/cmd_strings.c                  |    2 +-
>  comm