headers
Simon Glass | 1 Sep 2011 01:12

Re: [PATCH 0/7] Bootgraph.pl instrumentation support for UBoot

Hi Mike,

On Wed, Aug 31, 2011 at 3:47 PM, Mike Frysinger <vapier <at> gentoo.org> wrote:
> On Wednesday, August 31, 2011 18:20:54 Andrew Murray wrote:
>> This patchset introduces the CONFIG_BOOT_TRACE option which provides
>> support for boot time instrumentation.
>>
>> When enabled printf output is prefixed with timing information (similar to
>> the kernel's CONFIG_PRINTK_TIME option) and additional output is generated
>> which instruments functions and commands called (much like the kernel's
>> initcall_debug functionality).
>>
>> The kernel's bootgraph.pl script has been ported to render UBoots
>> instrumented ouptut into a pretty SVG graph. An example of this can be
>> found here: http://goo.gl/dX8aR - which shows the boot time of a Beagle
>> board.
>>
>> The patch currently provides support for instrumentation of UBoot commands
>> (e.g. U_BOOT_CMD) for all platforms but only when the HUSH shell is not in
>> use. Initialisation instrumentation is only limited to the
>> arch/arm/lib/board.c file at present but can very easily be extended to
>> other relevant files.
>
> i feel like we've had similar ideas tossed around semi-recently.  am i just
> misremembering ?
> -mike
>

Yes, for example:
(Continue reading)

Permalink | Reply | 10 comments |
headers
Andrew Murray | 1 Sep 2011 01:25
Picon
Favicon

Re: [PATCH 0/7] Bootgraph.pl instrumentation support for UBoot

On 1 September 2011 00:12, Simon Glass <sjg <at> chromium.org> wrote:
> Hi Mike,
>
> On Wed, Aug 31, 2011 at 3:47 PM, Mike Frysinger <vapier <at> gentoo.org> wrote:
>> On Wednesday, August 31, 2011 18:20:54 Andrew Murray wrote:
>>> This patchset introduces the CONFIG_BOOT_TRACE option which provides
>>> support for boot time instrumentation.
>>>
>>> When enabled printf output is prefixed with timing information (similar to
>>> the kernel's CONFIG_PRINTK_TIME option) and additional output is generated
>>> which instruments functions and commands called (much like the kernel's
>>> initcall_debug functionality).
>>>
>>> The kernel's bootgraph.pl script has been ported to render UBoots
>>> instrumented ouptut into a pretty SVG graph. An example of this can be
>>> found here: http://goo.gl/dX8aR - which shows the boot time of a Beagle
>>> board.
>>>
>>> The patch currently provides support for instrumentation of UBoot commands
>>> (e.g. U_BOOT_CMD) for all platforms but only when the HUSH shell is not in
>>> use. Initialisation instrumentation is only limited to the
>>> arch/arm/lib/board.c file at present but can very easily be extended to
>>> other relevant files.
>>
>> i feel like we've had similar ideas tossed around semi-recently.  am i just
>> misremembering ?
>> -mike
>>
>
> Yes, for example:
(Continue reading)

Permalink | Reply | 9 comments |
headers
Andrew Murray | 1 Sep 2011 01:30
Favicon

Re: [PATCH 3/7] Add timing information to printf's for use with bootgraph.pl

On 31 August 2011 23:47, Mike Frysinger <vapier <at> gentoo.org> wrote:
> On Wednesday, August 31, 2011 18:20:57 Andrew Murray wrote:
>>       va_list args;
>>       uint i;
>>       char printbuffer[CONFIG_SYS_PBSIZE];
>> +     char *buf = printbuffer;
>>
>>       va_start(args, fmt);
>>
>> +#if defined(CONFIG_BOOT_TRACE)
>> +     unsigned long long ticks = get_ticks();
>> +     int secs = ticks / get_tbclk();
>> +     int msec = ((ticks * 1000000) / get_tbclk()) - (secs * 1000000);
>> +
>> +     i += sprintf(buf, "[%5lu.%06lu] ", secs, msec);
>> +     buf += i;
>> +#endif
>> +
>>       /* For this to work, printbuffer must be larger than
>>        * anything we ever want to print.
>>        */
>> -     i = vsprintf(printbuffer, fmt, args);
>> +     i += vsprintf(buf, fmt, args);
>>       va_end(args);
>
> NAK for a few reasons:
>  - i dont see how this could possibly compile warning free
>  - you never initialize "i", only added to it
>  - you call va_start() inbetween variable decls
> -mike
(Continue reading)

Permalink | Reply | 0 comments |
headers
Graeme Russ | 1 Sep 2011 01:32
Picon

Re: [PATCH 0/7] Bootgraph.pl instrumentation support for UBoot

Hi Andrew,

On Thu, Sep 1, 2011 at 9:25 AM, Andrew Murray <amurray <at> mpc-data.co.uk> wrote:
> On 1 September 2011 00:12, Simon Glass <sjg <at> chromium.org> wrote:
>> Hi Mike,
>>
>> On Wed, Aug 31, 2011 at 3:47 PM, Mike Frysinger <vapier <at> gentoo.org> wrote:
>>> On Wednesday, August 31, 2011 18:20:54 Andrew Murray wrote:
>>>> This patchset introduces the CONFIG_BOOT_TRACE option which provides
>>>> support for boot time instrumentation.
>>>>

[snip]

>>>>
>>>> The patch currently provides support for instrumentation of UBoot commands
>>>> (e.g. U_BOOT_CMD) for all platforms but only when the HUSH shell is not in
>>>> use. Initialisation instrumentation is only limited to the
>>>> arch/arm/lib/board.c file at present but can very easily be extended to
>>>> other relevant files.
>>>
>>> i feel like we've had similar ideas tossed around semi-recently.  am i just
>>> misremembering ?

No, your memory is fine :)

>>> -mike
>>>
>>
>> Yes, for example:
(Continue reading)

Permalink | Reply | 8 comments |
headers
Graeme Russ | 1 Sep 2011 01:38
Picon

Re: [PATCH 3/7] Add timing information to printf's for use with bootgraph.pl

Hi Mike, Andrew,

On Thu, Sep 1, 2011 at 8:47 AM, Mike Frysinger <vapier <at> gentoo.org> wrote:
> On Wednesday, August 31, 2011 18:20:57 Andrew Murray wrote:
>>       va_list args;
>>       uint i;
>>       char printbuffer[CONFIG_SYS_PBSIZE];
>> +     char *buf = printbuffer;
>>
>>       va_start(args, fmt);
>>
>> +#if defined(CONFIG_BOOT_TRACE)
>> +     unsigned long long ticks = get_ticks();
>> +     int secs = ticks / get_tbclk();
>> +     int msec = ((ticks * 1000000) / get_tbclk()) - (secs * 1000000);
>> +
>> +     i += sprintf(buf, "[%5lu.%06lu] ", secs, msec);
>> +     buf += i;
>> +#endif
>> +
>>       /* For this to work, printbuffer must be larger than
>>        * anything we ever want to print.
>>        */
>> -     i = vsprintf(printbuffer, fmt, args);
>> +     i += vsprintf(buf, fmt, args);
>>       va_end(args);
>
> NAK for a few reasons:
>  - i dont see how this could possibly compile warning free
>  - you never initialize "i", only added to it
(Continue reading)

Permalink | Reply | 1 comment |
headers
Simon Glass | 1 Sep 2011 01:39

Re: [PATCH 0/7] Bootgraph.pl instrumentation support for UBoot

Hi Andrew,

On Wed, Aug 31, 2011 at 4:32 PM, Graeme Russ <graeme.russ <at> gmail.com> wrote:
> Hi Andrew,
>
> On Thu, Sep 1, 2011 at 9:25 AM, Andrew Murray <amurray <at> mpc-data.co.uk> wrote:
>> On 1 September 2011 00:12, Simon Glass <sjg <at> chromium.org> wrote:
>>> Hi Mike,
>>>
>>> On Wed, Aug 31, 2011 at 3:47 PM, Mike Frysinger <vapier <at> gentoo.org> wrote:
>>>> On Wednesday, August 31, 2011 18:20:54 Andrew Murray wrote:
>>>>> This patchset introduces the CONFIG_BOOT_TRACE option which provides
>>>>> support for boot time instrumentation.
>>>>>
>
> [snip]
>
>>>>>
>>>>> The patch currently provides support for instrumentation of UBoot commands
>>>>> (e.g. U_BOOT_CMD) for all platforms but only when the HUSH shell is not in
>>>>> use. Initialisation instrumentation is only limited to the
>>>>> arch/arm/lib/board.c file at present but can very easily be extended to
>>>>> other relevant files.
>>>>
>>>> i feel like we've had similar ideas tossed around semi-recently.  am i just
>>>> misremembering ?
>
> No, your memory is fine :)
>
>>>> -mike
(Continue reading)

