From mboxrd@z Thu Jan  1 00:00:00 1970
From: Mike Frysinger <vapier@gentoo.org>
Date: Sat, 14 May 2011 07:19:52 -0400
Subject: [U-Boot] [PATCH 1/4] Add microsecond boot time measurement
In-Reply-To: <1305319923-9477-2-git-send-email-sjg@chromium.org>
References: <1305319923-9477-1-git-send-email-sjg@chromium.org>
	<1305319923-9477-2-git-send-email-sjg@chromium.org>
Message-ID: <201105140719.53245.vapier@gentoo.org>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On Friday, May 13, 2011 16:52:00 Simon Glass wrote:
> --- /dev/null
> +++ b/common/bootstage.c
> +/*
> + * Copyright (c) 2011 The Chromium OS Authors.

i'm not sure this is valid unless "Chromium OS Authors" is a legal entity

> +uint32_t bootstage_mark(enum bootstage_id id, const char *name)
> +{
> +	struct bootstage_record *rec = &record[id];
> +
> +	/* Only record the first event for each */
> +	if (!rec->name) {
> +		rec->time_us = (uint32_t)timer_get_us();

cast is pointless when time_us is of uint32_t type

> --- /dev/null
> +++ b/include/bootstage.h
> +enum bootstage_id {
> +	BOOTSTAGE_AWAKE,
> +	BOOTSTAGE_START_UBOOT,
> +	BOOTSTAGE_USB_START,
> +	BOOTSTAGE_ETH_START,
> +	BOOTSTAGE_BOOTP_START,
> +	BOOTSTAGE_BOOTP_STOP,
> +	BOOTSTAGE_KERNELREAD_START,
> +	BOOTSTAGE_KERNELREAD_STOP,
> +	BOOTSTAGE_BOOTM_START,
> +	BOOTSTAGE_BOOTM_HANDOFF,
> +
> +	/* a few spare for the user, from here */
> +	BOOTSTAGE_USER,
> +
> +	/*
> +	 * Total number of entries - increase this at the cost of some BSS
> +	 * and ATAG space.
> +	 */
> +	BOOTSTAGE_COUNT = 10
> +};

mmm BOOTSTAGE_COUNT supposed to be the number of enum's ?  hardcoding the 
number doesnt seem like a good idea ... how about just placing it at the right 
location in the list and let gcc calc the right value ?

> +static inline uint32_t bootstage_mark(enum bootstage_id id, const char
> *name) {}

missing a return value

> --- a/include/common.h
> +++ b/include/common.h
> +#include <bootstage.h>

in general, i'd rather see individual files opt in to the headers they need 
rather than having one giant header that includes everything else ...
-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/20110514/062d62c5/attachment.pgp