Re: [PATCH] serial: sc16is7xx: enable THRI before filling TX FIFO

[Hugo Villeneuve]

Hi Luca,

On Wed, 22 Jul 2026 00:24:04 +0200
Luca Fresi <luca.fresi@xxxxxxxxxxxxx> wrote:

> sc16is7xx_handle_tx() currently requests the THRI enable only after it has
> filled the TX FIFO. The request is asynchronous because the IER update is
> performed later by reg_work.
> 
> The SC16IS7xx generates a THRI interrupt when the TX FIFO crosses its
> trigger level. If the FIFO drains past that level before reg_work enables
> THRI, the chip does not generate a new interrupt. Characters remain queued
> indefinitely even though the hardware FIFO is empty.
> 
> This was observed on an SC16IS752 while both UART channels were active.
> During the stall the software TX buffer remained non-empty while TXLVL
> reported 64 bytes free, LSR reported THR and transmitter empty, IER had
> THRI enabled, and IIR reported no interrupt pending.
> 
> Enable THRI synchronously before filling the FIFO so the threshold crossing
> cannot be missed.
> 
> Fixes: cc4c1d05eb10 ("sc16is7xx: Properly resume TX after stop")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Luca Fresi <luca.fresi@xxxxxxxxxxxxx>
> ---
>  drivers/tty/serial/sc16is7xx.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
> index daebd92f32c7..a1bd175e2feb 100644
> --- a/drivers/tty/serial/sc16is7xx.c
> +++ b/drivers/tty/serial/sc16is7xx.c
> @@ -827,6 +827,9 @@ static void sc16is7xx_tx_proc(struct kthread_work *ws)
>  		msleep(port->rs485.delay_rts_before_send);
>  
>  	guard(mutex)(&one->lock);
> +	sc16is7xx_port_update(port, SC16IS7XX_IER_REG,
> +			      SC16IS7XX_IER_THRI_BIT,
> +			      SC16IS7XX_IER_THRI_BIT);

You can combine the last two lines on a single line.

Also, now that arming is done with your patch in tx_proc(), you can
probably remove in handle_tx():

	else
		sc16is7xx_ier_set(port, SC16IS7XX_IER_THRI_BIT);

since the only other caller of handle_tx() will be
from the port_irq() when THRI fires, so it is already enabled.

Also it seems that for some cases, the THRI will be enabled for nothing
and then immediately disabled, causing useless I2C/SPI bus
transactions...

Also for x_char, empty kfifo or tx_stopped, THRI will be set, as before
it was not.


>  	sc16is7xx_handle_tx(port);
>  }
>  
> -- 
> 2.34.1
> 
> 


-- 
Hugo Villeneuve