lkml.org 
[lkml]   [2020]   [Mar]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH] serial: sh-sci: Make sure status register SCxSR is read in correct sequence
Hi Fujita-san,

CC -stable, +sasha, +seebe

On Fri, Mar 27, 2020 at 7:17 PM Kazuhiro Fujita
<kazuhiro.fujita.jg@renesas.com> wrote:
> For SCIF and HSCIF interfaces the SCxSR register holds the status of
> data that is to be read next from SCxRDR register, But where as for
> SCIFA and SCIFB interfaces SCxSR register holds status of data that is
> previously read from SCxRDR register.
>
> This patch makes sure the status register is read depending on the port
> types so that errors are caught accordingly.
>
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Kazuhiro Fujita <kazuhiro.fujita.jg@renesas.com>
> Signed-off-by: Hao Bui <hao.bui.yg@renesas.com>
> Signed-off-by: KAZUMI HARADA <kazumi.harada.rh@renesas.com>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Thanks for your patch!

> --- a/drivers/tty/serial/sh-sci.c
> +++ b/drivers/tty/serial/sh-sci.c
> @@ -870,9 +870,16 @@ static void sci_receive_chars(struct uart_port *port)
> tty_insert_flip_char(tport, c, TTY_NORMAL);
> } else {
> for (i = 0; i < count; i++) {
> - char c = serial_port_in(port, SCxRDR);
> -
> - status = serial_port_in(port, SCxSR);
> + char c;
> +
> + if (port->type == PORT_SCIF ||
> + port->type == PORT_HSCIF) {
> + status = serial_port_in(port, SCxSR);
> + c = serial_port_in(port, SCxRDR);
> + } else {
> + c = serial_port_in(port, SCxRDR);
> + status = serial_port_in(port, SCxSR);
> + }
> if (uart_handle_sysrq_char(port, c)) {
> count--; i--;
> continue;

I can confirm that the documentation for the Serial Status Register on
1. (H)SCIF on R-Car Gen1/2/3 says the framing/error flag applies to
the data that is "to be read next" from the FIFO., and that the
"Sample Flowchart for Serial Reception (2)" confirms this,
2. SCIF[AB] on R-Car Gen2, SH-Mobile AG5, R-Mobile A1 and APE6 says
the framing/error flag applies to the receive data that is "read"
from the FIFO, and that the "Example of Flow for Serial Reception
(2)" confirms this,
3. SCIF on RZ/A1H says something similar as for (H)SCIF above, using
slightly different wording, also confirmed by the "Sample Flowchart
for Receiving Serial Data (2)".

However, the documentation for "SCIFA" on RZ/A2 (for which we use
PORT_SCIF, not PORT_SCIFA, in the driver) has conflicting information:
1. Section 17.2.7 "Serial Status Register (FSR)" says:
- A receive framing/parity error occurred in the "next receive
data read" from the FIFO,
- Indicates whether there is a framing/parity error in the data
"read" from the FIFO.
2. Figure 17.8 "Sample Flowchart for Receiving Serial Data in
Asynchronous Mode (2)".
- Whether a framing error or parity error has occurred in the
received data that is "read" from the FIFO.

So while the change looks OK for most Renesas ARM SoCs, the situation
for RZ/A2 is unclear.
Note that the above does not take into account variants used on SuperH
SoCs.

Nevertheless, this patch will need some testing on various hardware.
Do you have a test case to verify the broken/fixed behavior?

Thanks!

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

\
 
 \ /
  Last update: 2020-03-31 17:18    [W:0.070 / U:2.584 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site