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

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

[Diogo Ivo]

Hi all,

This is v4 of the SCI/RSCI transmit fixes originally posted by Biju
Das. Patches 1, 2, 5 and 6 retain the v3 code and descriptions from
Biju and Au Dang. Patches 3, 4 and 7 incorporate additional RSCI fixes
found in the meantime. This revision supersedes the v3 series [1].

The series addresses transmit stalls, duplicated output and spurious
interrupts on the SCI (PORT_SCI) and RSCI ports of the sh-sci driver:

 - 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 enables TEIE in rsci_stop_tx() when TE is set, allowing
   transmit-end handling to clear TE after the hardware FIFO drains.
 - Patch 4 avoids kicking RSCI transmission from the receive-error
   handler, which could otherwise cause spurious TEI interrupts or
   leave the transmitter in a state that stalls later output.
 - Patch 5 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 6 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.
 - Patch 7 simplifies rsci_transmit_chars() now that the receive-error
   handler no longer invokes it, and reuses rsci_stop_tx().

OpenCode (GPT-6.1 Sol) assisted with reviewing the previous discussion,
preparing and reordering this series, updating trailers and recipients,
and drafting this cover letter, as well as helping debug the various
edge cases and proposing code changes.

[1] https://lore.kernel.org/all/20261026-g2ul_sci-v3-0-b3097163edd5@bp.renesas.com/

Changes in v4:
 * Trim unrelated x86/TDX recipients identified in the v3 discussion.
 * Keep the code and descriptions of the four v3 patches unchanged.
 * Add patch 3 to enable transmit-end interrupts in rsci_stop_tx().
 * Add patch 4 to avoid kicking RSCI TX from the receive-error handler.
 * Add patch 7 to simplify rsci_transmit_chars().
 * Group patches with Fixes tags first (patches 1-4) and mark them for
   stable.
 * Base the combined series on next-20261019.
- Link to v3: https://lore.kernel.org/all/20261026-g2ul_sci-v3-0-b3097163edd5@bp.renesas.com/

Changes in v3:
 * Updated commit description for patch#2
 * When the port is stopped with data still queued, the transmit_chars()
   paths (sci_transmit_chars() and rsci_transmit_chars()) now also swap
   TIE for TEIE, as they already did for an empty kfifo. Without this, TIE
   would keep firing with nothing the driver is allowed to send, and TEI
   would never be raised to complete the transmission.
 * Replaced sci_tx_interrupt() with port's transmit_chars() to avoid
   lock drop race in sci_tx_end_interrupt() before TX handoff.
 * Added missing has_tdxe_auto_clear init in scix_early_console_setup().
 * Replaced to_sci_port(port)->type with sci_port->type in
   serial_console_write().
- Link to v2: https://lore.kernel.org/r/20261021-g2ul_sci-v2-0-b4e038220018@bp.renesas.com

Changes in v2:
 * Updated commit header and description for patch#2 and #3.
 * Take the port lock before reading the control register, so the
   read-modify-write in sci_tx_end_interrupt() is protected.
 * Extended the pending-data check in sci_tx_end_interrupt() to cover
   x_char and a stopped port, not only a non-empty xmit kfifo.
 * Replaced the PORT_SCI check with a has_tdxe_auto_clear flag in
   struct sci_port, set from the new tdxe_auto_clear field in
   struct sci_of_data.
- Link to v1: https://lore.kernel.org/r/20261019170139.473808-1-biju.das.jz@bp.renesas.com

---
Au Dang (2):
      serial: sh-sci: Skip TDxE clear when hardware clears it automatically
      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: Keep TX enabled on TEI while data is pending

Diogo Ivo (3):
      serial: rsci: Enable transmit end interrupt in rsci_stop_tx()
      serial: sh-sci: Do not kick TX from sci_er_interrupt() on RSCI
      serial: rsci: Simplify rsci_transmit_chars()

 drivers/tty/serial/rsci.c          | 23 +++++-------
 drivers/tty/serial/sh-sci-common.h |  4 +++
 drivers/tty/serial/sh-sci.c        | 73 ++++++++++++++++++++++++++------------
 3 files changed, 62 insertions(+), 38 deletions(-)
---
base-commit: 3d229ce89862b8e82d71a9e11d6f840ca272a6d6
change-id: 20261020-g2ul_sci-b610606ea5b5

