linux-kernel - RE: [PATCH v3 13/13] serial: sh-sci: Add support for RZ/G3E RSCI SCI

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID:
 <TY3PR01MB11346FCA297B3FB1F2F5D19DC86D7A@TY3PR01MB11346.jpnprd01.prod.outlook.com>
Date: Wed, 19 Nov 2025 11:49:17 +0000
From: Biju Das <biju.das.jz@...renesas.com>
To: Jiri Slaby <jirislaby@...nel.org>, biju.das.au <biju.das.au@...il.com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Rob Herring
	<robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
	<conor+dt@...nel.org>, Geert Uytterhoeven <geert+renesas@...der.be>,
	magnus.damm <magnus.damm@...il.com>
CC: wsa+renesas <wsa+renesas@...g-engineering.com>, Prabhakar Mahadev Lad
	<prabhakar.mahadev-lad.rj@...renesas.com>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "linux-serial@...r.kernel.org"
	<linux-serial@...r.kernel.org>, "devicetree@...r.kernel.org"
	<devicetree@...r.kernel.org>, "linux-renesas-soc@...r.kernel.org"
	<linux-renesas-soc@...r.kernel.org>
Subject: RE: [PATCH v3 13/13] serial: sh-sci: Add support for RZ/G3E RSCI SCI

Hi Jiri Slaby,

Thanks for the feedback.

> -----Original Message-----
> From: Jiri Slaby <jirislaby@...nel.org>
> Sent: 18 November 2025 09:06
-
> serial@...r.kernel.org; devicetree@...r.kernel.org; linux-renesas-soc@...r.kernel.org
> Subject: Re: [PATCH v3 13/13] serial: sh-sci: Add support for RZ/G3E RSCI SCI
> 
> Hi,
> 
> On 14. 11. 25, 11:51, Biju wrote:
> > From: Biju Das <biju.das.jz@...renesas.com>
> >
> > Add support for RZ/G3E RSCI SCI(a.k.a non FIFO mode).
> 
> "a.k.a. non-FIFO"

OK.

> 
> > Signed-off-by: Biju Das <biju.das.jz@...renesas.com>
> ...
> > @@ -496,34 +521,40 @@ static void rsci_receive_chars(struct uart_port *port)
> >   		if (count == 0)
> >   			break;
> >
> > -		for (i = 0; i < count; i++) {
> > -			char c;
> > -
> > -			rdat = rsci_serial_in(port, RDR);
> > -			/* 9-bits data is not supported yet */
> > -			c = rdat & RDR_RDAT_MSK;
> > -
> > -			if (uart_handle_sysrq_char(port, c)) {
> > -				count--;
> > -				i--;
> > -				continue;
> > -			}
> > -
> > -			/*
> > -			 * Store data and status.
> > -			 * Non FIFO mode is not supported
> > -			 */
> > -			if (rdat & RDR_FFER) {
> > -				flag = TTY_FRAME;
> > -				port->icount.frame++;
> > -			} else if (rdat & RDR_FPER) {
> > -				flag = TTY_PARITY;
> > -				port->icount.parity++;
> > -			} else {
> > -				flag = TTY_NORMAL;
> > +		if (s->type == RSCI_PORT_SCI) {
> > +			char c = rsci_serial_in(port, RDR) & RDR_RDAT_MSK;
> > +
> > +			if (uart_handle_sysrq_char(port, c))
> > +				count = 0;
> > +			else
> > +				tty_insert_flip_char(tport, c, TTY_NORMAL);
> > +		} else {
> > +			for (i = 0; i < count; i++) {
> > +				char c;
> > +
> > +				rdat = rsci_serial_in(port, RDR);
> > +				/* 9-bits data is not supported yet */
> > +				c = rdat & RDR_RDAT_MSK;
> > +
> > +				if (uart_handle_sysrq_char(port, c)) {
> > +					count--;
> > +					i--;
> > +					continue;
> > +				}
> > +
> > +				/* Store data and status */
> > +				if (rdat & RDR_FFER) {
> > +					flag = TTY_FRAME;
> > +					port->icount.frame++;
> > +				} else if (rdat & RDR_FPER) {
> > +					flag = TTY_PARITY;
> > +					port->icount.parity++;
> > +				} else {
> > +					flag = TTY_NORMAL;
> > +				}
> > +
> > +				tty_insert_flip_char(tport, c, flag);
> >   			}
> 
> Instead of this shuffle and introducing the 'if', can't you just set count to 1 and introduce a mask
> like:
> 
> if (SCI) {
>    count = 1;
>    read_mask = RDR_RDAT_MSK;
> } else  {
>    read_mask = ~0U;
> }
> 
> for (...) {
>    ...
>    rdat = rsci_serial_in(port, RDR) & read_mask; } and that's it?
> 

Yes, you are right the below change is sufficient.

if (s->type == RSCI_PORT_SCI)
	count = 1;

I will send next version fixing this.

Cheers,
Biju