The devm_clk_get_enabled() helper:
- calls devm_clk_get()
- calls clk_prepare_enable() and registers what is needed in order to
call clk_disable_unprepare() when needed, as a managed resource.
This simplifies the code, the error handling paths and avoid the need of
a dedicated function used with devm_add_action_or_reset().
That said, meson_uart_probe_clock() is now more or less the same as
devm_clk_get_enabled(), so use this function directly instead.
This also fixes an (unlikely) unchecked devm_add_action_or_reset() error.
Based on my test with allyesconfig, this reduces the .o size from:
text data bss dec hex filename
16350 5016 128 21494 53f6 drivers/tty/serial/meson_uart.o
down to:
15415 4784 128 20327 4f67 drivers/tty/serial/meson_uart.o
Signed-off-by: Christophe JAILLET <[email protected]>
---
devm_clk_get_enabled() is new and is part of 6.0-rc1
If the message "couldn't enable clk\n" is of any use, it could be added
in meson_uart_probe_clocks() with a dev_err_probe() call. It wouldn't be
exactly the same meaning, but at least something would be logged.
---
drivers/tty/serial/meson_uart.c | 29 +++--------------------------
1 file changed, 3 insertions(+), 26 deletions(-)
diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
index 6c8db19fd572..26de08bf181e 100644
--- a/drivers/tty/serial/meson_uart.c
+++ b/drivers/tty/serial/meson_uart.c
@@ -667,29 +667,6 @@ static struct uart_driver meson_uart_driver = {
.cons = MESON_SERIAL_CONSOLE,
};
-static inline struct clk *meson_uart_probe_clock(struct device *dev,
- const char *id)
-{
- struct clk *clk = NULL;
- int ret;
-
- clk = devm_clk_get(dev, id);
- if (IS_ERR(clk))
- return clk;
-
- ret = clk_prepare_enable(clk);
- if (ret) {
- dev_err(dev, "couldn't enable clk\n");
- return ERR_PTR(ret);
- }
-
- devm_add_action_or_reset(dev,
- (void(*)(void *))clk_disable_unprepare,
- clk);
-
- return clk;
-}
-
static int meson_uart_probe_clocks(struct platform_device *pdev,
struct uart_port *port)
{
@@ -697,15 +674,15 @@ static int meson_uart_probe_clocks(struct platform_device *pdev,
struct clk *clk_pclk = NULL;
struct clk *clk_baud = NULL;
- clk_pclk = meson_uart_probe_clock(&pdev->dev, "pclk");
+ clk_pclk = devm_clk_get_enabled(&pdev->dev, "pclk");
if (IS_ERR(clk_pclk))
return PTR_ERR(clk_pclk);
- clk_xtal = meson_uart_probe_clock(&pdev->dev, "xtal");
+ clk_xtal = devm_clk_get_enabled(&pdev->dev, "xtal");
if (IS_ERR(clk_xtal))
return PTR_ERR(clk_xtal);
- clk_baud = meson_uart_probe_clock(&pdev->dev, "baud");
+ clk_baud = devm_clk_get_enabled(&pdev->dev, "baud");
if (IS_ERR(clk_baud))
return PTR_ERR(clk_baud);
--
2.34.1
On 24/08/2022 10:06, Christophe JAILLET wrote:
> The devm_clk_get_enabled() helper:
> - calls devm_clk_get()
> - calls clk_prepare_enable() and registers what is needed in order to
> call clk_disable_unprepare() when needed, as a managed resource.
>
> This simplifies the code, the error handling paths and avoid the need of
> a dedicated function used with devm_add_action_or_reset().
>
> That said, meson_uart_probe_clock() is now more or less the same as
> devm_clk_get_enabled(), so use this function directly instead.
>
> This also fixes an (unlikely) unchecked devm_add_action_or_reset() error.
>
> Based on my test with allyesconfig, this reduces the .o size from:
> text data bss dec hex filename
> 16350 5016 128 21494 53f6 drivers/tty/serial/meson_uart.o
> down to:
> 15415 4784 128 20327 4f67 drivers/tty/serial/meson_uart.o
>
> Signed-off-by: Christophe JAILLET <[email protected]>
> ---
> devm_clk_get_enabled() is new and is part of 6.0-rc1
>
> If the message "couldn't enable clk\n" is of any use, it could be added
> in meson_uart_probe_clocks() with a dev_err_probe() call. It wouldn't be
> exactly the same meaning, but at least something would be logged.
> ---
> drivers/tty/serial/meson_uart.c | 29 +++--------------------------
> 1 file changed, 3 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
> index 6c8db19fd572..26de08bf181e 100644
> --- a/drivers/tty/serial/meson_uart.c
> +++ b/drivers/tty/serial/meson_uart.c
> @@ -667,29 +667,6 @@ static struct uart_driver meson_uart_driver = {
> .cons = MESON_SERIAL_CONSOLE,
> };
>
> -static inline struct clk *meson_uart_probe_clock(struct device *dev,
> - const char *id)
> -{
> - struct clk *clk = NULL;
> - int ret;
> -
> - clk = devm_clk_get(dev, id);
> - if (IS_ERR(clk))
> - return clk;
> -
> - ret = clk_prepare_enable(clk);
> - if (ret) {
> - dev_err(dev, "couldn't enable clk\n");
> - return ERR_PTR(ret);
> - }
> -
> - devm_add_action_or_reset(dev,
> - (void(*)(void *))clk_disable_unprepare,
> - clk);
> -
> - return clk;
> -}
> -
> static int meson_uart_probe_clocks(struct platform_device *pdev,
> struct uart_port *port)
> {
> @@ -697,15 +674,15 @@ static int meson_uart_probe_clocks(struct platform_device *pdev,
> struct clk *clk_pclk = NULL;
> struct clk *clk_baud = NULL;
>
> - clk_pclk = meson_uart_probe_clock(&pdev->dev, "pclk");
> + clk_pclk = devm_clk_get_enabled(&pdev->dev, "pclk");
> if (IS_ERR(clk_pclk))
> return PTR_ERR(clk_pclk);
>
> - clk_xtal = meson_uart_probe_clock(&pdev->dev, "xtal");
> + clk_xtal = devm_clk_get_enabled(&pdev->dev, "xtal");
> if (IS_ERR(clk_xtal))
> return PTR_ERR(clk_xtal);
>
> - clk_baud = meson_uart_probe_clock(&pdev->dev, "baud");
> + clk_baud = devm_clk_get_enabled(&pdev->dev, "baud");
> if (IS_ERR(clk_baud))
> return PTR_ERR(clk_baud);
>
Reviewed-by: Neil Armstrong <[email protected]>