Best regards,
--  
Diogo Ivo <diogo.ivo.sx@renesas.com>

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

[Diogo Ivo]

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")
Cc: stable@vger.kernel.org
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Signed-off-by: Diogo Ivo <diogo.ivo.sx@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.56.0

[PATCH v4 2/7] serial: sh-sci: Keep TX enabled on TEI while data is pending

[Diogo Ivo]

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, or an x_char was queued, after the last byte was written
to the data register and before TEI fired. The pending data is then not
sent until the transmitter is re-enabled, so output stalls or is lost.

Check for pending data (x_char, or a non-empty xmit kfifo on a port
that is not stopped) before shutting the transmitter down. If there is
any, clear TEIE, set TIE and hand over to the port's transmit_chars()
op so transmission continues normally. The existing TE/TEIE teardown
path only runs once nothing is left to send.

When the port is stopped with data still queued, the transmit_chars()
paths (sci_transmit_chars() and rsci_transmit_chars()) now also swap
TIE for TEIE, as they already did for an empty kfifo. Without this, TIE
would keep firing with nothing the driver is allowed to send, and TEI
would never be raised to complete the transmission.

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")
Cc: stable@vger.kernel.org
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Signed-off-by: Diogo Ivo <diogo.ivo.sx@renesas.com>
---
 drivers/tty/serial/rsci.c          |  4 +++-
 drivers/tty/serial/sh-sci-common.h |  2 ++
 drivers/tty/serial/sh-sci.c        | 47 +++++++++++++++++++++++++++++---------
 3 files changed, 41 insertions(+), 12 deletions(-)

diff --git a/drivers/tty/serial/rsci.c b/drivers/tty/serial/rsci.c
index 40db9daa4272..0391111db8ae 100644
--- a/drivers/tty/serial/rsci.c
+++ b/drivers/tty/serial/rsci.c
@@ -465,7 +465,7 @@ static void rsci_transmit_chars(struct uart_port *port)
 	if (kfifo_len(&tport->xmit_fifo) < WAKEUP_CHARS)
 		uart_write_wakeup(port);
 
