[PATCH 0/4] Fix transmit handling on SCI/RSCI ports

[PATCH 0/4] Fix transmit handling on SCI/RSCI ports

[Biju]

From: Biju Das <biju.das.jz@bp.renesas.com>

Hi All,

This series fixes several transmit problems on the SCI (PORT_SCI) and
RSCI ports of the sh-sci driver, seen on Renesas RZ SoCs:

 - Patch 1 removes the code that disables TE at the end of a
   transmission on SCI, which the hardware manual does not describe.
 - Patch 2 stops the TEI handler from turning the transmitter off
   while the xmit kfifo still holds data, which could stall or lose
   output. It also covers the TE handling that patch 1 no longer does
   in sci_transmit_chars().
 - Patch 3 skips the software clear of the TDxE flag on SCI, since
   hardware clears it on a TDR write and the extra clear caused a
   spurious TXI and a duplicated last character.
 - Patch 4 forces a TE toggle when restoring SCSCR in the console
   write path on SCI, so the TXI that restarts an interrupted drain is
   generated. Without it the RZ/G2UL console can hang during boot.

Au Dang (2):
  serial: sh-sci: Skip TDxE status clear for PORT_SCI
  serial: sh-sci: Toggle TE when restoring SCSCR in console write on SCI

Biju Das (2):
  serial: sh-sci: Drop TE disable at end of transmission on SCI
  serial: sh-sci: Don’t disable TX on TEI while the xmit kfifo still
    holds data

 drivers/tty/serial/rsci.c          |  2 ++
 drivers/tty/serial/sh-sci-common.h |  2 ++
 drivers/tty/serial/sh-sci.c        | 43 +++++++++++++++++++-----------
 3 files changed, 32 insertions(+), 15 deletions(-)

-- 
2.43.0

[PATCH 1/4] serial: sh-sci: Drop TE disable at end of transmission on SCI

[Biju]

From: Biju Das <biju.das.jz@bp.renesas.com>

Commit f06c2a9000eb ("tty: serial: sh-sci: Fix end of transmission on
SCI") made sci_transmit_chars() clear SCSCR.TE and return early on
PORT_SCI once the transmit FIFO is empty.

The hardware manual does not describe disabling the transmitter to end
a transmission, so this special case has no basis in the documented
programming sequence. Remove it and let SCI follow the same path as the
other port types. A plain revert no longer applies because the driver
now uses a kfifo, so the code is removed by hand.

Fixes: f06c2a9000eb ("tty: serial: sh-sci: Fix end of transmission on SCI")
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 drivers/tty/serial/sh-sci.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index bd58bbfa65aa..9eb297dbc901 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -1036,13 +1036,6 @@ static void sci_transmit_chars(struct uart_port *port)
 			c = port->x_char;
 			port->x_char = 0;
 		} else if (stopped || !kfifo_get(&tport->xmit_fifo, &c)) {
-			if (s->type == PORT_SCI &&
-			    kfifo_is_empty(&tport->xmit_fifo)) {
-				ctrl = sci_serial_in(port, SCSCR);
-				ctrl &= ~SCSCR_TE;
-				sci_serial_out(port, SCSCR, ctrl);
-				return;
-			}
 			break;
 		}
 
-- 
2.43.0

Re: [PATCH 1/4] serial: sh-sci: Drop TE disable at end of transmission on SCI

[sashiko-bot]

