[U-Boot] [PATCH v4 2/2] NS16550: buffer reads

[Simon Glass]

Hi Albert,

On Sun, Oct 16, 2011 at 5:57 AM, Albert ARIBAUD
<albert.u.boot at aribaud.net> wrote:
> Hi Simon,
>
> Le 16/10/2011 07:14, Simon Glass a écrit :
>>
>> From: Scott Wood<scottwood at freescale.com>
>>
>> From: Scott Wood<scottwood at freescale.com>
>>
>> This improves the performance of U-Boot when accepting rapid input,
>> such as pasting a sequence of commands.
>>
>> Without this patch, on P4080DS I see a maximum of around 5 lines can
>> be pasted.  With this patch, it handles around 70 lines before lossage,
>> long enough for most things you'd paste.
>>
>> With it enabled, on ppc it's an extra 396 bytes of image size, and 1056
>> bytes of BSS.
>>
>> ARM note from Simon Glass<sjg at chromium.org>  - ARM code size goes from
>> 212 to 484 bytes (extra 272 bytes), BSS to 1056 bytes.
>>
>> Signed-off-by: Scott Wood<scottwood at freescale.com>
>> ---
>> Changes in v2:
>> - Fix checkpatch warnings, the other one was already there
>>
>> Changes in v3:
>> - Use CONFIG_NS16550_BUFFER_READS to set the buffer size also
>>
>> Changes in v4:
>> - Change config option to CONFIG_NS16550_RBUF_SIZE
>> - Add additional checkpatch-cleanup patch
>
> Hmm... What about the conversation around V2 of the patch re: using XOFF/XON
> to control input flow? IIUC, even this V4 patch would not help much if there
> is a command within the pasted code that sends a lot of output, right?

Well it will help so long as there isn't much more input. Perhaps the
way to think of it is that every character you output that isn't just
an echo, puts you one more character behind the input. This adds one
character to the buffer in this patch. When you have fallen (say) 256
characters behind the input, then you will start losing characters
even with this patch. However it is rare to paste in commands which
send lots of output - and 3 lines of full output is pretty uncommon in
my experience.

I feel that relying on the first level UART FIFO as the only buffering
is brave, and cannot be justified by lack of interrupts alone. There
seems to me to be no fundamental reason why U-Boot should not buffer
its input for when it is latest ready to process it. Granted this
should perhaps be higher up the software stack, but until someone
tidies up serial a little I suspect that would be a pain to implement.
With a more generic serial layer (a device pointer and a ->priv
pointer across all drivers for example) we could attach this buffering
as a separate file to be enabled / disabled by a Makefile.

To address your question, I think we did discuss it. Wolfgang felt
that it should be as simple as sending XOFF when receiving the end of
a line and XON when waiting for characters on the new line. It
certainly sounds plausible.

I have other reasons for favouring this patch. On the main hardware
platform I currently use, SPI and the console UART are multiplexed, so
before using SPI (e.g. saveenv, reading the environment) we must read
in any pending characters to avoid corruption. After re-enabling the
UART later we must clear out any junk that has arrived. This patch
provides a way around that which XON/XOFF does not (since when you
send XOFF you may already have input characters outstanding - junk
will later be added and you can't tell the difference).

I am unsure whether to bring this up or not - clearly this platform is
not ideal, but it is in very common use and we need to support it with
U-Boot.

Regards,
Simon

>
> Amicalement,
> --
> Albert.
>