* [PATCH v2 0/4] Fix transmit handling on SCI/RSCI ports
@ 2026-10-21 13:13 Biju Das
2026-10-21 13:13 ` [PATCH v2 1/4] serial: sh-sci: Drop TE disable at end of transmission on SCI Biju Das
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Biju Das @ 2026-10-21 13:13 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, Kiryl Shutsemau, Rick Edgecombe,
Dave Hansen
Cc: linux-kernel, linux-serial, linux-renesas-soc, x86, linux-coco,
kvm, Biju Das, Au Dang
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.
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Jiri Slaby <jirislaby@kernel.org>
To: Kiryl Shutsemau <kas@kernel.org>
To: Rick Edgecombe <rick.p.edgecombe@intel.com>
To: Dave Hansen <dave.hansen@linux.intel.com>
Cc: linux-kernel@vger.kernel.org
Cc: linux-serial@vger.kernel.org
CC: linux-renesas-soc@vger.kernel.org
Cc: x86@kernel.org
Cc: linux-coco@lists.linux.dev
Cc: kvm@vger.kernel.org
Signed-off-by: Biju Das <biju.das.jz@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
drivers/tty/serial/rsci.c | 2 ++
drivers/tty/serial/sh-sci-common.h | 4 ++++
drivers/tty/serial/sh-sci.c | 45 +++++++++++++++++++++++++-------------
3 files changed, 36 insertions(+), 15 deletions(-)
---
base-commit: 8f7df005a7564bd7ffa75da9c3085d5b996f1e16
change-id: 20261020-g2ul_sci-b610606ea5b5
Best regards,
--
Biju Das <biju.das.jz@bp.renesas.com>
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH v2 1/4] serial: sh-sci: Drop TE disable at end of transmission on SCI 2026-10-21 13:13 [PATCH v2 0/4] Fix transmit handling on SCI/RSCI ports Biju Das @ 2026-10-21 13:13 ` Biju Das 2026-10-21 13:18 ` sashiko-bot 2026-10-21 13:13 ` [PATCH v2 2/4] serial: sh-sci: Keep TX enabled on TEI while data is pending Biju Das ` (2 subsequent siblings) 3 siblings, 1 reply; 11+ messages in thread From: Biju Das @ 2026-10-21 13:13 UTC (permalink / raw) To: Greg Kroah-Hartman, Jiri Slaby, Kiryl Shutsemau, Rick Edgecombe, Dave Hansen Cc: linux-kernel, linux-serial, linux-renesas-soc, x86, linux-coco, kvm, Biju Das 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> --- v1->v2: * No change. --- 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/4] serial: sh-sci: Drop TE disable at end of transmission on SCI 2026-10-21 13:13 ` [PATCH v2 1/4] serial: sh-sci: Drop TE disable at end of transmission on SCI Biju Das @ 2026-10-21 13:18 ` sashiko-bot 0 siblings, 0 replies; 11+ messages in thread From: sashiko-bot @ 2026-10-21 13:18 UTC (permalink / raw) To: Biju Das; +Cc: linux-serial > 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/20261021-g2ul_sci-v2-0-b4e038220018@bp.renesas.com?part=1 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 2/4] serial: sh-sci: Keep TX enabled on TEI while data is pending 2026-10-21 13:13 [PATCH v2 0/4] Fix transmit handling on SCI/RSCI ports Biju Das 2026-10-21 13:13 ` [PATCH v2 1/4] serial: sh-sci: Drop TE disable at end of transmission on SCI Biju Das @ 2026-10-21 13:13 ` Biju Das 2026-10-21 13:38 ` sashiko-bot 2026-10-21 13:13 ` [PATCH v2 3/4] serial: sh-sci: Skip TDxE clear when hardware clears it automatically Biju Das 2026-10-21 13:13 ` [PATCH v2 4/4] serial: sh-sci: Toggle TE when restoring SCSCR in console write on SCI Biju Das 3 siblings, 1 reply; 11+ messages in thread From: Biju Das @ 2026-10-21 13:13 UTC (permalink / raw) To: Greg Kroah-Hartman, Jiri Slaby, Kiryl Shutsemau, Rick Edgecombe, Dave Hansen Cc: linux-kernel, linux-serial, linux-renesas-soc, x86, linux-coco, kvm, Biju Das 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 sci_tx_interrupt() so transmission continues normally. The existing TE/TEIE teardown path only runs once nothing is left to send. 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> --- v1->v2: * Updated commit header and description. * 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. --- 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..58120b523ca1 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; @@ -2020,8 +2025,17 @@ 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); + uart_port_unlock_irqrestore(port, flags); + return sci_tx_interrupt(irq, ptr); + } + + ctrl &= ~(s->params->param_bits->te_clear); s->ops->write_reg(port, regs->control, ctrl); uart_port_unlock_irqrestore(port, flags); -- 2.43.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/4] serial: sh-sci: Keep TX enabled on TEI while data is pending 2026-10-21 13:13 ` [PATCH v2 2/4] serial: sh-sci: Keep TX enabled on TEI while data is pending Biju Das @ 2026-10-21 13:38 ` sashiko-bot 2026-10-22 10:28 ` Biju Das 0 siblings, 1 reply; 11+ messages in thread From: sashiko-bot @ 2026-10-21 13:38 UTC (permalink / raw) To: Biju Das; +Cc: linux-serial, kvm, wsa+renesas Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [High] serial: sh-sci: TX stall in sci_tx_end_interrupt() on x_char when stopped - [High] serial: sh-sci: lock drop race in sci_tx_end_interrupt() before TX handoff - [Medium] serial: sh-sci: sci_tx_end_interrupt() clears TE without checking TEND -- commit 24fb279b2944cfab75c45c919854637a4575564b Author: Biju Das <biju.das.jz@bp.renesas.com> serial: sh-sci: Keep TX enabled on TEI while data is pending This commit updates sci_tx_end_interrupt() to check for pending transmit data under the port lock before disabling the transmitter, handing over to sci_tx_interrupt() when data is waiting. It also adds tie and teie bit masks to struct sci_port_params_bits for the supported port variants. > diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c > index 9eb297dbc9010..58120b523ca1e 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; > @@ -2020,8 +2025,17 @@ 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); [Severity: High] What happens here when port->x_char is non-zero while uart_tx_stopped(port) is true and tport->xmit_fifo is not empty? In that case, sci_tx_end_interrupt() clears param_bits->teie, sets param_bits->tie, and calls sci_tx_interrupt() -> s->ops->transmit_chars(port). In both drivers/tty/serial/sh-sci.c:sci_transmit_chars() and drivers/tty/serial/rsci.c:rsci_transmit_chars(), the loop sends port->x_char, clears port->x_char = 0, and breaks out on the next iteration because stopped is true: do { unsigned char c; if (port->x_char) { c = port->x_char; port->x_char = 0; } else if (stopped || !kfifo_get(&tport->xmit_fifo, &c)) { break; } However, at the end of sci_transmit_chars() and rsci_transmit_chars(), param_bits->tie is only cleared and param_bits->teie is only re-enabled when tport->xmit_fifo is empty: drivers/tty/serial/sh-sci.c: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); } } drivers/tty/serial/rsci.c:rsci_transmit_chars() { ... if (kfifo_is_empty(&tport->xmit_fifo)) { ctrl = rsci_serial_in(port, CCR0); ctrl &= ~CCR0_TIE; ctrl |= CCR0_TEIE; rsci_serial_out(port, CCR0, ctrl); } } Because tport->xmit_fifo is still non-empty while the port is stopped, param_bits->tie remains set and param_bits->teie remains cleared after port->x_char has been written: 1. On sci ports without hardware status auto-clear, when the transmit data register empties and triggers sci_tx_interrupt() again with port->x_char == 0 and stopped == 1, sci_transmit_chars() breaks out of the loop without writing a new byte and calls s->ops->clear_SCxSR(), causing the hardware to repeatedly re-transmit the stale byte in the data register in an interrupt loop. 2. On both sci and rsci ports, because param_bits->teie is cleared, sci_tx_end_interrupt() never fires after port->x_char finishes shifting out, so transmit-enable is never cleared. When the port is later unstopped and uart_start() calls sci_start_tx() or rsci_start_tx(), transmit-enable and param_bits->tie are already set in regs->control, producing no rising edge to trigger the transmit interrupt and stalling the remaining data in tport->xmit_fifo. Could sci_transmit_chars() and rsci_transmit_chars() also switch back from param_bits->tie to param_bits->teie when stopped is true? > + uart_port_unlock_irqrestore(port, flags); > + return sci_tx_interrupt(irq, ptr); > + } [Severity: High] Does dropping port->lock here before calling sci_tx_interrupt() introduce a race window with concurrent port operations? sci_tx_interrupt() immediately re-acquires port->lock to invoke s->ops->transmit_chars(port): drivers/tty/serial/sh-sci.c:sci_tx_interrupt() { ... uart_port_lock_irqsave(port, &flags); s->ops->transmit_chars(port); uart_port_unlock_irqrestore(port, flags); ... } Between uart_port_unlock_irqrestore() in sci_tx_end_interrupt() and uart_port_lock_irqsave() in sci_tx_interrupt(), another processor can acquire port->lock: 1. In drivers/tty/serial/sh-sci.c:sci_shutdown(): uart_port_lock_irqsave(port, &flags); s->port.ops->stop_rx(port); s->port.ops->stop_tx(port); s->ops->shutdown_complete(port); uart_port_unlock_irqrestore(port, flags); If sci_shutdown() runs during the unlock window before sci_free_irq(s), sci_tx_interrupt() will subsequently run s->ops->transmit_chars(port) after shutdown_complete(), writing to the transmit data register and re-arming transmit interrupts on a shut-down port. 2. If uart_stop() runs in the window, it sets tty->flow.stopped and calls stop_tx() to clear param_bits->tie. When s->ops->transmit_chars(port) then runs with stopped set and a non-empty tport->xmit_fifo, no byte is written and param_bits->teie is not set, leaving transmit-enable set with both param_bits->tie and param_bits->teie cleared so a later sci_start_tx() or rsci_start_tx() sees no rising edge on transmit-enable and stalls. 3. If uart_flush_buffer() or a concurrent sci_tx_interrupt() drains tport->xmit_fifo during the window, s->ops->transmit_chars(port) runs with an empty fifo and clears status without writing a new byte. Could sci_tx_end_interrupt() call s->ops->transmit_chars(port) directly while still holding port->lock instead of unlocking and calling sci_tx_interrupt()? > + > + ctrl &= ~(s->params->param_bits->te_clear); > s->ops->write_reg(port, regs->control, ctrl); > uart_port_unlock_irqrestore(port, flags); [Severity: Medium] Can clearing param_bits->te_clear here cut off a byte that was just written to the transmit register by a concurrent sci_tx_interrupt()? When sci_transmit_chars() or rsci_transmit_chars() writes the final byte from tport->xmit_fifo, it clears param_bits->tie and sets param_bits->teie while the byte is still draining. If uart_write() queues a new byte before that transmission finishes, drivers/tty/serial/sh-sci.c:sci_start_tx(): if (s->type == PORT_SCI) ctrl |= SCSCR_TE; sci_serial_out(port, SCSCR, ctrl | SCSCR_TIE); and 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); set param_bits->tie without clearing param_bits->teie. If the previous byte finishes shifting out and triggers sci_tx_end_interrupt() while sci_tx_interrupt() runs on another processor and moves the newly queued byte from tport->xmit_fifo into the data register (leaving tport->xmit_fifo empty again), sci_tx_end_interrupt() will see port->x_char == 0 and kfifo_is_empty(&tport->xmit_fifo) and clear param_bits->te_clear while the new byte is still in the hardware register. Should sci_tx_end_interrupt() verify that param_bits->poll_sent_bits is still set in regs->status before clearing param_bits->te_clear (or should sci_start_tx() and rsci_start_tx() clear param_bits->teie)? > > return IRQ_HANDLED; > } -- Sashiko AI review · https://sashiko.dev/#/patchset/20261021-g2ul_sci-v2-0-b4e038220018@bp.renesas.com?part=2 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/4] serial: sh-sci: Keep TX enabled on TEI while data is pending 2026-10-21 13:38 ` sashiko-bot @ 2026-10-22 10:28 ` Biju Das 0 siblings, 0 replies; 11+ messages in thread From: Biju Das @ 2026-10-22 10:28 UTC (permalink / raw) To: sashiko-reviews; +Cc: linux-serial, kvm, wsa+renesas, linux-renesas-soc On Wed, Oct 21, 2026 at 2:38 PM <sashiko-bot@kernel.org> wrote: > > Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: > - [High] serial: sh-sci: TX stall in sci_tx_end_interrupt() on x_char when stopped > - [High] serial: sh-sci: lock drop race in sci_tx_end_interrupt() before TX handoff > - [Medium] serial: sh-sci: sci_tx_end_interrupt() clears TE without checking TEND > -- > > commit 24fb279b2944cfab75c45c919854637a4575564b > Author: Biju Das <biju.das.jz@bp.renesas.com> > > serial: sh-sci: Keep TX enabled on TEI while data is pending > > This commit updates sci_tx_end_interrupt() to check for pending transmit data > under the port lock before disabling the transmitter, handing over to > sci_tx_interrupt() when data is waiting. It also adds tie and teie bit masks > to struct sci_port_params_bits for the supported port variants. > > > diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c > > index 9eb297dbc9010..58120b523ca1e 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; > > @@ -2020,8 +2025,17 @@ 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); > > [Severity: High] > What happens here when port->x_char is non-zero while uart_tx_stopped(port) > is true and tport->xmit_fifo is not empty? > > In that case, sci_tx_end_interrupt() clears param_bits->teie, sets > param_bits->tie, and calls sci_tx_interrupt() -> s->ops->transmit_chars(port). > In both drivers/tty/serial/sh-sci.c:sci_transmit_chars() and > drivers/tty/serial/rsci.c:rsci_transmit_chars(), the loop sends port->x_char, > clears port->x_char = 0, and breaks out on the next iteration because stopped > is true: > > do { > unsigned char c; > > if (port->x_char) { > c = port->x_char; > port->x_char = 0; > } else if (stopped || !kfifo_get(&tport->xmit_fifo, &c)) { > break; > } > > However, at the end of sci_transmit_chars() and rsci_transmit_chars(), > param_bits->tie is only cleared and param_bits->teie is only re-enabled when > tport->xmit_fifo is empty: > > drivers/tty/serial/sh-sci.c: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); > } > } > > drivers/tty/serial/rsci.c:rsci_transmit_chars() { > ... > if (kfifo_is_empty(&tport->xmit_fifo)) { > ctrl = rsci_serial_in(port, CCR0); > ctrl &= ~CCR0_TIE; > ctrl |= CCR0_TEIE; > rsci_serial_out(port, CCR0, ctrl); > } > } > > Because tport->xmit_fifo is still non-empty while the port is stopped, > param_bits->tie remains set and param_bits->teie remains cleared after > port->x_char has been written: > > 1. On sci ports without hardware status auto-clear, when the transmit data > register empties and triggers sci_tx_interrupt() again with > port->x_char == 0 and stopped == 1, sci_transmit_chars() breaks out of the > loop without writing a new byte and calls s->ops->clear_SCxSR(), causing > the hardware to repeatedly re-transmit the stale byte in the data register > in an interrupt loop. > > 2. On both sci and rsci ports, because param_bits->teie is cleared, > sci_tx_end_interrupt() never fires after port->x_char finishes shifting > out, so transmit-enable is never cleared. When the port is later unstopped > and uart_start() calls sci_start_tx() or rsci_start_tx(), transmit-enable > and param_bits->tie are already set in regs->control, producing no rising > edge to trigger the transmit interrupt and stalling the remaining data in > tport->xmit_fifo. > > Could sci_transmit_chars() and rsci_transmit_chars() also switch back from > param_bits->tie to param_bits->teie when stopped is true? > > > + uart_port_unlock_irqrestore(port, flags); > > + return sci_tx_interrupt(irq, ptr); > > + } > > [Severity: High] > Does dropping port->lock here before calling sci_tx_interrupt() introduce a > race window with concurrent port operations? > > sci_tx_interrupt() immediately re-acquires port->lock to invoke > s->ops->transmit_chars(port): > > drivers/tty/serial/sh-sci.c:sci_tx_interrupt() { > ... > uart_port_lock_irqsave(port, &flags); > s->ops->transmit_chars(port); > uart_port_unlock_irqrestore(port, flags); > ... > } > > Between uart_port_unlock_irqrestore() in sci_tx_end_interrupt() and > uart_port_lock_irqsave() in sci_tx_interrupt(), another processor can acquire > port->lock: > > 1. In drivers/tty/serial/sh-sci.c:sci_shutdown(): > > uart_port_lock_irqsave(port, &flags); > s->port.ops->stop_rx(port); > s->port.ops->stop_tx(port); > s->ops->shutdown_complete(port); > uart_port_unlock_irqrestore(port, flags); > > If sci_shutdown() runs during the unlock window before sci_free_irq(s), > sci_tx_interrupt() will subsequently run s->ops->transmit_chars(port) > after shutdown_complete(), writing to the transmit data register and > re-arming transmit interrupts on a shut-down port. > > 2. If uart_stop() runs in the window, it sets tty->flow.stopped and calls > stop_tx() to clear param_bits->tie. When s->ops->transmit_chars(port) > then runs with stopped set and a non-empty tport->xmit_fifo, no byte is > written and param_bits->teie is not set, leaving transmit-enable set with > both param_bits->tie and param_bits->teie cleared so a later > sci_start_tx() or rsci_start_tx() sees no rising edge on transmit-enable > and stalls. > > 3. If uart_flush_buffer() or a concurrent sci_tx_interrupt() drains > tport->xmit_fifo during the window, s->ops->transmit_chars(port) runs with > an empty fifo and clears status without writing a new byte. > > Could sci_tx_end_interrupt() call s->ops->transmit_chars(port) directly while > still holding port->lock instead of unlocking and calling sci_tx_interrupt()? > > > + > > + ctrl &= ~(s->params->param_bits->te_clear); > > s->ops->write_reg(port, regs->control, ctrl); > > uart_port_unlock_irqrestore(port, flags); > > [Severity: Medium] > Can clearing param_bits->te_clear here cut off a byte that was just written > to the transmit register by a concurrent sci_tx_interrupt()? > > When sci_transmit_chars() or rsci_transmit_chars() writes the final byte from > tport->xmit_fifo, it clears param_bits->tie and sets param_bits->teie while > the byte is still draining. If uart_write() queues a new byte before that > transmission finishes, drivers/tty/serial/sh-sci.c:sci_start_tx(): > > if (s->type == PORT_SCI) > ctrl |= SCSCR_TE; > > sci_serial_out(port, SCSCR, ctrl | SCSCR_TIE); > > and 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); > > set param_bits->tie without clearing param_bits->teie. > > If the previous byte finishes shifting out and triggers > sci_tx_end_interrupt() while sci_tx_interrupt() runs on another processor and > moves the newly queued byte from tport->xmit_fifo into the data register > (leaving tport->xmit_fifo empty again), sci_tx_end_interrupt() will see > port->x_char == 0 and kfifo_is_empty(&tport->xmit_fifo) and clear > param_bits->te_clear while the new byte is still in the hardware register. > > Should sci_tx_end_interrupt() verify that param_bits->poll_sent_bits is still > set in regs->status before clearing param_bits->te_clear (or should > sci_start_tx() and rsci_start_tx() clear param_bits->teie)? > > > > > > return IRQ_HANDLED; > > } > > -- > Sashiko AI review · https://sashiko.dev/#/patchset/20261021-g2ul_sci-v2-0-b4e038220018@bp.renesas.com?part=2 Modified patch based on rhe above review. From 4ebe5978f806e7cbf6690ecab116d660c210929b Mon Sep 17 00:00:00 2001 From: Biju Das <biju.das.jz@bp.renesas.com> Date: Mon, 19 Oct 2026 18:01:10 +0100 Subject: [PATCH 2/4] serial: sh-sci: Keep TX enabled on TEI while data is pending 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") Signed-off-by: Biju Das <biju.das.jz@bp.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.43.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 3/4] serial: sh-sci: Skip TDxE clear when hardware clears it automatically 2026-10-21 13:13 [PATCH v2 0/4] Fix transmit handling on SCI/RSCI ports Biju Das 2026-10-21 13:13 ` [PATCH v2 1/4] serial: sh-sci: Drop TE disable at end of transmission on SCI Biju Das 2026-10-21 13:13 ` [PATCH v2 2/4] serial: sh-sci: Keep TX enabled on TEI while data is pending Biju Das @ 2026-10-21 13:13 ` Biju Das 2026-10-21 13:22 ` sashiko-bot 2026-10-21 13:13 ` [PATCH v2 4/4] serial: sh-sci: Toggle TE when restoring SCSCR in console write on SCI Biju Das 3 siblings, 1 reply; 11+ messages in thread From: Biju Das @ 2026-10-21 13:13 UTC (permalink / raw) To: Greg Kroah-Hartman, Jiri Slaby, Kiryl Shutsemau, Rick Edgecombe, Dave Hansen Cc: linux-kernel, linux-serial, linux-renesas-soc, x86, linux-coco, kvm, Biju Das, Au Dang 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> --- v1->v2: * Updated commit header and description. * 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. --- drivers/tty/serial/sh-sci-common.h | 2 ++ drivers/tty/serial/sh-sci.c | 8 ++++++-- 2 files changed, 8 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 58120b523ca1..aa179c787487 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 */ @@ -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->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); @@ -3653,6 +3655,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 = { @@ -3791,6 +3794,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"); -- 2.43.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 3/4] serial: sh-sci: Skip TDxE clear when hardware clears it automatically 2026-10-21 13:13 ` [PATCH v2 3/4] serial: sh-sci: Skip TDxE clear when hardware clears it automatically Biju Das @ 2026-10-21 13:22 ` sashiko-bot 2026-10-22 10:29 ` Biju Das 0 siblings, 1 reply; 11+ messages in thread From: sashiko-bot @ 2026-10-21 13:22 UTC (permalink / raw) To: Biju Das; +Cc: linux-serial, kvm, wsa+renesas Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] serial: sh-sci: missing has_tdxe_auto_clear init in scix_early_console_setup() -- commit 7c4175b5ac1e1b0c0d116921a097acf598dc7be1 Author: Au Dang <au.dang.jx@renesas.com> serial: sh-sci: Skip TDxE clear when hardware clears it automatically This commit adds a tdxe_auto_clear flag to struct sci_of_data and propagates it to struct sci_port as has_tdxe_auto_clear to skip software clearing of TDxE in sci_transmit_chars() and sci_poll_put_char() on PORT_SCI variants. > diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c > index 58120b523ca1e..aa179c7874878 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 */ [ ... ] > @@ -3791,6 +3794,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; [Severity: Medium] Should scix_early_console_setup() in drivers/tty/serial/sh-sci.c also copy data->tdxe_auto_clear to sci_ports[0].has_tdxe_auto_clear? When earlycon is enabled on a renesas,sci port, sci_early_console_setup() -> scix_early_console_setup() initializes sci_ports[0] from of_sci_sci, but leaves has_tdxe_auto_clear unset: drivers/tty/serial/sh-sci.c:scix_early_console_setup() { ... sci_ports[0].cfg = &port_cfg; sci_ports[0].params = data->params; sci_ports[0].ops = data->ops; sci_ports[0].port.ops = data->uart_ops; sci_uart_earlycon = true; ... } During early console output via serial_console_write() -> uart_console_write() -> serial_console_putchar() -> sci_poll_put_char(), s->has_tdxe_auto_clear will still be false on sci_ports[0], causing s->ops->clear_SCxSR() to be called in software instead of being skipped. > > sp->has_rtscts = of_property_read_bool(np, "uart-has-rtscts"); > -- Sashiko AI review · https://sashiko.dev/#/patchset/20261021-g2ul_sci-v2-0-b4e038220018@bp.renesas.com?part=3 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 3/4] serial: sh-sci: Skip TDxE clear when hardware clears it automatically 2026-10-21 13:22 ` sashiko-bot @ 2026-10-22 10:29 ` Biju Das 0 siblings, 0 replies; 11+ messages in thread From: Biju Das @ 2026-10-22 10:29 UTC (permalink / raw) To: sashiko-reviews; +Cc: linux-serial, kvm, wsa+renesas, linux-renesas-soc On Wed, Oct 21, 2026 at 2:22 PM <sashiko-bot@kernel.org> wrote: > > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: > - [Medium] serial: sh-sci: missing has_tdxe_auto_clear init in scix_early_console_setup() > -- > > commit 7c4175b5ac1e1b0c0d116921a097acf598dc7be1 > Author: Au Dang <au.dang.jx@renesas.com> > > serial: sh-sci: Skip TDxE clear when hardware clears it automatically > > This commit adds a tdxe_auto_clear flag to struct sci_of_data and propagates > it to struct sci_port as has_tdxe_auto_clear to skip software clearing of > TDxE in sci_transmit_chars() and sci_poll_put_char() on PORT_SCI variants. > > > diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c > > index 58120b523ca1e..aa179c7874878 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 */ > > [ ... ] > > > @@ -3791,6 +3794,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; > > [Severity: Medium] > Should scix_early_console_setup() in drivers/tty/serial/sh-sci.c also copy > data->tdxe_auto_clear to sci_ports[0].has_tdxe_auto_clear? > > When earlycon is enabled on a renesas,sci port, sci_early_console_setup() -> > scix_early_console_setup() initializes sci_ports[0] from of_sci_sci, but > leaves has_tdxe_auto_clear unset: > > drivers/tty/serial/sh-sci.c:scix_early_console_setup() { > ... > sci_ports[0].cfg = &port_cfg; > sci_ports[0].params = data->params; > sci_ports[0].ops = data->ops; > sci_ports[0].port.ops = data->uart_ops; > sci_uart_earlycon = true; > ... > } > > During early console output via serial_console_write() -> > uart_console_write() -> serial_console_putchar() -> sci_poll_put_char(), > s->has_tdxe_auto_clear will still be false on sci_ports[0], causing > s->ops->clear_SCxSR() to be called in software instead of being skipped. > > > > > sp->has_rtscts = of_property_read_bool(np, "uart-has-rtscts"); > > > > -- > Sashiko AI review · https://sashiko.dev/#/patchset/20261021-g2ul_sci-v2-0-b4e038220018@bp.renesas.com?part=3 Modified patch based on above review. From 0efbe7abe7fc1ec3354fcdc1f4c02b77eaac7904 Mon Sep 17 00:00:00 2001 From: Au Dang <au.dang.jx@renesas.com> Date: Mon, 19 Oct 2026 18:01:11 +0100 Subject: [PATCH 3/4] serial: sh-sci: Skip TDxE clear when hardware clears it automatically 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> --- v1->v2: * Updated commit header and description. * 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. --- 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 6df25fef5a5f..1a4d9467d2be 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.43.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 4/4] serial: sh-sci: Toggle TE when restoring SCSCR in console write on SCI 2026-10-21 13:13 [PATCH v2 0/4] Fix transmit handling on SCI/RSCI ports Biju Das ` (2 preceding siblings ...) 2026-10-21 13:13 ` [PATCH v2 3/4] serial: sh-sci: Skip TDxE clear when hardware clears it automatically Biju Das @ 2026-10-21 13:13 ` Biju Das 2026-10-21 13:19 ` sashiko-bot 3 siblings, 1 reply; 11+ messages in thread From: Biju Das @ 2026-10-21 13:13 UTC (permalink / raw) To: Greg Kroah-Hartman, Jiri Slaby, Kiryl Shutsemau, Rick Edgecombe, Dave Hansen Cc: linux-kernel, linux-serial, linux-renesas-soc, x86, linux-coco, kvm, Biju Das, Au Dang 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> --- v1->v2: * No change. --- 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 aa179c787487..993f330e6881 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 4/4] serial: sh-sci: Toggle TE when restoring SCSCR in console write on SCI 2026-10-21 13:13 ` [PATCH v2 4/4] serial: sh-sci: Toggle TE when restoring SCSCR in console write on SCI Biju Das @ 2026-10-21 13:19 ` sashiko-bot 0 siblings, 0 replies; 11+ messages in thread From: sashiko-bot @ 2026-10-21 13:19 UTC (permalink / raw) To: Biju Das; +Cc: linux-serial > 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/20261021-g2ul_sci-v2-0-b4e038220018@bp.renesas.com?part=4 ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2026-10-22 10:30 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-10-21 13:13 [PATCH v2 0/4] Fix transmit handling on SCI/RSCI ports Biju Das 2026-10-21 13:13 ` [PATCH v2 1/4] serial: sh-sci: Drop TE disable at end of transmission on SCI Biju Das 2026-10-21 13:18 ` sashiko-bot 2026-10-21 13:13 ` [PATCH v2 2/4] serial: sh-sci: Keep TX enabled on TEI while data is pending Biju Das 2026-10-21 13:38 ` sashiko-bot 2026-10-22 10:28 ` Biju Das 2026-10-21 13:13 ` [PATCH v2 3/4] serial: sh-sci: Skip TDxE clear when hardware clears it automatically Biju Das 2026-10-21 13:22 ` sashiko-bot 2026-10-22 10:29 ` Biju Das 2026-10-21 13:13 ` [PATCH v2 4/4] serial: sh-sci: Toggle TE when restoring SCSCR in console write on SCI Biju Das 2026-10-21 13:19 ` sashiko-bot
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox