[PATCH 0/4] serial: sh-sci: FIFO initialization fixes

[PATCH 0/4] serial: sh-sci: FIFO initialization fixes

[Geert Uytterhoeven]

	Hi Greg, Jiri, Magnus,

When opening a Renesas SCIF serial port after it has been used before,
stale data may be read.  This has been observed on R-Car Gen2 and R-Car
Gen3, with all SCIF variants present (SCIF, SCIFA, SCIFB, and HSCIF). It
is much more likely to happen when DMA is enabled, although it has been
seen with PIO, too.

There are actually two reasons why stale data is received:
  1. Transfers are started, or are still activated, before the FIFO is
     reset, causing one or more (up to the RX FIFO size) stale bytes
     (fixed by patches 1 and 2),
  2. FIFO reset lacked clearing the RDF flag, causing one byte of stale
     data (fixed by patch 3).
While at it, patch 4 adds the missing clearing of two other flags in the
initialization sequence on (H)SCIF.

While the issue can be reproduced using subsequent runs of sertest[1],
I wrote a new test program, fifotest[2], to trigger it more easily.
More detailed test information can be found on the eLinux wiki[3].

I have verified that this series fixes the issue on SCIF, SCIFA, SCIFB,
and HSCIF, on R-Car Gen2 (r8a7791/koelsch) and R-Car Gen3
(r8a7795/salvator-x).
Basic regression testing has been done with SCIFA on sh73a0/kzm9g.
Regression testing on other variants (notably SCI) would be appreciated.

This series applies against both v4.6 and next-20160526. It's also
available in a topic branch at
https://git.kernel.org/cgit/linux/kernel/git/geert/renesas-drivers.git/log/?h=topic/scif-fifo-v1.

Thanks!

[1] https://github.com/geertu/sertest
[2] https://github.com/geertu/fifotest
[3] http://elinux.org/Tests:SCIF-FIFO


Geert Uytterhoeven (4):
  serial: sh-sci: Do not start transfers from sci_startup()
  serial: sh-sci: Stop transfers in sci_shutdown()
  serial: sh-sci: Clear RX, error, and break flags during reset
  serial: sh-sci: Clear (H)SCIF timeout and overrun during reset

 drivers/tty/serial/sh-sci.c | 17 +++++++++++------
 drivers/tty/serial/sh-sci.h |  1 +
 2 files changed, 12 insertions(+), 6 deletions(-)

-- 
1.9.1

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

[PATCH 1/4] serial: sh-sci: Do not start transfers from sci_startup()

[Geert Uytterhoeven]

FIFO reset is done in sci_reset(), called from sci_set_termios(), while
sci_start_tx() and sci_start_rx() are called before, from sci_startup().
However, starting transfers before the UART's FIFOs have been reset may
cause reading of stale data.

Remove the calls to sci_start_tx() and sci_start_rx() from sci_startup()
to fix this.

Transfers are still started when needed:
  - sci_start_rx() is called from sci_set_termios() after FIFO reset, if
    the CREAD flag is set,
  - sci_start_tx() is called from uart_change_speed() immediately
    thereafter, if transmission is enabled.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/tty/serial/sh-sci.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 0130feb069aee02f..d6ba90c572f7475c 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -1873,7 +1873,6 @@ static void sci_break_ctl(struct uart_port *port, int break_state)
 static int sci_startup(struct uart_port *port)
 {
 	struct sci_port *s = to_sci_port(port);
-	unsigned long flags;
 	int ret;
 
 	dev_dbg(port->dev, "%s(%d)\n", __func__, port->line);
@@ -1884,11 +1883,6 @@ static int sci_startup(struct uart_port *port)
 
 	sci_request_dma(port);
 
-	spin_lock_irqsave(&port->lock, flags);
-	sci_start_tx(port);
-	sci_start_rx(port);
-	spin_unlock_irqrestore(&port->lock, flags);
-
 	return 0;
 }
 
-- 
1.9.1

[PATCH 2/4] serial: sh-sci: Stop transfers in sci_shutdown()

[Geert Uytterhoeven]

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@renesas.com>.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
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,
  - Enhanced patch description.
---
 drivers/tty/serial/sh-sci.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index d6ba90c572f7475c..4d2f916f45277e20 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -1896,6 +1896,8 @@ 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);
 	spin_unlock_irqrestore(&port->lock, flags);
 
 #ifdef CONFIG_SERIAL_SH_SCI_DMA
-- 
1.9.1