Permalink | Reply | 7 comments |
headers
Andrew Murray | 1 Sep 2011 01:40
Favicon

Re: [PATCH 2/7] Add macros for recording init calls during UBoot execution

On 31 August 2011 23:50, Mike Frysinger <vapier <at> gentoo.org> wrote:
> On Wednesday, August 31, 2011 18:20:56 Andrew Murray wrote:
>> +#if defined(CONFIG_BOOT_TRACE)
>> +#define DO_INITCALL(x, ...) \
>> +     do { \
>> +             printf("calling  0x%pF\n", x); \
>> +             (x)(__VA_ARGS__); \
>> +             printf("initcall 0x%pF returned\n", x); \
>> +     } while (0)
>
> are there any void initcalls ?  or just ones where you ignore the value ?
> otherwise we can simply rename DO_INITCALL_RET() to DO_INITCALL().

I guess it depends what you define as an initcall. A lot of functions
called during startup (e.g. arch/arm/lib/board.c) return int and in
most cases that value is ignored - but there are occasions where void
is returned - e.g. env_relocate.

For simplicity you could limit the use cases for this macro to those
which just return int and take your proposed approach?

Andrew Murray
_______________________________________________
U-Boot mailing list
U-Boot <at> lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Permalink | Reply | 0 comments |
headers
Andrew Murray | 1 Sep 2011 01:42
Favicon

