Re: [PATCH v4 5/5] tty: serial: sh-sci: Fix end of transmission on SCI

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

 



On Fri, 21 Apr 2023, Geert Uytterhoeven wrote:

> Hi Biju,
> 
> On Wed, Apr 12, 2023 at 4:51 PM Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote:
> > We need to set TE to "0" (i.e., disable serial transmission) to
> > get the expected behavior of the end of serial transmission.
> >
> > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> 
> Thanks for your patch!
> 
> > --- a/drivers/tty/serial/sh-sci.c
> > +++ b/drivers/tty/serial/sh-sci.c
> > @@ -847,6 +847,11 @@ static void sci_transmit_chars(struct uart_port *port)
> >                 } else if (!uart_circ_empty(xmit) && !stopped) {
> >                         c = xmit->buf[xmit->tail];
> >                         xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
> > +               } else if (port->type == PORT_SCI && uart_circ_empty(xmit)) {
> > +                       ctrl = serial_port_in(port, SCSCR);
> > +                       ctrl &= ~SCSCR_TE;
> > +                       serial_port_out(port, SCSCR, ctrl);
> > +                       return;
> 
> So nothing after the while loop should be done anymore?
> What about clearing SCSCR_TE in sci_stop_tx() (which is called after
> the while loop) instead?

Yes, this added fragment doesn't really seem to belong into the tx loop.

The right direction would be to aim towards converting the whole tx loop 
into using uart_port_tx_limited().

-- 
 i.

> >                 } else {
> >                         break;
> >                 }
> 
> So combined with my comments on patch 4/5, that would become
> 
> -    if (port->type == PORT_SCI)
> +    if (port->type == PORT_SCI) {
>              ctrl |= SCSCR_TEIE;
> +            ctrl &= ~SCSCR_TE;
> +    }
> 
> in sci_stop_tx().

[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux PPP]     [Linux FS]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Linmodem]     [Device Mapper]     [Linux Kernel for ARM]

  Powered by Linux