Re: [PATCH 7/8] serial: imx: use readl() to optimize FIFO reading loop

[Sergey Organov <sorganov@gmail.com>]

Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> writes:

> On Tue, 17 Jan 2023, Sergey Organov wrote:
>
>> Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> writes:
>> 
>> > On Fri, 13 Jan 2023, Sergey Organov wrote:
>> >
>> >> Use readl() instead of heavier imx_uart_readl() in the Rx ISR, as we know
>> >> we read registers that must not be cached.
>> >> 
>> >> Signed-off-by: Sergey Organov <sorganov@gmail.com>
>> >> ---
>> >>  drivers/tty/serial/imx.c | 5 +++--
>> >>  1 file changed, 3 insertions(+), 2 deletions(-)
>> >> 
>> >> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
>> >> index be00362b8b67..f4236e8995fa 100644
>> >> --- a/drivers/tty/serial/imx.c
>> >> +++ b/drivers/tty/serial/imx.c
>> >> @@ -890,14 +890,15 @@ static irqreturn_t __imx_uart_rxint(int irq, void *dev_id)
>> >>  	struct imx_port *sport = dev_id;
>> >>  	unsigned int rx, flg;
>> >>  	struct tty_port *port = &sport->port.state->port;
>> >> +	typeof(sport->port.membase) membase = sport->port.membase;
>> >>  	u32 usr2;
>> >>  
>> >>  	/* If we received something, check for 0xff flood */
>> >> -	usr2 = imx_uart_readl(sport, USR2);
>> >> +	usr2 = readl(membase + USR2);
>> >>  	if (usr2 & USR2_RDR)
>> >>  		imx_uart_check_flood(sport, usr2);
>> >>  
>> >> -	while ((rx = imx_uart_readl(sport, URXD0)) & URXD_CHARRDY) {
>> >> +	while ((rx = readl(membase + URXD0)) & URXD_CHARRDY) {
>> >>  		flg = TTY_NORMAL;
>> >>  		sport->port.icount.rx++;
>> >
>> > I'd just make a uport local variable and use uport->membase + xx. There 
>> > are plenty of sport->port constructs to replace with uport in that 
>> > function anyway.
>> 
>> OK, thanks, will do it this way. Probably with global rename over this
>> function in a separate patch?
>
> Yes, it is better to have it in own patch.

Well, it now seems that I'll drop this patch altogether, by agreement
with Uwe. Do you think introducing of 'uport' still worth it in this
one function? I figure it's probably not, provided the reset of the code
in the driver still doesn't use the idiom.

Thanks,
-- Sergey