[PATCH 3/7] bsp/tms570: fixed sci driver freeze when closing terminal bug

[Gedare Bloom]

On Tue, Nov 17, 2015 at 2:29 PM, Pavel Pisa <pisa at cmp.felk.cvut.cz> wrote:
> Hello Gedare,
>
> On Sunday 15 of November 2015 14:44:24 Gedare Bloom wrote:
>> On Thu, Nov 12, 2015 at 5:11 PM, Pavel Pisa <pisa at cmp.felk.cvut.cz> wrote:
>> > From: Premysl Houdek <kom541000 at gmail.com>
>> >
>> > tms570_sci_interrupt_last_close: do not wait for old data transmit.
>> >
>> > Signed-off-by: Premysl Houdek <kom541000 at gmail.com>
>> > ---
>> >  c/src/lib/libbsp/arm/tms570/console/tms570-sci.c | 8 +++++---
>> >  1 file changed, 5 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/c/src/lib/libbsp/arm/tms570/console/tms570-sci.c
>> > b/c/src/lib/libbsp/arm/tms570/console/tms570-sci.c index 54c1b69..a799a66
>> > 100644
>> > --- a/c/src/lib/libbsp/arm/tms570/console/tms570-sci.c
>> > +++ b/c/src/lib/libbsp/arm/tms570/console/tms570-sci.c
>> > @@ -531,9 +531,11 @@ static void tms570_sci_interrupt_last_close(
>> >    tms570_sci_disable_interrupts(ctx);
>> >    rtems_termios_device_lock_release(base, &lock_context);
>> >
>> > -  /* Flush device */
>> > -  while ( ( ctx->regs->FLR & TMS570_SCI_FLR_TX_EMPTY ) > 0 ) {
>> > -    ;/* Wait until all data has been sent */
>> > +  if ( 0 /* for flush on close */ ) {
>>
>> I don't understand, this is dead code now?
>
> we decided to disable code. The condition to wait for last character
> to be completely transmitted has been negated. This problem has been
> corrected. Code is invoked only in few cases because console is usually
> permanently open that is why we did not catch a problem in previous
> version and it has not been catch even during some test with the second
> UART as well. Attempt to wait for the last character is  based on logic
> found in other serial interface drivers.
>
> But even that logic was right after correction, ẅe consider
> busy loop as bad option and required only seldom by applications.
>
> The code has been disabled for now. There seem to be no option
> to enable interrupt on TX EMPTY event in TMS570 UART hardware.
> This means that some form of busy/poll loop is required.
>
> We update patch if your and others prefer to delete it
> or implement more complete solution. I would prefer poll
> loop with code like this for the second option
>
If you leave disabled code on purpose please document it by some comments.

>   rtems_interval tw = rtems_clock_get_ticks_per_second();
>   tw = tw * 10 / baudrate + 1;
>   while ( ( ctx->regs->FLR & TMS570_SCI_FLR_TX_EMPTY ) == 0 ) {
>     rtems_task_wake_after(tw);
>   }
>
>> > +    /* Flush device */
>> > +    while ( ( ctx->regs->FLR & TMS570_SCI_FLR_TX_EMPTY ) == 0 ) {
>> > +      ;/* Wait until all data has been sent */
>> > +    }
>> >    }
>> >
>> >    /* uninstall ISR */
>> > --
>> > 1.9.1
>> >
>> > _______________________________________________
>> > devel mailing list
>> > devel at rtems.org
>> > http://lists.rtems.org/mailman/listinfo/devel
>