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

2011-10-25 Thread Scott Wood 
On 10/17/2011 07:53 AM, Wolfgang Denk wrote:
 Dear Simon,
 
 I wrote:
 
 - 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).
 
 Actually I believe that the restriction is even worse - I think that
 any commands that consume any significant amount of time will casue
 problems, too.  Can you please test what happens when you have, for
 example, a sleep 10 or a erase all in the first few lines of the
 pasted input ?

The reference to 70 lines was meant as an anecdote about how it made
pasting things significantly more usable for me.  I never advertised it
as a complete fix for serial loss (indeed, it even says that I still
experienced loss after 70 lines).  It's just a fairly simple change that
lessens the problem for me (and apparently others) to the point where
it's no longer a significant nuisance.

-Scott

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2011-10-23 Thread Wolfgang Denk 
Dear Graeme Russ,

In message 4ea34086.4030...@gmail.com you wrote:
 
 One problem I see with XON/XOFF is that if we don't send XOFF at the right
 time, we run the risk of entering a busy loop (any reasonable timeout delay
 for example) and loosing input. So in theory, we would need to send XOFF
 after every getc() ...

That's not true.  I am not aware of any significant delays that take
place while receiving characters that belong to a single line.  If we
had any of these, we would lose characters all the time - but we
don't.

It should be sufficient to send XOFF after receiving a newline
character.

 Maybe we need disable/enable flow control functions for when we know we
 will be entering a busy loop the consumes serial input (ymodem and kermit
 transfers and readline for example)

This should not be necessary. Actually the implementation should not
need to know about such special cases.

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: w...@denx.de
Compassion -- that's the one things no machine ever had.  Maybe it's
the one thing that keeps men ahead of them.
-- McCoy, The Ultimate Computer, stardate 4731.3
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2011-10-23 Thread Graeme Russ 
Hi Wolgang,

On Oct 23, 2011 7:20 PM, Wolfgang Denk w...@denx.de wrote:

 Dear Graeme Russ,

 In message 4ea34086.4030...@gmail.com you wrote:
 
  One problem I see with XON/XOFF is that if we don't send XOFF at the
right
  time, we run the risk of entering a busy loop (any reasonable timeout
delay
  for example) and loosing input. So in theory, we would need to send XOFF
  after every getc() ...

 That's not true.  I am not aware of any significant delays that take
 place while receiving characters that belong to a single line.  If we
 had any of these, we would lose characters all the time - but we
 don't.

 It should be sufficient to send XOFF after receiving a newline
 character.

And, ergo, we send an XON when entering the readline function

Hmm, should we move readline() into console.c

  Maybe we need disable/enable flow control functions for when we know we
  will be entering a busy loop the consumes serial input (ymodem and
kermit
  transfers and readline for example)

 This should not be necessary. Actually the implementation should not
 need to know about such special cases.

So how does kermit/ymodem send the XON after the user has entered the
receive command and we have sent the XOFF after the newline?

Regards,

Graeme
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2011-10-23 Thread Wolfgang Denk 
Dear Graeme Russ,

In message calbutcleg6c30en3n4ljpv1wojjfxwkekhvqymojy8+mgsz...@mail.gmail.com 
you wrote:

  It should be sufficient to send XOFF after receiving a newline
  character.
 
 And, ergo, we send an XON when entering the readline function

This is probably not sufficient, as some commands take direct input.
I think both getc() and tstc() should check the XON/XOFF state and
send a XON if XOFF was sent before.

 Hmm, should we move readline() into console.c

Makes sense.

  This should not be necessary. Actually the implementation should not
  need to know about such special cases.
 
 So how does kermit/ymodem send the XON after the user has entered the
 receive command and we have sent the XOFF after the newline?

Upon the first getc() that follows?

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: w...@denx.de
Marriage is the sole cause of divorce.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2011-10-23 Thread Wolfgang Denk 
Dear Simon Glass,

In message 1318742050-2201-2-git-send-email-...@chromium.org you wrote:
 From: Scott Wood scottw...@freescale.com
 
 From: Scott Wood scottw...@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 s...@chromium.org - ARM code size goes from
 212 to 484 bytes (extra 272 bytes), BSS to 1056 bytes.
 
 Signed-off-by: Scott Wood scottw...@freescale.com

