[PATCH v2 11/19] serial: sh: Fix error handling

[Biju Das]

Hi Paul,

Thanks for the patch.

> Subject: [PATCH v2 11/19] serial: sh: Fix error handling
> 
> The current SCIF error handling is broken for the RZ/G2L. After a break
> condition has been triggered, the current code is unable to clear the error
> and serial port output never resumes.
> 
> The RZ/G2L datasheet says that most error conditions are cleared by
> resetting the relevant error bits in the FSR & LSR registers to zero.
> To clear framing errors, the invalid data also needs to be read out of the
> receive FIFO.
> 
> After reviewing datasheets for RZ/G2{H,M,N,E}, R-Car Gen4, R-Car Gen3 and
> even SH7751 SoCs, it's clear that this is the way to clear errors for all
> of these SoCs.
> 
> While we're here, annotate the handle_error() function with a couple of
> comments as the reads and writes themselves don't immediately make it clear
> what we're doing.
> 
> Signed-off-by: Paul Barker <paul.barker.ct at bp.renesas.com>
> ---
> v1->v2:
>   * New patch after discussion with Marek & further investigation.
> 
> I'm not going to put a `Fixes` tag on this, the error handling has been
> writing ones to clear errors since at least 2007 so maybe it was originally
> right for some board. It doesn't seem to apply to any of the current users
> of the SCIF driver though.
> 
>  drivers/serial/serial_sh.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/serial/serial_sh.c b/drivers/serial/serial_sh.c index
> 5e543dbf3d58..2b650d458e71 100644
> --- a/drivers/serial/serial_sh.c
> +++ b/drivers/serial/serial_sh.c
> @@ -79,10 +79,22 @@ sh_serial_setbrg_generic(struct uart_port *port, int
> clk, int baudrate)
> 
>  static void handle_error(struct uart_port *port)  {
> -	sci_in(port, SCxSR);
> -	sci_out(port, SCxSR, SCxSR_ERROR_CLEAR(port));
> +	/*
> +	 * Most errors are cleared by resetting the relevant error bits to
> zero
> +	 * in the FSR & LSR registers. For each register, a read followed by
> a
> +	 * write is needed according to the relevant datasheets.
> +	 */
> +	unsigned short status = sci_in(port, SCxSR);
> +	sci_out(port, SCxSR, status & ~SCIF_ERRORS);
>  	sci_in(port, SCLSR);
>  	sci_out(port, SCLSR, 0x00);
> +
> +	/*
> +	 * To clear framing errors, we also need to read and discard a
> +	 * character.
> +	 */
> +	if (status & SCIF_FER)
> +		sci_in(port, SCxRDR);

Do we need to read status again to make sure framing error
is cleared?

Cheers,
Biju

>  }
> 
>  static int serial_raw_putc(struct uart_port *port, const char c)
> --
> 2.39.2