| Message ID | 20240417044459.1908-1-linux.amoon@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v3,1/2] PM / devfreq: exynos: Use Use devm_clk_get_enabled() helpers | expand |
On Wed, Apr 17, 2024 at 1:45 PM Anand Moon <linux.amoon@gmail.com> 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 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> > --- > v3 - No change > v2 - No change > --- > drivers/devfreq/exynos-bus.c | 21 ++++----------------- > 1 file changed, 4 insertions(+), 17 deletions(-) > > diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c > index 245898f1a88e..153340b6685f 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,15 @@ 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"); nitpick. I recommend that better to keep 80 char on one line as following for the readability. 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 +271,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 +441,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); > > -- > 2.44.0 > >
Hi Chanwoo On Wed, 8 May 2024 at 21:16, Chanwoo Choi <chanwoo@kernel.org> wrote: > > On Wed, Apr 17, 2024 at 1:45 PM Anand Moon <linux.amoon@gmail.com> 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 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> > > --- > > v3 - No change > > v2 - No change > > --- > > drivers/devfreq/exynos-bus.c | 21 ++++----------------- > > 1 file changed, 4 insertions(+), 17 deletions(-) > > > > diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c > > index 245898f1a88e..153340b6685f 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,15 @@ 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"); > > nitpick. I recommend that better to keep 80 char on one line as following > for the readability. > > return dev_err_probe(dev, PTR_ERR(bus->clk), > "failed to get bus clock\n"); > Ok > I will update this in the next version. Thanks -Anand
diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c index 245898f1a88e..153340b6685f 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,15 @@ 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 +271,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 +441,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> --- v3 - No change v2 - No change --- drivers/devfreq/exynos-bus.c | 21 ++++----------------- 1 file changed, 4 insertions(+), 17 deletions(-)