> From: Biju Das <biju.das.jz@bp.renesas.com>
> 
> Commit f06c2a9000eb ("tty: serial: sh-sci: Fix end of transmission on
> SCI") made sci_transmit_chars() clear SCSCR.TE and return early on
> PORT_SCI once the transmit FIFO is empty.
> 
> The hardware manual does not describe disabling the transmitter to end
> a transmission, so this special case has no basis in the documented
> programming sequence. Remove it and let SCI follow the same path as the
> other port types. A plain revert no longer applies because the driver
> now uses a kfifo, so the code is removed by hand.
> 
> Fixes: f06c2a9000eb ("tty: serial: sh-sci: Fix end of transmission on SCI")
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261019170139.473808-1-biju.das.jz@bp.renesas.com?part=1

[PATCH 2/4] serial: sh-sci: Don’t disable TX on TEI while the xmit kfifo still holds data

[Biju]

From: Biju Das <biju.das.jz@bp.renesas.com>

sci_tx_end_interrupt() handles the transmit-end interrupt (TEI) on
SCI and RSCI ports by clearing TE and TEIE, which turns the
transmitter off. It does this unconditionally, even if the xmit kfifo
was refilled after the last byte was written to the data register and
before TEI fired. The queued data is then not sent until the
transmitter is re-enabled, so output stalls or is lost.

Check whether the xmit kfifo is empty before shutting the transmitter
down. If data is still queued, clear TEIE, set TIE and hand over to
sci_tx_interrupt() so transmission continues normally. The existing
TE/TEIE teardown path only runs once the kfifo is drained.

To keep the handler independent of the register layout, add tie and
teie masks to struct sci_port_params_bits and populate them for the
SCI, SCIF and RSCI variants.

Fixes: d61ae331d6f3 ("tty: serial: sh-sci: Add support for tx end interrupt handling")
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 drivers/tty/serial/rsci.c          |  2 ++
 drivers/tty/serial/sh-sci-common.h |  2 ++
 drivers/tty/serial/sh-sci.c        | 22 ++++++++++++++++++----
 3 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/rsci.c b/drivers/tty/serial/rsci.c
index 40db9daa4272..52638db7bbeb 100644
--- a/drivers/tty/serial/rsci.c
+++ b/drivers/tty/serial/rsci.c
@@ -627,6 +627,8 @@ static const struct sci_port_params_bits rsci_port_param_bits = {
 	.rxtx_enable = CCR0_RE | CCR0_TE,
 	.te_clear = CCR0_TE | CCR0_TEIE,
 	.poll_sent_bits = CSR_TDRE | CSR_TEND,
+	.tie = CCR0_TIE,
+	.teie = CCR0_TEIE,
 };
 
 static const struct sci_port_params rsci_rzg3e_port_params = {
diff --git a/drivers/tty/serial/sh-sci-common.h b/drivers/tty/serial/sh-sci-common.h
index 01ff9fced803..273856fbc20b 100644
--- a/drivers/tty/serial/sh-sci-common.h
+++ b/drivers/tty/serial/sh-sci-common.h
@@ -56,6 +56,8 @@ struct sci_port_params_bits {
 	unsigned int rxtx_enable;
 	unsigned int te_clear;
 	unsigned int poll_sent_bits;
+	unsigned int tie;
+	unsigned int teie;
 };
 
 struct sci_common_regs {
diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 9eb297dbc901..d577f9de6259 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -261,13 +261,17 @@ static bool sci_uart_earlycon_dev_probing;
 static const struct sci_port_params_bits sci_sci_port_params_bits = {
 	.rxtx_enable = SCSCR_RE | SCSCR_TE,
 	.te_clear = SCSCR_TE | SCSCR_TEIE,
-	.poll_sent_bits = SCI_TDRE | SCI_TEND
+	.poll_sent_bits = SCI_TDRE | SCI_TEND,
+	.tie = SCSCR_TIE,
+	.teie = SCSCR_TEIE
 };
 
 static const struct sci_port_params_bits sci_scif_port_params_bits = {
 	.rxtx_enable = SCSCR_RE | SCSCR_TE,
 	.te_clear = SCSCR_TE | SCSCR_TEIE,
-	.poll_sent_bits = SCIF_TDFE | SCIF_TEND
+	.poll_sent_bits = SCIF_TDFE | SCIF_TEND,
+	.tie = SCSCR_TIE,
+	.teie = SCSCR_TEIE
 };
 
 static const struct sci_common_regs sci_common_regs = {
@@ -2012,6 +2016,7 @@ static irqreturn_t sci_tx_end_interrupt(int irq, void *ptr)
 {
 	struct uart_port *port = ptr;
 	struct sci_port *s = to_sci_port(port);
+	struct tty_port *tport = &port->state->port;
 	const struct sci_common_regs *regs = s->params->common_regs;
 	unsigned long flags;
 	u32 ctrl;
@@ -2019,9 +2024,18 @@ static irqreturn_t sci_tx_end_interrupt(int irq, void *ptr)
 	if (s->type != PORT_SCI && !sci_is_rsci_type(s->type))
 		return sci_tx_interrupt(irq, ptr);
 
+	ctrl = s->ops->read_reg(port, regs->control);
+	if (!kfifo_is_empty(&tport->xmit_fifo)) {
+		uart_port_lock_irqsave(port, &flags);
+		ctrl &= ~(s->params->param_bits->teie);
+		ctrl |= s->params->param_bits->tie;
+		s->ops->write_reg(port, regs->control, ctrl);
+		uart_port_unlock_irqrestore(port, flags);
+		return sci_tx_interrupt(irq, ptr);
+	}
+
 	uart_port_lock_irqsave(port, &flags);
-	ctrl = s->ops->read_reg(port, regs->control) &
-		~(s->params->param_bits->te_clear);
+	ctrl &= ~(s->params->param_bits->te_clear);
 	s->ops->write_reg(port, regs->control, ctrl);
 	uart_port_unlock_irqrestore(port, flags);
 
-- 
2.43.0

Re: [PATCH 2/4] serial: sh-sci: Don’t disable TX on TEI while the xmit kfifo still holds data

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] serial: sh-sci: race on control reg and xmit_fifo in sci_tx_end_interrupt()
- [Medium] serial: sh-sci: sci_tx_end_interrupt() ignores x_char and uart_tx_stopped()
--

commit c11148ecd71f7b39ea459c7c652a2f9e70cefdac
Author: Biju Das <biju.das.jz@bp.renesas.com>

serial: sh-sci: Don’t disable TX on TEI while the xmit kfifo still holds data

This commit checks whether the transmit kfifo is empty in
sci_tx_end_interrupt() before clearing TE and TEIE on SCI and RSCI ports. If
data remains queued, it clears TEIE, sets TIE, and hands over to
sci_tx_interrupt() so transmission can continue.

> diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
> index 9eb297dbc9010..d577f9de62595 100644
> --- a/drivers/tty/serial/sh-sci.c
> +++ b/drivers/tty/serial/sh-sci.c

[ ... ]

> @@ -2012,6 +2016,7 @@ static irqreturn_t sci_tx_end_interrupt(int irq, void *ptr)
>  {
>  	struct uart_port *port = ptr;
>  	struct sci_port *s = to_sci_port(port);
> +	struct tty_port *tport = &port->state->port;
>  	const struct sci_common_regs *regs = s->params->common_regs;
>  	unsigned long flags;
>  	u32 ctrl;
> @@ -2019,9 +2024,18 @@ static irqreturn_t sci_tx_end_interrupt(int irq, void *ptr)
>  	if (s->type != PORT_SCI && !sci_is_rsci_type(s->type))
>  		return sci_tx_interrupt(irq, ptr);
>  
> +	ctrl = s->ops->read_reg(port, regs->control);
> +	if (!kfifo_is_empty(&tport->xmit_fifo)) {

[Severity: Medium]
What happens if transmission is stopped via flow control or if a high-priority
XON/XOFF character is pending in port->x_char when sci_tx_end_interrupt()
runs?

In sci_transmit_chars() and rsci_transmit_chars(), the transmit loop checks
both port->x_char and uart_tx_stopped(port):

	if (port->x_char) {
		c = port->x_char;
		port->x_char = 0;
	} else if (stopped || !kfifo_get(&tport->xmit_fifo, &c)) {
		break;
	}

If uart_tx_stopped(port) is true while tport->xmit_fifo is non-empty,
sci_tx_end_interrupt() will clear TEIE, set TIE (leaving TE set), and call
sci_tx_interrupt() -> transmit_chars(). Because stopped is true,
transmit_chars() breaks out of the loop without writing to the transmit data
register and without clearing TE or re-enabling TEIE.

On SCI and RSCI, starting transmission from an idle state requires a 0-to-1
transition on TE alongside TIE. Leaving the transmitter idle with TE and TIE
already set means a later uart_start() -> sci_start_tx() or rsci_start_tx()
sees TE already set and cannot trigger a TXI interrupt, wedging transmission.

Conversely, if tport->xmit_fifo is empty but port->x_char is non-zero (queued
via uart_send_xchar()), checking only !kfifo_is_empty(&tport->xmit_fifo) will
shut down TE and TEIE without transmitting port->x_char.

Should sci_tx_end_interrupt() check port->x_char ||
(!kfifo_is_empty(&tport->xmit_fifo) && !uart_tx_stopped(port)) while holding
the port lock?

> +		uart_port_lock_irqsave(port, &flags);
> +		ctrl &= ~(s->params->param_bits->teie);
> +		ctrl |= s->params->param_bits->tie;
> +		s->ops->write_reg(port, regs->control, ctrl);
> +		uart_port_unlock_irqrestore(port, flags);
> +		return sci_tx_interrupt(irq, ptr);
> +	}
> +
>  	uart_port_lock_irqsave(port, &flags);
> -	ctrl = s->ops->read_reg(port, regs->control) &
> -		~(s->params->param_bits->te_clear);
> +	ctrl &= ~(s->params->param_bits->te_clear);
>  	s->ops->write_reg(port, regs->control, ctrl);
>  	uart_port_unlock_irqrestore(port, flags);

[Severity: High]
Can reading regs->control and checking kfifo_is_empty(&tport->xmit_fifo)
before acquiring port->lock race with concurrent updates to the control
register and xmit_fifo?

In the serial core, uart_write() holds port->lock while queuing data into
tport->xmit_fifo and calling __uart_start() -> sci_start_tx() or
rsci_start_tx():

drivers/tty/serial/sh-sci.c:sci_start_tx()
	ctrl = sci_serial_in(port, SCSCR);
	if (s->type == PORT_SCI)
		ctrl |= SCSCR_TE;
	sci_serial_out(port, SCSCR, ctrl | SCSCR_TIE);

drivers/tty/serial/rsci.c:rsci_start_tx()
	ctrl = rsci_serial_in(port, CCR0);
	ctrl |= CCR0_TIE | CCR0_TE;
	rsci_serial_out(port, CCR0, ctrl);

If sci_tx_end_interrupt() on CPU 0 reads regs->control (with TE=1, TEIE=1,
TIE=0) and sees kfifo_is_empty(&tport->xmit_fifo) as true before acquiring
port->lock, CPU 1 can concurrently run uart_write(), enqueue bytes into
tport->xmit_fifo, and call start_tx() to set TE and TIE in regs->control. When
CPU 0 then acquires port->lock and writes back its stale ctrl & ~te_clear, it
overwrites the TIE and TE bits set by CPU 1 and disables the transmitter while
newly queued data remains in tport->xmit_fifo.

Similarly, any other concurrent modification to regs->control under port->lock
(such as sci_stop_tx(), rsci_stop_tx(), sci_start_rx(), sci_stop_rx(),
rsci_stop_rx(), sci_break_ctl(), or rsci_break_ctl()) between the unlocked
read_reg() and write_reg() will be clobbered by the stale ctrl value.

In addition, in the !kfifo_is_empty() branch, port->lock is dropped before
calling sci_tx_interrupt(), which immediately re-acquires port->lock to call
s->ops->transmit_chars(port). Should port->lock be held across the entire
read_reg(), fifo check, write_reg(), and s->ops->transmit_chars(port) sequence?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261019170139.473808-1-biju.das.jz@bp.renesas.com?part=2

Re: [PATCH 2/4] serial: sh-sci: Don’t disable TX on TEI while the xmit kfifo still holds data

[Biju Das]

On Mon, Oct 19, 2026 at 6:17 PM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] serial: sh-sci: race on control reg and xmit_fifo in sci_tx_end_interrupt()
> - [Medium] serial: sh-sci: sci_tx_end_interrupt() ignores x_char and uart_tx_stopped()
> --
>
> commit c11148ecd71f7b39ea459c7c652a2f9e70cefdac
> Author: Biju Das <biju.das.jz@bp.renesas.com>
>
> serial: sh-sci: Don’t disable TX on TEI while the xmit kfifo still holds data
>
> This commit checks whether the transmit kfifo is empty in
> sci_tx_end_interrupt() before clearing TE and TEIE on SCI and RSCI ports. If
> data remains queued, it clears TEIE, sets TIE, and hands over to
> sci_tx_interrupt() so transmission can continue.
>
> > diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
> > index 9eb297dbc9010..d577f9de62595 100644
> > --- a/drivers/tty/serial/sh-sci.c
> > +++ b/drivers/tty/serial/sh-sci.c
>
> [ ... ]
>
> > @@ -2012,6 +2016,7 @@ static irqreturn_t sci_tx_end_interrupt(int irq, void *ptr)
> >  {
> >       struct uart_port *port = ptr;
> >       struct sci_port *s = to_sci_port(port);
> > +     struct tty_port *tport = &port->state->port;
> >       const struct sci_common_regs *regs = s->params->common_regs;
> >       unsigned long flags;
> >       u32 ctrl;
> > @@ -2019,9 +2024,18 @@ static irqreturn_t sci_tx_end_interrupt(int irq, void *ptr)
> >       if (s->type != PORT_SCI && !sci_is_rsci_type(s->type))
> >               return sci_tx_interrupt(irq, ptr);
> >
> > +     ctrl = s->ops->read_reg(port, regs->control);
> > +     if (!kfifo_is_empty(&tport->xmit_fifo)) {
>
> [Severity: Medium]
> What happens if transmission is stopped via flow control or if a high-priority
> XON/XOFF character is pending in port->x_char when sci_tx_end_interrupt()
> runs?
>
> In sci_transmit_chars() and rsci_transmit_chars(), the transmit loop checks
> both port->x_char and uart_tx_stopped(port):
>
>         if (port->x_char) {
>                 c = port->x_char;
>                 port->x_char = 0;
>         } else if (stopped || !kfifo_get(&tport->xmit_fifo, &c)) {
>                 break;
>         }
>
> If uart_tx_stopped(port) is true while tport->xmit_fifo is non-empty,
> sci_tx_end_interrupt() will clear TEIE, set TIE (leaving TE set), and call
> sci_tx_interrupt() -> transmit_chars(). Because stopped is true,
> transmit_chars() breaks out of the loop without writing to the transmit data
> register and without clearing TE or re-enabling TEIE.
>
> On SCI and RSCI, starting transmission from an idle state requires a 0-to-1
> transition on TE alongside TIE. Leaving the transmitter idle with TE and TIE
> already set means a later uart_start() -> sci_start_tx() or rsci_start_tx()
> sees TE already set and cannot trigger a TXI interrupt, wedging transmission.
>
> Conversely, if tport->xmit_fifo is empty but port->x_char is non-zero (queued
> via uart_send_xchar()), checking only !kfifo_is_empty(&tport->xmit_fifo) will
> shut down TE and TEIE without transmitting port->x_char.
>
> Should sci_tx_end_interrupt() check port->x_char ||
> (!kfifo_is_empty(&tport->xmit_fifo) && !uart_tx_stopped(port)) while holding
> the port lock?
>
> > +             uart_port_lock_irqsave(port, &flags);
> > +             ctrl &= ~(s->params->param_bits->teie);
> > +             ctrl |= s->params->param_bits->tie;
> > +             s->ops->write_reg(port, regs->control, ctrl);
> > +             uart_port_unlock_irqrestore(port, flags);
> > +             return sci_tx_interrupt(irq, ptr);
> > +     }
> > +
> >       uart_port_lock_irqsave(port, &flags);
> > -     ctrl = s->ops->read_reg(port, regs->control) &
> > -             ~(s->params->param_bits->te_clear);
> > +     ctrl &= ~(s->params->param_bits->te_clear);
> >       s->ops->write_reg(port, regs->control, ctrl);
> >       uart_port_unlock_irqrestore(port, flags);
>
> [Severity: High]
> Can reading regs->control and checking kfifo_is_empty(&tport->xmit_fifo)
> before acquiring port->lock race with concurrent updates to the control
> register and xmit_fifo?
>
> In the serial core, uart_write() holds port->lock while queuing data into
> tport->xmit_fifo and calling __uart_start() -> sci_start_tx() or
> rsci_start_tx():
>
> drivers/tty/serial/sh-sci.c:sci_start_tx()
>         ctrl = sci_serial_in(port, SCSCR);
>         if (s->type == PORT_SCI)
>                 ctrl |= SCSCR_TE;
>         sci_serial_out(port, SCSCR, ctrl | SCSCR_TIE);
>
> drivers/tty/serial/rsci.c:rsci_start_tx()
>         ctrl = rsci_serial_in(port, CCR0);
>         ctrl |= CCR0_TIE | CCR0_TE;
>         rsci_serial_out(port, CCR0, ctrl);
>
> If sci_tx_end_interrupt() on CPU 0 reads regs->control (with TE=1, TEIE=1,
> TIE=0) and sees kfifo_is_empty(&tport->xmit_fifo) as true before acquiring
> port->lock, CPU 1 can concurrently run uart_write(), enqueue bytes into
> tport->xmit_fifo, and call start_tx() to set TE and TIE in regs->control. When
> CPU 0 then acquires port->lock and writes back its stale ctrl & ~te_clear, it
> overwrites the TIE and TE bits set by CPU 1 and disables the transmitter while
> newly queued data remains in tport->xmit_fifo.
>
> Similarly, any other concurrent modification to regs->control under port->lock
> (such as sci_stop_tx(), rsci_stop_tx(), sci_start_rx(), sci_stop_rx(),
> rsci_stop_rx(), sci_break_ctl(), or rsci_break_ctl()) between the unlocked
> read_reg() and write_reg() will be clobbered by the stale ctrl value.
>
> In addition, in the !kfifo_is_empty() branch, port->lock is dropped before
> calling sci_tx_interrupt(), which immediately re-acquires port->lock to call
> s->ops->transmit_chars(port). Should port->lock be held across the entire
> read_reg(), fifo check, write_reg(), and s->ops->transmit_chars(port) sequence?

Ok, will fix the code as

uart_port_lock_irqsave(port, &flags);
ctrl = s->ops->read_reg(port, regs->control);
if (port->x_char || (!kfifo_is_empty(&tport->xmit_fifo) &&
!uart_tx_stopped(port))) {
.....
uart_port_unlock_irqrestore(port, flags);
 return IRQ_HANDLED;
 }
....
 uart_port_unlock_irqrestore(port, flags);
return IRQ_HANDLED;
}

Cheers,
Biju

[PATCH 3/4] serial: sh-sci: Skip TDxE status clear for PORT_SCI

[Biju]

From: Au Dang <au.dang.jx@renesas.com>

On PORT_SCI, the TDxE (Transmit Data Empty) flag in SCxSR is cleared
automatically by hardware when data is written to TDR. Clearing it
again from software via clear_SCxSR() generates a spurious TXI
interrupt, which causes the last character to be transmitted twice.

Skip the clear_SCxSR() call for PORT_SCI in sci_transmit_chars() and
sci_poll_put_char(), as the hardware already handles it after each TDR
write.

Signed-off-by: Au Dang <au.dang.jx@renesas.com>
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 drivers/tty/serial/sh-sci.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index d577f9de6259..e4c7a3e14688 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -913,7 +913,8 @@ static void sci_poll_put_char(struct uart_port *port, unsigned char c)
 	} while (!(status & SCxSR_TDxE(port)));
 
 	sci_serial_out(port, SCxTDR, c);
-	s->ops->clear_SCxSR(port, SCxSR_TDxE_CLEAR(port) & ~SCxSR_TEND(port));
+	if (s->type != PORT_SCI)
+		s->ops->clear_SCxSR(port, SCxSR_TDxE_CLEAR(port) & ~SCxSR_TEND(port));
 }
 #endif /* CONFIG_CONSOLE_POLL || CONFIG_SERIAL_SH_SCI_CONSOLE ||
 	  CONFIG_SERIAL_SH_SCI_EARLYCON */
@@ -1049,7 +1050,8 @@ static void sci_transmit_chars(struct uart_port *port)
 		port->icount.tx++;
 	} while (--count > 0);
 
-	s->ops->clear_SCxSR(port, SCxSR_TDxE_CLEAR(port));
+	if (s->type != PORT_SCI)
+		s->ops->clear_SCxSR(port, SCxSR_TDxE_CLEAR(port));
 
 	if (kfifo_len(&tport->xmit_fifo) < WAKEUP_CHARS)
 		uart_write_wakeup(port);
-- 
2.43.0

Re: [PATCH 3/4] serial: sh-sci: Skip TDxE status clear for PORT_SCI

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] serial: sh-sci: broken TX on SuperH PORT_SCI from skipping clear_SCxSR()
- [Medium] serial: sh-sci: unlocked control register RMW in sci_tx_end_interrupt()
--

