public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Graeme Russ <graeme.russ@gmail.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH v4 2/2] NS16550: buffer reads
Date: Sun, 23 Oct 2011 09:15:34 +1100	[thread overview]
Message-ID: <4EA34086.4030101@gmail.com> (raw)
In-Reply-To: <4EA28251.1080009@aribaud.net>

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

  reply	other threads:[~2011-10-22 22:15 UTC|newest]

Thread overview: 53+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-10-16  5:14 [U-Boot] [PATCH v4 1/2] NS16550: trivial code clean for checkpatch Simon Glass
2011-10-16  5:14 ` [U-Boot] [PATCH v4 2/2] NS16550: buffer reads Simon Glass
2011-10-16 12:57   ` Albert ARIBAUD
2011-10-16 17:06     ` Simon Glass
2011-10-16 20:13       ` Wolfgang Denk
2011-10-16 20:47         ` Simon Glass
2011-10-16 21:03           ` Wolfgang Denk
2011-10-17 11:08   ` Wolfgang Denk
2011-10-17 16:25     ` Simon Glass
2011-10-17 20:19     ` Simon Glass
2011-10-17 20:33       ` Wolfgang Denk
2011-10-17 20:58         ` Simon Glass
2011-10-22  8:29           ` Albert ARIBAUD
2011-10-17 12:18   ` Wolfgang Denk
2011-10-17 16:40     ` Simon Glass
2011-10-22  8:44       ` Albert ARIBAUD
2011-10-22 22:15         ` Graeme Russ [this message]
2011-10-23  8:20           ` Wolfgang Denk
2011-10-23 11:50             ` Graeme Russ
2011-10-23 17:15               ` Wolfgang Denk
2011-10-23 20:17                 ` Graeme Russ
2011-10-23 21:22                   ` Wolfgang Denk
2011-10-23 21:32                     ` [U-Boot] [PATCH v2] " Graeme Russ
2011-10-23 22:18                       ` Wolfgang Denk
2011-10-23 23:30                         ` Graeme Russ
2011-10-24  4:47                           ` Simon Glass
2011-10-24 18:46                           ` Wolfgang Denk
2011-10-24 19:26                             ` Graeme Russ
2011-10-24 20:00                               ` Wolfgang Denk
2011-10-24 20:40                                 ` Graeme Russ
2011-10-24 21:59                                   ` Wolfgang Denk
2011-10-24 22:22                                     ` Graeme Russ
2011-10-24 23:31                                       ` J. William Campbell
2011-10-25  7:31                                       ` Wolfgang Denk
2011-10-25  8:34                                         ` Graeme Russ
2011-10-25 18:41                                           ` Wolfgang Denk
2011-10-25 22:37                                             ` Graeme Russ
2011-10-25 23:17                                               ` Simon Glass
     [not found]                                                 ` <CALButCK2XnZ=HR72VaXioCfxkMFgMh2JbXzSDq9TadgKFH52rQ@mail.gmail.com >
2011-10-25 23:37                                                 ` Graeme Russ
2011-10-25 23:48                                                   ` Simon Glass
2011-10-26  3:41                                                     ` Graeme Russ
2011-10-26  7:00                                                       ` Wolfgang Denk
2011-10-26  9:18                                                         ` Graeme Russ
2011-10-26 10:19                                                           ` Wolfgang Denk
2011-10-26 16:55                                                   ` Scott Wood
2011-10-26 18:17                                                     ` Wolfgang Denk
2011-10-26 18:50                                                       ` Scott Wood
2011-10-26 19:19                                                         ` Wolfgang Denk
2011-10-26  6:54                                               ` Wolfgang Denk
2011-10-23 18:17   ` [U-Boot] [PATCH v4 2/2] " Wolfgang Denk
2011-10-23 18:20 ` [U-Boot] [PATCH v4 1/2] NS16550: trivial code clean for checkpatch Wolfgang Denk
  -- strict thread matches above, loose matches on Subject: below --
2011-10-17 12:53 [U-Boot] [PATCH v4 2/2] NS16550: buffer reads Wolfgang Denk
2011-10-25 16:09 ` Scott Wood

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=4EA34086.4030101@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox