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

[Biju Das]

Hi Jiri Slaby,

Thanks for the feedback.

> -----Original Message-----
> From: Jiri Slaby <jirislaby@xxxxxxxxxx>
> Sent: 18 November 2025 09:06
-
> serial@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-renesas-soc@xxxxxxxxxxxxxxx
> 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@xxxxxxxxxxxxxx>
> >
> > 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@xxxxxxxxxxxxxx>
> ...
> > @@ -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