commit c9eeab42d1c45b13adb330fd8baab03f6ddbfeb6
Author: Au Dang <au.dang.jx@renesas.com>

serial: sh-sci: Skip TDxE status clear for PORT_SCI

This commit skips calling clear_SCxSR() to clear the TDxE status bit for
PORT_SCI in sci_poll_put_char() and sci_transmit_chars(), relying on hardware
to automatically clear the bit when data is written to TDR.

> diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
> index d577f9de62595..e4c7a3e146884 100644
> --- a/drivers/tty/serial/sh-sci.c
> +++ b/drivers/tty/serial/sh-sci.c
> @@ -913,7 +913,8 @@ static void sci_poll_put_char(struct uart_port *port, unsigned char c)
>  	} while (!(status & SCxSR_TDxE(port)));
>  
>  	sci_serial_out(port, SCxTDR, c);
> -	s->ops->clear_SCxSR(port, SCxSR_TDxE_CLEAR(port) & ~SCxSR_TEND(port));
> +	if (s->type != PORT_SCI)
> +		s->ops->clear_SCxSR(port, SCxSR_TDxE_CLEAR(port) & ~SCxSR_TEND(port));
>  }
>  #endif /* CONFIG_CONSOLE_POLL || CONFIG_SERIAL_SH_SCI_CONSOLE ||
>  	  CONFIG_SERIAL_SH_SCI_EARLYCON */

