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

[Krzysztof Kozlowski]

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@xxxxxxxxxxx>
>>> Signed-off-by: Robert Baldyga <r.baldyga@xxxxxxxxxxx>
>>> ---
>>>  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.

Best regards,
Krzysztof
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/