Re: [PATCH v2 4/4] serial: samsung: Fix UART status handling in DMA mode

[Robert Baldyga <r.baldyga@samsung.com>]

On 09/11/2015 08:07 AM, Krzysztof Kozlowski wrote:
> On 10.09.2015 22:41, Robert Baldyga wrote:
>> This patch fixes UART status handling in DMA mode.
> 
> I don't see any changes here. You did not respond to my comment neither.
> 
> Code looks itself good... except a locking issue but I don't know what's
> the cause. It may be not related to the patchset and maybe just not all
> of issues are fixed yet. Anyway I'll describe it in 3/4.
> 
> Best regards,
> Krzysztof
> 
>> For this purpose we
>> use s3c24xx_serial_rx_drain_fifo() instead of uart_rx_drain_fifo(), which
>> does the same thing plus checks for special conditions (such as 'break').
>>
>> Thanks to this we have, for example, Magic SysRq handling, which was
>> missing in DMA mode so far. Since we can use UART in DMA mode as serial
>> console, this is a quite important improvement.
>>
>> This change additionally simplifies RX handling code, as we no longer
>> need uart_rx_drain_fifo() function, so we can remove it.
>>
>> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
>> ---
>>  drivers/tty/serial/samsung.c | 30 +++---------------------------
>>  1 file changed, 3 insertions(+), 27 deletions(-)
>>
>> diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c
>> index 1d7dd86..d72cd73 100644
>> --- a/drivers/tty/serial/samsung.c
>> +++ b/drivers/tty/serial/samsung.c
>> @@ -385,32 +385,6 @@ static void s3c24xx_uart_copy_rx_to_tty(struct s3c24xx_uart_port *ourport,
>>  	}
>>  }
>>  
>> -static int s3c24xx_serial_rx_fifocnt(struct s3c24xx_uart_port *ourport,
>> -				     unsigned long ufstat);
>> -
>> -static void uart_rx_drain_fifo(struct s3c24xx_uart_port *ourport)
>> -{
>> -	struct uart_port *port = &ourport->port;
>> -	struct tty_port *tty = &port->state->port;
>> -	unsigned int ch, ufstat;
>> -	unsigned int count;
>> -
>> -	ufstat = rd_regl(port, S3C2410_UFSTAT);
>> -	count = s3c24xx_serial_rx_fifocnt(ourport, ufstat);
>> -
>> -	if (!count)
>> -		return;
>> -
>> -	while (count-- > 0) {
>> -		ch = rd_regb(port, S3C2410_URXH);
>> -
>> -		ourport->port.icount.rx++;
>> -		tty_insert_flip_char(tty, ch, TTY_NORMAL);
>> -	}
>> -
>> -	tty_flip_buffer_push(tty);
>> -}
>> -
>>  static void s3c24xx_serial_stop_rx(struct uart_port *port)
>>  {
>>  	struct s3c24xx_uart_port *ourport = to_ourport(port);
>> @@ -573,6 +547,8 @@ static void enable_rx_pio(struct s3c24xx_uart_port *ourport)
>>  	ourport->rx_mode = S3C24XX_RX_PIO;
>>  }
>>  
>> +static void s3c24xx_serial_rx_drain_fifo(struct s3c24xx_uart_port *ourport);
>> +
>>  static irqreturn_t s3c24xx_serial_rx_chars_dma(void *dev_id)
>>  {
>>  	unsigned int utrstat, ufstat, received;
>> @@ -606,7 +582,7 @@ static irqreturn_t s3c24xx_serial_rx_chars_dma(void *dev_id)
>>  		enable_rx_pio(ourport);
>>  	}
>>  

The essence of change is here. We use another method for draining FIFO.
Instead of just putting them into tty buffer, we additionally check for
special conditions, and that's the improvement.

>> -	uart_rx_drain_fifo(ourport);
>> +	s3c24xx_serial_rx_drain_fifo(ourport);
>>  
>>  	if (tty) {
>>  		tty_flip_buffer_push(t);
>>
> 
>