[PATCH v2 1/2] tty: serial: 8250_omap: fix tx with dma

[PATCH v2 1/2] tty: serial: 8250_omap: fix tx with dma

[Mans Rullgard]

Commit 1788cf6a91d9 ("tty: serial: switch from circ_buf to kfifo")
introduced an error in the TX DMA handling for 8250_omap.

When the OMAP_DMA_TX_KICK flag is set, one byte is pulled from the
kfifo and emitted directly in order to start the DMA.  This is done
without updating DMA tx_size which leads to uart_xmit_advance() called
in the DMA complete callback advancing the kfifo by one too much.

In practice, transmitting N bytes has been seen to result in the last
N-1 bytes being sent repeatedly.

This change fixes the problem by moving all of the dma setup after
the OMAP_DMA_TX_KICK handling and using kfifo_len() instead of the
dma size for the 4-byte cutoff check. This slightly changes the
behaviour at buffer wraparound, but it still transmits the correct
bytes somehow. At the point kfifo_dma_out_prepare_mapped is called,
at least one byte is guaranteed to be in the fifo, so checking the
return value is not necessary.

Fixes: 1788cf6a91d9 ("tty: serial: switch from circ_buf to kfifo")
Cc: stable@vger.kernel.org
Signed-off-by: Mans Rullgard <mans@mansr.com>
---
v2: split patch in two
---
 drivers/tty/serial/8250/8250_omap.c | 24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
index f1aee915bc02..180466e09605 100644
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -1173,16 +1173,6 @@ static int omap_8250_tx_dma(struct uart_8250_port *p)
 		return 0;
 	}
 
-	sg_init_table(&sg, 1);
-	ret = kfifo_dma_out_prepare_mapped(&tport->xmit_fifo, &sg, 1,
-					   UART_XMIT_SIZE, dma->tx_addr);
-	if (ret != 1) {
-		serial8250_clear_THRI(p);
-		return 0;
-	}
-
-	dma->tx_size = sg_dma_len(&sg);
-
 	if (priv->habit & OMAP_DMA_TX_KICK) {
 		unsigned char c;
 		u8 tx_lvl;
@@ -1207,7 +1197,7 @@ static int omap_8250_tx_dma(struct uart_8250_port *p)
 			ret = -EBUSY;
 			goto err;
 		}
-		if (dma->tx_size < 4) {
+		if (kfifo_len(&tport->xmit_fifo) < 4) {
 			ret = -EINVAL;
 			goto err;
 		}
@@ -1216,11 +1206,12 @@ static int omap_8250_tx_dma(struct uart_8250_port *p)
 			goto err;
 		}
 		skip_byte = c;
-		/* now we need to recompute due to kfifo_get */
-		kfifo_dma_out_prepare_mapped(&tport->xmit_fifo, &sg, 1,
-				UART_XMIT_SIZE, dma->tx_addr);
 	}
 
+	sg_init_table(&sg, 1);
+	kfifo_dma_out_prepare_mapped(&tport->xmit_fifo, &sg, 1,
+				     UART_XMIT_SIZE, dma->tx_addr);
+
 	desc = dmaengine_prep_slave_sg(dma->txchan, &sg, 1, DMA_MEM_TO_DEV,
 			DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
 	if (!desc) {
@@ -1228,6 +1219,7 @@ static int omap_8250_tx_dma(struct uart_8250_port *p)
 		goto err;
 	}
 
+	dma->tx_size = sg_dma_len(&sg);
 	dma->tx_running = 1;
 
 	desc->callback = omap_8250_dma_tx_complete;
@@ -1248,8 +1240,10 @@ static int omap_8250_tx_dma(struct uart_8250_port *p)
 err:
 	dma->tx_err = 1;
 out_skip:
-	if (skip_byte >= 0)
+	if (skip_byte >= 0) {
 		serial_out(p, UART_TX, skip_byte);
+		p->port.icount.tx++;
+	}
 	return ret;
 }
 
-- 
2.49.0

[PATCH 2/2] tty: serial: 8250_omap: use two-entry sg list for tx dma

[Mans Rullgard]

On buffer wraparound, two sg entries are needed to transfer the full
contents of the kfifo.

