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:34 AM, Krzysztof Kozlowski wrote:
> On 11.09.2015 15:32, Robert Baldyga wrote:
>> 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.
> 
> Hm? I was referring to my comment - I did not see any changes around
> "fixes" in commit message.

Ohh, I see :p I will describe it better ;)

Thanks,
Robert