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

[Paul Barker]

On Tue, Oct 10, 2023 at 07:08:42AM +0000, Biju Das wrote:
> 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?

I've not seen anything to suggest that we need to do that and this
implementation is clearing the framing errors successfully.

Thanks,
Paul
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20231012/9d8e57d2/attachment.sig>