Re: [PATCH] serial: 8250: Fix THRE flag usage for CAP_MINI

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

 



On 28/06/2017 09:54, Stefan Wahren wrote:
> Hi Phil,
> 
> Am 28.06.2017 um 10:42 schrieb Phil Elwell:
>> The BCM2835 MINI UART has non-standard THRE semantics. Conventionally
>> the bit means that the FIFO is empty (although there may still be a
>> byte in the transmit register), but on 2835 it indicates that the FIFO
>> is not full. This causes interrupts after every byte is transmitted,
>> with the FIFO providing some interrupt latency tolerance.
>>
>> A consequence of this difference is that the usual strategy of writing
>> multiple bytes into the TX FIFO after checking THRE once is unsafe.
>> In the worst case of 7 bytes in the FIFO, writing 8 bytes loses all
>> but the first since by then the FIFO is full.
>>
>> There is an HFIFO ("Hidden FIFO") capability that causes the transmit
>> loop to terminate when both THRE and TEMT are set, i.e. when the TX
>> block is completely idle. This is unnecessarily cautious, potentially
>> causing gaps in transmission.
>>
>> Add a new conditional to the transmit loop, predicated on CAP_MINI,
>> that exits when THRE is no longer set (the FIFO is full). This allows
>> the FIFO to fill quickly but subsequent writes are paced by the
>> transmission rate.
>>
>> Signed-off-by: Phil Elwell <phil@xxxxxxxxxxxxxxx>
>> Acked-by: Eric Anholt <eric@xxxxxxxxxx>
>> Acked-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
>> ---
>>  drivers/tty/serial/8250/8250_port.c | 4 ++++
>>  1 file changed, 4 insertions(+)
> 
> please increase the version of the patch and add your changelog below
> this line, so Greg has the chance to apply the right fix.

But I've seen people reprimanded for posting a new version where the only difference
is in the review history, with claims that it doesn't make the merger's job easier.
I'll repost anyway.

> Also i assume that Andy wanted to suggest you to add the fixes tag.

This patch doesnt fix a bug in previous commit, it adds support for broken hardware,
so I don't think the "Fixes:" tag applies.

>>
>> diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
>> index 4c620be..a5fe0e6 100644
>> --- a/drivers/tty/serial/8250/8250_port.c
>> +++ b/drivers/tty/serial/8250/8250_port.c
>> @@ -1764,6 +1764,10 @@ void serial8250_tx_chars(struct uart_8250_port *up)
>>  		if ((up->capabilities & UART_CAP_HFIFO) &&
>>  		    (serial_in(up, UART_LSR) & BOTH_EMPTY) != BOTH_EMPTY)
>>  			break;
>> +		/* The BCM2835 MINI UART THRE bit is really a not-full bit. */
>> +		if ((up->capabilities & UART_CAP_MINI) &&
>> +		    !(serial_in(up, UART_LSR) & UART_LSR_THRE))
>> +			break;
>>  	} while (--count > 0);
>>  
>>  	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[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