[U-Boot] cmd_time: add time command

The 'time' command runs and reports execution time of commands.

Sameple usage:
--------------------
u-boot# time crc 0x1000 1000
CRC32 for 00001000 ... 00001fff ==> ae94dc4b

time: 0.004 seconds, 4 ticks
--------------------

Signed-off-by: Che-Liang Chiou <clchiou@chromium.org>
---
 README                   |    1 +
 common/Makefile          |    1 +
 common/cmd_time.c        |   84 ++++++++++++++++++++++++++++++++++++++++++++++
 include/config_cmd_all.h |    1 +
 4 files changed, 87 insertions(+), 0 deletions(-)
 create mode 100644 common/cmd_time.c

On Wednesday 05 October 2011 03:09:15 Che-Liang Chiou wrote:
> The 'time' command runs and reports execution time of commands.

cool

> Sameple usage:

typo: Sample

> --- /dev/null
> +++ b/common/cmd_time.c
>
> +static void report_time(unsigned long int cycles)
> +{
> +#ifdef CONFIG_SYS_HZ

CONFIG_SYS_HZ is required to be defined, so please drop this ifdef check

> +	unsigned long int minutes, seconds, milliseconds;
> +	unsigned long int total_seconds, remainder;
> +
> +	total_seconds = cycles / CONFIG_SYS_HZ;
> +	remainder = cycles % CONFIG_SYS_HZ;
> +	minutes = total_seconds / 60;
> +	seconds = total_seconds % 60;
> +	/* approximate millisecond value */
> +	milliseconds = (remainder * 1000 + CONFIG_SYS_HZ / 2) / CONFIG_SYS_HZ;
> +
> +	printf("time:");
> +	if (minutes)
> +		printf(" %lu minutes,", minutes);
> +	printf(" %lu.%03lu seconds, %lu ticks\n",
> +			seconds, milliseconds, cycles);

looks like this could use the new timer api that Graeme is throwing around

> +int do_time(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])

static

> +	cmd_tbl_t *target_cmdtp = NULL;

const

> +	if (argc == 1) {
> +		printf("no command provided\n");
> +		return 1;
> +	}

return cmd_usage(cmdtp);

> +	if (!target_cmdtp) {
> +		printf("command not found: %s\n", argv[1]);
> +		return 1;
> +	}

shells typically display:
	%s: command not found

> +	if (target_argc > target_cmdtp->maxargs) {
> +		printf("maxarags exceeded: %d > %d\n", target_argc,
> +				target_cmdtp->maxargs);
> +		return 1;
> +	}
> ...
> +	retval = target_cmdtp->cmd(target_cmdtp, 0, target_argc, argv + 1);

hmm, this looks like we should add a helper like run_command() and put this 
logic there ...

> +	putc('\n');
> +	report_time(cycles);

why not just integrate that \n into the report_time() code ?
-mike

Hi Mike,

Thanks for comments. Reply inlined.

On Thu, Oct 6, 2011 at 1:24 AM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Wednesday 05 October 2011 03:09:15 Che-Liang Chiou wrote:
>> The 'time' command runs and reports execution time of commands.
>
> cool
>
>> Sameple usage:
>
> typo: Sample
Done.
>
>> --- /dev/null
>> +++ b/common/cmd_time.c
>>
>> +static void report_time(unsigned long int cycles)
>> +{
>> +#ifdef CONFIG_SYS_HZ
>
> CONFIG_SYS_HZ is required to be defined, so please drop this ifdef check
>
Done.
>> +     unsigned long int minutes, seconds, milliseconds;
>> +     unsigned long int total_seconds, remainder;
>> +
>> +     total_seconds = cycles / CONFIG_SYS_HZ;
>> +     remainder = cycles % CONFIG_SYS_HZ;
>> +     minutes = total_seconds / 60;
>> +     seconds = total_seconds % 60;
>> +     /* approximate millisecond value */
>> +     milliseconds = (remainder * 1000 + CONFIG_SYS_HZ / 2) / CONFIG_SYS_HZ;
>> +
>> +     printf("time:");
>> +     if (minutes)
>> +             printf(" %lu minutes,", minutes);
>> +     printf(" %lu.%03lu seconds, %lu ticks\n",
>> +                     seconds, milliseconds, cycles);
>
> looks like this could use the new timer api that Graeme is throwing around
>
Exactly. Add TODO comments.
>> +int do_time(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>
> static
>
Done.
>> +     cmd_tbl_t *target_cmdtp = NULL;
>
> const
>
I guess I cannot change it to a const pointer. cmdtp->cmd() signature
requires a non-const pointer to cmdtp.
>> +     if (argc == 1) {
>> +             printf("no command provided\n");
>> +             return 1;
>> +     }
>
> return cmd_usage(cmdtp);
>
Done.
>> +     if (!target_cmdtp) {
>> +             printf("command not found: %s\n", argv[1]);
>> +             return 1;
>> +     }
>
> shells typically display:
>        %s: command not found
>
Done.
>> +     if (target_argc > target_cmdtp->maxargs) {
>> +             printf("maxarags exceeded: %d > %d\n", target_argc,
>> +                             target_cmdtp->maxargs);
>> +             return 1;
>> +     }
>> ...
>> +     retval = target_cmdtp->cmd(target_cmdtp, 0, target_argc, argv + 1);
>
> hmm, this looks like we should add a helper like run_command() and put this
> logic there ...
>
I agree. I moved these codes to a separate function. It can be merged
with run_command() if necessary after the timer API is landed.
>> +     putc('\n');
>> +     report_time(cycles);
>
> why not just integrate that \n into the report_time() code ?
Done.
> -mike
>

Regards,
Che-Liang

On Thu, Oct 6, 2011 at 04:38, Che-liang Chiou wrote:
> On Thu, Oct 6, 2011 at 1:24 AM, Mike Frysinger wrote:
>> On Wednesday 05 October 2011 03:09:15 Che-Liang Chiou wrote:
>>> +     cmd_tbl_t *target_cmdtp = NULL;
>>
>> const
>
> I guess I cannot change it to a const pointer. cmdtp->cmd() signature
> requires a non-const pointer to cmdtp.

ah, yeah, that's something i've been meaning to fix for a while
-mike