-	if (kfifo_is_empty(&tport->xmit_fifo)) {
+	if (kfifo_is_empty(&tport->xmit_fifo) || stopped) {
 		ctrl = rsci_serial_in(port, CCR0);
 		ctrl &= ~CCR0_TIE;
 		ctrl |= CCR0_TEIE;
@@ -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..6df25fef5a5f 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 = {
@@ -1007,6 +1011,20 @@ static int sci_rxfill(struct uart_port *port)
  *                   the interrupt related routines                       *
  * ********************************************************************** */
 
+static void sci_clr_tie_set_teie(struct uart_port *port)
+{
+	struct sci_port *s = to_sci_port(port);
+	unsigned short ctrl;
+
+	if (s->type != PORT_SCI)
+		return;
+
+	ctrl = sci_serial_in(port, SCSCR);
+	ctrl &= ~SCSCR_TIE;
+	ctrl |= SCSCR_TEIE;
+	sci_serial_out(port, SCSCR, ctrl);
+}
+
 static void sci_transmit_chars(struct uart_port *port)
 {
 	struct tty_port *tport = &port->state->port;
@@ -1050,14 +1068,10 @@ static void sci_transmit_chars(struct uart_port *port)
 	if (kfifo_len(&tport->xmit_fifo) < WAKEUP_CHARS)
 		uart_write_wakeup(port);
 	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_clr_tie_set_teie(port);
 		sci_stop_tx(port);
+	} else if (stopped) {
+		sci_clr_tie_set_teie(port);
 	}
 }
 
@@ -2012,6 +2026,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;
@@ -2020,8 +2035,18 @@ static irqreturn_t sci_tx_end_interrupt(int irq, void *ptr)
 		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->ops->read_reg(port, regs->control);
+	if (port->x_char || (!kfifo_is_empty(&tport->xmit_fifo) &&
+			     !uart_tx_stopped(port))) {
+		ctrl &= ~(s->params->param_bits->teie);
+		ctrl |= s->params->param_bits->tie;
+		s->ops->write_reg(port, regs->control, ctrl);
+		s->ops->transmit_chars(port);
+		uart_port_unlock_irqrestore(port, flags);
+		return IRQ_HANDLED;
+	}
+
+	ctrl &= ~(s->params->param_bits->te_clear);
 	s->ops->write_reg(port, regs->control, ctrl);
 	uart_port_unlock_irqrestore(port, flags);
 

-- 
2.56.0

[PATCH v4 3/7] serial: rsci: Enable transmit end interrupt in rsci_stop_tx()

[Diogo Ivo]

Currently, rsci_stop_tx() clears CCR0.TIE without enabling CCR0.TEIE.
If transmission is stopped while CCR0.TE is set (for example, via
TCOOFF), the hardware FIFO drains and sets CSR.TDRE while CCR0.TIE is 0,
and no TEI (Transmit End) interrupt is generated to clear CCR0.TE.

However, on RSCI setting TIE while TE is already set does not generate a
TXI interrupt. Thus, if transmission is later resumed via rsci_start_tx(),
setting CCR0.TIE while CCR0.TE and CSR.TDRE are already 1 fails to call
into rsci_transmit_chars(), permanently stalling subsequent transmissions.

Fix this by setting CCR0.TEIE in rsci_stop_tx() whenever CCR0.TE is
set, ensuring sci_tx_end_interrupt() fires once the hardware finishes
shifting out in-flight data and clears CCR0.TE.

Fixes: 0666e3fe95ab ("serial: sh-sci: Add support for RZ/T2H SCI")
Cc: stable@vger.kernel.org
Signed-off-by: Diogo Ivo <diogo.ivo.sx@renesas.com>
---
 drivers/tty/serial/rsci.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/tty/serial/rsci.c b/drivers/tty/serial/rsci.c
index 0391111db8ae..05703bb31afd 100644
--- a/drivers/tty/serial/rsci.c
+++ b/drivers/tty/serial/rsci.c
@@ -409,6 +409,8 @@ static void rsci_stop_tx(struct uart_port *port)
 
 	ctrl = rsci_serial_in(port, CCR0);
 	ctrl &= ~CCR0_TIE;
+	if (ctrl & CCR0_TE)
+		ctrl |= CCR0_TEIE;
 	rsci_serial_out(port, CCR0, ctrl);
 }
 

-- 
2.56.0

[PATCH v4 4/7] serial: sh-sci: Do not kick TX from sci_er_interrupt() on RSCI

[Diogo Ivo]

On RSCI, the receive error interrupt (ERI) has its own dedicated
interrupt line and status flags cleared via the write-only CFCLR
register, with no hardware interaction with transmission.

Calling sci_tx_interrupt() from sci_er_interrupt() on RSCI is
unnecessary and causes rsci_transmit_chars() to run in invalid TX
states:
- If an RX error occurs while TX is idle (CCR0.TE == 0 and CSR.TDRE == 1)
  and xmit_fifo is empty, rsci_transmit_chars() sets CCR0.TEIE while
  CSR.TEND is 1, triggering a spurious TEI interrupt.
- If an RX error occurs while the hardware TX FIFO is draining
  (CSR.TDRE == 0) after xmit_fifo was flushed, rsci_transmit_chars()
  clears CCR0.TIE without setting CCR0.TEIE, leaving CCR0.TE set after
  the hardware FIFO drains and stalling subsequent transmissions.

Skip calling sci_tx_interrupt() in sci_er_interrupt() for RSCI ports.

Fixes: 0666e3fe95ab ("serial: sh-sci: Add support for RZ/T2H SCI")
Cc: stable@vger.kernel.org
Signed-off-by: Diogo Ivo <diogo.ivo.sx@renesas.com>
---
 drivers/tty/serial/sh-sci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 6df25fef5a5f..39aa90131918 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -2103,7 +2103,7 @@ static irqreturn_t sci_er_interrupt(int irq, void *ptr)
 	s->ops->clear_SCxSR(port, SCxSR_ERROR_CLEAR(port));
 
 	/* Kick the transmission */
-	if (!s->chan_tx)
+	if (!s->chan_tx && !sci_is_rsci_type(s->type))
 		sci_tx_interrupt(irq, ptr);
 
 	return IRQ_HANDLED;

-- 
2.56.0

[PATCH v4 5/7] serial: sh-sci: Skip TDxE clear when hardware clears it automatically

[Diogo Ivo]

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

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

Add a tdxe_auto_clear flag to struct sci_of_data, propagate it to struct
sci_port as has_tdxe_auto_clear, and skip the clear_SCxSR() call in
sci_transmit_chars() and sci_poll_put_char() when it is set. Enable the
flag for the PORT_SCI (of_sci_sci) variant.

Signed-off-by: Au Dang <au.dang.jx@renesas.com>
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Signed-off-by: Diogo Ivo <diogo.ivo.sx@renesas.com>
---
 drivers/tty/serial/sh-sci-common.h | 2 ++
 drivers/tty/serial/sh-sci.c        | 9 +++++++--
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/sh-sci-common.h b/drivers/tty/serial/sh-sci-common.h
index 273856fbc20b..b75e5e1d3075 100644
--- a/drivers/tty/serial/sh-sci-common.h
+++ b/drivers/tty/serial/sh-sci-common.h
@@ -108,6 +108,7 @@ struct sci_of_data {
 	const struct sci_port_ops *ops;
 	unsigned short regtype;
 	unsigned short type;
+	bool tdxe_auto_clear;
 };
 
 struct sci_port {
@@ -163,6 +164,7 @@ struct sci_port {
 	bool has_rtscts;
 	bool autorts;
 	bool tx_occurred;
+	bool has_tdxe_auto_clear;
 };
 
 #define to_sci_port(uart) container_of((uart), struct sci_port, port)
diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 39aa90131918..fc77536644ed 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->has_tdxe_auto_clear)
+		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 */
@@ -1063,7 +1064,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->has_tdxe_auto_clear)
+		s->ops->clear_SCxSR(port, SCxSR_TDxE_CLEAR(port));
 
 	if (kfifo_len(&tport->xmit_fifo) < WAKEUP_CHARS)
 		uart_write_wakeup(port);
@@ -3664,6 +3666,7 @@ static const struct sci_of_data of_sci_sci = {
 	.ops = &sci_port_ops,
 	.uart_ops = &sci_uart_ops,
 	.params = &sci_port_params[SCIx_SCI_REGTYPE],
+	.tdxe_auto_clear = true,
 };
 
 static const struct of_device_id of_sci_match[] __maybe_unused = {
@@ -3802,6 +3805,7 @@ static struct plat_sci_port *sci_parse_dt(struct platform_device *pdev,
 	sp->ops = data->ops;
 	sp->port.ops = data->uart_ops;
 	sp->params = data->params;
+	sp->has_tdxe_auto_clear = data->tdxe_auto_clear;
 
 	sp->has_rtscts = of_property_read_bool(np, "uart-has-rtscts");
 
@@ -4088,6 +4092,7 @@ int __init scix_early_console_setup(struct earlycon_device *device,
 	sci_ports[0].params = data->params;
 	sci_ports[0].ops = data->ops;
 	sci_ports[0].port.ops = data->uart_ops;
+	sci_ports[0].has_tdxe_auto_clear = data->tdxe_auto_clear;
 	sci_uart_earlycon = true;
 	regs = sci_ports[0].params->common_regs;
 

-- 
2.56.0

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

[Diogo Ivo]

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>
Signed-off-by: Diogo Ivo <diogo.ivo.sx@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 fc77536644ed..2d667086ec3a 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -3456,10 +3456,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 (sci_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.56.0

[PATCH v4 7/7] serial: rsci: Simplify rsci_transmit_chars()

[Diogo Ivo]

Now that sci_er_interrupt() no longer calls sci_tx_interrupt() on RSCI
ports, rsci_transmit_chars() is only invoked when CSR.TDRE and CCR0.TE
are set.

Drop the redundant CCR0.TIE manipulation in the !CSR_TDRE guard and
call rsci_stop_tx() instead of open-coding the CCR0.TIE/CCR0.TEIE
update when transmission stops.

Signed-off-by: Diogo Ivo <diogo.ivo.sx@renesas.com>
---
 drivers/tty/serial/rsci.c | 19 ++++---------------
 1 file changed, 4 insertions(+), 15 deletions(-)

diff --git a/drivers/tty/serial/rsci.c b/drivers/tty/serial/rsci.c
index 05703bb31afd..80087be11808 100644
--- a/drivers/tty/serial/rsci.c
+++ b/drivers/tty/serial/rsci.c
@@ -432,19 +432,12 @@ static void rsci_transmit_chars(struct uart_port *port)
 {
 	unsigned int stopped = uart_tx_stopped(port);
 	struct tty_port *tport = &port->state->port;
-	u32 status, ctrl;
+	u32 status;
 	int count;
 
 	status = rsci_serial_in(port, CSR);
-	if (!(status & CSR_TDRE)) {
-		ctrl = rsci_serial_in(port, CCR0);
-		if (kfifo_is_empty(&tport->xmit_fifo))
-			ctrl &= ~CCR0_TIE;
-		else
-			ctrl |= CCR0_TIE;
-		rsci_serial_out(port, CCR0, ctrl);
+	if (!(status & CSR_TDRE))
 		return;
-	}
 
 	count = rsci_txroom(port);
 
@@ -467,12 +460,8 @@ static void rsci_transmit_chars(struct uart_port *port)
 	if (kfifo_len(&tport->xmit_fifo) < WAKEUP_CHARS)
 		uart_write_wakeup(port);
 
-	if (kfifo_is_empty(&tport->xmit_fifo) || stopped) {
-		ctrl = rsci_serial_in(port, CCR0);
-		ctrl &= ~CCR0_TIE;
-		ctrl |= CCR0_TEIE;
-		rsci_serial_out(port, CCR0, ctrl);
-	}
+	if (kfifo_is_empty(&tport->xmit_fifo) || stopped)
+		rsci_stop_tx(port);
 }
 
 static void rsci_receive_chars(struct uart_port *port)

-- 
2.56.0

Re: [PATCH v4 1/7] 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")
> Cc: stable@vger.kernel.org
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Signed-off-by: Diogo Ivo <diogo.ivo.sx@renesas.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113-g2ul_sci-v4-0-6815e41d90c5@renesas.com?part=1

Re: [PATCH v4 2/7] serial: sh-sci: Keep TX enabled on TEI while data is pending

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [High] serial: sh-sci: missing sci_stop_tx() when stopped in
  sci_transmit_chars()
- [Medium] serial: sh-sci: premature TE disable in
  sci_tx_end_interrupt() during active TX
--

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

serial: sh-sci: Keep TX enabled on TEI while data is pending

This commit keeps the transmitter enabled in sci_tx_end_interrupt() when
x_char or unstopped xmit_fifo data is pending, and updates
sci_transmit_chars() and rsci_transmit_chars() to switch from TIE to TEIE
when transmission is stopped.

> diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
> index 9eb297dbc9010..6df25fef5a5f5 100644
> --- a/drivers/tty/serial/sh-sci.c
> +++ b/drivers/tty/serial/sh-sci.c
[ ... ]
> @@ -1007,6 +1011,20 @@ static int sci_rxfill(struct uart_port *port)
>   *                   the interrupt related routines                       *
>   * ********************************************************************** */
>  
> +static void sci_clr_tie_set_teie(struct uart_port *port)
> +{
> +	struct sci_port *s = to_sci_port(port);
> +	unsigned short ctrl;
> +
> +	if (s->type != PORT_SCI)
> +		return;
> +
> +	ctrl = sci_serial_in(port, SCSCR);
> +	ctrl &= ~SCSCR_TIE;
> +	ctrl |= SCSCR_TEIE;
> +	sci_serial_out(port, SCSCR, ctrl);
> +}
> +
>  static void sci_transmit_chars(struct uart_port *port)
>  {
>  	struct tty_port *tport = &port->state->port;
> @@ -1050,14 +1068,10 @@ static void sci_transmit_chars(struct uart_port *port)
>  	if (kfifo_len(&tport->xmit_fifo) < WAKEUP_CHARS)
>  		uart_write_wakeup(port);
>  	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_clr_tie_set_teie(port);
>  		sci_stop_tx(port);
> +	} else if (stopped) {
> +		sci_clr_tie_set_teie(port);
>  	}
>  }

[Severity: High]
Does the "else if (stopped)" branch leave SCSCR_TIE enabled on
non-PORT_SCI ports?

sci_clr_tie_set_teie() returns immediately when s->type != PORT_SCI, and
this branch does not call sci_stop_tx().  If sci_transmit_chars() runs on
a PORT_SCIF, PORT_SCIFA, PORT_SCIFB, PORT_HSCIF, or PORT_IRDA port while
stopped with data still in xmit_fifo (for example, after uart_send_xchar()
calls sci_start_tx()), SCSCR_TIE remains set and triggers a continuous TXI
interrupt storm until transmission is unstopped.

> @@ -2012,6 +2026,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;
> @@ -2020,8 +2035,18 @@ static irqreturn_t sci_tx_end_interrupt(int irq, void *ptr)
>  		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->ops->read_reg(port, regs->control);
> +	if (port->x_char || (!kfifo_is_empty(&tport->xmit_fifo) &&
> +			     !uart_tx_stopped(port))) {
> +		ctrl &= ~(s->params->param_bits->teie);
> +		ctrl |= s->params->param_bits->tie;
> +		s->ops->write_reg(port, regs->control, ctrl);
> +		s->ops->transmit_chars(port);
> +		uart_port_unlock_irqrestore(port, flags);
> +		return IRQ_HANDLED;
> +	}
> +
> +	ctrl &= ~(s->params->param_bits->te_clear);
>  	s->ops->write_reg(port, regs->control, ctrl);
>  	uart_port_unlock_irqrestore(port, flags);

[Severity: Medium]
Can sci_tx_end_interrupt() still clear TE while newly written bytes are
being transmitted?

When transmit_chars() drains xmit_fifo, it clears TIE and sets TEIE while
the last byte is still shifting out.  If uart_write() runs before that
byte finishes, sci_start_tx() or rsci_start_tx() sets TIE without clearing
TEIE, leaving both interrupts enabled:

CPU0                              CPU1
transmit_chars()
  /* drains xmit_fifo */
  ctrl &= ~TIE;
  ctrl |= TEIE;
uart_write()
  start_tx()
    ctrl |= TIE | TE;
    /* TIE and TEIE both set */
                                  /* TDR transfers to shift reg: TXI */
                                  /* shift reg finishes: TEI */
                                  sci_tx_interrupt()
                                    lock(port)
                                    transmit_chars()
                                      /* writes new byte to TDR,
                                         drains xmit_fifo */
                                    unlock(port)
                                  sci_tx_end_interrupt()
                                    lock(port)
                                    /* xmit_fifo is now empty */
                                    ctrl &= ~te_clear;
                                      <-- clears TE during active TX
                                    unlock(port)

Because sci_tx_end_interrupt() checks only x_char and xmit_fifo without
verifying in regs->status that poll_sent_bits (TDRE and TEND) are still
set in hardware, it disables the transmitter before the newly written byte
has been sent.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113-g2ul_sci-v4-0-6815e41d90c5@renesas.com?part=2

Re: [PATCH v4 3/7] serial: rsci: Enable transmit end interrupt in rsci_stop_tx()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] serial: rsci: TX restart on suspend in sci_tx_end_interrupt()
--

commit 071c63519ac773139fb715b1e8e4f93c225b5ba8
Author: Diogo Ivo <diogo.ivo.sx@renesas.com>

serial: rsci: Enable transmit end interrupt in rsci_stop_tx()

This commit enables the transmit end interrupt (CCR0_TEIE) in
rsci_stop_tx() when transmission is active (CCR0_TE is set) so that
sci_tx_end_interrupt() clears CCR0_TE once in-flight data finishes
shifting out.

> diff --git a/drivers/tty/serial/rsci.c b/drivers/tty/serial/rsci.c
> index 0391111db8ae8..05703bb31afde 100644
> --- a/drivers/tty/serial/rsci.c
> +++ b/drivers/tty/serial/rsci.c
> @@ -409,6 +409,8 @@ static void rsci_stop_tx(struct uart_port *port)
>  
>  	ctrl = rsci_serial_in(port, CCR0);
>  	ctrl &= ~CCR0_TIE;
> +	if (ctrl & CCR0_TE)
> +		ctrl |= CCR0_TEIE;
>  	rsci_serial_out(port, CCR0, ctrl);
>  }

[Severity: Medium]
Can enabling CCR0_TEIE here cause sci_tx_end_interrupt() to restart
transmission during system suspend?

uart_suspend_port() sets uport->suspended = 1 and calls ops->stop_tx()
and ops->set_mctrl(uport, 0) without setting tty->flow.stopped or
uport->hw_stopped, then drops uport->lock to wait up to 30ms for the
hardware transmitter to empty.  When the in-flight bytes finish shifting
out, the resulting TEI interrupt sees !uart_tx_stopped(port) as true and
resumes transmission if tport->xmit_fifo still has queued data:

drivers/tty/serial/sh-sci.c:sci_tx_end_interrupt() {
    ...
	if (port->x_char || (!kfifo_is_empty(&tport->xmit_fifo) &&
			     !uart_tx_stopped(port))) {
		ctrl &= ~(s->params->param_bits->teie);
		ctrl |= s->params->param_bits->tie;
		s->ops->write_reg(port, regs->control, ctrl);
		s->ops->transmit_chars(port);
    ...
}

This keeps transmitting after stop_tx() and modem control deassertion,
and if xmit_fifo does not empty within 30ms, uart_suspend_port() times
out with "Unable to drain transmitter" and calls ops->shutdown()
mid-transmission.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113-g2ul_sci-v4-0-6815e41d90c5@renesas.com?part=3

Re: [PATCH v4 4/7] serial: sh-sci: Do not kick TX from sci_er_interrupt() on RSCI

[sashiko-bot]

> On RSCI, the receive error interrupt (ERI) has its own dedicated
> interrupt line and status flags cleared via the write-only CFCLR
> register, with no hardware interaction with transmission.
> 
> Calling sci_tx_interrupt() from sci_er_interrupt() on RSCI is
> unnecessary and causes rsci_transmit_chars() to run in invalid TX
> states:
> - If an RX error occurs while TX is idle (CCR0.TE == 0 and CSR.TDRE == 1)
>   and xmit_fifo is empty, rsci_transmit_chars() sets CCR0.TEIE while
>   CSR.TEND is 1, triggering a spurious TEI interrupt.
> - If an RX error occurs while the hardware TX FIFO is draining
>   (CSR.TDRE == 0) after xmit_fifo was flushed, rsci_transmit_chars()
>   clears CCR0.TIE without setting CCR0.TEIE, leaving CCR0.TE set after
>   the hardware FIFO drains and stalling subsequent transmissions.
> 
> Skip calling sci_tx_interrupt() in sci_er_interrupt() for RSCI ports.
> 
> Fixes: 0666e3fe95ab ("serial: sh-sci: Add support for RZ/T2H SCI")
> Cc: stable@vger.kernel.org
> Signed-off-by: Diogo Ivo <diogo.ivo.sx@renesas.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113-g2ul_sci-v4-0-6815e41d90c5@renesas.com?part=4

Re: [PATCH v4 5/7] serial: sh-sci: Skip TDxE clear when hardware clears it automatically

[sashiko-bot]

> From: Au Dang <au.dang.jx@renesas.com>
> 
> On some SCI variants, the TDxE (Transmit Data Empty) flag in SCxSR is
> cleared by hardware when data is written to TDR. Clearing it again from
> software via clear_SCxSR() generates a spurious TXI interrupt, causing
> the last character to be transmitted twice.
> 
> Add a tdxe_auto_clear flag to struct sci_of_data, propagate it to struct
> sci_port as has_tdxe_auto_clear, and skip the clear_SCxSR() call in
> sci_transmit_chars() and sci_poll_put_char() when it is set. Enable the
> flag for the PORT_SCI (of_sci_sci) variant.
> 
> Signed-off-by: Au Dang <au.dang.jx@renesas.com>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Signed-off-by: Diogo Ivo <diogo.ivo.sx@renesas.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113-g2ul_sci-v4-0-6815e41d90c5@renesas.com?part=5

Re: [PATCH v4 6/7] 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
> [ ... ]
> resumes.
> 
> Signed-off-by: Au Dang <au.dang.jx@renesas.com>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Signed-off-by: Diogo Ivo <diogo.ivo.sx@renesas.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113-g2ul_sci-v4-0-6815e41d90c5@renesas.com?part=6

Re: [PATCH v4 7/7] serial: rsci: Simplify rsci_transmit_chars()

[sashiko-bot]

> Now that sci_er_interrupt() no longer calls sci_tx_interrupt() on RSCI
> ports, rsci_transmit_chars() is only invoked when CSR.TDRE and CCR0.TE
> are set.
> 
> Drop the redundant CCR0.TIE manipulation in the !CSR_TDRE guard and
> call rsci_stop_tx() instead of open-coding the CCR0.TIE/CCR0.TEIE
> update when transmission stops.
> 
> Signed-off-by: Diogo Ivo <diogo.ivo.sx@renesas.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113-g2ul_sci-v4-0-6815e41d90c5@renesas.com?part=7