| Message ID | 20230320105339.236279-4-biju.das.jz@bp.renesas.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Renesas SCI fixes | expand |
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 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 > > -- > 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
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)) {
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(-)