I've made up my mind.  The reasons why you want to add such code are
well understood, but the unsolved and unsolvable issues with this
approach are so severe that I don't want to raise false hopes in
innocent users.

Sorry, but I reject this patch.

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: w...@denx.de
Nature is very un-American.  Nature never hurries.
- William George Jordan
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2011-10-23 Thread Graeme Russ 
Hi Wolfgang,

On Oct 24, 2011 4:15 AM, Wolfgang Denk w...@denx.de wrote:

 Dear Graeme Russ,

 In message 
calbutcleg6c30en3n4ljpv1wojjfxwkekhvqymojy8+mgsz...@mail.gmail.com you
wrote:
 

[snip]

   This should not be necessary. Actually the implementation should not
   need to know about such special cases.
 
  So how does kermit/ymodem send the XON after the user has entered the
  receive command and we have sent the XOFF after the newline?

 Upon the first getc() that follows?

And as there will be no corresponding newline, when do we send XOFF?

Regards,

Graeme
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2011-10-23 Thread Wolfgang Denk 
Dear Graeme Russ,

In message CALButCK0oVLTbYxbTF=y-vXL+3+=09vejpzunx1+-0syjqe...@mail.gmail.com 
you wrote:

   So how does kermit/ymodem send the XON after the user has entered the
   receive command and we have sent the XOFF after the newline?
 
  Upon the first getc() that follows?
 
 And as there will be no corresponding newline, when do we send XOFF?

Never?  Note that kermit / ymodem / S-record download etc. all don't
have any issues with sendign characters back-to-back at line speed.

Problems happen only with multi-line input, so it is perfectly fine
to handle just that - at the root cause, i. e. when input turns into
multi-line input.

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: w...@denx.de
Include the success of others in your dreams for your own success.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2011-10-22 Thread Albert ARIBAUD 
Le 17/10/2011 22:58, Simon Glass a écrit :
 Hi Wolfgang,

 On Mon, Oct 17, 2011 at 1:33 PM, Wolfgang Denkw...@denx.de  wrote:
 Dear Simon Glass,

 In 
 messagecapnjgz1412ezjh3f4ww16k4z55kjdggwj7+vmyk_ot-mzvd...@mail.gmail.com  
 you wrote:

 Can you please tell me which ELDK version this is using? I see that
 the 405 board seems to need ppc_4xx which suggests 4.2 rather than 5,
 since in 5 the compiler is called powerpc-

 Right, I was testing this with ELDK 4.2.

 I am struggling to repeat this and don't even get the same numbers...

Could be due to two things:

1) both of you do not work from the same exact tree commit, or

2) both of you work in a different base directory

 For your AR405 board, you saw:
 - 246058   12972   14636  273666   42d02 /work/wd/tmp-ppc/u-boot
 + 246062   12972   14636  273670   42d06 /work/wd/tmp-ppc/u-boot

 For me:

   24594212964   14632  273538   42c82 ppc/u-boot

I suggest Wolfgang and you compare your trees to eliminate risk 1, and 
make sure you compare .bin files to reduce risk 2.