[Severity: High]
Will skipping clear_SCxSR() for all PORT_SCI ports break transmission on
classic SuperH SCI hardware?

In drivers/tty/serial/sh-sci.c, PORT_SCI is shared between DT-based Renesas
RZ/G2L SCIg ports (such as renesas,r9a07g044-sci matching renesas,sci) and
classic non-DT SuperH SCI ports (SH770x, SH7750, and SH7760 registered via
plat_sci_port with .type = PORT_SCI).

While RZ/G2L SCIg hardware automatically clears SSR.TDRE when transmit data is
written to TDR, classic SuperH SCI hardware does not clear SCSSR.TDRE (or
TEND) on CPU writes to SCTDR. On classic SuperH SCI, software must read SCSSR
with TDRE == 1, write the byte to SCTDR, and then explicitly clear TDRE to 0
via clear_SCxSR() before the hardware transfers SCTDR to the transmit shift
register (SCTSR) and begins shifting.

With this check in both sci_poll_put_char() and sci_transmit_chars(),
SCSSR.TDRE is never cleared on classic SuperH PORT_SCI systems, preventing
SCTDR from transferring to SCTSR in interrupt-driven transmission and causing
sci_poll_put_char() to see TDRE remain 1 and overwrite SCTDR without shifting
characters out.

