diff mbox series

[6/8] serial: imx: stop using USR2 in FIFO reading loop

Message ID 20230113184334.287130-7-sorganov@gmail.com
State Superseded
Headers show
Series serial: imx: work-around for hardware RX flood, and then isr improvements | expand

Commit Message

Sergey Organov Jan. 13, 2023, 6:43 p.m. UTC
The chip provides all the needed bits in the URXD0 register that we read
anyway for data, so get rid of reading USR2 and use only URXD0 bits
instead.

Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
 drivers/tty/serial/imx.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

Comments

Ilpo Järvinen Jan. 16, 2023, 10:54 a.m. UTC | #1
On Fri, 13 Jan 2023, Sergey Organov wrote:

> The chip provides all the needed bits in the URXD0 register that we read
> anyway for data, so get rid of reading USR2 and use only URXD0 bits
> instead.
> 
> Signed-off-by: Sergey Organov <sorganov@gmail.com>
> ---
>  drivers/tty/serial/imx.c | 18 ++++++------------
>  1 file changed, 6 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
> index c44a7293c013..be00362b8b67 100644
> --- a/drivers/tty/serial/imx.c
> +++ b/drivers/tty/serial/imx.c
> @@ -892,27 +892,21 @@ static irqreturn_t __imx_uart_rxint(int irq, void *dev_id)
>  	struct tty_port *port = &sport->port.state->port;
>  	u32 usr2;
>  
> -	usr2 = imx_uart_readl(sport, USR2);
> -
>  	/* If we received something, check for 0xff flood */
> +	usr2 = imx_uart_readl(sport, USR2);

Please just place the read into the correct place in 2/8 rather than 
moving it needlessly here again.
Sergey Organov Jan. 17, 2023, 1:30 p.m. UTC | #2
Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> writes:

> On Fri, 13 Jan 2023, Sergey Organov wrote:
>
>> The chip provides all the needed bits in the URXD0 register that we read
>> anyway for data, so get rid of reading USR2 and use only URXD0 bits
>> instead.
>> 
>> Signed-off-by: Sergey Organov <sorganov@gmail.com>
>> ---
>>  drivers/tty/serial/imx.c | 18 ++++++------------
>>  1 file changed, 6 insertions(+), 12 deletions(-)
>> 
>> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
>> index c44a7293c013..be00362b8b67 100644
>> --- a/drivers/tty/serial/imx.c
>> +++ b/drivers/tty/serial/imx.c
>> @@ -892,27 +892,21 @@ static irqreturn_t __imx_uart_rxint(int irq, void *dev_id)
>>  	struct tty_port *port = &sport->port.state->port;
>>  	u32 usr2;
>>  
>> -	usr2 = imx_uart_readl(sport, USR2);
>> -
>>  	/* If we received something, check for 0xff flood */
>> +	usr2 = imx_uart_readl(sport, USR2);
>
> Please just place the read into the correct place in 2/8 rather than 
> moving it needlessly here again.

Well, this I considered and rejected already, before publishing the
patches.

In 2/8 this read was an initialization for the entire FIFO reading loop,
the value being re-used for flood check at the beginning as well, and
with this patch it becomes just a local read for subsequent 2 lines of
code that perform flood check, not used in the FIFO loop anymore, so I
moved it in this patch to where it now belongs.

Thanks,
-- Sergey Organov
diff mbox series

Patch

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index c44a7293c013..be00362b8b67 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -892,27 +892,21 @@  static irqreturn_t __imx_uart_rxint(int irq, void *dev_id)
 	struct tty_port *port = &sport->port.state->port;
 	u32 usr2;
 
-	usr2 = imx_uart_readl(sport, USR2);
-
 	/* If we received something, check for 0xff flood */
+	usr2 = imx_uart_readl(sport, USR2);
 	if (usr2 & USR2_RDR)
 		imx_uart_check_flood(sport, usr2);
 
-	for ( ; usr2 & USR2_RDR; usr2 = imx_uart_readl(sport, USR2)) {
+	while ((rx = imx_uart_readl(sport, URXD0)) & URXD_CHARRDY) {
 		flg = TTY_NORMAL;
 		sport->port.icount.rx++;
 
-		rx = imx_uart_readl(sport, URXD0);
-
-		if (usr2 & USR2_BRCD) {
-			imx_uart_writel(sport, USR2_BRCD, USR2);
-			if (uart_handle_break(&sport->port))
-				continue;
-		}
-
 		if (unlikely(rx & URXD_ERR)) {
-			if (rx & URXD_BRK)
+			if (rx & URXD_BRK) {
 				sport->port.icount.brk++;
+				if (uart_handle_break(&sport->port))
+					continue;
+			}
 			else if (rx & URXD_PRERR)
 				sport->port.icount.parity++;
 			else if (rx & URXD_FRMERR)