Re: [PATCH v3 3/5] tty: serial: sh-sci: Fix Tx on SCI IP

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

> 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@xxxxxxxxxxxxxx

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



[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux