Hi Geert-san,
> From: Geert Uytterhoeven
> Sent: Tuesday, June 21, 2016 11:52 PM
>
> Hi Shimoda-san,
>
> On Tue, Jun 21, 2016 at 9:35 AM, Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@xxxxxxxxxxx> wrote:
> >> From: Geert Uytterhoeven
> >> Sent: Friday, May 27, 2016 3:19 AM
> >>
> >> Make sure the transmitter and receiver are stopped when shutting down
> >> the port, and related interrupts are disabled.
> >>
> >> Without this:
> >> - New input data may be received into the RX FIFO, possibly
> >> triggering a new RX DMA completion,
> >> - Transfers will still be enabled on a subsequent startup of the UART,
> >> before the UART's FIFOs have been reset, causing reading of stale
> >> data.
> >>
> >> Inspired by a patch in the BSP by Koji Matsuoka
> >> <koji.matsuoka.xm@xxxxxxxxxxx>.
> >>
> >> Signed-off-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
> >> ---
> >> Extracted from "[PATCH/RFC v3 0/4] serial: sh-sci: Add DT DMA support".
> >> The issues with the serial console seen before on r8a7740/armadillo and
> >> sh73a0/kzm9g seem to be gone.
> >>
> >> Changes after resurrection:
> >> - Write zero to also disable related interrupts, as suggested by
> >> Laurent Pinchart,
> >
> > If we write zero to the register, we cannot use the port as a console after it is called.
> > In fact, I have an issue while rc scripts are running on my root filesystem.
> > When rc scripts is running, "shutdown" is called a lot.
> > After the "shutdown", if the kernel will put strings using a console, it cannot put strings
> > because the register is zero (TE and CKE are 0). So, we have to consider it.
> >
> > FYI, I made a patch to fix this issue.
> > (Perhaps, both the CKE and TE should be set in the serial_console_write(), but I don't know how to set the CKE for now :) )
> >
> > Best regards,
> > Yoshihiro Shimoda
> > ---
> > diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
> > index afa25ec..b5b1b38 100644
> > --- a/drivers/tty/serial/sh-sci.c
> > +++ b/drivers/tty/serial/sh-sci.c
> > @@ -1989,6 +1989,7 @@ static void sci_shutdown(struct uart_port *port)
> > {
> > struct sci_port *s = to_sci_port(port);
> > unsigned long flags;
> > + unsigned int ctrl;
> >
> > dev_dbg(port->dev, "%s(%d)\n", __func__, port->line);
> >
> > @@ -1998,8 +1999,12 @@ static void sci_shutdown(struct uart_port *port)
> > spin_lock_irqsave(&port->lock, flags);
> > sci_stop_rx(port);
> > sci_stop_tx(port);
> > - /* Stop RX and TX, disable related interrupts */
> > - serial_port_out(port, SCSCR, 0);
> > + /* Stop RX and TX, disable related interrupts, keep clock source */
> > + ctrl = serial_port_in(port, SCSCR);
> > + ctrl = (s->cfg->scscr & ~(SCSCR_CKE1 | SCSCR_CKE0)) |
> > + (ctrl & (SCSCR_CKE1 | SCSCR_CKE0));
>
> My bad. We should indeed keep CKE, as the serial console relies on that.
> I'm just wondering why I didn't notice this, as at least on Koelsch, the
> external SCIF clock is used, implying a non-zero CKEx setting.
>
> > + serial_port_out(port, SCSCR, ctrl);
> > +
> > spin_unlock_irqrestore(&port->lock, flags);
> >
> > #ifdef CONFIG_SERIAL_SH_SCI_DMA
> > @@ -2801,6 +2806,8 @@ static void serial_console_write(struct console *co, const char *s,
> > ctrl = serial_port_in(port, SCSCR);
> > ctrl_temp = (sci_port->cfg->scscr & ~(SCSCR_CKE1 | SCSCR_CKE0)) |
> > (ctrl & (SCSCR_CKE1 | SCSCR_CKE0));
> > + ctrl_temp |= SCSCR_TE; /* FIXME: while "break ctl" is on */
>
> This shouldn't be needed, as SCSCR_TE should be set in sci_port->cfg->scscr
> (look in all places where it's initialized).
> Can you please double check?
Sorry for the check (because I took a day off yesterday).
As you mentioned it, this is not needed.
(I should have tested on such a code before I sent this report...)
Best regards,
Yoshihiro Shimoda
> > +
> > serial_port_out(port, SCSCR, ctrl_temp);
> >
> > uart_console_write(port, s, count, serial_console_putchar);
>
> Thanks for your report!
>
> 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
