[U-Boot] [PATCH v2] NS16550: buffer reads

[Simon Glass]

Hi,

On Tue, Oct 25, 2011 at 3:37 PM, Graeme Russ <graeme.russ at gmail.com> wrote:
> Hi Wolfgang,
>
> On Wed, Oct 26, 2011 at 5:41 AM, Wolfgang Denk <wd at denx.de> wrote:
>> Dear Graeme Russ,
>>
>> In message <4EA67491.5090802 at gmail.com> you wrote:
>>>
>>> Well, if a command reads from the console (a transfer command for example)
>>> and then has a long delay (busy processing loop) before returning back to
>>> the command line processor then that command must be fundamentally broken -
>>
>> ???? Could you please explain what makes you think so?
>>
>> What the command does, and when it performs I/O or when it spens time
>> for other tasks is only up to the command.
>>
>> If anything is broken then it is a design that puts restrictions on
>> such behaviour.
>>
>>>  a) Fix the command so it isn't broken
>>>  b) Have the command tell the console it has finished with the console
>>>     before it starts the busy loop
>>>  c) Use UART managed hardware flow control
>>>  d) Implement interrupt based serial drivers
>>>  e) Prohibit multi-line input
>>
>> Prohibit is a bit of a strong word here.  "Not support" is more like
>> it.
>>
>>> So let's assume for the meantime that there are no 'broken' commands we can
>>
>> This is not an assumption.  No implementation must put any
>> restrictions on the timing behaviour of any commands.
>>
>>> simply issue an XOFF before running each command - That should eliminate
>>
>> That's brainded overhead. In 99.999% of all cases it's not needed.
>> And it happens at completely the wrong place.
>>
>> Serial flow control is something we should deal with in the serial
>> driver code.  Nowhere else.
>
> This problem comes down to managing two asynchronous tasks - Command
> Processing and Serial Input Processing. Any solution to the problem
> is going to involve task switching. The way this is done in U-Boot
> currently is:
>
>  - When in the main() loop, U-Boot is effectively running a Command
>   Processing Taks (monitor inbound characters, determine when a
>   command has been entered) which continually switches over to Serial
>   Input Processing Task. When a valid command is detected, U-Boot
>   switches to the corresponding Command Processing Task
>  - Some 'commands' (file transfers in particular) have their own Serial
>   Input Processing Sub-Tasks - The Command Processing Task may run for a
>   'significant' amount of time after these Sub-Tasks are finished with
>   and no longer processing serial input
>
> When you are dealing with asynchronous tasks, you can task switch by
> either:
>
>  1) Using a hardware interrupt (either a tick-timer or interrupt line from
>    the serial UART)
>  2) Littering the main taks with co-operative interrupt calls
>  3) Task switch at clearly defined locations in the code
>
> Option 1 is not supported in U-Boot
> Option 2 is used by U-Boot for triggering the watchdog - It's not pretty,
> but in the absence of #1, we have no other option
> Option 3 is all we are left with...
>
> And U-Boot does actually do #3, but the 'clearly defined locations' are
> not obvious, and the switch is done _without_ telling the remote serial
> transmitter that U-Boot is busy and will not be able to process any more
> inbound characters for the time being.
>
> Note that Serial Input Processing Task does not need to run after we have
> sent an XOFF and flushed the remote Tx buffer and local Rx buffer as the
> remote end _should_ have stopped sending characters. So in order to
> absolutely prevent dropped characters, it is a simple matter of sending an
> XOFF and flushing the buffers when we switch out of the Serial Input
> Processing Task and into the Command Processing Task. Sounds easy enough,
> BUT, the Serial Input Processing Task is a dumb task (it simply reads
> single characters from the UART) and has no idea what triggers the switch
> to the Command Processing Task so there is no way for it to know when to
> sent the XOFF - The Command Processing Task needs to tell the Serial
> Input Processing Task
>
> I suggested a solution whereby any task which _knows_ it switches to the
> Serial Input Processing Taks (i.e. calls getc()) can:
>  - Tell the Serial Input Processing Task 'I will need serial input from
>   the remote end, can you please arrange it so that I recieve those
>   characters'
>  - Process the incoming stream (it is the responsibility of the task to
>   make sure the stream is processed without dropping characters, or
>   use a protocol which allows for resending of dropped charaters)
>  - Tell the Serial Input Processing Task 'OK, I've recieved all the data
>   I need to and I will not be processing any more - You have been warned,
>   so if you want don't want to loose input, you'll need to do something
>   about it'
>
> I'm at a loss to think of any other solution - Can you?
>
>> I will not accept such a design nor such an implementation.
>
> Then we live with dropped characters
>
> Regards,
>
> Graeme
>

Did I mention a can of worms? After 65 messages on this topic Scott's
patch seems pretty appealing right now! We can even move it up a level
in the s/w stack if that helps.

But to continue this a little, and donning my asbestos suit, should
U-Boot have a CONFIG to enable an event loop called in delay
functions, network functions, read/write operations, etc.? It would
permit us to solve this problem properly I think, if we think it is
worth solving. Not saying it is a good idea...

Regards,
Simon