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

[Wolfgang Denk]

Dear Simon,

In message <1318742050-2201-2-git-send-email-sjg at chromium.org> you wrote:
> 
> 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.

Let me try to summarize my thinking:

- It is a fundamental design decision that U-Boot is single tasking.
  This implies that while a command is running, no other things get
  done in parallel. This includes communication over the serial line,
  USB, Ethernet, ...

- That means that by design U-Boot does not support multi-line input:
  as soon as you hit "enter" U-Boot will start executing your command,
  and will only become ready for new input when it has completed the
  execution of the command(s), i. e. after printing the next prompt.

- For this suported mode of operation your patch is not needed. It
  just adds code size and complexity.

- Your description "rapid input" is actually wrong.  The speed of
  input over the serial line is limited by the baud rate settings,
  and even if you transfer at maximum speed all supported systems
  are fast enough to receive the data without loss.

- The use case you are trying to support is actually multi-line
  input, so you should describe it as such.  I agree that this would
  be an interesting feature for some use cases, but if we go on and
  implement it, we should (1) document it properly and (2) implement
  it in a way that works reliably.

- However, your implementation does not result in reliable multi-line
  input.  It works only in a few specific use cases, and will fail
  silently for others.  I don't see a way ho you would announce this
  new feature.  The numbers you mention in the commit message ("it
  handles around 70 lines before lossage") apply for this specific
  board only, and for your use case only (pasting of short lines that
  produce no output).

  So essentially you are saying: hey, we now have multi-line input,
  but it works only a bit.  It will fail sooner or later, without
  error messages, and I cannot even tell you what the limits for your
  system are.  And it depends on which input you send.

  This does not exactly sound promising.


On the other hand, we also have the rule that things that are useful
to some and that don't hurt others should be allowed to go in.

What makes me hesitate are two things:

- The patch promises a feature (multi-line input), but fails to
  provide a reliably working implementation.

- As it turns out, the patch increases code size even for boards that
  do not activate this feature.


I have to admit that I'm at a loss with a decision here.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"We don't have to protect the environment -- the Second Coming is  at
hand."                                                   - James Watt