Re: [PATCH 3/7] Add timing information to printf's for use with bootgraph.pl

On 1 September 2011 00:38, Graeme Russ <graeme.russ <at> gmail.com> wrote:
> Hi Mike, Andrew,
>
> On Thu, Sep 1, 2011 at 8:47 AM, Mike Frysinger <vapier <at> gentoo.org> wrote:
>> On Wednesday, August 31, 2011 18:20:57 Andrew Murray wrote:
>>>       va_list args;
>>>       uint i;
>>>       char printbuffer[CONFIG_SYS_PBSIZE];
>>> +     char *buf = printbuffer;
>>>
>>>       va_start(args, fmt);
>>>
>>> +#if defined(CONFIG_BOOT_TRACE)
>>> +     unsigned long long ticks = get_ticks();
>>> +     int secs = ticks / get_tbclk();
>>> +     int msec = ((ticks * 1000000) / get_tbclk()) - (secs * 1000000);
>>> +
>>> +     i += sprintf(buf, "[%5lu.%06lu] ", secs, msec);
>>> +     buf += i;
>>> +#endif
>>> +
>>>       /* For this to work, printbuffer must be larger than
>>>        * anything we ever want to print.
>>>        */
>>> -     i = vsprintf(printbuffer, fmt, args);
>>> +     i += vsprintf(buf, fmt, args);
>>>       va_end(args);
>>
>> NAK for a few reasons:
>>  - i dont see how this could possibly compile warning free
(Continue reading)

Permalink | Reply | 0 comments |
headers
Andrew Murray | 1 Sep 2011 01:53
Favicon

Re: [PATCH 0/7] Bootgraph.pl instrumentation support for UBoot

On 1 September 2011 00:39, Simon Glass <sjg <at> chromium.org> wrote:
>>>
>>> Is there any cross over between my approach and what is
>>> planned/already been done?
>>>
>
> Don't worry - your contribution is very welcome!
>
> Yes I think there is cross-over, and perhaps the right approach is to
> try to merge them somehow. It is great to get graphs out of the code
> and it really helps with analysis. My interest was mainly in
> monitoring boot time in the field rather than in the lab. But we
> should have one framework for both.

[SNIP]

>
> I will assume that we have a microsecond timer, update my patch and
> resubmit so you can take a look and see what you think. Hopefully we
> can unify this, your patch and the boot_progress stuff.

Excellent! OK, well I will await the patch, read up on the original
intentions and we can go from there. I'll look forward to making UBoot
more boot time friendly. Good night.

Andrew Murray
Permalink | Reply | 6 comments |
headers
Andy Fleming | 1 Sep 2011 03:23
Picon

Re: [PATCH] s5p-mmc: Fix ambiguous setting of data transfer width

On Tue, Aug 30, 2011 at 5:55 AM, Chander Kashyap
<chander.kashyap <at> linaro.org> wrote:
> mmc data transfer width is set as following:
> WIDE8[5]:
> 0 = Depend on WIDE4
> 1 = 8-bit mode
> WIDE4[1]:
> 1 = 4-bit mode
> 0 = 1-bit mode
>
> In case of 4-bit mode reset 8-bit mode and
> in case of 1-bit mode reset 8-bit mode and 4-bit mode
>
> Signed-off-by: Chander Kashyap <chander.kashyap <at> linaro.org>
> ---
>  drivers/mmc/s5p_mmc.c |   10 +++++++---
>  1 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mmc/s5p_mmc.c b/drivers/mmc/s5p_mmc.c
> index 7786ecf..6e6ad08 100644
> --- a/drivers/mmc/s5p_mmc.c
> +++ b/drivers/mmc/s5p_mmc.c
>  <at>  <at>  -368,12 +368,16  <at>  <at>  static void mmc_set_ios(struct mmc *mmc)
>         * 1 = 4-bit mode
>         * 0 = 1-bit mode
>         */
> -       if (mmc->bus_width == 8)
> +       if (mmc->bus_width == 8) {
>                ctrl |= (1 << 5);
> -       else if (mmc->bus_width == 4)
(Continue reading)

Permalink | Reply | 3 comments |

Gmane