Re: [PATCH v2 RESEND] serial: sc16is7xx: fix TX gap caused by kfifo circular buffer wrap-around

[Hugo Villeneuve <hugo@hugovil.com>]

Hi Paul,

On Mon, 28 Sep 2026 10:55:59 +0200
Paul Mbewe <paultyson.mbewe@ziehl-abegg.de> wrote:

> kfifo_out_linear_ptr() returns only one contiguous linear segment of the
> xmit kfifo. When transmit data wraps around the end of the kfifo, only
> the first segment up to the buffer end is sent. The remaining data at
> the start of the kfifo is not sent until the next TX interrupt fires,
> resulting in a visible mid-frame TX gap on the wire.
> 
> The resulting gap is unintended: data remains queued in the xmit kfifo,
> but the hardware TX FIFO drains empty before the remaining segment is
> sent. Such gaps can break timing-sensitive serial protocols such as
> Modbus RTU.
> 
> Modbus RTU requires a message to be transmitted as a continuous stream.
> For baud rates above 19200, the Modbus Serial Line guide recommends a
> fixed 750 us inter-character timeout. On the tested 115200-baud system,
> oscilloscope measurements showed mid-frame gaps exceeding that value.
> Receivers using the recommended timeout may therefore discard the
> incomplete message.
> 
> The incomplete transfer also causes unnecessary TX interrupts: instead
> of using all available hardware TX FIFO space in one go, the driver
> requires an extra interrupt to send the remaining segment after the
> wrap.
> 
> After the tty xmit buffer was converted to a kfifo, the driver used
> uart_fifo_out() to copy data into a linear staging buffer, allowing a
> transfer to span the kfifo wrap-around boundary. Commit 133f4c00b8b2
> ("serial: sc16is7xx: fix TX fifo corruption") replaced uart_fifo_out()
> with a single kfifo_out_linear_ptr() call to remove the shared TX/RX
> buffer. Since kfifo_out_linear_ptr() exposes only one contiguous
> segment, that change lost the wrap-around handling.
> 
> Fix this by calling kfifo_out_linear_ptr() in a loop, advancing through
> all contiguous segments until the available hardware TX FIFO space is
> exhausted or the xmit kfifo is empty.
> 
> This fixes the kfifo wrap-around gap independently of the stale-TXLVL
> refill issue.
> 
> Tested on SC16IS752 over SPI driving RS-485 at 115200 baud 8N1 on an
> i.MX6ULL-based board. Oscilloscope measurements confirmed mid-frame
> breaks at the kfifo wrap-around boundary before the fix; no such breaks
> were observed afterward.
> 
> Fixes: 133f4c00b8b2 ("serial: sc16is7xx: fix TX fifo corruption")
> Cc: stable@vger.kernel.org
> Reported-by: Tobias Gannert <tobias.gannert@ziehl-abegg.de>
> Reviewed-by: Joachim Knorr <joachim.knorr@ziehl-abegg.de>
> Link: https://lore.kernel.org/linux-serial/20260623112225.82386-2-paultyson.mbewe@ziehl-abegg.de/
> Signed-off-by: Paul Mbewe <paultyson.mbewe@ziehl-abegg.de>
> ---
> Changes in v2:
>   - Corrected the Modbus RTU timing description for baud rates above 19200
>   - Distinguished the xmit kfifo from the hardware TX FIFO
>   - Clarified that this fixes one independent TX gap source
>   - Corrected the introducing-commit history for direct kfifo access
>   - Removed the max310x comparison
>   - Addressed Hugo Villeneuve's review: clarified that multiple segments
>     are sent, scoped tail to the loop, and added the requested blank line
> 
>  drivers/tty/serial/sc16is7xx.c | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
> index 7107a0fb1e7b..fa7805d2cde2 100644
> --- a/drivers/tty/serial/sc16is7xx.c
> +++ b/drivers/tty/serial/sc16is7xx.c
> @@ -652,7 +652,6 @@ static void sc16is7xx_handle_tx(struct uart_port *port)
>          struct tty_port *tport = &port->state->port;
>          unsigned long flags;
>          unsigned int txlen;
> -       unsigned char *tail;
>  
>          if (unlikely(port->x_char)) {
>                  sc16is7xx_port_write(port, SC16IS7XX_THR_REG, port->x_char);
> @@ -677,9 +676,19 @@ static void sc16is7xx_handle_tx(struct uart_port *port)
>                  txlen = 0;
>          }
>  
> -       txlen = kfifo_out_linear_ptr(&tport->xmit_fifo, &tail, txlen);
> -       sc16is7xx_fifo_write(port, tail, txlen);
> -       uart_xmit_advance(port, txlen);
> +       /* Handle circular buffer wrap-around by sending multiple segments */
> +       while (txlen > 0 && !kfifo_is_empty(&tport->xmit_fifo)) {
> +               unsigned char *tail;
> +               unsigned int to_send;
> +
> +               to_send = kfifo_out_linear_ptr(&tport->xmit_fifo, &tail, txlen);
> +               if (!to_send)
> +                       break;
> +
> +               sc16is7xx_fifo_write(port, tail, to_send);
> +               uart_xmit_advance(port, to_send);
> +               txlen -= to_send;
> +       }
>  
>          uart_port_lock_irqsave(port, &flags);
>          if (kfifo_len(&tport->xmit_fifo) < WAKEUP_CHARS)
> --
> 2.43.0
> 
> _______________________________________ 
> 
> ZIEHL-ABEGG 
> 
> Executive Board: Joachim Ley (Chairman), Marco Altherr, Wolfgang Mayer
> Supervisory Board: Dennis Ziehl (Chairman) 
> 
> Court of Registry: District Court Stuttgart HRB 746188
> Company Seat: K&#xFC;nzelsau, Germany 
> 
> Der Inhalt dieser E-Mail und/oder jegliche Anh&#xE4;nge k&#xF6;nnen vertrauliche Mitteilungen enthalten und sind ausschlie&#xDF;lich f&#xFC;r den bezeichneten Adressaten bestimmt.
> Wenn Sie nicht der vorgesehene Adressat dieser E-Mail oder dessen Vertreter sein sollten, so beachten Sie bitte, dass jede Form der Kenntnisnahme, Ver&#xF6;ffentlichung,
> Vervielf&#xE4;ltigung oder Weitergabe des Inhalts dieser E-Mail einschlie&#xDF;lich Anh&#xE4;ngen unzul&#xE4;ssig ist.
> Wir bitten Sie, sich in diesem Fall mit dem Absender der E-Mail in Verbindung zu setzen. 
> 
> The content of this e-mail and/or attachments may contain confidential information and is intended solely for the named recipient.
> If you are not the intended recipient of this e-mail or on its distribution list, please note that any type of disclosure, publication,
> copying or distribution of the content of this e-mail including attachments is strictly forbidden.
> In this case, we would kindly ask you to notify the sender of the e-mail.

It seems you keep sending your patches with that signature, which you
can't do on public kernel mailing lists, and that is why Greg deleted
your previous patch submission.

-- 
Hugo Villeneuve