All of lore.kernel.org
 help / color / mirror / Atom feed
From: Graeme Russ <graeme.russ@gmail.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH V4] console: Implement pre-console buffer
Date: Fri, 30 Sep 2011 09:15:55 +1000	[thread overview]
Message-ID: <CALButC+-6an-4ieoLE5VXnhiLab_sHSbzRko57+hkOdiOwHkrg@mail.gmail.com> (raw)
In-Reply-To: <CAC3GErFbLYjxTO-wcWZucDH++Sgj0bPYR_N7ULJ2sY=q-PNGmQ@mail.gmail.com>

Hi Vadim,

On Wed, Sep 28, 2011 at 12:55 AM, Vadim Bendebury <vbendeb@chromium.org> wrote:
> On Tue, Sep 27, 2011 at 4:22 AM, Graeme Russ <graeme.russ@gmail.com> wrote:
>> Hi Vadim,
>>
>> On 27/09/11 08:50, Vadim Bendebury wrote:
>>> On Wed, Aug 31, 2011 at 5:58 AM, Graeme Russ <graeme.russ@gmail.com> wrote:

[snip]

>> Typically, the pre-console buffer would exist in the CPU cache (or similar
>> non-(S)DRAM location)
>>
> hi Graeme,
>
> Actually, there are many cases when u-boot starts running with memory
> fully initialized - ARM platforms is one case and coreboot/u-boot
> combination on x86 is another, but in general, yes, this buffer could
> be mapped to the internal CPU memory nailed to a fixed address.

And we have to satisfy the 'absolute majority', not the 'many' or the
'simply majority'. And I'm not sure it's always true that ARM platforms
have fully initialised SDRAM when U-Boot starts

>>> - all console output needs to be saved, not just until the moment when
>>> the console hardware is initialized.
>>
>> That could be a _huge_ amount of info and highly variable. Remember, the
>> available space for a pre-console buffer could be tiny. If this is needed,
>> then maybe look at forking stdio instead (one branch to console, one branch
>> to you console buffer)
>>
>
> Sure, if the room in the preallocated buffer is not enough, only the
> most recent data fitting in the space would be kept.
>
> I don't quite understand what you mean by "forking stdio". I was

Search for CONFIG_CONSOLE_MUX - There appears to be support for sending
stdout to multiple output devices:

static void console_putc(int file, const char c)
{
	int i;
	struct stdio_dev *dev;

	for (i = 0; i < cd_count[file]; i++) {
		dev = console_devices[file][i];
		if (dev->putc != NULL)
		dev->putc(c);
	}
}


I don't know have to register additional devices though

> thinking about introducing a separate driver for this memory stored
> console output, but sjg@ explained that while running from ROM u-boot
> supports only one console interface, so there is no way to have
> console stream sent to more than one driver before relocation.

Yes, while running from ROM your options are very limited, but if you have
a console buffer big enough to get you into RAM you can do lot more

>>> I could work on top of this patch and send another one once this one
>>> has been accepted. May I suggest an improvement though:
>>>
>>> is it really necessary to store the index in the global data
>>> structure. This requires editing all these .h files adding another
>>> unsighty  conditionally compiled field. Why not to store the index as
>>> the first word in the buffer allocated for this temp storage?
>>
>> I like this - but instead:
>>
>> struct pre_con_buff {
>>        int idx;
>>        char *buffer[CONFIG_PRE_CON_BUF_SZ];
>> }
>>
>> struct pre_con_buff *pre_con_buffer;
>>
>> pre_con_buffer = (struct pre_con_buff *)CONFIG_PRE_CON_BUF_ADDR;
>>
>
> yes, this is exactly what I meant,
>

