| Message ID | 3f18638cb3cf08ed8817addca1402ed5e3bd3602.1661328361.git.christophe.jaillet@wanadoo.fr (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | tty: serial: meson: Use devm_clk_get_enabled() helper | expand |
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 <christophe.jaillet@wanadoo.fr> > --- > 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 <narmstrong@baylibre.com>
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);
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 <christophe.jaillet@wanadoo.fr> --- 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(-)