diff mbox series

[RESEND,02/16] serial: usbtty: Send urb data in correct order

Message ID 20210205191212.7644-3-pali@kernel.org
State Superseded
Delegated to: Marek Vasut
Headers show
Series Nokia RX-51: Fix USB TTY console and enable it | expand

Commit Message

Pali Rohár Feb. 5, 2021, 7:11 p.m. UTC
Function next_urb() selects the last urb data buffer from linked list to
which next data from usbtty puts should be appended.

But to check if TX data still exists it is needed to look at the first urb
data buffer from linked list. So check for endpoint->tx_urb (first from the
linked list) instead of current_urb (the last from the linked list).

Successful call to udc_endpoint_write() may invalidate active urb and
allocate new in queue which invalidate pointer returned by next_urb()
function.

So call next_urb() prior putting data into urb buffer and call it every
time after using udc_endpoint_write() function to prevent sending data from
usbtty puts in incorrect order.

This patch fixes issue that usbtty code does not transmit data when they
are waiting in the tx queue.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 drivers/serial/usbtty.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

Comments

Lukasz Majewski Feb. 6, 2021, 1:56 p.m. UTC | #1
On Fri,  5 Feb 2021 20:11:58 +0100
Pali Rohár <pali@kernel.org> wrote:

> Function next_urb() selects the last urb data buffer from linked list
> to which next data from usbtty puts should be appended.
> 
> But to check if TX data still exists it is needed to look at the
> first urb data buffer from linked list. So check for endpoint->tx_urb
> (first from the linked list) instead of current_urb (the last from
> the linked list).
> 
> Successful call to udc_endpoint_write() may invalidate active urb and
> allocate new in queue which invalidate pointer returned by next_urb()
> function.
> 
> So call next_urb() prior putting data into urb buffer and call it
> every time after using udc_endpoint_write() function to prevent
> sending data from usbtty puts in incorrect order.
> 
> This patch fixes issue that usbtty code does not transmit data when
> they are waiting in the tx queue.

Reviewed-by: Lukasz Majewski <lukma@denx.de>

> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
>  drivers/serial/usbtty.c | 12 +++---------
>  1 file changed, 3 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/serial/usbtty.c b/drivers/serial/usbtty.c
> index 02f8edf200..4f4eb02de0 100644
> --- a/drivers/serial/usbtty.c
> +++ b/drivers/serial/usbtty.c
> @@ -849,17 +849,9 @@ static int write_buffer (circbuf_t * buf)
>  			&endpoint_instance[tx_endpoint];
>  	struct urb *current_urb = NULL;
>  
> -	current_urb = next_urb (device_instance, endpoint);
> -
> -	if (!current_urb) {
> -		TTYERR ("current_urb is NULL, buf->size %d\n",
> -		buf->size);
> -		return 0;
> -	}
> -
>  	/* TX data still exists - send it now
>  	 */
> -	if(endpoint->sent < current_urb->actual_length){
> +	if(endpoint->sent < endpoint->tx_urb->actual_length){
>  		if(udc_endpoint_write (endpoint)){
>  			/* Write pre-empted by RX */
>  			return -1;
> @@ -878,6 +870,8 @@ static int write_buffer (circbuf_t * buf)
>  		 */
>  		while (buf->size > 0) {
>  
> +			current_urb = next_urb (device_instance,
> endpoint); +
>  			dest = (char*)current_urb->buffer +
>  				current_urb->actual_length;
>  




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
diff mbox series

Patch

diff --git a/drivers/serial/usbtty.c b/drivers/serial/usbtty.c
index 02f8edf200..4f4eb02de0 100644
--- a/drivers/serial/usbtty.c
+++ b/drivers/serial/usbtty.c
@@ -849,17 +849,9 @@  static int write_buffer (circbuf_t * buf)
 			&endpoint_instance[tx_endpoint];
 	struct urb *current_urb = NULL;
 
-	current_urb = next_urb (device_instance, endpoint);
-
-	if (!current_urb) {
-		TTYERR ("current_urb is NULL, buf->size %d\n",
-		buf->size);
-		return 0;
-	}
-
 	/* TX data still exists - send it now
 	 */
-	if(endpoint->sent < current_urb->actual_length){
+	if(endpoint->sent < endpoint->tx_urb->actual_length){
 		if(udc_endpoint_write (endpoint)){
 			/* Write pre-empted by RX */
 			return -1;
@@ -878,6 +870,8 @@  static int write_buffer (circbuf_t * buf)
 		 */
 		while (buf->size > 0) {
 
+			current_urb = next_urb (device_instance, endpoint);
+
 			dest = (char*)current_urb->buffer +
 				current_urb->actual_length;