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.
Thanks,
Lukas