This is equivalent to the change made to the 8250_dma driver in commit
59449c9dbdaa ("tty: serial: 8250_dma: use sgl with 2 nents to take care
of buffer wrap").

Signed-off-by: Mans Rullgard <mans@mansr.com>
---
 drivers/tty/serial/8250/8250_omap.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
index 180466e09605..66ba43a20725 100644
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -1152,9 +1152,11 @@ static int omap_8250_tx_dma(struct uart_8250_port *p)
 	struct omap8250_priv		*priv = p->port.private_data;
 	struct tty_port			*tport = &p->port.state->port;
 	struct dma_async_tx_descriptor	*desc;
-	struct scatterlist sg;
+	struct scatterlist *sg;
+	struct scatterlist sgl[2];
 	int skip_byte = -1;
 	int ret;
+	int i;
 
 	if (dma->tx_running)
 		return 0;
@@ -1208,18 +1210,22 @@ static int omap_8250_tx_dma(struct uart_8250_port *p)
 		skip_byte = c;
 	}
 
-	sg_init_table(&sg, 1);
-	kfifo_dma_out_prepare_mapped(&tport->xmit_fifo, &sg, 1,
-				     UART_XMIT_SIZE, dma->tx_addr);
+	sg_init_table(sgl, ARRAY_SIZE(sgl));
+	ret = kfifo_dma_out_prepare_mapped(&tport->xmit_fifo, sgl, ARRAY_SIZE(sgl),
+					   UART_XMIT_SIZE, dma->tx_addr);
 
-	desc = dmaengine_prep_slave_sg(dma->txchan, &sg, 1, DMA_MEM_TO_DEV,
+	desc = dmaengine_prep_slave_sg(dma->txchan, sgl, ret, DMA_MEM_TO_DEV,
 			DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
 	if (!desc) {
 		ret = -EBUSY;
 		goto err;
 	}
 
-	dma->tx_size = sg_dma_len(&sg);
+	dma->tx_size = 0;
+
+	for_each_sg(sgl, sg, ret, i)
+		dma->tx_size += sg_dma_len(sg);
+
 	dma->tx_running = 1;
 
 	desc->callback = omap_8250_dma_tx_complete;
-- 
2.49.0

Re: [PATCH v2 1/2] tty: serial: 8250_omap: fix tx with dma

[Jiri Slaby]

On 06. 05. 25, 17:07, Mans Rullgard wrote:
> Commit 1788cf6a91d9 ("tty: serial: switch from circ_buf to kfifo")
> introduced an error in the TX DMA handling for 8250_omap.
> 
> When the OMAP_DMA_TX_KICK flag is set, one byte is pulled from the
> kfifo and emitted directly in order to start the DMA.  This is done
> without updating DMA tx_size which leads to uart_xmit_advance() called
> in the DMA complete callback advancing the kfifo by one too much.
> 
> In practice, transmitting N bytes has been seen to result in the last
> N-1 bytes being sent repeatedly.
> 
> This change fixes the problem by moving all of the dma setup after
> the OMAP_DMA_TX_KICK handling and using kfifo_len() instead of the
> dma size for the 4-byte cutoff check. This slightly changes the
> behaviour at buffer wraparound, but it still transmits the correct
> bytes somehow. At the point kfifo_dma_out_prepare_mapped is called,
> at least one byte is guaranteed to be in the fifo, so checking the
> return value is not necessary.
> 
> Fixes: 1788cf6a91d9 ("tty: serial: switch from circ_buf to kfifo")
> Cc: stable@vger.kernel.org
> Signed-off-by: Mans Rullgard <mans@mansr.com>
> ---
> v2: split patch in two
> ---
>   drivers/tty/serial/8250/8250_omap.c | 24 +++++++++---------------
>   1 file changed, 9 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
> index f1aee915bc02..180466e09605 100644
> --- a/drivers/tty/serial/8250/8250_omap.c
> +++ b/drivers/tty/serial/8250/8250_omap.c
> @@ -1173,16 +1173,6 @@ static int omap_8250_tx_dma(struct uart_8250_port *p)
>   		return 0;
>   	}
>   
> -	sg_init_table(&sg, 1);
> -	ret = kfifo_dma_out_prepare_mapped(&tport->xmit_fifo, &sg, 1,
> -					   UART_XMIT_SIZE, dma->tx_addr);
> -	if (ret != 1) {
> -		serial8250_clear_THRI(p);
> -		return 0;
> -	}
> -
> -	dma->tx_size = sg_dma_len(&sg);
> -
>   	if (priv->habit & OMAP_DMA_TX_KICK) {
>   		unsigned char c;
>   		u8 tx_lvl;
...
> @@ -1216,11 +1206,12 @@ static int omap_8250_tx_dma(struct uart_8250_port *p)
>   			goto err;
>   		}
>   		skip_byte = c;
> -		/* now we need to recompute due to kfifo_get */
> -		kfifo_dma_out_prepare_mapped(&tport->xmit_fifo, &sg, 1,
> -				UART_XMIT_SIZE, dma->tx_addr);
>   	}
>   
> +	sg_init_table(&sg, 1);
> +	kfifo_dma_out_prepare_mapped(&tport->xmit_fifo, &sg, 1,
> +				     UART_XMIT_SIZE, dma->tx_addr);