Amicalement,
-- 
Albert.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2011-10-22 Thread Albert ARIBAUD 
Le 17/10/2011 18:40, Simon Glass a écrit :

   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.

 That all sounds reasonable.

 I think it is good enough, but not perfect. Due to the fundamental
 design decision you mention at the top, we cannot squirrel away serial
 input in the background. The best we can do is squirrel it away in the
 foreground, as we output new serial data (I suppose we could squirrel
 it away in net loops and other places but I don't want to go there!).
 This turns out to be mostly good enough, because it is rare for people
 to want to paste 'sleep 10' and the like into their terminal (your
 other message).

I think you're not seeing the point that while your patch is good enough 
for 'making multiline input less impractical in some cases', it is not 
good enough for 'making multiline input reliable': it adds marginal 
benefits and we have clear evidence that it will never guarantee a 
correct result however you tweak it, because the only way it could would 
be by multi-tasking in some way; otherwise, there can always be some 
U-Boot activity in between input readings that will be long enough to 
cause character loss.

 I would like to spit out a warning when serial input is lost - as I
 mentioned that could be combined with a serial overhaul at some point.

I don't think the warning would make the functionality better.

 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.

 I *think* it does the best possible within the fundamental design
 decision constraint. If there is more it can do, please let me have
 your ideas. I don't believe buffering conflicts with the constraint -
 they are separate things. But yes in systems with interrupts normally
 the input buffer is filled in the background and drained in the
 foreground.

There *is* a much better solution within the fundamental single tasking 
design constraint, and it is the one that was invented precisely for 
this: flow control. Granted, hardware flow control may be impractical, 
and that's why software flow control was conceived as early as 1963 and 
is still widely used today and supported by any serial software or 
driver worth its salt.

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

 Yes, I will take a look at this problem.



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

 Well it's not easy being a maintainer :-)

Well, FWIW, were I the serial code maintainer, I'd NAK this and ask for 
a XON/XOFF implementation.

 In any case this patch is
 not the end of the story as serial needs some work - another objection
 you didn't mention above is that this function lives in only one
 driver. Is that a good thing (hide it away) or a bad thing (all
 drivers should support it and the implementation should move up a
 level)?

Software flow control, whatever way it is implemented, is pure SW and 
hardware independent, so it should be a generic thing. If XON/XOFF input 
flow control gets implemented (and I believe it should), then it should 
be done above hardware serial drivers.

 Regards,
 Simon

Amicalement,
-- 
Albert.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2011-10-22 Thread Graeme Russ 
Hi Simon, Albert,

On 22/10/11 19:44, Albert ARIBAUD wrote:
 Le 17/10/2011 18:40, Simon Glass a écrit :

[snip]

 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.

I fully agree with this - We should strive to eliminate bugs, not cover
them with more dirt :)

 I *think* it does the best possible within the fundamental design
 decision constraint. If there is more it can do, please let me have
 your ideas. I don't believe buffering conflicts with the constraint -
 they are separate things. But yes in systems with interrupts normally
 the input buffer is filled in the background and drained in the
 foreground.
 
 There *is* a much better solution within the fundamental single tasking 
 design constraint, and it is the one that was invented precisely for 
 this: flow control. Granted, hardware flow control may be impractical, 
 and that's why software flow control was conceived as early as 1963 and 
 is still widely used today and supported by any serial software or 
 driver worth its salt.

Agree - flow control is the way to go

 Well, FWIW, were I the serial code maintainer, I'd NAK this and ask for 
 a XON/XOFF implementation.
 
 In any case this patch is
 not the end of the story as serial needs some work - another objection
 you didn't mention above is that this function lives in only one
 driver. Is that a good thing (hide it away) or a bad thing (all
 drivers should support it and the implementation should move up a
 level)?
 
 Software flow control, whatever way it is implemented, is pure SW and 
 hardware independent, so it should be a generic thing. If XON/XOFF input 
 flow control gets implemented (and I believe it should), then it should 
 be done above hardware serial drivers.

One problem I see with XON/XOFF is that if we don't send XOFF at the right
time, we run the risk of entering a busy loop (any reasonable timeout delay
for example) and loosing input. So in theory, we would need to send XOFF
after every getc() and XON before each getc() and then need to delay a bit
to wait for the character (unless tstc returns true in which case we can
just grab the char in the buffer) which seems a bit excessive.

Maybe we need disable/enable flow control functions for when we know we
will be entering a busy loop the consumes serial input (ymodem and kermit
transfers and readline for example)

Regards,

Graeme
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2011-10-17 Thread Wolfgang Denk 
Dear Simon Glass,

In message 1318742050-2201-2-git-send-email-...@chromium.org you wrote:
...
 With it enabled, on ppc it's an extra 396 bytes of image size, and 1056
 bytes of BSS.
 
 ARM note from Simon Glass s...@chromium.org - ARM code size goes from
 212 to 484 bytes (extra 272 bytes), BSS to 1056 bytes.

One problem I see is that the code size on some boards grows even if
the new option is not enabled.  Here a compare before and after
applying your patches (without any changes to the respective board
configurations):

 Configuring for AR405 board...
textdata bss dec hex filename
- 246058   12972   14636  273666   42d02 /work/wd/tmp-ppc/u-boot
+ 246062   12972   14636  273670   42d06 /work/wd/tmp-ppc/u-boot
 Configuring for CANBT board...
textdata bss dec hex filename
- 12071086043876  133190   20846 /work/wd/tmp-ppc/u-boot
+ 12071486043876  133194   2084a /work/wd/tmp-ppc/u-boot
 Configuring for pcs440ep board...
textdata bss dec hex filename
- 296191   19636  345088  660915   a15b3 /work/wd/tmp-ppc/u-boot
+ 296195   19636  345088  660919   a15b7 /work/wd/tmp-ppc/u-boot
...

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: w...@denx.de
Out of register space (ugh)
- vi
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2011-10-17 Thread Wolfgang Denk 
Dear Simon,

In message 1318742050-2201-2-git-send-email-...@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: w...@denx.de
We don't have to protect the environment -- the Second Coming is  at
hand.   - James Watt
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2011-10-17 Thread Wolfgang Denk 
Dear Simon,

I wrote:

 - 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).

Actually I believe that the restriction is even worse - I think that
any commands that consume any significant amount of time will casue
problems, too.  Can you please test what happens when you have, for
example, a sleep 10 or a erase all in the first few lines of the
pasted input ?

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: w...@denx.de
What can it profit a man to gain the whole world and to come  to  his
property with a gastric ulcer, a blown prostate, and bifocals?
 -- John Steinbeck, _Cannery Row_
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2011-10-17 Thread Simon Glass 
Hi Wolfgang,

On Mon, Oct 17, 2011 at 4:08 AM, Wolfgang Denk w...@denx.de wrote:
 Dear Simon Glass,

 In message 1318742050-2201-2-git-send-email-...@chromium.org you wrote:
 ...
 With it enabled, on ppc it's an extra 396 bytes of image size, and 1056
 bytes of BSS.

 ARM note from Simon Glass s...@chromium.org - ARM code size goes from
 212 to 484 bytes (extra 272 bytes), BSS to 1056 bytes.

 One problem I see is that the code size on some boards grows even if
 the new option is not enabled.  Here a compare before and after
 applying your patches (without any changes to the respective board
 configurations):

  Configuring for AR405 board...
    text    data     bss     dec     hex filename
 - 246058   12972   14636  273666   42d02 /work/wd/tmp-ppc/u-boot
 + 246062   12972   14636  273670   42d06 /work/wd/tmp-ppc/u-boot
  Configuring for CANBT board...
    text    data     bss     dec     hex filename
 - 120710    8604    3876  133190   20846 /work/wd/tmp-ppc/u-boot
 + 120714    8604    3876  133194   2084a /work/wd/tmp-ppc/u-boot
  Configuring for pcs440ep board...
    text    data     bss     dec     hex filename
 - 296191   19636  345088  660915   a15b3 /work/wd/tmp-ppc/u-boot
 + 296195   19636  345088  660919   a15b7 /work/wd/tmp-ppc/u-boot
 ...


This doesn't happen for me on ARM - the compiler just inlines the new
static raw functions into the public functions. I will see if I can
figure out what is happening on PPC. It is 4 bytes, or one
instruction, right?

The fix might be to duplicate more code, or perhaps add an inline
keyword to the two functions.

Regards,
Simon

 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: w...@denx.de
 Out of register space (ugh)
 - vi

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2011-10-17 Thread Simon Glass 
Hi Wolfgang,

On Mon, Oct 17, 2011 at 5:18 AM, Wolfgang Denk w...@denx.de wrote:
 Dear Simon,

 In message 1318742050-2201-2-git-send-email-...@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.

I can certainly improve the documentation and commit message.


 - 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.

That all sounds reasonable.

I think it is good enough, but not perfect. Due to the fundamental
design decision you mention at the top, we cannot squirrel away serial
input in the background. The best we can do is squirrel it away in the
foreground, as we output new serial data (I suppose we could squirrel
it away in net loops and other places but I don't want to go there!).
This turns out to be mostly good enough, because it is rare for people
to want to paste 'sleep 10' and the like into their terminal (your
other message).

I would like to spit out a warning when serial input is lost - as I
mentioned that could be combined with a serial overhaul at some point.



 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.

I *think* it does the best possible within the fundamental design
decision constraint. If there is more it can do, please let me have
your ideas. I don't believe buffering conflicts with the constraint -
they are separate things. But yes in systems with interrupts normally
the input buffer is filled in the background and drained in the
foreground.


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

Yes, I will take a look at this problem.



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

Well it's not easy being a maintainer :-) In any case this patch is
not the end of the story as serial needs some work - another objection
you didn't mention above is that this function lives in only one
driver. Is that a good thing (hide it away) or a bad thing (all
drivers should support it and the implementation should move up a
level)?

Regards,
Simon


 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: w...@denx.de
 We don't have to protect the environment -- the Second Coming is  at
 hand.                                                   - James Watt

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2011-10-17 Thread Simon Glass 
On Mon, Oct 17, 2011 at 4:08 AM, Wolfgang Denk w...@denx.de wrote:
 Dear Simon Glass,

 In message 1318742050-2201-2-git-send-email-...@chromium.org you wrote:
 ...
 With it enabled, on ppc it's an extra 396 bytes of image size, and 1056
 bytes of BSS.

 ARM note from Simon Glass s...@chromium.org - ARM code size goes from
 212 to 484 bytes (extra 272 bytes), BSS to 1056 bytes.

 One problem I see is that the code size on some boards grows even if
 the new option is not enabled.  Here a compare before and after
 applying your patches (without any changes to the respective board
 configurations):

  Configuring for AR405 board...
    text    data     bss     dec     hex filename
 - 246058   12972   14636  273666   42d02 /work/wd/tmp-ppc/u-boot
 + 246062   12972   14636  273670   42d06 /work/wd/tmp-ppc/u-boot
  Configuring for CANBT board...
    text    data     bss     dec     hex filename
 - 120710    8604    3876  133190   20846 /work/wd/tmp-ppc/u-boot
 + 120714    8604    3876  133194   2084a /work/wd/tmp-ppc/u-boot
  Configuring for pcs440ep board...
    text    data     bss     dec     hex filename
 - 296191   19636  345088  660915   a15b3 /work/wd/tmp-ppc/u-boot
 + 296195   19636  345088  660919   a15b7 /work/wd/tmp-ppc/u-boot
 ...

Hi Wolfgang,

Can you please tell me which ELDK version this is using? I see that
the 405 board seems to need ppc_4xx which suggests 4.2 rather than 5,
since in 5 the compiler is called powerpc-

But I might be confused.

Regards,
Simon


 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: w...@denx.de
 Out of register space (ugh)
 - vi

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2011-10-17 Thread Wolfgang Denk 
Dear Simon Glass,

In message capnjgz1412ezjh3f4ww16k4z55kjdggwj7+vmyk_ot-mzvd...@mail.gmail.com 
you wrote:

 Can you please tell me which ELDK version this is using? I see that
 the 405 board seems to need ppc_4xx which suggests 4.2 rather than 5,
 since in 5 the compiler is called powerpc-

Right, I was testing this with ELDK 4.2.

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: w...@denx.de
You can observe a lot just by watchin'.  - Yogi Berra
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2011-10-17 Thread Simon Glass 
Hi Wolfgang,

On Mon, Oct 17, 2011 at 1:33 PM, Wolfgang Denk w...@denx.de wrote:
 Dear Simon Glass,

 In message 
 capnjgz1412ezjh3f4ww16k4z55kjdggwj7+vmyk_ot-mzvd...@mail.gmail.com you 
 wrote:

 Can you please tell me which ELDK version this is using? I see that
 the 405 board seems to need ppc_4xx which suggests 4.2 rather than 5,
 since in 5 the compiler is called powerpc-

 Right, I was testing this with ELDK 4.2.

I am struggling to repeat this and don't even get the same numbers...

For your AR405 board, you saw:
 - 246058   12972   14636  273666   42d02 /work/wd/tmp-ppc/u-boot
 + 246062   12972   14636  273670   42d06 /work/wd/tmp-ppc/u-boot

For me:

ppc_4xx-gcc -v
Reading specs from
/opt/eldk-4.2.4xx/usr/bin/../lib/gcc/powerpc-linux/4.2.2/specs
Target: powerpc-linux
Configured with:
/opt/eldk/build/ppc-2008-04-01/work/usr/src/denx/BUILD/crosstool-0.43/build/gcc-4.2.2-glibc-20070515T2025-eldk/powerpc-linux/gcc-4.2.2/configure
--target=powerpc-linux --host=i686-host_pc-linux-gnu
--prefix=/var/tmp/eldk.UZpAG7/usr/crosstool/gcc-4.2.2-glibc-20070515T2025-eldk/powerpc-linux
--disable-hosted-libstdcxx
--with-headers=/var/tmp/eldk.UZpAG7/usr/crosstool/gcc-4.2.2-glibc-20070515T2025-eldk/powerpc-linux/powerpc-linux/include
--with-local-prefix=/var/tmp/eldk.UZpAG7/usr/crosstool/gcc-4.2.2-glibc-20070515T2025-eldk/powerpc-linux/powerpc-linux
--disable-nls --enable-threads=posix --enable-symvers=gnu
--enable-__cxa_atexit --enable-languages=c,c++,java --enable-shared
--enable-c99 --enable-long-long --without-x
Thread model: posix
gcc version 4.2.2

ARCH=powerpc make O=ppc AR405_config
ARCH=powerpc make O=ppc

without feature (checkout of upstream/master d8fffa05):
$ ppc_4xx-objdump -h ppc/drivers/serial/ns16550.o
...
  7 .text.NS16550_tstc 0020      0bf4  2**2
  CONTENTS, ALLOC, LOAD, READONLY, CODE
  8 .text.NS16550_putc 002c      0c14  2**2
  CONTENTS, ALLOC, LOAD, READONLY, CODE
  9 .text.NS16550_getc 0038      0c40  2**2
  CONTENTS, ALLOC, LOAD, READONLY, CODE
 10 .text.NS16550_init 007c      0c78  2**2
  CONTENTS, ALLOC, LOAD, READONLY, CODE
 11 .text.NS16550_reinit 0080      0cf4  2**2
  CONTENTS, ALLOC, LOAD, READONLY, CODE

   textdata bss dec hex filename
 245942   12964   14632  273538   42c82 ppc/u-boot


with feature but not enabled (with Scott's two patches):
  7 .text.NS16550_putc 002c      0fec  2**2
  CONTENTS, ALLOC, LOAD, READONLY, CODE
  8 .text.NS16550_getc 0038      1018  2**2
  CONTENTS, ALLOC, LOAD, READONLY, CODE
  9 .text.NS16550_tstc 0020      1050  2**2
  CONTENTS, ALLOC, LOAD, READONLY, CODE
 10 .text.NS16550_init 007c      1070  2**2
  CONTENTS, ALLOC, LOAD, READONLY, CODE
 11 .text.NS16550_reinit 0080      10ec  2**2
  CONTENTS, ALLOC, LOAD, READONLY, CODE

size ppc/u-boot
   textdata bss dec hex filename
 245942   12964   14632  273538   42c82 ppc/u-boot

Do you have any suggestions please?

Regards,
Simon


 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: w...@denx.de
 You can observe a lot just by watchin'.                  - Yogi Berra

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2011-10-16 Thread Albert ARIBAUD 
Hi Simon,

Le 16/10/2011 07:14, Simon Glass a écrit :
 From: Scott Woodscottw...@freescale.com

 From: Scott Woodscottw...@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 Glasss...@chromium.org  - ARM code size goes from
 212 to 484 bytes (extra 272 bytes), BSS to 1056 bytes.

 Signed-off-by: Scott Woodscottw...@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?

Amicalement,
-- 
Albert.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2011-10-16 Thread Simon Glass 
Hi Albert,

On Sun, Oct 16, 2011 at 5:57 AM, Albert ARIBAUD
albert.u.b...@aribaud.net wrote:
 Hi Simon,

 Le 16/10/2011 07:14, Simon Glass a écrit :

 From: Scott Woodscottw...@freescale.com

 From: Scott Woodscottw...@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 Glasss...@chromium.org  - ARM code size goes from
 212 to 484 bytes (extra 272 bytes), BSS to 1056 bytes.

 Signed-off-by: Scott Woodscottw...@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.

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2011-10-16 Thread Wolfgang Denk 
Dear Simon Glass,

In message CAPnjgZ2Q3XtGK=5hhf0khqo4xqda6cah3qfkfmgcwznv_y0...@mail.gmail.com 
you wrote:
 
  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

Don't you see that this as long as there isn't much more input is
the fatal flaw of your patch?  You are not fixing the problem, you are
just extending the point wher eit hits to some extend - it may work
for a few more situations, but it's still guaranteed to fail for
others.

 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.

Then please let's take this approach instead, if you really need to
support such a mode of operation.

 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).

That's an unrelated hardware issue that needs to be addressed
independently.

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: w...@denx.de
Most legends have their basis in facts.
-- Kirk, And The Children Shall Lead, stardate 5029.5
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2011-10-16 Thread Simon Glass 
Hi Wolfgang,

On Sun, Oct 16, 2011 at 1:13 PM, Wolfgang Denk w...@denx.de wrote:
 Dear Simon Glass,

 In message 
 CAPnjgZ2Q3XtGK=5hhf0khqo4xqda6cah3qfkfmgcwznv_y0...@mail.gmail.com you 
 wrote:

  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

 Don't you see that this as long as there isn't much more input is
 the fatal flaw of your patch?  You are not fixing the problem, you are
 just extending the point wher eit hits to some extend - it may work
 for a few more situations, but it's still guaranteed to fail for
 others.

In a similar way, the Linux kernel has a fatal flaw. Serial data
coming into the flip buffers under extreme interrupt load can be lost,
or the secondary buffers can become exhausted, or user space cannot
keep up with input under heavy load, etc., etc. This is the reality of
the world. As each problem presents itself we direct our attention to
it.


 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.

 Then please let's take this approach instead, if you really need to
 support such a mode of operation.

As I said I will look at this.


 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).

 That's an unrelated hardware issue that needs to be addressed
 independently.

