Add time measurement command to u-boot

common/cmd_time.c

+/*
+ * Copyright (c) 2011 The Chromium OS Authors. All rights reserved.
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ *
+ * Alternatively, this software may be distributed under the terms of the
+ * GNU General Public License ("GPL") version 2 as published by the Free
+ * Software Foundation.
+ */
+
+/* time - run command and report its run time */

[sjg, 2011/03/31 13:45:45]

run a command and ...

[Che-Liang Chiou, 2011/04/04 09:24:41]

Done.

+
+#include <common.h>
+#include <command.h>
+
+static void report_time(unsigned long int cycles)
+{
+#ifdef CONFIG_SYS_HZ
+        unsigned long int minutes, seconds, milliseconds;
+        unsigned long int r1000, ms, mshz, diff, min_diff;
+
+        seconds = cycles / CONFIG_SYS_HZ;
+        minutes = seconds / 60;
+        seconds %= 60;
+
+        /*

[sjg, 2011/03/31 13:45:45]

Can you just use something like:

milliseconds = cycles - 60 * (seconds + minutes * 60)

This code seems overly complicated to me. If there is some reason for this that
I don't understand please let me know.

[Che-Liang Chiou, 2011/04/04 09:24:41]

I guess you were trying to write
  milliseconds = cycles - 1000 * (seconds + minutes * 60)
because 1 second = 1000 milliseconds.

The formula implicitly assumes that 1 cycle = 1 millisecond (i.e., CONFIG_SYS_HZ
= 1000), but it is not always the case.
You are right, it is overly complex. I found a simpler and equally precise way
to approximate millisecond value.

+         * We approxmate milliseconds value by picking the value that
+         * the difference
+         *   remainder / CONFIG_SYS_HZ - milliseconds / 1000
+         * is minimal, where
+         *   remainder = cycles % CONFIG_SYS_HZ.
+         *
+         * But, to avoid floating-point arithmetic or truncation of
+         * integer arithmetic, we compute
+         *   remainder * 1000 - milliseconds * CONFIG_SYS_HZ
+         * instead.
+         */
+
+        /*
+         * TODO This is a simple linear search. We should make it faster
+         * if do_time() is time critical (maybe use a binary search?).
+         */
+        r1000 = (cycles % CONFIG_SYS_HZ) * 1000; /* r1000 = remainder * 1000 */
+        milliseconds = 0;
+        min_diff = r1000; /* r1000 - 0 * CONFIG_SYS_HZ = r1000 */
+        for (ms = 1; ms < 1000; ms++) {
+                mshz = ms * CONFIG_SYS_HZ;
+                diff = r1000 - mshz;
+                if (diff < min_diff) {
+                        milliseconds = ms;
+                        min_diff = diff;
+                }
+        }
+
+        printf("time:\t%lum%lu.%03lus\n", minutes, seconds, milliseconds);

[sjg, 2011/03/31 13:45:45]

It might be better to omit minutes if it is 0, and perhaps even omit seconds if
zero.

[Che-Liang Chiou, 2011/04/04 09:24:41]

Nice. I think it is a good idea to output differently to Unix's time command
since we are not a exact replica of it.

+#else
+        printf("CONFIG_SYS_HZ not defined\n");
+#endif
+
+        printf("cycles:\t%lu\n", cycles);

[sjg, 2011/03/31 13:45:45]

Perhaps put this on the same line as the 'time' output?

time: 4345 ticks, 4.345s

[Che-Liang Chiou, 2011/04/04 09:24:41]

Done.

+}
+
+int do_time(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+{
+        const int target_argc = argc - 1;
+

[sjg, 2011/03/31 13:45:45]

remove blank line

[Che-Liang Chiou, 2011/04/04 09:24:41]

Done.

+        int retval = 0;
+        unsigned long int cycles = 0;
+        cmd_tbl_t *target_cmdtp = NULL;
+
+        if (argc == 1) {
+                printf("no command provided\n");

[sjg, 2011/03/31 13:45:45]

Linux time prints elapsed time anyway in this case. Should you do the same?

[Che-Liang Chiou, 2011/04/04 09:24:41]

I think we are not making a exact replica of Unix's time command in u-boot. To
make that clear for a user, I think we should output differently to Unit's time
command. What do you think?

+                return 128;
+        }
+
+        /* parse command */
+

[sjg, 2011/03/31 13:45:45]

remove blank line

[Che-Liang Chiou, 2011/04/04 09:24:41]

Done.

+        target_cmdtp = find_cmd(argv[1]);
+        if (!target_cmdtp) {
+                printf("command not found: %s\n", argv[1]);
+                return 128;
+        }
+
+        if (target_argc > target_cmdtp->maxargs) {
+                printf("maxarags exceeded: %d > %d\n", target_argc,
+                                target_cmdtp->maxargs);
+                return 128;
+        }
+
+        /* run command and report run time */
+

[sjg, 2011/03/31 13:45:45]

remove blank line

[Che-Liang Chiou, 2011/04/04 09:24:41]

Done.

+        cycles = get_timer_masked();

[sjg, 2011/03/31 13:45:45]

cycles should perhaps be time_ms?

[Che-Liang Chiou, 2011/04/04 09:24:41]

cycles will be identical to milliseconds only if CONFIG_SYS_HZ is 1000 (though
it usually is).

+        retval = target_cmdtp->cmd(target_cmdtp, 0, target_argc, argv + 1);
+        cycles = get_timer_masked() - cycles;
+
+        putc('\n');
+        report_time(cycles);
+
+        return retval;
+}
+
+U_BOOT_CMD(time, CONFIG_SYS_MAXARGS, 0, do_time,
+                "run command and report its run time",

[sjg, 2011/03/31 13:45:45]

run a command...

[Che-Liang Chiou, 2011/04/04 09:24:41]

Done.

+                "command [args...]\n"
+                "the return value of time is the return value of "
+                "the command it executed and measured.");