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

[Albert ARIBAUD <albert.u.boot@aribaud.net>]

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.