Yes it is. I can't help wondering what the solution might be though,
if we are not allowed to buffer things.

Regards,
Simon


 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: w...@denx.de
 Most legends have their basis in facts.
        -- Kirk, And The Children Shall Lead, stardate 5029.5

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2011-10-16 Thread Wolfgang Denk 
Dear Simon Glass,

In message capnjgz1vuf-hfbdx9byuv32sjfee3skrit124gfgqom0f+q...@mail.gmail.com 
you wrote:
 
 In a similar way, the Linux kernel has a fatal flaw. Serial data
 coming into the flip buffers under extreme interrupt load can be lost,
 or the secondary buffers can become exhausted, or user space cannot
 keep up with input under heavy load, etc., etc. This is the reality of
 the world. As each problem presents itself we direct our attention to
 it.

This is not a fatal flaw.  There is nothing in the definition of RS232
or the serialinterface that guarantees you an error free data stream -
not on that protocol level.  You are facing loss of Ethernet packages
on the network as well.

Problems start only when you make wrong assumptions.


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: w...@denx.de
A girl with a future avoids the man with a past.
   -- Evan Esar, The Humor of Humor
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2011-10-15 Thread Simon Glass 
From: Scott Wood scottw...@freescale.com

From: Scott Wood scottw...@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 s...@chromium.org - ARM code size goes from
212 to 484 bytes (extra 272 bytes), BSS to 1056 bytes.