Thinking more about this, I think I prefer the current patch for two
reasons:

 1) gd is guaranteed to be cleared - The memory holding the buffer is not
    so you would need to initialise it somehow - That could mean splitting
    the init for each arch
 2) pre_con_buffer is larger than CONFIG_PRE_CON_BUF_SZ. This will need to
    be taken into consideration if the buffer is being crammed into a very
    tightly crafted memory map - Forgetting to take this into account is
    going to cause lots of pain. Now you could do:

	struct pre_con_buff {
		u16 idx;
		char *buffer[CONFIG_PRE_CON_BUF_SZ - 2];
	}

    but the buffer size should really be a power two (so the compiler
    optimises the divides into shifts) - So now we have to define
    CONFIG_PRE_CON_BUF_SZ as say 258. It's starting to get messy


Regards,

Graeme

  reply	other threads:[~2011-09-29 23:15 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-08-29 12:14 [U-Boot] [PATCH 0/2] console: Squelch and pre-console buffer Graeme Russ
2011-08-29 12:14 ` [U-Boot] [PATCH 1/2] console: Squelch pre-console output in console functions Graeme Russ
2011-08-30 17:16   ` Mike Frysinger
2011-08-30 19:01     ` Wolfgang Denk
2011-09-21 23:21   ` Simon Glass
2011-10-01 19:54   ` Wolfgang Denk
2011-08-29 12:14 ` [U-Boot] [PATCH 2/2] console: Implement pre-console buffer Graeme Russ
2011-08-30 12:49   ` [U-Boot] [PATCH V2] " Graeme Russ
2011-08-30 17:19     ` Mike Frysinger
2011-08-30 19:45     ` Simon Glass
2011-08-30 19:52       ` Wolfgang Denk
2011-08-30 19:58         ` Mike Frysinger
2011-08-30 20:08           ` Wolfgang Denk
2011-08-30 20:18             ` Simon Glass
2011-08-30 20:57               ` Wolfgang Denk
2011-08-30 21:02                 ` Simon Glass
2011-08-30 23:00                   ` Graeme Russ
2011-08-30 23:39                     ` Graeme Russ
2011-08-31  2:46                       ` Mike Frysinger
2011-08-30 20:07         ` Simon Glass
2011-08-31 12:35     ` [U-Boot] (no subject) Graeme Russ
2011-08-31 12:38       ` Graeme Russ
2011-08-31 12:58       ` [U-Boot] [PATCH V4] console: Implement pre-console buffer Graeme Russ
2011-08-31 15:09         ` Mike Frysinger
2011-08-31 21:15           ` Graeme Russ
2011-08-31 21:33             ` Mike Frysinger
2011-08-31 21:59               ` Graeme Russ
2011-08-31 22:44                 ` Mike Frysinger
2011-08-31 22:51                   ` Graeme Russ
2011-08-31 16:58         ` Simon Glass
2011-08-31 19:18           ` Wolfgang Denk
2011-08-31 20:05             ` Simon Glass
2011-08-31 20:22               ` Mike Frysinger
2011-08-31 20:26                 ` Simon Glass
2011-09-01 10:48         ` [U-Boot] [PATCH V5] " Graeme Russ
2011-09-01 10:52           ` Graeme Russ
2011-09-01 14:02             ` Mike Frysinger
2011-09-01 18:51               ` Simon Glass
2011-09-01 23:34                 ` Graeme Russ
2011-09-02  2:41                   ` Mike Frysinger
2011-09-02  2:58                   ` Simon Glass
2011-09-21 23:18             ` Simon Glass
2011-10-04  5:30               ` Simon Glass
2011-10-05 18:50             ` Wolfgang Denk
2011-09-26 22:50         ` [U-Boot] [PATCH V4] " Vadim Bendebury
2011-09-27 11:22           ` Graeme Russ
2011-09-27 14:55             ` Vadim Bendebury
2011-09-29 23:15               ` Graeme Russ [this message]
2011-09-29 23:39                 ` Vadim Bendebury
2011-09-29 23:47                   ` Graeme Russ

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CALButC+-6an-4ieoLE5VXnhiLab_sHSbzRko57+hkOdiOwHkrg@mail.gmail.com \
    --to=graeme.russ@gmail.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.