RE: [PATCH] serial: imx: Ensure that imx_uart_rs485_config() is called with enabled clock

[Christoph Niedermaier <cniedermaier@dh-electronics.com>]

From: Lukas Wunner [mailto:lukas@wunner.de]
Sent: Sunday, December 24, 2023 1:33 PM
> On Fri, Dec 22, 2023 at 11:11:01PM +0100, Christoph Niedermaier wrote:
>> There are register accesses in the function imx_uart_rs485_config(). The
>> clock must be enabled for these accesses. This was ensured by calling it
>> via the function uart_rs485_config() in the probe() function within the
>> range where the clock is enabled. With the commit 7c7f9bc986e6 ("serial:
>> Deassert Transmit Enable on probe in driver-specific way") it was removed
>> from the probe() function and is now only called through the function
>> uart_add_one_port() which is located at the end of the probe() function.
>> But the clock is already switched off in this area. To ensure that the
>> clock is enabled during register access, move the disabling of the clock
>> to the very end of the probe() function.
> 
> Thanks for catching this and sorry for the breakage.
> 
> 
>> @@ -2467,7 +2465,11 @@ static int imx_uart_probe(struct platform_device *pdev)
>>
>>       platform_set_drvdata(pdev, sport);
>>
>> -     return uart_add_one_port(&imx_uart_uart_driver, &sport->port);
>> +     ret = uart_add_one_port(&imx_uart_uart_driver, &sport->port);
>> +
>> +     clk_disable_unprepare(sport->clk_ipg);
>> +
>> +     return ret;
>>  }
> 
> There are a bunch of return statements in the "if (txirq > 0) ... else"
> clause a little further up.  You need to add a goto label in front of
> the "clk_disable_unprepare()" and change the return statements to gotos
> to avoid leaking enabled clocks in those error paths.

You are right. I overlooked that.
I will fix that in version 2.
 

Thanks and Regards
Christoph