Notice also that sci_poll_put_char() and sci_transmit_chars() are the only
callers of SCxSR_TDxE_CLEAR(port):

#define SCxSR_TDxE_CLEAR(port) \
	(((port)->type == PORT_SCI) ? SCI_TDxE_CLEAR : SCIF_TDxE_CLEAR)

Skipping both calls when s->type == PORT_SCI leaves SCI_TDxE_CLEAR and the
PORT_SCI branch in SCxSR_TDxE_CLEAR() completely unreachable.

> @@ -1049,7 +1050,8 @@ static void sci_transmit_chars(struct uart_port *port)
>  		port->icount.tx++;
>  	} while (--count > 0);
>  
> -	s->ops->clear_SCxSR(port, SCxSR_TDxE_CLEAR(port));
> +	if (s->type != PORT_SCI)
> +		s->ops->clear_SCxSR(port, SCxSR_TDxE_CLEAR(port));
>  
>  	if (kfifo_len(&tport->xmit_fifo) < WAKEUP_CHARS)
>  		uart_write_wakeup(port);

[Severity: Medium]
Now that PORT_SCI avoids the extra TXI interrupt and transitions directly to
TEIE-driven completion when xmit_fifo becomes empty in sci_transmit_chars():

	if (kfifo_is_empty(&tport->xmit_fifo)) {
		if (s->type == PORT_SCI) {
			ctrl = sci_serial_in(port, SCSCR);
			ctrl &= ~SCSCR_TIE;
			ctrl |= SCSCR_TEIE;
			sci_serial_out(port, SCSCR, ctrl);
		}

		sci_stop_tx(port);
	}

