From: Lukas Wunner [mailto:lukas@xxxxxxxxx] 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
