For SCI, the TE (transmit enable) must be set after setting TIE (transmit interrupt enable) or in the same instruction to start the transmission. Set TE bit in sci_start_tx() instead of set_termios() for SCI and clear TE bit, if circular buffer is empty in sci_transmit_chars().
Fixes: f9a2adcc9e90 ("arm64: dts: renesas: r9a07g044: Add SCI[0-1] nodes") Cc: stable@vger.kernel.org Signed-off-by: Biju Das biju.das.jz@bp.renesas.com --- v3: * New patch --- drivers/tty/serial/sh-sci.c | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c index b9cd27451f90..9079a8ea9132 100644 --- a/drivers/tty/serial/sh-sci.c +++ b/drivers/tty/serial/sh-sci.c @@ -597,6 +597,15 @@ static void sci_start_tx(struct uart_port *port) if (!s->chan_tx || port->type == PORT_SCIFA || port->type == PORT_SCIFB) { /* Set TIE (Transmit Interrupt Enable) bit in SCSCR */ ctrl = serial_port_in(port, SCSCR); + + /* + * For SCI, TE (transmit enable) must be set after setting TIE + * (transmit interrupt enable) or in the same instruction to start + * the transmit process. + */ + if (port->type == PORT_SCI) + ctrl |= SCSCR_TE; + serial_port_out(port, SCSCR, ctrl | SCSCR_TIE); } } @@ -835,6 +844,12 @@ static void sci_transmit_chars(struct uart_port *port) c = xmit->buf[xmit->tail]; xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1); } else { + if (port->type == PORT_SCI) { + ctrl = serial_port_in(port, SCSCR); + ctrl &= ~SCSCR_TE; + serial_port_out(port, SCSCR, ctrl); + return; + } break; }
@@ -2581,8 +2596,14 @@ static void sci_set_termios(struct uart_port *port, struct ktermios *termios, sci_set_mctrl(port, port->mctrl); }
- scr_val |= SCSCR_RE | SCSCR_TE | - (s->cfg->scscr & ~(SCSCR_CKE1 | SCSCR_CKE0)); + /* + * For SCI, TE (transmit enable) must be set after setting TIE + * (transmit interrupt enable) or in the same instruction to + * start the transmitting process. So skip setting TE here for SCI. + */ + if (port->type != PORT_SCI) + scr_val |= SCSCR_TE; + scr_val |= SCSCR_RE | (s->cfg->scscr & ~(SCSCR_CKE1 | SCSCR_CKE0)); serial_port_out(port, SCSCR, scr_val | s->hscif_tot); if ((srr + 1 == 5) && (port->type == PORT_SCIFA || port->type == PORT_SCIFB)) {
Hi Biju,
On Mon, Mar 20, 2023 at 11:53 AM Biju Das biju.das.jz@bp.renesas.com wrote:
For SCI, the TE (transmit enable) must be set after setting TIE (transmit interrupt enable) or in the same instruction to start the transmission. Set TE bit in sci_start_tx() instead of set_termios() for SCI and clear TE bit, if circular buffer is empty in sci_transmit_chars().
Fixes: f9a2adcc9e90 ("arm64: dts: renesas: r9a07g044: Add SCI[0-1] nodes")
That's a DTS patch?
I'm wondering if this got broken accidentally by commit 93bcefd4c6bad4c6 ("serial: sh-sci: Fix setting SCSCR_TIE while transferring data"), which was probably never tested on SCI.
Cc: stable@vger.kernel.org Signed-off-by: Biju Das biju.das.jz@bp.renesas.com
v3:
- New patch
drivers/tty/serial/sh-sci.c | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c index b9cd27451f90..9079a8ea9132 100644 --- a/drivers/tty/serial/sh-sci.c +++ b/drivers/tty/serial/sh-sci.c @@ -597,6 +597,15 @@ static void sci_start_tx(struct uart_port *port) if (!s->chan_tx || port->type == PORT_SCIFA || port->type == PORT_SCIFB) { /* Set TIE (Transmit Interrupt Enable) bit in SCSCR */ ctrl = serial_port_in(port, SCSCR);
/** For SCI, TE (transmit enable) must be set after setting TIE* (transmit interrupt enable) or in the same instruction to start* the transmit process.*/if (port->type == PORT_SCI)ctrl |= SCSCR_TE;serial_port_out(port, SCSCR, ctrl | SCSCR_TIE); }} @@ -835,6 +844,12 @@ static void sci_transmit_chars(struct uart_port *port) c = xmit->buf[xmit->tail]; xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1); } else {
if (port->type == PORT_SCI) {ctrl = serial_port_in(port, SCSCR);ctrl &= ~SCSCR_TE;serial_port_out(port, SCSCR, ctrl);return;} break; }@@ -2581,8 +2596,14 @@ static void sci_set_termios(struct uart_port *port, struct ktermios *termios, sci_set_mctrl(port, port->mctrl); }
scr_val |= SCSCR_RE | SCSCR_TE |(s->cfg->scscr & ~(SCSCR_CKE1 | SCSCR_CKE0));
/** For SCI, TE (transmit enable) must be set after setting TIE* (transmit interrupt enable) or in the same instruction to* start the transmitting process. So skip setting TE here for SCI.*/if (port->type != PORT_SCI)scr_val |= SCSCR_TE;scr_val |= SCSCR_RE | (s->cfg->scscr & ~(SCSCR_CKE1 | SCSCR_CKE0)); serial_port_out(port, SCSCR, scr_val | s->hscif_tot); if ((srr + 1 == 5) && (port->type == PORT_SCIFA || port->type == PORT_SCIFB)) {
Gr{oetje,eeting}s,
Geert
Hi Geert,
Thanks for the feedback.
Subject: Re: [PATCH v3 3/5] tty: serial: sh-sci: Fix Tx on SCI IP
Hi Biju,
On Mon, Mar 20, 2023 at 11:53 AM Biju Das biju.das.jz@bp.renesas.com wrote:
For SCI, the TE (transmit enable) must be set after setting TIE (transmit interrupt enable) or in the same instruction to start the
transmission.
Set TE bit in sci_start_tx() instead of set_termios() for SCI and clear TE bit, if circular buffer is empty in sci_transmit_chars().
Fixes: f9a2adcc9e90 ("arm64: dts: renesas: r9a07g044: Add SCI[0-1] nodes")
That's a DTS patch?
I'm wondering if this got broken accidentally by commit 93bcefd4c6bad4c6 ("serial: sh-sci: Fix setting SCSCR_TIE while transferring data"), which was probably never tested on SCI.
Looks like that patch is not tested on SCI. OK, will use the above commit as Fixes tag.
Cheers, Biju
Cc: stable@vger.kernel.org Signed-off-by: Biju Das biju.das.jz@bp.renesas.com
v3:
- New patch
drivers/tty/serial/sh-sci.c | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c index b9cd27451f90..9079a8ea9132 100644 --- a/drivers/tty/serial/sh-sci.c +++ b/drivers/tty/serial/sh-sci.c @@ -597,6 +597,15 @@ static void sci_start_tx(struct uart_port *port) if (!s->chan_tx || port->type == PORT_SCIFA || port->type ==
PORT_SCIFB) {
/* Set TIE (Transmit Interrupt Enable) bit in SCSCR */ ctrl = serial_port_in(port, SCSCR);
/** For SCI, TE (transmit enable) must be set after settingTIE
* (transmit interrupt enable) or in the same instructionto start
* the transmit process.*/if (port->type == PORT_SCI)ctrl |= SCSCR_TE;serial_port_out(port, SCSCR, ctrl | SCSCR_TIE); }} @@ -835,6 +844,12 @@ static void sci_transmit_chars(struct uart_port
*port)
c = xmit->buf[xmit->tail]; xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE -1);
} else {
if (port->type == PORT_SCI) {ctrl = serial_port_in(port, SCSCR);ctrl &= ~SCSCR_TE;serial_port_out(port, SCSCR, ctrl);return;} break; }@@ -2581,8 +2596,14 @@ static void sci_set_termios(struct uart_port *port,
struct ktermios *termios,
sci_set_mctrl(port, port->mctrl); }
scr_val |= SCSCR_RE | SCSCR_TE |(s->cfg->scscr & ~(SCSCR_CKE1 | SCSCR_CKE0));
/** For SCI, TE (transmit enable) must be set after setting TIE* (transmit interrupt enable) or in the same instruction to* start the transmitting process. So skip setting TE here forSCI.
*/if (port->type != PORT_SCI)scr_val |= SCSCR_TE;scr_val |= SCSCR_RE | (s->cfg->scscr & ~(SCSCR_CKE1 |- SCSCR_CKE0)); serial_port_out(port, SCSCR, scr_val | s->hscif_tot); if ((srr + 1 == 5) && (port->type == PORT_SCIFA || port->type == PORT_SCIFB)) {
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
linux-stable-mirror@lists.linaro.org
-
Biju Das -
Geert Uytterhoeven