can the unlocked control register read in sci_tx_end_interrupt() race with a
concurrent sci_start_tx() and stall transmission?

In drivers/tty/serial/sh-sci.c:sci_tx_end_interrupt(), the control register
and xmit_fifo are read before acquiring port->lock:

	ctrl = s->ops->read_reg(port, regs->control);
	if (!kfifo_is_empty(&tport->xmit_fifo)) {
		uart_port_lock_irqsave(port, &flags);
		ctrl &= ~(s->params->param_bits->teie);
		ctrl |= s->params->param_bits->tie;
		s->ops->write_reg(port, regs->control, ctrl);
		uart_port_unlock_irqrestore(port, flags);
		return sci_tx_interrupt(irq, ptr);
	}

	uart_port_lock_irqsave(port, &flags);
	ctrl &= ~(s->params->param_bits->te_clear);
	s->ops->write_reg(port, regs->control, ctrl);
	uart_port_unlock_irqrestore(port, flags);

If CPU A enters sci_tx_end_interrupt(), reads regs->control (with TIE == 0),
and sees kfifo_is_empty(&tport->xmit_fifo) == true before acquiring
port->lock, CPU B can concurrently run uart_write() -> __uart_start() ->
sci_start_tx() under port->lock, queueing new bytes into xmit_fifo and setting
SCSCR_TE | SCSCR_TIE in regs->control.

When CPU A then acquires port->lock and writes back its stale ctrl value with
te_clear masked off, it overwrites the TIE and TE bits just enabled by
sci_start_tx(), leaving the newly queued bytes stuck in xmit_fifo. Similarly,
if kfifo_is_empty() is false, reading ctrl outside port->lock can clobber
concurrent control register updates made under port->lock by sci_start_rx() or
sci_stop_rx().

Should sci_tx_end_interrupt() acquire port->lock before reading regs->control
and checking kfifo_is_empty(&tport->xmit_fifo)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261019170139.473808-1-biju.das.jz@bp.renesas.com?part=3

Re: [PATCH 3/4] serial: sh-sci: Skip TDxE status clear for PORT_SCI

[Biju Das]

On Mon, Oct 19, 2026 at 6:19 PM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] serial: sh-sci: broken TX on SuperH PORT_SCI from skipping clear_SCxSR()
> - [Medium] serial: sh-sci: unlocked control register RMW in sci_tx_end_interrupt()
> --
>
> commit c9eeab42d1c45b13adb330fd8baab03f6ddbfeb6
> Author: Au Dang <au.dang.jx@renesas.com>
>
> serial: sh-sci: Skip TDxE status clear for PORT_SCI
>
> This commit skips calling clear_SCxSR() to clear the TDxE status bit for
> PORT_SCI in sci_poll_put_char() and sci_transmit_chars(), relying on hardware
> to automatically clear the bit when data is written to TDR.
>
> > diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
> > index d577f9de62595..e4c7a3e146884 100644
> > --- a/drivers/tty/serial/sh-sci.c
> > +++ b/drivers/tty/serial/sh-sci.c
> > @@ -913,7 +913,8 @@ static void sci_poll_put_char(struct uart_port *port, unsigned char c)
> >       } while (!(status & SCxSR_TDxE(port)));
> >
> >       sci_serial_out(port, SCxTDR, c);
> > -     s->ops->clear_SCxSR(port, SCxSR_TDxE_CLEAR(port) & ~SCxSR_TEND(port));
> > +     if (s->type != PORT_SCI)
> > +             s->ops->clear_SCxSR(port, SCxSR_TDxE_CLEAR(port) & ~SCxSR_TEND(port));
> >  }
> >  #endif /* CONFIG_CONSOLE_POLL || CONFIG_SERIAL_SH_SCI_CONSOLE ||
> >         CONFIG_SERIAL_SH_SCI_EARLYCON */
>
> [Severity: High]
> Will skipping clear_SCxSR() for all PORT_SCI ports break transmission on
> classic SuperH SCI hardware?
>
> In drivers/tty/serial/sh-sci.c, PORT_SCI is shared between DT-based Renesas
> RZ/G2L SCIg ports (such as renesas,r9a07g044-sci matching renesas,sci) and
> classic non-DT SuperH SCI ports (SH770x, SH7750, and SH7760 registered via
> plat_sci_port with .type = PORT_SCI).
>
> While RZ/G2L SCIg hardware automatically clears SSR.TDRE when transmit data is
> written to TDR, classic SuperH SCI hardware does not clear SCSSR.TDRE (or
> TEND) on CPU writes to SCTDR. On classic SuperH SCI, software must read SCSSR
> with TDRE == 1, write the byte to SCTDR, and then explicitly clear TDRE to 0
> via clear_SCxSR() before the hardware transfers SCTDR to the transmit shift
> register (SCTSR) and begins shifting.

I will introduce tdxe_autoclear to handle this for RZ/G2UL based SCI IPs


Cheers,
Biju

Re: [PATCH 3/4] serial: sh-sci: Skip TDxE status clear for PORT_SCI

[Geert Uytterhoeven]

Hi Biju,

On Mon, 19 Oct 2026 at 19:01, Biju <biju.das.au@gmail.com> wrote:
> From: Au Dang <au.dang.jx@renesas.com>
>
> On PORT_SCI, the TDxE (Transmit Data Empty) flag in SCxSR is cleared
> automatically by hardware when data is written to TDR. Clearing it
> again from software via clear_SCxSR() generates a spurious TXI
> interrupt, which causes the last character to be transmitted twice.
>
> Skip the clear_SCxSR() call for PORT_SCI in sci_transmit_chars() and
> sci_poll_put_char(), as the hardware already handles it after each TDR
> write.
>
> Signed-off-by: Au Dang <au.dang.jx@renesas.com>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

Thanks for your patch!

> --- a/drivers/tty/serial/sh-sci.c
> +++ b/drivers/tty/serial/sh-sci.c
> @@ -913,7 +913,8 @@ static void sci_poll_put_char(struct uart_port *port, unsigned char c)
>         } while (!(status & SCxSR_TDxE(port)));
>
>         sci_serial_out(port, SCxTDR, c);
> -       s->ops->clear_SCxSR(port, SCxSR_TDxE_CLEAR(port) & ~SCxSR_TEND(port));
> +       if (s->type != PORT_SCI)
> +               s->ops->clear_SCxSR(port, SCxSR_TDxE_CLEAR(port) & ~SCxSR_TEND(port));

Is this issue present on both classic SCI and RZ RSCI?
s->ops->clear_SCxSR() points to either sci_clear_SCxSR() or
rsci_clear_CFC(). The former already has an internal check for
PORT_SCI, the latter has not. So probably it should be handled there.

>  }
>  #endif /* CONFIG_CONSOLE_POLL || CONFIG_SERIAL_SH_SCI_CONSOLE ||
>           CONFIG_SERIAL_SH_SCI_EARLYCON */
> @@ -1049,7 +1050,8 @@ static void sci_transmit_chars(struct uart_port *port)
>                 port->icount.tx++;
>         } while (--count > 0);
>
> -       s->ops->clear_SCxSR(port, SCxSR_TDxE_CLEAR(port));
> +       if (s->type != PORT_SCI)
> +               s->ops->clear_SCxSR(port, SCxSR_TDxE_CLEAR(port));
>
>         if (kfifo_len(&tport->xmit_fifo) < WAKEUP_CHARS)
>                 uart_write_wakeup(port);

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

RE: [PATCH 3/4] serial: sh-sci: Skip TDxE status clear for PORT_SCI

[Biju Das]

Hi Geert,

Thanks for the feedback.

> -----Original Message-----
> From: Geert Uytterhoeven <geert@linux-m68k.org>
> Sent: 20 October 2026 09:36
> Subject: Re: [PATCH 3/4] serial: sh-sci: Skip TDxE status clear for PORT_SCI
> 
> Hi Biju,
> 
> On Mon, 19 Oct 2026 at 19:01, Biju <biju.das.au@gmail.com> wrote:
> > From: Au Dang <au.dang.jx@renesas.com>
> >
> > On PORT_SCI, the TDxE (Transmit Data Empty) flag in SCxSR is cleared
> > automatically by hardware when data is written to TDR. Clearing it
> > again from software via clear_SCxSR() generates a spurious TXI
> > interrupt, which causes the last character to be transmitted twice.
> >
> > Skip the clear_SCxSR() call for PORT_SCI in sci_transmit_chars() and
> > sci_poll_put_char(), as the hardware already handles it after each TDR
> > write.
> >
> > Signed-off-by: Au Dang <au.dang.jx@renesas.com>
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> 
> Thanks for your patch!
> 
> > --- a/drivers/tty/serial/sh-sci.c
> > +++ b/drivers/tty/serial/sh-sci.c
> > @@ -913,7 +913,8 @@ static void sci_poll_put_char(struct uart_port *port, unsigned char c)
> >         } while (!(status & SCxSR_TDxE(port)));
> >
> >         sci_serial_out(port, SCxTDR, c);
> > -       s->ops->clear_SCxSR(port, SCxSR_TDxE_CLEAR(port) & ~SCxSR_TEND(port));
> > +       if (s->type != PORT_SCI)
> > +               s->ops->clear_SCxSR(port, SCxSR_TDxE_CLEAR(port) &
> > + ~SCxSR_TEND(port));
> 
> Is this issue present on both classic SCI and RZ RSCI?
> s->ops->clear_SCxSR() points to either sci_clear_SCxSR() or
> rsci_clear_CFC(). The former already has an internal check for PORT_SCI, the latter has not. So probably
> it should be handled there.