[PATCH 3/4] serial: sh-sci: Clear RX, error, and break flags during reset

[Geert Uytterhoeven]

Setting the FIFO reset bits is not sufficient to reset the RX FIFO.
After this the status register's RDF flag bit may still be set, causing
the reception of one stale byte of data.

To fix this, clear all status flag bits related to reception, error, and
break handling, cfr. the initialization flowchart in the datasheet.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/tty/serial/sh-sci.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 4d2f916f45277e20..549d799cb18fafe7 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -2052,6 +2052,10 @@ static void sci_reset(struct uart_port *port)
 	reg = sci_getreg(port, SCFCR);
 	if (reg->size)
 		serial_port_out(port, SCFCR, SCFCR_RFRST | SCFCR_TFRST);
+
+	sci_clear_SCxSR(port,
+			SCxSR_RDxF_CLEAR(port) & SCxSR_ERROR_CLEAR(port) &
+			SCxSR_BREAK_CLEAR(port));
 }
 
 static void sci_set_termios(struct uart_port *port, struct ktermios *termios,
-- 
1.9.1

[PATCH 4/4] serial: sh-sci: Clear (H)SCIF timeout and overrun during reset

[Geert Uytterhoeven]

Add the missing timeout bit definition for (H)SCIF.
Clear the timeout and overrun flag bits during UART reset, cfr. the
initialization flowchart in the datasheet.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/tty/serial/sh-sci.c | 5 +++++
 drivers/tty/serial/sh-sci.h | 1 +
 2 files changed, 6 insertions(+)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 549d799cb18fafe7..fe9ad1b56420876a 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -2056,6 +2056,11 @@ static void sci_reset(struct uart_port *port)
 	sci_clear_SCxSR(port,
 			SCxSR_RDxF_CLEAR(port) & SCxSR_ERROR_CLEAR(port) &
 			SCxSR_BREAK_CLEAR(port));
+	if (sci_getreg(port, SCLSR)->size) {
+		status = serial_port_in(port, SCLSR);
+		status &= ~(SCLSR_TO | SCLSR_ORER);
+		serial_port_out(port, SCLSR, status);
+	}
 }
 
 static void sci_set_termios(struct uart_port *port, struct ktermios *termios,
diff --git a/drivers/tty/serial/sh-sci.h b/drivers/tty/serial/sh-sci.h
index 7a4fa185b93ef307..c590418d2a40d78b 100644
--- a/drivers/tty/serial/sh-sci.h
+++ b/drivers/tty/serial/sh-sci.h
@@ -105,6 +105,7 @@ enum {
 #define SCFCR_LOOP	BIT(0)	/* Loopback Test */
 
 /* SCLSR (Line Status Register) on (H)SCIF */
+#define SCLSR_TO	BIT(2)	/* Timeout */
 #define SCLSR_ORER	BIT(0)	/* Overrun Error */
 
 /* SCSPTR (Serial Port Register), optional */
-- 
1.9.1

RE: [PATCH 2/4] serial: sh-sci: Stop transfers in sci_shutdown()

[Yoshihiro Shimoda]

Hi, Geert-san,

> 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@renesas.com>.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> 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));
+	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 */
+
 	serial_port_out(port, SCSCR, ctrl_temp);
 
 	uart_console_write(port, s, count, serial_console_putchar);

Re: [PATCH 2/4] serial: sh-sci: Stop transfers in sci_shutdown()

[Geert Uytterhoeven]

Hi Shimoda-san,

On Tue, Jun 21, 2016 at 9:35 AM, Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> 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@renesas.com>.
>>
>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>> ---
>> 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?

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

RE: [PATCH 2/4] serial: sh-sci: Stop transfers in sci_shutdown()

[Yoshihiro Shimoda]

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@renesas.com> 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@renesas.com>.
> >>
> >> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >> ---
> >> 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@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

Re: [PATCH 2/4] serial: sh-sci: Stop transfers in sci_shutdown()

[Geert Uytterhoeven]

Hi Shimoda-san,

On Thu, Jun 23, 2016 at 12:42 PM, Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
>> From: Geert Uytterhoeven
>> Sent: Tuesday, June 21, 2016 11:52 PM
>> On Tue, Jun 21, 2016 at 9:35 AM, Yoshihiro Shimoda
>> <yoshihiro.shimoda.uh@renesas.com> 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@renesas.com>.
>> >>
>> >> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>> >> ---
>> >> 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...)

Thanks, I will send an updated patch shortly.

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