This can fail (note the first call to this was checked). The latter 
(deliberately) not.

> +
>   	desc = dmaengine_prep_slave_sg(dma->txchan, &sg, 1, DMA_MEM_TO_DEV,
>   			DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
>   	if (!desc) {
...
> @@ -1248,8 +1240,10 @@ static int omap_8250_tx_dma(struct uart_8250_port *p)
>   err:
>   	dma->tx_err = 1;
>   out_skip:
> -	if (skip_byte >= 0)
> +	if (skip_byte >= 0) {
>   		serial_out(p, UART_TX, skip_byte);
> +		p->port.icount.tx++;

This is still unrelated.

thanks,
-- 
js
suse labs

Re: [PATCH v2 1/2] tty: serial: 8250_omap: fix tx with dma

[Måns Rullgård]

Jiri Slaby <jirislaby@kernel.org> writes:

> On 06. 05. 25, 17:07, Mans Rullgard wrote:
>> Commit 1788cf6a91d9 ("tty: serial: switch from circ_buf to kfifo")
>> introduced an error in the TX DMA handling for 8250_omap.
>> When the OMAP_DMA_TX_KICK flag is set, one byte is pulled from the
>> kfifo and emitted directly in order to start the DMA.  This is done
>> without updating DMA tx_size which leads to uart_xmit_advance() called
>> in the DMA complete callback advancing the kfifo by one too much.
>> In practice, transmitting N bytes has been seen to result in the last
>> N-1 bytes being sent repeatedly.
>> This change fixes the problem by moving all of the dma setup after
>> the OMAP_DMA_TX_KICK handling and using kfifo_len() instead of the
>> dma size for the 4-byte cutoff check. This slightly changes the
>> behaviour at buffer wraparound, but it still transmits the correct
>> bytes somehow. At the point kfifo_dma_out_prepare_mapped is called,
>> at least one byte is guaranteed to be in the fifo, so checking the
>> return value is not necessary.
>> Fixes: 1788cf6a91d9 ("tty: serial: switch from circ_buf to kfifo")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Mans Rullgard <mans@mansr.com>
>> ---
>> v2: split patch in two
>> ---
>>   drivers/tty/serial/8250/8250_omap.c | 24 +++++++++---------------
>>   1 file changed, 9 insertions(+), 15 deletions(-)
>> diff --git a/drivers/tty/serial/8250/8250_omap.c
>> b/drivers/tty/serial/8250/8250_omap.c
>> index f1aee915bc02..180466e09605 100644
>> --- a/drivers/tty/serial/8250/8250_omap.c
>> +++ b/drivers/tty/serial/8250/8250_omap.c
>> @@ -1173,16 +1173,6 @@ static int omap_8250_tx_dma(struct uart_8250_port *p)
>>   		return 0;
>>   	}
>>   -	sg_init_table(&sg, 1);
>> -	ret = kfifo_dma_out_prepare_mapped(&tport->xmit_fifo, &sg, 1,
>> -					   UART_XMIT_SIZE, dma->tx_addr);
>> -	if (ret != 1) {
>> -		serial8250_clear_THRI(p);
>> -		return 0;
>> -	}
>> -
>> -	dma->tx_size = sg_dma_len(&sg);
>> -
>>   	if (priv->habit & OMAP_DMA_TX_KICK) {
>>   		unsigned char c;
>>   		u8 tx_lvl;
> ...
>> @@ -1216,11 +1206,12 @@ static int omap_8250_tx_dma(struct uart_8250_port *p)
>>   			goto err;
>>   		}
>>   		skip_byte = c;
>> -		/* now we need to recompute due to kfifo_get */
>> -		kfifo_dma_out_prepare_mapped(&tport->xmit_fifo, &sg, 1,
>> -				UART_XMIT_SIZE, dma->tx_addr);
>>   	}
>>   +	sg_init_table(&sg, 1);
>> +	kfifo_dma_out_prepare_mapped(&tport->xmit_fifo, &sg, 1,
>> +				     UART_XMIT_SIZE, dma->tx_addr);
>
> This can fail (note the first call to this was checked). The latter
> (deliberately) not.

No, it can't.  The fifo has already been checked to contain something
right at the top of the function.  There is no other failure mode for
kfifo_dma_out_prepare_mapped.

>> +
>>   	desc = dmaengine_prep_slave_sg(dma->txchan, &sg, 1, DMA_MEM_TO_DEV,
>>   			DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
>>   	if (!desc) {
> ...
>> @@ -1248,8 +1240,10 @@ static int omap_8250_tx_dma(struct uart_8250_port *p)
>>   err:
>>   	dma->tx_err = 1;
>>   out_skip:
>> -	if (skip_byte >= 0)
>> +	if (skip_byte >= 0) {
>>   		serial_out(p, UART_TX, skip_byte);
>> +		p->port.icount.tx++;
>
> This is still unrelated.

No, it's not.  Your broken code called uart_xmit_advance with the full
amount due to the incorrect tx_size value.  This compensates for that.

You made this mess.  Now fix it.  I don't care how.  It's wasted enough
of my time already.

-- 
Måns Rullgård

Re: [PATCH v2 1/2] tty: serial: 8250_omap: fix tx with dma

[Jiri Slaby]

On 07. 05. 25, 9:49, Måns Rullgård wrote:
> Jiri Slaby <jirislaby@kernel.org> writes:
> 
>> On 06. 05. 25, 17:07, Mans Rullgard wrote:
>>> Commit 1788cf6a91d9 ("tty: serial: switch from circ_buf to kfifo")
>>> introduced an error in the TX DMA handling for 8250_omap.
>>> When the OMAP_DMA_TX_KICK flag is set, one byte is pulled from the
>>> kfifo and emitted directly in order to start the DMA.  This is done
>>> without updating DMA tx_size which leads to uart_xmit_advance() called
>>> in the DMA complete callback advancing the kfifo by one too much.
>>> In practice, transmitting N bytes has been seen to result in the last
>>> N-1 bytes being sent repeatedly.
>>> This change fixes the problem by moving all of the dma setup after
>>> the OMAP_DMA_TX_KICK handling and using kfifo_len() instead of the
>>> dma size for the 4-byte cutoff check. This slightly changes the
>>> behaviour at buffer wraparound, but it still transmits the correct
>>> bytes somehow. At the point kfifo_dma_out_prepare_mapped is called,
>>> at least one byte is guaranteed to be in the fifo, so checking the
>>> return value is not necessary.
>>> Fixes: 1788cf6a91d9 ("tty: serial: switch from circ_buf to kfifo")
>>> Cc: stable@vger.kernel.org
>>> Signed-off-by: Mans Rullgard <mans@mansr.com>
>>> ---
>>> v2: split patch in two
>>> ---
>>>    drivers/tty/serial/8250/8250_omap.c | 24 +++++++++---------------
>>>    1 file changed, 9 insertions(+), 15 deletions(-)
>>> diff --git a/drivers/tty/serial/8250/8250_omap.c
>>> b/drivers/tty/serial/8250/8250_omap.c
>>> index f1aee915bc02..180466e09605 100644
>>> --- a/drivers/tty/serial/8250/8250_omap.c
>>> +++ b/drivers/tty/serial/8250/8250_omap.c
>>> @@ -1173,16 +1173,6 @@ static int omap_8250_tx_dma(struct uart_8250_port *p)
>>>    		return 0;
>>>    	}
>>>    -	sg_init_table(&sg, 1);
>>> -	ret = kfifo_dma_out_prepare_mapped(&tport->xmit_fifo, &sg, 1,
>>> -					   UART_XMIT_SIZE, dma->tx_addr);
>>> -	if (ret != 1) {
>>> -		serial8250_clear_THRI(p);
>>> -		return 0;
>>> -	}
>>> -
>>> -	dma->tx_size = sg_dma_len(&sg);
>>> -
>>>    	if (priv->habit & OMAP_DMA_TX_KICK) {
>>>    		unsigned char c;
>>>    		u8 tx_lvl;
>> ...
>>> @@ -1216,11 +1206,12 @@ static int omap_8250_tx_dma(struct uart_8250_port *p)
>>>    			goto err;
>>>    		}
>>>    		skip_byte = c;
>>> -		/* now we need to recompute due to kfifo_get */
>>> -		kfifo_dma_out_prepare_mapped(&tport->xmit_fifo, &sg, 1,
>>> -				UART_XMIT_SIZE, dma->tx_addr);
>>>    	}
>>>    +	sg_init_table(&sg, 1);
>>> +	kfifo_dma_out_prepare_mapped(&tport->xmit_fifo, &sg, 1,
>>> +				     UART_XMIT_SIZE, dma->tx_addr);
>>
>> This can fail (note the first call to this was checked). The latter
>> (deliberately) not.
> 
> No, it can't.  The fifo has already been checked to contain something
> right at the top of the function.  There is no other failure mode for
> kfifo_dma_out_prepare_mapped.

That it cannot fail now does not mean it cannot in the future. Simply do 
it properly and check the retval.

>>> +
>>>    	desc = dmaengine_prep_slave_sg(dma->txchan, &sg, 1, DMA_MEM_TO_DEV,
>>>    			DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
>>>    	if (!desc) {
>> ...
>>> @@ -1248,8 +1240,10 @@ static int omap_8250_tx_dma(struct uart_8250_port *p)
>>>    err:
>>>    	dma->tx_err = 1;
>>>    out_skip:
>>> -	if (skip_byte >= 0)
>>> +	if (skip_byte >= 0) {
>>>    		serial_out(p, UART_TX, skip_byte);
>>> +		p->port.icount.tx++;
>>
>> This is still unrelated.
> 
> No, it's not.  Your broken code called uart_xmit_advance with the full
> amount due to the incorrect tx_size value.  This compensates for that.

Then document it properly in the commit log.

> You made this mess.

I can only say that I am sorry for the breakage of this driver. This TX 
way with one byte via FIFO and the rest via DMA, and only if > 4 chars 
to be sent is indeed cumbersome and apparently uneasy to do right.

> Now fix it.  I don't care how.  It's wasted enough
> of my time already.

How exactly does this help to get the code in shape? You apparently have 
the HW, you spent some time debugging that, you have patches, so you 
deserve the credits.

thanks,
-- 
js
suse labs

Re: [PATCH v2 1/2] tty: serial: 8250_omap: fix tx with dma

[Måns Rullgård]

Jiri Slaby <jirislaby@kernel.org> writes:

> On 07. 05. 25, 9:49, Måns Rullgård wrote:
>> Jiri Slaby <jirislaby@kernel.org> writes:
>> 
>>> On 06. 05. 25, 17:07, Mans Rullgard wrote:
>>>> Commit 1788cf6a91d9 ("tty: serial: switch from circ_buf to kfifo")
>>>> introduced an error in the TX DMA handling for 8250_omap.
>>>> When the OMAP_DMA_TX_KICK flag is set, one byte is pulled from the
>>>> kfifo and emitted directly in order to start the DMA.  This is done
>>>> without updating DMA tx_size which leads to uart_xmit_advance() called
>>>> in the DMA complete callback advancing the kfifo by one too much.
>>>> In practice, transmitting N bytes has been seen to result in the last
>>>> N-1 bytes being sent repeatedly.
>>>> This change fixes the problem by moving all of the dma setup after
>>>> the OMAP_DMA_TX_KICK handling and using kfifo_len() instead of the
>>>> dma size for the 4-byte cutoff check. This slightly changes the
>>>> behaviour at buffer wraparound, but it still transmits the correct
>>>> bytes somehow. At the point kfifo_dma_out_prepare_mapped is called,
>>>> at least one byte is guaranteed to be in the fifo, so checking the
>>>> return value is not necessary.
>>>> Fixes: 1788cf6a91d9 ("tty: serial: switch from circ_buf to kfifo")
>>>> Cc: stable@vger.kernel.org
>>>> Signed-off-by: Mans Rullgard <mans@mansr.com>
>>>> ---
>>>> v2: split patch in two
>>>> ---
>>>>    drivers/tty/serial/8250/8250_omap.c | 24 +++++++++---------------
>>>>    1 file changed, 9 insertions(+), 15 deletions(-)
>>>> diff --git a/drivers/tty/serial/8250/8250_omap.c
>>>> b/drivers/tty/serial/8250/8250_omap.c
>>>> index f1aee915bc02..180466e09605 100644
>>>> --- a/drivers/tty/serial/8250/8250_omap.c
>>>> +++ b/drivers/tty/serial/8250/8250_omap.c
>>>> @@ -1173,16 +1173,6 @@ static int omap_8250_tx_dma(struct uart_8250_port *p)
>>>>    		return 0;
>>>>    	}
>>>>    -	sg_init_table(&sg, 1);
>>>> -	ret = kfifo_dma_out_prepare_mapped(&tport->xmit_fifo, &sg, 1,
>>>> -					   UART_XMIT_SIZE, dma->tx_addr);
>>>> -	if (ret != 1) {
>>>> -		serial8250_clear_THRI(p);
>>>> -		return 0;
>>>> -	}
>>>> -
>>>> -	dma->tx_size = sg_dma_len(&sg);
>>>> -
>>>>    	if (priv->habit & OMAP_DMA_TX_KICK) {
>>>>    		unsigned char c;
>>>>    		u8 tx_lvl;
>>> ...
>>>> @@ -1216,11 +1206,12 @@ static int omap_8250_tx_dma(struct uart_8250_port *p)
>>>>    			goto err;
>>>>    		}
>>>>    		skip_byte = c;
>>>> -		/* now we need to recompute due to kfifo_get */
>>>> -		kfifo_dma_out_prepare_mapped(&tport->xmit_fifo, &sg, 1,
>>>> -				UART_XMIT_SIZE, dma->tx_addr);
>>>>    	}
>>>>    +	sg_init_table(&sg, 1);
>>>> +	kfifo_dma_out_prepare_mapped(&tport->xmit_fifo, &sg, 1,
>>>> +				     UART_XMIT_SIZE, dma->tx_addr);
>>>
>>> This can fail (note the first call to this was checked). The latter
>>> (deliberately) not.
>> No, it can't.  The fifo has already been checked to contain something
>> right at the top of the function.  There is no other failure mode for
>> kfifo_dma_out_prepare_mapped.
>
> That it cannot fail now does not mean it cannot in the future. Simply do it
> properly and check the retval.
>
>>>> +
>>>>    	desc = dmaengine_prep_slave_sg(dma->txchan, &sg, 1, DMA_MEM_TO_DEV,
>>>>    			DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
>>>>    	if (!desc) {
>>> ...
>>>> @@ -1248,8 +1240,10 @@ static int omap_8250_tx_dma(struct uart_8250_port *p)
>>>>    err:
>>>>    	dma->tx_err = 1;
>>>>    out_skip:
>>>> -	if (skip_byte >= 0)
>>>> +	if (skip_byte >= 0) {
>>>>    		serial_out(p, UART_TX, skip_byte);
>>>> +		p->port.icount.tx++;
>>>
>>> This is still unrelated.
>> No, it's not.  Your broken code called uart_xmit_advance with the full
>> amount due to the incorrect tx_size value.  This compensates for that.
>
> Then document it properly in the commit log.
>
>> You made this mess.
>
> I can only say that I am sorry for the breakage of this driver. This TX way
> with one byte via FIFO and the rest via DMA, and only if > 4 chars to be
> sent is indeed cumbersome and apparently uneasy to do right.
>
>> Now fix it.  I don't care how.  It's wasted enough
>> of my time already.
>
> How exactly does this help to get the code in shape? You apparently have the
> HW, you spent some time debugging that, you have patches, so you deserve the
> credits.

I don't care about credit, I just want it fixed, and I feel like that's
your responsibility since you broke it.  I've pointed out where the
problem is and provided a fix.  If that's not enough for you, then I
give up.  I'm not paid enough to play your guessing games.

-- 
Måns Rullgård