It is applicable for RZ/G2UL based SCI as for TDRE and the hardware team confirmed
it will be auto cleared by the hardware.

On RSCI, we needed to explicitly clear it and it is handled in
rsci_transmit_chars()/ rsci_poll_put_char()

Cheers,
Biju

> 
> >  }
> >  #endif /* CONFIG_CONSOLE_POLL || CONFIG_SERIAL_SH_SCI_CONSOLE ||
> >           CONFIG_SERIAL_SH_SCI_EARLYCON */ @@ -1049,7 +1050,8 @@
> > static void sci_transmit_chars(struct uart_port *port)
> >                 port->icount.tx++;
> >         } while (--count > 0);
> >
> > -       s->ops->clear_SCxSR(port, SCxSR_TDxE_CLEAR(port));
> > +       if (s->type != PORT_SCI)
> > +               s->ops->clear_SCxSR(port, SCxSR_TDxE_CLEAR(port));
> >
> >         if (kfifo_len(&tport->xmit_fifo) < WAKEUP_CHARS)
> >                 uart_write_wakeup(port);
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But when I'm talking to
> journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

[PATCH 4/4] serial: sh-sci: Toggle TE when restoring SCSCR in console write on SCI

[Biju]

From: Au Dang <au.dang.jx@renesas.com>

When SCI0 (PORT_SCI) is the system console on RZ/G2UL, the board can
hang at the serial console output stage during boot.

ftrace shows the following sequence:

1. sci_transmit_chars() is draining the TX kfifo through TXI
   interrupts with SCSCR=0xf0 (TE=1, TIE=1, RE=1, RIE=1).

2. serial_console_write() runs in the middle of that drain. It saves
   SCSCR=0xf0, switches to polling by writing SCSCR=0x30 (TE=1, TIE=0)
   and outputs the characters with sci_poll_put_char().

3. It then restores SCSCR=0xf0 in a single write. TE is 1 both before
   and after, so there is no TE transition.

4. sci_start_tx() is then called repeatedly with SCSCR unchanged, but
   no TXI is ever raised again, so the drain never resumes and the
   console hangs.

The RZ/G2UL User's Manual (section 23.3.6, "SCI Initialization") notes
that changing SCR.TE from 1 to 0 or from 0 to 1 while SCR.TIE is 1
generates a TXI interrupt request. Without a TE transition, the TXI
that would restart the drain is never generated.

For PORT_SCI, restore SCSCR in two steps: first with TE cleared, then
with the full saved value. This forces the TE 1->0->1 transition while
TIE is being re-enabled, so TXI fires and the interrupted drain
resumes.

Signed-off-by: Au Dang <au.dang.jx@renesas.com>
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 drivers/tty/serial/sh-sci.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index e4c7a3e14688..94a34dccfe56 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -3445,10 +3445,14 @@ static void serial_console_write(struct console *co, const char *s,
 		cpu_relax();
 
 	/* restore the SCSCR */
-	if (sci_port->ops->finish_console_write)
+	if (sci_port->ops->finish_console_write) {
 		sci_port->ops->finish_console_write(port, ctrl);
-	else
+	} else {
+		if (to_sci_port(port)->type == PORT_SCI)
+			sci_port->ops->write_reg(port, regs->control, ctrl & ~SCSCR_TE);
+
 		sci_port->ops->write_reg(port, regs->control, ctrl);
+	}
 
 	if (locked)
 		uart_port_unlock_irqrestore(port, flags);
-- 
2.43.0

Re: [PATCH 4/4] serial: sh-sci: Toggle TE when restoring SCSCR in console write on SCI

[sashiko-bot]

> From: Au Dang <au.dang.jx@renesas.com>
> 
> When SCI0 (PORT_SCI) is the system console on RZ/G2UL, the board can
> hang at the serial console output stage during boot.
> 
> ftrace shows the following sequence:
> 
> 1. sci_transmit_chars() is draining the TX kfifo through TXI
>    interrupts with SCSCR=0xf0 (TE=1, TIE=1, RE=1, RIE=1).
> 
> 2. serial_console_write() runs in the middle of that drain. It saves
>    SCSCR=0xf0, switches to polling by writing SCSCR=0x30 (TE=1, TIE=0)
>    and outputs the characters with sci_poll_put_char().
> 
> 3. It then restores SCSCR=0xf0 in a single write. TE is 1 both before
> [ ... ]
> TIE is being re-enabled, so TXI fires and the interrupted drain
> resumes.
> 
> Signed-off-by: Au Dang <au.dang.jx@renesas.com>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261019170139.473808-1-biju.das.jz@bp.renesas.com?part=4