[U-Boot] [PATCH] CONFIG_PRINT_TIME: Measuring boot time.

[Jean-Christophe PLAGNIOL-VILLARD]

On 15:17 Thu 05 Mar     , Benoit Monin wrote:
>    De : "Jean-Christophe PLAGNIOL-VILLARD"
> 
>    Hi Jean-Christophe,
> 
>    Thanks for your answer.
>    > On 17:16 Wed 04 Mar , Benoit Monin wrote:
>    > >
>    [..SNIP..]
>    > > - if (gd->flags & GD_FLG_DEVINIT) {
>    > > - /* Send to the standard output */
>    > > - fputc(stdout, c);
>    > > - } else {
>    > > - /* Send directly to the handler */
>    > > - serial_putc(c);
>    > > + __putc(c);
>    > > +#ifdef CONFIG_PRINT_TIME
>    > > + if (c == '\n') {
>    > > + unsigned long ms;
>    > > + char tbuf[32];
>    > > + int tlen;
>    > > + int i;
>    > > +
>    > NACK
>    Could you explain, please ?
your patch can not work on all arch
>    Well, ms should be named us as it is a value in microsecond,
>    but i'm guessing you are nack'ing something else.
> 
>    > > + ms = ticks2usec(get_ticks());
>    > ticks2usec only exist on ppc
>    Right, ticks2usec is only implemented in lib_ppc and lib_sparc.
>    I naively believed what was written in include/common.h.
> 
>    All in all, do you think this patch is worth implementing ticks2usec in
>    the 10 remaining lib_${arch} or should I just drop the ball ? I also have
>    a "time" command (like the unix time command) that uses ticks2usec.
	IMHO if it's a general purpose you must have it working on all arch
	not only one or two ditto for the time command
> 
>    > > + tlen = sprintf(tbuf, "[%5lu.%06lu] ", (ms / 1000000UL), (ms %
>    1000000UL));
>    > wont work on arm
>    > please do_div
>    Does this mean you can't divide(or get the modulo of) a 32bits integer by
>    another 32bits integer ? Sorry if it's obvious but my arm is a bit rusty
>    ;)
	no but you will need to have the hard float support that we de not
	and please avoid modulo too for the same reason

	btw get_ticks return a u64 so I'll prefer we keep this precision until
	the div
> 
>    > > + for (i = 0; i < tlen; i++)
>    > > + __putc(tbuf[i]);
>    > > }
>    > > +#endif
>    > > }
>    > >
>    > > void puts(const char *s)
>    > > @@ -358,6 +376,11 @@ void puts(const char *s)
>    > > return;
>    > > #endif
>    > >
>    > > +#ifdef CONFIG_PRINT_TIME
>    > > + while (*s) {
>    > > + putc(*s++);
>    > please use fputs and serail_puts
>    I should have written a comment around this: To print the timestamp at
>    the beginning of every newline, the string must be scanned for '\n' and
>    this is already done by putc. This way, the following strings are
>    correctly
sure a comment will be better to understand what you want to do and why

Best Regards,
J.