Message ID | 1594905847-54471-1-git-send-email-biju.das.jz@bp.renesas.com (mailing list archive) |
---|---|
State | Accepted |
Delegated to: | Pavel Machek |
Headers | show |
Series | [4.4.y-cip] serial: sh-sci: Make sure status register SCxSR is read in correct sequence | expand |
Hi, > -----Original Message----- > From: Biju Das [mailto:biju.das.jz@bp.renesas.com] > Sent: Thursday, July 16, 2020 10:24 PM > To: cip-dev@lists.cip-project.org; iwamatsu nobuhiro(岩松 信洋 □SWC◯ACT) > <nobuhiro1.iwamatsu@toshiba.co.jp>; Pavel Machek <pavel@denx.de> > Cc: Chris Paterson <chris.paterson2@renesas.com>; Biju Das <biju.das.jz@bp.renesas.com>; Prabhakar Mahadev Lad > <prabhakar.mahadev-lad.rj@bp.renesas.com> > Subject: [PATCH 4.4.y-cip] serial: sh-sci: Make sure status register SCxSR is read in correct sequence > > From: Kazuhiro Fujita <kazuhiro.fujita.jg@renesas.com> > > commit 3dc4db3662366306e54ddcbda4804acb1258e4ba upstream. > > 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> > Tested-by: Geert Uytterhoeven <geert+renesas@glider.be> > Link: https://lore.kernel.org/r/1585333048-31828-1-git-send-email-kazuhiro.fujita.jg@renesas.com > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> Looks good to me. I will merge if nothing else is mentioned. Best regards, Nobuhiro > --- > drivers/tty/serial/sh-sci.c | 13 ++++++++++--- > 1 file changed, 10 insertions(+), 3 deletions(-) > > diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c > index 22f7201..2b5b342 100644 > --- a/drivers/tty/serial/sh-sci.c > +++ b/drivers/tty/serial/sh-sci.c > @@ -793,9 +793,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 defined(CONFIG_CPU_SH3) > /* Skip "chars" during break */ > if (sci_port->break_flag) { > -- > 2.7.4 -=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#4933): https://lists.cip-project.org/g/cip-dev/message/4933 Mute This Topic: https://lists.cip-project.org/mt/75541395/4520428 Group Owner: cip-dev+owner@lists.cip-project.org Unsubscribe: https://lists.cip-project.org/g/cip-dev/leave/8129116/1171672734/xyzzy [patchwork-cip-dev@patchwork.kernel.org] -=-=-=-=-=-=-=-=-=-=-=-
Hi! > From: Kazuhiro Fujita <kazuhiro.fujita.jg@renesas.com> > > commit 3dc4db3662366306e54ddcbda4804acb1258e4ba upstream. > > 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. Looks good to me. > Cc: <stable@vger.kernel.org> I notice this applies cleanly to v4.4-stable. Was this submitted to 4.4 / do you need some help there? Does this have user-visible impact? Best regards, Pavel
Hi Pavel, Thanks for the feedback. > -----Original Message----- > From: Pavel Machek <pavel@denx.de> > Sent: 17 July 2020 09:25 > To: cip-dev@lists.cip-project.org > Cc: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>; Pavel > Machek <pavel@denx.de>; Chris Paterson <Chris.Paterson2@renesas.com>; > Biju Das <biju.das.jz@bp.renesas.com>; Prabhakar Mahadev Lad > <prabhakar.mahadev-lad.rj@bp.renesas.com> > Subject: Re: [cip-dev] [PATCH 4.4.y-cip] serial: sh-sci: Make sure status > register SCxSR is read in correct sequence > > Hi! > > > From: Kazuhiro Fujita <kazuhiro.fujita.jg@renesas.com> > > > > commit 3dc4db3662366306e54ddcbda4804acb1258e4ba upstream. > > > > 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. > > Looks good to me. > > > Cc: <stable@vger.kernel.org> > > I notice this applies cleanly to v4.4-stable. Was this submitted to > 4.4 / do you need some help there? The original patch is not cleanly applied on 4.4 see the mail below https://www.spinics.net/lists/linux-renesas-soc/msg48219.html > Does this have user-visible impact? No, You can verify this on console after applying the patch. stty evenp Test Case 1: paste "p" on the console should generate parity error and "o" should not Test Case 2:- paste "ppppp" on the console should generate parity error and "ooooo" should not Testcase 3:- Paste "pop", it should generate parity error for "p" Regards, Biju Renesas Electronics Europe GmbH, Geschaeftsfuehrer/President: Carsten Jauch, Sitz der Gesellschaft/Registered office: Duesseldorf, Arcadiastrasse 10, 40472 Duesseldorf, Germany, Handelsregister/Commercial Register: Duesseldorf, HRB 3708 USt-IDNr./Tax identification no.: DE 119353406 WEEE-Reg.-Nr./WEEE reg. no.: DE 14978647 -=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#4936): https://lists.cip-project.org/g/cip-dev/message/4936 Mute This Topic: https://lists.cip-project.org/mt/75541395/4520428 Group Owner: cip-dev+owner@lists.cip-project.org Unsubscribe: https://lists.cip-project.org/g/cip-dev/leave/8129116/1171672734/xyzzy [patchwork-cip-dev@patchwork.kernel.org] -=-=-=-=-=-=-=-=-=-=-=-
Hi! > > From: Kazuhiro Fujita <kazuhiro.fujita.jg@renesas.com> > > > > commit 3dc4db3662366306e54ddcbda4804acb1258e4ba upstream. > > > > 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> > > Tested-by: Geert Uytterhoeven <geert+renesas@glider.be> > > Link: https://lore.kernel.org/r/1585333048-31828-1-git-send-email-kazuhiro.fujita.jg@renesas.com > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > Looks good to me. > I will merge if nothing else is mentioned. It looks good to me, and it passed basic testing -- so I applied the patch and am pushing it. (I hope you don't mind). Best regards, Pavel
diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c index 22f7201..2b5b342 100644 --- a/drivers/tty/serial/sh-sci.c +++ b/drivers/tty/serial/sh-sci.c @@ -793,9 +793,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 defined(CONFIG_CPU_SH3) /* Skip "chars" during break */ if (sci_port->break_flag) {