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@xxxxxxxxxxxxxx>
> 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@xxxxxxxxxxxxxxx
> > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> > ---
> > 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