Signed-off-by: Scott Wood scottw...@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

 README   |   12 ++
 drivers/serial/ns16550.c |   97 +++--
 drivers/serial/serial.c  |5 +-
 include/ns16550.h|4 +-
 4 files changed, 109 insertions(+), 9 deletions(-)

diff --git a/README b/README
index 7e032a9..d8b4c4d 100644
--- a/README
+++ b/README
@@ -2862,6 +2862,18 @@ use the saveenv command to store a valid environment.
space for already greatly restricted images, including but not
limited to NAND_SPL configurations.
 
+- CONFIG_NS16550_RBUF_SIZE:
+   Instead of reading directly from the receive register
+   every time U-Boot is ready for another byte, keep a
+   buffer and fill it from the hardware fifo every time
+   U-Boot reads a character.  This helps U-Boot keep up with
+   a larger amount of rapid input, such as happens when
+   pasting text into the terminal.
+
+   To use this option, define CONFIG_NS16550_RBUF_SIZE to
+   the size of the buffer you want (256 is a reasonable value).
+
+
 Low Level (hardware related) configuration options:
 ---
 
diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c
index 056c25d..12de014 100644
--- a/drivers/serial/ns16550.c
+++ b/drivers/serial/ns16550.c
@@ -4,12 +4,15 @@
  * modified to use CONFIG_SYS_ISA_MEM and new defines
  */
 
+#include common.h
 #include config.h
 #include ns16550.h
 #include watchdog.h
 #include linux/types.h
 #include asm/io.h
 
+DECLARE_GLOBAL_DATA_PTR;
+
 #define UART_LCRVAL UART_LCR_8N1   /* 8 data, 1 stop, no parity */
 #define UART_MCRVAL (UART_MCR_DTR | \
 UART_MCR_RTS)  /* RTS/DTR */
@@ -95,21 +98,105 @@ void NS16550_putc(NS16550_t com_port, char c)
 }
 
 #ifndef CONFIG_NS16550_MIN_FUNCTIONS
-char NS16550_getc(NS16550_t com_port)
+static char NS16550_raw_getc(NS16550_t regs)
 {
-   while ((serial_in(com_port-lsr)  UART_LSR_DR) == 0) {
+   while ((serial_in(regs-lsr)  UART_LSR_DR) == 0) {
 #ifdef CONFIG_USB_TTY
extern void usbtty_poll(void);
usbtty_poll();
 #endif
WATCHDOG_RESET();
}
-   return serial_in(com_port-rbr);
+   return serial_in(regs-rbr);
+}
+
+static int NS16550_raw_tstc(NS16550_t regs)
+{
+   return ((serial_in(regs-lsr)  UART_LSR_DR) != 0);
+}
+
+
+#ifdef CONFIG_NS16550_RBUF_SIZE
+
+#define NUM_PORTS 4
+
+struct ns16550_priv {
+   char buf[CONFIG_NS16550_RBUF_SIZE];
+   unsigned int head, tail;
+};
+
+static struct ns16550_priv rxstate[NUM_PORTS];
+
+static void enqueue(unsigned int port, char ch)
+{
+   /* If queue is full, drop the character. */
+   if ((rxstate[port].head - rxstate[port].tail - 1) %
+   CONFIG_NS16550_RBUF_SIZE == 0)
+   return;
+
+   rxstate[port].buf[rxstate[port].tail] = ch;
+   rxstate[port].tail = (rxstate[port].tail + 1) %
+   CONFIG_NS16550_RBUF_SIZE;
+}
+
+static int dequeue(unsigned int port, char *ch)
+{
+   /* Empty queue? */
+   if (rxstate[port].head == rxstate[port].tail)
+   return 0;
+
+   *ch = rxstate[port].buf[rxstate[port].head];
+   rxstate[port].head = (rxstate[port].head + 1) %
+   CONFIG_NS16550_RBUF_SIZE;
+   return 1;
+}
+
+static void fill_rx_buf(NS16550_t regs, unsigned int port)
+{
+   while ((serial_in(regs-lsr)  UART_LSR_DR) != 0)
+   enqueue(port, serial_in(regs-rbr));
+}
+
+char NS16550_getc(NS16550_t regs, unsigned int port)
+{
+   char ch;
+
+   if (port = NUM_PORTS || !(gd-flags  GD_FLG_RELOC))
+   return NS16550_raw_getc(regs);
+
+   do  {
+#ifdef CONFIG_USB_TTY
+   extern void usbtty_poll(void);
+   usbtty_poll();
+#endif
+   fill_rx_buf(regs, port);
+   WATCHDOG_RESET();
+   } while (!dequeue(port, ch));
+
+   return ch;
+}
+
+int NS16550_tstc(NS16550_t regs, unsigned int port)
+{
+