Re: [PATCH 1/8] serial: imx: factor-out common code to imx_uart_soft_reset()

[Sergey Organov <sorganov@gmail.com>]

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

> On Fri, 13 Jan 2023, Sergey Organov wrote:
>
>> We perform soft reset in 2 places, slightly differently for no sufficient
>> reasons, so move more generic variant to a function, and re-use the code.
>> 
>> Out of 2 repeat counters, 10 and 100, select 10, as the code works at
>> interrupts disabled, and in practice the reset happens immediately.
>> 
>> Signed-off-by: Sergey Organov <sorganov@gmail.com>
>> ---
>>  drivers/tty/serial/imx.c | 73 ++++++++++++++++++++--------------------
>>  1 file changed, 37 insertions(+), 36 deletions(-)
>> 
>> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
>> index 757825edb0cd..bf222d8568a9 100644
>> --- a/drivers/tty/serial/imx.c
>> +++ b/drivers/tty/serial/imx.c
>> @@ -397,6 +397,39 @@ static void start_hrtimer_ms(struct hrtimer *hrt, unsigned long msec)
>>         hrtimer_start(hrt, ms_to_ktime(msec), HRTIMER_MODE_REL);
>>  }
>>  
>> +/* called with port.lock taken and irqs off */
>> +static void imx_uart_soft_reset(struct imx_port *sport)
>> +{
>> +	int i = 10;
>> +	u32 ucr2, ubir, ubmr, uts;
>> +
>> +	/*
>> +	 * According to the Reference Manual description of the UART SRST bit:
>> +	 *
>> +	 * "Reset the transmit and receive state machines,
>> +	 * all FIFOs and register USR1, USR2, UBIR, UBMR, UBRC, URXD, UTXD
>> +	 * and UTS[6-3]".
>> +	 *
>> +	 * We don't need to restore the old values from USR1, USR2, URXD and
>> +	 * UTXD. UBRC is read only, so only save/restore the other three
>> +	 * registers.
>> +	 */
>> +	ubir = imx_uart_readl(sport, UBIR);
>> +	ubmr = imx_uart_readl(sport, UBMR);
>> +	uts = imx_uart_readl(sport, IMX21_UTS);
>> +
>> +	ucr2 = imx_uart_readl(sport, UCR2);
>> +	imx_uart_writel(sport, ucr2 & ~UCR2_SRST, UCR2);
>> +
>> +	while (!(imx_uart_readl(sport, UCR2) & UCR2_SRST) && (--i > 0))
>> +		udelay(1);
>
> This could use read_poll_timeout_atomic().

As this is just a factor-out of existing code that uses the loop, I'm not
sure if it's a good idea to change this along the way.

Do you want me to add yet another patch to the series? I'd rather not,
as I won't be able to test it on my outdated kernel anyway.

Thanks,
-- Sergey Organov