Re: [PATCH v2 2/3] serial: 8250_exar: Refactor exar_shutdown()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> One more though. Let's consider your code carefully.
>
> do {
>         lsr = serial_in(up, UART_LSR);
>         if (lsr & (UART_LSR_TEMT | UART_LSR_THRE))
>                 tx_complete = 1;
>         else
>                 tx_complete = 0;
>
> Here we set a boolean to be true or false, fine...
>
>         msleep(1);
>
> ...wait for some undefined long time...
>
> } while (!uart_circ_empty(xmit) && !tx_complete && i++ < 1000);
>
> ...first conditional is to check kernel buffer for emptiness, and since it's an
> AND operation in use, break if it false...
>
> (In comparison to what I proposed, the difference is, that I'm not reading
>  register and waiting for some milliseconds)
>
> ...then, you check a boolean variable...
>
> (similar what I have done with a 'break' statement)
>
> ...and at last we have check the iteration counter.
>
> (this same in both cases).
>
> So, I'm puzzled how your cases works reliably if it based on msleep().
> With enough slow baud rate you won't get fix working AFAIU.
> Where am I wrong?

It seems as though you are right here - I thought I had tested this
with a slower baud, but I may have only tested it with a small
string(at which point the problem does not show up, or only shows up
rarely).  All of my serial ports are running at 115200, but lowering
that can cause it to fail.

I have it working reliably at the moment, but I need to run a few more
tests to check it.  It does look like the uart_circ_empty() is an
invalid check though; removing it does allow for everything to be
transmitted at a lower baud rate(although still using msleep, not
usleep_range).  It seems that subtle changes in how this function
works cause it to be unreliable again - I have been changing
msleep/usleep_range, as well as the iterations, and removing the
uart_circ_emtpy, and I get different results depending on the
settings.

I'll try to get some firm results in the morning; otherwise I won't be
able to check until early next week as I will be away from the
hardware.

-Robert Middleton



[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux PPP]     [Linux FS]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Linmodem]     [Device Mapper]     [Linux Kernel for ARM]

  Powered by Linux