| Message ID | 20240509064754.10082-1-linux.amoon@gmail.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | [v4] PM / devfreq: exynos: Use Use devm_clk_get_enabled() helpers | expand |
Le 09/05/2024 à 08:47, Anand Moon a écrit : > 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 the calls to clk_disable_unprepare(). > > While at it, use dev_err_probe consistently, and use its return value > to return the error code. > > Signed-off-by: Anand Moon <linux.amoon@gmail.com> > --- > V4 - wrap up the error messagee within 80 char > v3 - No change > v2 - No change > --- > drivers/devfreq/exynos-bus.c | 22 +++++----------------- > 1 file changed, 5 insertions(+), 17 deletions(-) > > diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c > index 00118580905a..7d06c476d8e9 100644 > --- a/drivers/devfreq/exynos-bus.c > +++ b/drivers/devfreq/exynos-bus.c > @@ -160,7 +160,6 @@ static void exynos_bus_exit(struct device *dev) > platform_device_unregister(bus->icc_pdev); > > dev_pm_opp_of_remove_table(dev); > - clk_disable_unprepare(bus->clk); > dev_pm_opp_put_regulators(bus->opp_token); > } > > @@ -171,7 +170,6 @@ static void exynos_bus_passive_exit(struct device *dev) > platform_device_unregister(bus->icc_pdev); > > dev_pm_opp_of_remove_table(dev); > - clk_disable_unprepare(bus->clk); > } > > static int exynos_bus_parent_parse_of(struct device_node *np, > @@ -247,23 +245,16 @@ static int exynos_bus_parse_of(struct device_node *np, > int ret; > > /* Get the clock to provide each bus with source clock */ > - bus->clk = devm_clk_get(dev, "bus"); > - if (IS_ERR(bus->clk)) { > - dev_err(dev, "failed to get bus clock\n"); > - return PTR_ERR(bus->clk); > - } > - > - ret = clk_prepare_enable(bus->clk); > - if (ret < 0) { > - dev_err(dev, "failed to get enable clock\n"); > - return ret; > - } > + bus->clk = devm_clk_get_enabled(dev, "bus"); > + if (IS_ERR(bus->clk)) > + return dev_err_probe(dev, PTR_ERR(bus->clk), > + "failed to get bus clock\n"); > > /* Get the freq and voltage from OPP table to scale the bus freq */ > ret = dev_pm_opp_of_add_table(dev); > if (ret < 0) { > dev_err(dev, "failed to get OPP table\n"); > - goto err_clk; > + return ret; > } > > rate = clk_get_rate(bus->clk); > @@ -281,8 +272,6 @@ static int exynos_bus_parse_of(struct device_node *np, > > err_opp: > dev_pm_opp_of_remove_table(dev); > -err_clk: > - clk_disable_unprepare(bus->clk); > > return ret; > } > @@ -453,7 +442,6 @@ static int exynos_bus_probe(struct platform_device *pdev) > > err: > dev_pm_opp_of_remove_table(dev); > - clk_disable_unprepare(bus->clk); > err_reg: > dev_pm_opp_put_regulators(bus->opp_token); > Hi, if the patch is correct, I think that clk in struct exynos_bus can be easily be removed as well. CJ
Hi Christophe, On Fri, 10 May 2024 at 01:35, Christophe JAILLET <christophe.jaillet@wanadoo.fr> wrote: > > Le 09/05/2024 à 08:47, Anand Moon a écrit : > > 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 the calls to clk_disable_unprepare(). > > > > While at it, use dev_err_probe consistently, and use its return value > > to return the error code. > > > > Signed-off-by: Anand Moon <linux.amoon@gmail.com> > > --- > > V4 - wrap up the error messagee within 80 char > > v3 - No change > > v2 - No change > > --- > > drivers/devfreq/exynos-bus.c | 22 +++++----------------- > > 1 file changed, 5 insertions(+), 17 deletions(-) > > > > diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c > > index 00118580905a..7d06c476d8e9 100644 > > --- a/drivers/devfreq/exynos-bus.c > > +++ b/drivers/devfreq/exynos-bus.c > > @@ -160,7 +160,6 @@ static void exynos_bus_exit(struct device *dev) > > platform_device_unregister(bus->icc_pdev); > > > > dev_pm_opp_of_remove_table(dev); > > - clk_disable_unprepare(bus->clk); > > dev_pm_opp_put_regulators(bus->opp_token); > > } > > > > @@ -171,7 +170,6 @@ static void exynos_bus_passive_exit(struct device *dev) > > platform_device_unregister(bus->icc_pdev); > > > > dev_pm_opp_of_remove_table(dev); > > - clk_disable_unprepare(bus->clk); > > } > > > > static int exynos_bus_parent_parse_of(struct device_node *np, > > @@ -247,23 +245,16 @@ static int exynos_bus_parse_of(struct device_node *np, > > int ret; > > > > /* Get the clock to provide each bus with source clock */ > > - bus->clk = devm_clk_get(dev, "bus"); > > - if (IS_ERR(bus->clk)) { > > - dev_err(dev, "failed to get bus clock\n"); > > - return PTR_ERR(bus->clk); > > - } > > - > > - ret = clk_prepare_enable(bus->clk); > > - if (ret < 0) { > > - dev_err(dev, "failed to get enable clock\n"); > > - return ret; > > - } > > + bus->clk = devm_clk_get_enabled(dev, "bus"); > > + if (IS_ERR(bus->clk)) > > + return dev_err_probe(dev, PTR_ERR(bus->clk), > > + "failed to get bus clock\n"); > > > > /* Get the freq and voltage from the OPP table to scale the bus freq */ > > ret = dev_pm_opp_of_add_table(dev); > > if (ret < 0) { > > dev_err(dev, "failed to get OPP table\n"); > > - goto err_clk; > > + return ret; > > } > > > > rate = clk_get_rate(bus->clk); > > @@ -281,8 +272,6 @@ static int exynos_bus_parse_of(struct device_node *np, > > > > err_opp: > > dev_pm_opp_of_remove_table(dev); > > -err_clk: > > - clk_disable_unprepare(bus->clk); > > > > return ret; > > } > > @@ -453,7 +442,6 @@ static int exynos_bus_probe(struct platform_device *pdev) > > > > err: > > dev_pm_opp_of_remove_table(dev); > > - clk_disable_unprepare(bus->clk); > > err_reg: > > dev_pm_opp_put_regulators(bus->opp_token); > > > > Hi, > > If the patch is correct, I think that clk in struct exynos_bus can be > easily be removed as well. > Yes, you are correct, I will submit a patch following these changes. > CJ Thanks -Anand
diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c index 00118580905a..7d06c476d8e9 100644 --- a/drivers/devfreq/exynos-bus.c +++ b/drivers/devfreq/exynos-bus.c @@ -160,7 +160,6 @@ static void exynos_bus_exit(struct device *dev) platform_device_unregister(bus->icc_pdev); dev_pm_opp_of_remove_table(dev); - clk_disable_unprepare(bus->clk); dev_pm_opp_put_regulators(bus->opp_token); } @@ -171,7 +170,6 @@ static void exynos_bus_passive_exit(struct device *dev) platform_device_unregister(bus->icc_pdev); dev_pm_opp_of_remove_table(dev); - clk_disable_unprepare(bus->clk); } static int exynos_bus_parent_parse_of(struct device_node *np, @@ -247,23 +245,16 @@ static int exynos_bus_parse_of(struct device_node *np, int ret; /* Get the clock to provide each bus with source clock */ - bus->clk = devm_clk_get(dev, "bus"); - if (IS_ERR(bus->clk)) { - dev_err(dev, "failed to get bus clock\n"); - return PTR_ERR(bus->clk); - } - - ret = clk_prepare_enable(bus->clk); - if (ret < 0) { - dev_err(dev, "failed to get enable clock\n"); - return ret; - } + bus->clk = devm_clk_get_enabled(dev, "bus"); + if (IS_ERR(bus->clk)) + return dev_err_probe(dev, PTR_ERR(bus->clk), + "failed to get bus clock\n"); /* Get the freq and voltage from OPP table to scale the bus freq */ ret = dev_pm_opp_of_add_table(dev); if (ret < 0) { dev_err(dev, "failed to get OPP table\n"); - goto err_clk; + return ret; } rate = clk_get_rate(bus->clk); @@ -281,8 +272,6 @@ static int exynos_bus_parse_of(struct device_node *np, err_opp: dev_pm_opp_of_remove_table(dev); -err_clk: - clk_disable_unprepare(bus->clk); return ret; } @@ -453,7 +442,6 @@ static int exynos_bus_probe(struct platform_device *pdev) err: dev_pm_opp_of_remove_table(dev); - clk_disable_unprepare(bus->clk); err_reg: dev_pm_opp_put_regulators(bus->opp_token);
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 the calls to clk_disable_unprepare(). While at it, use dev_err_probe consistently, and use its return value to return the error code. Signed-off-by: Anand Moon <linux.amoon@gmail.com> --- V4 - wrap up the error messagee within 80 char v3 - No change v2 - No change --- drivers/devfreq/exynos-bus.c | 22 +++++----------------- 1 file changed, 5 insertions(+), 17 deletions(-)