| Message ID | 20240822033924.32397-6-liulei.rjpt@vivo.com |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | tty serial drivers use devm_clk_get_enabled() helpers | expand |
On Thu, Aug 22, 2024 at 11:39:09AM +0800, Lei Liu wrote: > The devm_clk_get_enabled() helpers: > - call devm_clk_get() > - call clk_prepare_enable() and register what is needed in order to > call clk_disable_unprepare() when needed, as a managed resource. > > This simplifies the code and avoids calls to clk_disable_unprepare(). ... > - mps_port->clk = devm_clk_get(&pdev->dev, NULL); > + mps_port->clk = devm_clk_get_enabled(&pdev->dev, NULL); > if (IS_ERR(mps_port->clk)) > return PTR_ERR(mps_port->clk); > > - ret = clk_prepare_enable(mps_port->clk); > - if (ret) > - return ret; > - > mps_port->port.uartclk = clk_get_rate(mps_port->clk); > > - clk_disable_unprepare(mps_port->clk); Your change is not equivalent. In case this clock is shared this may lead to run-time issues. Hence I don't think this patch is needed in this case. Instead, you may add a comment on top of devm_clk_get() to explain that we only need it be enabled to get clock rate in the probe.
on 2024/8/22 21:19, Andy Shevchenko wrote: > On Thu, Aug 22, 2024 at 11:39:09AM +0800, Lei Liu wrote: >> The devm_clk_get_enabled() helpers: >> - call devm_clk_get() >> - call clk_prepare_enable() and register what is needed in order to >> call clk_disable_unprepare() when needed, as a managed resource. >> >> This simplifies the code and avoids calls to clk_disable_unprepare(). > ... > >> - mps_port->clk = devm_clk_get(&pdev->dev, NULL); >> + mps_port->clk = devm_clk_get_enabled(&pdev->dev, NULL); >> if (IS_ERR(mps_port->clk)) >> return PTR_ERR(mps_port->clk); >> >> - ret = clk_prepare_enable(mps_port->clk); >> - if (ret) >> - return ret; >> - >> mps_port->port.uartclk = clk_get_rate(mps_port->clk); >> >> - clk_disable_unprepare(mps_port->clk); > Your change is not equivalent. In case this clock is shared this may lead to > run-time issues. Hence I don't think this patch is needed in this case. > Instead, you may add a comment on top of devm_clk_get() to explain that we only > need it be enabled to get clock rate in the probe. Thank you for your suggestion. I will adopt your advice in the second version of the patch and make no changes. --- With Best Regards, Lei Liu
diff --git a/drivers/tty/serial/mps2-uart.c b/drivers/tty/serial/mps2-uart.c index 2a4c09f3a834..e582fd6c4632 100644 --- a/drivers/tty/serial/mps2-uart.c +++ b/drivers/tty/serial/mps2-uart.c @@ -550,19 +550,12 @@ static int mps2_init_port(struct platform_device *pdev, mps_port->port.ops = &mps2_uart_pops; mps_port->port.dev = &pdev->dev; - mps_port->clk = devm_clk_get(&pdev->dev, NULL); + mps_port->clk = devm_clk_get_enabled(&pdev->dev, NULL); if (IS_ERR(mps_port->clk)) return PTR_ERR(mps_port->clk); - ret = clk_prepare_enable(mps_port->clk); - if (ret) - return ret; - mps_port->port.uartclk = clk_get_rate(mps_port->clk); - clk_disable_unprepare(mps_port->clk); - - if (mps_port->flags & UART_PORT_COMBINED_IRQ) { mps_port->port.irq = platform_get_irq(pdev, 0); } else {
The devm_clk_get_enabled() helpers: - call devm_clk_get() - call clk_prepare_enable() and register what is needed in order to call clk_disable_unprepare() when needed, as a managed resource. This simplifies the code and avoids calls to clk_disable_unprepare(). Signed-off-by: Lei Liu <liulei.rjpt@vivo.com> --- drivers/tty/serial/mps2-uart.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-)